-
Notifications
You must be signed in to change notification settings - Fork 2
[BE] SISC1-61 [FEAT] api url 및 오류 수정 #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The head ref may contain hidden characters: "SISC1-61-api-url-\uC218\uC815"
Conversation
WalkthroughOAuth 및 사용자 인증 엔드포인트를 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant JwtAuthenticationFilter
participant JwtParser
participant CustomUserDetailsService
participant UserRepository
participant SecurityContext
Client->>JwtAuthenticationFilter: HTTP Request + Authorization: Bearer <token>
JwtAuthenticationFilter->>JwtAuthenticationFilter: shouldNotFilter(path) 검사
alt 제외 경로
JwtAuthenticationFilter-->>Client: 필터 스킵 (로그)
else 보호된 경로
JwtAuthenticationFilter->>JwtParser: validationToken(token)
JwtParser->>JwtParser: 토큰 파싱, uid(claim) 추출
alt 토큰 유효
JwtParser->>CustomUserDetailsService: loadUserByUsername(userId)
CustomUserDetailsService->>UserRepository: findById(UUID)
UserRepository-->>CustomUserDetailsService: User 반환
CustomUserDetailsService-->>JwtParser: UserDetails 반환
JwtParser-->>JwtAuthenticationFilter: Authentication 생성
JwtAuthenticationFilter->>SecurityContext: 인증 저장
else 무효/누락
JwtAuthenticationFilter-->>Client: 경고 로그 후 계속
end
end
sequenceDiagram
participant Client
participant OauthLoginController
participant LoginServiceImpl
participant JwtProvider
participant RefreshTokenRepository
Client->>OauthLoginController: POST /api/auth/signup (SignupRequest)
OauthLoginController->>LoginServiceImpl: signUp(dto)
LoginServiceImpl->>LoginServiceImpl: 사용자 생성 및 검증
LoginServiceImpl->>JwtProvider: createToken(userId, role, email)
JwtProvider-->>LoginServiceImpl: accessToken (sub=email, uid=userId)
LoginServiceImpl->>JwtProvider: createRefreshToken()
JwtProvider-->>LoginServiceImpl: refreshToken
LoginServiceImpl->>RefreshTokenRepository: findByUserId(userId)
alt 기존 토큰 존재
RefreshTokenRepository-->>LoginServiceImpl: 기존 토큰
LoginServiceImpl->>RefreshTokenRepository: delete(existing)
end
LoginServiceImpl->>RefreshTokenRepository: save(new RefreshToken(userId, token))
LoginServiceImpl-->>OauthLoginController: SignupResponse (201 Created)
OauthLoginController-->>Client: HTTP 201 + body
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60분
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtProvider.java (1)
66-75: 메서드명/동작 불일치: getUserIdFromToken이 이메일(subject)을 반환합니다.이 메서드는 이제 사용자 이메일을 반환합니다. 이름상 userId로 오인될 여지가 큽니다. 실제 userId 파싱은
JwtParser.getUserIdFromToken(UUID)가 담당합니다. 혼동을 줄이기 위해 아래 중 하나로 조치해 주세요.
- 메서드명을
getSubjectFromToken또는getEmailFromToken으로 변경(+ 기존 메서드 @deprecated).- 혹은 이 메서드를 제거하고 파싱은 전부
JwtParser로 일원화.빠른 완화를 위한 제안(diff: Deprecated 표시):
- // 토큰에서 사용자 ID 추출 - public String getUserIdFromToken(String token) { + // Deprecated: subject(이메일)를 반환합니다. 사용자 ID는 JwtParser.getUserIdFromToken을 사용하세요. + @Deprecated(since = "SISC1-61", forRemoval = true) + public String getUserIdFromToken(String token) { Claims claims = Jwts.parserBuilder() .setSigningKey(secretKey) .build() .parseClaimsJws(token) .getBody(); return claims.getSubject(); }backend/src/main/java/org/sejongisc/backend/auth/controller/OauthLoginController.java (2)
112-114: 로그에 OAuth state/전체 URL 노출 최소화.state는 CSRF 방어 토큰 성격으로, 전체 URL을 로그로 남기면 불필요한 노출이 됩니다. state 마스킹 또는 호스트만 로깅을 권장합니다.
- log.debug("Generated OAuth URL for {}: {}", provider, authUrl); + log.debug("Generated OAuth URL for {} (state masked): {}", + provider, authUrl.replaceFirst("([?&])state=[^&]+", "$1state=***"));
71-84: 보안 권고: 리프레시 토큰은 응답 바디에서 제거하고 HttpOnly 쿠키만 사용하세요.현재 비(소셜) 로그인은 바디에
refreshToken을 포함하고, 소셜 로그인은 쿠키만 설정합니다. 정책을 통일해 바디에서 리프레시 토큰을 제거하는 편이 안전합니다. 서버 응답 DTO에서 해당 필드를 제외하거나, 컨트롤러에서 마스킹하도록 변경을 제안합니다.backend/src/test/java/org/sejongisc/backend/auth/controller/OauthLoginControllerTest.java (1)
163-176: 컴파일 오류 및 HTTP 상태 코드 오류:jwtProvider.createToken()스텁 4개 위치와 유효성 검증 에러 핸들링다음 2가지 문제를 해결해야 합니다:
OauthLoginControllerTest.java의 3개 위치와 LoginServiceImplTest.java의 1개 위치에서
createToken()스텁이 2개 인자만 전달합니다. 3개 인자(email 포함)로 수정하세요:
- Line 174:
when(jwtProvider.createToken(user.getUserId(), user.getRole()))→ 3번째 인자user.getEmail()추가- Line 336:
when(jwtProvider.createToken(any(), any()))→ 3번째 인자any(String.class)추가- Line 359:
when(jwtProvider.createToken(any(), any()))→ 3번째 인자any(String.class)추가- LoginServiceImplTest.java Line 79:
given(jwtProvider.createToken(any(UUID.class), any(Role.class)))→ 3번째 인자any(String.class)추가OauthLoginControllerTest.java Line 314-318: HTTP 상태 코드를 500에서 400으로 수정하세요
MethodArgumentNotValidException은 유효성 검증 오류이므로 HTTP 400(BAD_REQUEST)이어야 하는데, 현재 500으로 설정되어 있습니다.- GlobalExceptionHandler에
MethodArgumentNotValidException명시적 핸들러가 없어 제너릭 Exception 핸들러로 폴백되어 500을 반환합니다..andExpect(status().isInternalServerError())→.andExpect(status().isBadRequest())로 변경하세요.
🧹 Nitpick comments (11)
backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/CustomUserDetails.java (1)
41-44: Lombok @Getter와 중복된 명시적 구현
@Getter어노테이션이 이미 모든 필드에 대한 getter를 생성하므로,getPassword()메서드를 명시적으로 구현할 필요는 없습니다. 다만UserDetails인터페이스 계약을 명시적으로 표현하기 위한 의도라면 문제없습니다.만약 코드를 간소화하려면 이 메서드를 제거할 수 있습니다:
- @Override - public String getPassword() { - return password; - } -backend/src/main/java/org/sejongisc/backend/auth/service/LoginServiceImpl.java (1)
49-60: Refresh 토큰 저장 로직 개선 권장현재 구현은 정상 작동하지만, 더 효율적인 방식으로 개선할 수 있습니다.
현재는 조회 → 삭제 → 저장의 3단계를 거치지만, JPA의
save()메서드는 이미 존재하는 엔티티를 업데이트하므로 다음과 같이 간소화할 수 있습니다:옵션 1: 단순 삭제 후 저장 (현재 방식 개선)
- // 기존 토큰 삭제 후 새로 저장 - refreshTokenRepository.findByUserId(user.getUserId()) - .ifPresent(refreshTokenRepository::delete); + // 기존 토큰이 있으면 삭제 + refreshTokenRepository.deleteByUserId(user.getUserId()); refreshTokenRepository.save( org.sejongisc.backend.auth.entity.RefreshToken.builder() .userId(user.getUserId()) .token(refreshToken) .build() );옵션 2: Upsert 패턴 (더 효율적)
- // 기존 토큰 삭제 후 새로 저장 - refreshTokenRepository.findByUserId(user.getUserId()) - .ifPresent(refreshTokenRepository::delete); - - refreshTokenRepository.save( - org.sejongisc.backend.auth.entity.RefreshToken.builder() - .userId(user.getUserId()) - .token(refreshToken) - .build() - ); + // 기존 토큰 조회 후 업데이트 또는 새로 생성 + RefreshToken refreshTokenEntity = refreshTokenRepository.findByUserId(user.getUserId()) + .orElse(RefreshToken.builder().userId(user.getUserId()).build()); + refreshTokenEntity.setToken(refreshToken); + refreshTokenRepository.save(refreshTokenEntity);옵션 1이 더 간단하고 명확하지만, 옵션 2가 데이터베이스 작업을 줄여 성능이 약간 더 좋습니다.
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtProvider.java (1)
90-97: 역할 중복: validationToken은 JwtParser에 이미 존재합니다.토큰 파싱/검증은
JwtParser로 단일화하는 편이 안전합니다. 본 메서드는 Deprecated 처리하거나 내부적으로JwtParser를 위임하도록 정리해 주세요. 장기적으로는JwtProvider는 “발급 전용”이 명확합니다.backend/src/main/java/org/sejongisc/backend/common/auth/config/SecurityConfig.java (2)
55-57: 불필요한 null 체크 제거 제안.
@RequiredArgsConstructor주입 실패 시 애플리케이션이 기동되지 않으므로,if (jwtAuthenticationFilter != null)가드가 실효성이 낮습니다. 간결히 제거하세요.- if(jwtAuthenticationFilter != null) { - http.addFilterBefore(jwtAuthenticationFilter, UsernamePasswordAuthenticationFilter.class); - } + http.addFilterBefore(jwtAuthenticationFilter, UsernamePasswordAuthenticationFilter.class);
64-76: CORS 허용 도메인 프로퍼티화 권장.현재
http://localhost:5173만 허용합니다. 배포 환경 도메인을application-*.yml로 분리(예:cors.allowed-origins)해 환경별로 주입하도록 하면 운영 전환 시 안전합니다.backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/JwtAuthenticationFilter.java (2)
54-66: 중복/사실상 데드 로직: doFilterInternal 내shouldNotFilter재호출.
OncePerRequestFilter는 이미 외부에서shouldNotFilter를 평가합니다. 본 블록은 실행되지 않으므로 제거하세요.- // 인증 제외 경로 - if (shouldNotFilter(request)) { - filterChain.doFilter(request, response); - return; - }
101-108: 로그 레벨 완화 제안.모든 요청마다
info는 소음이 큽니다.debug로 낮추는 편이 좋습니다.- log.info("JwtFilter check path: {} → excluded={}", path, excluded); + log.debug("JwtFilter check path: {} → excluded={}", path, excluded);또한 EXCLUDE 목록은
SecurityConfig의 permitAll과 중복되므로 한 곳으로 통합하는 방안을 고려해 주세요.backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (4)
37-47: 검증 로그 레벨/스택트레이스 개선유효 토큰마다 INFO 로그는 노이즈가 큽니다. 실패 로그는 메시지만 남겨 원인 추적이 어렵습니다. 성공은 DEBUG, 실패는 WARN/ERROR + 예외 객체를 함께 기록하세요.
- log.info("Token validation success"); + log.debug("Token validation success"); @@ - } catch (JwtException | IllegalArgumentException e) { - log.error("Token validation failed: {}", e.getMessage()); + } catch (JwtException | IllegalArgumentException e) { + log.warn("Token validation failed: {}", e.getMessage(), e)
65-76: parseClaims가 만료 토큰도 허용함 — 계약 명시 필요
ExpiredJwtException에서 클레임을 그대로 반환하여 만료 토큰도 소비됩니다. 의도라면 메서드명을parseClaimsAllowExpired로 바꾸거나 Javadoc/주석으로 계약을 명시하세요. 반대로 인증 경로에서 오용될 위험이 있다면getAuthentication내부에서 만료 여부를 재검증/차단하세요.
31-35: secret 키 형식/길이 검증 추가 권장 (HS256 최소 256비트)Base64 디코딩 실패나 키 길이 부족 시 조기 실패하도록 가드하세요. 운영 환경에서 미묘한 설정 오류를 빠르게 드러낼 수 있습니다.
- public void init() { - byte[] keyBytes = Base64.getDecoder().decode(rawSecretKey); - this.secretKey = Keys.hmacShaKeyFor(keyBytes); - } + public void init() { + byte[] keyBytes; + try { + keyBytes = Base64.getDecoder().decode(rawSecretKey); + } catch (IllegalArgumentException e) { + throw new IllegalStateException("jwt.secret가 Base64 형식이 아닙니다.", e); + } + if (keyBytes.length < 32) { // 256비트 + throw new IllegalStateException("jwt.secret 길이가 HS256 최소 256비트(32바이트) 미만입니다."); + } + this.secretKey = Keys.hmacShaKeyFor(keyBytes); + }
3-3: 와일드카드 임포트 정리(선택 사항)
io.jsonwebtoken.*,java.util.*대신 필요한 타입만 명시 임포트하면 가독성과 IDE 자동 정리에 유리합니다.Also applies to: 17-17
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
backend/src/main/java/org/sejongisc/backend/auth/controller/OauthLoginController.java(6 hunks)backend/src/main/java/org/sejongisc/backend/auth/entity/RefreshToken.java(1 hunks)backend/src/main/java/org/sejongisc/backend/auth/repository/RefreshTokenRepository.java(1 hunks)backend/src/main/java/org/sejongisc/backend/auth/service/LoginServiceImpl.java(1 hunks)backend/src/main/java/org/sejongisc/backend/common/auth/config/SecurityConfig.java(1 hunks)backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java(1 hunks)backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtProvider.java(2 hunks)backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/CustomUserDetails.java(1 hunks)backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/CustomUserDetailsService.java(2 hunks)backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/JwtAuthenticationFilter.java(2 hunks)backend/src/main/java/org/sejongisc/backend/user/controller/UserController.java(1 hunks)backend/src/main/java/org/sejongisc/backend/user/service/UserServiceImpl.java(1 hunks)backend/src/test/java/org/sejongisc/backend/auth/controller/OauthLoginControllerTest.java(6 hunks)backend/src/test/java/org/sejongisc/backend/user/controller/UserControllerTest.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
backend/src/main/java/org/sejongisc/backend/auth/entity/RefreshToken.java (1)
backend/src/main/java/org/sejongisc/backend/template/dto/TemplateResponse.java (1)
Builder(10-15)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (3)
backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/CustomUserDetailsService.java (1)
RequiredArgsConstructor(16-38)backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/JwtAuthenticationFilter.java (1)
Slf4j(30-124)backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtProvider.java (1)
Component(18-99)
backend/src/main/java/org/sejongisc/backend/user/controller/UserController.java (2)
backend/src/main/java/org/sejongisc/backend/auth/controller/OauthLoginController.java (1)
Slf4j(25-224)backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/JwtAuthenticationFilter.java (1)
Slf4j(30-124)
backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/JwtAuthenticationFilter.java (5)
backend/src/main/java/org/sejongisc/backend/auth/controller/OauthLoginController.java (1)
Slf4j(25-224)backend/src/main/java/org/sejongisc/backend/auth/service/LoginServiceImpl.java (1)
Slf4j(25-80)backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (1)
Slf4j(19-87)backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtProvider.java (1)
Component(18-99)backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/CustomUserDetailsService.java (1)
RequiredArgsConstructor(16-38)
🔇 Additional comments (17)
backend/src/main/java/org/sejongisc/backend/user/controller/UserController.java (2)
22-22: LGTM! API 경로 표준화
/api/user로 경로를 변경하여 RESTful API 규칙을 따르고 있습니다.
38-38: LGTM! 로깅 개선파라미터화된 로깅 형식을 사용하여 성능과 가독성이 향상되었습니다.
backend/src/test/java/org/sejongisc/backend/user/controller/UserControllerTest.java (1)
66-94: LGTM! 테스트가 API 변경사항을 올바르게 반영테스트가 새로운 API 경로(
/api/user/details)를 사용하도록 업데이트되었고,phoneNumber와point필드에 대한 검증이 추가되어 테스트 커버리지가 향상되었습니다.backend/src/main/java/org/sejongisc/backend/auth/entity/RefreshToken.java (2)
13-13: LGTM! Builder 패턴 추가
@Builder어노테이션 추가로 객체 생성이 더 명확하고 안전해졌습니다.
18-18: LGTM! 필드명 표준화
userid에서userId로 변경하여 Java 네이밍 컨벤션(camelCase)을 준수합니다. JPA 컬럼 매핑은user_id로 유지되어 데이터베이스 스키마에 영향이 없습니다.backend/src/main/java/org/sejongisc/backend/auth/repository/RefreshTokenRepository.java (1)
11-11: LGTM! 메서드명 일관성 개선엔티티 필드명 변경(
userId)에 맞춰 메서드명을findByUserId로 변경하여 Spring Data JPA 네이밍 규칙을 준수합니다.backend/src/main/java/org/sejongisc/backend/auth/service/LoginServiceImpl.java (1)
46-46: LGTM! 토큰 생성에 이메일 포함JWT 토큰에 이메일을 포함하도록 업데이트되었습니다. 다른 인증 컴포넌트들과 일관성이 유지됩니다.
backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/CustomUserDetailsService.java (1)
23-32: UUID 기반 사용자 조회로 변경됨메서드가 이메일 대신 UUID로 사용자를 조회하도록 변경되었습니다. UUID 파싱 예외 처리가 적절하게 구현되어 있습니다.
참고:
loadUserByUsername메서드명이 실제 동작(userId로 조회)과 일치하지 않지만, 이는 Spring Security의UserDetailsService인터페이스 제약사항입니다. 코드 주석을 추가하여 이를 명확히 하는 것을 고려해보세요:@Override + // Note: Despite the method name, this loads user by userId (UUID) not username public UserDetails loadUserByUsername(String userId) throws UsernameNotFoundException {backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtProvider.java (1)
38-45: AccessToken subject/email 전환 자체는 OK, uid 클레임 추가도 합리적입니다.권한 검증·로딩은
uid를 사용하고, subject에는 이메일을 둔 설계는 일관됩니다. 다만 아래 파싱 헬퍼와의 정합성 점검이 필요합니다.backend/src/main/java/org/sejongisc/backend/common/auth/config/SecurityConfig.java (1)
39-46: 경로 재정렬/접두사(/api/auth) 전환은 적절합니다.Swagger, OAuth, 로그인/회원가입 경로의 permitAll 구성이 일관되며 필터 제외 패턴과도 맞습니다.
중복 드리프트 방지를 위해 JwtAuthenticationFilter의 EXCLUDE_PATTERNS와 이 목록을 한 곳(상수/설정 프로퍼티)으로 모으는 것을 권장합니다.
backend/src/test/java/org/sejongisc/backend/auth/controller/OauthLoginControllerTest.java (1)
81-90: 엔드포인트 경로 /api/auth 전환 및 예외 케이스 커버리지는 좋습니다.MockMvc 설정(Validator, ArgumentResolver, JSR310 모듈)과 주요 성공/실패 시나리오가 잘 구성되어 있습니다. 위 컴파일 이슈와 상태코드만 정리하면 테스트 신뢰도가 높아집니다.
Also applies to: 94-121, 122-150, 233-244, 246-266, 268-299, 321-343, 345-367, 368-377
backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/JwtAuthenticationFilter.java (1)
75-81: 유효하지 않은/부재 토큰 시 경고 로그 후 체인 지속은 합리적입니다.보호 리소스는 SecurityConfig에서 차단되므로 흐름이 일관됩니다.
backend/src/main/java/org/sejongisc/backend/auth/controller/OauthLoginController.java (4)
27-28: API 베이스 경로/api/auth전환 적절합니다.다른 구성(Swagger, SecurityConfig, 필터)와 일관됩니다.
58-64: 회원가입 201 처리 OK.생성 성공 시 201 + DTO 반환은 REST 규약과 부합합니다.
157-159: OAuth 로그인에서 이메일 포함 액세스 토큰 발급 및 쿠키 세팅 흐름은 적절합니다.
uid기반 인증과 HttpOnly SameSite=None 설정이 일관적입니다.Also applies to: 163-170, 171-186
188-221: 로그아웃 멱등 처리와 쿠키 제거 OK.잘못된/만료 토큰에서도 200을 유지하고 refresh 쿠키를 삭제합니다. 운영 로깅 수준만 확인해 주세요.
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (1)
19-25: DI 전환과 로깅 어노테이션 적용은 적절합니다
@RequiredArgsConstructor로 서비스 의존성 명시,@Slf4j추가 모두 적절합니다. 생성 시점 키 초기화와 함께 컴포넌트 생명주기에 맞습니다.
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/user/controller/UserController.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/user/service/UserServiceImpl.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/org/sejongisc/backend/auth/controller/OauthLoginControllerTest.java
Show resolved
Hide resolved
backend/src/test/java/org/sejongisc/backend/auth/controller/OauthLoginControllerTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (1)
49-52: getAuthentication가 만료 토큰을 수용할 수 있는 경로 차단(엄격 파싱으로 변경)parseClaims는 만료 토큰에서 Claims를 반환합니다. 현재는 필터에서 사전 검증되지만, 다른 호출 경로에서 오용 시 인증이 생성될 수 있습니다. getAuthentication에서는 만료 허용 없이 엄격 파싱을 사용하세요. 이전 리뷰와 동일 맥락입니다.
- public UsernamePasswordAuthenticationToken getAuthentication(String token) { - Claims claims = parseClaims(token); + public UsernamePasswordAuthenticationToken getAuthentication(String token) { + Claims claims = parseClaimsStrict(token); // 만료 허용 안 함 String userId = claims.get("uid", String.class); ... } @@ - // Claims 파싱 + // Claims 파싱(만료 허용) private Claims parseClaims(String token) { try { return Jwts.parserBuilder() .setSigningKey(secretKey) .build() .parseClaimsJws(token) .getBody(); } catch (ExpiredJwtException e) { return e.getClaims(); } } + + // Claims 파싱(만료 불허) — 인증 생성 시 사용 + private Claims parseClaimsStrict(String token) { + return Jwts.parserBuilder() + .setSigningKey(secretKey) + .build() + .parseClaimsJws(token) // ExpiredJwtException 발생 시 그대로 전파 + .getBody(); + }다른 경로에서
getAuthentication호출이 없는지 재확인해 주세요:#!/bin/bash rg -nP -C2 '\bgetAuthentication\s*\(' --type=javaAlso applies to: 77-88
🧹 Nitpick comments (5)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (2)
37-47: 토큰 검증 로그 레벨 하향(과다 로그 방지) 및 예외 로깅 톤 조정요청마다 INFO 로그는 과합니다. 성공은 DEBUG, 실패는 WARN(스택은 필요 시에만)으로 낮추는 것을 권장합니다.
public boolean validationToken(String token) { try { Jwts.parserBuilder().setSigningKey(secretKey).build().parseClaimsJws(token); - log.info("Token validation success"); + log.debug("Token validation success"); return true; } catch (JwtException | IllegalArgumentException e) { - log.error("Token validation failed: {}", e.getMessage()); + log.warn("Token validation failed: {}", e.getMessage()); return false; } }
39-44: 옵션: 시계 차(Clock Skew) 허용으로 신뢰성 개선서버/클라이언트 간 수 초의 시계 차를 허용하면 간헐 만료 이슈를 줄일 수 있습니다(예: 30~60초).
- Jwts.parserBuilder().setSigningKey(secretKey).build().parseClaimsJws(token); + Jwts.parserBuilder().setSigningKey(secretKey) + .setAllowedClockSkewSeconds(60) + .build().parseClaimsJws(token);같은 설정을 parseClaimsStrict에도 적용 가능.
Also applies to: 79-84
backend/src/test/java/org/sejongisc/backend/auth/controller/OauthLoginControllerTest.java (3)
200-210: 알 수 없는 provider는 5xx가 아니라 400이 타당클라이언트 입력 오류이므로 400 Bad Request가 적절합니다. 컨트롤러/글로벌 핸들러 매핑을 400으로 조정 후 테스트를 아래처럼 변경하세요.
- @DisplayName("POST /api/auth/login/unknown - 존재하지 않는 provider 요청 시 500") + @DisplayName("POST /api/auth/login/unknown - 존재하지 않는 provider 요청 시 400") ... - mockMvc.perform(post("/api/auth/login/unknown") + mockMvc.perform(post("/api/auth/login/unknown") .param("code", "code") .param("state", "test-state")) - .andExpect(status().is5xxServerError()) - .andExpect(result -> - assertTrue(result.getResolvedException() instanceof IllegalArgumentException)); + .andExpect(status().isBadRequest()); @@ - mockMvc.perform(get("/api/auth/oauth/unknown/init")) - .andExpect(status().is5xxServerError()); + mockMvc.perform(get("/api/auth/oauth/unknown/init")) + .andExpect(status().isBadRequest()); @@ - @DisplayName("POST /api/auth/login/UNKNOWN - Unknown provider 커버") + @DisplayName("POST /api/auth/login/UNKNOWN - Unknown provider는 400") ... - mockMvc.perform(post("/api/auth/login/UNDEFINED") + mockMvc.perform(post("/api/auth/login/UNDEFINED") .param("code", "dummy") .param("state", "state123")) - .andExpect(status().is5xxServerError()); + .andExpect(status().isBadRequest());핸들러가
IllegalArgumentException을 400으로 매핑하는지 확인해 주세요.Also applies to: 230-232, 369-378
314-320: 검증 실패 시 에러 페이로드도 함께 검증상태코드뿐 아니라 에러 스펙 필드(errorCode, fieldErrors 등)도 검증하면 회귀 방지에 유리합니다.
- .andExpect(status().isBadRequest()) - .andExpect(result -> - assertTrue(result.getResolvedException() instanceof org.springframework.web.bind.MethodArgumentNotValidException)); + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.errorCode").exists()) + .andExpect(jsonPath("$.message").exists()) + .andExpect(jsonPath("$.fieldErrors").isArray());
182-184: 옵션: OAuth 로그인 성공 응답에서 refreshToken도 검증컨트랙트에 refreshToken이 포함된다면 동일하게 단언을 추가해 계약을 고정하세요.
- .andExpect(jsonPath("$.accessToken").value("jwt-token")) - .andExpect(jsonPath("$.name").value("구글유저")); + .andExpect(jsonPath("$.accessToken").value("jwt-token")) + .andExpect(jsonPath("$.refreshToken").value("refresh-token")) + .andExpect(jsonPath("$.name").value("구글유저"));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java(2 hunks)backend/src/main/java/org/sejongisc/backend/user/controller/UserController.java(1 hunks)backend/src/main/java/org/sejongisc/backend/user/service/UserServiceImpl.java(1 hunks)backend/src/test/java/org/sejongisc/backend/auth/controller/OauthLoginControllerTest.java(6 hunks)backend/src/test/java/org/sejongisc/backend/auth/service/LoginServiceImplTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/main/java/org/sejongisc/backend/user/service/UserServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (2)
backend/src/main/java/org/sejongisc/backend/user/controller/UserController.java (3)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (1)
Slf4j(19-110)backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/JwtAuthenticationFilter.java (1)
Slf4j(30-124)backend/src/main/java/org/sejongisc/backend/auth/controller/OauthLoginController.java (1)
Slf4j(25-224)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (3)
backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/CustomUserDetailsService.java (1)
RequiredArgsConstructor(16-38)backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/JwtAuthenticationFilter.java (1)
Slf4j(30-124)backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtProvider.java (1)
Component(18-99)
🔇 Additional comments (5)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (1)
90-109: uid→subject 폴백 및 NPE/IAE 방어 로직 좋습니다리프레시 토큰(subject=userId)과 액세스 토큰(uid 클레임) 모두 호환하며, 공백/누락/형식 오류에 대한 메시지도 명확합니다. LGTM.
backend/src/test/java/org/sejongisc/backend/auth/service/LoginServiceImplTest.java (1)
79-81: JWT 3‑인자 시그니처로의 스텁 교정 LGTM
createToken(UUID, Role, String)로 스텁 업데이트가 정확합니다. 로그인 응답 검증도 일관됩니다.backend/src/main/java/org/sejongisc/backend/user/controller/UserController.java (3)
22-22: API 경로 통합이 적절합니다.
/apiprefix로 경로를 통합한 것은 API 구조 개선에 적합하며,JwtAuthenticationFilter가/api/user경로에 대해 인증을 요구하도록 올바르게 구성되어 있습니다.
38-38: 개인정보(PII) 로깅 검토 필요로깅에 이메일(PII)을 포함하는 것은 디버깅에 유용하지만, 개인정보 보호 정책(GDPR, 개인정보보호법 등)에 따라 문제가 될 수 있습니다.
프로덕션 환경에서는 이메일 대신
userId나 익명화된 식별자를 로깅하는 것을 권장합니다.조직의 개인정보 보호 정책을 확인하고, 필요하다면 다음과 같이 수정하세요:
- log.info("email: {} 권한: {}", user.getUsername(), user.getAuthorities()); + log.info("userId: {} 권한: {}", user.getUserId(), user.getAuthorities());
54-61: 보안 설정이 올바르게 구성되어 있습니다.검증 결과, 이 리뷰 의견은 잘못되었습니다.
SecurityConfig에서
/api/user/**엔드포인트는.anyRequest().authenticated()로 보호되고 있으며, JwtAuthenticationFilter도/api/user/**를 제외 목록에 두지 않아 JWT 토큰 검증을 수행합니다. 따라서 이들 엔드포인트는 이미 올바르게 보호되고 있습니다.
authenticatedUser또는usernull 체크는 방어적 프로그래밍(defensive programming) 관행이며, 보안 설정 오류를 나타내지 않습니다. 현재 구현은 적절합니다.Likely an incorrect or invalid review comment.
| public ResponseEntity<?> getUserInfo(@AuthenticationPrincipal CustomUserDetails user) { | ||
| if (user == null) { | ||
| log.warn("인증되지 않은 사용자 접근 시도"); | ||
| return ResponseEntity.status(HttpStatus.UNAUTHORIZED) | ||
| .body(Map.of("message", "인증이 필요합니다.")); | ||
| } | ||
|
|
||
| log.info("email: {} 권한: {}", user.getUsername(), user.getAuthorities()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
타입 안정성 개선 및 보안 설정 재검토 필요
현재 구현에는 다음 문제들이 있습니다:
-
타입 안정성 저하:
ResponseEntity<?>는 클라이언트가 응답 타입을 예측할 수 없게 만듭니다. 성공 응답(UserInfoResponse)과 오류 응답의 타입이 다르므로 명시적인 타입을 사용하거나 공통 응답 래퍼를 고려하세요. -
불필요한 null 체크:
@AuthenticationPrincipal로 주입되는user가 null인 경우는 Spring Security가 올바르게 구성되었다면 발생하지 않아야 합니다. 이 체크가 실제로 필요하다면JwtAuthenticationFilter와SecurityConfig에서/api/user/details경로가 인증 필수로 설정되었는지 확인이 필요합니다.
다음 스크립트로 Security 설정을 확인해 주세요:
추가로 타입 안정성을 개선하려면:
- public ResponseEntity<?> getUserInfo(@AuthenticationPrincipal CustomUserDetails user) {
+ public ResponseEntity<UserInfoResponse> getUserInfo(@AuthenticationPrincipal CustomUserDetails user) {
if (user == null) {
- log.warn("인증되지 않은 사용자 접근 시도");
- return ResponseEntity.status(HttpStatus.UNAUTHORIZED)
- .body(Map.of("message", "인증이 필요합니다."));
+ // Security 설정이 올바르다면 이 분기는 도달 불가능
+ throw new IllegalStateException("인증된 사용자가 필요하지만 SecurityContext에 없습니다.");
}🏁 Script executed:
#!/bin/bash
# SecurityConfig에서 /api/user/** 경로의 보호 설정 확인
ast-grep --pattern 'authorizeHttpRequests($$$)'
# SecurityConfig 파일 전체 내용 확인
fd -e java SecurityConfig --exec cat {}Length of output: 5643
🏁 Script executed:
#!/bin/bash
# Find UserController and check the endpoint mapping for getUserInfo
fd -e java -path '*/user/controller/UserController.java' --exec cat {}
# Find JwtAuthenticationFilter to understand auth logic
fd -e java -name 'JwtAuthenticationFilter.java' --exec cat {}Length of output: 399
🏁 Script executed:
#!/bin/bash
# Search for UserController and find the endpoint mapping
rg -n "class UserController" --type java -A 30
# Search for getUserInfo method with its annotations
rg -n "getUserInfo" --type java -B 5 -A 10
# Search for JwtAuthenticationFilter
rg -n "class JwtAuthenticationFilter" --type java -A 50Length of output: 44560
🏁 Script executed:
#!/bin/bash
# Find UserController full file including class-level annotations
rg -n "@RequestMapping|@RestController|public class UserController" --type java -A 5 backend/src/main/java/org/sejongisc/backend/user/controller/UserController.java
# Search for @AuthenticationPrincipal usage with required parameter
rg "@AuthenticationPrincipal" --type java -B 2 -A 2Length of output: 14918
보안 필터의 미흡한 검증 로직으로 인한 방어 코드 필요
현재 코드에서 user == null 체크가 필요한 근본 원인을 파악했습니다.
JwtAuthenticationFilter의 로직 문제:
- SecurityConfig는
/api/user/details경로에 대해.anyRequest().authenticated()로 인증을 필수로 설정 - 하지만 필터 내부(lines 72-81)에서 토큰이 없거나 유효하지 않을 때 단순히 경고만 로깅하고 요청을 계속 진행
- 결과적으로 인증되지 않은 요청이 컨트롤러에 도달하게 되고,
@AuthenticationPrincipal이 null이 될 수 있음
개선 방안:
-
근본 해결:
JwtAuthenticationFilter에서 보호된 엔드포인트에 토큰이 없거나 유효하지 않으면 즉시 401 응답을 반환하도록 수정// doFilterInternal 메서드 내 (line 79-81 수정) } else { log.warn("토큰이 없거나 유효하지 않음"); response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "인증이 필요합니다."); return; // 여기서 요청 중단 }
-
그 다음: 필터가 제대로 작동하면 컨트롤러의 null 체크 제거 가능
public ResponseEntity<UserInfoResponse> getUserInfo(@AuthenticationPrincipal CustomUserDetails user) { // null 체크 불필요 - 필터가 사전에 검증 완료
-
타입 안정성: ResponseEntity의 제네릭 타입을 명확히 지정
public ResponseEntity<UserInfoResponse> getUserInfo(...)
🤖 Prompt for AI Agents
backend/src/main/java/org/sejongisc/backend/user/controller/UserController.java
lines 31-38: the controller currently defensively checks for a null
@AuthenticationPrincipal because JwtAuthenticationFilter allows unauthenticated
requests through; fix by updating the filter to immediately send a 401 and
return when token is missing/invalid (so protected endpoints never reach the
controller unauthenticated), then remove the user == null branch from this
controller and change the method signature/return type to a concrete generic
(e.g., ResponseEntity<UserInfoResponse>) so the controller assumes a valid
CustomUserDetails and returns strongly typed responses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (3)
3-3: 와일드카드 임포트를 명시적 임포트로 변경 권장
io.jsonwebtoken.*와일드카드 임포트 대신 사용하는 클래스만 명시적으로 임포트하면 네임스페이스 충돌을 방지하고 코드 가독성이 향상됩니다.예시:
-import io.jsonwebtoken.*; +import io.jsonwebtoken.Claims; +import io.jsonwebtoken.ExpiredJwtException; +import io.jsonwebtoken.JwtException; +import io.jsonwebtoken.Jwts;
36-45: 토큰 검증 성공 로그를 DEBUG 레벨로 변경 권장매 요청마다 호출되는
validationToken메서드에서 성공 케이스를 INFO 레벨로 로깅하면 프로덕션 환경에서 로그 볼륨이 과도해집니다.다음과 같이 수정하세요:
- log.info("Token validation success"); + log.debug("Token validation success");
48-73: 인증 객체 생성 로직 개선 확인과거 리뷰에서 지적된 에러 메시지 불일치(line 65)와 PII 로깅 문제(line 71)가 올바르게 수정되었습니다.
운영 참고사항: Line 69에서 매 요청마다 DB 조회가 발생합니다. 권한 변경 빈도가 낮다면 Caffeine 등의 캐시 도입이나 토큰에 인가 정보 포함을 검토하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (4)
backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/CustomUserDetailsService.java (1)
RequiredArgsConstructor(16-38)backend/src/main/java/org/sejongisc/backend/auth/service/LoginServiceImpl.java (1)
Slf4j(25-80)backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/JwtAuthenticationFilter.java (1)
Slf4j(30-124)backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtProvider.java (1)
Component(18-99)
🔇 Additional comments (2)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (2)
89-108: LGTM! RefreshToken 호환성 문제 해결 확인과거 리뷰에서 지적된 NPE 및 RefreshToken 호환성 문제가 올바르게 수정되었습니다:
- uid 클레임 우선 확인 후 subject로 폴백 (lines 91-96)
- null/공백 검증 추가 (lines 99-101)
- 명확한 예외 처리 (lines 103-107)
구현이 깔끔하고 두 토큰 타입을 모두 지원합니다.
77-87: 확인 완료: 만료된 토큰 로그아웃은 의도된 동작입니다검증 결과, 만료된 토큰으로 로그아웃이 가능한 것은 의도된 설계입니다:
/api/auth/logout은 JWT 필터 대상이지만,validationToken이 만료된 토큰에 대해false를 반환할 뿐 예외를 발생시키지 않음- 필터는 경고를 기록하고 요청을 통과시킴
- 엔드포인트에서 명시적으로
JwtException을 처리하며, 만료된 토큰도 200 OK로 응답 (코드 주석: "멱등성 보장")parseClaims가 만료된 토큰의 클레임을 반환하므로 userId 추출 가능- 보안: 안전함 (로그아웃은 userId만 필요하며, 만료된 토큰도 클레임 포함)
이 동작은 UX 측면에서 합리적입니다(토큰 만료 후에도 로그아웃 가능).
…-web into SISC1-61-api-url-수정
Summary by CodeRabbit
새로운 기능
개선 사항
테스트