[refactor] 리졸버 로직 개선, swagger에서 userId 필수 파라미터 입력인곳 제거 #169
Conversation
WalkthroughCurrentUserArgumentResolver에 JwtTokenProvider를 주입(@requiredargsconstructor)하고 Authentication principal(Long/Integer/숫자 문자열)과 Authorization Bearer 토큰에서 userId를 추출하도록 로직을 확장함. 추출 실패 시 ApplicationException(AuthErrorCase.UNAUTHORIZED)을 던지도록 변경하고, AuthErrorCase enum을 추가했으며 일부 컨트롤러의 @currentuser 파라미터를 OpenAPI에서 숨기도록 @parameter(hidden = true)를 적용함. 또한 addReviewCount의 파라미터 타입 변경에 따라 테스트를 수정함. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant SpringMVC as Spring MVC
participant Resolver as CurrentUserArgumentResolver
participant SecCtx as SecurityContext
participant Request as HttpServletRequest
participant JWT as JwtTokenProvider
Client->>Controller: 요청 (userId 필요)
Controller->>SpringMVC: 핸들러 실행
SpringMVC->>Resolver: @CurrentUser 파라미터 해석 요청
Resolver->>SecCtx: Authentication 조회
alt principal에서 userId 추출 가능
Resolver-->>SpringMVC: userId 반환
else principal에서 추출 불가
Resolver->>Request: Authorization 헤더 조회
alt Bearer 토큰 존재
Resolver->>JWT: getUserId(token)
JWT-->>Resolver: userId
Resolver-->>SpringMVC: userId 반환
else 없음/추출실패
Resolver-->>SpringMVC: ApplicationException(AuthErrorCase.UNAUTHORIZED) 발생
end
end
SpringMVC-->>Controller: userId 인자 주입 또는 예외 전달
Controller-->>Client: 응답 또는 에러
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
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: 0
🧹 Nitpick comments (3)
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java (3)
36-47: principal 파싱 분기 강화 좋음. 숫자 판별 유틸로 추출 고려Long/Integer/숫자 문자열까지 폭넓게 처리한 점 좋습니다. 유지보수를 위해 숫자 판별 로직(s.chars().allMatch(...), name.chars().allMatch(...))을 private 헬퍼 메서드로 추출하면 중복이 줄고 가독성이 좋아집니다. 필요하다면 선/후행 공백 제거(trim)도 함께 고려해 주세요.
51-55: Authorization Bearer 파싱을 대소문자/공백 허용 방식으로 견고화헤더 스킴은 관례상 대소문자를 구분하지 않습니다. 또한 공백이 여러 개이거나 탭이 포함될 수 있으니 split 기반 파싱으로 더 견고하게 처리하는 것을 권장합니다. 토큰 파싱 실패나 Provider 예외도 안전하게 흘려보내고 최종 예외로 수렴시키면 500을 방지할 수 있습니다.
아래 변경을 제안합니다:
- if (StringUtils.hasText(authz) && authz.startsWith("Bearer ")) { - String token = authz.substring(7); - Long userId = jwtTokenProvider.getUserId(token); - if (userId != null) { return userId; } - } + if (StringUtils.hasText(authz)) { + String[] parts = authz.split("\\s+"); + if (parts.length >= 2 && "Bearer".equalsIgnoreCase(parts[0])) { + String token = parts[1].trim(); + try { + Long userId = jwtTokenProvider.getUserId(token); + if (userId != null) { return userId; } + } catch (Exception ignored) { + // invalid/expired token 등은 아래 공통 예외로 수렴 + } + } + }
57-57: 401 상태로 매핑되도록 예외 타입 조정 권장리졸버 단계에서 IllegalStateException을 던지면 500으로 귀결될 수 있습니다. 인증 누락/파싱 실패는 401(Unauthorized)이 적절하므로 ResponseStatusException으로 전환하는 것을 권장합니다.
- throw new IllegalStateException("인증 정보가 없거나 userId를 추출할 수 없습니다."); + throw new ResponseStatusException(HttpStatus.UNAUTHORIZED, "인증 정보가 없거나 userId를 추출할 수 없습니다.");해당 변경을 위해 다음 import가 필요합니다(범위 밖 참고용):
import org.springframework.http.HttpStatus; import org.springframework.web.server.ResponseStatusException;
📜 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 (4)
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java(1 hunks)src/main/java/com/wayble/server/review/controller/ReviewController.java(2 hunks)src/main/java/com/wayble/server/user/controller/UserPlaceController.java(5 hunks)src/test/java/com/wayble/server/review/service/ReviewServiceTest.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (4)
src/main/java/com/wayble/server/user/service/UserPlaceService.java (2)
Service(27-137)Transactional(36-72)src/main/java/com/wayble/server/user/entity/UserPlaceWaybleZoneMapping.java (1)
Entity(10-29)src/main/java/com/wayble/server/user/entity/UserPlace.java (1)
Entity(13-47)src/main/java/com/wayble/server/user/dto/UserPlaceRemoveRequestDto.java (1)
UserPlaceRemoveRequestDto(5-8)
src/test/java/com/wayble/server/review/service/ReviewServiceTest.java (2)
src/main/java/com/wayble/server/wayblezone/entity/WaybleZone.java (2)
addReviewCount(96-99)addLikes(101-105)src/main/java/com/wayble/server/review/service/ReviewService.java (1)
Service(23-81)
src/main/java/com/wayble/server/review/controller/ReviewController.java (2)
src/main/java/com/wayble/server/review/service/ReviewService.java (1)
Service(23-81)src/main/java/com/wayble/server/review/dto/ReviewRegisterDto.java (1)
Schema(8-31)
🔇 Additional comments (4)
src/test/java/com/wayble/server/review/service/ReviewServiceTest.java (1)
69-69: addReviewCount 인자 타입 변경 반영 OK. 엔티티 시그니처와 일관성만 재확인 권장테스트에서 int 리터럴(1) 사용은 long 파라미터에도 자동 승격되어 문제 없습니다. 다만 현재 컨텍스트(제공 스니펫)에서는 WaybleZone.addReviewCount가 long 시그니처로 보입니다. PR 목적대로 int로 변경했다면 엔티티 시그니처가 실제로 반영됐는지 확인해 주세요. 그대로 long 유지라면 현 테스트도 정상 동작합니다.
src/main/java/com/wayble/server/review/controller/ReviewController.java (1)
16-16: Swagger 문서에서 인증 사용자 파라미터 숨김 처리 적절@parameter(hidden = true)로 @currentuser 파라미터를 문서에서 노출하지 않도록 한 점 좋습니다. 런타임에는 그대로 리졸버가 주입하고, 문서 혼란을 줄일 수 있습니다.
Also applies to: 40-41
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java (1)
17-21: JwtTokenProvider 주입 방식 전환(생성자 주입) 👍@requiredargsconstructor + final 필드로 DI 전환 깔끔합니다. 테스트 용이성과 불변성 측면에서 바람직합니다.
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (1)
12-12: Swagger에서 @currentuser 숨김 처리 일관 적용 👍4개 엔드포인트에 동일하게 @parameter(hidden = true)를 적용해 문서 노이즈를 줄였습니다. 실제 주입은 리졸버가 처리하므로 런타임 동작에 영향 없이 API 사용성이 개선됩니다.
Also applies to: 38-39, 53-54, 70-71, 90-91
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/main/java/com/wayble/server/auth/exception/AuthErrorCase.java (1)
10-10: 에러코드 중복 확인 및 도메인별 범위 분리 권장아래 에러코드들이 여러 도메인에서 중복 정의되어 있어, 로그/모니터링·문서화 시 혼선을 줄 수 있습니다:
- 1001
- AuthErrorCase.UNAUTHORIZED @ src/main/java/com/wayble/server/auth/exception/AuthErrorCase.java:10
- MainErrorCase.MAIN_TEST_ERROR @ src/main/java/com/wayble/server/common/MainErrorCase.java:11
- UserErrorCase.USER_NOT_FOUND @ src/main/java/com/wayble/server/user/exception/UserErrorCase.java:11
- 4002
- DirectionErrorCase.ES_INDEXING_FAILED @ src/main/java/com/wayble/server/direction/exception/DirectionErrorCase.java:12
- DirectionErrorCase.DISTANCE_TOO_FAR @ src/main/java/com/wayble/server/direction/exception/DirectionErrorCase.java:14
- 8001
- AwsErrorCase.IMAGE_UPLOAD_ERROR @ src/main/java/com/wayble/server/aws/AwsErrorCase.java:11
- WalkingErrorCase.T_MAP_API_FAILED @ src/main/java/com/wayble/server/direction/exception/WalkingErrorCase.java:11
- 8002
- AwsErrorCase.IMAGE_DELETE_ERROR @ src/main/java/com/wayble/server/aws/AwsErrorCase.java:12
- WalkingErrorCase.GRAPH_FILE_NOT_FOUND @ src/main/java/com/wayble/server/direction/exception/WalkingErrorCase.java:12
→ Auth 도메인 전용 구간(예: 2000~2099)처럼 각 도메인별 errorCode 범위를 할당하고, 중복된 상수 값을 재조정하시길 권장드립니다.
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java (5)
25-28: @currentuser 파라미터에서 primitive long도 지원하세요현재 Long 래퍼만 허용합니다. 컨트롤러 시그니처에서 long을 사용할 가능성을 고려해 둘 다 지원하는 편이 안전합니다.
다음 변경을 제안합니다:
- return parameter.hasParameterAnnotation(CurrentUser.class) - && Long.class.equals(parameter.getParameterType()); + return parameter.hasParameterAnnotation(CurrentUser.class) + && (Long.class.equals(parameter.getParameterType()) || long.class.equals(parameter.getParameterType()));
42-44: 숫자 문자열 판별 방식 미세 개선(국제 숫자 문자 대응 이슈 회피)Character.isDigit는 비 ASCII 숫자 문자(예: 아라비아-인디언 숫자)를 true로 판단할 수 있어 parseLong에서 예외가 날 수 있습니다. 단순히 0-9만 허용하려면 정규식 사용이 안전합니다.
- if (principal instanceof String s && s.chars().allMatch(Character::isDigit)) { + if (principal instanceof String s && s.matches("\\d+")) { return Long.parseLong(s); }
46-48: auth.getName() 숫자 문자열 체크도 동일 기준으로 통일위와 같은 이유로 정규식 사용을 권장합니다.
- if (name != null && name.chars().allMatch(Character::isDigit)) { + if (name != null && name.matches("\\d+")) { return Long.parseLong(name); }
53-54: Authorization 스킴 대소문자 허용(옵션)일부 클라이언트/프록시에서 “bearer” 소문자 등으로 전달하는 경우가 있어 스킴 비교를 case-insensitive로 처리하는 걸 고려해볼 수 있습니다. 기능 영향은 작지만 실전 호환성이 좋아집니다.
- if (StringUtils.hasText(authz) && authz.startsWith("Bearer ")) { - String token = authz.substring(7); + if (StringUtils.hasText(authz) && authz.regionMatches(true, 0, "Bearer ", 0, 7)) { + String token = authz.substring(7);
31-60: 리졸버 경로별 동작 보장을 위한 테스트 추가 제안다양한 principal/헤더 조합에서 기대 동작을 보장하는 단위/통합 테스트를 권장합니다.
권장 케이스:
- principal이 Long/Integer/String("123")/그 외(UserDetails 등)일 때 각각 userId 추출
- principal 비정상 + Authorization Bearer 토큰 유효 → 토큰에서 userId 추출
- Authorization 헤더 없음/공백/스킴 오탈자 → UNAUTHORIZED
- 토큰 유효하지 않음(getUserId 예외) → UNAUTHORIZED로 매핑됨(본 PR 제안 반영 시)
원하시면 MockMvc + SecurityContext mocking, 또는 NativeWebRequest 모킹 기반의 단위 테스트 템플릿을 제공해드리겠습니다.
📜 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 (2)
src/main/java/com/wayble/server/auth/exception/AuthErrorCase.java(1 hunks)src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java (3)
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthenticationFilter.java (2)
RequiredArgsConstructor(13-43)Override(18-42)src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthentication.java (3)
Override(18-18)JwtAuthentication(7-19)JwtAuthentication(11-15)src/main/java/com/wayble/server/common/config/security/jwt/JwtTokenProvider.java (1)
getUserId(53-64)
src/main/java/com/wayble/server/auth/exception/AuthErrorCase.java (3)
src/main/java/com/wayble/server/user/exception/UserErrorCase.java (1)
Getter(7-30)src/main/java/com/wayble/server/common/MainErrorCase.java (1)
Getter(7-16)src/main/java/com/wayble/server/common/exception/ErrorCase.java (1)
ErrorCase(3-10)
🔇 Additional comments (2)
src/main/java/com/wayble/server/auth/exception/AuthErrorCase.java (1)
7-15: AuthErrorCase 도입 및 기존 패턴과의 일관성 유지 — 좋습니다
- ErrorCase 인터페이스 구현, @Getter/@requiredargsconstructor 사용, 필드 가시성/타입 모두 일관적입니다.
- HTTP 상태코드/메시지 표현도 도메인에 적합해 보입니다.
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java (1)
19-23: 생성자 주입(@requiredargsconstructor)으로 JwtTokenProvider DI — 방향성 좋습니다컴포넌트 스캔 및 빈 생명주기와도 자연스럽게 맞고, 테스트 작성 시에도 주입이 용이합니다.
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java (1)
55-60: 이전 피드백 반영 완료 — 토큰 파싱 실패 시 UNAUTHORIZED로 정상 매핑됩니다getUserId(token)에서 발생 가능한 IllegalArgumentException을 401(ApplicationException(AuthErrorCase.UNAUTHORIZED))로 매핑하도록 수정된 점 확인했습니다. 일관된 에러 응답 보장에 도움이 됩니다.
🧹 Nitpick comments (3)
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java (3)
40-47: 숫자 문자열 판별 방식으로 인한 빈 문자열/비-ASCII 숫자 처리 취약점 (NFE 가능성)Character.isDigit 기반 판별은
- 빈 문자열에서 “모두 참”이 되어 parseLong에서 NumberFormatException 발생,
- 유니코드 숫자(예: ١٢٣)를 true로 보고 parseLong에서 실패
할 수 있습니다. ASCII 숫자만, 1글자 이상인 경우만 통과하도록 바꾸면 안전합니다.다음 최소 패치 제안:
- if (principal instanceof String s && s.chars().allMatch(Character::isDigit)) { - return Long.parseLong(s); - } + if (principal instanceof String s && s.matches("\\d+")) { + return Long.parseLong(s); + } @@ - if (name != null && name.chars().allMatch(Character::isDigit)) { - return Long.parseLong(name); - } + if (StringUtils.hasText(name) && name.matches("\\d+")) { + return Long.parseLong(name); + }참고:
- 매우 긴 숫자( long 범위 초과 )도 matches("\d+")를 통과하므로, 극단값 입력까지 방어하려면 parseLong을 try-catch(NumberFormatException)로 감싸 401로 매핑하는 것도 고려해 보세요.
53-55: Authorization 헤더 처리: 소소한 내구성 개선 제안 (대소문자 무시, 토큰 트리밍)표준은 “Bearer”지만, 일부 클라이언트가 소문자 “bearer”를 보내는 경우도 있어 케이스 무시가 실전 내구성을 높입니다. 토큰 양끝 공백도 트리밍하면 좋습니다.
아래처럼 미세 조정 가능합니다:
- if (StringUtils.hasText(authz) && authz.startsWith("Bearer ")) { - String token = authz.substring(7); + if (StringUtils.hasText(authz) && authz.regionMatches(true, 0, "Bearer ", 0, 7)) { + String token = authz.substring(7).trim();
51-61: (선택) 예외 주도 흐름 최소화: validateToken 선행 검증 고려현재도 충분히 안전하지만, provider에 validateToken이 있다면 먼저 검증 후 getUserId를 호출하면 예외 기반 흐름을 줄일 수 있습니다.
예시 패치(택1, 참고용):
- try { - Long userId = jwtTokenProvider.getUserId(token); - if (userId != null) { return userId; } - } catch (IllegalArgumentException e) { - throw new ApplicationException(AuthErrorCase.UNAUTHORIZED); - } + if (jwtTokenProvider.validateToken(token)) { + Long userId = jwtTokenProvider.getUserId(token); + if (userId != null) { return userId; } + } else { + throw new ApplicationException(AuthErrorCase.UNAUTHORIZED); + }
📜 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 (1)
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java (3)
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthenticationFilter.java (2)
RequiredArgsConstructor(13-43)Override(18-42)src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthentication.java (3)
Override(18-18)JwtAuthentication(7-19)JwtAuthentication(11-15)src/main/java/com/wayble/server/common/config/security/jwt/JwtTokenProvider.java (1)
getUserId(53-64)
🔇 Additional comments (3)
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java (3)
19-23: JwtTokenProvider 생성자 주입 도입 굿 — 테스트 용이성과 불변성 모두 개선됩니다@requiredargsconstructor + final 필드로 DI 전환한 점 👍. 순환참조 위험도 낮고, 단위테스트 작성도 수월해집니다.
63-64: 추출 실패 시 일관되게 401 매핑: 합리적 기본값모든 경로에서 최종적으로 UNAUTHORIZED로 정규화되는 흐름이 명확합니다. 토큰/컨텍스트가 모두 부재하거나 파싱 불가한 케이스에 대한 UX 일관성이 좋아집니다.
25-28: Primitive long 사용처 없음 확인됨레포 전반에서
@CurrentUser는 모두Long타입으로만 사용되고 있어, primitivelong미지원에 따른 동작 불일치 이슈는 존재하지 않습니다.
현 구현대로 유지하셔도 무방합니다.
✔️ 연관 이슈
📝 작업 내용
스크린샷 (선택)
Summary by CodeRabbit