-
Notifications
You must be signed in to change notification settings - Fork 0
[refactor] 소셜 로그인 동적 리다이렉트되도록 변경 #307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
80bea58
2e3b64e
9d428ff
fe121b3
c5b31fc
3f96981
7accbaf
ef217a4
d300c60
4c263aa
a780168
2052fcf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,8 @@ | |
| import jakarta.servlet.ServletException; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import konkuk.thip.common.exception.AuthException; | ||
| import konkuk.thip.common.exception.code.ErrorCode; | ||
| import org.springframework.beans.factory.annotation.Qualifier; | ||
| import org.springframework.security.core.AuthenticationException; | ||
| import org.springframework.security.web.AuthenticationEntryPoint; | ||
|
|
@@ -15,16 +17,32 @@ | |
| public class JwtAuthenticationEntryPoint implements AuthenticationEntryPoint { | ||
| private final HandlerExceptionResolver resolver; | ||
|
|
||
| public JwtAuthenticationEntryPoint(@Qualifier("handlerExceptionResolver") HandlerExceptionResolver resolver){ | ||
| public JwtAuthenticationEntryPoint(@Qualifier("handlerExceptionResolver") | ||
| HandlerExceptionResolver resolver) { | ||
| this.resolver = resolver; | ||
| } | ||
|
|
||
| @Override | ||
| public void commence(HttpServletRequest request, HttpServletResponse response, AuthenticationException authException) throws IOException, ServletException { | ||
| Exception e = (Exception) request.getAttribute("exception"); | ||
| if(e == null){ | ||
| e = authException; | ||
| public void commence(HttpServletRequest request, | ||
| HttpServletResponse response, | ||
| AuthenticationException authException) throws IOException, ServletException { | ||
|
|
||
| // 필터에서 set한 예외 우선 | ||
| Exception original = (Exception) request.getAttribute("exception"); | ||
| if (original == null) { | ||
| original = authException; | ||
| } | ||
|
|
||
| Exception mapped = wrapAsAuthException(original); | ||
|
|
||
| resolver.resolveException(request, response, null, mapped); | ||
| } | ||
|
|
||
| // 모든 예외를 AuthException(401)으로 감싸는 메서드 | ||
| private Exception wrapAsAuthException(Exception e) { | ||
| if (e instanceof AuthException) { | ||
| return e; | ||
| } | ||
| resolver.resolveException(request, response, null, e); | ||
| return new AuthException(ErrorCode.AUTH_INTERNAL_SERVER_ERROR, e); | ||
| } | ||
|
Comment on lines
+41
to
47
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 확인했습니다 |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,31 +3,33 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import jakarta.servlet.ServletException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import jakarta.servlet.http.HttpServletRequest; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import jakarta.servlet.http.HttpServletResponse; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.common.exception.AuthException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.common.exception.code.ErrorCode; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.common.security.oauth2.tokenstorage.LoginTokenStorage; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.common.security.util.JwtUtil; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.config.properties.WebDomainProperties; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import lombok.extern.slf4j.Slf4j; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.beans.factory.annotation.Value; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.security.core.Authentication; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.security.web.authentication.SimpleUrlAuthenticationSuccessHandler; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.stereotype.Component; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.io.IOException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.time.Duration; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Objects; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.UUID; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import static konkuk.thip.common.security.constant.AuthParameters.REDIRECT_HOME_URL; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import static konkuk.thip.common.security.constant.AuthParameters.REDIRECT_SIGNUP_URL; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import static konkuk.thip.common.security.constant.AuthParameters.*; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Slf4j | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Component | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public class CustomSuccessHandler extends SimpleUrlAuthenticationSuccessHandler { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static final int COOKIE_MAX_AGE = 60 * 60 * 24; // 1일 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final LoginTokenStorage loginTokenStorage; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Value("${server.web-redirect-url}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private String webRedirectUrl; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final WebDomainProperties webDomainProperties; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
-29
to
+32
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 굳굳 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final JwtUtil jwtUtil; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -38,6 +40,22 @@ public void onAuthenticationSuccess( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Authentication authentication | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) throws IOException, ServletException { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Resolver에서 세션에 저장한 origin을 복원 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String webRedirectDomain = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (request.getSession(false) != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| webRedirectDomain = (String) request.getSession(false).getAttribute(REDIRECT_SESSION_KEY.getValue()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request.getSession(false).removeAttribute(REDIRECT_SESSION_KEY.getValue()); // 사용했으면 제거(일회성) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 굿굿 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 허용 오리진 검증 및 폴백 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!webDomainProperties.isAllowed(Objects.toString(webRedirectDomain, ""))) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<String> origins = webDomainProperties.getWebDomainUrls(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (origins == null || origins.isEmpty()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new AuthException(ErrorCode.WEB_DOMAIN_ORIGIN_EMPTY); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| webRedirectDomain = origins.get(0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+43
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 리다이렉트 도메인 검증 강화 및 Origin 정규화 현재 문자열 기반 검증/결합은 오리진이 아닌 전체 URL이 들어오거나 경계 케이스(중복 슬래시, 포트/IPv6, 국제화 도메인)에서 취약할 수 있습니다. URI 파싱으로 오리진을 정규화한 뒤 허용 목록과 비교하도록 개선하세요. +import org.springframework.web.util.UriComponentsBuilder;
+import java.net.URI;
...
- // 허용 오리진 검증 및 폴백
- if (!webDomainProperties.isAllowed(Objects.toString(webRedirectDomain, ""))) {
+ // 허용 오리진 검증 및 폴백
+ String candidate = Objects.toString(webRedirectDomain, "");
+ if (!candidate.isEmpty()) {
+ try {
+ URI parsed = URI.create(candidate).normalize();
+ candidate = parsed.getScheme() + "://" + parsed.getAuthority();
+ } catch (IllegalArgumentException e) {
+ candidate = "";
+ }
+ }
+ if (!webDomainProperties.isAllowed(candidate)) {
List<String> origins = webDomainProperties.getWebDomainUrls();
if (origins == null || origins.isEmpty()) {
throw new AuthException(ErrorCode.WEB_DOMAIN_ORIGIN_EMPTY);
}
- webRedirectDomain = origins.get(0);
+ candidate = origins.get(0);
}
+ // 최종 오리진
+ webRedirectDomain = candidate;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CustomOAuth2User oAuth2User = (CustomOAuth2User) authentication.getPrincipal(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LoginUser loginUser = oAuth2User.getLoginUser(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -48,15 +66,15 @@ public void onAuthenticationSuccess( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String loginTokenKey = UUID.randomUUID().toString(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| loginTokenStorage.put(loginTokenKey, TokenType.TEMP, tempToken, Duration.ofMinutes(5)); // ttl 5분 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getRedirectStrategy().sendRedirect(request, response, webRedirectUrl + REDIRECT_SIGNUP_URL.getValue() + "?loginTokenKey=" + loginTokenKey); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getRedirectStrategy().sendRedirect(request, response, webRedirectDomain + REDIRECT_SIGNUP_URL.getValue() + "?loginTokenKey=" + loginTokenKey); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 기존 유저 - 로그인용 액세스 토큰 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String accessToken = jwtUtil.createAccessToken(loginUser.userId()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String loginTokenKey = UUID.randomUUID().toString(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| loginTokenStorage.put(loginTokenKey, TokenType.ACCESS, accessToken, Duration.ofMinutes(5)); // ttl 5분 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getRedirectStrategy().sendRedirect(request, response, webRedirectUrl + REDIRECT_HOME_URL.getValue() + "?loginTokenKey=" + loginTokenKey); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getRedirectStrategy().sendRedirect(request, response, webRedirectDomain + REDIRECT_HOME_URL.getValue() + "?loginTokenKey=" + loginTokenKey); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,19 @@ | ||||||||||||||||||||||||||||||||||||||||||
| package konkuk.thip.common.security.oauth2.tokenstorage; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.common.security.oauth2.TokenType; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import java.time.Duration; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| public interface LoginTokenStorage { | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| void put(String key, TokenType type, String token, Duration ttl); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||
| * 저장된 토큰을 1회용으로 소비 후 삭제한다. | ||||||||||||||||||||||||||||||||||||||||||
| * 존재하지 않으면 null 반환. | ||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||
| Entry consume(String key); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| record Entry(TokenType type, String token) { | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+11
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 소비(consume) 원자성 명시 필요 보안상 “1회용”은 get+delete가 원자적이어야 합니다. 인터페이스 주석에 원자성/스레드 안전성 계약을 명문화해 구현체 일관성을 보장하세요. - /**
- * 저장된 토큰을 1회용으로 소비 후 삭제한다.
- * 존재하지 않으면 null 반환.
- */
+ /**
+ * 저장된 토큰을 1회용으로 원자적으로(atomic) 소비 후 삭제한다.
+ * - 구현체는 getAndDelete(예: Redis GETDEL) 등으로 동시성 환경에서도 중복 소비가 불가능해야 한다.
+ * - key가 존재하지 않으면 null을 반환한다.
+ * - put의 ttl은 0보다 커야 한다.
+ */
Entry consume(String key);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| package konkuk.thip.common.security.oauth2.tokenstorage; | ||
|
|
||
| import konkuk.thip.common.security.oauth2.TokenType; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.context.annotation.Profile; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| import java.time.Duration; | ||
| import java.time.Instant; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
|
|
||
| @Profile("test") | ||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class MemoryLoginTokenStorage implements LoginTokenStorage{ | ||
|
Comment on lines
+12
to
+15
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 확인했습니다 |
||
| private record InternalEntry(TokenType type, String token, long expireAtEpochMillis) { } | ||
|
|
||
| private final ConcurrentHashMap<String, InternalEntry> store = new ConcurrentHashMap<>(); | ||
|
|
||
| /** | ||
| * 토큰을 메모리에 저장 (TTL 적용) | ||
| */ | ||
| @Override | ||
| public void put(String key, TokenType type, String token, Duration ttl) { | ||
| long expiredAt = Instant.now().plus(ttl).toEpochMilli(); | ||
| store.put(key, new InternalEntry(type, token, expiredAt)); | ||
| } | ||
|
|
||
| /** | ||
| * 토큰을 일회성으로 조회 후 제거 (만료 시 null 반환) | ||
| */ | ||
| @Override | ||
| public Entry consume(String key) { | ||
| InternalEntry entry = store.remove(key); | ||
| if (entry == null) return null; | ||
|
|
||
| if (entry.expireAtEpochMillis() < Instant.now().toEpochMilli()) { | ||
| return null; // 만료 | ||
| } | ||
|
|
||
| // 외부에는 최소 DTO만 반환 (내부 정보 캡슐화) | ||
| return new Entry(entry.type(), entry.token()); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,48 @@ | ||||||||||||||||||||||||||||||||||||||||||||
| package konkuk.thip.common.security.oauth2.tokenstorage; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.common.exception.AuthException; | ||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.common.exception.code.ErrorCode; | ||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.common.security.oauth2.TokenType; | ||||||||||||||||||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.context.annotation.Profile; | ||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.data.redis.core.RedisTemplate; | ||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.stereotype.Component; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| import java.time.Duration; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| @Profile({"!test"}) | ||||||||||||||||||||||||||||||||||||||||||||
| @Component | ||||||||||||||||||||||||||||||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||||||||||||||||||||||||||||||
| public class RedisLoginTokenStorage implements LoginTokenStorage { | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+13
to
+16
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 확인했습니다 |
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| private final RedisTemplate<String, Object> redisTemplate; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| private static final String PREFIX = "auth:login-token:"; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||
| public void put(String key, TokenType type, String token, Duration ttl) { | ||||||||||||||||||||||||||||||||||||||||||||
| String redisKey = toRedisKey(key); | ||||||||||||||||||||||||||||||||||||||||||||
| Entry entry = new Entry(type, token); | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| redisTemplate.opsForValue().set(redisKey, entry, ttl); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+18
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 직렬화 호환성 이슈: Entry 객체를 값으로 저장 시 역직렬화 실패 가능 RedisTemplate<String, Object>의 기본 시리얼라이저 설정에 따라 Entry 레코드가 Map/바이트로 역직렬화되어 instanceof Entry가 실패할 수 있습니다. 또한 JDK 직렬화를 쓰면 Entry가 Serializable이어야 합니다. 구현 간 일관성을 위해 문자열 값으로 저장하는 방식을 권장합니다. - public void put(String key, TokenType type, String token, Duration ttl) {
- String redisKey = toRedisKey(key);
- Entry entry = new Entry(type, token);
-
- redisTemplate.opsForValue().set(redisKey, entry, ttl);
- }
+ public void put(String key, TokenType type, String token, Duration ttl) {
+ String redisKey = toRedisKey(key);
+ String value = type.name() + ":" + token; // 직렬화 독립
+ redisTemplate.opsForValue().set(redisKey, value, ttl);
+ }📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||
| public Entry consume(String key) { | ||||||||||||||||||||||||||||||||||||||||||||
| String redisKey = toRedisKey(key); | ||||||||||||||||||||||||||||||||||||||||||||
| Object value = redisTemplate.opsForValue().getAndDelete(redisKey); | ||||||||||||||||||||||||||||||||||||||||||||
| if (value == null) { | ||||||||||||||||||||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| if (value instanceof Entry entry) { | ||||||||||||||||||||||||||||||||||||||||||||
| return entry; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| throw new AuthException(ErrorCode.JSON_PROCESSING_ERROR); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| private String toRedisKey(String key) { | ||||||||||||||||||||||||||||||||||||||||||||
| return PREFIX + key; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| package konkuk.thip.common.security.argument_resolver; | ||
| package konkuk.thip.common.security.resolver; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오 패키지정리 굿임다 |
||
|
|
||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import konkuk.thip.common.exception.AuthException; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Verification inconclusive
파라미터 키 불일치 가능성(redirect_url vs redirect_uri)
PR 요약/가이드에선
redirect_uri를 예시로 들었습니다. 현재 상수는redirect_url입니다. 클라이언트와 불일치 시 로그인 플로우가 깨집니다. 호환성 위해 둘 다 지원하는 방안을 추천합니다.COOKIE_TEMP_TOKEN("temp_token"), - REDIRECT_URL_KEY("redirect_url"), + REDIRECT_URL_KEY("redirect_url"), + REDIRECT_URI_KEY("redirect_uri"), REDIRECT_SESSION_KEY("oauth2_return_to"),추가로 Resolver에서 두 키 중 하나를 우선순위로 읽는 로직을 반영해 주세요.
필수: redirect_uri 파라미터 호환성 추가
PR 예시가 redirect_uri인 반면 코드엔 redirect_url만 있어 클라이언트 불일치 시 OAuth 로그인 흐름이 깨질 수 있습니다. 다음 변경 적용 필요 (파일: src/main/java/konkuk/thip/common/security/constant/AuthParameters.java):
COOKIE_TEMP_TOKEN("temp_token"), - REDIRECT_URL_KEY("redirect_url"), + REDIRECT_URL_KEY("redirect_url"), + REDIRECT_URI_KEY("redirect_uri"), REDIRECT_SESSION_KEY("oauth2_return_to"),Resolver에서 REDIRECT_URI_KEY를 우선 읽고 없으면 REDIRECT_URL_KEY를 사용하도록 우선순위 로직 반영.
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pr 의 예시가 오타인 거야. FE와 'redirect_url' 을 key로 설정하는 것으로 결정했어.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seongjunnoh 감사합니다! PR 설명의 예시가 오타였다는 점을 명확히 해주셔서 이해했습니다.
FE와
redirect_url을 키로 사용하기로 합의하셨다면 현재 구현이 올바르네요. 제가 제안한REDIRECT_URI_KEY추가와 우선순위 로직은 불필요합니다.현재
REDIRECT_URL_KEY("redirect_url")구현이 FE와의 합의된 사양에 맞게 잘 되어 있습니다.🐰✨
╰( ◕ ᗜ ◕ )╯
✏️ Learnings added
🧠 Learnings used