pkp-lib icon indicating copy to clipboard operation
pkp-lib copied to clipboard

[GDPR] | Invite to a role - Removing Role, with access to settings does not remove all attributes of the previous role

Open Tribunal33 opened this issue 10 months ago • 1 comments

Valid Title

  • [x] I have updated the title to accurately reflect the bug description

Description

When removing a Journal Editor role but I think it might be for all roles. There are traces of the role left over. I will go over the easiest to identify but I'm worried this might mean that the database tables for removed role are not fully being flushed correctly. Easiest way to see this is with the Journal Editor role from Dbarnes.

I think this works for removing role either when inviting to a new role or from existing user table. I will focus on when inviting to a new role.

Steps to Reproduce

  1. Login with a User that can access settings and is not Dbarnes, who has Journal Editor role
  2. Navigate to Users and Roles
  3. Invite Dbarnes (or an existing user with JE permissions) to a new role.
  4. Remove the Journal Editor role
  5. Proceed with inviting Dbarnes to any other role that should not have access to settings like author
  6. After invitation is completed login with Dbarnes
  7. Go to Dashboard observe that Settings is not hidden for this user. Author's, for example, should not have access to settings.
  8. Select settings Users & Roles and keep a bookmark of the URL (for another bug)

Expected Result

Should not be able to view the settings navigation options unless you have access to that.

Actual Result

Can access the settings options and

Environment Details

No response

Application Version

OJS stable-3_5_0

Logs

No response

Additional Information

No response

Tribunal33 avatar Mar 02 '25 18:03 Tribunal33

I just tested this as Dbarnes and found that I cannot see anything. However, despite not accepting the role of Production Editor, I can still access the Dashboard from the dropdown and then shown the profile, which seems inconsistent.

If a user has no assigned role—not even as a reader—and has not accepted any role, they should not be shown the dashboard. This behavior needs to be reviewed.

Image

Devika008 avatar Mar 03 '25 22:03 Devika008

@Tribunal33 @Devika008 there is a check in the code. The navigation URL will be change according to the user roles. I will not going to change this until reviewed this

ipula avatar Mar 04 '25 21:03 ipula

@ipula sorry ipula this might be a bigger issue. The focus of my point was the Dashboard Settings is only available to the admin, JM and JE roles or if that role was granted access as in the ticket #5504

Tribunal33 avatar Mar 05 '25 07:03 Tribunal33

@ipula, a check to see if a role is active should be performed higher up the chain here: https://github.com/pkp/pkp-lib/blob/13c78ada3008a76e11a42af80f1d66b7d92f4a84/classes/security/authorization/UserRolesRequiredPolicy.php#L57-L60

and can be done so with

->withUserUserGroupStatus('active')

@asmecher, does this make sense to put this additional constraint on the role check in the policy here?

ewhanson avatar Mar 06 '25 18:03 ewhanson

Note: this will also need to be forwarded ported to main if this is the direction we choose to take.

ewhanson avatar Mar 06 '25 18:03 ewhanson

@ewhanson Somehow it returns the all user groups. I have to do some small changes. now Authorization works fine

$userGroups = UserGroup::withUserIds([$user->getId()])
            ->withContextIds($context ? [$context->getId(), Application::SITE_CONTEXT_ID] : [Application::SITE_CONTEXT_ID])
            ->whereHas('userUserGroups', function ($query) use ($user) {
                $query->withUserId($user->getId())->withActive();
            })
            ->get()
            ->all(); 

ipula avatar Mar 06 '25 19:03 ipula

Hey @ipula, could you share some more details about what returns all user groups? When I tested the change I mentioned, it worked as expected and only checked the active user groups with the scope method on the model.

ewhanson avatar Mar 07 '25 19:03 ewhanson

Its return the same user groups even I use ->withUserUserGroupStatus('active') did not filter the active ones. Then I used changed that I mention. I think ->withUserUserGroupStatus('active') didn't use the user id to filter from UserUserGroups

ipula avatar Mar 07 '25 19:03 ipula

Hi @Tribunal33

This change was implemented here and can be tested. https://github.com/ipula/pkp- lib/blob/039dd1745ef7e8ebad5868e856de285f340a1da9/classes/security/authorization/UserRolesRequiredPolicy.php#L57

withanage avatar Mar 10 '25 12:03 withanage

@withanage Don't know if this is related, but I'm not able to see any of the submissions now for any of the roles. Getting a console error.

Image

Image

Tribunal33 avatar Mar 10 '25 22:03 Tribunal33

@Tribunal33 this does not seem to be related to the GDOR inviation or user roles.

withanage avatar Mar 11 '25 17:03 withanage

This passes QA on the Test VM. No longer seeing the ghost of previous roles. Will test again once it is all merged to main/stable-3_5_0.

Tribunal33 avatar Apr 02 '25 21:04 Tribunal33