Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NotificationConfig permission to edit only own needs account-id placeholder in permission/ability system #2847

Closed
sleidig opened this issue Feb 11, 2025 · 2 comments · Fixed by Aam-Digital/replication-backend#171 or #2848
Assignees
Milestone

Comments

@sleidig
Copy link
Member

sleidig commented Feb 11, 2025

original issue:

"Activate Notifications" button in toolbar section should close the popup (currently stay open after navigating to settings page)

After further investigation, the change detection works correctly but there can be problems due to permissions: The user needs to have "manage" permission on the own NotificationConfig entity. If that is not given, then the config cannot be synced and the system will remain in state without "activated" notifications.


For privacy and data management, each user should only have access to the own NotificationConfig entity (i.e. the doc where "conditions": { "_id": { "$eq": "${user.id}" } }). However, while we do have a placeholder for ${user.name} that reference the current user's linked profile entityId, we currently do not have the account (keycloak) ID available on the replication-backend's ability implementation yet.

https://github.com/Aam-Digital/replication-backend/blob/1090b71d0409596483a762c8a0965a327259ace9/src/auth/guards/jwt-cookie/jwt-cookie-strategy.service.ts#L30-L40
(and also JwtBearerStrategyService --> centralize and re-use code to map token to UserInfo object?)

@github-project-automation github-project-automation bot moved this to Triage / Analysis in All Tasks & Issues Feb 11, 2025
@sleidig sleidig changed the title "Activate Notifications" button in toolbar section should close the popup (currently stay open after navigating to settings page) NotificationConfig permission to edit only own needs account-id placeholder in permission/ability system Feb 11, 2025
@sleidig sleidig added this to the Notifications milestone Feb 11, 2025
@sleidig sleidig moved this from Triage / Analysis to Technical Review in All Tasks & Issues Feb 11, 2025
@sleidig
Copy link
Member Author

sleidig commented Feb 11, 2025

The default permissions should be:

    {
      "subject": "NotificationEvent",
      "action": "manage"
    },
    {
      "subject": "NotificationConfig",
      "action": "manage",
      "conditions": {
        "userId": "${user.id}"
      }
    },

@sleidig
Copy link
Member Author

sleidig commented Feb 11, 2025

@tomwwinter , could you review the two PRs (replication-backend + frontend) for this?

@sleidig sleidig self-assigned this Feb 11, 2025
sleidig added a commit to Aam-Digital/replication-backend that referenced this issue Feb 11, 2025
@github-project-automation github-project-automation bot moved this from Technical Review to Done in All Tasks & Issues Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
1 participant