feat(BA-7364): deliver the login user id in the auth response - #13777
Open
jopemachine wants to merge 8 commits into
Open
feat(BA-7364): deliver the login user id in the auth response#13777jopemachine wants to merge 8 commits into
jopemachine wants to merge 8 commits into
Conversation
`AuthSuccessResponse` carries the login user's id and the keypair rate limit, and the web server stores both in the session token, so callers can identify and throttle a login session without asking the manager again. Both fields are optional to keep the wire format compatible with managers that do not send them yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
size:M
This was referenced Aug 14, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Adds user identity and keypair rate-limit metadata to successful authorization responses and persists them in web login sessions.
Changes:
- Extends authorization DTOs and service results with
user_idandrate_limit. - Stores both fields for password and token-based web logins.
- Adds serialization, compatibility, service, and API tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/ai/backend/common/dto/manager/auth/types.py |
Adds optional response fields. |
src/ai/backend/manager/data/auth/types.py |
Extends authorization result data. |
src/ai/backend/manager/services/auth/service.py |
Returns the keypair rate limit. |
src/ai/backend/manager/api/rest/auth/handler.py |
Exposes user ID and rate limit. |
src/ai/backend/web/server.py |
Persists both fields in web sessions. |
tests/unit/common/dto/manager/auth/test_auth_types.py |
Tests parsing and serialization. |
tests/unit/manager/services/auth/test_authorize.py |
Tests service propagation. |
tests/unit/manager/api/auth/test_handlers.py |
Tests API response fields. |
changes/13777.feature.md |
Documents the feature. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
`AuthorizationResult` and `AuthSuccessResponse` carry `UserID`, `AccessKey`, `SecretKey` and `UserRole` instead of bare `uuid.UUID` / `str`. The user id is required on the wire: the manager always sends it, and a web server or SDK newer than the manager it talks to is not a supported combination. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d fields CI typechecks and runs the whole tree, so the response wrapper, both parse cases, the handler fixture and the client-side authorize fixture all needed the typed key values and the user id. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jopemachine
force-pushed
the
feature/BA-7364-auth-response-user-identity
branch
from
August 14, 2026 09:37
1c8bb74 to
b581df7
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/ai/backend/web/server.py:772
- The token-login path also turns a missing optional user ID into the literal string
"None". Preserve the null value so sessions created against an older manager remain identifiable as lacking a user ID.
"user_id": str(token.user_id),
src/ai/backend/common/dto/manager/auth/types.py:60
user_idis required here, soparse_auth_response()rejects successful responses from older managers that omit it. That contradicts the stated mixed-version wire compatibility; make it nullable with a default just likerate_limit.
user_id: UserID
src/ai/backend/web/server.py:509
- Once the advertised optional
user_idis accepted,str(None)stores the literal string"None"rather than an absent ID. Downstream session consumers can then treat it as an ID and fail to parse it; preserveNonein the session instead.
This issue also appears on line 772 of the same file.
"user_id": str(token.user_id),
…r secret `keypairs.secret_key` is NOT NULL, so the fallback could never be reached. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A missing field defaulting to null reads as unlimited, so an incomplete payload would silently disable the limit. Null stays a valid value — it is what an unset limit means — but it now has to be sent explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jopemachine
force-pushed
the
feature/BA-7364-auth-response-user-identity
branch
from
August 14, 2026 09:43
bc83a6c to
c3fdc55
Compare
seedspirit
previously approved these changes
Aug 14, 2026
HyeockJinKim
requested changes
Aug 14, 2026
The limit is a resource policy value, not part of who authenticated, and a session snapshot of it goes stale: the manager reads the policy per request while a web server session would hold the login-time value for up to a week. The web server will read it from the shared rate limit Redis DB instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jopemachine
changed the title
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
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.
📚 Stacked PRs
This PR is part of a 5-PR stack. Merge in order:
feat(BA-7364): deliver the login user id in the auth response← you are herefeat(BA-7365): key the rate limit counter by user idfeat(BA-7362): move the per-user API rate limit to the user resource policyfeat(BA-7363): publish the per-user rate limit to the shared Redis DBfeat(BA-7365): add a per-user rate limit middleware to the web serverOnly the final tip (#13771) is guaranteed to build / pass CI; intermediate PRs are logical slices for reviewability.
Summary
user_id(UserID) field toAuthSuccessResponse, filled in the manager RESTauthorizehandler, and store it in the web server session token in both login paths. The web server needs the login user's identity to key a per-user rate limit counter without asking the manager again.AuthorizationResultandAuthSuccessResponsenow useUserID,AccessKey,SecretKeyandUserRoleinstead of bareuuid.UUID/str.or ""fallbacks onkeypairs.secret_key, which is NOT NULL.The rate limit value is deliberately not part of this response (see #13777 review): it is a resource policy value rather than part of who authenticated, and a session snapshot would go stale — the manager reads the policy per request, while a web server session would hold the login-time value for up to a week. The web server will read it from the shared rate limit Redis DB instead (BA-7363).
Resolves BA-7364.