Merged
Conversation
- 경우에 맞춰 생성한 token을 메모리 저장소에 저장 - token을 찾을 수 있는 key를 포함해서 redirect
2 tasks
Walkthrough소셜 로그인 이후 토큰 전달 방식을 쿠키 직접 설정에서 서버측 임시 저장소(LoginTokenStorage)와 /api/set-cookie 교환 방식으로 변경했다. JWT 필터는 새 쿠키 이름(access_token, temp_token)을 인식하고 /api/set-cookie 경로는 필터 제외 처리됐다. 관련 상수와 TokenType enum이 추가됐다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자 브라우저
participant OAuth as OAuth2 Provider
participant Server as 서버
participant Store as LoginTokenStorage
User->>OAuth: 소셜 로그인
OAuth-->>Server: OAuth2 콜백
Server->>Server: 사용자 신규/기존 판별, 토큰 생성
Server->>Store: put(loginTokenKey, TokenType, token, 5분)
Server-->>User: 클라이언트로 리다이렉트 (?loginTokenKey=...)
User->>Server: POST /api/set-cookie {loginTokenKey}
Server->>Store: consume(loginTokenKey)
Store-->>Server: {type, token} 또는 null
Server-->>User: Set-Cookie(access_token|temp_token), 응답 본문 {type}
User->>Server: 이후 API 요청(Authorization 헤더 또는 쿠키)
Server->>Server: JwtAuthenticationFilter가 헤더/쿠키에서 토큰 추출
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Test Results383 tests 383 ✅ 31s ⏱️ Results for commit 857429a. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java (1)
52-63: TEMP 토큰에 대해 SecurityContext 설정을 피하세요.임시 토큰까지 인증 컨텍스트에 올리면 권한 검사 방식에 따라 의도치 않은 접근 허용 가능성이 있습니다. userId가 존재(ACCESS 토큰)할 때만 Authentication을 세팅하세요.
- CustomOAuth2User customOAuth2User = new CustomOAuth2User(loginUser); - - Authentication authToken = new UsernamePasswordAuthenticationToken(customOAuth2User, null, customOAuth2User.getAuthorities()); - SecurityContextHolder.getContext().setAuthentication(authToken); + if (loginUser.userId() != null) { + CustomOAuth2User customOAuth2User = new CustomOAuth2User(loginUser); + Authentication authToken = + new UsernamePasswordAuthenticationToken(customOAuth2User, null, customOAuth2User.getAuthorities()); + SecurityContextHolder.getContext().setAuthentication(authToken); + } else { + log.debug("TEMP token detected. Skipping SecurityContext authentication."); + }또한 TEMP 토큰 허용 경로를 최소화하려면 화이트리스트에만 TEMP 사용을 기대하는 엔드포인트를 남기고, 일반 보호 API에서는 access_token만 사용하도록 위 쿠키 우선순위 변경과 함께 운용하는 것을 권장합니다.
🧹 Nitpick comments (9)
src/main/java/konkuk/thip/common/security/oauth2/TokenType.java (1)
6-13: 중복 상태 제거: value 필드 없이 enum 자체 이름 활용 권장.현 구현은 enum 상수 이름과 동일한 문자열을 다시 보관합니다. 불필요한 상태/보일러플레이트를 제거해 단순화할 수 있습니다.
다음과 같이 정리하면 용도 유지하면서 코드/메모리 낭비를 줄일 수 있습니다.
-@Getter -@RequiredArgsConstructor -public enum TokenType { - - TEMP("TEMP"), ACCESS("ACCESS"); - - private final String value; -} +public enum TokenType { + TEMP, ACCESS; + + public String getValue() { + return name(); + } +}src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java (1)
74-76: 미세 최적화: split 대신 substring 사용 권장.split은 배열 생성이 불필요합니다. 접두사 길이를 활용해 토큰을 잘라내면 가비지와 비용을 줄일 수 있습니다.
- return authorization.split(" ")[1]; + return authorization.substring(JWT_PREFIX.getValue().length()).trim();src/main/java/konkuk/thip/common/security/oauth2/AuthCookieController.java (2)
28-35: DTO 사용으로 입력 검증/가독성 향상.Map<String, String> 대신 전용 요청 DTO로 변경하면 필드 검증과 스키마 관리가 수월합니다.
// 신규 DTO public record SetCookieRequest(String loginTokenKey) {}- public ResponseEntity<?> setCookie(@RequestBody Map<String, String> body, HttpServletResponse response) { - String loginTokenKey = body.get("loginTokenKey"); + public ResponseEntity<?> setCookie(@RequestBody SetCookieRequest req, HttpServletResponse response) { + String loginTokenKey = req.loginTokenKey();
67-124: 대규모 주석 블록 제거 또는 이슈로 이관.주석 처리된 교환 엔드포인트는 유지보수성 저하를 유발합니다. 필요 없다면 제거하고, 필요하다면 별 브랜치/이슈로 관리해 주세요.
원하시면 실제 구현 초안을 생성해 드릴 수 있습니다. 이슈를 열어드릴까요?
src/main/java/konkuk/thip/common/security/oauth2/CustomSuccessHandler.java (2)
26-27: 미사용 상수 제거.COOKIE_MAX_AGE 상수는 현재 사용되지 않습니다. 혼란 방지를 위해 제거하세요.
- private static final int COOKIE_MAX_AGE = 60 * 60 * 24; // 1일
65-77: 이전 쿠키 부여 로직 삭제 권장.새 흐름에서는 서버 저장소 + /api/set-cookie를 사용합니다. addTokenCookie는 미사용이므로 제거해 코드를 간결히 유지하세요.
- private void addTokenCookie(HttpServletResponse response, String token) { - Cookie cookie = new Cookie(JWT_HEADER_KEY.getValue(), token); - if(webRedirectUrl.startsWith(HTTPS_PREFIX.getValue())) { - cookie.setDomain(webRedirectUrl.replace(HTTPS_PREFIX.getValue(), "")); - } else { - cookie.setDomain("localhost"); - } - cookie.setSecure(webRedirectUrl.startsWith(HTTPS_PREFIX.getValue())); - cookie.setHttpOnly(false); - cookie.setPath("/"); - cookie.setMaxAge(COOKIE_MAX_AGE); - response.addCookie(cookie); - }이와 함께 상단의 Cookie import도 제거 가능합니다.
src/main/java/konkuk/thip/common/security/oauth2/LoginTokenStorage.java (3)
45-46: 경계값/성능 미세개선: 만료 비교는 <=, Instant.now() 대신 System.currentTimeMillis()동일 ms에 만료된 토큰을 허용하지 않도록 <= 비교를 권장합니다. 또한 로그인 한 건당 한 번 호출되는 코드이긴 하나, 객체 생성 없는 System.currentTimeMillis()가 더 저오버헤드입니다.
- if (entry.expireAtEpochMillis < Instant.now().toEpochMilli()) return null; + long now = System.currentTimeMillis(); + if (entry.expireAtEpochMillis <= now) return null;
15-26: DTO 성격의 Entry는 record로 간결화 가능 + Lombok 제거불변 값 홀더는 record가 적합합니다. Lombok @Getter와 수동 생성자를 제거해 보일러플레이트를 없앨 수 있습니다.
-import lombok.Getter; +// Lombok 불필요- @Getter - public static final class Entry { - private final TokenType type; - private final String token; - private final long expireAtEpochMillis; - - private Entry(TokenType type, String token, long expireAtEpochMillis) { - this.type = type; - this.token = token; - this.expireAtEpochMillis = expireAtEpochMillis; - } - } + public static record Entry(TokenType type, String token, long expireAtEpochMillis) {}참고: record로 전환 시 consume의 접근자도 메서드 형태로 바뀝니다.
예) entry.expireAtEpochMillis() (본 파일의 관련 참조도 함께 치환 필요)Also applies to: 3-3
4-4: @requiredargsconstructor는 불필요합니다주입받는 필드가 없고, final 필드(store)는 필드 초기화로 구성되어 생성자 생성이 무의미합니다. 혼란을 줄이기 위해 제거 권장합니다.
-import lombok.RequiredArgsConstructor; @@ -@RequiredArgsConstructorAlso applies to: 12-12
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/konkuk/thip/common/security/constant/AuthParameters.java(1 hunks)src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java(2 hunks)src/main/java/konkuk/thip/common/security/oauth2/AuthCookieController.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/CustomSuccessHandler.java(3 hunks)src/main/java/konkuk/thip/common/security/oauth2/LoginTokenStorage.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/TokenType.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
src/main/java/konkuk/thip/common/security/constant/AuthParameters.java (1)
20-21: 쿠키 이름 상수 추가는 적절합니다 (LGTM).새 쿠키 네이밍을 상수화해 중복/실수를 줄였습니다. 이후 레이어에서 동일 값 하드코딩 금지 유지 부탁드립니다.
src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java (1)
102-104: 화이트리스트 경로 검토 결과현재
/api/exchange-temp-token엔드포인트는
AuthCookieController에서 주석 처리되어 있어 활성화되지 않은 상태입니다.
따라서JwtAuthenticationFilter의shouldNotFilter목록에 포함되어 있지 않아도 무방합니다.다만, 해당 엔드포인트를 활성화할 경우 반드시 예외 경로로 추가해야 합니다:
- 파일:
src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java- 메서드:
shouldNotFilter(HttpServletRequest request)(약 102행)활성화 시 복원할 코드:
- // || path.equals("/api/exchange-temp-token") + || path.equals("/api/exchange-temp-token")src/main/java/konkuk/thip/common/security/oauth2/AuthCookieController.java (1)
52-64: 쿠키 수명과 JWT 만료의 정합성 확인 요청.access_token 쿠키는 30일, TEMP 쿠키는 10분입니다. JWT 자체 만료와 불일치하면 만료된 토큰이 쿠키에 남아 UX 저하(연속 401) 가능성이 있습니다. JWT 만료 ≤ 쿠키 만료로 정렬하거나, 만료 동기화 전략을 정의해 주세요.
src/main/java/konkuk/thip/common/security/oauth2/LoginTokenStorage.java (1)
41-43: 원타임 소비(atomic remove) 동시성 안전성은 적절합니다ConcurrentHashMap에서 remove로 먼저 제거한 뒤 반환하는 방식은 중복 소비를 방지합니다. 이 부분의 동작은 의도에 부합합니다.
Comment on lines
+81
to
83
| // access_token 또는 temp_token 둘 다 허용 | ||
| if (COOKIE_ACCESS_TOKEN.getValue().equals(cookie.getName()) || COOKIE_TEMP_TOKEN.getValue().equals(cookie.getName())) { | ||
| return cookie.getValue(); |
There was a problem hiding this comment.
쿠키 우선순위 명시: access_token을 우선, 없을 때만 temp_token 허용.
현재 구현은 쿠키 배열 순서에 따라 temp_token이 먼저 선택될 수 있습니다. 일반 API 접근 시 임시 토큰 선택을 피하려면 access_token 우선 검색이 안전합니다.
- if (request.getCookies() != null) {
- for (Cookie cookie : request.getCookies()) {
- // access_token 또는 temp_token 둘 다 허용
- if (COOKIE_ACCESS_TOKEN.getValue().equals(cookie.getName()) || COOKIE_TEMP_TOKEN.getValue().equals(cookie.getName())) {
- return cookie.getValue();
- }
- }
- }
+ if (request.getCookies() != null) {
+ String access = null;
+ String temp = null;
+ for (Cookie cookie : request.getCookies()) {
+ if (COOKIE_ACCESS_TOKEN.getValue().equals(cookie.getName())) {
+ access = cookie.getValue();
+ } else if (COOKIE_TEMP_TOKEN.getValue().equals(cookie.getName())) {
+ temp = cookie.getValue();
+ }
+ }
+ if (access != null) return access;
+ if (temp != null) return temp;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // access_token 또는 temp_token 둘 다 허용 | |
| if (COOKIE_ACCESS_TOKEN.getValue().equals(cookie.getName()) || COOKIE_TEMP_TOKEN.getValue().equals(cookie.getName())) { | |
| return cookie.getValue(); | |
| if (request.getCookies() != null) { | |
| String access = null; | |
| String temp = null; | |
| for (Cookie cookie : request.getCookies()) { | |
| if (COOKIE_ACCESS_TOKEN.getValue().equals(cookie.getName())) { | |
| access = cookie.getValue(); | |
| } else if (COOKIE_TEMP_TOKEN.getValue().equals(cookie.getName())) { | |
| temp = cookie.getValue(); | |
| } | |
| } | |
| if (access != null) return access; | |
| if (temp != null) return temp; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java
around lines 81 to 83, the current cookie selection allows whichever matching
cookie appears first (so temp_token can be chosen before access_token); change
the logic to explicitly prefer COOKIE_ACCESS_TOKEN: first search cookies for a
cookie whose name equals COOKIE_ACCESS_TOKEN and return its value if found,
otherwise search for COOKIE_TEMP_TOKEN and return its value if found; if neither
exists, return null. Ensure you do not rely on array order and handle null/empty
cookie arrays safely.
Comment on lines
+42
to
+64
| if (entry.getType() == TokenType.ACCESS) { | ||
| ResponseCookie cookie = ResponseCookie.from(COOKIE_ACCESS_TOKEN.getValue(), entry.getToken()) | ||
| .httpOnly(true) | ||
| .secure(true) | ||
| .sameSite("None") | ||
| .path("/") | ||
| .maxAge(Duration.ofDays(30)) // 30일짜리 쿠키 | ||
| .build(); | ||
| response.addHeader("Set-Cookie", cookie.toString()); | ||
|
|
||
| return ResponseEntity.ok(Map.of("type", TokenType.ACCESS.getValue())); | ||
| } else { | ||
| ResponseCookie cookie = ResponseCookie.from(COOKIE_TEMP_TOKEN.getValue(), entry.getToken()) | ||
| .httpOnly(true) | ||
| .secure(true) | ||
| .sameSite("None") | ||
| .path("/") // 일단 모든 | ||
| .maxAge(Duration.ofMinutes(10)) // 10분짜리 쿠키 | ||
| .build(); | ||
| response.addHeader("Set-Cookie", cookie.toString()); | ||
|
|
||
| return ResponseEntity.ok(Map.of("type", TokenType.TEMP.getValue())); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
쿠키 Domain 설정을 프로퍼티로 분리하여 환경별 호환성 확보.
현재 Domain 미설정으로 응답 호스트에 종속됩니다. 프론트-API가 서로 다른 서브도메인에 있는 경우, 공용 도메인 설정이 필요할 수 있습니다.
권장 접근:
- application 설정에 security.cookie-domain(예: .thip.com) 도입
- 로컬/테스트는 빈 값 허용, 빈 값일 때는 Domain 미지정
예시(필요한 위치에 추가):
@Value("${security.cookie-domain:}")
private String cookieDomain;체이닝 대신 빌더를 변수로 받아 조건부 domain 설정:
ResponseCookie.ResponseCookieBuilder builder =
ResponseCookie.from(COOKIE_ACCESS_TOKEN.getValue(), entry.getToken())
.httpOnly(true)
.secure(true)
.sameSite("None")
.path("/")
.maxAge(Duration.ofDays(30));
if (cookieDomain != null && !cookieDomain.isBlank()) {
builder.domain(cookieDomain);
}
response.addHeader("Set-Cookie", builder.build().toString());TEMP 쿠키도 동일하게 적용하세요.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/security/oauth2/AuthCookieController.java
around lines 42 to 64, the cookie Domain is not set which ties cookies to the
response host; add a configurable property and conditionally set the domain on
the ResponseCookie builders. Inject a property like security.cookie-domain (e.g.
@Value("${security.cookie-domain:}") private String cookieDomain) and replace
the direct ResponseCookie.from(...).build() calls with a builder variable for
both ACCESS and TEMP branches; if cookieDomain is non-empty (not null/blank)
call builder.domain(cookieDomain) before builder.build(), otherwise leave domain
unset for local/testing. Ensure both cookies use the same conditional domain
logic and keep existing flags (httpOnly, secure, sameSite, path, maxAge).
| public class CustomSuccessHandler extends SimpleUrlAuthenticationSuccessHandler { | ||
|
|
||
| private static final int COOKIE_MAX_AGE = 60 * 60 * 24; // 1일 | ||
| private final LoginTokenStorage loginTokenStorage; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
확장/배포 환경 고려: LoginTokenStorage의 인스턴스 간 일관성.
인메모리 저장소는 멀티 인스턴스/무상태 배포에서 다른 노드로 라우팅 시 토큰 검색 실패를 유발합니다. Sticky session을 강제하거나, Redis 같은 공유 스토리지로 전환하세요.
레포 내 구현체를 확인해 단일 노드 전제인지 점검해 주세요:
🏁 Script executed:
#!/bin/bash
# LoginTokenStorage 타입과 빈 스코프/설정 확인
rg -n "class LoginTokenStorage" -A 50
rg -n "@Bean|@Configuration|@Enable" -A 10 | rg -n "LoginTokenStorage"Length of output: 3866
🏁 Script executed:
#!/bin/bash
# 1. LoginTokenStorage에 @Component 등 스프링 빈 어노테이션이 있는지 확인
rg -n "@Component" -A 1 src/main/java/konkuk/thip/common/security/oauth2/LoginTokenStorage.java
# 2. CustomSuccessHandler에서 LoginTokenStorage가 어떻게 주입되는지 확인
rg -n "class CustomSuccessHandler" -A 50 src/main/java/konkuk/thip/common/security/oauth2/CustomSuccessHandler.javaLength of output: 2681
인메모리 LoginTokenStorage는 단일 노드 전제
현재 src/main/java/konkuk/thip/common/security/oauth2/LoginTokenStorage.java는
@component 싱글톤 빈으로 ConcurrentHashMap 기반 인메모리 스토어를 사용합니다.
멀티 인스턴스/무상태 배포 환경에서 다른 노드로 요청이 라우팅되면 저장된 토큰 조회가 실패하게 됩니다.
해결 방안:
- 세션 어피니티(Sticky Session)를 적용하여 항상 동일 노드로 라우팅되도록 보장
- Redis, Hazelcast 등 외부 공유 스토리지를 사용하도록
LoginTokenStorage구현을 변경
Comment on lines
+49
to
+61
| 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); | ||
| } else { | ||
| // 기존 유저 - 로그인용 액세스 토큰 | ||
| String accessToken = jwtUtil.createAccessToken(loginUser.userId()); | ||
| addTokenCookie(response, accessToken); | ||
| getRedirectStrategy().sendRedirect(request, response, webRedirectUrl + REDIRECT_HOME_URL.getValue()); | ||
| // addTokenCookie(response, accessToken); | ||
|
|
||
| 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); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
loginTokenKey TTL(5분) 재검토: 사용자 흐름과 TEMP 쿠키(10분)와의 정합성.
웹 리디렉션 후 셋-쿠키 호출까지 5분을 넘길 경우 400이 발생합니다. TEMP 쿠키 TTL(10분)과 정렬하거나 환경 변수로 관리하는 편이 안전합니다.
- loginTokenStorage.put(loginTokenKey, TokenType.TEMP, tempToken, Duration.ofMinutes(5)); // ttl 5분
+ loginTokenStorage.put(loginTokenKey, TokenType.TEMP, tempToken, Duration.ofMinutes(10)); // ttl 10분
...
- loginTokenStorage.put(loginTokenKey, TokenType.ACCESS, accessToken, Duration.ofMinutes(5)); // ttl 5분
+ loginTokenStorage.put(loginTokenKey, TokenType.ACCESS, accessToken, Duration.ofMinutes(10)); // ttl 10분추가로 application 설정(ex. security.login-token-ttl-minutes)으로 외부화하는 것을 추천합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 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); | |
| } else { | |
| // 기존 유저 - 로그인용 액세스 토큰 | |
| String accessToken = jwtUtil.createAccessToken(loginUser.userId()); | |
| addTokenCookie(response, accessToken); | |
| getRedirectStrategy().sendRedirect(request, response, webRedirectUrl + REDIRECT_HOME_URL.getValue()); | |
| // addTokenCookie(response, accessToken); | |
| 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); | |
| String loginTokenKey = UUID.randomUUID().toString(); | |
| loginTokenStorage.put(loginTokenKey, TokenType.TEMP, tempToken, Duration.ofMinutes(10)); // ttl 10분 | |
| getRedirectStrategy().sendRedirect(request, response, webRedirectUrl + REDIRECT_SIGNUP_URL.getValue() + "?loginTokenKey=" + loginTokenKey); | |
| } else { | |
| // 기존 유저 - 로그인용 액세스 토큰 | |
| String accessToken = jwtUtil.createAccessToken(loginUser.userId()); | |
| // addTokenCookie(response, accessToken); | |
| String loginTokenKey = UUID.randomUUID().toString(); | |
| loginTokenStorage.put(loginTokenKey, TokenType.ACCESS, accessToken, Duration.ofMinutes(10)); // ttl 10분 | |
| getRedirectStrategy().sendRedirect(request, response, webRedirectUrl + REDIRECT_HOME_URL.getValue() + "?loginTokenKey=" + loginTokenKey); |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/security/oauth2/CustomSuccessHandler.java
around lines 49–61, the loginTokenKey TTL is hardcoded to 5 minutes which can
expire before the TEMP cookie (10 minutes) and cause 400 errors; change both
places that call Duration.ofMinutes(5) to use a shared configurable value (e.g.
read from application properties security.login-token-ttl-minutes or an env var)
and ensure the TEMP cookie TTL and ACCESS/TEMP loginToken storage TTL are
aligned (use the same config value, default to 10 minutes if unspecified).
Update the class to inject the config (or a properties holder) and replace the
hardcoded Duration.ofMinutes(5) with Duration.ofMinutes(configValue) for both
branches so the redirect -> set-cookie flow uses consistent TTLs.
| @@ -0,0 +1,48 @@ | |||
| package konkuk.thip.common.security.oauth2; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
보안/운영 검증 요청: key 생성 강도, 쿠키 속성, 멀티 인스턴스 대응
- loginTokenKey 생성이 충분히 예측 불가한지(SecureRandom/UUID v4 이상) 확인 필요.
- /api/set-cookie에서 발급하는 쿠키에 Secure, HttpOnly, SameSite=None이 설정되어 있는지 확인 필요.
- 본 구현은 인메모리이므로 멀티 인스턴스 환경에서 set-cookie 요청이 다른 인스턴스로 라우팅되면 조회 실패합니다. Sticky session, Redis 등 중앙 저장소, 또는 토큰을 직접 서명해 전달하는 대안 고려가 필요합니다.
다음 스크립트로 PR 브랜치에서 관련 구현을 빠르게 점검할 수 있습니다.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== LoginTokenStorage 사용처 확인 =="
rg -n "LoginTokenStorage|loginTokenKey|TokenType|set-cookie" -A 5 -B 5
echo
echo "== key 생성 로직 후보 확인(SecureRandom/UUID v4 등) =="
rg -n "SecureRandom|UUID\.randomUUID|KeyGenerator|Base64" -A 2 -B 2
echo
echo "== AuthCookieController에서 쿠키 속성 확인(ResponseCookie/Cookie 사용 여부) =="
fd -a "AuthCookieController.java" | while read -r f; do
echo "---- $f ----"
rg -n "ResponseCookie|new Cookie|HttpOnly|SameSite|Secure" -A 2 -B 2 "$f" || true
done
echo
echo "== Security 설정에서 /api/set-cookie 필터/권한 예외 확인 =="
rg -n "/api/set-cookie|set-cookie" src/main/java -A 5 -B 5
echo
echo "== TTL 설정(예: 5분) 실제 전달 여부 확인 =="
fd -a "CustomSuccessHandler.java" | while read -r f; do
echo "---- $f ----"
rg -n "LoginTokenStorage|Duration|ofMinutes|put\(" -A 3 -B 3 "$f" || true
doneLength of output: 24513
보안/운영 검증 완료 (key 생성, 쿠키 속성, 보안 필터, TTL 확인) – 다중 인스턴스 저장소 검토 필요
- Key 생성
UUID.randomUUID()를 사용해 버전4 UUID(≈122비트 랜덤)를 생성하므로 예측 불가능성 충분함
- 쿠키 속성
ResponseCookie.from(...)에.httpOnly(true),.secure(true),.sameSite("None")가 모두 적용되어 있음
- 보안 필터
JwtAuthenticationFilter에서/api/set-cookie경로를path.equals("/api/set-cookie")로 skip 처리해 인가 예외가 확인됨
- TTL 설정
CustomSuccessHandler에서loginTokenStorage.put(..., Duration.ofMinutes(5))로 5분 TTL이 올바르게 적용됨
현재 LoginTokenStorage는 ConcurrentHashMap 기반의 인메모리 저장소이므로, 인스턴스 간 라우팅이 바뀔 경우 키 조회에 실패할 수 있습니다.
- 스티키 세션 유지
- Redis / 분산 캐시 등 중앙 저장소 도입
- 또는 로그인 토큰에 서명을 붙여 자체 검증하는 방식
위 중 하나 이상의 대안을 검토·적용해 주세요.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/security/oauth2/LoginTokenStorage.java
around line 1, the current implementation uses an in-memory ConcurrentHashMap
which will fail in multi-instance deployments; replace or augment this
implementation with a distributed/session-consistent approach: switch storage to
a shared cache (e.g., Redis) and use its TTL feature when storing tokens, or
implement sticky sessions at the load-balancer level, or alternatively stop
storing state and embed a signed login token (JWT or HMAC-signed value) that can
be validated without central storage; ensure the chosen approach preserves the
current TTL semantics, concurrency safety, and proper
serialization/deserialization of keys/values and update callers to use the new
storage API.
| } | ||
| } | ||
|
|
||
| private final ConcurrentHashMap<String, Entry> store = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
만료 토큰이 영구 잔존 → OOM/DoS 리스크. 주기적 제거 또는 Caffeine/Redis로 전환 필요
현재 만료 처리는 consume 시점에만 이뤄져, 소비되지 않은 만료 토큰이 메모리에 무기한 남습니다. 로그인 시도 스팸으로 메모리를 지속적으로 증가시킬 수 있어 OOM/DoS 위험이 있습니다. 최소한 주기적 스위핑 또는 TTL/사이즈 기반의 자동 제거가 필요합니다.
옵션 A: 간단한 주기적 스위핑 추가(1분마다 만료 토큰 제거)
+import org.springframework.scheduling.annotation.Scheduled;+ // 만료 토큰 주기적 제거 (DoS 방어의 1차 안전망)
+ @Scheduled(fixedDelay = 60_000)
+ void evictExpired() {
+ long now = System.currentTimeMillis();
+ store.entrySet().removeIf(e -> e.getValue().expireAtEpochMillis <= now);
+ }옵션 B: Caffeine 캐시(권장)로 교체하여 TTL/사이즈 기반 자동 제거
- 장점: 고성능, 자동 만료/용량 제한, 통계/모니터링 지원.
- TTL이 호출마다 달라야 한다면 per-entry expiry(Expiry) 구현을 사용. TTL이 고정(예: 5분)이면 expireAfterWrite로 간단히 구성 가능.
- 멀티 인스턴스 환경까지 고려하면 Redis 같은 중앙 스토리지로 전환하는 것이 가장 안전합니다(아래 다른 코멘트 참고).
Also applies to: 38-47
🤖 Prompt for AI Agents
src/main/java/konkuk/thip/common/security/oauth2/LoginTokenStorage.java around
lines 28 and 38-47: 현재 ConcurrentHashMap에 만료 토큰이 소비될 때만 제거되어 메모리에 무기한 잔존해
OOM/DoS 위험이 있으니, 수정방법은 두가지 중 하나를 선택하여 적용하세요 — (A) 간단한 주기 스위퍼를 추가:
ScheduledExecutorService로 1분 간격으로 맵을 순회해 만료된 Entry를 제거하도록 구현(동시성 고려하여 iterator
사용 또는 keySet을 복사한 후 제거), 또는 (B, 권장) Caffeine으로 교체:
Caffeine.newBuilder().expireAfterWrite(TTL) 및 maximumSize(...)를 설정해 자동 만료/용량제한을
적용(토큰별 가변 TTL이 필요하면 Expiry 구현 사용); 멀티인스턴스 환경에서는 중앙 저장소(예: Redis)로 대체하는 방안도
검토하세요.
Comment on lines
+33
to
+36
| public void put(String key, TokenType type, String token, Duration ttl) { | ||
| long expiredAt = Instant.now().plus(ttl).toEpochMilli(); | ||
| store.put(key, new Entry(type, token, expiredAt)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
입력값 검증 누락(null/blank/비양수 TTL 허용) → 방어적 코딩 필요
키/토큰/타입/TTL에 대한 기본 검증이 없어 의도치 않은 상태가 저장될 수 있습니다. 최소한의 가드 추가를 권합니다.
- public void put(String key, TokenType type, String token, Duration ttl) {
- long expiredAt = Instant.now().plus(ttl).toEpochMilli();
- store.put(key, new Entry(type, token, expiredAt));
- }
+ public void put(String key, TokenType type, String token, Duration ttl) {
+ if (key == null || key.isBlank()) throw new IllegalArgumentException("key must not be blank");
+ if (type == null) throw new IllegalArgumentException("type must not be null");
+ if (token == null || token.isBlank()) throw new IllegalArgumentException("token must not be blank");
+ if (ttl == null || ttl.isZero() || ttl.isNegative()) throw new IllegalArgumentException("ttl must be positive");
+
+ long expiresAt = System.currentTimeMillis() + ttl.toMillis();
+ store.put(key, new Entry(type, token, expiresAt));
+ }추가 제안(선택): 동일 key 재사용 방지를 위해 putIfAbsent로 단일 발급 보장, 필요 시 반환형을 boolean으로 변경해 중복 발급 시도를 감지할 수 있습니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public void put(String key, TokenType type, String token, Duration ttl) { | |
| long expiredAt = Instant.now().plus(ttl).toEpochMilli(); | |
| store.put(key, new Entry(type, token, expiredAt)); | |
| } | |
| public void put(String key, TokenType type, String token, Duration ttl) { | |
| if (key == null || key.isBlank()) { | |
| throw new IllegalArgumentException("key must not be blank"); | |
| } | |
| if (type == null) { | |
| throw new IllegalArgumentException("type must not be null"); | |
| } | |
| if (token == null || token.isBlank()) { | |
| throw new IllegalArgumentException("token must not be blank"); | |
| } | |
| if (ttl == null || ttl.isZero() || ttl.isNegative()) { | |
| throw new IllegalArgumentException("ttl must be positive"); | |
| } | |
| long expiresAt = System.currentTimeMillis() + ttl.toMillis(); | |
| store.put(key, new Entry(type, token, expiresAt)); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/security/oauth2/LoginTokenStorage.java
around lines 33 to 36, the put method lacks input validation allowing null/blank
key/token/type and non-positive TTL; add defensive checks at the start to throw
IllegalArgumentException for null or blank key/token, null type, and ttl == null
or ttl.isZero() or ttl.isNegative(); compute expiredAt only after validation and
then store. Optionally, to prevent duplicate issuance, replace store.put(...)
with store.putIfAbsent(...) and change the method to return boolean indicating
whether insertion succeeded.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#️⃣ 연관된 이슈
📝 작업 내용
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신기능
변경사항