fix: let normal users access their information in the v3 API#445
fix: let normal users access their information in the v3 API#445alemar99 wants to merge 4 commits into
Conversation
|
Check where you would like a Mattermost message to be sent to when CI completes and this PR is merged
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
[maas-code-reviewer review]
LLM-generated review from https://github.com/canonical/maas-code-reviewer.
Intended to assist a human reviewer, not replace one — suggestions may be
incorrect, please verify before acting.
Overall, the logic to restrict non-admin users from viewing or updating other users correctly raises a 404 (preventing user enumeration) instead of a 403. However, there is a critical security vulnerability: non-admin users can escalate their privileges by setting is_superuser=True when updating their own profile.
Make sure to add authorization checks inside update_user to restrict which fields a non-admin user can update, or separate the models for admin updates and user self-updates.
| assert response.status_code == 200 | ||
| assert len(response.headers["ETag"]) > 0 | ||
| user_response = UserResponse(**response.json()) | ||
| assert user_response.id == 1 |
There was a problem hiding this comment.
Minor readability issue: The test requests user ID 0 (line 271), but asserts that the response has user ID 1. This happens because USER_1 is hardcoded to id=1. While functionally correct since the mock just returns what it's told, it makes the test logic confusing. Consider returning a mock user with id=0 instead.
2 participants
Resolves: LP:2148485