Skip to content

🐛(backend) remove user list endpoint#511

Open
maxenceh wants to merge 1 commit into
mainfrom
maxenceh/fix-improper-access-control
Open

🐛(backend) remove user list endpoint#511
maxenceh wants to merge 1 commit into
mainfrom
maxenceh/fix-improper-access-control

Conversation

@maxenceh
Copy link
Copy Markdown
Collaborator

@maxenceh maxenceh commented May 28, 2026

Purpose

  • Fix an improper access control vulnerability on the GET /api/v1.0/users/ endpoint.
  • Any authenticated user could search for other users by email and retrieve their full profile data, including the OIDC sub identifier (which enables cross-system identity correlation), personal preferences, and language settings.
  • The user search feature was built in anticipation of a future document-sharing UI but is not called by any frontend code. Removing it eliminates the attack surface with no functional regression.

Proposal

  • Remove ListModelMixin from UserViewSet — the /api/v1.0/users/ list URL is no longer registered by the router (returns 404)
  • Remove the associated get_queryset() search logic (Levenshtein/trigram email search, document_id filter)
  • Remove the get_throttles() method and throttle classes that only served the list action
  • Update tests: remove all user-search tests, update list-URL assertions to 404

Summary by CodeRabbit

  • Bug Fixes

    • Removed the user list API endpoint. Collection-level operations for retrieving all users, creating new users, and deleting users via the API are no longer available.
  • Documentation

    • Updated changelog to document the removal of the user list endpoint.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: cf8a29a3-1310-4aa7-8f2d-7a64b487f2a3

📥 Commits

Reviewing files that changed from the base of the PR and between 119b814 and 8742cd3.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/backend/core/api/viewsets.py
  • src/backend/core/tests/test_api_users.py

Walkthrough

The PR removes the /api/v1.0/users/ collection endpoint by stripping list/search/throttle functionality from UserViewSet, reducing it to update-only behavior. Tests are updated to expect 404 responses, and the breaking change is documented in the changelog.

Changes

User list endpoint removal

Layer / File(s) Summary
UserViewSet refactor: remove list and search
src/backend/core/api/viewsets.py
UserViewSet drops ListModelMixin, removes get_throttles() and get_queryset() overrides, and eliminates throttle classes UserListThrottleBurst and UserListThrottleSustained. Imports are cleaned to remove unused UserRateThrottle, TrigramSimilarity, and RawSQL.
Collection endpoint tests updated for 404 responses
src/backend/core/tests/test_api_users.py
GET, POST, and DELETE requests to /api/v1.0/users/ now expect 404 instead of authentication errors or method-not-allowed responses. Assertions confirm user count remains unchanged for all collection-level operations.
Changelog documentation
CHANGELOG.md
The unreleased section adds a Fixed entry documenting the removal of the backend user list endpoint.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'remove user list endpoint' accurately describes the main change—removal of the /api/v1.0/users/ list endpoint to fix an access control vulnerability. While the emoji and prefix are stylistic, the core message is clear and directly related to the primary objective.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch maxenceh/fix-improper-access-control

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@maxenceh maxenceh marked this pull request as ready for review May 28, 2026 14:43
@maxenceh maxenceh requested a review from providenz May 28, 2026 14:43
Disable a user to search other users.
Update tests to return 404
@maxenceh maxenceh force-pushed the maxenceh/fix-improper-access-control branch from 130a452 to 8742cd3 Compare May 28, 2026 14:44
@sonarqubecloud
Copy link
Copy Markdown

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant