feat(user): revoke sessions on user disable#1595
Conversation
When a user is disabled, soft-delete all of their active sessions so outstanding session cookies stop authenticating immediately rather than lingering until expires_at (up to 7 days). The lazy notDisabledUserExp filter at the user repo already blocks disabled-user auth, so this is a hygiene/audit improvement rather than a security fix: it produces an explicit revocation trail (per-session SessionRevokedEvent records via the existing audit hook) and prevents pre-disable sessions from silently resuming if the user is re-enabled within the validity window. Reuses repo.List + repo.Delete instead of adding a bulk SQL path, so each revoked session goes through the same audit pipeline as a manual admin revoke. Refs: #1585 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds automatic session revocation when a user is disabled. It introduces a ChangesUser Session Deletion on Disable
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
core/authenticate/session/service.go (1)
91-93: ⚡ Quick winUse the service deletion path inside bulk revoke.
Calling
s.Delete(ctx, sess.ID)here avoids behavior drift between single-session and bulk-session revocation paths.Suggested diff
for _, sess := range sessions { - if err := s.repo.Delete(ctx, sess.ID); err != nil { + if err := s.Delete(ctx, sess.ID); err != nil { return err } }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 22390923-e1ad-4ce1-b9fd-baa4a4a1163b
📒 Files selected for processing (6)
cmd/serve.gocore/authenticate/session/service.gocore/authenticate/session/service_test.gocore/user/mocks/session_service.gocore/user/service.gocore/user/service_test.go
Coverage Report for CI Build 25658348755Coverage increased (+0.02%) to 41.985%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Route bulk session revoke through Service.Delete instead of repo.Delete so future Delete changes propagate; strengthen early-exit test to assert the second session's delete is never attempted after the first fails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
user.Service.Disablenow soft-deletes all of the user's active sessions after flipping state. Outstanding session cookies stop authenticating immediately instead of lingering untilexpires_at. Not a security fix — the existingnotDisabledUserExpfilter at the user repo already blocks disabled-user auth at lookup time — but a hygiene and pre-disable sessions can no longer silently resume if the user is re-enabled within the validity window. They will need to re-login.Refs umbrella issue #1585.
Changes
core/authenticate/session/service.go— newService.DeleteByUserIDthat reusesrepo.List+repo.Delete.core/user/service.go— newSessionServiceinterface and field;DisablecallsDeleteByUserIDafter a successfulSetState. Invalid-id and not-found paths skip the revocation as before.cmd/serve.go— pass the existingsessionServiceintouser.NewService.core/user/mocks/session_service.go(regenerated via mockery;core/useris already in.mockery.yaml).Technical Details
The DRY choice:
DeleteByUserIDlists the user's active sessions via the existing repo method and callsrepo.Deleteper session. Each delete goes through the existing TX +createSessionRevokeAuditRecordpath, which already emits aSessionRevokedEventwhen the actor is different from the session owner (the admin-initiated disable case).Mirrors what
serviceUserService.Deletealready does for credentials.Test Plan
go test ./core/user/... ./core/authenticate/session/...passesgo build ./...cleanTestService_DeleteByUserIDsubtests (happy, empty, list-error, delete-error);TestService_Disableupdated with revoke-on-success, skip-on-failure, and revoke-error-propagation casessessions.deleted_atis set🤖 Generated with Claude Code