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

Design fine-grained access control within record manager #202

Open
blcham opened this issue Aug 1, 2024 · 12 comments
Open

Design fine-grained access control within record manager #202

blcham opened this issue Aug 1, 2024 · 12 comments
Assignees

Comments

@blcham
Copy link

blcham commented Aug 1, 2024

One of main goals is to remove extensions SUPPLIER and OPERATOR.

  • explicit keycloak roles:
    • publishing records
    • rejecting records
    • complete records
    • delete/edit/view organization records
    • delete/edit/view all records
    • edit users
    • codelist import

Note:

  • internal authorization needs to save everything related to roles/role groups to the repository.
  • PUBLISH button is available only in extension SUPPLIER. (see e.g., 23ava-distribution/supplier-ava/docker-compose.yml EXTENSIONS: "${RECORD_MANAGER_EXTENSIONS:-supplier}")

Related to issue: #206

A/C:

  • fine-grained roles work, and we can compose roles into groups within keycloak
  • it is usable at least in a restricted but meaningful way in internal authorization
@palagdan
Copy link
Collaborator

palagdan commented Aug 1, 2024

@blcham

If I understand the task correctly, we can add two different groups for operators and suppliers, as shown in the picture below:

Image

Each group can have its own subgroups that inherit roles from the parent group. This allows us to create different subgroups with specific roles.

@blcham
Copy link
Author

blcham commented Aug 8, 2024

Open questions:

  • Should we show a group of roles in the RM-UI? Yes but maybe only for internal authorization

    • It makes sense, at least in the internal authorization case. Moreover, we will have at least some role names prepared with some initial semantics on how It could be used and redefined in the Keycloak. Note that the Inheritance of roles is working in groups within Keycloak.
      image
  • @palagdan Is it possible to retrieve role groups in the token?

  • How will we treat users not being in any institution?

@palagdan
Copy link
Collaborator

Yes, we can retrieve groups in the token, see https://stackoverflow.com/questions/56362197/keycloak-oidc-retrieve-user-groups-attributes

@blcham
Copy link
Author

blcham commented Aug 28, 2024

  1. ok, try to implement part of the code related to differences between supplier and operator, do it in a way it would be possible to merge with keycloak reconfiguration

  2. investigate how to assign in keycloak default roles and role groups. it should help u with 1)

@palagdan
Copy link
Collaborator

@blcham
I set up Keycloak with two groups (Operator and Supplier) and configured client scopes to include this information in the token. From the access token, I can retrieve this information. It seems to be working

Image

@palagdan
Copy link
Collaborator

palagdan commented Aug 28, 2024

@blcham
Regarding your second task, I have set up the groups with default roles. As shown in the picture, the supplier group has assigned roles to the user. I've only set Admin role myself.

We should discuss a list of roles for each group.

@blcham
Copy link
Author

blcham commented Aug 28, 2024

  • roles delete/edit/view should be 3 separate roles
  • there should be global prefix to this role i.e. record_manager_ or just rm_
  • please unify naming e.g. completing_... --> complete_records
  • btw you do not need to put it to export-realm.xml yet but need to mark down what to do first
  • FYI, RM should not do anything with role_groups beside visualizing it (rely on roles)

@palagdan
Copy link
Collaborator

@blcham

  1. I want to discuss what roles should I assign to each group.
  2. Should I change 'Roles' to 'Group' and display the Group name instead? Since a user can have multiple roles, it might be more effective to show the group to which the user is assigned.
    Image

@palagdan
Copy link
Collaborator

palagdan commented Sep 2, 2024

@blcham
We currently have the following roles:

  • ROLE_ADMIN
  • ROLE_USER
  • rm_complete_records
  • rm_delete_all_records
  • rm_delete_organizations_records
  • rm_edit_all_records
  • rm_edit_organizations_records
  • rm_edit_users
  • rm_import_codelist
  • rm_publish_records
  • rm_reject_records
  • rm_view_all_records
  • rm_view_organizations_records

If Keycloak saves personal information about users to the repository, why can't it save roles and groups as well? It doesn't make sense to handle this on the backend because, with every request, the system would need to check Keycloak for changes and then update the repository accordingly.

@palagdan
Copy link
Collaborator

palagdan commented Sep 3, 2024

@blcham

The Publish button is in the Operator extension (not Supplier). Can you explain what this button does?

Image

@blcham
Copy link
Author

blcham commented Sep 3, 2024

publish button imports all completed records from operator deployment to supplier deployment

rm_edit_organizations_records --> rm_edit_organization_records

rm_import_codelist --> rm_import_codelists

palagdan added a commit to kbss-cvut/record-manager that referenced this issue Sep 3, 2024
palagdan added a commit to kbss-cvut/record-manager that referenced this issue Sep 3, 2024
palagdan added a commit that referenced this issue Sep 3, 2024
palagdan added a commit that referenced this issue Sep 5, 2024
palagdan added a commit to kbss-cvut/record-manager that referenced this issue Sep 5, 2024
palagdan added a commit that referenced this issue Dec 6, 2024
palagdan added a commit that referenced this issue Dec 6, 2024
palagdan added a commit that referenced this issue Dec 6, 2024
palagdan added a commit that referenced this issue Dec 6, 2024
palagdan added a commit to kbss-cvut/record-manager that referenced this issue Dec 6, 2024
palagdan added a commit to kbss-cvut/record-manager that referenced this issue Dec 6, 2024
palagdan added a commit that referenced this issue Dec 6, 2024
palagdan added a commit that referenced this issue Dec 6, 2024
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
… exception if the provided role does not exist
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
http-nio-8080-exec-7] WARN  o.s.w.s.m.s.DefaultHandlerExceptionResolver - Resolved [org.springframework.http.converter.HttpMessageNotWritableException: Could not write JSON: Cannot invoke "cz.cvut.kbss.study.model.RoleGroup.getRoles()" because "this.roleGroup" is null]
blcham added a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
blcham pushed a commit to kbss-cvut/record-manager that referenced this issue Dec 10, 2024
palagdan added a commit to kbss-cvut/record-manager that referenced this issue Dec 11, 2024
palagdan added a commit to kbss-cvut/record-manager that referenced this issue Dec 11, 2024
palagdan added a commit to kbss-cvut/record-manager that referenced this issue Dec 11, 2024
palagdan added a commit to kbss-cvut/record-manager that referenced this issue Dec 11, 2024
palagdan added a commit to kbss-cvut/record-manager that referenced this issue Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants