fix: Limit shopping list owners to current group #3305
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
(REQUIRED)
What this PR does / why we need it:
(REQUIRED)
The shopping list owner added in #3213 adds a setting on the shopping list to update the owner. This API call actually uses the admin route erroneously, so only admins can update the owner. Furthermore, it shows users from all groups, not just the current group.
This PR adds a new route to fetch only the users belonging to the current user's group, and loosens permissions so that any user can use this API (rather than only admins).
Which issue(s) this PR fixes:
(REQUIRED)
N/A
Special notes for your reviewer:
(fill-in or delete this section)
We should probably update the frontend's user API to prevent this from happening; the user API shouldn't be able to accidentally access the admin API routes, but due to how we've structured the user controller on the backend (the non-admin routes use the same base URL) it's a bit messy when using our generic frontend client. Ideally these are two separate base URLs (e.g.
/usersand/admin/usersbut since this would introduce breaking changes it's probably best to just work around it.Testing
(fill-in or delete this section)
Added backend tests and verified proper behavior on the frontend using a non-admin user.