diff --git a/src/main/java/com/ddf/vodsystem/controllers/UserController.java b/src/main/java/com/ddf/vodsystem/controllers/UserController.java index c0e4241..eb1cb59 100644 --- a/src/main/java/com/ddf/vodsystem/controllers/UserController.java +++ b/src/main/java/com/ddf/vodsystem/controllers/UserController.java @@ -80,20 +80,20 @@ public class UserController { } /** - * Exchanges a refresh token for a new refresh token and JWT, replacing both cookies. + * Exchanges the {@code refreshToken} cookie for a new refresh token and JWT, replacing both cookies. * - * @param token the payload carrying the current refresh token - * @param response the response the new cookies are written to - * @param request the incoming request, used to decide the cookies' {@code Secure} flag + * @param refreshToken the refresh token from the {@code refreshToken} cookie, if present + * @param request the incoming request, used to decide the cookies' {@code Secure} flag + * @param response the response the new cookies are written to * @return {@code 200 OK} with no data - * @throws NotAuthenticated if the refresh token is rejected + * @throws NotAuthenticated if the refresh token is missing or rejected */ @PostMapping("/refresh") public ResponseEntity> refresh( - @RequestBody Token token, - HttpServletResponse response, - HttpServletRequest request) { - TokenPackage tokenPackage = userService.refresh(token.token()); + @CookieValue(name = "refreshToken", required = false) String refreshToken, + HttpServletRequest request, + HttpServletResponse response) { + TokenPackage tokenPackage = userService.refresh(refreshToken); addCookies(request, response, tokenPackage.accessToken(), tokenPackage.refreshToken()); return ResponseEntity.ok( @@ -102,35 +102,46 @@ public class UserController { } /** - * Logs the user out by revoking the session tied to the {@code refreshToken} cookie. - * The auth cookies are not cleared. + * Logs the user out by revoking the session tied to the {@code refreshToken} cookie and clearing + * both auth cookies. Succeeds even if no refresh token cookie is present. * * @param refreshToken the refresh token from the {@code refreshToken} cookie, if present + * @param request the incoming request, used to decide the cookies' {@code Secure} flag + * @param response the response the expired cookies are written to * @return {@code 200 OK} confirming logout */ @PostMapping("/logout") public ResponseEntity> logout( - @CookieValue(name = "refreshToken", required = false) String refreshToken) { + @CookieValue(name = "refreshToken") String refreshToken, + HttpServletRequest request, + HttpServletResponse response) { userService.logout(refreshToken); + writeCookies(request, response, "", Duration.ZERO, "", Duration.ZERO); return ResponseEntity.ok( new APIResponse<>(SUCCESS, "Logged out successfully", null) ); } - /** - * Writes the {@code accessToken} cookie ({@code SameSite=Lax}, path {@code /}) and the - * {@code refreshToken} cookie ({@code SameSite=Strict}, path {@code /api/v1/users}), both HttpOnly - * and {@code Secure} when the request is. - */ private void addCookies( HttpServletRequest request, HttpServletResponse response, String accessToken, String refreshToken) { + writeCookies(request, response, accessToken, jwtExpiration, refreshToken, refreshTokenExpiration); + } + + + private void writeCookies( + HttpServletRequest request, + HttpServletResponse response, + String accessToken, + Duration accessTokenMaxAge, + String refreshToken, + Duration refreshTokenMaxAge) { ResponseCookie accessTokenCookie = ResponseCookie.from("accessToken", accessToken) .httpOnly(true) - .maxAge(jwtExpiration.getSeconds()) + .maxAge(accessTokenMaxAge) .sameSite("Lax") .secure(request.isSecure()) .path(ACCESS_COOKIE_PATH) @@ -138,7 +149,7 @@ public class UserController { ResponseCookie refreshTokenCookie = ResponseCookie.from("refreshToken", refreshToken) .httpOnly(true) - .maxAge(refreshTokenExpiration.getSeconds()) + .maxAge(refreshTokenMaxAge) .sameSite("Strict") .secure(request.isSecure()) .path(REFRESH_COOKIE_PATH) diff --git a/src/main/java/com/ddf/vodsystem/security/TokenService.java b/src/main/java/com/ddf/vodsystem/security/TokenService.java index 1442ff6..259e966 100644 --- a/src/main/java/com/ddf/vodsystem/security/TokenService.java +++ b/src/main/java/com/ddf/vodsystem/security/TokenService.java @@ -158,13 +158,14 @@ public class TokenService { /** * Revokes a refresh token and its whole token family, ending the session. Does nothing if the - * token is unknown, and leaves existing revocation timestamps unchanged. + * token is missing or unknown, and leaves existing revocation timestamps unchanged. * - * @param hexToken the raw refresh token as a hex string + * @param hexToken the raw refresh token as a hex string, may be {@code null} * @throws NotAuthenticated if the token is not valid hex */ + @Transactional public void revoke(String hexToken) { - Instant now = Instant.now(clock); + if (hexToken == null || hexToken.isBlank()) return; byte[] hash; try { @@ -173,14 +174,22 @@ public class TokenService { throw new NotAuthenticated("Malformed refresh token"); } - Optional found = refreshTokenRepository.findByHash(sha256(hash)); + Optional found = refreshTokenRepository.findByHash(hash); if (found.isEmpty()) return; + Instant now = Instant.now(clock); RefreshToken refreshToken = found.get(); TokenFamily family = refreshToken.getTokenFamily(); - if (family.getRevokedAt() == null) family.setRevokedAt(now); - if (refreshToken.getRevokedAt() == null) refreshToken.setRevokedAt(now); + if (family.getRevokedAt() == null) { + family.setRevokedAt(now); + tokenFamilyRepository.save(family); + } + + if (refreshToken.getRevokedAt() == null) { + refreshToken.setRevokedAt(now); + refreshTokenRepository.save(refreshToken); + } } /** diff --git a/src/main/java/com/ddf/vodsystem/services/UserService.java b/src/main/java/com/ddf/vodsystem/services/UserService.java index 801f775..968895f 100644 --- a/src/main/java/com/ddf/vodsystem/services/UserService.java +++ b/src/main/java/com/ddf/vodsystem/services/UserService.java @@ -99,7 +99,7 @@ public class UserService { * Ends the session tied to a refresh token by revoking its token family. * Already-issued JWT access tokens stay valid until they expire. * - * @param refreshToken the raw refresh token as a hex string + * @param refreshToken the raw refresh token as a hex string, may be {@code null} * @throws NotAuthenticated if the token is not valid hex */ public void logout(String refreshToken) { diff --git a/src/test/java/com/ddf/vodsystem/security/TokenServiceTest.java b/src/test/java/com/ddf/vodsystem/security/TokenServiceTest.java index a1ab1d2..68b62ee 100644 --- a/src/test/java/com/ddf/vodsystem/security/TokenServiceTest.java +++ b/src/test/java/com/ddf/vodsystem/security/TokenServiceTest.java @@ -17,6 +17,7 @@ import org.mockito.Captor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import java.security.MessageDigest; import java.time.*; import java.util.HexFormat; import java.util.Optional; @@ -518,20 +519,110 @@ class TokenServiceTest { verify(refreshTokenRepository, never()).save(any()); verifyNoInteractions(tokenFamilyRepository); } + } - private TokenFamily activeFamily() { - TokenFamily tokenFamily = new TokenFamily(); - tokenFamily.setId(1L); - tokenFamily.setUser(createUser(1L, "user", "user")); - tokenFamily.setExpiresAt(clock.instant().plusSeconds(10)); - return tokenFamily; + @Nested + @DisplayName("revoke()") + class revoke { + @Test + void knownToken_revokesTokenAndFamily() { + TokenFamily tokenFamily = activeFamily(); + RefreshToken refreshToken = tokenIn(tokenFamily); + when(refreshTokenRepository.findByHash(any())).thenReturn(Optional.of(refreshToken)); + + tokenService.revoke(rawToken); + + assertEquals(clock.instant(), tokenFamily.getRevokedAt()); + assertEquals(clock.instant(), refreshToken.getRevokedAt()); } - private RefreshToken tokenIn(TokenFamily tokenFamily) { - RefreshToken refreshToken = new RefreshToken(); - refreshToken.setTokenFamily(tokenFamily); - refreshToken.setExpiresAt(clock.instant().plusSeconds(10)); - return refreshToken; + @Test + void knownToken_savesTokenAndFamily() { + TokenFamily tokenFamily = activeFamily(); + RefreshToken refreshToken = tokenIn(tokenFamily); + when(refreshTokenRepository.findByHash(any())).thenReturn(Optional.of(refreshToken)); + + tokenService.revoke(rawToken); + + verify(tokenFamilyRepository).save(tokenFamily); + verify(refreshTokenRepository).save(refreshToken); + } + + @Test + void knownToken_looksUpBySingleSha256Hash() throws Exception { + when(refreshTokenRepository.findByHash(any())).thenReturn(Optional.empty()); + + tokenService.revoke(rawToken); + + byte[] expectedHash = MessageDigest.getInstance("SHA-256").digest(HEX.parseHex(rawToken)); + verify(refreshTokenRepository).findByHash(expectedHash); + } + + @Test + void alreadyRevoked_keepsOriginalTimestampsAndDoesNotSave() { + Instant revokedAt = clock.instant(); + TokenFamily tokenFamily = activeFamily(); + tokenFamily.setRevokedAt(revokedAt); + RefreshToken refreshToken = tokenIn(tokenFamily); + refreshToken.setRevokedAt(revokedAt); + when(refreshTokenRepository.findByHash(any())).thenReturn(Optional.of(refreshToken)); + + clock.advance(Duration.ofMinutes(1)); + tokenService.revoke(rawToken); + + assertEquals(revokedAt, tokenFamily.getRevokedAt()); + assertEquals(revokedAt, refreshToken.getRevokedAt()); + verify(tokenFamilyRepository, never()).save(any()); + verify(refreshTokenRepository, never()).save(any()); + } + + @Test + void unknownToken_doesNothing() { + when(refreshTokenRepository.findByHash(any())).thenReturn(Optional.empty()); + + tokenService.revoke(rawToken); + + verify(refreshTokenRepository, never()).save(any()); + verifyNoInteractions(tokenFamilyRepository); + } + + @Test + void nullToken_doesNothing() { + tokenService.revoke(null); + + verifyNoInteractions(refreshTokenRepository, tokenFamilyRepository); + } + + @Test + void blankToken_doesNothing() { + tokenService.revoke(""); + + verifyNoInteractions(refreshTokenRepository, tokenFamilyRepository); + } + + @Test + void malformedToken_throwsNotAuthenticated() { + assertThrows( + NotAuthenticated.class, + () -> tokenService.revoke("not-hex") + ); + + verifyNoInteractions(refreshTokenRepository, tokenFamilyRepository); } } -} \ No newline at end of file + + private TokenFamily activeFamily() { + TokenFamily tokenFamily = new TokenFamily(); + tokenFamily.setId(1L); + tokenFamily.setUser(createUser(1L, "user", "user")); + tokenFamily.setExpiresAt(clock.instant().plusSeconds(10)); + return tokenFamily; + } + + private RefreshToken tokenIn(TokenFamily tokenFamily) { + RefreshToken refreshToken = new RefreshToken(); + refreshToken.setTokenFamily(tokenFamily); + refreshToken.setExpiresAt(clock.instant().plusSeconds(10)); + return refreshToken; + } +}