-
Notifications
You must be signed in to change notification settings - Fork 189
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
feat: Normal user list #7887
feat: Normal user list #7887
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
changelog/unreleased/enhancement-allow-listing-regular-users.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add a test to validate that only the minimal set of id,displayName and mail is present if the regular user is requesting the users endpoint ?
I took over here as @jvillafanez can't currently work it. |
@janackermann done. Please re-review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💪
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One change request, rest looks fine.
Some getUser tests are failing. |
Should be fixed now. I took the chance to also add the group search here now as well. |
0c6cc02
to
d539361
Compare
Previously we only did a prefix match.
Neither 'BadRequest' (as expected in the unit test) nor 'Unauthorized' (as expected in the API tests) seem correct here. We're no returning 'Forbidden' when an unprivileged users issues a GetUsers request that it is not allowed to perform.
d539361
to
5193c4b
Compare
Kudos, SonarCloud Quality Gate passed! |
we can bug where we can get full info about user #5125 |
Description
Allow regular users to list other users.
People with account management permissions (such as admins) can list users as they have been until now. This PR won't affect them in any way.
Regular users (without account management permission) can list users with the following restrictions:
Related Issue
#7782
Motivation and Context
How Has This Been Tested?
Manually tested.
Unit tests have been adjusted because some of them where failing due to the user not having enough permissions
Screenshots (if appropriate):
Types of changes
Checklist: