Repository navigation
UN-4344 [FEAT] Signed-in sessions list and sign out everywhere API - #2354
Open
Deepak-Kesavan wants to merge 7 commits into
Open
Deepak-Kesavan wants to merge 7 commits into
Deepak-Kesavan wants to merge 7 commits into
Conversation
Record each browser session at sign-in (user_logged_in, reached by every auth plugin via django login) and refresh last-active from CustomAuthMiddleware at most once a minute. Add org-scoped users/sessions/ (list), users/sessions/revoke-all/ and users/sessions/<id>/ (revoke one). Additive only.
…t-effort recording - Revoke-all decoded every unrecorded live session platform-wide on each call, including anonymous pre-login sessions that never get a record. Confine it to sessions expiring before migration-applied time plus SESSION_COOKIE_AGE, so it covers only pre-rollout sessions and turns itself off a day after rollout. - Prune records of dead sessions at sign-in so stored IPs do not outlive their session. - Sign-in/sign-out recording failures are logged, never raised. - Tests: device info in list, flush-path sign-in, IdP hook failures, browser-session guard, legacy-sweep bound.
Sign-in pruned every record whose session was not live, an anti-join of session_record against all of django_session on the login path. Prune only the signing-in user's records exactly, and sweep globally on an indexed last_active_at older than SESSION_COOKIE_AGE (a live session is touched on every authenticated request). 0008 regenerated with the index. Document and test that a record recreated by an in-flight request after revoke is kept out of the list by the django_session liveness check.
This was referenced Oct 21, 2026
Deepak-Kesavan
marked this pull request as ready for review
October 26, 2026 04:52
Contributor
 [Medium impact] The PR appears safe to merge, with non-blocking improvements needed for IP retention and optional location lookup failures.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
# Conflicts: # backend/account_v2/serializer.py
Sign out everywhere only ended sessions it could find through a SessionRecord, so a session whose recording failed at sign-in survived it. User.session_generation is now folded into the session auth hash and bumped on revoke-all, so Django refuses every session signed in under the earlier generation, recorded or not. Generation 0 keeps Django's own hash, so deploying signs nobody out. This replaces the migration-time-bounded decode sweep for pre-rollout sessions. Also addresses the SonarCloud issues: non-null SessionRecord.location, a Safari user-agent pattern without backtracking, and the no-op IdP hooks on AuthenticationService (the tracker already looks them up with getattr).
…m the previous commit The previous commit only carried the removal of 0011_sessionrecord; this adds the change it describes: User.session_generation in the session auth hash, the revoke-all bump, the regenerated migration, the tests and the SonarCloud fixes.
|
Comment on lines
+155
to
+157
| idle_cutoff = timezone.now() - timedelta(seconds=settings.SESSION_COOKIE_AGE) | ||
| SessionRecord.objects.filter(last_active_at__lt=idle_cutoff).delete() | ||
| cls.live_records(user) |
Contributor
There was a problem hiding this comment.
The global cleanup only runs when someone signs in. If nobody signs in again, expired SessionRecord rows can keep their raw IP addresses indefinitely. The age filter limits which rows a sweep deletes, not how long rows remain stored. Run cleanup on a schedule so the promised retention bound also holds during quiet periods.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/account_v2/session_tracking.py
Line: 155-157
Comment:
**Expired sessions keep IP addresses**
The global cleanup only runs when someone signs in. If nobody signs in again, expired `SessionRecord` rows can keep their raw IP addresses indefinitely. The age filter limits which rows a sweep deletes, not how long rows remain stored. Run cleanup on a schedule so the promised retention bound also holds during quiet periods.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+91
to
+95
| resolver = _location_resolver() | ||
| if resolver is None: | ||
| return None | ||
| try: | ||
| return resolver(ip_address) |
Contributor
There was a problem hiding this comment.
If SESSION_LOCATION_RESOLVER points to an unavailable module, session recording fails instead of falling back to no location. _location_resolver() runs before the try, and _upsert evaluates it before saving the record. Sign-in still succeeds, but new sessions stay absent from the list. Move resolver loading inside the guarded block.
Suggested change
| resolver = _location_resolver() | |
| if resolver is None: | |
| return None | |
| try: | |
| return resolver(ip_address) | |
| try: | |
| resolver = _location_resolver() | |
| if resolver is None: | |
| return None | |
| return resolver(ip_address) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/account_v2/session_tracking.py
Line: 91-95
Comment:
**Resolver errors hide new sessions**
If `SESSION_LOCATION_RESOLVER` points to an unavailable module, session recording fails instead of falling back to no location. `_location_resolver()` runs before the `try`, and `_upsert` evaluates it before saving the record. Sign-in still succeeds, but new sessions stay absent from the list. Move resolver loading inside the guarded block.
```suggestion
try:
resolver = _location_resolver()
if resolver is None:
return None
return resolver(ip_address)
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
Unstract test resultsPer-group results
Critical paths
|
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.
What
account_v2.SessionRecordmodel. Sessions are recorded at sign-in and their last-active time is refreshed at most once a minute.Why
How
user_logged_inreceiver writes aSessionRecord(user agent,REMOTE_ADDR, optional location). Every distribution reaches it, because OSS basic auth, the cloud Auth0 plugin and the cloud Azure AD plugin all calldjango.contrib.auth.login(). Auser_logged_outreceiver removes it. Both receivers log errors and never raise.SessionTracker.touch()runs insideCustomAuthMiddleware, but only in its session-authenticated branch. That branch skips Platform API key Bearer requests,X-API-KEYinternal-service calls, whitelisted paths and anonymous requests. The touch lives there, not in a middleware of its own, because cloud'sPRODUCTION_MIDDLEWAREis a separate list, while every settings file already referencesCUSTOM_AUTH_MIDDLEWARE. Acache.addgate limits writes to one per session per minute. A session that has no record yet (signed in before deploy) gets one on its first request.django_session.expire_datedecides whether a session is live. Records of dead sessions are pruned exactly for the user when they list their sessions or sign in. At each sign-in, a global sweep also drops any record idle for longer thanSESSION_COOKIE_AGE, using the indexedlast_active_at, so stored IPs outlive their session by at most one cookie age. A record recreated by a request that was in flight during a revoke is kept out of the list by the same liveness check.SessionStore(key).delete()clears both the cache entry and the DB row undercached_db. For the caller's own session it usesrequest.session.flush(), so the 204 also clears the cookie. Each revoke also runsLogService.remove_logs_on_logout, the same as logout does.SessionRecord: ones signed in before the deploy that have been idle since, and ones whose recording failed at sign-in (recording is best-effort). Revoke-all ends these too, without having to find them.User.session_generationis folded intoget_session_auth_hash(), and revoke-all bumps it with an atomicF() + 1UPDATE. Django'sAuthenticationMiddlewarechecks that hash on every request, so any session signed in under the earlier generation is refused, recorded or not. Generation 0 hashes exactly as Django does, so deploying this signs nobody out. Both the current-key and theSECRET_KEY_FALLBACKShashes include the generation.User.save()leavessession_generationout of a full save, so a save of aUserloaded before the bump can't write the old value back. No session table is decoded. Recorded sessions are still deleted outright. Unrecorded ones keep theirdjango_sessionrow until their next request (which flushes it) or until they expire.list_idp_sessions(request)andrevoke_idp_sessions(request, session_id=None). Both are optional and looked up withgetattr, because the cloud plugins don't subclassAuthenticationService. OSS basic auth defines neither. They're called best-effort.SESSION_LOCATION_RESOLVERtakes a dotted path tocallable(ip) -> str | None. It's unset by default, solocationisnull. No GeoIP dependency.backend/uv.lockhas no UA parser. Anything it doesn't recognise comes back asnull; it never guesses.Contract
Base:
/api/v1/unstract/{org_id}/users/sessions/. The org segment is only an auth gate (the caller's session must be on that org). The data is per user: the list returns all of the caller's sessions, on any org. Callers only ever see their own sessions.Auth: same as other session-authenticated endpoints. This includes CSRF: POST/DELETE need the
X-CSRFTokenheader like every other unsafe request from the SPA. Platform API key (Bearer) callers get 403; anonymous callers get 401.GET users/sessions/SignedInSession[]POST users/sessions/revoke-all/DELETE users/sessions/{id}/list_idp_sessions(request) -> list[SignedInSession], those entries are appended after ours, with non-uuid ids;DELETEon such an id goes torevoke_idp_sessions(request, session_id=id). Revoke-all callsrevoke_idp_sessions(request, session_id=None). Provider failures are logged and never fail the request.Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
users/(nothing existed atusers/sessions/), a new table, a newuser.session_generationcolumn, signal receivers, and one call inCustomAuthMiddleware's already-authenticated branch. No existing path, parameter, response field or default behaviour changes./login,/callbackand/sessionresponses are untouched.User.get_session_auth_hash()is unchanged at generation 0, the default for every existing user, so existing sessions stay valid. It changes only for a user who uses "sign out everywhere", and only to end their sessions. A fullUser.save()on an existing row now writes every field exceptsession_generation. Saves with explicitupdate_fields, and inserts, are unchanged.delete_patterncall per session, throughremove_logs_on_logout. That's the same per-session cost logout already has.Database Migrations
account_v2/0011_user_session_generation_sessionrecord: addsuser.session_generation(default 0) and creates thesession_recordtable. Generated withmakemigrationson top of main's0010_user_profile_fields. It replaces this branch's earlier0008_sessionrecord, which was never released. Any dev database that ran the earlier branch needssession_recorddropped, and itsaccount_v2 / 0008_sessionrecordrow removed fromdjango_migrations, before it can migrate.Env Config
SESSION_LOCATION_RESOLVER(dotted path). Unset means no location.Relevant Docs
Related Issues or PRs
django_login, and the touch through the sharedCUSTOM_AUTH_MIDDLEWARE.Dependencies Versions
Notes on Testing
backend/account_v2/test_session_tracking.py(34 tests). It drives two independent clients through a trimmed production middleware chain:is_currentand device info;Userloaded before revoke-all can't revive its sessions;SECRET_KEYrotation with fallbacks;IdentityProviderSessionTestswaps in a stand-in plugin whoseis_authenticatedrequiressession[Z_SESSION]the way Auth0Service/AzureADService do. With it, the test checks: the touch runs, revoke-all gets 401 for both sessions, IdP hooks are merged and called, and IdP failures are tolerated.pymssqlon macOS, unrelated to this change. Existing auth suites pass unchanged: platform-key middleware, deployment auth, global key, MCP auth, the account_v2 last-admin guard.Verified live on a dev deployment
Deployed this branch (
3eefbee9f, imagesnapshot.1793335986) to a cloud dev namespace (Auth0 sign-in). Migrationaccount_v2.0011_user_session_generation_sessionrecordapplied cleanly. Ran the endpoints from two independent signed-in browsers (separate cookie jars) as the same user:GET users/sessions/is_current: true,device_label: "Chrome on macOS",location: nullDELETE users/sessions/{other id}//sessioncall gets 401, and the caller's keeps working. An unknown id → 404, and a request with noX-CSRFToken→ 403POST users/sessions/revoke-all/sessionid=""; Max-Age=0, and both browsers then get 401session_generationdoesn't lock the user out/api/v1/sessionmain(the existing201on GET is main's behaviour, unchanged)Not run live:
DELETEon another user's session id, which needs a second account (covered by the unit tests above).