[Feat] refresh token으로 로그인 상태 유지 #24
Conversation
📝 WalkthroughWalkthrough이번 변경사항은 인증 및 토큰 관리 전반의 구조를 개편합니다. 기존 JWT 기반 방식에서 Auth 관련 구성요소로 전환하며, 인증 헤더 처리, 토큰 생성/검증, 리프레시 토큰 갱신 기능이 새롭게 구현되었습니다. API 엔드포인트의 반환 타입 및 메서드 시그니처가 업데이트 되었으며, 관련 DTO, 예외, 클라이언트 인터페이스 및 구성 파일이 재구성되었습니다. 또한, 보안 필터와 테스트 코드(통합/단위 테스트)도 수정 및 추가되어, 인증 흐름과 에러 핸들링 개선을 위한 전반적인 리팩토링이 이루어졌습니다. Changes
Sequence Diagram(s)로그인 플로우sequenceDiagram
participant C as 클라이언트
participant AC as AuthController
participant AS as AuthService
participant UC as UserClientComposite
participant TP as TokenProvider
participant DB as 사용자 저장소
C->>AC: GET /api/auth/login (oauthType, code)
AC->>AS: loginAndGenerateToken(oauthType, code)
AS->>UC: fetch(oauthType, code)
UC-->>AS: 사용자 정보 반환
AS->>DB: 사용자 존재 확인 및 저장
AS->>TP: provideTokens(user)
TP-->>AS: TokenDto (accessToken, refreshToken)
AS-->>AC: AuthResponse { token, user }
AC-->>C: SuccessResponse<AuthResponse>
토큰 갱신 플로우sequenceDiagram
participant C as 클라이언트
participant AC as AuthController
participant AS as AuthService
participant TP as TokenProvider
C->>AC: POST /api/auth/refresh (RefreshTokenRequest)
AC->>AS: refreshToken(request)
AS->>TP: provideAccessTokenByRefreshToken(refreshToken)
TP-->>AS: 새 accessToken 생성
AS-->>AC: TokenResponse { accessToken, refreshToken }
AC-->>C: SuccessResponse<TokenResponse>
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
size/XL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (14)
src/main/java/com/evenly/took/global/redis/RedisServiceImpl.java (1)
49-53: Redis 모든 키 삭제 기능 추가Redis에 저장된 모든 키를 삭제하는 메소드가 추가되었습니다. 이 기능은 테스트 환경에서 유용하게 사용될 수 있습니다.
다만, 다른 메소드들과 달리 boolean 값을 반환하지 않고 void를 반환하는데, 메소드 시그니처의 일관성을 위해 boolean을 반환하는 것을 고려해보세요.
src/main/resources/application-local.yml (1)
57-60: 환경 변수에 기본값 설정을 고려해 보세요.
jwt에서auth로의 키 이름 변경은 PR 목표에 맞게 인증 관련 설정을 일반화하는 좋은 접근입니다. 하지만refresh-token-expiration-time환경 변수에는 기본값이 설정되어 있지 않습니다. 다른 속성들처럼 기본값을 설정하면 환경 변수가 없는 환경에서도 안전하게 작동할 수 있습니다.다음과 같이 기본값을 추가하는 것을 권장합니다:
auth: access-token-secret: ${JWT_ACCESS_TOKEN_SECRET:secret} access-token-expiration-time: ${JWT_ACCESS_TOKEN_EXPIRATION_TIME:7200} - refresh-token-expiration-time: ${REFRESH_TOKEN_EXPIRATION_TIME} + refresh-token-expiration-time: ${REFRESH_TOKEN_EXPIRATION_TIME:604800}src/main/java/com/evenly/took/global/config/security/WebSecurityConfig.java (1)
39-48:❓ Verification inconclusive
보안 설정 변경 검토가 필요합니다.
API 엔드포인트에 대한 보안 설정이 변경되었습니다. 모든
/api/**경로에 대해 인증 없이 접근이 가능하도록 설정되었는데, 이는 보안 측면에서 위험할 수 있습니다. PR 목표에서는 refresh token 구현과 관련된 변경이라고 언급되었지만, 모든 API 엔드포인트를 개방하는 것이 의도된 것인지 확인이 필요합니다.401 오류 처리 로직은 적절하게 추가되었으나, TODO 코멘트에서 언급된 대로 예외 응답 형식의 통일이 필요해 보입니다.
🌐 Web query:
Spring Security에서 특정 엔드포인트만 인증을 건너뛰는 구성의 모범 사례💡 Result:
Spring Security에서 특정 엔드포인트에 대한 인증을 건너뛰기 위한 모범 사례는 **요청 매처(Request Matcher)**와 접근 제어 규칙을 조합하여 구현합니다. 주로 정적 리소스나 공개 API 엔드포인트에 적용되며, 아래 두 가지 주요 접근 방식을 활용할 수 있습니다.
1. 명시적 경로 허용 (Ant/Regex Matcher)
WebSecurityConfigurerAdapter상속 시HttpSecurity객체의authorizeRequests()메서드로 제어:@Configuration @EnableWebSecurity public class SecurityConfig extends WebSecurityConfigurerAdapter { @Override protected void configure(HttpSecurity http) throws Exception { http.authorizeRequests() .antMatchers("/public/**", "/api/v1/no-auth").permitAll() .antMatchers("/admin/**").hasRole("ADMIN") .anyRequest().authenticated(); } }
.antMatchers("/public/**").permitAll():/public경로 하위 모든 요청 인증 생략.regexMatchers(".*\\.js$"): 정규식으로 .js 파일 확장자 허용[3]2. 정적 리소스 자동 허용
Spring Boot의
PathRequest유틸리티 활용:@Bean public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { http.authorizeHttpRequests(auth -> auth .requestMatchers(PathRequest.toStaticResources().atCommonLocations()).permitAll() .requestMatchers("/health-check").permitAll() .anyRequest().authenticated() ); return http.build(); }
toStaticResources().atCommonLocations(): CSS/JS/이미지 등 정적 리소스 경로 자동 매핑[7]주의사항
- 매처 순서 중요성: 구체적인 규칙을 상단에 배치해야 함[6]
- CSRF 보호 예외 처리:
http.csrf().ignoringAntMatchers("/api/public-endpoint");- HTTP 메서드 명시적 지정:
.antMatchers(HttpMethod.GET, "/api/data").permitAll()고급 패턴: 조건부 접근 제어
JWT 검증 로직을 커스텀 필터에 통합하는 사례[6]:
private boolean isJwtAuthenticatedRequest(HttpServletRequest request) { String token = resolveToken(request); return StringUtils.hasText(token) && tokenProvider.validateToken(token); }이 방식은 헤더에 유효한 토큰이 존재할 경우 인증 절차를 생략하는 조건부 접근을 구현합니다.
💡 Spring Security 5.7+에서는
WebSecurityConfigurerAdapter대신SecurityFilterChain빈 등록 방식이 권장됩니다[2][7].Citations:
- 1: https://www.skshieldus.com/download/files/download.do?o_fname=EQST+insight_Research+Technique_202208.pdf&r_fname=20220818113152277.pdf
- 2: https://www.tempmail.us.com/ko/spring-security/java-%EC%84%B1%EA%B3%B5%EC%A0%81%EC%9D%B8-spring-%EB%B3%B4%EC%95%88-%EB%A1%9C%EA%B7%B8%EC%9D%B8-%ED%9B%84-403-%EC%98%A4%EB%A5%98-%ED%95%B4%EA%B2%B0
- 3: https://hackyboiz.github.io/2022/05/28/syru/cve-2022-22978/
- 4: https://unagi-zoso.tistory.com/310
- 5: https://velog.io/@springer/6-%EB%B3%B4%EC%95%88-%EC%9D%B8%EC%A6%9D%EA%B3%BC-%EA%B6%8C%ED%95%9C
- 6: https://velog.io/@rookedsysc/Spring-Security-Redirect-%EB%A1%9C%EC%A7%81-%EC%9A%B0%ED%9A%8C
- 7: https://walking-and-walking.com/entry/Spring-Security-%EB%B3%B4%EC%95%88-%EB%AA%A8%EB%B2%94-%EC%82%AC%EB%A1%80%EC%99%80-%ED%8C%A8%ED%84%B4
- 8: https://youngseo-computerblog.tistory.com/152
- 9: https://www.igloo.co.kr/security-information/spring-security-part1-%EC%9D%B8%EC%A6%9D%EA%B3%BC-%EC%9D%B8%EA%B0%80-%EC%95%84%ED%82%A4%ED%85%8D%EC%B2%98%EC%9D%98-%EC%9D%B4%ED%95%B4%EC%99%80-%EC%8B%A4%EC%A0%84-%ED%99%9C%EC%9A%A9/
- 10: https://www.igloo.co.kr/security-information/spring-security-part2-oauth-2-0-%EC%95%84%ED%82%A4%ED%85%8D%EC%B2%98-%EC%9D%B4%ED%95%B4%EC%99%80-%EB%B3%B4%EC%95%88-%EC%B7%A8%EC%95%BD%EC%A0%90-%EC%82%AC%EB%A1%80/
보안 설정 재검토 요청:
/api/**경로 접근 정책 확인 필요
- 현재 구성에서는
/api/health및/api/**경로가 인증 없이 접근 가능하도록 설정되어 있습니다. 이는 refresh token 구현과 별도로 보안상 위험을 초래할 수 있으므로, 의도한 동작인지 재검토해 주시기 바랍니다.- 만약 공개 API 엔드포인트만을 대상으로 한다면,
/public/**또는 개별적인 엔드포인트(예:/api/v1/no-auth)만 permit하도록 설정하는 것이 좋습니다. 최근 모범 사례에서는 Ant/Regex 매처를 사용하여 구체적인 경로에 한정하는 접근 방식을 권장합니다.- 401 오류 처리 로직은 적절히 구현되어 있으나, TODO에서 언급한 예외 응답 형식 통일 작업은 빠른 시일 내에 진행해 주시기 바랍니다.
src/main/java/com/evenly/took/global/config/properties/jwt/AuthProperties.java (1)
5-9: 속성 구성이 잘 확장되었습니다.JwtProperties에서 AuthProperties로 클래스명 변경과 함께 설정 prefix를 "jwt"에서 "auth"로 변경한 것은 적절합니다. 또한 refreshTokenExpirationTime이 추가되어 refresh token 기능을 위한 설정이 확장되었습니다.
다만 refreshTokenExpirationMilliTime 메서드를 accessTokenExpirationMilliTime처럼 추가하는 것을 고려해볼 수 있습니다. 이렇게 하면 코드 일관성이 향상될 것입니다.
public record AuthProperties( String accessTokenSecret, Long accessTokenExpirationTime, Long refreshTokenExpirationTime ) { public Long accessTokenExpirationMilliTime() { return accessTokenExpirationTime * 1000; } + + public Long refreshTokenExpirationMilliTime() { + return refreshTokenExpirationTime * 1000; + } }src/test/java/com/evenly/took/feature/auth/api/OAuthIntegrationTest.java (1)
46-64: 토큰 갱신 테스트가 적절히 구현되었습니다.액세스 토큰이 유효하지 않고 리프레시 토큰이 유효한 경우의 테스트가 잘 구현되었습니다. 로그인 요청 후 받은 토큰을 사용하고, 액세스 토큰만 무효화하여 갱신이 올바르게 작동하는지 확인하는 방식이 적절합니다.
다만, 라인 60에서 토큰을 조합할 때 문자열 분할 및 포맷팅을 사용하는 방식이 약간 복잡해 보입니다. HeaderHandler나 유틸리티 메서드를 통해 이를 더 명확하게 처리할 수 있을 것입니다.
src/test/java/com/evenly/took/feature/auth/application/UuidTokenProviderTest.java (1)
40-52: 토큰 만료 테스트에 대한 개선 제안현재 토큰 만료 테스트에서는 고정된 6초 대기 시간을 사용하고 있습니다. 이 값은
UuidTokenProvider의 설정과 일치해야 하지만, 하드코딩된 값은 유지보수성을 저하시킬 수 있습니다.설정 값을 직접 참조하거나 상수로 추출하여 아래와 같이 개선하는 것을 권장합니다:
- @Test - void 리프레쉬_토큰_생성_후_만료시간에_의한_만료_여부를_확인한다() throws InterruptedException { - // given - String userId = "1"; - - // when - String refreshToken = uuidTokenProvider.generateRefreshToken(userId); - - // then - assertThat(redisService.getValue(refreshToken)).isNotNull(); - Thread.sleep(6000); - assertThat(redisService.getValue(refreshToken)).isNull(); - } + @Test + void 리프레쉬_토큰_생성_후_만료시간에_의한_만료_여부를_확인한다() throws InterruptedException { + // given + String userId = "1"; + long testExpirationTimeMs = 6000; // 상수로 추출하거나 설정에서 가져오기 + + // when + String refreshToken = uuidTokenProvider.generateRefreshToken(userId); + + // then + assertThat(redisService.getValue(refreshToken)).isNotNull(); + Thread.sleep(testExpirationTimeMs); + assertThat(redisService.getValue(refreshToken)).isNull(); + }src/test/java/com/evenly/took/feature/auth/application/OAuthServiceTest.java (1)
52-66: 토큰 생성 및 사용자 정보 반환 테스트가 명확합니다.로그인 프로세스를 통해 액세스 토큰, 리프레시 토큰, 사용자 정보가 제대로 반환되는지 확인하는 테스트가 잘 작성되어 있습니다. PR의 주요 목표인 리프레시 토큰 기능 추가를 잘 테스트하고 있습니다.
다만, 더 강력한 테스트를 위해 토큰의 유효성을 더 자세히 검증하면 좋을 것 같습니다.
다음과 같이 검증 로직을 강화하는 것을 고려해 보세요:
- // then - assertThat(response.token().accessToken()).containsAnyOf("."); - assertThat(response.token().refreshToken()).isNotBlank(); - assertThat(response.user().name()).isEqualTo("took"); + // then + // 액세스 토큰 검증 - JWT 형식 확인(점이 두 개 있어야 함) + assertThat(response.token().accessToken()).contains("."); + assertThat(response.token().accessToken().split("\\.").length).isEqualTo(3); + + // 리프레시 토큰 검증 - UUID 형식 확인 + assertThat(response.token().refreshToken()).isNotBlank(); + assertThat(response.token().refreshToken()).matches("[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}"); + + // 사용자 정보 검증 + assertThat(response.user().name()).isEqualTo("took");src/main/java/com/evenly/took/global/security/auth/UuidTokenProvider.java (1)
29-35: 예외 처리 시 토큰 재사용에 대한 고려 제안
getUserId메서드에서 Redis에 해당 토큰이 없으면InvalidRefreshTokenException을 던지는 구조입니다. 만료 상황 외에 토큰 재사용 방지(1회성)도 고려를 원하는 경우, 인증 후 해당 토큰을 Redis에서 삭제하는 방식을 추가 검토할 수 있습니다.src/main/java/com/evenly/took/global/security/auth/AuthenticationFilter.java (1)
47-53: Unauthorized 에러 형식 통일의 필요성
response.sendError(HttpServletResponse.SC_UNAUTHORIZED, ex.getMessage())를 통해 에러를 반환하는 부분에서 TODO가 달려 있습니다. 다른 에러 응답 형식(예: JSON 형태)과 통일할 계획이 있다면, 이 부분을 추상화하거나 공통 예외 처리 로직으로 일원화하는 것을 권장합니다.src/main/java/com/evenly/took/feature/auth/api/OAuthApi.java (1)
37-42: Access Token과 Refresh Token을 하나의 헤더로 전달하는 구조를 재검토해 보세요.
한 헤더에 두 토큰을 모두 전달하면 토큰 파싱 로직이 길어지거나 혼동될 가능성이 큽니다.
대안으로 Refresh Token을 httpOnly 쿠키로 관리하거나, 각 토큰을 개별 헤더에 담을 수도 있습니다.src/main/java/com/evenly/took/global/security/auth/JwtTokenProvider.java (3)
3-3: AuthErrorCode를 통합 임포트로 사용하는 구문이 간결합니다.
다양한 에러 코드를 한 번에 사용하기 편리하겠지만, 필요한 코드만 임포트하는 방안도 고려해 보세요.
58-58:JWT_UNAUTHORIZED예외 메시지 로깅이 유용합니다.
문제 발생 시 빠르게 원인을 파악할 수 있으나, 민감한 정보를 포함하지 않도록 주의하세요.
81-81:accessTokenSecret()접근 방식이 간단합니다.
문자열 시크릿이 바로 노출되지 않도록.env, KeyVault 등 보안 스토어 방안을 고려해 보세요.src/main/java/com/evenly/took/feature/auth/api/HeaderHandler.java (1)
41-45:validateAuthHeader예외 처리가 명확합니다.
Bearer 접두어 체크만으로 충분한지, 추가 검사(토큰 길이, 공백 여부 등)는 없는지 고민해 볼 수 있습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
src/main/java/com/evenly/took/feature/auth/api/HeaderHandler.java(1 hunks)src/main/java/com/evenly/took/feature/auth/api/OAuthApi.java(2 hunks)src/main/java/com/evenly/took/feature/auth/api/OAuthController.java(2 hunks)src/main/java/com/evenly/took/feature/auth/application/OAuthService.java(2 hunks)src/main/java/com/evenly/took/feature/auth/dto/response/AuthResponse.java(1 hunks)src/main/java/com/evenly/took/feature/auth/dto/response/JwtResponse.java(0 hunks)src/main/java/com/evenly/took/feature/auth/dto/response/TokenResponse.java(1 hunks)src/main/java/com/evenly/took/feature/auth/dto/response/UserResponse.java(1 hunks)src/main/java/com/evenly/took/feature/user/domain/User.java(2 hunks)src/main/java/com/evenly/took/global/config/properties/PropertiesConfig.java(1 hunks)src/main/java/com/evenly/took/global/config/properties/jwt/AuthProperties.java(1 hunks)src/main/java/com/evenly/took/global/config/security/WebSecurityConfig.java(2 hunks)src/main/java/com/evenly/took/global/exception/auth/jwt/AuthErrorCode.java(1 hunks)src/main/java/com/evenly/took/global/exception/auth/oauth/InvalidAccessTokenException.java(1 hunks)src/main/java/com/evenly/took/global/exception/auth/oauth/InvalidRefreshTokenException.java(1 hunks)src/main/java/com/evenly/took/global/exception/auth/oauth/InvalidTokenException.java(1 hunks)src/main/java/com/evenly/took/global/redis/RedisService.java(1 hunks)src/main/java/com/evenly/took/global/redis/RedisServiceImpl.java(1 hunks)src/main/java/com/evenly/took/global/security/auth/AuthenticationFilter.java(1 hunks)src/main/java/com/evenly/took/global/security/auth/JwtTokenProvider.java(4 hunks)src/main/java/com/evenly/took/global/security/auth/UuidTokenProvider.java(1 hunks)src/main/java/com/evenly/took/global/security/jwt/JwtAuthenticationFilter.java(0 hunks)src/main/resources/application-local.yml(1 hunks)src/test/java/com/evenly/took/feature/auth/api/OAuthIntegrationTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/api/TestAuthController.java(1 hunks)src/test/java/com/evenly/took/feature/auth/application/OAuthServiceTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/application/UuidTokenProviderTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/jwt/JwtTokenProviderTest.java(3 hunks)src/test/java/com/evenly/took/global/domain/TestUserFactory.java(1 hunks)src/test/java/com/evenly/took/global/integration/IntegrationTest.java(1 hunks)src/test/java/com/evenly/took/global/service/ServiceTest.java(1 hunks)src/test/resources/application-test.yml(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/evenly/took/global/security/jwt/JwtAuthenticationFilter.java
- src/main/java/com/evenly/took/feature/auth/dto/response/JwtResponse.java
✅ Files skipped from review due to trivial changes (3)
- src/main/java/com/evenly/took/feature/auth/dto/response/TokenResponse.java
- src/main/java/com/evenly/took/global/exception/auth/oauth/InvalidTokenException.java
- src/test/java/com/evenly/took/feature/auth/api/TestAuthController.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (48)
src/main/java/com/evenly/took/global/exception/auth/oauth/InvalidAccessTokenException.java (1)
1-14: 잘 구현된 예외 클래스입니다.새로운 인증 메커니즘을 위한 예외 클래스가 명확히 정의되었습니다. 기존
TookException을 확장하여 시스템의 예외 처리 방식과 일관성을 유지했고, 필요한 생성자만 간결하게 구현되었습니다.src/main/java/com/evenly/took/global/redis/RedisService.java (1)
41-45: Redis 전체 키 삭제 기능 추가 - 잘 구현되었습니다.테스트 환경에서 Redis를 초기화하는 데 유용한 메서드입니다. 메서드 설명이 명확하고 인터페이스에 적절히 통합되었습니다.
src/main/java/com/evenly/took/global/exception/auth/oauth/InvalidRefreshTokenException.java (1)
1-14: 리프레시 토큰 관련 예외 클래스 적절히 구현되었습니다.새로운 인증 시스템의 일부로 리프레시 토큰 관련 예외를 처리하기 위한 클래스가 잘 정의되었습니다. 구조가 간결하고 기존 예외 처리 방식과 일관성이 있습니다.
src/main/java/com/evenly/took/feature/user/domain/User.java (1)
16-16: 전체 인자 생성자 추가 - 적절한 변경사항입니다.
@AllArgsConstructor애노테이션 추가는User객체의 인스턴스화 방법을 더 유연하게 해줍니다. 특히 인증 응답에서 사용자 정보를 포함하는 현재 PR의 변경사항과 잘 맞습니다.Also applies to: 29-29
src/main/java/com/evenly/took/feature/auth/dto/response/AuthResponse.java (1)
5-13: 인증 정보 응답 구조화 잘 되어 있습니다토큰 정보와 사용자 정보를 함께 반환하는 레코드 타입의 응답 객체가 잘 구현되어 있습니다. 생성자를 통해 TokenResponse와 UserResponse를 편리하게 초기화할 수 있어 좋습니다.
src/test/resources/application-test.yml (1)
27-30: 토큰 관련 테스트 설정이 추가되었습니다테스트 환경에서 사용할 토큰 관련 설정이 추가되었습니다.
다만, refresh-token-expiration-time이 5초로 매우 짧게 설정되어 있습니다. 이는 테스트 목적으로 의도적으로 설정한 것으로 보이지만, 실제 테스트 시 예상치 못한 만료 문제가 발생할 수 있으니 주의하세요.
src/test/java/com/evenly/took/global/service/ServiceTest.java (1)
9-13: 서비스 테스트를 위한 추상 클래스 구현이 좋습니다서비스 테스트를 위한 공통 설정을 추상 클래스로 잘 구현하였습니다. 모든 서비스 테스트 클래스에서 일관된 테스트 환경을 제공할 수 있습니다.
src/main/java/com/evenly/took/feature/auth/dto/response/UserResponse.java (1)
5-13: 깔끔한 DTO 구현입니다!User 도메인 객체에서 필요한 정보만 추출하여 응답 객체로 변환하는 패턴이 잘 적용되었습니다. Record 클래스를 사용하여 불변성을 보장하는 점도 좋습니다.
src/test/java/com/evenly/took/global/integration/IntegrationTest.java (1)
7-17:❓ Verification inconclusive
테스트 구성 변경에 대한 설명 추가가 필요합니다.
@ExtendWith,@Transactional,@Disabled등의 어노테이션이 제거되고 대신@Import(RedisTestConfig.class)가 추가되었습니다. 이러한 변경사항은 테스트 환경 및 동작에 큰 영향을 미칠 수 있습니다.트랜잭션 관리가 제거된 상태에서 테스트 환경이 적절하게 구성되는지 확인이 필요합니다. Redis 테스트 설정이 올바르게 작동하는지, 그리고 트랜잭션 없이 테스트가 서로 영향을 주지 않는지 검증해주세요.
🏁 Script executed:
#!/bin/bash # 관련 테스트 클래스들이 RedisTestConfig를 올바르게 사용하고 있는지 확인 echo "Checking test classes that extend IntegrationTest..." rg -l "extends\s+IntegrationTest" --type javaLength of output: 316
테스트 구성 변경 내용에 대한 추가 문서화가 필요합니다.
- 기존에 사용되던
@ExtendWith,@Transactional,@Disabled등의 어노테이션이 제거된 대신@Import(RedisTestConfig.class)가 도입됨에 따라 테스트 환경 및 동작에 미치는 영향을 명확히 문서화해 주세요.- 트랜잭션 없이 테스트가 실행될 경우, 테스트 간 상태 간섭이 발생하지 않도록 Redis 테스트 설정이 올바르게 작동하는지 재차 검증해주시기 바랍니다.
- 현재
IntegrationTest를 상속하는 테스트 클래스들(예:OAuthIntegrationTest.java,HealthControllerIntegrationTest.java)이 변경된 테스트 구성을 활용하고 있으므로, 이들에서 예상치 못한 동작이 발생하는지 확인해 주세요.src/main/java/com/evenly/took/global/config/properties/PropertiesConfig.java (1)
6-9: 클래스 이름 변경이 올바르게 적용되었습니다.JwtProperties에서 AuthProperties로의 변경이 PR 목표에 맞게 잘 이루어졌습니다. 이는 refresh token 구현에 따른 클래스 목적 변화를 잘 반영하고 있습니다.
src/test/java/com/evenly/took/feature/auth/api/OAuthIntegrationTest.java (2)
1-85: 통합 테스트가 잘 구현되었습니다.OAuth 로그인 및 토큰 갱신 기능에 대한 통합 테스트가 잘 작성되었습니다. 다양한 시나리오(유효한 토큰, 유효하지 않은 토큰, 토큰 갱신 등)를 테스트하여 refresh token 기능의 안정성을 검증하고 있습니다.
테스트 메서드 이름이 한글로 명확하게 작성되어 있어 테스트의 목적을 이해하기 쉽습니다.
33-44: 토큰 검증 테스트 확인이 필요합니다.이 테스트는 유효하지 않은 액세스 토큰과 유효하지 않은 리프레시 토큰의 경우 401 오류가 발생하는지 확인합니다. 그러나 테스트의 설정 부분에서
userClientComposite.fetch를 mock 설정하는 부분이 테스트의 주요 시나리오와 관련이 없어 보입니다. 이 mock 설정이 이 테스트에 필요한지 확인해 주세요.src/main/java/com/evenly/took/global/exception/auth/jwt/AuthErrorCode.java (2)
12-12: 클래스 이름 변경이 명확하게 목적을 반영합니다.JWT 토큰에 특화된 이름에서 더 일반적인 인증 관련 이름으로 변경한 것이 좋습니다. 이는 UUID 기반 리프레시 토큰을 도입하는 변경사항과 일치합니다.
15-17: 새로운 에러 코드가 적절히 추가되었습니다.리프레시 토큰과 액세스 토큰에 관련된 에러 코드를 추가한 것이 좋습니다. 각 에러 메시지가 명확하고 상태 코드도 적절합니다. 이는 PR의 목적인 리프레시 토큰을 통한 로그인 상태 유지 기능과 일치합니다.
src/test/java/com/evenly/took/feature/auth/application/UuidTokenProviderTest.java (4)
22-25: Redis 설정이 적절합니다.각 테스트 실행 전에 Redis의 모든 키를 삭제하여 테스트 간 격리를 보장하는 접근 방식이 좋습니다. PR 목표에서 언급한 "테스트 중 저장된 값을 정리하는 방법 개선"에 관한 고민을 해결한 좋은 사례입니다.
27-38: 리프레시 토큰 생성 테스트가 잘 작성되었습니다.토큰 생성 및 Redis에 저장 여부를 확인하는 테스트가 잘 구성되어 있습니다. 생성된 토큰의 유효성과 Redis에 사용자 ID가 올바르게 저장되었는지 확인하는 로직이 적절합니다.
54-58: 유효하지 않은 토큰 예외 테스트가 적절합니다.존재하지 않는 리프레시 토큰에 대한 예외 처리를 테스트하는 부분이 잘 구현되어 있습니다.
InvalidRefreshTokenException을 검증하는 방식이 적절합니다.
60-69: 토큰 변조 테스트가 잘 구성되어 있습니다.유효한 토큰을 변조했을 때의 예외 처리를 테스트하는 로직이 적절합니다. 실제 토큰 생성 후 변조된 값으로 테스트하는 방식이 실제 시나리오를 잘 반영합니다.
src/test/java/com/evenly/took/feature/auth/application/OAuthServiceTest.java (1)
23-50: 테스트 구성이 잘 설계되었습니다.
OAuthService의 의존성 주입과 설정이 적절히 구성되어 있습니다. Mock 객체와 실제 구현체가 적절히 조합되어 사용되었습니다. 특히 PR 목표에 부합하도록UuidTokenProvider와JwtTokenProvider를 모두 주입한 것이 좋습니다.src/test/java/com/evenly/took/feature/auth/jwt/JwtTokenProviderTest.java (4)
10-12: 임포트 경로 변경이 일관성 있게 적용되었습니다.JWT 관련 클래스들의 패키지 경로가 jwt에서 auth로 변경된 것이 PR 목표와 일치하며, 적절히 반영되었습니다.
27-28: 속성 클래스 변경 및 초기화가 적절합니다.
JwtProperties에서AuthProperties로 변경되었으며, 리프레시 토큰 만료 시간(1L)이 추가되었습니다. 이는 PR 목표에 부합하는 변경사항입니다.
34-34: 토큰 생성 메서드 변경이 적절합니다.User 객체 대신 사용자 ID만 전달하는 방식으로 변경한 것이 좋습니다. 이는 PR 설명에서 언급한 "액세스 토큰 갱신을 위해 User 객체를 다시 생성할 필요가 없도록 JWT 생성 시 사용자 ID만 사용하도록 변경"과 일치합니다.
Also applies to: 43-43
46-53: 토큰 검증 테스트가 적절합니다.토큰 생성 및 검증 테스트가 논리적으로 일관되게 구성되어 있습니다. 사용자 ID가 올바르게 추출되는지 확인하는 로직이 포함되어 있어 좋습니다.
src/main/java/com/evenly/took/global/security/auth/UuidTokenProvider.java (1)
22-27: Refresh 토큰 생성 로직이 명확합니다.
UUID.randomUUID()로 토큰을 생성하고, 적절한 TTL을 설정해 Redis에 보관하는 흐름이 명확합니다. 만료 시간이 지나면 자동으로 토큰이 폐기되므로 보안 측면에서도 불필요한 부담이 없습니다.src/main/java/com/evenly/took/feature/auth/application/OAuthService.java (1)
30-38: AccessToken과 RefreshToken 로직 분리가 명확합니다.
jwtTokenProvider.generateAccessToken과uuidTokenProvider.generateRefreshToken을 각각 호출해 다른 방식의 토큰을 발급하는 구조가 직관적이고 유지보수에도 용이해 보입니다. 이 반환값을AuthResponse로 일괄 관리하는 점도 좋아 보입니다.src/main/java/com/evenly/took/feature/auth/api/OAuthController.java (2)
26-30: OAuth 인증 URL 리다이렉션 처리가 깔끔합니다.
oauthService.getAuthCodeRequestUrl메서드를 통해 동적으로 URL을 생성한 뒤response.sendRedirect(url);로 바로 이동시키는 처리 방식이 정상 동작으로 보입니다.
33-37: Auth 헤더 설정 및 응답 구조가 명확합니다.
새로 생성된AuthResponse로부터token()을 받아 헤더를 설정하고, 사용자 정보를SuccessResponse로 반환하는 흐름이 일관성 있게 보입니다. 단, 프론트엔드에서 RefreshToken을 어떻게 활용할지 명확히 안내가 필요할 수 있습니다.src/main/java/com/evenly/took/global/security/auth/AuthenticationFilter.java (2)
24-29: 필터 제외 경로 설정이 적절합니다.
Swagger, 헬스 체크, 인증 관련 경로 등을 필터에서 제외함으로써 불필요한 인증 절차를 거치지 않아도 되어 효율적입니다.
55-63: AccessToken - 실패 시 RefreshToken 검사 흐름이 합리적입니다.
checkTokens메서드에서 AccessToken 검증에 실패하면 RefreshToken 검사로 넘어가는 구조가 명료합니다. RefreshToken까지 무효하면 예외를 던지므로 보안적으로도 문제가 없어 보입니다.src/main/java/com/evenly/took/feature/auth/api/OAuthApi.java (3)
14-14: 새로운@Header임포트 추가가 유효합니다.
OAuth 응답에 커스텀 헤더 관련 메타데이터를 제공하기 위해 필요한 임포트라서 문제 없습니다.
26-29: 리다이렉트 설명이 보다 명확해졌습니다.
description에 OAuthType별 리다이렉트 의도를 잘 설명하고 있어 좋은 개선입니다.
49-50:HttpServletResponse파라미터 추가로 확장성이 좋아졌습니다.
응답 헤더를 통한 토큰 전달이 필요하므로, 메서드 시그니처에HttpServletResponse추가는 타당합니다.src/main/java/com/evenly/took/global/security/auth/JwtTokenProvider.java (9)
1-1: 패키지 경로 변경에 따라 명칭이auth로 확장된 점이 의미있습니다.
JWT 뿐 아니라 전체 인증 로직을 포괄하도록 구조가 변경되어 적절해 보입니다.
11-13: 새로운 프로퍼티 클래스와 예외 클래스를 적절히 참조하고 있습니다.
AuthProperties와InvalidAccessTokenException사용으로 명확하게 식별 가능한 구조가 되었습니다.
29-29:authProperties주입이 적절합니다.
토큰 생성에 필요한 설정값을 분리해 관리하므로 유지보수성이 향상됩니다.
31-32:generateAccessToken(String userId)시그니처가 간소화되었습니다.
User객체 의존을 줄여서 서비스 레이어가 가벼워질 것으로 예상됩니다.
34-34: 액세스 토큰 만료시각 계산 로직이 명확합니다.
authProperties.accessTokenExpirationMilliTime()를 활용하여 시간이 잘 반영됩니다.
38-40:generateClaims(String userId)메서드가 단순화되어 가독성이 좋습니다.
사용자의 식별 정보만sub에 담아 JWT를 최소화합니다.
62-62:getUserId메서드의 선언부가 명확합니다.
토큰 유효성 검증과 함께 사용자 식별만 반환해, 단순하고 직관적입니다.
63-66: JWT 파싱 로직이 try 블록 내부에 잘 정리되어 있습니다.
유효하면getSubject()를 반환하고, 예외 시 별도 처리를 하므로 휴먼 에러가 줄어듭니다.
68-69:AuthErrorCode.INVALID_ACCESS_TOKEN를 명시적으로 사용해 예외 종류를 분리했습니다.
보다 구체적인 예외 처리를 지원하니 유지보수성이 높아집니다.src/main/java/com/evenly/took/feature/auth/api/HeaderHandler.java (7)
1-13: 클래스 전반이 헤더 처리를 전담하도록 설계되어 좋습니다.
@Component선언으로 스프링 Bean 주입이 수월해지고, 토큰 헤더 관리 로직이 한 곳에 모여 구조가 명확해집니다.
15-19: 헤더 관련 상수들이 명확하게 정의되었습니다.
가독성이 좋으며, 상수를 수정하면 전체 로직에 반영되어 유지보수에 유리합니다.
20-23:setAuthHeader메서드는 응답 헤더 설정을 깔끔하게 처리합니다.
buildAuthHeaderValue와 역할이 잘 분리되어 코드 가독성이 좋아졌습니다.
25-27:buildAuthHeaderValue에서 문자열 포맷을 간단히 설정했습니다.
조건문 없이 포맷팅만 담당해 메서드 역할이 명확합니다.
29-33:resolveAccessToken메서드에서 Bearer 방식 확인 후 토큰 부분만 추출합니다.
코드가 간결하나, 배열 접근 시 인덱스 에러가 날 가능성은 없을지 고려해 보세요.
35-39:resolveRefreshToken도 비슷한 접근 방식으로 통일성을 유지합니다.
동일한 구조로 작성되어 일관성이 있지만, 에러 처리 흐름 또한 일관성 있게 유지되는지 확인이 필요합니다.
46-46: 클래스 마지막 중괄호 입니다.
더 이상 개선할 부분이 없어 보이므로 승인합니다.
src/main/java/com/evenly/took/global/security/auth/AuthenticationFilter.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
새벽까지 작업하시느라 너무너무 수고하셨습니다!!
의견 요청하신 부분은 아래에 남겨둘게요~
JWT 생성 시 name 없이 id만 활용하는 것
이 부분은 클라이언트에서 JWT로 사용자 정보를 활용한다면 name이나 기타 보안에 해가되지 않는 선에서 제공하면 좋을거 같다는게 저의 의견입니다! 다만 클라이언트나 찬기님 말씀도 들어보고 채영님이 결정해주셔도 될거 같아요!
redis에 저장할 때 refresh token을 키로, 사용자 id를 값으로 저장
refresh token으로 사용자를 찾는거니 흐름상 맞아보이는데 저도 확신이..ㅎㅎ
중복 이슈도 refresh token이 만료되는 시점에는 이미 기존 refresh token은 만료되어 사라졌을테니 괜찮지 않을까 싶습니다.! 👀
ErrorCode를 삽입할 때 static import 하지 않는 것이 어떨지 제안드립니다.
확인했는데 가독성이 훨씬 좋네요! 찬성합니다.
access token 만료 시간을 더 짧게 가져가는 건..
이 부분은 로그인으로 위치 기록 등등의 테스크가 명확해지고 결정해도 좋을거 같아요!
제가 이전에 만들어둔 ErrorCode가 feature별로 생성하는 것을 의도했던 거라 현 OAuthErrorCode와 AuthErrorCode를 합치는 거 어떨까요
너어어무 좋아요!!! 감사합니다ㅎㅎㅎ
| public void deleteAllKeys() { | ||
| executeOperation(() -> { | ||
| redisTemplate.getConnectionFactory().getConnection().serverCommands().flushAll(); | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
네 맞습니다! 현재는 비즈니스 로직에선 사용되지 않아요. 하지만 더 고민을 해보았을 때 비즈니스 로직으로 침투시키는 것보다는 테스트 클래스에서 진행하는 것이 역할 측면에서 나을 것 같아서 RedisCleaner로 클래스 분리하였습니다! 그리고 IntegrationTest와 ServiceTest 실행 시 모든 테스트 직전에 실행되도록 구현해두었어요.
src/test/java/com/evenly/took/feature/auth/api/TestAuthController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/evenly/took/global/security/auth/UuidTokenProvider.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
구현의도는 파악했으나,
전체적인 AccessToken RefreshToken 처리방식이 일반적인 경우가 아니다보니, 클라이언트 입장에서 작업이 어려울 수 있을 것 같아요!
일반적인 플로우는
- 로그인 시, AccessToken(ATK), RefreshToken(RTK) 발급하여 Body JSON 에 담아 전달
- 클라이언트에서 ATK, RTK 저장
- 서버에 요청을 보낼 시, ATK 만 Authorization 헤더 필드에 Bearer {ATK} 형식으로 담아 요청
- ATK 가 만료되면, 서버에서 클라이언트에게 만료 응답 반환
- 클라이언트에서 만료응답을 받으면 Refresh API 에 RTK 를 담아 새로운 ATK 요청
- 새로 발급받은 ATK 로 Authorization 헤더 값을 교체하고 재요청
- 위 작업 RTK 만료 시간까지 반복
- RTK 만료 시, 재로그인 필요
위와 같이 이루어져요!
https://gong-story.tistory.com/44
해당 블로그 참고해보시면 더 좋을 것 같습니다~!
redis에 저장할 때 refresh token을 키로, 사용자 id를 값으로 저장했습니다. access token 만료 시에 refresh token으로 사용자 정보를 찾기 위해서요! 제 방식에 100% 확신이 없어서 관련하여 의견이 궁금합니다.
RTK 를 Key, UserID 를 Value 로 가져가는 부분 너무 좋습니다!
JWT 생성 시 name 없이 id만 활용하는 것이 어떤가 하여 일단 코드 수정해두었습니다. (새벽이라 의견을 물을 수가 없었어요..) 해당 방법을 제안하는 이유는 아래와 같아요. 관련해서 의견 주시면 좋을 것 같습니다!
JWT 에 UserID 정보만 담는 것 좋아보여요! 유저 정보는 ID 만 있어도 서버에서 읽어올 수 있으니 굿인 것 같습니다ㅎㅎ
access token 만료 시간을 더 짧게 가져가는 건 어떨지 제안드립니다. (1시간 정도?)
한시간 좋아요!! 규모가 큰기업들은 ATK 가 10분단위인 플랫폼도 있습니다
제가 이전에 만들어둔 ErrorCode가 feature별로 생성하는 것을 의도했던 거라 현 OAuthErrorCode와 AuthErrorCode를 합치는 거 어떨까요 (모두 동의하시면 제가 수정해두겠습니다)
합치는 의견 동의합니다!!!
src/main/java/com/evenly/took/feature/auth/api/HeaderHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/com/evenly/took/feature/auth/api/HeaderHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/com/evenly/took/feature/auth/api/HeaderHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/com/evenly/took/global/security/auth/AuthenticationFilter.java
Outdated
Show resolved
Hide resolved
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/test/java/com/evenly/took/feature/auth/application/OAuthServiceTest.java (1)
26-40: 로그인 케이스 외 다양한 시나리오 확장 권장현재 테스트는 정상 시나리오(로그인 성공)에 대해서만 검증합니다. 코드 유효하지 않은 경우나 사용자 정보가 없을 때 등 예외 케이스에 대한 테스트도 추가하면 전체적인 안정성이 향상됩니다.
src/test/java/com/evenly/took/global/integration/IntegrationTest.java (1)
27-28: RedisCleaner 주입 시점 확인
@BeforeEach에서 호출을 위해@Autowired로 인스턴스 주입을 진행하는 것 자체는 간단하고 직관적입니다. 다만, 다른 테스트 클래스에서도 동일하게 활용된다면 공동 부모 클래스에서 상속받을지 여부를 고민해 보시는 것도 좋습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/test/java/com/evenly/took/feature/auth/application/OAuthServiceTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/application/UuidTokenProviderTest.java(1 hunks)src/test/java/com/evenly/took/global/helper/RedisCleaner.java(1 hunks)src/test/java/com/evenly/took/global/integration/IntegrationTest.java(2 hunks)src/test/java/com/evenly/took/global/service/ServiceTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/java/com/evenly/took/global/service/ServiceTest.java
- src/test/java/com/evenly/took/feature/auth/application/UuidTokenProviderTest.java
🔇 Additional comments (4)
src/test/java/com/evenly/took/global/helper/RedisCleaner.java (1)
7-8: 테스트 프로필 사용 방식 확인 필요
@ActiveProfiles("test")를 사용하여 테스트 환경에서만 빈이 등록되는 점은 유용하지만, 다른 환경에서 해당 구성 요소가 전혀 등록되지 않아도 괜찮은지 확인해 주세요. 상황에 따라 Runtime 환경에서도 같은 로직이 필요할 수 있으므로, 운영환경에서 Redis를 지울 일이 없다면 현재 구조가 적합하나, 혹시 모를 확장 시점을 고려해 주의를 기울이는 것이 좋습니다.src/test/java/com/evenly/took/feature/auth/application/OAuthServiceTest.java (1)
7-9: @MockitoBean 사용 방식 점검스프링 테스트 환경에서는 일반적으로
@MockBean또는@Mock+@InjectMocks조합을 자주 사용합니다.@MockitoBean이 별도 설정으로 구성되어 있다면 문제없지만, 다른 팀원이나 협업 파트너에게 익숙하지 않을 수 있습니다. 문서화하거나 일반적인 어노테이션으로 대체 가능한지 고려해 보세요.src/test/java/com/evenly/took/global/integration/IntegrationTest.java (2)
18-18: RedisTestConfig 임포트 확인Redis 관련 설정을
@Import(RedisTestConfig.class)로 통합하여 테스트를 간소화하는 접근은 적절해 보입니다. 환경별로 구분된 설정이 유효한지 추가적으로 확인 부탁드립니다.
35-35: Redis 정상 연결 및 예외 처리 확인
redisCleaner.deleteAllKeys()호출 시 Redis 서버가 비정상인 경우 예외가 발생할 가능성이 있습니다. 테스트 환경에서 에러 처리가 적절히 되어 있는지, 실패 시 다른 테스트 흐름에 영향을 주지 않는지 확인이 필요합니다.
아 제가 이 방식을 채택한 이유에 대해 설명드리지 않았었네요! 먼저 제가 사용해본 토큰 처리 방식에는 두 가지가 있었는데요! 찬기님이 설명해주신 위 방식, 그리고 아래 방식이 존재했습니다.
두 방식이 존재할 때 어떤 방식이 편한지 저희 클라이언트 팀원에게 여쭈었을 때, 클라이언트 측에서는 후자가 편하다고 말씀해주셔서 후자로 진행하였습니다. 두 방식 모두 동작에는 문제가 없을 것으로 예상되지만, refresh token 탈취 가능성을 고려하면 찬기님 말씀대로 전자 방식이 나을 것 같다는 생각도 있어요. 한 번 더 클라이언트 측과 논의를 할지 이대로 진행할지 고민이 되는 것 같아요! |
github-actions
bot
added
size/XL
- 기존: AT 만료 시 함께 보낸 RT로 갱신 - 수정: AT 만료 시 /refresh API를 통해 갱신
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
src/main/java/com/evenly/took/global/exception/dto/ErrorResponse.java (1)
19-20: timestamp 필드에 JSON 형식 지정 추가timestamp 필드에
@JsonFormat어노테이션을 추가하고final키워드를 제거했습니다. JSON 형식 지정은 좋은 개선이지만,final키워드를 제거하면 불변성이 손상됩니다.JSON 형식 지정을 유지하면서 불변성도 보존하기 위해 다음과 같이 수정하는 것을 고려해보세요:
-@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd HH:mm:ss") -private LocalDateTime timestamp; +@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd HH:mm:ss") +private final LocalDateTime timestamp;src/test/java/com/evenly/took/feature/auth/api/TestAuthController.java (1)
7-13: 테스트 컨트롤러의 목적을 명확히 하는 주석을 추가하는 것이 좋겠습니다테스트용 컨트롤러를 만든 의도는 좋지만, 다른 개발자들이 이 컨트롤러의 목적을 쉽게 이해할 수 있도록 클래스 또는 메서드 레벨에 주석을 추가하는 것이 좋을 것 같습니다. 과거 리뷰 코멘트에서 언급된 "인증 과정을 거치지 않는 깨끗한 로직의 컨트롤러를 테스트하기 위한 용도"라는 설명을 코드에 반영하면 더 명확할 것입니다.
@ActiveProfiles("test") @RestController +/** + * 인증 과정을 거치지 않는 깨끗한 로직의 컨트롤러 테스트를 위한 용도 + */ public class TestAuthController { @GetMapping("/api/test") + /** + * 통합 테스트에서 인증 없이 API 요청을 테스트하기 위한 메서드 + */ public void test() { }src/main/java/com/evenly/took/feature/auth/api/HeaderHandler.java (1)
13-14: 상수 접근 제한자 수정 고려이 상수들은
HeaderHandler클래스 외부에서도 사용될 가능성이 있습니다. 필요에 따라private에서public static final로 변경하는 것을 고려해보세요.src/test/java/com/evenly/took/feature/auth/api/OAuthIntegrationTest.java (1)
25-35: 로그인 API 엔드포인트 HTTP 메서드 재검토로그인 API는 일반적으로 GET 대신 POST 메서드를 사용합니다. 로그인 작업은 서버 상태를 변경하는 작업이므로 POST나 PUT과 같은 HTTP 메서드가 더 적합합니다.
- .when().get("/api/oauth/login/KAKAO?code=code") + .when().post("/api/oauth/login/KAKAO?code=code")src/main/java/com/evenly/took/global/security/auth/JwtTokenProvider.java (1)
54-57: 에러 로깅과 예외 중복 개선 필요validateToken 메서드에서 로그 메시지와 예외에 동일한 에러 코드를 사용하고 있어, 로그에 중복된 메시지가 기록될 수 있습니다. 로그 메시지를 좀 더 상세하게 작성하는 것이 좋겠습니다.
try { parseClaims(token); } catch (JwtException | IllegalArgumentException e) { - log.error(AuthErrorCode.JWT_UNAUTHORIZED.getMessage(), e); + log.error("JWT 토큰 검증 실패: {}", e.getMessage(), e); throw new InvalidAccessTokenException(AuthErrorCode.JWT_UNAUTHORIZED); }src/main/java/com/evenly/took/feature/auth/api/OAuthController.java (1)
27-29: 302 상태 코드 사용 관련 권장사항
현재redirectAuthRequestUrl메서드에서HttpStatus.FOUND를 반환하고 있으나, 클라이언트 단에서 명시적 리다이렉션 처리를 진행해야 하므로 실제 HTTP 리다이렉트와는 다르다는 점에 유의가 필요합니다. 일반적인 리다이렉트 처리를 원하시면RedirectView혹은response.sendRedirect(...)를 고려해 보시는 것이 좋습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/main/java/com/evenly/took/feature/auth/api/HeaderHandler.java(1 hunks)src/main/java/com/evenly/took/feature/auth/api/OAuthApi.java(2 hunks)src/main/java/com/evenly/took/feature/auth/api/OAuthController.java(2 hunks)src/main/java/com/evenly/took/feature/auth/application/OAuthService.java(2 hunks)src/main/java/com/evenly/took/feature/auth/application/TokenProvider.java(1 hunks)src/main/java/com/evenly/took/feature/auth/dto/TokenDto.java(1 hunks)src/main/java/com/evenly/took/feature/auth/dto/request/RefreshTokenRequest.java(1 hunks)src/main/java/com/evenly/took/feature/auth/dto/response/AuthResponse.java(1 hunks)src/main/java/com/evenly/took/feature/auth/dto/response/TokenResponse.java(1 hunks)src/main/java/com/evenly/took/feature/auth/dto/response/UserResponse.java(1 hunks)src/main/java/com/evenly/took/global/exception/auth/jwt/AuthErrorCode.java(1 hunks)src/main/java/com/evenly/took/global/exception/auth/oauth/OAuthErrorCode.java(0 hunks)src/main/java/com/evenly/took/global/exception/dto/ErrorResponse.java(1 hunks)src/main/java/com/evenly/took/global/response/SuccessResponse.java(1 hunks)src/main/java/com/evenly/took/global/security/auth/AuthenticationFilter.java(1 hunks)src/main/java/com/evenly/took/global/security/auth/JwtTokenProvider.java(4 hunks)src/main/java/com/evenly/took/global/security/client/AuthCodeRequestUrlProviderComposite.java(2 hunks)src/main/java/com/evenly/took/global/security/client/UserClientComposite.java(2 hunks)src/test/java/com/evenly/took/feature/auth/api/OAuthIntegrationTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/api/TestAuthController.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/authcode/AuthCodeRequestUrlProviderCompositeTest.java(2 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/evenly/took/global/exception/auth/oauth/OAuthErrorCode.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/evenly/took/feature/auth/dto/request/RefreshTokenRequest.java
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/java/com/evenly/took/feature/auth/dto/response/UserResponse.java
- src/main/java/com/evenly/took/feature/auth/dto/response/TokenResponse.java
- src/main/java/com/evenly/took/global/security/auth/AuthenticationFilter.java
- src/main/java/com/evenly/took/feature/auth/api/OAuthApi.java
- src/main/java/com/evenly/took/feature/auth/application/OAuthService.java
🔇 Additional comments (25)
src/main/java/com/evenly/took/global/exception/dto/ErrorResponse.java (3)
8-8: JSON 형식 지정 임포트가 추가되었습니다.
JsonFormat어노테이션을 사용하여 timestamp 필드의 직렬화 형식을 지정하는 것은 좋은 개선입니다. 이를 통해 클라이언트에게 일관된 형식의 날짜/시간 문자열을 제공할 수 있습니다.
11-11: NoArgsConstructor 어노테이션이 추가되었습니다.
@NoArgsConstructor어노테이션을 추가하여 매개변수 없는 생성자를 생성하는 것은 유연성을 제공하지만, 이 생성자를 통해 생성된 객체는 초기화되지 않은 필드를 가질 수 있습니다. 이러한 객체가 제대로 초기화되었는지 확인하는 추가 검증이 필요할 수 있습니다.현재 코드에서는
of(ErrorCode)팩토리 메서드를 통해 객체를 생성하는 것이 권장되는 방식입니다. 매개변수 없는 생성자를 추가한 이유와 어떤 상황에서 사용될 예정인지 확인해보세요.Also applies to: 14-14
17-18: 필드에서 final 제거status와 message 필드에서
final키워드가 제거되었습니다. 이렇게 하면 객체 생성 후에도 이 값들을 변경할 수 있게 되어 불변성(immutability)이 손상됩니다. 에러 응답과 같은 DTO 객체는 일반적으로 불변으로 유지하는 것이 안전합니다.이러한 변경이 정말 필요한지 확인해보세요. 만약 필드 값을 수정해야 하는 특별한 사례가 없다면, 불변성을 유지하기 위해
final키워드를 다시 추가하는 것을 고려해보세요.src/main/java/com/evenly/took/feature/auth/dto/TokenDto.java (1)
3-6: 접근 토큰과 리프레시 토큰을 캡슐화하는 레코드 생성이 좋습니다!자바 레코드를 사용하여 토큰 정보를 불변 객체로 관리하는 것은 매우 효과적인 접근 방식입니다. 이렇게 하면 데이터의 무결성을 보장하고 코드 가독성도 향상됩니다.
src/main/java/com/evenly/took/global/security/client/UserClientComposite.java (1)
13-14: 오류 코드 참조 변경이 일관성 있게 적용되었습니다
OAuthErrorCode에서AuthErrorCode로의 전환이 적절하게 이루어졌습니다. 이러한 변경은 인증 관련 오류 코드를 일관되게 관리하는 데 도움이 됩니다.Also applies to: 28-28
src/main/java/com/evenly/took/global/security/client/AuthCodeRequestUrlProviderComposite.java (1)
12-13: 오류 코드 참조 변경이 일관성 있게 적용되었습니다
OAuthErrorCode에서AuthErrorCode로의 전환이 일관되게 적용되었습니다. 인증 관련 오류 코드를 중앙화하여 관리하는 것은 코드 유지보수성을 향상시킵니다.Also applies to: 27-27
src/main/java/com/evenly/took/feature/auth/dto/response/AuthResponse.java (1)
1-18: 깔끔한 인증 응답 구조 구현!인증 정보와 사용자 정보를 명확하게 분리하여 클라이언트 측에서 쉽게 처리할 수 있도록 설계했네요. 레코드를 사용하여 불변성을 확보한 것도 좋은 접근입니다. Swagger 문서화를 위한
@Schema어노테이션도 잘 적용되어 있습니다.src/main/java/com/evenly/took/feature/auth/api/HeaderHandler.java (2)
16-20: 헤더 처리 로직이 명확하게 구현되었습니다토큰 추출 및 검증 로직이 잘 구현되어 있습니다. 이전 리뷰 코멘트에서 언급했듯이, 서버가 클라이언트로 토큰을 보낼 때는 일반적으로 'Bearer' 접두사 없이 보내지만, 클라이언트에서 서버로 보낼 때는 이 접두사를 붙이는 것이 일반적입니다. 현재 구현은 이 패턴을 잘 따르고 있습니다.
22-26: 예외 처리 로직이 깔끔합니다헤더 유효성 검사 로직과 예외 처리가 명확하게 구현되어 있습니다.
src/test/java/com/evenly/took/feature/auth/client/authcode/AuthCodeRequestUrlProviderCompositeTest.java (2)
13-13: import 문 변경이 적절합니다
OAuthErrorCode에서AuthErrorCode로의 변경은 PR 목표에 언급된 인증 관련 클래스 이름 변경(JwtXXX에서 AuthXXX로)과 일치합니다.
51-51: 에러 코드 참조 업데이트가 잘 되었습니다기존
OAuthErrorCode에서 새로운AuthErrorCode로 참조를 업데이트한 것이 적절합니다. 이는 PR에서 언급된 인증 관련 클래스 이름 변경 작업의 일환으로 보입니다.src/test/java/com/evenly/took/feature/auth/api/OAuthIntegrationTest.java (2)
20-24: 통합 테스트 설계가 좋습니다
UserClientComposite를 목 객체로 설정하여 외부 의존성을 효과적으로 제어하고 있습니다. 이는 테스트의 안정성과 예측 가능성을 높이는 좋은 방법입니다.
89-109: 테스트 케이스가 잘 구성되어 있습니다유효한 액세스 토큰으로 API를 요청하는 테스트 케이스가 명확하게 구성되어 있습니다. 설정, 실행, 검증 단계가 잘 구분되어 있고, 예상 결과도 명확합니다. 다만, 여기서도 로그인 API 호출 시 HTTP 메서드를 POST로 변경하는 것이 좋겠습니다.
- .when().get("/api/oauth/login/KAKAO?code=code") + .when().post("/api/oauth/login/KAKAO?code=code")src/main/java/com/evenly/took/global/exception/auth/jwt/AuthErrorCode.java (2)
12-19: Error 코드가 적절하게 정의되었습니다AuthErrorCode enum이 ErrorCode 인터페이스를 잘 구현하고 있으며, 각 인증 오류에 대한 적절한 HTTP 상태 코드와 메시지가 설정되어 있습니다. OAuth 인증 및 JWT 토큰 관련 오류를 포괄적으로 처리할 수 있는 구조입니다.
21-22: 명확한 필드 구조status와 message 필드가 final로 선언되어 불변성을 보장하고, RequiredArgsConstructor와 함께 잘 사용되고 있습니다.
src/main/java/com/evenly/took/feature/auth/application/TokenProvider.java (2)
14-18: 토큰 제공자 클래스 구조가 잘 설계되었습니다JwtTokenProvider와 UuidTokenProvider를 조합하여 액세스 토큰과 리프레시 토큰을 함께 관리하는 구조가 좋습니다. 단일 책임 원칙(SRP)을 잘 따르고 있습니다.
23-27: 토큰 생성 로직이 명확합니다User 객체로부터 ID만 추출하여 토큰을 생성하는 방식이 깔끔합니다. TokenDto를 통해 두 토큰을 함께 반환하는 것도 적절합니다.
src/main/java/com/evenly/took/global/response/SuccessResponse.java (2)
7-8: 제네릭 타입 적용으로 타입 안전성 향상SuccessResponse 클래스에 제네릭 타입을 적용하여 타입 안전성을 향상시킨 것은 좋은 변경입니다. 또한 timestamp 필드에 JsonFormat 어노테이션을 추가하여 일관된 포맷으로 직렬화되도록 한 것도 적절합니다.
Also applies to: 15-15, 22-24
26-28: 제네릭 메서드로 개선된 팩토리 메서드정적 팩토리 메서드도 제네릭을 사용하도록 수정하여 타입 안전성을 보장하면서도 사용성을 유지한 것이 좋습니다.
Also applies to: 30-32
src/main/java/com/evenly/took/global/security/auth/JwtTokenProvider.java (4)
1-1: 패키지 및 임포트 변경 적절jwt 패키지에서 auth 패키지로 이동하고, 관련 임포트도 적절하게 변경되었습니다. 이는 리프레시 토큰 도입에 따른 인증 개념의 확장을 잘 반영하고 있습니다.
Also applies to: 9-11
27-28: AuthProperties 의존성 주입JwtProperties에서 AuthProperties로 변경된 것은 인증 관련 속성을 통합 관리하기 위한 좋은 개선입니다.
29-39: 토큰 생성 로직 단순화User 객체 대신 userId 문자열을 직접 받아 토큰을 생성하도록 변경하여 로직이 단순화되었습니다. 이는 PR 목표에 언급된 대로 "User 객체를 다시 만들 필요 없이 userId만으로 access token 갱신"하는 개선 방향과 일치합니다.
60-68: getUserId 메서드에 예외 처리 추가getUserId 메서드에 JwtException 처리를 추가한 것은 좋은 개선입니다. 이로 인해 유효하지 않은 토큰에 대해 더 명확한 오류 메시지를 제공할 수 있습니다.
src/main/java/com/evenly/took/feature/auth/api/OAuthController.java (2)
3-3: 새로운 import 구문에 대한 간단 피드백입니다.
인증·인가 로직에 필요한 DTO와 관련 클래스를 올바르게 import하신 것으로 보입니다. 현재 코드는 문제없이 동작할 것으로 보이며, 추가 수정사항은 없습니다.Also applies to: 6-6, 12-15
33-36: OAuth 콜백 파라미터 검증 필요성
login메서드에서code파라미터를 그대로 받아 처리하고 계신데,code가 누락되거나 유효하지 않은 경우에 대한 예외 처리가 별도로 필요한지 확인해 보시기 바랍니다. OAuth 서버 측에서 특정 에러가 발생했을 때도 고려가 필요한지 점검이 필요합니다.
src/test/java/com/evenly/took/feature/auth/api/OAuthIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/evenly/took/feature/auth/api/OAuthIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/evenly/took/feature/auth/application/TokenProvider.java
Show resolved
Hide resolved
src/main/java/com/evenly/took/feature/auth/api/OAuthController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/evenly/took/feature/auth/api/OAuthController.java
Outdated
Show resolved
Hide resolved
- 기존: global > exception > [feature] - 개선: [feature] > exception
- 기존: 여러개의 커스텀 예외 사용 - 개선: 하나의 커스텀 예외 사용
github-actions
bot
added
size/XXL
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (15)
src/main/java/com/evenly/took/feature/auth/application/UuidTokenProvider.java (1)
1-36: 리프레시 토큰 구현이 깔끔하게 작성되었습니다.전체적으로 코드가 잘 작성되었으며, UUID를 사용하여 리프레시 토큰을 생성하고 Redis에 저장하는 방식은 보안적으로 적절한 선택입니다. 몇 가지 고려사항을 제안합니다:
로그아웃 시 토큰 폐기 메커니즘이 보이지 않습니다. 사용자 보안을 위해 로그아웃 시 리프레시 토큰을 Redis에서 제거하는 메서드를 추가하는 것이 좋을 것 같습니다.
getUserId메서드에서 예외 메시지가 "EXPIRED_REFRESH_TOKEN"인데, 이는 토큰이 만료된 경우뿐만 아니라 토큰이 유효하지 않은 모든 경우에 발생합니다. 오류 코드를 더 명확하게 구분하거나 메시지를 보다 일반적인 "INVALID_OR_EXPIRED_REFRESH_TOKEN"으로 수정하는 것을 고려해보세요.Redis에서 값을 가져올 때 null 체크는 잘 구현되었지만, Object를 String으로 변환하는 과정에서 잠재적인 문제가 발생할 수 있습니다. 타입 안전성을 더 확보하기 위해 RedisService에서 타입 파라미터를 활용하는 방법을 고려해보세요.
src/test/java/com/evenly/took/feature/auth/api/TestController.java (1)
1-14: 컨트롤러의 목적을 명확히 설명하는 주석이 필요합니다.이 테스트 컨트롤러는 인증 메커니즘을 테스트하기 위한 것으로 보이지만, 클래스에 목적을 설명하는 주석이 없습니다. 또한 test() 메서드가 빈 구현으로 되어 있어 테스트 목적이 명확하지 않습니다.
@ActiveProfiles("test") @RestController +/** + * 인증 메커니즘 테스트를 위한 컨트롤러입니다. + * 주로 토큰 기반 인증 흐름을 검증하는데 사용됩니다. + */ public class TestController { @GetMapping("/api/test") + /** + * 인증된 요청을 테스트하기 위한 엔드포인트입니다. + * 성공적인 요청은 200 OK를 반환합니다. + */ public void test() { } }src/main/java/com/evenly/took/feature/auth/exception/AuthErrorCode.java (1)
10-23: 에러 메시지 일관성 개선 필요에러 코드의 메시지가 일관성 없이 작성되어 있습니다. 일부는 "~를 찾을 수 없습니다"와 같은 형식이고, 다른 것들은 "~이 만료되었습니다" 형식입니다. 또한 토큰 관련 메시지에서 'token'이라는 영어 단어와 한글을 혼용하고 있습니다.
@Getter @RequiredArgsConstructor public enum AuthErrorCode implements ErrorCode { OAUTH_TYPE_NOT_FOUND(HttpStatus.NOT_FOUND, "OAuth 타입을 찾을 수 없습니다."), JWT_UNAUTHORIZED(HttpStatus.UNAUTHORIZED, "JWT를 찾을 수 없습니다."), - EXPIRED_REFRESH_TOKEN(HttpStatus.UNAUTHORIZED, "refresh token이 만료되었습니다."), - EXPIRED_ACCESS_TOKEN(HttpStatus.UNAUTHORIZED, "access token이 만료되었습니다."), - INVALID_ACCESS_TOKEN(HttpStatus.UNAUTHORIZED, "access token이 유효하지 않습니다."), + EXPIRED_REFRESH_TOKEN(HttpStatus.UNAUTHORIZED, "리프레시 토큰이 만료되었습니다."), + EXPIRED_ACCESS_TOKEN(HttpStatus.UNAUTHORIZED, "액세스 토큰이 만료되었습니다."), + INVALID_ACCESS_TOKEN(HttpStatus.UNAUTHORIZED, "액세스 토큰이 유효하지 않습니다."), ; private final HttpStatus status; private final String message; }src/test/java/com/evenly/took/feature/auth/application/AuthServiceTest.java (1)
37-39: 토큰 검증 로직이 충분하지 않음현재 액세스 토큰 검증은 "." 문자를 포함하는지만 확인하고 있습니다. JWT 토큰의 형식과 구조를 더 엄격하게 검증해야 합니다. 또한 리프레시 토큰에 대한 검증도 단순히 빈 값이 아닌지만 확인하고 있습니다.
// then -assertThat(response.token().accessToken()).containsAnyOf("."); -assertThat(response.token().refreshToken()).isNotBlank(); +assertThat(response.token().accessToken()).matches("^[A-Za-z0-9-_]+\\.[A-Za-z0-9-_]+\\.[A-Za-z0-9-_]+$"); +assertThat(response.token().refreshToken()).matches("^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$"); assertThat(response.user().name()).isEqualTo("took");src/main/java/com/evenly/took/global/auth/AuthenticationFilter.java (1)
30-36: 환경 변수나 설정 파일로 인증 제외 경로를 관리하는 방안을 고려해주세요.
현재 EXCLUDE_PATHS 상수에 인증 제외 경로들을 직접 정의하고 있습니다. 운영 환경이 달라질 경우 이 목록이 자주 변경될 수도 있으니, 환경 변수나 별도의 설정 파일을 통해 주입받으면 유지보수성이 향상될 수 있습니다.src/main/java/com/evenly/took/feature/auth/api/AuthController.java (2)
26-30: 리다이렉트 처리 시 가급적 실제 HTTP 리다이렉트를 고려해주세요.
현재FOUND(302)상태 코드를 사용하지만, 실질적으로는 JSON 바디에 URL을 담아 반환하고 있습니다. 클라이언트가 직접 이 정보를 파싱한 다음 리다이렉트하는 구조라면 200 응답도 가능하니, 목적에 맞는 처리 방식을 재검토해 보세요.
32-36: 로그인 API에 GET 대신 POST 사용을 고려해주세요.
로그인 처리는 일반적으로 POST 요청을 권장합니다. 현재 GET 요청에@RequestParam으로 인증 코드를 전달하고 있어, 브라우저나 서버 로그에 민감 정보가 남을 수 있습니다. 보안 및 RESTful 설계 측면에서 POST 사용을 다시 검토해 보세요.src/test/java/com/evenly/took/feature/auth/client/AuthCodeRequestUrlProviderCompositeTest.java (2)
43-49: 구체적인 예외 클래스를 사용하면 테스트 가독성이 향상됩니다.
과거OAuthTypeNotFoundException대신TookException을 사용하여 예외 범위가 넓어졌습니다. 이 경우 테스트에서 명시적인 의도를 드러내기 어려울 수 있으니, 필요하다면 특정 예외 클래스를 추가 유지하는 방안도 검토해 보세요.
51-62: Mock 클래스를 별도 파일로 추출하면 재사용성과 가독성이 높아집니다.
MockGoogleAuthCodeRequestUrlProvider를 테스트 클래스 내부에 선언해도 동작에는 문제가 없지만, 다른 테스트에서 재사용하거나 가독성을 높이려면 분리해 두는 편이 좋습니다.src/main/java/com/evenly/took/feature/auth/api/AuthApi.java (2)
24-32: 302 리다이렉트 응답에 대한 API 문서가 적절합니다.
302 응답에서 실제로 응답 바디가 필요하지는 않겠지만, SuccessResponse 타입으로 스키마를 표시하여 API 사용자가 참조할 수 있도록 한 점은 이해할 수 있습니다. 다만, 리다이렉트 동작을 문서화할 때는 별도 안내 문구를 추가하는 것도 고려해 보세요.
48-48: refresh API 추가에 대한 TODO
현재 리프레시 토큰이 발급되는데도 별도의 리프레시 엔드포인트가 없어 기능이 불완전할 수 있습니다. 필요 시 추가 구현을 진행하고, 엔드포인트 작동 방식을 문서화하시는 것이 좋겠습니다.src/main/java/com/evenly/took/feature/auth/application/AuthService.java (2)
32-38: loginAndGenerateToken: 사용자 정보 저장 로직 확인
인증 후 사용자 정보가 DB에 없으면 저장하고, 있으면 재활용하는 방식은 일반적인 소셜 로그인 플로우에 부합합니다. 혹시 이메일 중복 검증 등 추가 로직이 필요하다면 관련 예외 처리를 별도 구현하는 것도 고려해 보세요.
40-44: refreshToken: Refresh 토큰을 별도 저장소에 보관하지 않는 점을 유의하십시오.
현재는 request로 받은 Refresh 토큰을 검증 없이 곧바로 새 Access 토큰을 생성하고 있습니다. Refresh 토큰 재사용에 대한 보안 위험(탈취 등)을 낮추기 위해, Redis와 같은 저장소에 토큰을 저장하고, 만료 혹은 블랙리스트 처리를 병행하는 방안을 검토해 보세요.src/main/java/com/evenly/took/feature/auth/application/JwtTokenProvider.java (2)
36-39: generateClaims: 간단한 Claim 구조
현재 subject에 userId만 담고 있는데, 필요한 부가 정보를 추후 추가할 수 있도록 확장성을 고려하면 좋겠습니다.
79-79: accessTokenSecret 보안 관리 주의
민감한 값이므로 운영 환경에서 안전하게 설정/주입되도록 신경쓰시고, 가능하다면 키 로테이션을 고려해 보시기 바랍니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
src/main/java/com/evenly/took/feature/auth/api/AuthApi.java(2 hunks)src/main/java/com/evenly/took/feature/auth/api/AuthController.java(1 hunks)src/main/java/com/evenly/took/feature/auth/api/HeaderHandler.java(1 hunks)src/main/java/com/evenly/took/feature/auth/application/AuthService.java(1 hunks)src/main/java/com/evenly/took/feature/auth/application/JwtTokenProvider.java(4 hunks)src/main/java/com/evenly/took/feature/auth/application/TokenProvider.java(1 hunks)src/main/java/com/evenly/took/feature/auth/application/UuidTokenProvider.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/AuthCodeRequestUrlProvider.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/AuthCodeRequestUrlProviderComposite.java(3 hunks)src/main/java/com/evenly/took/feature/auth/client/UserClient.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/UserClientComposite.java(3 hunks)src/main/java/com/evenly/took/feature/auth/exception/AuthErrorCode.java(1 hunks)src/main/java/com/evenly/took/feature/common/exception/CommonErrorCode.java(1 hunks)src/main/java/com/evenly/took/feature/common/exception/TookException.java(1 hunks)src/main/java/com/evenly/took/global/auth/AuthenticationFilter.java(1 hunks)src/main/java/com/evenly/took/global/exception/GlobalExceptionHandler.java(1 hunks)src/main/java/com/evenly/took/global/exception/auth/oauth/OAuthTypeNotFoundException.java(0 hunks)src/test/java/com/evenly/took/feature/auth/api/AuthIntegrationTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/api/TestController.java(1 hunks)src/test/java/com/evenly/took/feature/auth/application/AuthServiceTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/application/JwtTokenProviderTest.java(3 hunks)src/test/java/com/evenly/took/feature/auth/application/UserClientCompositeTest.java(3 hunks)src/test/java/com/evenly/took/feature/auth/application/UuidTokenProviderTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/AuthCodeRequestUrlProviderCompositeTest.java(3 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/evenly/took/global/exception/auth/oauth/OAuthTypeNotFoundException.java
✅ Files skipped from review due to trivial changes (5)
- src/main/java/com/evenly/took/feature/common/exception/CommonErrorCode.java
- src/main/java/com/evenly/took/feature/auth/client/UserClient.java
- src/main/java/com/evenly/took/feature/auth/client/AuthCodeRequestUrlProvider.java
- src/main/java/com/evenly/took/global/exception/GlobalExceptionHandler.java
- src/main/java/com/evenly/took/feature/common/exception/TookException.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/evenly/took/feature/auth/api/HeaderHandler.java
- src/test/java/com/evenly/took/feature/auth/application/UuidTokenProviderTest.java
- src/main/java/com/evenly/took/feature/auth/application/TokenProvider.java
🔇 Additional comments (22)
src/main/java/com/evenly/took/feature/auth/application/UuidTokenProvider.java (2)
22-27: 리프레시 토큰 생성 로직이 효율적입니다.UUID를 사용한 리프레시 토큰 생성 방식은 예측 불가능성을 보장하여 보안성을 높입니다. 또한 Redis TTL을 사용하여 토큰 만료를 자동으로 처리하는 방식도 좋습니다.
29-35: 토큰 검증에 관한 고려사항현재 구현에서는 토큰이 Redis에 없을 경우 만료된 것으로 간주합니다. 이는 정확하지만, 추가적인 유효성 검사나 다양한 오류 상황을 처리할 수 있는 방법을 고려해보세요. 예를 들어:
- 만료된 토큰과 잘못된 형식의 토큰을 구분하는 것이 도움이 될 수 있습니다.
- Redis에서 값을 가져올 때 발생할 수 있는 예외 처리를 추가하면 더 안정적인 구현이 가능합니다.
src/main/java/com/evenly/took/feature/auth/client/UserClientComposite.java (1)
26-30: 에러 처리 방식이 개선되었습니다.특정 예외(OAuthTypeNotFoundException)에서 일반 예외(TookException)로 변경하여 에러 코드를 활용하는 방식은 일관된 예외 처리를 위한 좋은 접근법입니다.
src/test/java/com/evenly/took/feature/auth/api/AuthIntegrationTest.java (3)
1-35: 통합 테스트 구현이 잘 되어 있습니다.로그인 기능 테스트가 잘 작성되어 있으며, 코드의 흐름이 명확합니다. 테스트에서 로그인 요청 및 응답을 검증하는 부분이 잘 구성되어 있습니다.
37-48: 유효하지 않은 토큰 테스트 케이스가 적절합니다.잘못된 액세스 토큰으로 API를 요청할 때 401 오류를 반환하는지 확인하는 테스트가 적절하게 구현되어 있습니다.
89-110: 유효한 액세스 토큰 테스트가 적절합니다.유효한 액세스 토큰으로 API 요청 시 정상 응답을 반환하는지 검증하는 테스트가 잘 구현되어 있습니다. 토큰 획득 후 테스트 요청이 적절히 구성되어 있습니다.
src/test/java/com/evenly/took/feature/auth/application/JwtTokenProviderTest.java (4)
1-12: 패키지 구조 변경과 의존성 업데이트가 적절합니다.패키지가
auth.jwt에서auth.application으로 변경되었고,JwtProperties에서AuthProperties로 변경된 점이 리프레시 토큰 구현에 맞게 적절히 반영되었습니다.
26-28: AuthProperties 생성자 파라미터가 업데이트되었습니다.리프레시 토큰 만료 시간을 위한 파라미터가 추가되었습니다. 이는 리프레시 토큰 기능 구현에 필요한 적절한 변경입니다.
33-34: 토큰 생성 방식이 변경되었습니다.사용자 객체 대신 사용자 ID 문자열을 사용하여 토큰을 생성하는 방식으로 변경되었습니다. 이는 PR 목표에 언급된 "액세스 토큰 갱신 시 사용자 ID만 사용"하는 개선 사항에 부합합니다.
42-43: 테스트 코드 내 동일 패턴 변경이 일관되게 적용되었습니다.두 번째 테스트 메서드에서도 동일하게 사용자 ID 문자열을 사용하도록 수정되어, 일관성이 잘 유지되었습니다.
src/test/java/com/evenly/took/feature/auth/application/UserClientCompositeTest.java (2)
1-17: 패키지 변경 및 예외 처리 방식의 변경이 적절합니다.패키지가
auth.client에서auth.application으로 변경되었고, OAuth 관련 예외가 더 일반적인TookException으로 통합된 것은 예외 처리 구조를 개선하는 좋은 변경입니다.
49-51: 예외 검증 로직이 일관되게 업데이트되었습니다.예외 유형이
OAuthTypeNotFoundException에서TookException으로 변경되어 전체적인 예외 처리 방식 변경과 일치합니다.src/main/java/com/evenly/took/feature/auth/client/AuthCodeRequestUrlProviderComposite.java (2)
1-14: 패키지 구조 변경 및 예외 처리 방식의 변경이 적절합니다.패키지가
global.security.client에서feature.auth.client로 변경되었고, 이는 인증 관련 코드를 feature 단위로 구조화하는 개선입니다. 또한OAuthTypeNotFoundException대신TookException과AuthErrorCode를 사용하는 방식으로 예외 처리를 통합한 것은 좋은 변경입니다.
27-28: 예외 처리 방식이 일관되게 변경되었습니다.예외 발생 코드가
TookException(AuthErrorCode.OAUTH_TYPE_NOT_FOUND)로 변경되어 애플리케이션 전체의 예외 처리 방식과 일관성이 유지되었습니다.src/main/java/com/evenly/took/global/auth/AuthenticationFilter.java (2)
66-69: 토큰이 누락되었을 때의 처리를 검토해주세요.
headerHandler.resolveAccessToken(request)가 null 또는 빈 문자열을 반환할 수 있는 경우를 고려하여,validateAccessToken호출 전에 별도 처리가 필요할 수 있습니다. 사용자 경험을 위해 명확한 에러 메시지나 에러 코드를 제공하는 방안을 추천드립니다.
71-77: 상태 코드와 메시지 구성에 대한 추가 검토를 권장합니다.
response.setStatus(errorCode.getStatus().value())로 설정된 HTTP 상태 코드와 실제 응답 바디가 의미적으로 일치하는지 점검해 보세요. 예를 들어, 만료된 토큰이지만 401 대신 403 상태를 내보내는 식의 불일치가 없도록 주의가 필요합니다.src/main/java/com/evenly/took/feature/auth/api/AuthApi.java (2)
8-9: AuthResponse와 OAuthUrlResponse를 도입한 점이 적절해 보입니다.
해당 DTO 사용하는 코드를 보면, 로그인 과정에서 필요한 정보(Access/Refresh Token, 리다이렉트 URL 등)를 명확히 분리하여 전달하고 있어 가독성과 유지보수성이 좋아질 것 같습니다.
19-20: 인터페이스 명 변경 및 태그(name = "[1. Auth]") 적용이 명확합니다.
OAuthApi에서 AuthApi로 변경하여 범용적인 인증을 의도하는 것처럼 보이며, Swagger 문서 내 태그 구분도 한층 직관적입니다.src/main/java/com/evenly/took/feature/auth/application/AuthService.java (1)
27-30: getAuthCodeRequestUrl: URL 생성 로직이 간결합니다.
소셜 인증별 URL만 받아 반환하므로, Composite를 통해 공급자별 로직을 캡슐화한 구조가 좋습니다. 필요에 따라 보안 검증 로직을 추가 검토하십시오.src/main/java/com/evenly/took/feature/auth/application/JwtTokenProvider.java (3)
29-33: generateAccessToken: 사용자 정보를 userId만으로 처리해 단순화했습니다.
User 엔티티를 직접 받지 않아도 필요한 식별자만 사용하므로 불필요한 의존성이 줄어듭니다. 이 접근법은 관리 면에서 명확하지만, 사용자 권한(roles 등)을 토큰에 담고 싶다면 추가 설계가 필요할 수 있습니다.
55-56: JWT_UNAUTHORIZED 로깅 후 예외 재발행은 문제 원인 파악에 유용합니다.
단, 민감 정보(토큰 값 등)가 로그에 노출되지 않도록 주의하시기 바랍니다.
60-67: getUserId: 예외 처리가 명확하며, INVALID_ACCESS_TOKEN을 던지는 로직이 일관됩니다.
JWT 검증 실패 시 TookException으로 일원화하여 핸들링하는 방식은 유지보수에 유리해 보입니다.
src/test/java/com/evenly/took/feature/auth/application/AuthServiceTest.java
Show resolved
Hide resolved
src/test/java/com/evenly/took/feature/auth/api/AuthIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/evenly/took/feature/auth/api/AuthIntegrationTest.java
Outdated
Show resolved
Hide resolved
| checkTokens(request); | ||
| filterChain.doFilter(request, response); | ||
| } catch (TookException ex) { | ||
| AuthErrorCode errorCode = AuthErrorCode.EXPIRED_ACCESS_TOKEN; | ||
| sendError(response, errorCode); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
구체적인 예외코드 매핑을 고려해주세요.
TookException을 포착할 때 항상 AuthErrorCode.EXPIRED_ACCESS_TOKEN만 반환하고 있는데, 실제로 만료 외의 다른 원인(유효하지 않은 형식 등)으로도 오류가 발생할 수 있습니다. 예외 인스턴스에 담긴 에러 코드를 기반으로 분기 처리하면 에러 상황을 좀 더 명확히 전달할 수 있습니다.
다음 예시처럼 ex.getErrorCode()를 사용하여 적절한 에러코드를 응답해 보세요:
- } catch (TookException ex) {
- AuthErrorCode errorCode = AuthErrorCode.EXPIRED_ACCESS_TOKEN;
+ } catch (TookException ex) {
+ ErrorCode errorCode = ex.getErrorCode();
sendError(response, errorCode);
}📝 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.
| checkTokens(request); | |
| filterChain.doFilter(request, response); | |
| } catch (TookException ex) { | |
| AuthErrorCode errorCode = AuthErrorCode.EXPIRED_ACCESS_TOKEN; | |
| sendError(response, errorCode); | |
| checkTokens(request); | |
| filterChain.doFilter(request, response); | |
| } catch (TookException ex) { | |
| ErrorCode errorCode = ex.getErrorCode(); | |
| sendError(response, errorCode); |
src/main/java/com/evenly/took/feature/auth/api/AuthController.java
Outdated
Show resolved
Hide resolved
github-actions
bot
added
size/XXL
github-actions
bot
added
size/XXL
github-actions
bot
added
size/XXL
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
src/test/java/com/evenly/took/feature/auth/api/AuthIntegrationTest.java (3)
25-35: 테스트 응답 본문도 함께 검증해 보세요.
현재 상태코드(200)만 검증하고 있습니다. 성공 응답 시body()항목에 담기는 유저 정보나 토큰 정보도 함께 검증하면 회귀 테스트에 유용합니다.given().log().all() .when().post("/api/auth/login/KAKAO?code=code") .then().log().all() .statusCode(200) + .body("data.userName", equalTo("took")) // 예시로 응답 본문 검증 ;
50-72: 토큰 갱신 성공 시, 새로 발급된 Access Token을 추가 검증해 보세요.
현재 상태코드(200) 확인만으로는 새 토큰이 정상 발급되었는지 판단하기 어렵습니다. 갱신된 토큰 값이나 만료 시간 등을 함께 검증하면 신뢰도를 높일 수 있습니다.
89-109: 성공 케이스에서도 응답 본문 검증을 권장합니다.
200 응답 외에 발급된 Access Token으로 실제 권한이 적용되었는지 서버 측 응답 본문 구조를 체크하면 좋겠습니다. 예:body("data.userName", ...)src/main/java/com/evenly/took/feature/auth/api/AuthController.java (2)
33-37: 로그인 API에서 응답 상태를 명시적으로 반환하세요.
기본적으로SuccessResponse.of(response)는 200을 반환하지만, 로그인 성공 시 200인지 201인지 명확히 지정하는 것도 가독성에 좋습니다.
39-43: Refresh 토큰 재발급 시 예외 케이스에 대한 구체적 status 반환도 고려하세요.
현재SuccessResponse.of(response)만 사용하면 401 등 에러 상황에서 일관된 예외 반환 구조를 가지기 어려울 수 있습니다. 에러 발생 시 적절한 status code와 에러 메시지 구조를 통일하는 방안을 검토해 보세요.src/main/java/com/evenly/took/feature/auth/api/AuthApi.java (2)
28-38: 리다이렉트 API에서 302 반환 시 실제 리다이렉트 처리나 Location 헤더를 활용하는 방법이 더 안전합니다.
현재는SuccessResponse<OAuthUrlResponse>형태로 결과를 넘기면서 302를 문서화하고 있으나, 일부 클라이언트가 이에 대응하지 못하면 문제가 생길 수 있습니다.
52-63: 401 에러에 대한 에러 바디 구조를 좀 더 구체적으로 설명해 주세요.
@ApiResponses에서 401 에러가 발생할 때 어떤 필드(예:errorCode,message)가 반환되는지 명시하면, 클라이언트 개발자가 대응 로직을 구성하기 더 수월합니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/evenly/took/feature/auth/api/AuthApi.java(1 hunks)src/main/java/com/evenly/took/feature/auth/api/AuthController.java(1 hunks)src/test/java/com/evenly/took/feature/auth/api/AuthIntegrationTest.java(1 hunks)
🔇 Additional comments (2)
src/test/java/com/evenly/took/feature/auth/api/AuthIntegrationTest.java (1)
37-48: 오류 내용을 명확히 확인하기 위해 응답 본문도 체크할 것을 권장합니다.
401 상태코드만 검사하면 충분히 이유를 추적하기 어려울 수 있습니다. 에러 메시지나 코드가 예상대로 전달되는지도 함께 검증하면 디버깅이 용이합니다.src/main/java/com/evenly/took/feature/auth/api/AuthApi.java (1)
39-50: GET 대신 POST 방식으로 토큰 발급을 진행하도록 수정된 점은 적절합니다.
이전 코멘트들에서 지적된 GET 메서드 사용 문제(쿼리 파라미터 노출, 캐싱 위험 등)를 해소했으며, 보안 측면에서 더욱 안전해 보입니다.
issue #12
📌 개발 이유
💻 수정 사항
기존 코드에서의 수정사항
로그인 시 반환되는 response 클래스에 사용자 정보를 삽입하였습니다.
Refresh Token의 경우 JWT가 아닌 UUID로 생성하도록 하였기 때문에, 기존 코드에서 인증 관련 클래스명이 JwtXXX로 사용되고 있는 경우 조금 더 범용적인 이름(AuthXXX)으로 변경하였습니다.
JWT 생성 시 name 없이 id만 활용하는 것이 어떤가 하여 일단 코드 수정해두었습니다. (새벽이라 의견을 물을 수가 없었어요..) 해당 방법을 제안하는 이유는 아래와 같아요. 관련해서 의견 주시면 좋을 것 같습니다!
기존 filter에서 헤더값을 추출하던 코드를 HeaderHandler로 이동시켰습니다. (제 코드 작성하면서 같이 이동시켜 보았어요!)
WebSecurityConfig에서 /api 경로 하위는 모두 열어두었습니다.
기존 filter에서 401 응답을 반환해도 Spring Security의 여러 filter를 거치면 403이 반환되는 것을 확인하였습니다. 따라서 WebSecurityConfig에 에러 핸들링 관련 코드 추가하였습니다.
🧪 테스트 방법
⛳️ 고민한 점
🎯 리뷰 포인트
아래 내용은 의견을 먼저 묻고 모두 동의하시면 제가 수정하는 방향으로 진행하겠습니다!
new TookException()에 ErrorCode를 삽입할 때 static import 하지 않는 것이 어떨지 제안드립니다.📁 레퍼런스
Summary by CodeRabbit
신규 기능
개선 사항
테스트