diff --git a/database/08-migrations.sql b/database/08-migrations.sql index 1ba226d..68457ff 100644 --- a/database/08-migrations.sql +++ b/database/08-migrations.sql @@ -1,17 +1,13 @@ --- Migration: add refresh_token column to users table --- Date: 2026-03-28 --- Reason: Support JWT refresh token invalidation on logout (OWASP A07) +-- Migration: remove raw refresh_token from users +-- Date: 2026-10-25 +-- Reason: user_sessions.token_hash is the single source of truth. A raw JWT +-- must never be stored because a database disclosure could reuse it. +-- OWASP A02/A07: revoke sessions by SHA-256 hash, not by a reusable token. -ALTER TABLE users - ADD COLUMN IF NOT EXISTS refresh_token VARCHAR(512); - --- Index for fast lookup on refresh token validation -CREATE INDEX IF NOT EXISTS idx_users_refresh_token - ON users(refresh_token) - WHERE refresh_token IS NOT NULL; +DROP INDEX IF EXISTS idx_users_refresh_token; -COMMENT ON COLUMN users.refresh_token IS - 'Active refresh token. NULL = logged out. Invalidated on logout.'; +ALTER TABLE users + DROP COLUMN IF EXISTS refresh_token; -- Migration: add WALLET_RESTORE to audit_action enum -- Date: 2026-05-02 @@ -19,4 +15,4 @@ COMMENT ON COLUMN users.refresh_token IS -- All other wallet admin ops (suspend, close) already have their own action. -- Using ALTER TYPE ... ADD VALUE because PostgreSQL enums are append-only. -ALTER TYPE audit_action ADD VALUE IF NOT EXISTS 'WALLET_RESTORE'; \ No newline at end of file +ALTER TYPE audit_action ADD VALUE IF NOT EXISTS 'WALLET_RESTORE'; diff --git a/docs/IMPLEMENTATION-PLAN.md b/docs/IMPLEMENTATION-PLAN.md index a7aaacc..5bb4122 100644 --- a/docs/IMPLEMENTATION-PLAN.md +++ b/docs/IMPLEMENTATION-PLAN.md @@ -123,7 +123,7 @@ The former public seed is no longer used and a controlled bootstrap now exists. **Current problem** -`SessionService` stores a SHA-256 token hash in `user_sessions`, but `AuthService` also assigns the original refresh JWT to `User.refreshToken`. This contradicts the requirement that reusable tokens are not stored directly. +`SessionService` stores a SHA-256 token hash in `user_sessions`, but `AuthService` also assigned the original refresh JWT to `User.refreshToken`. This duplicate raw-token store was removed; `user_sessions` is now the only refresh-session source of truth. **Expected behavior** @@ -143,10 +143,17 @@ The former public seed is no longer used and a controlled bootstrap now exists. **Acceptance criteria** -- [ ] No database column stores the original refresh JWT. -- [ ] Login, refresh, logout, single-session revocation, and all-session revocation still work. -- [ ] Tests prove that a stored session value cannot be sent as a refresh token. -- [ ] Schema migration preserves or explicitly invalidates existing sessions safely. +- [x] No database column stores the original refresh JWT. +- [x] Login, refresh, logout, single-session revocation, and all-session revocation still work. +- [x] Tests prove that a stored session value cannot be sent as a refresh token. +- [x] Schema migration preserves existing sessions safely. + +**Verification note — 2026-10-25** + +- The migration removed `users.refresh_token` and its index while preserving the three existing `user_sessions` rows. +- A real local flow returned `201` on registration, `200` on refresh, `200` on logout and `401` when the revoked refresh token was reused. +- The resulting `user_sessions.token_hash` value had 64 characters (SHA-256 hex), and the raw-token column was absent. +- `./scripts/test.sh` passed: 112 tests, 0 failures and 0 errors. ### SEC-004 — Implement and verify refresh-token rotation diff --git a/src/main/java/com/wallet/secure/auth/entity/Session.java b/src/main/java/com/wallet/secure/auth/entity/Session.java index 1eaef65..4eaf580 100644 --- a/src/main/java/com/wallet/secure/auth/entity/Session.java +++ b/src/main/java/com/wallet/secure/auth/entity/Session.java @@ -32,8 +32,7 @@ * A SHA-256 hash of a JWT cannot be reversed to obtain the token. * OWASP A02: sensitive tokens never stored in plain text in DB. * - * WHY this table exists alongside refreshToken in User entity: - * User.refreshToken → simple single-device session (current behavior) + * This table is the single source of truth for refresh-token sessions: * user_sessions → multi-device session management * → "Show me all devices where I'm logged in" * → "Log out from my phone but keep my laptop session" @@ -159,4 +158,4 @@ public void revokeNow() { public boolean isActive() { return !revoked && Instant.now().isBefore(expiresAt); } -} \ No newline at end of file +} diff --git a/src/main/java/com/wallet/secure/auth/service/AuthService.java b/src/main/java/com/wallet/secure/auth/service/AuthService.java index eaa7305..a420c98 100644 --- a/src/main/java/com/wallet/secure/auth/service/AuthService.java +++ b/src/main/java/com/wallet/secure/auth/service/AuthService.java @@ -64,7 +64,7 @@ public class AuthService { * Flow: * 1. Delegate account creation to UserService (validates email, hashes password) * 2. Generate access + refresh tokens - * 3. Save refresh token in DB + * 3. Store only the SHA-256 refresh-token hash in user_sessions * 4. Return AuthResponse * * @param request DTO with email and password (validated by @Valid at controller) @@ -102,11 +102,11 @@ public ApiResponse register(RegisterRequest request, HttpServletRe // --- Private Helpers /** - * Generates both tokens and persists the refresh token in DB. + * Generates both tokens and stores only the refresh-token hash in user_sessions. * Single method for register and login - avoids duplication. *

- * Why persist the refresh token: - * Enables real logout - setRefreshToken(null) = session revoked. + * Why persist a session hash: + * Enables real logout - the matching user_sessions row is revoked. *

* OWASP A07: without DB persistence, logout is just client-side */ @@ -114,10 +114,7 @@ private AuthResponse generateAndSaveTokens(User user, String ipAddress, String u String accessToken = jwtService.generateAccessToken(user.getEmail()); String refreshToken = jwtService.generateRefreshToken(user.getEmail()); - // Persist refresh token - enable revocation on logout - user.setRefreshToken(refreshToken); - userRepository.save(user); - + // OWASP A02: SessionService persists SHA-256(refreshToken), never the raw JWT. sessionService.createSession(user, refreshToken, ipAddress, userAgent); return AuthResponse.of( @@ -241,7 +238,7 @@ public ApiResponse login(LoginRequest request, HttpServletRequest *

* Flow: * 1. Validate JWT signature and expiration of refresh token - * 2. Check refresh token exists in DB (not revoked by logout) + * 2. Check the refresh-token hash exists in user_sessions (not revoked by logout) * 3. Generate new access token * 4. Return new access token (refresh token stays the same) * @@ -263,13 +260,14 @@ public ApiResponse refresh(RefreshTokenRequest request) { User user = userRepository.findByEmail(email) .orElseThrow(() -> new InvalidCredentialsException("Invalid refresh token")); - // Step 3 - verify token matches DB (not revoked) - // OWASP A07: token revocation - logout deletes the DB token - if (!refreshToken.equals(user.getRefreshToken())) { - log.warn("Refresh token mismatch for userId= {} - possible token reuse after logout", user.getId()); - throw new InvalidCredentialsException("Refresh token has been revoked"); + // Step 3 - user_sessions is the only revocation store. Its hash must + // belong to the same user named in the signed refresh JWT. + // OWASP A02/A07: never store or compare a raw reusable refresh token. + var session = sessionService.validateSession(refreshToken); + if (!session.getUser().getId().equals(user.getId())) { + log.warn("Refresh token session ownership mismatch for userId={}", user.getId()); + throw new InvalidCredentialsException("Invalid refresh token"); } - sessionService.validateSession(refreshToken); // Step 4 - generate only a new access token // Refresh token stays the same - client already has it @@ -286,12 +284,12 @@ public ApiResponse refresh(RefreshTokenRequest request) { // --- Logout /** - * Revokes the refresh token by removing it from DB + * Revokes the refresh-token session by marking its hash as revoked. *

- * Why logout only removes the refresh token (not access token): + * Why logout revokes only the refresh-token session (not the access token): * Access tokens are stateless - they cannot be "revoked" without a blacklist. * But they expire in 15 minutes - acceptable window. - * Removing the refresh token ensures: + * Revoking the refresh-token session ensures: * -> The user cannot silently obtain a new access token * -> After 15 min the access token expires and the session truly ends *

@@ -305,10 +303,8 @@ public ApiResponse logout(String email, LogoutRequest logoutRequest, HttpS User user = userRepository.findByEmail(email) .orElseThrow(() -> new InvalidCredentialsException("User not found")); - // Remove refresh token from DB - future refresh calls will fail - user.setRefreshToken(null); - userRepository.save(user); - + // SessionService hashes the presented token, finds its session and revokes it. + // No raw refresh JWT is persisted in users. sessionService.revokeByToken(logoutRequest.getRefreshToken()); //OWASP A09: log every logout - session termination must be auditable diff --git a/src/main/java/com/wallet/secure/user/entity/User.java b/src/main/java/com/wallet/secure/user/entity/User.java index 7d7dafa..3f9aacc 100644 --- a/src/main/java/com/wallet/secure/user/entity/User.java +++ b/src/main/java/com/wallet/secure/user/entity/User.java @@ -68,16 +68,6 @@ public class User { @Column(name = "password_hash", nullable = false) private String passwordHash; - /** - * Active refresh token for this user. - * NULL = user is logged out or has never logged in. - * Set on login, cleared on logout. - * OWASP A07: token revocation — logout truly invalidates the session. - * OWASP A02: stored as BCrypt hash, never the raw token. - */ - @Column(name = "refresh_token", length = 512) - private String refreshToken; - /** * Role-based access control. * USER = standard client, ADMIN = superadmin, MANAGER = support. @@ -220,4 +210,4 @@ public void resetFailedLoginAttempts() { this.failedLoginAttempts = 0; this.lockedUntil = null; } -} \ No newline at end of file +} diff --git a/src/test/java/com/wallet/secure/auth/service/AuthServiceTest.java b/src/test/java/com/wallet/secure/auth/service/AuthServiceTest.java index ec06b36..c1f3dea 100644 --- a/src/test/java/com/wallet/secure/auth/service/AuthServiceTest.java +++ b/src/test/java/com/wallet/secure/auth/service/AuthServiceTest.java @@ -5,6 +5,7 @@ import com.wallet.secure.auth.dto.LoginRequest; import com.wallet.secure.auth.dto.LogoutRequest; import com.wallet.secure.auth.dto.RefreshTokenRequest; +import com.wallet.secure.auth.entity.Session; import com.wallet.secure.auth.security.JwtService; import com.wallet.secure.common.exception.InvalidCredentialsException; import com.wallet.secure.common.response.ApiResponse; @@ -29,6 +30,7 @@ import java.util.Optional; import java.util.UUID; +import java.time.Instant; import static org.assertj.core.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; @@ -134,7 +136,6 @@ void register_validRequest_returnsTokens() { // After registration, AuthService loads the user by email to generate tokens when(userRepository.findByEmail(TEST_EMAIL)) .thenReturn(Optional.of(testUser)); - when(userRepository.save(any(User.class))).thenReturn(testUser); // WHEN ApiResponse response = authService.register(request, httpRequest); @@ -144,8 +145,9 @@ void register_validRequest_returnsTokens() { assertThat(response.getData().getAccessToken()).isEqualTo(ACCESS_TOKEN); assertThat(response.getData().getRefreshToken()).isEqualTo(REFRESH_TOKEN); - // Refresh token was persisted in DB — enables future revocation on logout - verify(userRepository, times(1)).save(any(User.class)); + // The raw token is not stored on User; SessionService stores only its SHA-256 hash. + verify(userRepository, never()).save(any(User.class)); + verify(sessionService).createSession(eq(testUser), eq(REFRESH_TOKEN), anyString(), anyString()); // Registration audit references users.id: the user must be flushed // before AuditService joins the same transaction (OWASP A09). @@ -192,7 +194,6 @@ void login_validCredentials_returnsTokens() { when(userRepository.findByEmail(TEST_EMAIL)) .thenReturn(Optional.of(testUser)); - when(userRepository.save(any(User.class))).thenReturn(testUser); // WHEN ApiResponse response = authService.login(request, httpRequest); @@ -217,7 +218,6 @@ void login_success_resetsFailedAttempts() { .thenReturn(mock(Authentication.class)); when(userRepository.findByEmail(TEST_EMAIL)) .thenReturn(Optional.of(testUser)); - when(userRepository.save(any(User.class))).thenReturn(testUser); // WHEN authService.login(request, httpRequest); @@ -241,7 +241,6 @@ void login_success_updatesLastLoginAt() { .thenReturn(mock(Authentication.class)); when(userRepository.findByEmail(TEST_EMAIL)) .thenReturn(Optional.of(testUser)); - when(userRepository.save(any(User.class))).thenReturn(testUser); // WHEN authService.login(request, httpRequest); @@ -285,8 +284,6 @@ class RefreshTests { @DisplayName("returns new access token with valid refresh token") void refresh_validToken_returnsNewAccessToken() { // GIVEN - testUser.setRefreshToken(REFRESH_TOKEN); - RefreshTokenRequest request = mock(RefreshTokenRequest.class); when(request.getRefreshToken()).thenReturn(REFRESH_TOKEN); @@ -294,6 +291,8 @@ void refresh_validToken_returnsNewAccessToken() { when(jwtService.extractEmail(REFRESH_TOKEN)).thenReturn(TEST_EMAIL); when(userRepository.findByEmail(TEST_EMAIL)) .thenReturn(Optional.of(testUser)); + when(sessionService.validateSession(REFRESH_TOKEN)) + .thenReturn(activeSessionFor(testUser)); String newAccessToken = "new.access.jwt.token"; when(jwtService.generateAccessToken(TEST_EMAIL)).thenReturn(newAccessToken); @@ -326,8 +325,7 @@ void refresh_invalidSignature_throwsException() { @Test @DisplayName("throws exception when refresh token was revoked after logout — OWASP A07") void refresh_revokedToken_throwsException() { - // GIVEN — user logged out → refreshToken in DB is null - testUser.setRefreshToken(null); + // GIVEN — user logged out → the session hash is marked revoked RefreshTokenRequest request = mock(RefreshTokenRequest.class); when(request.getRefreshToken()).thenReturn(REFRESH_TOKEN); @@ -335,6 +333,8 @@ void refresh_revokedToken_throwsException() { when(jwtService.extractEmail(REFRESH_TOKEN)).thenReturn(TEST_EMAIL); when(userRepository.findByEmail(TEST_EMAIL)) .thenReturn(Optional.of(testUser)); + doThrow(new InvalidCredentialsException("Session has been revoked or expired")) + .when(sessionService).validateSession(REFRESH_TOKEN); // WHEN / THEN // OWASP A07: token revoked after logout — cannot be reused @@ -344,11 +344,10 @@ void refresh_revokedToken_throwsException() { } @Test - @DisplayName("throws exception when token does not match DB — token reuse attack") + @DisplayName("throws exception when token session belongs to another user — OWASP A01") void refresh_tokenMismatch_throwsException() { - // GIVEN — attacker uses an old refresh token - // the user already has a new one in DB (token rotation scenario) - testUser.setRefreshToken("different.token.in.db"); + // GIVEN — a valid token hash must still belong to the JWT subject. + User anotherUser = User.builder().id(UUID.randomUUID()).email("other@test.com").build(); RefreshTokenRequest request = mock(RefreshTokenRequest.class); when(request.getRefreshToken()).thenReturn(REFRESH_TOKEN); @@ -356,12 +355,14 @@ void refresh_tokenMismatch_throwsException() { when(jwtService.extractEmail(REFRESH_TOKEN)).thenReturn(TEST_EMAIL); when(userRepository.findByEmail(TEST_EMAIL)) .thenReturn(Optional.of(testUser)); + when(sessionService.validateSession(REFRESH_TOKEN)) + .thenReturn(activeSessionFor(anotherUser)); // WHEN / THEN // OWASP A07: detects stale token reuse attempt assertThatThrownBy(() -> authService.refresh(request)) .isInstanceOf(InvalidCredentialsException.class) - .hasMessageContaining("revoked"); + .hasMessageContaining("Invalid refresh token"); } } @@ -372,14 +373,12 @@ void refresh_tokenMismatch_throwsException() { class LogoutTests { @Test - @DisplayName("clears refresh token from DB on logout — OWASP A07") - void logout_authenticatedUser_clearsRefreshToken() { + @DisplayName("revokes hashed refresh-token session on logout — OWASP A07") + void logout_authenticatedUser_revokesSession() { // GIVEN LogoutRequest logoutRequest = mock(LogoutRequest.class); when(logoutRequest.getRefreshToken()).thenReturn(REFRESH_TOKEN); - testUser.setRefreshToken(REFRESH_TOKEN); when(userRepository.findByEmail(TEST_EMAIL)).thenReturn(Optional.of(testUser)); - when(userRepository.save(any(User.class))).thenReturn(testUser); doNothing().when(sessionService).revokeByToken(anyString()); // WHEN @@ -387,8 +386,8 @@ void logout_authenticatedUser_clearsRefreshToken() { // THEN assertThat(response.isSuccess()).isTrue(); - assertThat(testUser.getRefreshToken()).isNull(); verify(sessionService).revokeByToken(REFRESH_TOKEN); + verify(userRepository, never()).save(any(User.class)); } @Test @@ -405,4 +404,12 @@ void logout_userNotFound_throwsException() { verify(userRepository, never()).save(any()); } } + + private Session activeSessionFor(User user) { + return Session.builder() + .user(user) + .tokenHash("a".repeat(64)) + .expiresAt(Instant.now().plusSeconds(60)) + .build(); + } } diff --git a/src/test/java/com/wallet/secure/auth/service/SessionServiceTest.java b/src/test/java/com/wallet/secure/auth/service/SessionServiceTest.java index 26c137c..55a87cc 100644 --- a/src/test/java/com/wallet/secure/auth/service/SessionServiceTest.java +++ b/src/test/java/com/wallet/secure/auth/service/SessionServiceTest.java @@ -191,6 +191,22 @@ void validateSession_sessionNotFound_throwsException() { .hasMessageContaining("revoked"); } + @Test + @DisplayName("rejects a stored token hash when presented as a refresh token — OWASP A02") + void validateSession_storedHashCannotBeUsedAsRefreshToken() { + // GIVEN — an attacker obtained the database value, not the raw JWT. + String storedHash = sessionService.hashToken(RAW_TOKEN); + String lookupHash = sessionService.hashToken(storedHash); + when(sessionRepository.findByTokenHash(lookupHash)).thenReturn(Optional.empty()); + + // WHEN / THEN — validateSession hashes the supplied value again. + assertThatThrownBy(() -> sessionService.validateSession(storedHash)) + .isInstanceOf(InvalidCredentialsException.class) + .hasMessageContaining("revoked"); + + verify(sessionRepository).findByTokenHash(lookupHash); + } + @Test @DisplayName("throws when session is revoked — user already logged out") void validateSession_revokedSession_throwsException() {