Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.Timo.Timo.global.auth.filter;

import com.Timo.Timo.global.auth.utils.CookieUtil;
import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.util.List;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.HttpHeaders;
import org.springframework.stereotype.Component;
import org.springframework.web.filter.OncePerRequestFilter;

@Component
public class LegacyCookieCleanupFilter extends OncePerRequestFilter {

private static final List<String> TARGET_PATHS = List.of(
"/api/v1/auth/reissue",
"/api/v1/auth/logout",
"/api/v1/auth/withdraw"
);

@Value("${app.auth.cookie-secure}")
private boolean cookieSecure;

@Override
protected void doFilterInternal(
HttpServletRequest request, HttpServletResponse response, FilterChain filterChain
) throws ServletException, IOException {
String path = request.getRequestURI().substring(request.getContextPath().length());

if (cookieSecure && TARGET_PATHS.contains(path)) {
response.addHeader(HttpHeaders.SET_COOKIE, CookieUtil.expireLegacyCookie("refreshToken").toString());
response.addHeader(HttpHeaders.SET_COOKIE, CookieUtil.expireLegacyCookie("sessionId").toString());
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] legacy 쿠키 정리는 다음 요청부터 적용되므로, 현재 reissue 요청은 여전히 실패할 수 있을 것 같습니다.

필터에서 Set-Cookie: Max Rhythm-Age=0을 추가하더라도 브라우저가 legacy 쿠키를 실제로 삭제하는 시점은 응답을 받은 이후입니다. 따라서 이번 요청에서 @CookieValue가 legacy 쿠키나 서로 맞지 않는 refreshToken/sessionId 조합을 선택하면 기존과 동일하게 AUTH_401 또는 USER_404가 발생합니다.

현재 구현은 동일 오류가 반복되는 것은 막아주지만, 프론트가 reissue 실패 즉시 로그아웃 처리한다면 사용자는 최초 한 번의 오류로 세션을 잃게 됩니다. 이 동작이 의도된 것인지 확인이 필요해 보입니다.

완전한 마이그레이션이 필요하다면 신규 쿠키 이름을 분리하거나, 중복 쿠키 후보 중 Redis에서 유효한 token/session 조합을 찾거나, 쿠키 정리 후 한 번만 재시도하는 방식도 검토할 수 있을 것 같습니다.

}
filterChain.doFilter(request, response);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class OriginValidationFilter extends OncePerRequestFilter {
protected void doFilterInternal(
HttpServletRequest request, HttpServletResponse response, FilterChain filterChain
) throws ServletException, IOException {
String path = request.getRequestURI();
String path = request.getRequestURI().substring(request.getContextPath().length());

if (PROTECTED_PATHS.stream().anyMatch(path::equals)) {
String origin = request.getHeader("Origin");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ public void onAuthenticationSuccess(
CookieUtil.createCookie("sessionId", sessionId,
jwtTokenProvider.getRefreshTokenExpiry(), cookieSecure).toString());

if (cookieSecure) {
response.addHeader(HttpHeaders.SET_COOKIE, CookieUtil.expireLegacyCookie("refreshToken").toString());
response.addHeader(HttpHeaders.SET_COOKIE, CookieUtil.expireLegacyCookie("sessionId").toString());
}

String code = authCodeService.generateAndSave(
String.valueOf(userId),
onboardingCompleted
Expand Down
29 changes: 21 additions & 8 deletions src/main/java/com/Timo/Timo/global/auth/service/AuthService.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.Timo.Timo.global.auth.service;

import com.Timo.Timo.domain.calendar.client.GoogleOAuthClient;
import com.Timo.Timo.domain.calendar.entity.CalendarRevocationOutbox;
import com.Timo.Timo.domain.calendar.repository.CalendarConnectionRepository;
import com.Timo.Timo.domain.calendar.repository.CalendarRevocationOutboxRepository;
Expand All @@ -13,6 +12,7 @@
import com.Timo.Timo.global.exception.CustomException;
import com.Timo.Timo.global.exception.code.ErrorCode;
import com.Timo.Timo.global.jwt.provider.JwtTokenProvider;
import java.util.Optional;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;
Expand Down Expand Up @@ -76,22 +76,35 @@ public ReissueResult reissue(String refreshToken, String sessionId) {
}

Long userId = jwtTokenProvider.getUserId(refreshToken);
String userIdKey = String.valueOf(userId);

if (!userRepository.existsById(userId)) {
throw new CustomException(UserErrorCode.USER_NOT_FOUND);
}

if (!refreshTokenService.isRefreshTokenValid(String.valueOf(userId), sessionId, refreshToken)){
throw new CustomException(AuthErrorCode.INVALID_REFRESH_TOKEN);
String newAccessToken = jwtTokenProvider.generateAccessToken(userId);
String newRefreshToken = jwtTokenProvider.generateRefreshToken(userId);

Optional<String> rotatedSessionId =
refreshTokenService.rotateIfValid(userIdKey, sessionId, refreshToken, newRefreshToken);

if (rotatedSessionId.isPresent()) {
return new ReissueResult(newAccessToken, newRefreshToken, rotatedSessionId.get());
}

refreshTokenService.deleteRefreshToken(String.valueOf(userId), sessionId);
return refreshTokenService.findRotatedSessionId(userIdKey, sessionId, refreshToken)
.map(newSessionId -> reissueFromAlreadyRotatedSession(userId, userIdKey, newSessionId))
.orElseThrow(() -> new CustomException(AuthErrorCode.INVALID_REFRESH_TOKEN));
}

String newAccessToken = jwtTokenProvider.generateAccessToken(userId);
String newRefreshToken = jwtTokenProvider.generateRefreshToken(userId);
String newSessionId = refreshTokenService.saveRefreshToken(String.valueOf(userId), newRefreshToken);
private ReissueResult reissueFromAlreadyRotatedSession(Long userId, String userIdKey, String newSessionId) {
String currentRefreshToken = refreshTokenService.getRefreshToken(userIdKey, newSessionId);
if (currentRefreshToken == null) {
throw new CustomException(AuthErrorCode.INVALID_REFRESH_TOKEN);
}

return new ReissueResult(newAccessToken, newRefreshToken, newSessionId);
String newAccessToken = jwtTokenProvider.generateAccessToken(userId);
return new ReissueResult(newAccessToken, currentRefreshToken, newSessionId);
}

public void logout(String accessToken, Long userId, String sessionId) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
package com.Timo.Timo.global.auth.service;

import com.Timo.Timo.global.jwt.provider.JwtTokenProvider;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.HexFormat;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import lombok.RequiredArgsConstructor;
import org.springframework.data.redis.core.Cursor;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.data.redis.core.ScanOptions;
import org.springframework.data.redis.core.script.DefaultRedisScript;
import org.springframework.data.redis.core.script.RedisScript;
import org.springframework.stereotype.Service;

@Service
Expand All @@ -21,6 +27,24 @@ public class RefreshTokenService {
private final JwtTokenProvider jwtTokenProvider;

private static final String KEY_PREFIX = "refresh:";
private static final String ROTATED_PREFIX = "refresh:rotated:";
private static final long ROTATION_GRACE_SECONDS = 5;

private static final String ROTATE_SCRIPT = """
local current = redis.call('GET', KEYS[1])
if current == false then
return 0
end
if current ~= ARGV[1] then
return -1
end
redis.call('SET', KEYS[3], ARGV[2], 'EX', ARGV[4])
redis.call('SET', KEYS[2], ARGV[6] .. ':' .. ARGV[3], 'EX', ARGV[5])
redis.call('DEL', KEYS[1])
return 1
""";

private final RedisScript<Long> rotateScript = new DefaultRedisScript<>(ROTATE_SCRIPT, Long.class);

public String saveRefreshToken(String userId, String refreshToken){
String sessionId = UUID.randomUUID().toString();
Expand All @@ -37,8 +61,30 @@ public String getRefreshToken(String userId, String sessionId) {
return redisTemplate.opsForValue().get(KEY_PREFIX + userId + ":" + sessionId);
}

private static final String DELETE_SCRIPT = """
local deleted = redis.call('DEL', KEYS[1])
if deleted == 0 then
local pointer = redis.call('GET', KEYS[2])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] 연속 rotation 이후에도 최신 세션이 삭제되도록 보완이 필요해 보입니다.

현재 삭제 로직은 기존 세션이 없으면 rotation mapping을 한 번만 따라가는 것 같네요. 따라서 유예시간 안에 S0 → S1 → S2로 연속 rotation된 뒤, 네트워크에서 늦게 도착한 S0 기반 로그아웃 요청이 처리되면 S1 삭제만 시도하고 실제 활성 세션인 S2는 남을 수 있습니다.

이 경우 로그아웃 API는 성공하지만 S2의 refresh token으로 계속 재발급할 수 있습니다. 포인터를 최종 세션까지 추적하거나, rotation 시 이전 세션들의 mapping도 최신 세션을 가리키도록 갱신하는 방식이 필요해 보입니다.

S0 → S1 → S2 이후 S0deleteRefreshToken()을 호출했을 때 S2까지 삭제되는 테스트도 추가하면 좋을 것 같습니다.

실은 저도 rotation 부분은 처음봐서 너무 어렵네요....

if pointer then
local sep = string.find(pointer, ':')
if sep then
local newSessionId = string.sub(pointer, sep + 1)
redis.call('DEL', ARGV[1] .. ARGV[2] .. ':' .. newSessionId)
end
end
end
redis.call('DEL', KEYS[2])
return deleted
""";

private final RedisScript<Long> deleteScript = new DefaultRedisScript<>(DELETE_SCRIPT, Long.class);

public void deleteRefreshToken(String userId, String sessionId) {
redisTemplate.delete(KEY_PREFIX + userId + ":" + sessionId);
List<String> keys = List.of(
KEY_PREFIX + userId + ":" + sessionId,
ROTATED_PREFIX + userId + ":" + sessionId
);
redisTemplate.execute(deleteScript, keys, KEY_PREFIX, userId);
}

public void deleteAllRefreshTokens(String userId) {
Expand All @@ -63,4 +109,68 @@ public void deleteAllRefreshTokens(String userId) {
public boolean isRefreshTokenValid(String userId, String sessionId, String refreshToken) {
return Objects.equals(refreshToken, getRefreshToken(userId, sessionId));
}

public Optional<String> rotateIfValid(
String userId, String oldSessionId, String expectedRefreshToken, String newRefreshToken
) {
String newSessionId = UUID.randomUUID().toString();

List<String> keys = List.of(
KEY_PREFIX + userId + ":" + oldSessionId,
ROTATED_PREFIX + userId + ":" + oldSessionId,
KEY_PREFIX + userId + ":" + newSessionId
);

Long result = redisTemplate.execute(
rotateScript,
keys,
expectedRefreshToken,
newRefreshToken,
newSessionId,
String.valueOf(jwtTokenProvider.getRefreshTokenExpiry()),
String.valueOf(ROTATION_GRACE_SECONDS),
sha256Hex(expectedRefreshToken)
);

if (result != null && result == 1L) {
return Optional.of(newSessionId);
}
return Optional.empty();
}

public Optional<String> findRotatedSessionId(String userId, String oldSessionId, String refreshToken) {
String stored = redisTemplate.opsForValue().get(ROTATED_PREFIX + userId + ":" + oldSessionId);
if (stored == null) {
return Optional.empty();
}

int separatorIndex = stored.indexOf(':');
if (separatorIndex < 0) {
return Optional.empty();
}

String storedDigest = stored.substring(0, separatorIndex);
String newSessionId = stored.substring(separatorIndex + 1);

boolean digestMatches = MessageDigest.isEqual(
storedDigest.getBytes(StandardCharsets.UTF_8),
sha256Hex(refreshToken).getBytes(StandardCharsets.UTF_8)
);

if (!digestMatches) {
return Optional.empty();
}

return Optional.of(newSessionId);
}

private static String sha256Hex(String value) {
try {
MessageDigest digest = MessageDigest.getInstance("SHA-256");
byte[] hash = digest.digest(value.getBytes(StandardCharsets.UTF_8));
return HexFormat.of().formatHex(hash);
} catch (NoSuchAlgorithmException e) {
throw new IllegalStateException("SHA-256 알고리즘을 사용할 수 없습니다.", e);
}
}
}
10 changes: 10 additions & 0 deletions src/main/java/com/Timo/Timo/global/auth/utils/CookieUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,14 @@ public static ResponseCookie expireCookie(String name, boolean secure) {

return builder.build();
}

public static ResponseCookie expireLegacyCookie(String name) {
return ResponseCookie.from(name, "")
.httpOnly(true)
.secure(true)
.path("/api/v1/auth")
.maxAge(0)
.sameSite("None")
.build();
}
}
15 changes: 14 additions & 1 deletion src/main/java/com/Timo/Timo/global/config/SecurityConfig.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.Timo.Timo.global.config;

import com.Timo.Timo.global.auth.filter.LegacyCookieCleanupFilter;
import com.Timo.Timo.global.auth.filter.OriginValidationFilter;
import com.Timo.Timo.global.auth.handler.JwtAuthenticationEntryPoint;
import com.Timo.Timo.global.auth.handler.OAuthFailureHandler;
Expand Down Expand Up @@ -34,6 +35,7 @@ public class SecurityConfig {
private final CorsConfigurationSource corsConfigurationSource;
private final OAuthOriginCaptureFilter oAuthOriginCaptureFilter;
private final OriginValidationFilter originValidationFilter;
private final LegacyCookieCleanupFilter legacyCookieCleanupFilter;

@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
Expand Down Expand Up @@ -72,7 +74,8 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti
.addFilterBefore(jwtAuthenticationFilter, UsernamePasswordAuthenticationFilter.class)
.addFilterBefore(mdcLoggingFilter(), JwtAuthenticationFilter.class)
.addFilterBefore(oAuthOriginCaptureFilter, OAuth2AuthorizationRequestRedirectFilter.class)
.addFilterBefore(originValidationFilter, JwtAuthenticationFilter.class);
.addFilterBefore(originValidationFilter, JwtAuthenticationFilter.class)
.addFilterBefore(legacyCookieCleanupFilter, JwtAuthenticationFilter.class);

return http.build();
}
Expand Down Expand Up @@ -100,4 +103,14 @@ public FilterRegistrationBean<OriginValidationFilter> originValidationFilterRegi
registrationBean.setEnabled(false);
return registrationBean;
}

@Bean
public FilterRegistrationBean<LegacyCookieCleanupFilter> legacyCookieCleanupFilterRegistration(
LegacyCookieCleanupFilter legacyCookieCleanupFilter
) {
FilterRegistrationBean<LegacyCookieCleanupFilter> registrationBean =
new FilterRegistrationBean<>(legacyCookieCleanupFilter);
registrationBean.setEnabled(false);
return registrationBean;
}
}