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

[roles_manager] new module #9054

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

regisoc
Copy link
Contributor

@regisoc regisoc commented Feb 14, 2024

Brief summary of changes

Extract of #8929. Contains only the roles_manager module and other necessary items.

Roles are pre-configured sets of permissions automatically assigned when the user is given a new role. This feature allows managers/coordinators to assign permissions to users based on their roles in a study.

New:

  • A main table managing roles and their associated permissions.
  • If users are attributed to a role, changing permissions of that role will trigger the update of user permissions for all user having this role.
  • A tool fix_role_anomalies to fix anomalies detected between user/role/permission tables and *_rel. E.g. in case a role-permission is inserted but corresponding user-permissions are not.
  • Added tables: roles, user_role_rel, role_permission_rel.

Future changes:

  • should include change in the user_accounts module. This module mainly focuses changes on roles and role-permissions. Users that are already assigned to a role will have a permission update once these role permissions are updated. No user-role assignation is done through this module.
  • tests are present for Role lib but not for the role manager at the moment.
  • permission_categories makes no sens when Roles will be fully integrated. References should be removed and permission categories will become roles.

Testing instructions (if applicable)

Module:

  1. Apply patch SQL/New_patches/2024-02-13_roles_modules.sql.
  2. Go to LORIS front-end > Admin > Roles Manager.
  3. Use test plan.

Tool:

  1. Use with php tools/fix_role_anomalies.php, check that it matches was is needed in db.
  2. Use confirm option to perform the change in db, check that it was succesfully done.

Link(s) to related issue(s)

Linked to #7416

@regisoc regisoc self-assigned this Feb 14, 2024
@regisoc
Copy link
Contributor Author

regisoc commented Feb 14, 2024

@driusan should be good for review once automated tests are ok.

@regisoc regisoc requested a review from driusan February 14, 2024 14:18
@regisoc
Copy link
Contributor Author

regisoc commented Feb 14, 2024

Also, I thought about what we discuss on the User::hasPermission method change. Not sure this change is necessary because the whole role-user-permission system is dynamic.
When checking if a user has a permission, the call UserPermission::hasPermission checks the user_perm_rel table.
We should not have to check all roles for that user because the user_perm_rel table should be up-to-date when roles are modified, meaning changing role-permission rel when users are affected to this role will trigger a user-permission check.
Maybe I am missing something, let me know.

@regisoc regisoc added this to the 26.0.0 milestone Feb 23, 2024
@ridz1208 ridz1208 removed this from the 26.0.0 milestone Jun 14, 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 this pull request may close these issues.

2 participants