refactor: accessToken 재발급을 refreshToken 쿠키를 통해 하도록 #452
Conversation
nayonsoso
requested review from
Gyuhyeok99,
lsy1307,
whqtker and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
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
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 (
|
![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
🧹 Nitpick comments (1)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java (1)
131-135: 리프레시 토큰 로테이션이 있다면 쿠키 갱신이 필요합니다.
- reissue에서 리프레시 토큰을 회전(rotating)한다면, 신규 토큰을 쿠키로 다시 내려주는 처리가 누락되어 있을 수 있습니다.
- 회전이 없다면 현 구현으로 충분하지만, 정책을 한 번 점검해 주세요.
(회전이 있을 때의 예시)
- public ResponseEntity<ReissueResponse> reissueToken( - HttpServletRequest request - ) { - String refreshToken = refreshTokenCookieManager.getRefreshToken(request); - ReissueResponse reissueResponse = authService.reissue(refreshToken); - return ResponseEntity.ok(reissueResponse); - } + public ResponseEntity<ReissueResponse> reissueToken( + HttpServletRequest request, + HttpServletResponse response + ) { + String refreshToken = refreshTokenCookieManager.getRefreshToken(request); + ReissueResponse reissueResponse = authService.reissue(refreshToken); + // TODO: ReissueResponse 구조에 맞춰 조건 분기 (예: 새 refreshToken 존재 시) + // if (reissueResponse.refreshToken() != null) { + // refreshTokenCookieManager.setCookie(response, reissueResponse.refreshToken()); + // } + return ResponseEntity.ok(reissueResponse); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java(2 hunks)src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java(2 hunks)src/main/java/com/example/solidconnection/auth/dto/ReissueRequest.java(0 hunks)src/main/java/com/example/solidconnection/auth/service/AuthService.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java(3 hunks)src/main/java/com/example/solidconnection/auth/service/SignInService.java(1 hunks)src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java(3 hunks)src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java(4 hunks)src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java(4 hunks)src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/example/solidconnection/auth/dto/ReissueRequest.java
🔇 Additional comments (14)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
59-59: REFRESH_TOKEN_NOT_EXISTS 상태 코드 변경 제안 — 401(Unauthorized) 사용리프레시 토큰 미존재는 인증 실패 흐름으로 분류하는 것이 일반적이므로 401이 더 적절합니다.
- 상태 코드 변경
- REFRESH_TOKEN_NOT_EXISTS 항목의 상태 코드를 400(BAD_REQUEST)에서 401(UNAUTHORIZED)으로 변경을 제안합니다.
검토 위치:
- src/main/java/com/example/solidconnection/common/exception/ErrorCode.java — 라인 59.
제안된 diff:
- REFRESH_TOKEN_NOT_EXISTS(HttpStatus.BAD_REQUEST.value(), "리프레시 토큰이 존재하지 않습니다."), + REFRESH_TOKEN_NOT_EXISTS(HttpStatus.UNAUTHORIZED.value(), "리프레시 토큰이 존재하지 않습니다."),변경이 클라이언트 처리 로직, 테스트, API 문서 등에 미치는 영향을 함께 검토해 주세요.
src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java (1)
83-83: LGTM: SiteUser 기반 토큰 발급 API로의 전환이 깔끔합니다.
- 테스트 가독성이 개선되고, 프로바이더의 새 API에 정확히 부합합니다.
src/main/java/com/example/solidconnection/auth/service/SignInService.java (1)
18-20: LGTM: SiteUser 직접 사용으로 중복 제거와 응집도 향상이 좋습니다.
- Subject/Role 중간 단계 제거로 API 표면이 단순해졌고, 유지보수성이 향상되었습니다.
src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java (2)
21-21: 테스트 코드의 상수 추출 좋습니다!
REFRESH_TOKEN_COOKIE_NAME상수로 추출하여 하드코딩된 문자열을 제거한 것은 유지보수성을 높이는 좋은 개선입니다.
73-112: 쿠키 추출 테스트 추가 훌륭합니다!새로운
getRefreshToken메서드에 대한 테스트 커버리지가 잘 구성되었습니다:
- 정상 케이스 (쿠키 존재 시)
- 쿠키가 전혀 없는 경우
- 다른 쿠키만 있는 경우
모든 엣지 케이스를 적절히 다루고 있습니다.
src/main/java/com/example/solidconnection/auth/service/AuthService.java (2)
30-31: SiteUser 기반 토큰 처리로의 리팩터링 깔끔합니다!
parseSiteUser를 사용하여 토큰에서 직접 SiteUser를 추출하는 방식으로 변경된 것이 코드를 더 간결하게 만들었습니다. Repository 조회와 Subject 파싱 과정을 제거하여 복잡도를 줄였네요.
56-65: 쿠키 기반 리프레시 토큰 재발급 로직 개선!메서드 시그니처가
reissue(String requestedRefreshToken)으로 단순화되어 쿠키에서 추출한 리프레시 토큰을 직접 받도록 변경되었습니다. 이제 불필요한 DTO와 userId 파라미터가 제거되어 더 명확해졌습니다.src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (2)
30-37: SiteUser 기반 테스트 설정 적절합니다!
SiteUserFixture를 사용하여 테스트 데이터를 생성하고,expectedSubject를 SiteUser의 ID로부터 도출하는 방식이 일관성 있고 명확합니다.
98-107: 토큰에서 SiteUser 추출 테스트 메서드명 개선!메서드명을
토큰으로부터_SiteUser_를_추출한다로 변경한 것이 실제 동작을 더 정확히 반영합니다. 테스트 로직도 SiteUser ID 비교로 적절히 구현되었습니다.src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java (2)
88-99: 재발급 테스트 로직 개선 확인!리프레시 토큰 재발급 테스트가 SiteUser 기반으로 잘 리팩터링되었습니다.
parseSiteUser를 사용하여 토큰에서 SiteUser를 추출하고 ID를 비교하는 방식이 명확합니다.
107-107: 리프레시 토큰 문자열 직접 전달로 간소화!
authService.reissue(invalidRefreshToken)호출이 새로운 API에 맞게 적절히 수정되었습니다.src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java (3)
26-35: SiteUser 기반 액세스 토큰 생성 리팩터링!
generateAccessToken(SiteUser siteUser)로 메서드 시그니처를 변경하여 더 직관적인 API가 되었습니다. SiteUser에서 Subject와 Role을 추출하는 방식이 깔끔합니다.
69-71: 헬퍼 메서드 유지 적절합니다!
toSubject헬퍼 메서드를 유지하여 SiteUser를 Subject로 변환하는 로직을 캡슐화한 것이 좋습니다.
62-67: 토큰 파싱 시 사용자 존재 검증 적용 — 보안 개선 확인 및 캐싱 고려
변경 요약
\tparseSiteUser가 토큰의 subject를 ID로 파싱한 뒤 siteUserRepository.findById(...)로 사용자 존재를 검증하고 없으면 CustomException(USER_NOT_FOUND)를 던집니다.호출 위치 확인
\tsrc/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java: 메서드 정의입니다.
\tsrc/main/java/com/example/solidconnection/auth/service/AuthService.java: 로그인·토큰 삭제·재발급 흐름에서 parseSiteUser가 호출됩니다.
\tsrc/test/java/com/example/solidconnection/auth/service/*.java: 단위 테스트들이 해당 메서드를 사용합니다.권장 사항
\t보안 개선은 적절하므로 승인합니다.
\t다만 호출 위치들이 인증·재발급 등 빈번히 실행되는 경로라 DB 조회가 매 호출마다 발생하면 성능 영향이 있을 수 있으니 캐싱 전략을 검토하세요.
\t옵션으로 로컬 캐시(Caffeine)·분산 캐시(Redis) 적용 또는 토큰에 포함 가능한 최소 불변 사용자 정보 유지 등을 고려하되, 캐시 무효화(사용자 삭제/권한 변경)와 토큰 보안(민감정보 포함 금지)을 반드시 설계하세요.
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java
Show resolved
Hide resolved
| .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); | ||
| AccessToken newAccessToken = authTokenProvider.generateAccessToken(subject, siteUser.getRole()); | ||
| SiteUser siteUser = authTokenProvider.parseSiteUser(requestedRefreshToken); | ||
| AccessToken newAccessToken = authTokenProvider.generateAccessToken(siteUser); |
There was a problem hiding this comment.
generateAccessToken 을 사용하는 주체 입장에서는 간단하고 좋네요 !
- AS-IS: long으로 SiteUser를 조회, SiteUser를 Subject로 만들어 authTokenProvider의 함수에 넘겨줬다. - TO-BS: authToken에는 어차피 '의미상' SiteUser라는 개념이 사용될 수 밖에 없다. 따라서 관련 로직을 함수 내부로 옮긴다. 이로 인해서 중복 코드를 줄일수도 있다.
nayonsoso
force-pushed
the
refactor/451-reissue-by-cookie-refresh-token
branch
from
9e81db5 to
e851aa2
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java (1)
88-94: 회원가입(sign-up) 성공 응답에 HttpOnly refreshToken 쿠키 누락 — 서버에서 쿠키를 설정해 주세요.검증 결과, OAuth 및 이메일 로그인 처리에는 refreshTokenCookieManager.setCookie 호출이 존재하지만 signUp 메서드에는 해당 호출이 없습니다.
- 회원가입 응답에 refreshToken 쿠키 설정 추가.
AuthController.signUp 메서드에 HttpServletResponse 매개변수를 추가하고, signUpService.signUp 호출 직후 refreshTokenCookieManager.setCookie(httpServletResponse, signInResponse.refreshToken())를 호출하세요.- 로컬 추가 점검 권장.
제공하신 스크립트로 다른 sign-up 경로에 누락된 호출이 없는지 확인하세요.적용 예시(diff)는 아래와 같습니다.
- public ResponseEntity<SignInResponse> signUp( - @Valid @RequestBody SignUpRequest signUpRequest - ) { - SignInResponse signInResponse = signUpService.signUp(signUpRequest); - return ResponseEntity.ok(signInResponse); - } + public ResponseEntity<SignInResponse> signUp( + @Valid @RequestBody SignUpRequest signUpRequest, + HttpServletResponse httpServletResponse + ) { + SignInResponse signInResponse = signUpService.signUp(signUpRequest); + refreshTokenCookieManager.setCookie(httpServletResponse, signInResponse.refreshToken()); + return ResponseEntity.ok(signInResponse); + }수정이 필요한 위치:
- src/main/java/com/example/solidconnection/auth/controller/AuthController.java — signUp 메서드 (현재 약 88–94행).
참고: 로컬 검증(rg) 결과 refreshTokenCookieManager.setCookie 호출은 AuthController.java의 52, 64, 75행에서 확인되었습니다; signUp에는 호출이 없습니다.
수정 후 재검증 부탁드립니다. 😊
♻️ Duplicate comments (1)
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java (1)
54-73: 1) 빈/공백 쿠키 값까지 방어한 점, 아주 좋습니다.쿠키 미존재/빈 문자열/공백 문자열을 모두 REFRESH_TOKEN_NOT_EXISTS로 일관 처리해서, 후속 단계의 NPE/검증 실패를 원천 차단했습니다. 이전 코멘트 취지와도 정확히 부합합니다.
🧹 Nitpick comments (2)
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java (2)
31-34: 2) maxAge 변환은 Duration 사용으로 의도를 더 명확히 해보세요.가독성을 위해 표준 타입을 활용하면 의도가 더 잘 드러납니다.
아래와 같이 교체를 제안드립니다.
- private long convertExpireTimeToCookieMaxAge(long milliSeconds) { - // jwt의 expireTime 단위인 millisecond를 cookie의 maxAge 단위인 second로 변환 - return milliSeconds / 1000; - } + private long convertExpireTimeToCookieMaxAge(long milliSeconds) { + // jwt의 expireTime 단위를 명확히 표현 + return java.time.Duration.ofMillis(milliSeconds).getSeconds(); + }
40-52: 3) Secure 하드코딩은 환경별 프로퍼티로 전환을 고려해 주세요.로컬 HTTP 환경에서
secure(true)쿠키는 저장되지 않으므로, 프로퍼티로 제어 가능하면 운영/개발 환경 모두 편해집니다. 단, SameSite=None인 경우 운영에서는 반드시 Secure가 유지되어야 합니다.
📜 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 (11)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java(2 hunks)src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java(2 hunks)src/main/java/com/example/solidconnection/auth/dto/ReissueRequest.java(0 hunks)src/main/java/com/example/solidconnection/auth/service/AuthService.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java(3 hunks)src/main/java/com/example/solidconnection/auth/service/SignInService.java(1 hunks)src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java(3 hunks)src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java(4 hunks)src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java(4 hunks)src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/example/solidconnection/auth/dto/ReissueRequest.java
🚧 Files skipped from review as they are similar to previous changes (6)
- src/main/java/com/example/solidconnection/common/exception/ErrorCode.java
- src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java
- src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java
- src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java
- src/main/java/com/example/solidconnection/auth/service/SignInService.java
- src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.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 (2)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java (1)
119-125: 1) 쿠키 기반 리이슈 흐름 전환, 목적에 부합합니다.요청에서 쿠키를 읽어 서비스에 위임하는 구성이 단순하고 명확합니다. 예외 전파도 CookieManager에서 일관 처리되어 좋습니다.
src/main/java/com/example/solidconnection/auth/service/AuthService.java (1)
56-65: 2) reissue(String) 흐름은 간결하고 목적에 부합합니다.토큰 유효성 검증 → 사용자 파싱 → 새 액세스 토큰 발급의 플로우가 명확합니다.
| @PostMapping("/reissue") | ||
| public ResponseEntity<ReissueResponse> reissueToken( | ||
| @AuthorizedUser long siteUserId, | ||
| @Valid @RequestBody ReissueRequest reissueRequest | ||
| HttpServletRequest request | ||
| ) { | ||
| ReissueResponse reissueResponse = authService.reissue(siteUserId, reissueRequest); | ||
| String refreshToken = refreshTokenCookieManager.getRefreshToken(request); | ||
| ReissueResponse reissueResponse = authService.reissue(refreshToken); | ||
| return ResponseEntity.ok(reissueResponse); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
3) CSRF/CORS 관점에서 재발급 엔드포인트 보호 상태를 확인해 주세요.
SameSite=None + Secure로 운영 시, CORS 정책이 빈틈없이 설정되었는지와 CSRF 방어(예: Double Submit, 전용 헤더) 전략이 적절한지 재확인하는 게 안전합니다.
아래 스크립트로 보안 설정 위치를 빠르게 조회해 보세요.
🏁 Script executed:
#!/usr/bin/env bash
# Spring Security/CORS 설정 탐색
rg -n -C3 '(SecurityFilterChain|WebSecurityConfigurerAdapter|CorsConfiguration|allowedOrigins?|allowedOriginPatterns|setAllowCredentials|csrf\()' \
-g '!**/build/**' -g '!**/out/**' -g '!**/target/**'Length of output: 7034
조치 권고 — /reissue 엔드포인트의 CSRF/CORS 보호 재검증 필요
검증 요약: SecurityConfiguration에서 CSRF가 비활성화(.csrf(AbstractHttpConfigurer::disable))되어 있고 CORS는 corsProperties.allowedOrigins()를 사용해 구성하며 configuration.setAllowCredentials(true)가 설정되어 있습니다.
확인한 위치:
- src/main/java/com/example/solidconnection/security/config/SecurityConfiguration.java — corsConfigurationSource()에서 setAllowedOrigins(corsProperties.allowedOrigins()) 및 configuration.setAllowCredentials(true) 설정, .csrf(...) 비활성화.
- src/main/java/com/example/solidconnection/security/config/CorsProperties.java — allowedOrigins를 properties로 주입함.
- src/main/java/com/example/solidconnection/chat/config/StompWebSocketConfig.java — registerStompEndpoints에서 setAllowedOrigins(allowedOrigins) 사용.
- src/main/java/com/example/solidconnection/auth/controller/AuthController.java — /reissue 엔드포인트가 쿠키에서 리프레시 토큰을 읽음.
권장 변경(간단 워크스루):
- CORS 허용 출처 고정.
- corsProperties.allowedOrigins에 정확한 신뢰 출처만 명시하고 "*" 또는 광범위 패턴을 사용하지 마세요.
- Credential 허용 시 정책 검증.
- allowCredentials(true)를 유지한다면 허용 출처 목록만 사용하거나 요청 Origin을 반사하도록 구현해 불특정 출처에 자격증명이 노출되지 않게 하세요.
- CSRF 보호 적용 또는 대체 메커니즘 도입.
- 쿠키 기반 리프레시를 유지하면 /reissue에 대해 CSRF를 활성화하거나 더블 서브밋 토큰 또는 별도 CSRF 헤더 검증을 구현하세요.
- 쿠키 속성 강화.
- 리프레시 쿠키에 HttpOnly, Secure, SameSite=None(교차 출처 사용 시)을 설정하고 도메인/경로 제한을 최소 권한으로 적용하세요.
- WebSocket 엔드포인트 검토.
- STOMP 핸드셰이크의 setAllowedOrigins도 동일 원칙으로 제한하고 핸드셰이크 인터셉터에서 추가 검증을 고려하세요.
한 줄 결론: 전역 CSRF 비활성화 + credentials 허용 조합은 리프레시 토큰을 쿠키로 읽는 상황에서 위험하므로 위 권장사항 중 최소 하나 이상을 적용해 주세요.
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/auth/controller/AuthController.java
around lines 119 to 125, the /reissue endpoint reads a refresh token from a
cookie while the app globally disables CSRF and allows credentials via CORS—this
combination is unsafe; update the security configuration and cookie handling as
follows: restrict corsProperties.allowedOrigins to explicit trusted origins
(remove "*" and wide patterns) or implement origin-reflection that only echoes
validated origins when configuration.setAllowCredentials(true) is used; enable
CSRF protection for cookie-based refresh or implement a CSRF mitigation such as
double-submit cookie or a required custom CSRF header for the /reissue endpoint;
harden the refresh cookie by setting HttpOnly, Secure, SameSite (None only if
cross-origin needed), and appropriate domain/path scope; and apply the same
origin restrictions and optional handshake validation to STOMP/WebSocket
endpoints.
| public void signOut(String token) { | ||
| Subject subject = authTokenProvider.parseSubject(token); | ||
| long siteUserId = Long.parseLong(subject.value()); | ||
| SiteUser siteUser = siteUserRepository.findById(siteUserId) | ||
| .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); | ||
|
|
||
| AccessToken accessToken = authTokenProvider.generateAccessToken(subject, siteUser.getRole()); | ||
| SiteUser siteUser = authTokenProvider.parseSiteUser(token); | ||
| AccessToken accessToken = authTokenProvider.generateAccessToken(siteUser); | ||
| authTokenProvider.deleteRefreshTokenByAccessToken(accessToken); | ||
| tokenBlackListService.addToBlacklist(accessToken); | ||
| } |
There was a problem hiding this comment.
1) signOut에서 새 토큰을 생성해 블랙리스트에 올리는 버그가 있습니다.
현재 코드는 신규 AccessToken을 만들어 블랙리스트에 올려 기존 토큰이 유효한 상태로 남습니다. 전달받은 실제 토큰 문자열을 블랙리스트에 올려야 합니다.
아래와 같이 원본 토큰 문자열로 AccessToken 객체를 구성해 사용하도록 수정해 주세요.
public void signOut(String token) {
- SiteUser siteUser = authTokenProvider.parseSiteUser(token);
- AccessToken accessToken = authTokenProvider.generateAccessToken(siteUser);
+ SiteUser siteUser = authTokenProvider.parseSiteUser(token);
+ AccessToken accessToken = new AccessToken(
+ authTokenProvider.toSubject(siteUser),
+ siteUser.getRole(),
+ token
+ );
authTokenProvider.deleteRefreshTokenByAccessToken(accessToken);
tokenBlackListService.addToBlacklist(accessToken);
}📝 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.
| public void signOut(String token) { | |
| Subject subject = authTokenProvider.parseSubject(token); | |
| long siteUserId = Long.parseLong(subject.value()); | |
| SiteUser siteUser = siteUserRepository.findById(siteUserId) | |
| .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); | |
| AccessToken accessToken = authTokenProvider.generateAccessToken(subject, siteUser.getRole()); | |
| SiteUser siteUser = authTokenProvider.parseSiteUser(token); | |
| AccessToken accessToken = authTokenProvider.generateAccessToken(siteUser); | |
| authTokenProvider.deleteRefreshTokenByAccessToken(accessToken); | |
| tokenBlackListService.addToBlacklist(accessToken); | |
| } | |
| public void signOut(String token) { | |
| SiteUser siteUser = authTokenProvider.parseSiteUser(token); | |
| AccessToken accessToken = new AccessToken( | |
| authTokenProvider.toSubject(siteUser), | |
| siteUser.getRole(), | |
| token | |
| ); | |
| authTokenProvider.deleteRefreshTokenByAccessToken(accessToken); | |
| tokenBlackListService.addToBlacklist(accessToken); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/auth/service/AuthService.java
around lines 29 to 34, the signOut method wrongly generates a new AccessToken
and blacklists that new token instead of the original token string; remove the
call that generates a new token, construct or wrap an AccessToken using the
original token string passed into signOut, and pass that AccessToken instance
(built from the original token) to
authTokenProvider.deleteRefreshTokenByAccessToken(...) and
tokenBlackListService.addToBlacklist(...).
| public SiteUser parseSiteUser(String token) { | ||
| String subject = tokenProvider.parseSubject(token); | ||
| return new Subject(subject); | ||
| long siteUserId = Long.parseLong(subject); | ||
| return siteUserRepository.findById(siteUserId) | ||
| .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
1) parseSiteUser에서 NumberFormatException을 처리해 500을 방지해 주세요.
비정상 토큰의 subject가 숫자가 아니면 런타임 예외로 500이 날 수 있습니다. 커스텀 예외로 변환해 4xx로 응답하는 게 안전합니다.
다음과 같이 보강을 제안합니다.
public SiteUser parseSiteUser(String token) {
String subject = tokenProvider.parseSubject(token);
- long siteUserId = Long.parseLong(subject);
+ long siteUserId;
+ try {
+ siteUserId = Long.parseLong(subject);
+ } catch (NumberFormatException e) {
+ // 토큰 변조/형식 오류를 서버 에러로 노출하지 않도록 처리
+ throw new CustomException(USER_NOT_FOUND);
+ }
return siteUserRepository.findById(siteUserId)
.orElseThrow(() -> new CustomException(USER_NOT_FOUND));
}원하시면, 이 경로에 대한 단위 테스트(비숫자 subject 케이스) 추가도 도와드리겠습니다.
📝 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.
| public SiteUser parseSiteUser(String token) { | |
| String subject = tokenProvider.parseSubject(token); | |
| return new Subject(subject); | |
| long siteUserId = Long.parseLong(subject); | |
| return siteUserRepository.findById(siteUserId) | |
| .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); | |
| } | |
| public SiteUser parseSiteUser(String token) { | |
| String subject = tokenProvider.parseSubject(token); | |
| long siteUserId; | |
| try { | |
| siteUserId = Long.parseLong(subject); | |
| } catch (NumberFormatException e) { | |
| // 토큰 변조/형식 오류를 서버 에러로 노출하지 않도록 처리 | |
| throw new CustomException(USER_NOT_FOUND); | |
| } | |
| return siteUserRepository.findById(siteUserId) | |
| .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java
around lines 62-67, parseSiteUser currently calls Long.parseLong(subject)
directly which can throw NumberFormatException for non-numeric subjects and
cause a 500; wrap the parse in a try-catch, catch NumberFormatException and
throw an appropriate CustomException (e.g., INVALID_TOKEN or a new
INVALID_SUBJECT) so the error maps to a 4xx response, keeping the repository
lookup and existing USER_NOT_FOUND behavior unchanged.
3 participants
관련 이슈
작업 내용
프론트엔드에서 브라우저의 쿠키에 접근할 수 없게 됨에 따라 이 방법이 불가능해졌습니다.
특이 사항
사실 이 PR에 포함되는 범위는 아니지만...
authTokenProvider의 몇몇 함수를 고쳐 중복 코드를 없앴습니다.. 🙄🙄
어차피 auth 패키지 리팩터링하려 했으니 이정도는 괜찮겠지요? 호호..
리뷰 요구사항 (선택)
커밋 따라가며 읽으시면 쉽습니다!
https://github.com/solid-connection/api-docs/pull/41 와 함께 머지합니다.