Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 46 minutes and 18 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
Walkthrough이 PR은 인증 시스템을 쿠키 기반 토큰 전달 방식으로 전환합니다. RefreshTokenService는 JWT에서 userId 추출 방식을 변경하고, AuthController의 엔드포인트들은 응답 형식을 수정하며, AuthCookieManager와 AuthRedirectResolver 컴포넌트가 신규 추가됩니다. OAuth 리다이렉트는 origin 허용 목록 검증을 포함하도록 개선됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant AuthCtrl as AuthController
participant CookieMgr as AuthCookieManager
participant JwtProvider as JwtProvider
participant UserRepo as UserRepository
participant Response as HTTP Response
rect rgb(200, 150, 100, 0.5)
Note over Client,Response: 게스트 토큰 발급 플로우
Client->>AuthCtrl: GET /api/v1/user/guest/token
AuthCtrl->>JwtProvider: generateAccessToken(null)
JwtProvider-->>AuthCtrl: accessToken JWT
AuthCtrl->>JwtProvider: generateRefreshToken(null)
JwtProvider-->>AuthCtrl: refreshToken JWT
AuthCtrl->>CookieMgr: createCookie("accessToken", token)
CookieMgr-->>AuthCtrl: Set-Cookie: accessToken=...
AuthCtrl->>CookieMgr: createCookie("refreshToken", token)
CookieMgr-->>AuthCtrl: Set-Cookie: refreshToken=...
AuthCtrl->>Response: 200 OK (empty body)
Response-->>Client: 200 OK + cookies
end
sequenceDiagram
participant Client as 클라이언트
participant AuthCtrl as AuthController
participant Redirect as AuthRedirectResolver
participant OAuthSvc as OAuth Service
participant CookieMgr as AuthCookieManager
participant UserRepo as UserRepository
participant Response as HTTP Response
rect rgb(100, 150, 200, 0.5)
Note over Client,Response: 카카오톡 소셜 로그인 콜백 플로우
Client->>AuthCtrl: GET /api/v1/login/oauth2/callback?code=...&state=...
AuthCtrl->>OAuthSvc: 카카오 사용자 정보 조회
OAuthSvc-->>AuthCtrl: 사용자 정보
AuthCtrl->>UserRepo: 사용자 저장/조회
UserRepo-->>AuthCtrl: User 엔티티
AuthCtrl->>Redirect: resolve(state, request)
Redirect-->>AuthCtrl: 리다이렉트 URL
AuthCtrl->>CookieMgr: createCookie("accessToken", token)
CookieMgr-->>AuthCtrl: Set-Cookie: accessToken=...
AuthCtrl->>CookieMgr: createCookie("refreshToken", token)
CookieMgr-->>AuthCtrl: Set-Cookie: refreshToken=...
AuthCtrl->>Response: 302 Found + Location + cookies
Response-->>Client: 리다이렉트 + 쿠키 설정
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
📝 테스트 커버리지 리포트입니다!
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/dnd/moddo/auth/application/RefreshTokenService.java (1)
23-43:⚠️ Potential issue | 🔴 Criticalrefresh token 유형 검증 누락
RefreshTokenService.execute()가 전달된 토큰이 실제로 refresh token인지 확인하지 않습니다. 현재는 AUTH_ID 클레임만 검증하는데, access token도 같은 구조를 가지므로 access token으로도 새 access token을 재발급받을 수 있습니다.
JwtAuth.isNotExpectedToken()처럼 JWT 헤더의 TYPE 클레임을 REFRESH_KEY.getMessage()와 비교하여 검증하세요:
String tokenType = jwtUtil.getJwt(token).getHeader().get(JwtConstants.TYPE.message).toString(); if (!tokenType.equals(JwtConstants.REFRESH_KEY.getMessage())) { throw new TokenInvalidException(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/dnd/moddo/auth/application/RefreshTokenService.java` around lines 23 - 43, RefreshTokenService.execute currently only extracts AUTH_ID and never verifies the JWT's type, allowing access tokens to be used as refresh tokens; update execute to read the JWT header via jwtUtil.getJwt(token).getHeader().get(JwtConstants.TYPE.message) and compare it to JwtConstants.REFRESH_KEY.getMessage(), and if it does not match throw TokenInvalidException (same as other invalid cases) before extracting AUTH_ID and issuing a new access token.src/main/java/com/dnd/moddo/auth/presentation/AuthController.java (1)
88-106:⚠️ Potential issue | 🟠 Major로그아웃/연결해제 시
refreshToken쿠키도 만료시켜야 합니다.현재
kakaoLogout()과kakaoUnlink()에서accessToken쿠키만 만료시키고 있습니다.refreshToken쿠키가 유효한 상태로 남아있으면, 사용자가 로그아웃 후에도/user/reissue/token엔드포인트를 통해 새로운 액세스 토큰을 발급받을 수 있습니다.🔒 refreshToken 쿠키 만료 추가
`@PostMapping`("/logout") public ResponseEntity<?> kakaoLogout(`@CookieValue`(value = "accessToken") String token, `@LoginUser` LoginUserInfo loginUser) { - String cookie = authCookieManager.expireCookie("accessToken"); + String accessTokenCookie = authCookieManager.expireCookie("accessToken"); + String refreshTokenCookie = authCookieManager.expireCookie("refreshToken"); authService.logout(loginUser.userId()); return ResponseEntity.ok() - .header(HttpHeaders.SET_COOKIE, cookie) + .header(HttpHeaders.SET_COOKIE, accessTokenCookie, refreshTokenCookie) .body(Collections.singletonMap("message", "Logout successful")); } `@DeleteMapping`("/unlink") public ResponseEntity<?> kakaoUnlink(`@CookieValue`(value = "accessToken") String token, `@LoginUser` LoginUserInfo loginUser) { - String cookie = authCookieManager.expireCookie("accessToken"); + String accessTokenCookie = authCookieManager.expireCookie("accessToken"); + String refreshTokenCookie = authCookieManager.expireCookie("refreshToken"); authService.unlink(loginUser.userId()); return ResponseEntity.ok() - .header(HttpHeaders.SET_COOKIE, cookie) + .header(HttpHeaders.SET_COOKIE, accessTokenCookie, refreshTokenCookie) .body(Collections.singletonMap("message", "Unlink successful")); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/dnd/moddo/auth/presentation/AuthController.java` around lines 88 - 106, Both kakaoLogout and kakaoUnlink only expire the accessToken cookie; you must also expire the refreshToken cookie to prevent reissue. Call authCookieManager.expireCookie("refreshToken") inside both kakaoLogout and kakaoUnlink (alongside the existing expireCookie("accessToken")), capture the returned refresh cookie string, and include it as an additional Set-Cookie header on the ResponseEntity (e.g., add a second .header(HttpHeaders.SET_COOKIE, refreshCookie) or combine into response headers) after calling authService.logout(loginUser.userId()) / authService.unlink(loginUser.userId()).
🧹 Nitpick comments (3)
src/main/java/com/dnd/moddo/auth/infrastructure/security/JwtFilter.java (1)
37-40:startsWith대신 정확한 경로 매칭을 권장합니다.
startsWith("/api/v1/user/reissue/token")는/api/v1/user/reissue/token-something같은 의도치 않은 경로까지 필터를 우회시킬 수 있습니다. 정확한 경로 매칭이 더 안전합니다.♻️ 수정 제안
`@Override` protected boolean shouldNotFilter(HttpServletRequest request) { - return request.getRequestURI().startsWith("/api/v1/user/reissue/token"); + return request.getRequestURI().equals("/api/v1/user/reissue/token"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/dnd/moddo/auth/infrastructure/security/JwtFilter.java` around lines 37 - 40, In JwtFilter.shouldNotFilter change the loose startsWith check to an exact path match so only the exact reissue endpoint is excluded; locate the shouldNotFilter(HttpServletRequest request) method in JwtFilter and replace the request.getRequestURI().startsWith("/api/v1/user/reissue/token") condition with a precise comparison (e.g., equals after normalizing or checking request.getRequestURI() equals the exact "/api/v1/user/reissue/token" and, if needed, account for context path) so paths like "/api/v1/user/reissue/token-something" are not bypassed.src/main/java/com/dnd/moddo/auth/presentation/AuthRedirectResolver.java (1)
13-21: 하드코딩된 URL을 설정 파일로 외부화하는 것을 고려해 보세요.
DEFAULT_LOCAL_REDIRECT_URL,DEFAULT_PROD_REDIRECT_URL,ALLOWED_REDIRECT_ORIGINS가 여기에 하드코딩되어 있고,WebConfig.java의 CORS 설정에도 유사한 URL들이 하드코딩되어 있습니다. 이러한 URL들을application.yml에서 관리하면 유지보수성이 향상되고 환경별 설정 변경이 용이해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/dnd/moddo/auth/presentation/AuthRedirectResolver.java` around lines 13 - 21, The hard-coded redirect URLs and allowed origins (DEFAULT_LOCAL_REDIRECT_URL, DEFAULT_PROD_REDIRECT_URL, ALLOWED_REDIRECT_ORIGINS in AuthRedirectResolver) should be externalized to configuration: add properties in application.yml (e.g. auth.redirect.defaultLocal, auth.redirect.defaultProd, auth.redirect.allowedOrigins) and load them via a Spring `@ConfigurationProperties` or `@Value` in AuthRedirectResolver (or a dedicated AuthProperties class) so the resolver uses injected values instead of the static literals; also update WebConfig to read the same properties for CORS so both use the centralized config source.src/test/java/com/dnd/moddo/domain/auth/controller/AuthRedirectResolverTest.java (1)
11-66: 테스트 커버리지가 양호합니다.허용된 origin, 허용되지 않은 origin(로컬/운영), 잘못된 형식의 state에 대한 핵심 시나리오가 잘 테스트되어 있습니다.
선택적으로
127.0.0.1을 serverName으로 사용하는 케이스도 추가하면isLocalRequest()로직의 완전한 커버리지를 확보할 수 있습니다.🧪 127.0.0.1 테스트 케이스 추가 제안
+ `@Test` + `@DisplayName`("127.0.0.1 요청도 로컬 환경으로 인식하여 localhost:3000으로 리다이렉트한다.") + void fallbackToLocalUrlFor127Address() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setServerName("127.0.0.1"); + + String redirectUrl = authRedirectResolver.resolve( + "https://malicious.example.com/login/callback", + request + ); + + assertThat(redirectUrl).isEqualTo("http://localhost:3000"); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/dnd/moddo/domain/auth/controller/AuthRedirectResolverTest.java` around lines 11 - 66, Add a test covering the IPv4 loopback case by creating a new test method (e.g., fallbackToLocalUrlForInvalidOriginWith127001) that constructs a MockHttpServletRequest with request.setServerName("127.0.0.1"), calls AuthRedirectResolver.resolve with a malicious origin (e.g., "https://malicious.example.com/login/callback"), and asserts the result equals the local fallback ("http://localhost:3000"); this verifies AuthRedirectResolver.resolve and its isLocalRequest logic handle the 127.0.0.1 case the same as "localhost".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/dnd/moddo/auth/presentation/AuthController.java`:
- Around line 59-69: The RefreshTokenService.execute() must validate the token
type before trusting AUTH_ID: call jwtProvider.getTokenType(token) inside
RefreshTokenService.execute() and verify it equals the refresh-token type used
by your app (e.g., TokenType.REFRESH or "refresh_token"); if it does not match,
throw a suitable authentication/invalid-token exception so
AuthController.reissueAccessToken cannot accept an access token in place of a
refresh token. Ensure the check happens prior to extracting AUTH_ID and issuing
a new access token.
---
Outside diff comments:
In `@src/main/java/com/dnd/moddo/auth/application/RefreshTokenService.java`:
- Around line 23-43: RefreshTokenService.execute currently only extracts AUTH_ID
and never verifies the JWT's type, allowing access tokens to be used as refresh
tokens; update execute to read the JWT header via
jwtUtil.getJwt(token).getHeader().get(JwtConstants.TYPE.message) and compare it
to JwtConstants.REFRESH_KEY.getMessage(), and if it does not match throw
TokenInvalidException (same as other invalid cases) before extracting AUTH_ID
and issuing a new access token.
In `@src/main/java/com/dnd/moddo/auth/presentation/AuthController.java`:
- Around line 88-106: Both kakaoLogout and kakaoUnlink only expire the
accessToken cookie; you must also expire the refreshToken cookie to prevent
reissue. Call authCookieManager.expireCookie("refreshToken") inside both
kakaoLogout and kakaoUnlink (alongside the existing
expireCookie("accessToken")), capture the returned refresh cookie string, and
include it as an additional Set-Cookie header on the ResponseEntity (e.g., add a
second .header(HttpHeaders.SET_COOKIE, refreshCookie) or combine into response
headers) after calling authService.logout(loginUser.userId()) /
authService.unlink(loginUser.userId()).
---
Nitpick comments:
In `@src/main/java/com/dnd/moddo/auth/infrastructure/security/JwtFilter.java`:
- Around line 37-40: In JwtFilter.shouldNotFilter change the loose startsWith
check to an exact path match so only the exact reissue endpoint is excluded;
locate the shouldNotFilter(HttpServletRequest request) method in JwtFilter and
replace the request.getRequestURI().startsWith("/api/v1/user/reissue/token")
condition with a precise comparison (e.g., equals after normalizing or checking
request.getRequestURI() equals the exact "/api/v1/user/reissue/token" and, if
needed, account for context path) so paths like
"/api/v1/user/reissue/token-something" are not bypassed.
In `@src/main/java/com/dnd/moddo/auth/presentation/AuthRedirectResolver.java`:
- Around line 13-21: The hard-coded redirect URLs and allowed origins
(DEFAULT_LOCAL_REDIRECT_URL, DEFAULT_PROD_REDIRECT_URL, ALLOWED_REDIRECT_ORIGINS
in AuthRedirectResolver) should be externalized to configuration: add properties
in application.yml (e.g. auth.redirect.defaultLocal, auth.redirect.defaultProd,
auth.redirect.allowedOrigins) and load them via a Spring
`@ConfigurationProperties` or `@Value` in AuthRedirectResolver (or a dedicated
AuthProperties class) so the resolver uses injected values instead of the static
literals; also update WebConfig to read the same properties for CORS so both use
the centralized config source.
In
`@src/test/java/com/dnd/moddo/domain/auth/controller/AuthRedirectResolverTest.java`:
- Around line 11-66: Add a test covering the IPv4 loopback case by creating a
new test method (e.g., fallbackToLocalUrlForInvalidOriginWith127001) that
constructs a MockHttpServletRequest with request.setServerName("127.0.0.1"),
calls AuthRedirectResolver.resolve with a malicious origin (e.g.,
"https://malicious.example.com/login/callback"), and asserts the result equals
the local fallback ("http://localhost:3000"); this verifies
AuthRedirectResolver.resolve and its isLocalRequest logic handle the 127.0.0.1
case the same as "localhost".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0f8caf48-94e3-4cbd-a150-c77433a72c3b
📒 Files selected for processing (13)
src/docs/asciidoc/auth.adocsrc/main/java/com/dnd/moddo/auth/application/RefreshTokenService.javasrc/main/java/com/dnd/moddo/auth/infrastructure/security/JwtFilter.javasrc/main/java/com/dnd/moddo/auth/presentation/AuthController.javasrc/main/java/com/dnd/moddo/auth/presentation/AuthCookieManager.javasrc/main/java/com/dnd/moddo/auth/presentation/AuthRedirectResolver.javasrc/main/resources/application.ymlsrc/test/java/com/dnd/moddo/domain/auth/controller/AuthControllerTest.javasrc/test/java/com/dnd/moddo/domain/auth/controller/AuthCookieManagerTest.javasrc/test/java/com/dnd/moddo/domain/auth/controller/AuthRedirectResolverTest.javasrc/test/java/com/dnd/moddo/domain/auth/service/RefreshTokenServiceTest.javasrc/test/java/com/dnd/moddo/global/util/ControllerTest.javasrc/test/resources/application.yml
📝 테스트 커버리지 리포트입니다!
|
📝 테스트 커버리지 리포트입니다!
|
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.
#️⃣연관된 이슈
X
🔀반영 브랜치
fix/login -> develop
🔧변경 사항
HttpOnly=true기반으로 정리하고, 게스트 토큰 발급/카카오 로그인/토큰 재발급 응답이 쿠키를 통해 토큰을 전달하도록 수정했습니다.AuthCookieManager를 추가해 쿠키 생성/만료 로직을 분리했습니다.AuthRedirectResolver를 추가해 카카오 로그인 callback의stateURL을 검증하고, 허용되지 않은 origin은 로컬/운영 환경에 맞는 기본 URL로 fallback 하도록 처리했습니다.RefreshTokenService가 refresh token에서AUTH_ID를 기준으로 사용자를 조회하도록 보완했습니다.RefreshTokenService의 누락 분기와AuthCookieManager,AuthRedirectResolver에 대한 테스트 코드를 추가했습니다.💬리뷰 요구사항(선택)
X
체크
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정