refactor(api): remove redundant GET /api/users/security route#6398
Merged
Conversation
API keys are namespace-scoped tokens and should not be usable on account-level routes. Four routes were missing the BlockAPIKey middleware, allowing API keys to reach handlers they should never reach. The most concrete gap was ListAPIKeys: an API key could enumerate all other API keys in the namespace. The remaining three routes (GetNamespaceList, CreateNamespace, LeaveNamespace) were safe by accident -- the service layer rejected them -- but returned confusing 404s instead of a clean 403. Added BlockAPIKey to all four routes, matching the pattern already used by CreateAPIKey, UpdateAPIKey and DeleteAPIKey. Removed the x-internal overrides and api-key security entries from the OpenAPI spec for the same operations, since the spec now accurately reflects what the code enforces. Removed TestListNamespacesAPIKeyScope, which tested the old scoped behavior that no longer applies.
The GET /api/users/security route returned a single boolean
(namespace session record status) that is already available from
GET /api/namespaces/{tenant} via settings.session_record.
The active frontend (ui-react) does not call it. The only reference
was in the dead Vue UI under /v1/. The path was also misleading:
/users/security for a namespace-level setting, with no tenant guard
unlike its sibling PUT route.
Removed the route registration, URL constant, handler, service
method, mock, and all associated tests.
geovannewashington
added
area/api
go
|
Claude finished @geovannewashington's task in 6m 38s —— View job Code Review CompleteReviewed 10 files across code quality, security, testing, language patterns, and architecture — no issues found. The code looks good as-is. Highlights:
If you push additional changes and want a new review, tag |
This file contains hidden or 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
2 participants
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
Removed the
GET /api/users/securityroute and all associated code:route registration, URL constant, handler, service method, mock, and
tests.
Why
The route returned a single boolean (namespace session record status)
already available from
GET /api/namespaces/{tenant}viasettings.session_record. The active frontend (ui-react) does notcall it -- the only reference was in the dead Vue UI under
/v1/.The path was also misleading for a namespace-level setting, and had
no tenant guard unlike its sibling
PUT /api/users/security/{tenant}.Closes #6391.
Testing
./bin/docker-compose up$TOKENcurl -v -H "Authorization: Bearer $TOKEN" http://localhost/api/users/securityShould return 404.