Fix RoleService: remove hardcoded return values for isController() and isAdmin()#5274
Fix RoleService: remove hardcoded return values for isController() and isAdmin()#5274GauravD2t wants to merge 5 commits intoDSpace:mainfrom
Conversation
nwoodward
left a comment
There was a problem hiding this comment.
Hi @GauravD2t. Thank you for this PR. I found a problem when testing it locally, and I think it needs to be fixed for the default new user scenario.
When I registered as a new user and clicked on MyDSpace the page flashed continuously because it was in an infinite loop. I saw the following log on the backend repeating over and over until I stopped the frontend:
2026-03-11 16:31:11,092 INFO f491abc1-73bf-4b05-8ac0-8fb43eb3d2d7 b5427d96-a378-4ac0-aeed-40c3bf28056c org.dspace.app.rest.utils.DSpaceAPIRequestLoggingFilter @ Before request [GET /server/api] originated from /home
Then I logged in as an admin in another browser and tested 1) adding the new user to the Administrator group and 2) making the new user a Community or a Collection admin. In every case, this stopped the infinite loop, and the MyDSpace page loaded successfully.
So the problem is with a new user who isn't a controller or an admin. In that scenario there is some code in the PR that continuously pings the backend.
|
Thank you again for testing and identifying this issue. After investigating the behavior for newly registered users, I found that when a user does not have any roles (not a submitter, controller, or admin), the available configuration list could be empty. This caused the MyDSpace page to repeatedly re-evaluate the configuration, which resulted in the continuous requests to I updated the logic to ensure that the Please let me know if you notice any further issues while testing. |
|
Hi @GauravD2t. Thank you for the most recent change. The MyDSpace page now loads successfully for a new user without any roles. I think the logic can be simplified even further, since the first If fact, the last What do you think? |
References
(#3331)
Description
Previously:
isController() always returned true.
isAdmin() always returned false.
This caused incorrect UI behavior where users could see controller-related options even when they did not have the appropriate permissions.
This PR replaces those hardcoded values with proper authorization checks using AuthorizationService and FeatureID.
Changes
Updated isController() to check whether the authenticated user has either:
IsCollectionAdmin
IsCommunityAdmin
Updated isAdmin() to check whether the user has the AdministratorOf authorization.
Added distinctUntilChanged() to prevent unnecessary observable emissions.
Instructions for Reviewers
Verify that isController() returns true only when the user is a Collection Admin or Community Admin.
Verify that isAdmin() correctly detects repository administrators.
Confirm that MyDSpace UI options are displayed correctly based on user permissions.
List of changes in this PR:
Include guidance for how to test or review your PR. This may include: steps to reproduce a bug, screenshots or description of a new feature, or reasons behind specific changes.
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
mainbranch of code (unless it is a backport or is fixing an issue specific to an older branch).npm run lintnpm run check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.