diff --git a/src/main/java/com/ibrasoft/lensbridge/controller/AuthController.java b/src/main/java/com/ibrasoft/lensbridge/controller/AuthController.java index 92bbf78..d6b9276 100644 --- a/src/main/java/com/ibrasoft/lensbridge/controller/AuthController.java +++ b/src/main/java/com/ibrasoft/lensbridge/controller/AuthController.java @@ -92,7 +92,7 @@ public ResponseEntity authenticateUser(@Valid @RequestBody LoginReq loginAttemptService.recordSuccessfulAttempt(clientKey); String jwt = jwtUtils.generateJwtToken(authentication); - RefreshToken refreshToken = refreshTokenService.createRefreshToken( + RefreshTokenService.IssuedRefreshToken issuedRefreshToken = refreshTokenService.createRefreshToken( user.getId(), request.getHeader("User-Agent"), getClientIpAddress(request)); // TODO: This is probably slow and should be fitted in the data model somewhere @@ -109,7 +109,7 @@ public ResponseEntity authenticateUser(@Valid @RequestBody LoginReq .collect(Collectors.toList()); return ResponseEntity.ok(new JwtResponse(jwt, - refreshToken.getTokenHash(), + issuedRefreshToken.plaintext(), user.getFirstName(), user.getLastName(), user.getId(), @@ -247,9 +247,10 @@ public ResponseEntity refreshToken(@Valid @RequestBody Tok String newAccessToken = jwtUtils.generateJwtToken(auth); refreshTokenService.revokeRefreshToken(requestRefreshToken); - RefreshToken newRefreshToken = refreshTokenService.createRefreshToken(userId, "Token refresh", "System"); + RefreshTokenService.IssuedRefreshToken newRefreshToken = + refreshTokenService.createRefreshToken(userId, "Token refresh", "System"); - return ResponseEntity.ok(new TokenRefreshResponse(newAccessToken, newRefreshToken.getToken())); + return ResponseEntity.ok(new TokenRefreshResponse(newAccessToken, newRefreshToken.plaintext())); } @Operation(operationId = "logout", diff --git a/src/main/java/com/ibrasoft/lensbridge/model/auth/RefreshToken.java b/src/main/java/com/ibrasoft/lensbridge/model/auth/RefreshToken.java index 1fbf2c5..e1077dc 100644 --- a/src/main/java/com/ibrasoft/lensbridge/model/auth/RefreshToken.java +++ b/src/main/java/com/ibrasoft/lensbridge/model/auth/RefreshToken.java @@ -29,6 +29,10 @@ public class RefreshToken { @GeneratedValue(strategy = GenerationType.UUID) private UUID id; + /** + * SHA-256 (hex) of the token handed to the client — never the token itself. See + * {@code RefreshTokenService#resolveHash}. + */ @NotBlank @Column(nullable = false, unique = true) private String tokenHash; @@ -49,10 +53,6 @@ public class RefreshToken { @Column(nullable = false) private boolean revoked; - public String getToken() { - return tokenHash; - } - public UUID getUserId() { return user != null ? user.getId() : null; } diff --git a/src/main/java/com/ibrasoft/lensbridge/service/RefreshTokenService.java b/src/main/java/com/ibrasoft/lensbridge/service/RefreshTokenService.java index ea347e3..7fb3dee 100644 --- a/src/main/java/com/ibrasoft/lensbridge/service/RefreshTokenService.java +++ b/src/main/java/com/ibrasoft/lensbridge/service/RefreshTokenService.java @@ -13,30 +13,62 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.time.Duration; import java.time.Instant; import java.util.Base64; +import java.util.HexFormat; import java.util.List; import java.util.Optional; import java.util.UUID; +/** + * Issues and redeems refresh tokens. + *

+ * The plaintext token exists only in the response to the client that asked for it. What is + * persisted is a SHA-256 digest of that value, so a read of {@code refresh_tokens} — a leaked + * backup, a misconfigured replica, SQL injection elsewhere — yields nothing that can be replayed + * against {@code /api/auth/refresh-token}. This mirrors {@link VerificationTokenService} and + * {@code service.agent.EnrollmentTokenService}. + *

+ * A plain digest (not bcrypt/argon2) is the right choice here: the input is 512 bits of + * {@link SecureRandom} output, so there is no guessable password to slow an attacker down to, and + * redemption has to stay a single indexed equality lookup. + */ @Service @RequiredArgsConstructor @Slf4j public class RefreshTokenService { + /** 64 random bytes -> 86 chars of url-safe base64. 512 bits of entropy. */ + private static final int TOKEN_RANDOM_BYTES = 64; + private final RefreshTokenRepository refreshTokenRepository; private final UserRepository userRepository; - + @Value("${lensbridge.app.refreshTokenExpirationMs:604800000}") // 7 days default private long refreshTokenDurationMs; - + @Value("${lensbridge.app.maxRefreshTokensPerUser:5}") private int maxRefreshTokensPerUser; - + + /** Optional extra input to the token hash. Empty disables it. */ + @Value("${lensbridge.app.refreshTokenPepper:}") + private String pepper; + private final SecureRandom secureRandom = new SecureRandom(); + /** + * A freshly minted token: the stored row, plus the plaintext to hand back to the client. + * The plaintext is not recoverable from the row and is never persisted, so this is the only + * point at which it is available. + */ + public record IssuedRefreshToken(RefreshToken token, String plaintext) { + } + public void invalidateAllRefreshTokensForUser(UUID userId) { List tokens = refreshTokenRepository.findByUser_IdAndRevokedFalse(userId); tokens.forEach(token -> token.setRevoked(true)); @@ -44,46 +76,49 @@ public void invalidateAllRefreshTokensForUser(UUID userId) { } /** - * Create a new refresh token for a user + * Create a new refresh token for a user. + * + * @return the persisted row and the plaintext token; only the caller of this method ever sees + * the plaintext. */ @Transactional - public RefreshToken createRefreshToken(UUID userId, String deviceInfo, String ipAddress) { + public IssuedRefreshToken createRefreshToken(UUID userId, String deviceInfo, String ipAddress) { // Clean up expired tokens first deleteExpiredTokensByUser(userId); - + // Check if user has too many active tokens long activeTokenCount = refreshTokenRepository.countByUser_IdAndRevokedFalse(userId); if (activeTokenCount >= maxRefreshTokensPerUser) { // Revoke oldest token List userTokens = refreshTokenRepository.findByUser_IdAndRevokedFalse(userId); - if (!userTokens.isEmpty()) { - RefreshToken oldestToken = userTokens.stream() + userTokens.stream() .min((t1, t2) -> t1.getCreatedDate().compareTo(t2.getCreatedDate())) - .orElse(null); - if (oldestToken != null) { - revokeRefreshToken(oldestToken.getTokenHash()); - } - } + .ifPresent(this::revokeToken); } Instant now = Instant.now(); - + + String plaintext = generateRefreshToken(); + RefreshToken refreshToken = RefreshToken.builder() - .tokenHash(generateRefreshToken()) + .tokenHash(resolveHash(plaintext)) .user(userRepository.findById(userId).orElseThrow(() -> new IllegalArgumentException("User not found"))) .expiryDate(now.plusSeconds(refreshTokenDurationMs / 1000)) .createdDate(now) .lastUsedDate(now) .revoked(false) .build(); - - return refreshTokenRepository.save(refreshToken); + + return new IssuedRefreshToken(refreshTokenRepository.save(refreshToken), plaintext); } /** - * Find refresh token by token string + * Find a refresh token by the plaintext value the client presented. */ - public Optional findByToken(String token) { - return refreshTokenRepository.findByTokenHash(token); + public Optional findByToken(String plaintextToken) { + if (plaintextToken == null || plaintextToken.isBlank()) { + return Optional.empty(); + } + return refreshTokenRepository.findByTokenHash(resolveHash(plaintextToken)); } /** @@ -104,14 +139,11 @@ public RefreshToken verifyExpiration(RefreshToken token) { } /** - * Revoke a refresh token + * Revoke a refresh token, given the plaintext value the client holds. */ @Transactional - public void revokeRefreshToken(String token) { - refreshTokenRepository.findByTokenHash(token).ifPresent(refreshToken -> { - refreshToken.setRevoked(true); - refreshTokenRepository.save(refreshToken); - }); + public void revokeRefreshToken(String plaintextToken) { + findByToken(plaintextToken).ifPresent(this::revokeToken); } /** @@ -133,7 +165,7 @@ public void deleteExpiredTokensByUser(UUID userId) { List expiredTokens = userTokens.stream() .filter(token -> token.isExpired() || token.isRevoked()) .toList(); - + if (!expiredTokens.isEmpty()) { refreshTokenRepository.deleteAll(expiredTokens); } @@ -146,15 +178,36 @@ public List getActiveTokensForUser(UUID userId) { return refreshTokenRepository.findByUser_IdAndRevokedFalse(userId); } + private void revokeToken(RefreshToken token) { + token.setRevoked(true); + refreshTokenRepository.save(token); + } + /** * Generate a secure random refresh token */ private String generateRefreshToken() { - byte[] randomBytes = new byte[64]; + byte[] randomBytes = new byte[TOKEN_RANDOM_BYTES]; secureRandom.nextBytes(randomBytes); return Base64.getUrlEncoder().withoutPadding().encodeToString(randomBytes); } + /** + * SHA-256 of the plaintext, hex-encoded, optionally peppered. 64 chars, so it fits the + * existing {@code varchar(255)} column. + */ + String resolveHash(String plaintext) { + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + if (pepper != null && !pepper.isEmpty()) { + digest.update(pepper.getBytes(StandardCharsets.UTF_8)); + } + return HexFormat.of().formatHex(digest.digest(plaintext.getBytes(StandardCharsets.UTF_8))); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("SHA-256 not available", e); + } + } + /** * Scheduled task to clean up expired tokens (runs daily at 2 AM) */ @@ -164,16 +217,16 @@ public void cleanupExpiredTokens() { try { Instant now = Instant.now(); refreshTokenRepository.deleteByExpiryDateBefore(now); - + // Clean up revoked tokens (older than 7 days) List oldRevokedTokens = refreshTokenRepository .findByRevokedTrueAndCreatedDateBefore(now.minus(Duration.ofDays(7))); - + if (!oldRevokedTokens.isEmpty()) { refreshTokenRepository.deleteAll(oldRevokedTokens); log.info("Cleaned up {} old revoked refresh tokens", oldRevokedTokens.size()); } - + log.info("Cleaned up expired refresh tokens"); } catch (Exception e) { log.error("Failed to cleanup expired refresh tokens", e); diff --git a/src/main/resources/application.properties.example b/src/main/resources/application.properties.example index 165d227..54de79a 100644 --- a/src/main/resources/application.properties.example +++ b/src/main/resources/application.properties.example @@ -105,6 +105,9 @@ lensbridge.app.jwtSecret=replace-with-base64-32-bytes lensbridge.app.jwtExpirationMs=900000 lensbridge.app.refreshTokenExpirationMs=604800000 lensbridge.app.maxRefreshTokensPerUser=5 +# Optional extra input to the refresh-token hash. Leave empty to disable. +# Changing or setting this invalidates every existing refresh token (everyone signs in again). +lensbridge.app.refreshTokenPepper= # Auth token TTLs auth.token.verification-expiration-ms=86400000 diff --git a/src/main/resources/db/migration/postgresql/V8__hash_refresh_tokens.sql b/src/main/resources/db/migration/postgresql/V8__hash_refresh_tokens.sql new file mode 100644 index 0000000..8f6f062 --- /dev/null +++ b/src/main/resources/db/migration/postgresql/V8__hash_refresh_tokens.sql @@ -0,0 +1,26 @@ +-- +-- V8: invalidate every pre-existing refresh token, because refresh_tokens.token_hash +-- used to hold the raw token rather than a hash of it. +-- +-- See the SQLite twin for the full reasoning. In short: the column named token_hash stored +-- the base64 SecureRandom value verbatim and redemption was an equality lookup against it, +-- so any read of this table yielded directly replayable 7-day sessions. The service now +-- stores sha256(pepper || plaintext) hex-encoded and looks up by that digest, matching +-- VerificationTokenService and EnrollmentTokenService. +-- +-- Deleting rather than rehashing in place is deliberate. Postgres could do the rehash with +-- pgcrypto's digest(), but that would require the extension on every deployment target and, +-- more to the point, those values have sat at rest in plaintext for the life of the table: +-- rehashing keeps any already-leaked copy valid for another 7 days. Deleting is the clean +-- cut. Nothing here can be salvaged anyway -- an old token cannot match a hash column. +-- +-- Consequence: everyone with a live session is logged out once, at deploy. Access tokens +-- (15 min) keep working until they expire; the next refresh attempt returns 401 and the +-- client re-authenticates. No user data is touched. +-- +-- No schema change is needed: token_hash is already varchar(255) not null unique, and a +-- SHA-256 hex digest is 64 chars. +-- +-- The SQLite twin of this file must stay at the same version. + +delete from refresh_tokens; diff --git a/src/main/resources/db/migration/sqlite/V8__hash_refresh_tokens.sql b/src/main/resources/db/migration/sqlite/V8__hash_refresh_tokens.sql new file mode 100644 index 0000000..5a0c848 --- /dev/null +++ b/src/main/resources/db/migration/sqlite/V8__hash_refresh_tokens.sql @@ -0,0 +1,33 @@ +-- +-- V8: invalidate every pre-existing refresh token, because refresh_tokens.token_hash +-- used to hold the raw token rather than a hash of it. +-- +-- RefreshTokenService generated 64 bytes of SecureRandom, base64'd it, handed it to the +-- client, and stored that exact string in a column named token_hash. Redemption was a +-- direct equality lookup against the value the client holds, so anything that could read +-- the table -- a backup, a replica, SQL injection elsewhere, ORM debug logging -- came away +-- with usable 7-day sessions needing no further work. The service now stores +-- sha256(pepper || plaintext) hex-encoded, matching VerificationTokenService and +-- EnrollmentTokenService, and looks up by that digest. +-- +-- The rows already in the table are plaintext and cannot be distinguished from hashes by +-- the lookup path -- a client presenting its old token would hash it, miss, and be rejected +-- anyway. Rather than leave dead plaintext secrets sitting in the table, they are deleted. +-- +-- We could instead have rehashed in place (update ... set token_hash = sha256(token_hash)), +-- which would have kept every session alive. We deliberately did not: neither SQLite nor a +-- stock Postgres install has SHA-256 available in plain SQL without an extension, and more +-- importantly those values have been at rest in plaintext for the life of the table. Any +-- copy already taken stays valid for up to 7 days if we preserve them. Deleting is the +-- clean cut. +-- +-- Consequence: everyone with a live session is logged out once, at deploy. Access tokens +-- (15 min) keep working until they expire; the next refresh attempt returns 401 and the +-- client re-authenticates. No user data is touched. +-- +-- No schema change is needed: token_hash is already varchar(255) not null unique, and a +-- SHA-256 hex digest is 64 chars. +-- +-- The postgres twin of this file must stay at the same version. + +delete from refresh_tokens; diff --git a/src/test/java/com/ibrasoft/lensbridge/service/RefreshTokenServiceTest.java b/src/test/java/com/ibrasoft/lensbridge/service/RefreshTokenServiceTest.java index a194ed1..2ff4e6c 100644 --- a/src/test/java/com/ibrasoft/lensbridge/service/RefreshTokenServiceTest.java +++ b/src/test/java/com/ibrasoft/lensbridge/service/RefreshTokenServiceTest.java @@ -14,7 +14,10 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.test.util.ReflectionTestUtils; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; import java.time.Instant; +import java.util.HexFormat; import java.util.List; import java.util.Optional; import java.util.UUID; @@ -47,6 +50,7 @@ class RefreshTokenServiceTest { void setUp() { ReflectionTestUtils.setField(service, "refreshTokenDurationMs", 604_800_000L); ReflectionTestUtils.setField(service, "maxRefreshTokensPerUser", 5); + ReflectionTestUtils.setField(service, "pepper", ""); userId = UUID.randomUUID(); user = new User("A", "B", "a@b.ca", "p"); user.setId(userId); @@ -54,6 +58,16 @@ void setUp() { .thenAnswer(inv -> inv.getArgument(0)); } + /** Independent SHA-256 hex, so the test does not just re-run the implementation. */ + private static String sha256Hex(String plaintext) { + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + return HexFormat.of().formatHex(digest.digest(plaintext.getBytes(StandardCharsets.UTF_8))); + } catch (Exception e) { + throw new IllegalStateException(e); + } + } + private RefreshToken token(Instant created, boolean revoked) { return RefreshToken.builder() .tokenHash(UUID.randomUUID().toString()) @@ -72,7 +86,7 @@ void createRefreshTokenSetsExpiryFromDurationProperty() { when(userRepository.findById(userId)).thenReturn(Optional.of(user)); Instant before = Instant.now(); - RefreshToken created = service.createRefreshToken(userId, "device", "1.2.3.4"); + RefreshToken created = service.createRefreshToken(userId, "device", "1.2.3.4").token(); long ttlSeconds = created.getExpiryDate().getEpochSecond() - before.getEpochSecond(); assertThat(ttlSeconds).isCloseTo(604_800L, org.assertj.core.data.Offset.offset(5L)); @@ -80,6 +94,50 @@ void createRefreshTokenSetsExpiryFromDurationProperty() { assertThat(created.getTokenHash()).isNotBlank(); } + @Test + void createRefreshTokenStoresHashNeverThePlaintext() { + when(refreshTokenRepository.findByUser_Id(userId)).thenReturn(List.of()); + when(refreshTokenRepository.countByUser_IdAndRevokedFalse(userId)).thenReturn(0L); + when(userRepository.findById(userId)).thenReturn(Optional.of(user)); + + RefreshTokenService.IssuedRefreshToken issued = service.createRefreshToken(userId, "device", "1.2.3.4"); + + ArgumentCaptor saved = ArgumentCaptor.forClass(RefreshToken.class); + verify(refreshTokenRepository).save(saved.capture()); + + assertThat(issued.plaintext()).isNotBlank(); + assertThat(saved.getValue().getTokenHash()) + .isNotEqualTo(issued.plaintext()) + .isEqualTo(sha256Hex(issued.plaintext())) + .hasSize(64); + } + + @Test + void createRefreshTokenGeneratesADistinctPlaintextEachTime() { + when(refreshTokenRepository.findByUser_Id(userId)).thenReturn(List.of()); + when(refreshTokenRepository.countByUser_IdAndRevokedFalse(userId)).thenReturn(0L); + when(userRepository.findById(userId)).thenReturn(Optional.of(user)); + + String first = service.createRefreshToken(userId, "device", "1.2.3.4").plaintext(); + String second = service.createRefreshToken(userId, "device", "1.2.3.4").plaintext(); + + assertThat(first).isNotEqualTo(second); + } + + @Test + void pepperIsMixedIntoTheStoredHash() { + ReflectionTestUtils.setField(service, "pepper", "a-secret-pepper"); + when(refreshTokenRepository.findByUser_Id(userId)).thenReturn(List.of()); + when(refreshTokenRepository.countByUser_IdAndRevokedFalse(userId)).thenReturn(0L); + when(userRepository.findById(userId)).thenReturn(Optional.of(user)); + + RefreshTokenService.IssuedRefreshToken issued = service.createRefreshToken(userId, "device", "1.2.3.4"); + + assertThat(issued.token().getTokenHash()) + .isNotEqualTo(sha256Hex(issued.plaintext())) + .isEqualTo(sha256Hex("a-secret-pepper" + issued.plaintext())); + } + @Test void createRefreshTokenRevokesOldestWhenAtMaxLimit() { RefreshToken oldest = token(Instant.now().minusSeconds(1000), false); @@ -87,7 +145,6 @@ void createRefreshTokenRevokesOldestWhenAtMaxLimit() { when(refreshTokenRepository.findByUser_Id(userId)).thenReturn(List.of()); when(refreshTokenRepository.countByUser_IdAndRevokedFalse(userId)).thenReturn(5L); when(refreshTokenRepository.findByUser_IdAndRevokedFalse(userId)).thenReturn(List.of(newer, oldest)); - when(refreshTokenRepository.findByTokenHash(oldest.getTokenHash())).thenReturn(Optional.of(oldest)); when(userRepository.findById(userId)).thenReturn(Optional.of(user)); service.createRefreshToken(userId, "device", "1.2.3.4"); @@ -152,25 +209,52 @@ void verifyExpirationUpdatesLastUsedDateForValidToken() { } @Test - void revokeRefreshTokenMarksTokenRevoked() { + void revokeRefreshTokenHashesThePlaintextBeforeLookup() { RefreshToken t = token(Instant.now(), false); - when(refreshTokenRepository.findByTokenHash(t.getTokenHash())).thenReturn(Optional.of(t)); + when(refreshTokenRepository.findByTokenHash(sha256Hex("the-plaintext"))).thenReturn(Optional.of(t)); - service.revokeRefreshToken(t.getTokenHash()); + service.revokeRefreshToken("the-plaintext"); assertThat(t.isRevoked()).isTrue(); verify(refreshTokenRepository).save(t); + verify(refreshTokenRepository, never()).findByTokenHash("the-plaintext"); } @Test void revokeRefreshTokenIsNoOpForUnknownToken() { - when(refreshTokenRepository.findByTokenHash("nope")).thenReturn(Optional.empty()); + when(refreshTokenRepository.findByTokenHash(sha256Hex("nope"))).thenReturn(Optional.empty()); service.revokeRefreshToken("nope"); verify(refreshTokenRepository, never()).save(any()); } + @Test + void revokeRefreshTokenIsNoOpForNullOrBlank() { + service.revokeRefreshToken(null); + service.revokeRefreshToken(" "); + + verify(refreshTokenRepository, never()).findByTokenHash(any()); + verify(refreshTokenRepository, never()).save(any()); + } + + @Test + void findByTokenLooksUpTheHashNotThePlaintext() { + RefreshToken t = token(Instant.now(), false); + when(refreshTokenRepository.findByTokenHash(sha256Hex("client-token"))).thenReturn(Optional.of(t)); + + assertThat(service.findByToken("client-token")).contains(t); + verify(refreshTokenRepository, never()).findByTokenHash("client-token"); + } + + @Test + void findByTokenReturnsEmptyForNullOrBlankWithoutTouchingTheRepository() { + assertThat(service.findByToken(null)).isEmpty(); + assertThat(service.findByToken(" ")).isEmpty(); + + verify(refreshTokenRepository, never()).findByTokenHash(any()); + } + @Test void revokeAllUserTokensRevokesEveryActiveToken() { RefreshToken a = token(Instant.now(), false);