Conversation
- store refresh tokens hashed (SHA-256) at rest; migration hashes existing rows in place so live sessions survive - add POST /auth/logout and POST /v1/auth/logout-all with audit events - revoke all sessions on password change - return a rotated refresh token on refresh (additive field; strict rotation will be enforced once clients adopt it) - accept JWT via Authorization header only (no query string) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- move session revocation into updatePassword so the forgot-password path also kills live sessions; revoke sessions on admin status/role change - drop the refresh-token rotation prep: it grew live tokens without bound and weakened logout; full rotation lands in a later phase - cleanup job now also reclaims revoked-but-unexpired tokens - make the hash migration idempotent (hex-digest guard) - misc: reuse apitypes.OkResponse, drop json tags from the DB model, context-aware DB calls in touched tests, CLAUDE.md token doc update Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens refresh-token storage and session lifecycle management by hashing refresh tokens at rest, introducing logout endpoints, revoking sessions on key account events, and removing query-string JWT transport to reduce token leakage risk.
Changes:
- Store refresh tokens as hex-encoded SHA-256 hashes (including an in-place migration) and update lookup/revocation logic to hash presented tokens.
- Add
POST /auth/logoutandPOST /v1/auth/logout-allendpoints with audit logging and anti-enumeration behavior. - Revoke all refresh tokens on password changes and on admin-driven status/role changes; cleanup job now also reclaims revoked tokens.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/api-scenarios/001-user-registration-auth.yaml | Adds end-to-end scenario coverage for logout/logout-all, password-change revocation, and query-token rejection. |
| pkg/security/types.go | Makes RefreshToken a storage-only model (no JSON tags) and adds LogoutAllResponse type. |
| pkg/security/tokens.go | Removes query-string JWT extraction; header-only bearer token support. |
| pkg/security/security_test.go | Updates ExtractToken test to assert query tokens are ignored. |
| pkg/security/refresh_tokens.go | Hash-at-rest implementation plus new revoke APIs and expanded cleanup behavior. |
| pkg/security/refresh_tokens_test.go | Adds/updates unit tests for hashed storage, revocation semantics, and cleanup of revoked tokens. |
| pkg/security/handlers.go | Adds logout and logout-all HTTP handlers and wires them to revocation + audit logging. |
| pkg/security/handlers_test.go | Adds handler-level tests for logout/logout-all and refactors request posting helper. |
| pkg/security/audit_log.go | Adds logout-all audit event and renames logout event constant to success-specific names. |
| pkg/database/migration/migration_scripts/000026_refresh_token_hash.up.sql | Migrates existing refresh_token rows to hashed storage (intended idempotent). |
| pkg/database/migration/migration_scripts/000026_refresh_token_hash.down.sql | Down migration truncates refresh_token (one-way hashing rollback strategy). |
| pkg/accounts/handlers.go | Notes password change now revokes all live sessions. |
| pkg/accounts/accounts.go | Revokes all refresh tokens on password change and on admin status/role changes. |
| pkg/accounts/accounts_test.go | Adds regression tests ensuring status change and password change revoke refresh tokens. |
| main.go | Registers the new logout and logout-all routes. |
| claude.md | Updates documentation to reflect header-only JWT transport. |
| api-doc/swagger.yaml | Documents new logout endpoints and logout-all response schema. |
| api-doc/swagger.json | Generated swagger JSON updates for logout/logout-all. |
| api-doc/docs.go | Generated swagger docs template updates for logout/logout-all. |
Files not reviewed (1)
- api-doc/docs.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- RevokeRefreshToken / RevokeAllUserTokens now only match live (unexpired) tokens so logout audits and revoked_sessions counts reflect real sessions - postJSON test helper fails fast on marshal errors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
Hardening of the refresh-token layer and session lifecycle:
000026hashes existing rows in place (idempotent), so live sessions survive the deploy. The plaintext only ever exists in transit to the client.POST /auth/logout— revokes the presented refresh token; always returns 200 so token existence cannot be probed; auditslogout_success.POST /v1/auth/logout-all— authenticated; revokes every live session of the user and returnsrevoked_sessions; auditslogout_all_success.Authorization: Beareris the only supported transport (no known client used the query form).Refresh-token rotation is intentionally not part of this PR: it will land in a follow-up once clients store the rotated token, so that issuance and revocation ship atomically with reuse detection.
apitest scenario 001 covers the new flows end-to-end (logout, logout-all, revocation on password change, query-token rejection, anti-enumeration logout).
Closes #158
🤖 Generated with Claude Code