feat(auth): Refresh Token Rotation, Clean Architecture & Enterprise Hardening (#54)

[rajeshm20]

Summary

Closes #54

Implements enterprise-grade architectural hardening of StudentAppBackend:

• Dual-Token Lifecycle with SHA-256 Hashed Refresh Token Rotation: Implements short-lived JWT Access Tokens (15-min TTL) and 32-byte cryptographically secure random Refresh Tokens. Only the SHA-256 digest is persisted in refresh_tokens. Calling POST /auth/refresh rotates the token pair and revokes the used token.
• Compromised Token Theft Reuse Detection: Replaying an already-revoked refresh token immediately invalidates all active sessions for that user ID and logs a critical security alert.
• Domain & Clean Architecture (Repository Pattern): Introduces StudentRepository and RefreshTokenRepository protocols with Fluent database implementations to decouple persistence from controllers and GraphQL resolvers.
• Concurrency & Database Pool Optimization: Tunes PostgreSQL and MySQL connection pools (maxConnectionsPerEventLoop: 8, connectionPoolTimeout: .seconds(10)) and enforces in-memory SQLite isolation for test environments.
• Enterprise Standard Error Envelope: Introduces UnifiedErrorMiddleware formatting all 4xx/5xx API responses into predictable { "error": { "code": "...", "message": "...", "timestamp": "..." } } contracts while preserving domain error identifiers (EMAIL_ALREADY_EXISTS, etc.).

---

Key Changes

1. Models & Migrations:
   • RefreshToken.swift: Fluent model tracking token hash, user reference with cascade deletion, expiry, and revocation flag.
   • CreateRefreshToken.swift: Migration defining refresh_tokens schema with unique index on token_hash.
2. Repositories:
   • StudentRepository.swift: Protocol and DatabaseStudentRepository implementation.
   • RefreshTokenRepository.swift: Protocol and DatabaseRefreshTokenRepository implementation.
3. Services & Controllers:
   • TokenService.swift: Conforms to TokenServiceProtocol with SHA-256 digest hashing, token pair generation, rotation with reuse detection, and session revocation.
   • StudentService.swift: Refactored to operate through StudentRepository.
   • AuthController.swift: Added POST /auth/refresh, enhanced POST /auth/login to return dual tokens (with backward compatibility), and updated POST /auth/logout to revoke refresh tokens.
   • UnifiedErrorMiddleware.swift: Enterprise JSON error response pipeline.
   • configure.swift: Registered CreateRefreshToken migration, connection pool tuning, and test environment isolation.
4. Integration Test Suite:
   • Added integration tests for dual-token login, refresh token rotation, token reuse detection, invalid token rejection, and unified error envelopes.
   • All 125 tests passing.

---

Verification

􁁛  Test "Login returns dual token pair (access + refresh token)" passed
􁁛  Test "Refresh token rotation: valid refresh token issues new pair and revokes old" passed
􁁛  Test "Refresh token reuse detection: replaying revoked token revokes entire token family" passed
􁁛  Test "Refresh token: invalid token returns 401 unauthorized" passed
􁁛  Test "Enterprise Unified Error: 401 returns standardized error envelope" passed
􁁛  Suite "App Tests with DB" passed after 30.662 seconds.
􁁛  Test run with 125 tests in 1 suite passed after 30.663 seconds.

[nishark90]

Assessment: Not production-safe yet
The PostgreSQL/MySQL path is close, but the refresh-token security guarantees currently depend on database-specific row-lock behavior and have a race during reuse-detection cleanup. The SQLite test path does not validate the same guarantees as production.

Blocking findings
Reuse detection revokes sessions outside the transaction that detected reuse. In TokenService.rotateRefreshToken, TokenReuseError causes the transaction to roll back, and revokeAll(forUserID:) is then executed using a separate database operation. A legitimate refresh can commit a newly issued token between the rollback and revokeAll, leaving an active token after compromise detection. Move revokeAll into the original transaction and serialize it with the consumed token/user row, or introduce a per-user session-version/family-generation check that is updated atomically with reuse detection.
TokenService.swift#L84-L121

The implementation is not actually atomic on every configured database. The code uses SELECT ... FOR UPDATE followed by a separate read and a separate model save. This is safe only when the driver supports row locks and all operations remain on the same transaction connection. SQLite does not provide the same FOR UPDATE semantics, yet the test suite explicitly defaults to in-memory SQLite. Either implement an atomic conditional update (is_revoked = false and expires_at > now) or add database-specific write-lock handling and a production-like PostgreSQL/MySQL concurrency test.
RefreshTokenRepository.swift#L32-L61

The expired-token mutation is rolled back. consumeIfActive sets isRevoked = true for an expired token and returns .expired, but rotateRefreshToken then throws inside the transaction. The transaction therefore rolls back the revocation, so expired tokens remain unrevoked indefinitely. This is not an immediate token-replay vulnerability because expiry is checked on every use, but it defeats the lifecycle invariant and causes repeated writes/queries for the same expired token. Either commit the expiry revocation separately or treat expiry as a successful transactional state transition before returning the 401 response.
RefreshTokenRepository.swift#L53-L57
TokenService.swift#L100-L101

Recommended transaction shape
For PostgreSQL, the strongest implementation is an update-first consume:

SQL
UPDATE refresh_tokens
SET is_revoked = TRUE
WHERE token_hash = $1
AND is_revoked = FALSE
AND expires_at > CURRENT_TIMESTAMP
RETURNING id, user_id, expires_at;
Execute that inside the same transaction that creates the replacement token.

One returned row: the request won; issue the new pair.
Zero rows: perform a lookup only to distinguish not-found, expired, or already-revoked.
Already-revoked: revoke all sessions inside the same transaction, preferably after locking the user/session-generation row.
Expired: mark it revoked in a committed transaction, then return 401.
For MySQL versions without RETURNING, use:

SQL
UPDATE refresh_tokens
SET is_revoked = TRUE
WHERE token_hash = ?
AND is_revoked = FALSE
AND expires_at > CURRENT_TIMESTAMP;
Then inspect the affected-row count. The update must execute inside a transaction, with an appropriate index on token_hash.

Additional database concerns
revokeAll(forUserID:) is a bulk update, but it is not coordinated with token creation. Any login or refresh path that can create a token should use the same user/session lock or generation mechanism if “revoke all” must be absolute.
RefreshTokenRepository.swift#L72-L77
The migration has a unique index on token_hash, which is correct, and indexes user_id and expires_at. The consume path should also explicitly rely on the unique hash index; that is important for predictable locking and update plans.
CreateRefreshToken.swift#L8-L16
The concurrency test should run against PostgreSQL or MySQL. An in-memory SQLite test can pass while failing to prove production row-lock semantics.
Merge recommendation
I would request changes before approval:

Replace the read-then-save consume flow with an atomic conditional update.
Perform reuse detection and session-family/user revocation within one coordinated transaction.
Add a real PostgreSQL/MySQL concurrent-refresh integration test.
Fix the expired-token rollback behavior.
Until those are addressed, the PR demonstrates the intended token lifecycle but does not yet prove single-use rotation and compromise invalidation under production concurrency.

[nishark90]

I checked the current code in the PR branch, not the comment’s resolved state, and there is no remaining review feedback from that comment that is still unaddressed.

What the reviewer asked for, and whether the code now satisfies it:

Reuse detection and session-family revocation in the same transaction

Addressed in Sources/StudentAppBackend/Services/TokenService.swift
rotateRefreshToken performs the consume check and, on reuse detection, immediately revokes the user’s sessions inside the same database transaction closure.
Atomic consume path instead of read-then-save

Addressed in Sources/StudentAppBackend/Repositories/RefreshTokenRepository.swift
consumeIfActive uses conditional UPDATE logic with WHERE token_hash = ... AND is_revoked = false AND expires_at > now, with driver-specific handling for PostgreSQL/MySQL/SQLite.
Expired-token revocation not being rolled back

Addressed in the same repo file and token service flow
The expired branch marks the token revoked before returning the unauthorized result.
Production DB concurrency validation

Addressed in Tests/StudentAppBackendTests/StudentAppBackendTests.swift
The suite includes PostgreSQL/MySQL concurrent refresh tests and a single-use rotation check.
Unique hash index / database access pattern

Addressed in Sources/StudentAppBackend/Migrations/CreateRefreshToken.swift
The migration creates the unique token_hash index and the repo uses hash-based lookup and revocation.
Bottom line:

Unresolved reviewer feedback: none
Current code status: the reviewer’s blocking concerns appear to be implemented
If you want, I can also turn this into a short “review status” summary you can paste into the PR conversation.