-
Notifications
You must be signed in to change notification settings - Fork 16
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
IBX-9002: Added endpoint for list users with permission info #1372
base: 4.6
Are you sure you want to change the base?
IBX-9002: Added endpoint for list users with permission info #1372
Conversation
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.
Current implementation seems to - correct me if I'm wrong - not distinguish between Admin UI users and "frontend" users.
I.e. you will get results with store front customer accounts, if they are created. They will also heavily pollute this list, probably making it hardly usable (if what I'm saying is true).
On a separate note: this makes any user that is able to log into the admin to discover all usernames and emails (as this is what your endpoint basically does). Given this is public and deals with sensitive information, I don't think this can be exposed like that.
Additionally, it should be enabled only if there are features that rely on it IMO. And I would even go as far as ask developer/admin to toggle availability of this endpoint on explicitly, maybe through bundle configuration.
To sum up, it's too exposed and sensitive to be enabled by default.
src/lib/User/Mapper/UsersWithPermissionInfoToContentItemMapper.php
Outdated
Show resolved
Hide resolved
2be3fd8
to
47b4139
Compare
47b4139
to
3abf68b
Compare
Sorry, previous version was sent for review too early. I pushed new version including new Query based on new Criteria (the previous ones did not work for elasticsearch and solr) and now guest and corporate users are skipped. About availability of this endpoint. I have same feelings like you, and access for sensitive informations worries me. I'd to discuss this internally. |
21d6cb7
to
b2dc741
Compare
b2dc741
to
f936d48
Compare
Quality Gate passedIssues Measures |
Description:
This PR adds internal endpoint to load list of users with their permissions to given content item. This is required by share dialog feature to search for users by given phrase e.g.: name, surname or email. Full list of results should be displayed, grouped by users with and without permissions.
A new endpoint is not exposed in REST API to avoid public access to list of users with permissions for not logged users. To make this feature useful we cannot filter search results by permissions. Otherwise we would have to change permission settings for users to allow read User content.
Supported query params:
location={locationId}
- (int) target Location for permission checklimit={limit}
- (int) number of resultsoffset={offset}
- (int) results offsetphrase={phrase}
- (string) phrase using for search user by first name, last name or email.For QA:
Documentation: