feat(BA-5733): record, expose, and filter login_history by client IP#11733
Draft
jopemachine wants to merge 9 commits into
Draft
feat(BA-5733): record, expose, and filter login_history by client IP#11733jopemachine wants to merge 9 commits into
jopemachine wants to merge 9 commits into
Conversation
Add a nullable `client_ip` column to `login_history` and populate it from REST handlers and the GraphQL revoke adapter for every user-initiated event (authorize success/failure, logout, signout, admin/user revocation). System-driven events (eviction, expiration) remain NULL. Expose the field on `LoginHistoryV2` (`clientIp`) so admins and users can see where each event originated. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: octodog <mu001@lablup.com>
Resolve diverged Alembic heads caused by b8a85c96607c (start_command rewrap) landing on main with the same parent revision (7a9be5b982c0) as this branch's client_ip migration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…flows Drop the ``client_ip`` field from Actions whose handlers are already authenticated (``signout``, ``my_revoke_login_session``, ``admin_revoke_login_session``) and have the service resolve it via ``current_client_ip()``, mirroring how ``current_user()`` is consumed in service logic. The auth middleware already populates the contextvar for authenticated routes, so handlers no longer have to thread the value through the Action. Unauthenticated entry points (``authorize``, ``logout``) keep the explicit ``client_ip`` Action field — the middleware does not set the contextvar before authentication, so the REST handler still extracts it from the request directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
9772d32 to
e402ef3
Compare
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Expose client_ip in LoginHistoryFilter (v2 DTO + GraphQL input) and LoginHistoryOrderField so admins and users can drill into history by source IP. Wire the new field through the adapter and add matching StringFilter / order helpers in the auth repository options. Rename the news fragment to 11733.feature.md to match the PR number. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cover every AuthDBSource path that writes login_history (create, record_login_history, delete_session_by_token/id, delete_sessions_by_user, delete_sessions_by_tokens) and confirm the supplied client_ip lands on the row verbatim. System-driven eviction without a client_ip still leaves the column NULL, matching the production call sites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rals Inject a single ``client_ip`` fixture (``1.2.3.4``) into each test rather than sprinkling distinct hard-coded IP strings across scenarios. The fixture-based form keeps the assertions readable and removes incidental variation between tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rename and expand the eviction test to make the data model explicit: ``login_history`` is append-only, so the original ``SUCCESS`` row keeps its ``client_ip`` even after the ``login_session`` is force-evicted. The newly written ``EVICTED`` row carries ``client_ip = NULL`` because eviction is system-driven and no client initiated it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end support for persisting the originating client IP address on login_history rows, and exposes/filter/sort support for the new field across REST v2 and Strawberry GraphQL v2.
Changes:
- Add nullable
client_ipcolumn tologin_historyand propagate it through auth session/history write paths. - Expose
client_ipvia REST v2 DTOs and Strawberry GraphQLLoginHistoryV2, including filter and order support. - Add repository-level unit tests ensuring
client_ipis persisted (and remains NULL for system-driven events).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/manager/repositories/auth/test_login_history_client_ip.py | Adds unit tests covering client_ip persistence across AuthDBSource write paths. |
| src/ai/backend/manager/services/auth/service.py | Threads client_ip into login-history writes; uses request-scoped current_client_ip() where available. |
| src/ai/backend/manager/services/auth/actions/logout.py | Extends LogoutAction to carry optional client_ip. |
| src/ai/backend/manager/services/auth/actions/authorize.py | Extends AuthorizeAction to carry optional client_ip. |
| src/ai/backend/manager/repositories/auth/repository.py | Extends repository APIs to accept and forward client_ip to DB source. |
| src/ai/backend/manager/repositories/auth/options.py | Adds client_ip filter and order factories for login history querying. |
| src/ai/backend/manager/repositories/auth/db_source/db_source.py | Persists client_ip in login history insert paths and delete-session history generation. |
| src/ai/backend/manager/models/login_session/row.py | Adds client_ip column mapping and includes it in to_data(). |
| src/ai/backend/manager/models/alembic/versions/a1b3e7c2d4f5_add_client_ip_to_login_history.py | Alembic migration adding/dropping login_history.client_ip. |
| src/ai/backend/manager/data/auth/login_session_types.py | Extends LoginHistoryData with client_ip. |
| src/ai/backend/manager/api/rest/auth/handler.py | Extracts client IP for authorize/logout and passes it into actions. |
| src/ai/backend/manager/api/gql/login_history/types/order.py | Adds CLIENT_IP to GraphQL order field enum. |
| src/ai/backend/manager/api/gql/login_history/types/node.py | Exposes clientIp on LoginHistoryV2 (version-gated). |
| src/ai/backend/manager/api/gql/login_history/types/filter.py | Adds GraphQL clientIp filter field (version-gated). |
| src/ai/backend/manager/api/adapters/login_history/adapter.py | Converts client_ip filters/orders and maps data to response nodes. |
| src/ai/backend/common/dto/manager/v2/login_history/types.py | Adds CLIENT_IP to REST v2 order field enum. |
| src/ai/backend/common/dto/manager/v2/login_history/response.py | Exposes client_ip in REST v2 response node DTO. |
| src/ai/backend/common/dto/manager/v2/login_history/request.py | Adds client_ip to REST v2 filter DTO. |
| docs/manager/graphql-reference/v2-schema.graphql | Regenerates schema docs to include clientIp filter/field and ordering. |
| docs/manager/graphql-reference/supergraph.graphql | Regenerates supergraph schema docs to include clientIp filter/field and ordering. |
| changes/11733.feature.md | Adds changelog entry for the new client_ip field exposure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
151
to
156
| stoken=params.stoken, | ||
| otp=params.otp, | ||
| client_type_id=params.client_type_id, | ||
| client_ip=extract_client_ip(ctx.request), | ||
| force=params.force, | ||
| ) |
Comment on lines
179
to
187
|
|
||
| async def logout(self, body: BodyParam[LogoutRequest], ctx: RequestCtx) -> APIResponse: | ||
| params = body.parsed | ||
| log.info("AUTH.LOGOUT(session_token:{}...)", params.session_token[:8]) | ||
| action = LogoutAction(session_token=params.session_token) | ||
| action = LogoutAction( | ||
| session_token=params.session_token, | ||
| client_ip=extract_client_ip(ctx.request), | ||
| ) | ||
| await self._auth.logout.wait_for_complete(action) |
8 tasks
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.
Summary
client_ipcolumn tologin_history(new Alembic migrationa1b3e7c2d4f5, chained afterb8a85c96607c).authorize(success and failure),logout,signout, and admin/user revocation. System-driven events (EVICTED,EXPIRED) intentionally leaveclient_ipNULL.LoginHistoryV2.clientIp(Strawberry,NEXT_RELEASE_VERSION) and the v2 REST response DTOLoginHistoryNode.client_ip; supergraph schema regenerated accordingly.Resolves BA-5733.
client_ip propagation strategy
Two patterns are used depending on whether
auth_middlewarehas populated the request-scopedcurrent_client_ip()contextvar by the time the service runs. The middleware only enterswith_client_ip(...)for endpoints whose handler is decorated withauth_required(i.e. JWT/HMAC-authenticated routes) — see_setup_user_contextinapi/rest/middleware/auth.py.Passed explicitly via Action
Endpoints that do not go through
auth_required— the contextvar is unset, so the REST handler callsextract_client_ip(request)and threads the value through the Action'sclient_ipfield.auth_requiredPOST /auth/authorizeAuthorizeAction.client_ipAuthHandler.authorize→extract_client_ip(ctx.request)POST /auth/logoutLogoutAction.client_ipAuthHandler.logout→extract_client_ip(ctx.request)session_tokenin the request body, not JWT/HMAC headersResolved by the service via
current_client_ip()Endpoints behind
auth_required—auth_middlewareenterswith_client_ip(...)inside_setup_user_context, so the service layer can callcurrent_client_ip()directly (mirroring the existingcurrent_user()pattern). The Action no longer carriesclient_ip.POST /auth/signout(REST)SignoutActionAuthService.signout→current_client_ip()myRevokeLoginSessionmutation (GQL)MyRevokeLoginSessionActionAuthService.my_revoke_login_session→current_client_ip()adminRevokeLoginSessionmutation (GQL)AdminRevokeLoginSessionActionAuthService.admin_revoke_login_session→current_client_ip()Test plan
pants fmt --changed-since=origin/main— cleanpants lint --changed-since=origin/main— cleanpants check --changed-since=origin/main— mypy cleanpants test tests/unit/manager/services/auth::— passespants test tests/unit/manager/api/auth::— passeslogin_history.client_ipis populated for: authorize success, authorize failure, logout, admin revoke, user revokeclientIpappears inmyLoginHistoryV2/adminLoginHistoryV2GraphQL responses after restarting the manager and Hive (Apollo Router) containersEVICTED/EXPIREDrows still haveclient_ip = NULL🤖 Generated with Claude Code
📚 Documentation preview 📚: https://sorna--11733.org.readthedocs.build/en/11733/
📚 Documentation preview 📚: https://sorna-ko--11733.org.readthedocs.build/ko/11733/
📚 Documentation preview 📚: https://sorna--11733.org.readthedocs.build/en/11733/
📚 Documentation preview 📚: https://sorna-ko--11733.org.readthedocs.build/ko/11733/