Conversation
Walkthrough쿠키 기반 토큰 처리와 관련 상수를 제거하고, JWT 추출을 Authorization 헤더 전용으로 단순화했다. OAuth2 로그인 성공 시 토큰을 서버 측 저장소에 저장하도록 변경하고 리디렉트 경로를 명시적으로 구성했다. 인증 필터 화이트리스트와 보안 설정(쿠키 secure 고정)을 갱신했으며 일부 엔드포인트를 Deprecated로 표시했다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OAuth2Provider as OAuth2 Provider
participant Server as THIP Server
participant TokenStore as LoginTokenStorage
Client->>Server: OAuth2 로그인 시작
Server->>OAuth2Provider: 인증 요청
OAuth2Provider-->>Server: 콜백 (사용자 정보)
Server->>TokenStore: 토큰 저장 (TEMP 또는 ACCESS, TTL 5분)
alt 신규 사용자
Server-->>Client: 302 Redirect (webRedirect + /signup)
else 기존 사용자
Server-->>Client: 302 Redirect (webRedirect + /feed)
end
sequenceDiagram
participant Client
participant Server as THIP Server
participant JwtFilter as JwtAuthenticationFilter
Client->>Server: 보호 API 요청
Server->>JwtFilter: 요청 필터링
alt Authorization 헤더에 Bearer 토큰 존재
JwtFilter-->>Server: 인증 컨텍스트 설정
Server-->>Client: 응답
else 토큰 없음/비정상
JwtFilter-->>Server: 인증 없음 처리
Server-->>Client: 401/필요 시 핸들러 처리
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(해당 없음) Possibly related PRs
Suggested reviewersPoem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 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 (
|
| // // 2. Cookie에서 JWT 추출 (웹) | ||
| // 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(); | ||
| // } | ||
| // } | ||
| // } |
There was a problem hiding this comment.
쿠키 안쓰니까 그냥 지우는건 어떤가요?
There was a problem hiding this comment.
정말 혹시 모를 것을 대비해서 Deprecated로 남겨두었습니다! 화욜에 컨펌받고 모두 제거하겠습니다!!
Test Results407 tests 407 ✅ 29s ⏱️ Results for commit 520070c. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/main/java/konkuk/thip/common/security/oauth2/auth/AuthController.java (1)
190-199: 중대한 보안 이슈: 전달된 userId를 신뢰하고 토큰을 발급함.
/auth/exchange-temp-token에서userId를 요청 바디에서 받아 그대로 액세스 토큰을 발급합니다.oauth2Id와의 소유 관계 검증이 없어, 유효한 임시 토큰 보유자가 임의의userId로 액세스 토큰을 탈취할 수 있습니다. 최소한oauth2Id로 사용자를 조회하여 해당 사용자의userId를 사용하거나, 요청 바디의userId와 일치하는지 검증이 필요합니다.권장 수정안 1:
oauth2Id로userId를 조회해 바디 값을 무시- String userIdStr = body.get("userId"); - if (userIdStr == null || userIdStr.isBlank()) { - return ResponseEntity.status(HttpStatus.BAD_REQUEST).build(); - } - Long userId = Long.valueOf(userIdStr); + var user = userJpaRepository.findByOauth2Id(oauth2Id) + .orElseThrow(() -> new BusinessException(ErrorCode.USER_NOT_SIGNED_UP)); + Long userId = user.getUserId();권장 수정안 2: 바디의
userId와 소유 관계 일치 검증- Long userId = Long.valueOf(userIdStr); + Long userId = Long.valueOf(userIdStr); + var user = userJpaRepository.findByOauth2Id(oauth2Id) + .orElseThrow(() -> new BusinessException(ErrorCode.USER_NOT_SIGNED_UP)); + if (!user.getUserId().equals(userId)) { + return ResponseEntity.status(HttpStatus.FORBIDDEN).build(); + }
🧹 Nitpick comments (8)
src/main/java/konkuk/thip/common/security/oauth2/CustomSuccessHandler.java (3)
26-26: 사용되지 않는 상수 제거 제안(COOKIE_MAX_AGE).쿠키 기반 전달을 제거했으므로,
COOKIE_MAX_AGE는 더 이상 사용되지 않습니다. 정리해 주세요.- private static final int COOKIE_MAX_AGE = 60 * 60 * 24; // 1일
49-50: 매직 넘버(5분 TTL) 상수화로 중복 제거.동일한 TTL이 두 군데 반복됩니다. 상수로 추출해 가독성과 유지보수성을 높이는 게 좋습니다.
- loginTokenStorage.put(loginTokenKey, TokenType.TEMP, tempToken, Duration.ofMinutes(5)); // ttl 5분 + loginTokenStorage.put(loginTokenKey, TokenType.TEMP, tempToken, LOGIN_TOKEN_TTL); // ttl 5분 @@ - loginTokenStorage.put(loginTokenKey, TokenType.ACCESS, accessToken, Duration.ofMinutes(5)); // ttl 5분 + loginTokenStorage.put(loginTokenKey, TokenType.ACCESS, accessToken, LOGIN_TOKEN_TTL); // ttl 5분다음 상수를 클래스 상단에 추가해 주세요:
// 추가: 클래스 필드 private static final Duration LOGIN_TOKEN_TTL = Duration.ofMinutes(5);Also applies to: 57-58
51-51: 리다이렉트 URL 문자열 연결 대신 안전한 빌더 사용.쿼리 파라미터 인코딩 등 안전성을 위해
UriComponentsBuilder사용을 권장합니다.- getRedirectStrategy().sendRedirect(request, response, webRedirectUrl + REDIRECT_SIGNUP_URL.getValue() + "?loginTokenKey=" + loginTokenKey); + String redirectUrl = org.springframework.web.util.UriComponentsBuilder + .fromHttpUrl(webRedirectUrl + REDIRECT_SIGNUP_URL.getValue()) + .queryParam("loginTokenKey", loginTokenKey) + .build() + .toUriString(); + getRedirectStrategy().sendRedirect(request, response, redirectUrl); @@ - getRedirectStrategy().sendRedirect(request, response, webRedirectUrl + REDIRECT_HOME_URL.getValue() + "?loginTokenKey=" + loginTokenKey); + String redirectUrl = org.springframework.web.util.UriComponentsBuilder + .fromHttpUrl(webRedirectUrl + REDIRECT_HOME_URL.getValue()) + .queryParam("loginTokenKey", loginTokenKey) + .build() + .toUriString(); + getRedirectStrategy().sendRedirect(request, response, redirectUrl);참고: import를 추가하면 가독성이 좋아집니다.
import org.springframework.web.util.UriComponentsBuilder;Also applies to: 59-59
src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java (3)
76-85: 주석 처리된 쿠키 경로는 제거 권장.쿠키 기반 경로를 주석으로 남겨두면 혼동을 유발합니다. 히스토리는 VCS가 보장하므로 완전히 삭제하는 것을 권장합니다.
-// // 2. Cookie에서 JWT 추출 (웹) -// 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(); -// } -// } -// }
85-85: 불필요한 INFO 로그를 DEBUG로 하향 조정.허용된(permitAll) 엔드포인트에서도 토큰 부재 로그가 다량 발생할 수 있습니다. 레벨을 낮추거나 경로 조건부 로깅을 고려해 주세요.
- log.info("토큰이 없습니다."); + log.debug("토큰이 없습니다. path={}", request.getRequestURI());
100-105: 화이트리스트/필터 제외 정책 정합성 점검.
/auth/exchange-temp-token은@Oauth2Id해석을 위해 필터를 타는 것이 맞습니다. 반면/auth/set-cookie는 JWT가 필요 없으므로 필터 제외(shouldNotFilter)에 넣어 로그/오버헤드를 줄일 수 있습니다. 현재는 SecurityConfig에서 permitAll이지만, 필터는 여전히 실행됩니다.필요 시 아래처럼
shouldNotFilter에 추가 고려:|| path.equals("/auth/set-cookie")src/main/java/konkuk/thip/common/security/oauth2/auth/AuthController.java (2)
130-147: 쿠키 secure(true) 강제의 운영 적합성 확인.
SameSite=None과 함께secure(true)는 보안상 적절합니다. 다만 로컬 HTTP 환경에서는 쿠키가 설정되지 않으므로(브라우저 정책) 개발/테스트 경로가 남아있는지 확인해 주세요. 어차피 deprecated라면 곧 제거 일정을 함께 문서화하는 것을 권장합니다.
154-158: Deprecated 표기 적절. 제거 일정/마이그레이션 경로 명시 권장.API 문서(Swagger)에도 제거 예정 버전/대체 플로우(
/auth/token사용)를 명확히 안내해 주세요.원하시면 OpenAPI description에 마이그레이션 가이드를 추가하는 PR 문구까지 마련해 드리겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/main/java/konkuk/thip/common/security/constant/AuthParameters.java(0 hunks)src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java(2 hunks)src/main/java/konkuk/thip/common/security/oauth2/CustomSuccessHandler.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/auth/AuthController.java(5 hunks)src/main/java/konkuk/thip/config/SecurityConfig.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/konkuk/thip/common/security/constant/AuthParameters.java
⏰ 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 (1)
src/main/java/konkuk/thip/config/SecurityConfig.java (1)
55-58: 화이트리스트 재구성 방향은 타당.헤더 기반 토큰 흐름으로의 전환과 맞물려,
/auth/users,/auth/token허용과 쿠키 관련 엔드포인트의 deprecate 표식이 일관됩니다. 다만/auth/set-cookie는 JWT가 불필요하므로 필터 제외(OncePerRequestFilter.shouldNotFilter)와의 정합성도 함께 고려해 주세요.
| String authorization = request.getHeader(JWT_HEADER_KEY.getValue()); | ||
| if (authorization != null && authorization.startsWith(JWT_PREFIX.getValue())) { | ||
| return authorization.split(" ")[1]; | ||
| } |
There was a problem hiding this comment.
Authorization 파싱 안전성 개선 필요 (AIOOBE 가능성).
split(" ")[1]은 Bearer 뒤에 토큰이 없거나 공백이 연속인 경우 예외가 발생할 수 있습니다. substring 기반 파싱으로 안전하게 처리하세요.
- if (authorization != null && authorization.startsWith(JWT_PREFIX.getValue())) {
- return authorization.split(" ")[1];
- }
+ if (authorization != null && authorization.startsWith(JWT_PREFIX.getValue())) {
+ String token = authorization.substring(JWT_PREFIX.getValue().length()).trim();
+ if (!token.isEmpty()) {
+ return token;
+ }
+ }📝 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.
| String authorization = request.getHeader(JWT_HEADER_KEY.getValue()); | |
| if (authorization != null && authorization.startsWith(JWT_PREFIX.getValue())) { | |
| return authorization.split(" ")[1]; | |
| } | |
| String authorization = request.getHeader(JWT_HEADER_KEY.getValue()); | |
| if (authorization != null && authorization.startsWith(JWT_PREFIX.getValue())) { | |
| String token = authorization.substring(JWT_PREFIX.getValue().length()).trim(); | |
| if (!token.isEmpty()) { | |
| return token; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java
around lines 71 to 74, the current authorization parsing uses
authorization.split(" ")[1] which can throw an AIOOBE when the header has no
token or extra spaces; replace that split with a safer substring-based parse:
after verifying authorization startsWith the JWT_PREFIX value, obtain the token
via authorization.substring(JWT_PREFIX.getValue().length()), trim the result,
and return it only if non-empty (otherwise return null/handle as
unauthenticated) to avoid index errors and handle extra whitespace robustly.
2 participants
#️⃣ 연관된 이슈
📝 작업 내용
보일러플레이트 코드를 제거합니다. (쿠키 관련 로직 + 상수)
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit