feature/#49 oauth2 kakao, google social login 구현 #54
Conversation
|
Warning Rate limit exceeded@minjo-on has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 26 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
Walkthrough이 풀 리퀘스트는 OAuth2를 사용하여 카카오와 구글 로그인 기능을 추가합니다. 새로운 서비스, 설정, 예외 처리 클래스들이 도입되었으며, 기존 인증 시스템에 소셜 로그인 기능을 통합했습니다. 사용자 정보 수집, 토큰 생성, 보안 설정 등 다양한 측면에서 애플리케이션의 인증 메커니즘을 확장했습니다. Changes
Assessment against linked issues
Poem
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
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: 6
🧹 Nitpick comments (31)
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/UserPrincipal.java (3)
22-23: 필드에 대한 기본 검증 로직을 고려해주세요
user와idToken필드가null일 가능성이 있는지 확인 후, 필요한 경우@NonNull등의 애노테이션이나 추가적인 예외 처리로 NPE를 방지할 수 있습니다.
30-35: 정적 팩터리 메서드에서의 인자 유효성 검사 권장
UserPrincipal.create(user, idToken)에서user나idToken이null인 경우를 미리 체크하는 로직을 추가하면, 이후의 NPE나 불안정한 상태를 사전에 방지할 수 있습니다.
58-60: 권한 리스트 확장 가능성 고려
현재getAuthorities()가 하나의 권한만 부여하고 있습니다. 향후 다양한 권한(예: 여러 Role 지원)이 필요할 수 있으므로, 리스트 확장이나 추상화 구조를 고려해주세요.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/handler/OicdSuccessHandler.java (1)
26-27: Refresh 토큰 보안 강화 제안
사용자 식별 정보가 담긴 Refresh 토큰을 그대로 클라이언트 측에 전달하면 탈취 위험이 있습니다. Refresh 토큰은 일반적으로HttpOnly설정이 적용된 쿠키에 보관하거나, 별도의 저장소(예: DB, Redis)를 통해 관리하는 방안을 검토해 보시는 것이 좋습니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcExceptionCode.java (1)
15-18: 에러 메시지 국제화(I18n) 적용 고려 필요
외부 파일(메시지 프로퍼티 등)을 사용하여 다국어 지원이 가능하도록 에러 메시지를 분리하는 방안을 검토해 보세요.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcInvalidIssuerException.java (2)
3-3: [주의] Static import 사용 시 유지보수성 확인 필요
OIDC_INVALID_ISSUER를 바로 호출하기 위해 static import를 사용하는 것은 편리하지만, 해당 상수나 클래스의 변경이 발생할 경우 전체 코드 영향 범위를 주의 깊게 살펴야 합니다.
7-11: [권장] 예외 클래스에 추가 생성자를 고려해 보세요
현재 기본 생성자만 존재하므로, 예외 메시지나Throwable cause를 직접 전달할 수 있는 생성자를 함께 제공하는 것이 디버깅 및 예외 처리에 유용할 수 있습니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcInvalidAudienceException.java (1)
7-11: CustomException 상속 시 직렬화 여부를 고려해 보세요.
Java 직렬화가 필요한 상황이라면serialVersionUID필드를 명시적으로 선언하는 것이 좋습니다. 또한 예외 처리 시 사용자 정의 메시지나 다양한 인자를 받고 싶다면 생성자 오버로딩도 고려해 볼 수 있습니다.soridam-api/src/main/java/sorisoop/soridam/api/auth/application/AuthService.java (2)
24-25: 의존성 주입
KakaoOidcService, GoogleOidcService 의존성이 추가되었습니다. OIDC 인증 과정에서 발생할 수 있는 다양한 예외 상황(HTTP 통신 실패, 잘못된 토큰 등)에 대비한 로깅·예외 처리가 필요해 보입니다.
46-50: googleLogin 메서드
카카오 로직과 유사한 흐름을 구성하였으므로, 중복되는 부분이 많다면 공통화(리팩토링)를 추진해도 좋겠습니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/CustomOidcUserService.java (2)
23-36: loadUser 메서드
부모 클래스의loadUser를 확장하여 provider와 사용자 정보를 매핑하는 로직이 깔끔해 보입니다. 다만, provider 판단 로직에서 예외가 발생할 경우 로그나 사용자 알림이 충분한지 점검이 필요합니다.
53-60: updateUserInfo 메서드
프로필 이미지 변경을 감지하여 업데이트하는 방식이 효율적이며, 로그인 시간도 함께 갱신함으로써 사용자 활동 이력을 주기적으로 관리할 수 있습니다. User 도메인 내부에서 중복 코드를 줄이는 방안도 고민해 볼 수 있습니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/OidcService.java (5)
23-25: buildDecoder 메서드
JWK Set URI를 통한NimbusJwtDecoder생성 활용이 적절합니다. 네트워크 이슈나 URI 변경 등에 대비해 캐싱 또는 예외 처리 로직도 고려하시면 좋겠습니다.
27-42: validateAndDecodeIdToken 메서드
토큰 검증 과정 중 모든 예외를 RuntimeException으로 통합 처리하고 있습니다. 상황별로 상세 예외를 분리하면 디버깅 및 에러 추적이 더 수월해질 수 있습니다.
51-55: validateIssuer 메서드
토큰 발급자의 일치 여부를 확인해 불일치 시 예외 발생하도록 한 로직이 적합합니다. 추후 issuer가 여러 개인 상황을 허용해야 하는 경우 확장성을 검토해 보셔야 합니다.
57-61: validateAudience 메서드
토큰의 audience 목록 중getClientId()가 없으면 예외를 발생시키는 방식이 직관적입니다. 다만, 여러 클라이언트 혹은 환경을 지원해야 한다면 확장 방안을 마련할 필요가 있습니다.
63-77: validateExpiration 메서드
Instant와 Long 타입을 모두 처리하고, 만료 여부를 명확히 판별해OidcExpiredException을 던지도록 한 부분이 좋습니다. 만료 처리 정책이 복잡해질 경우를 대비해 별도 확장 포인트를 두는 것도 고려해 보세요.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcProperties.java (2)
8-9: Lombok 사용
@Getter와@Setter애노테이션을 통한 롬복 사용으로 보일러플레이트 코드를 줄이는 것은 좋은 선택입니다. 가능하다면 생성자나 빌더를 통해 필드 불변성을 유지할 수 있는지 한 번 고려해보시는 것을 제안드립니다.
10-15: OIDC 파라미터 사용성 검토
jwkSetUri,issuer,clientId는 외부 설정에 따라 동적으로 바뀔 가능성이 높으므로, 테스트 프로필 및 운영 프로필에서 각각 다른 값이 유입되는지 검증이 필요합니다. 예외 처리나 유효성 검증 로직을 추가하여 틀린 설정으로 인해 발생할 수 있는 문제를 최소화할 것을 권장드립니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcProperties.java (1)
8-9: Lombok 사용
@Getter와@Setter만으로도 기본적인 접근자/설정자 자동 생성을 충분히 활용하고 있습니다. 불변성을 강화하기 위해 생성자나 빌더 패턴 고려를 제안드리며, 실수로 인한 프로퍼티 수정이 발생하지 않도록 주의해주세요.soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/Provider.java (3)
1-4: 패키지 및 임포트 상태 확인
Provider열거형이 사용자 도메인 내에서 직접 관리되는 것은 읽기 편의성과 유지 보수에 유리합니다. 다만, 프로젝트 확장 시 다른 소셜 로그인이 많은 경우, 별도의 모듈화나 관리 전략을 고려하는 것도 좋은 방법입니다.
9-14: 열거형 정의 및 필드 구성
attributeKey,providerCode,identifier필드로 각 플랫폼마다 필요한 정보를 모아둔 구조가 알기 쉽습니다. 필요 시, 필드 명칭을 좀 더 명확하게 수정(예:claimsKey,uniqueCode등)하는 방안도 고려해보세요.
20-28: 문자열로부터Provider를 취득하는 메서드
대소문자 차이가 있는 문자열 입력을 모두 대응하기 위해toUpperCase()적용이 적절합니다. 추후에 프로바이더가 추가될 경우,orElseThrow로 발생하는ProviderNotFoundException메시지를 상세히 제공하여 디버깅 편의성을 높여주세요.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OicdUserFactory.java (2)
21-28: [KO] GOOGLE 영역에서 속성 검증 로직 보강 제안
Google OIDC 응답 형식이 변경되거나 특정 필드를 제외해 응답할 경우를 대비하여, 속성 조회 시null가능성에 대한 처리나 예외 대응 방식을 고려해주세요.
30-30: [KO] 예외 처리 계층 분리 제안
ProviderNotFoundException를 던지고 있으나 필요 시 각 공급자별 세분화된 예외 처리를 통해 문제 원인을 명확히 파악할 수 있도록 구조를 개선해볼 수 있습니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcService.java (1)
36-39: [KO] claims 필드명 상이 발생 가능성 주의
카카오 응답에서sub,nickname,picture키들이 변경될 가능성이 있으므로, kakaoOidcProperties 또는 별도 상수를 사용해 문자열 키를 관리하는 방안을 고려해보세요.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java (1)
36-41: [KO] 불필요한 ClassCastException 방지
claims.get("sub")등 형변환 시 의도치 않은 예외가 발생할 수 있으므로,instanceof검사 후 변환 또는String.valueOf활용 등을 고려해주세요.soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java (1)
42-42: 필드명 카멜 케이스 권장
OAuthIdentity대신oauthIdentity등 카멜 케이스로 변경하는 것이 자바 표준 컨벤션에 맞습니다.- private String OAuthIdentity; + private String oauthIdentity;soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/JwtProvider.java (1)
51-51: 역할 정보 클레임 저장 형식
.claim("role", role.name())로 역할을 추가하는 방식은 합리적입니다. 다만, “ROLE_” 접두어가 필요한 경우 Security 설정에 맞추어 고려가 필요합니다.soridam-auth/src/main/java/sorisoop/soridam/auth/config/SecurityConfig.java (1)
65-70: OAuth2 로그인 설정 시도
소셜 로그인 후처리를 위한successHandler와oidcUserService등록이 잘 이루어졌습니다. 실패 핸들러가 별도로 필요한 경우에는 추가 고려 바랍니다.soridam-api/src/main/resources/application-oicd.yml (1)
26-33: 파일 끝에 새 줄 추가 필요
static analysis 도구(yamllint)에서 보고된 바와 같이, 파일 마지막에 newline이 없습니다. YAML 포맷 표준 및 Git 관리 관행을 위해 파일 끝에 newline을 추가해 주세요.+# 파일 끝에 공백 줄 추가🧰 Tools
🪛 yamllint (1.35.1)
[error] 33-33: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
soridam-api/src/main/java/sorisoop/soridam/api/auth/application/AuthService.java(1 hunks)soridam-api/src/main/java/sorisoop/soridam/api/auth/presentation/AuthApiController.java(3 hunks)soridam-api/src/main/resources/application-oicd.yml(1 hunks)soridam-api/src/main/resources/application.yml(2 hunks)soridam-auth/build.gradle(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/config/OidcConfig.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/config/SecurityConfig.java(4 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/JwtAuthenticationEntryPoint.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/JwtProvider.java(3 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/CustomOidcUserService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OicdUserFactory.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/UserPrincipal.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcExceptionCode.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcExpiredException.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcInvalidAudienceException.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcInvalidIssuerException.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcProperties.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/OidcService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/handler/OicdSuccessHandler.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcProperties.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/request/OidcLoginRequest.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/Provider.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/Role.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java(4 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/exception/ProviderNotFoundException.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/exception/UserDomainExceptionCode.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/infrastructure/JpaUserRepository.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- soridam-auth/src/main/java/sorisoop/soridam/auth/config/OidcConfig.java
🧰 Additional context used
🪛 Gitleaks (8.21.2)
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/request/OidcLoginRequest.java
9-9: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 yamllint (1.35.1)
soridam-api/src/main/resources/application-oicd.yml
[error] 33-33: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (45)
soridam-auth/build.gradle (1)
18-18: 새로운 도메인 모듈 종속성 추가 확인 요청
soridam-domain프로젝트 모듈이 새로 추가됨에 따라 기존 모듈 내 클래스 간 참조 및 빌드 과정에서 호환성이 필요한지 확인이 필요합니다.
추가된 종속성을 통해 내부 모델과 로직이 올바르게 연결되는지, 테스트에서 문제 없이 작동하는지 검증 부탁드립니다.soridam-domain/src/main/java/sorisoop/soridam/domain/user/exception/UserDomainExceptionCode.java (1)
17-17: 새로운 예외 Enum 추가에 대한 의견새로운 예외 값
PROVIDER_NOT_FOUND를 추가하여 잘못된 소셜 로그인 제공자에 대해 명확한 예외 처리를 제공하는 것은 적절합니다. 해당 예외 코드를 활용한 일관된 예외 처리가 가능해졌으니, 다른 OAuth2 연동 부분에서도 통일성을 유지해 주시기 바랍니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/UserPrincipal.java (2)
26-28:getName()메서드 반환값에 대한 검증
현재getName()메서드는user.getId()를 직접 반환합니다. 혹시User엔티티의id가null이거나 예상치 못한 값을 가질 수 있는지 확인이 필요합니다. 로그인이나 권한부여 로직에서 문제가 발생할 여지가 없는지 검토해주세요.
37-40:idToken이null일 경우 예외 처리
getClaims()메서드는idToken.getClaims()를 직접 반환합니다.idToken이null이라면 예상치 못한 예외가 발생할 수 있으므로, 안전한 반환 처리가 필요할 수 있습니다.soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/JwtAuthenticationEntryPoint.java (3)
9-9: 프로젝트 로깅 기능에 Lombok 적용이 적절합니다
Lombok을 통해 로깅 기능을 간결하게 사용할 수 있어 유지보수성이 향상됩니다.
12-12: @slf4j 애너테이션을 통한 간결한 로깅 구현
필드 선언 없이 간편하게 log 객체를 사용할 수 있어 코드 가독성과 유지보수성에 이점이 있습니다.
18-18: 로그에 민감한 정보가 노출되지 않도록 주의하세요
예외 메시지에 보안 민감 정보가 포함될 수 있음을 유의해야 합니다. 특히 인증 토큰이나 사용자 정보 등이 로그에 출력되지 않도록 검토가 필요합니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/handler/OicdSuccessHandler.java (2)
29-29: Access 토큰 보안을 위한 표준 헤더 권장
Access 토큰을Authorization헤더에 담는 구현은 표준(OAuth2 컨벤션)에 부합합니다. 다만, 이후의 요청에서 어떠한 인증 스킴이 적용되는지 일관성을 유지하고, CORS 정책 또는 HTTPS 환경에서 안전하게 전송되는지 확인하는 것이 좋습니다.
21-31: 로그인 후 리다이렉트와 예외 처리 확인 필요
현재는 성공 시 단순히 토큰을 헤더에 추가하지만, 사용자가 인증 후 이동할 페이지나 추가 정보가 필요한 경우가 있을 수 있습니다. 예외 케이스나 추가 로직(로그 저장, 알림 등)에 대한 처리가 필요한지 검토해주세요.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcExceptionCode.java (1)
20-21: 필드 네이밍 및 접근성 적절성 검토
status와message필드는 적절히 네이밍되었으며, 접근 수준도 문제없어 보입니다. 현재 구현 상태 유지에 동의합니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcExpiredException.java (2)
3-4: 정적 임포트 사용을 권장하는 이유
OidcExceptionCode에서 정의된 상수를 직관적으로 활용하고 있어 코드 가독성이 좋습니다. 유지보수성도 향상되니 현 방식을 추천드립니다.
7-11: OIDC 만료 예외 클래스 도입
CustomException을 상속받아 만료 관련 오류를 명확히 처리하는 구조가 깔끔합니다. 필요하다면 메시지나 cause를 인자로 받는 추가 생성자를 마련해 확장성을 높이는 것도 고려해보세요.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcInvalidAudienceException.java (2)
1-2: 패키지 선언이 적절하게 이루어졌습니다.
새로운 예외를 의도한 위치에 깔끔하게 정의하여 유지보수성이 향상되었습니다.
3-4: 정적 import 사용으로 가독성이 향상되었습니다.
해당 상수를 다른 클래스명 없이 직접 사용할 수 있어 코드가 더욱 직관적입니다.soridam-api/src/main/java/sorisoop/soridam/api/auth/application/AuthService.java (5)
14-16: kakaoOidcService/GoogleOidcService import 추가
OAuth 인증 기능을 위해 필요한 의존성이 추가되었습니다. 현재 코드 단에서 문제는 없어 보이며, 서비스 구현체에서의 예외 처리를 꼼꼼히 점검해 보시기 바랍니다.
20-20: @transactional 전체 적용
클래스 레벨에 @transactional을 적용함으로써, 메서드 전반에서 DB 변경을 안전하게 처리할 수 있습니다. 트랜잭션 범위가 넓어지므로 의도치 않은 부작용이 없는지 확인해 보세요.
35-38: 사용자 마지막 로그인 시간 업데이트
user.updateLastLoginTime()메서드를 로그인 시점에 호출하여 사용자의 접속 기록을 관리하고, 이후 즉시 토큰 발급을 진행합니다. 로직의 순서가 자연스러우며 문제 없어 보입니다.
40-44: kakaoLogin 메서드
kakaoOidcService.processLogin(idToken)을 통해 사용자 정보를 불러온 뒤, 마지막 로그인 시간을 갱신하여 토큰을 발급합니다. 예외 발생 시점(토큰 만료, 발급 실패 등)에 대한 로그 확인과 재시도 로직도 고려할 수 있겠습니다.
52-54: getToken 메서드
Refresh 토큰(7일)과 Access 토큰(2시간)을 분리 발급해 보안성과 편의성을 모두 고려한 점이 좋습니다. 만료 기간이 실제 요구사항과 정합하는지 재확인해 보세요.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/CustomOidcUserService.java (2)
1-19: 새 클래스 CustomOidcUserService
OidcUserService를 확장하여 OIDC 사용자 정보를 커스텀 처리하는 방식이 일관성 있습니다. @transactional 범위와 예외 처리가 적절하게 구성되어 있는지 살펴보시면 좋겠습니다.
38-51: getUserAndUpdateIfNeeded 메서드
이미 존재하는 사용자를 갱신하고, 없으면 새로 생성하는 로직이 명확합니다. attributes에서 필요한 키가 누락될 수 있는 상황에 대비해 null 체크를 철저히 하는지 확인해 보세요.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/OidcService.java (2)
17-22: 추상 클래스 OidcService 정의
OIDC 인증에서 필요한 핵심 로직(issuer, jwkSetUri, clientId)을 추상 메서드로 두어, 서비스별 구현 부를 명확히 분리했습니다. 유지보수와 확장성 측면에서 바람직해 보입니다.
44-49: extractUserFromClaims 및 processLogin 메서드
클레임으로부터 사용자 정보를 추출하는 부분을 추상화하고, 실제 로직에서 구현하게끔 한 점이 구성이 깔끔합니다. claim 값이 예상치 못한 형식이 될 경우를 대비해 방어 로직 추가를 고려해 보세요.soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/Role.java (1)
6-15: 새 Enum Role
관리자, 유료 사용자, 일반 사용자에 따른 구분과 한글 설명이 명확하여 코드 가독성이 좋습니다. 향후 역할이 확장될 가능성이 있다면, enum의 변경으로 인한 영향 범위를 사전에 검토해 두면 좋겠습니다.soridam-domain/src/main/java/sorisoop/soridam/domain/user/exception/ProviderNotFoundException.java (1)
7-11: 새 예외 클래스 ProviderNotFoundException
존재하지 않는 Provider가 주어졌을 때 명확한 예외를 던지는 형태로, 문제 파악이 쉬워집니다. 에러 메시지를 세밀하게 구성해 사용자나 API 소비자에게 정확한 정보를 줄 수도 있습니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcProperties.java (1)
1-4: 카카오 OIDC 프로퍼티 패키지 및 임포트 확인
패키지 및@ConfigurationProperties관련 임포트 구성이 적절합니다. OIDC 속성 바인딩 시'oidc.kakao'프리픽스를 인식할 수 있도록 Spring Boot 설정(@EnableConfigurationProperties혹은@ConfigurationPropertiesScan)에서도 해당 클래스를 등록했는지 확인 부탁드립니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcProperties.java (1)
1-4: 구글 OIDC 프로퍼티 패키지 및 임포트 확인
패키지명과@ConfigurationProperties애노테이션 설정이 적절해 보입니다. 다만, 환경 설정 시'oidc.google'프리픽스 하에서 올바르게 값이 매핑되는지 재확인 부탁드립니다.soridam-domain/src/main/java/sorisoop/soridam/domain/user/infrastructure/JpaUserRepository.java (2)
7-7:Provider임포트 추가
OAuth Provider 정보를 구분하기 위해Provider열거형을 임포트한 것은 적절해 보입니다. 동일한 방식으로 다른 부분에서도Provider를 일관성 있게 사용할 수 있도록 검토해주세요.
13-13: OAuth 사용자 조회 메서드 추가
findByOAuthIdentityAndProvider메서드를 통해 OAuth 식별자와 공급자를 함께 식별하는 로직이 잘 드러납니다. 단,Optional반환값 처리 시, 불필요한 예외가 발생하지 않도록 적절한 대응 로직(예: orElseThrow, orElse 등)을 구현해 주세요.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OicdUserFactory.java (2)
12-12: [KO] 메서드 시그니처 명확성 검토
getOAuth2UserInfo메서드는 OAuth2 로그인 시 필요한 사용자 정보를 생성하는 역할을 수행합니다. 메서드명과 동작이 잘 부합하므로 가독성이 좋습니다.
14-19: [KO] KAKAO 영역에서 속성 누락 가능성 주의
Kakao 측에서 전달되는 속성 중sub,nickname,picture가 무조건 존재한다고 가정하지만, 공급자 측 변경이 발생하거나 일부 필드가 null로 들어올 수 있습니다. 사용 전 null 체크 방안을 고려해보세요.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcService.java (1)
41-45: [KO] orElseGet 패턴 활용 적절
findByOAuthIdentityAndProvider의 결과가 없을 때만 새 사용자를 생성하여 저장하는 로직이 명확합니다. 기존 사용자와 새 사용자를 분리 관리하는 구조가 잘 설계되었습니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java (1)
42-47: [KO] 리포지토리 접근 구조 간결화
orElseGet블록에서 사용자 생성 후 바로save를 호출하는 구조가 간결하며, 확장성도 좋습니다. 새로운 필드를 추가해야 할 경우, 동일한 구조로 쉽게 적용 가능합니다.soridam-api/src/main/java/sorisoop/soridam/api/auth/presentation/AuthApiController.java (2)
17-17: OIDC 요청 객체 Import 확인
이OidcLoginRequest임포트는 정상적으로 사용되고 있으며, 별다른 문제가 없습니다.
30-30: 엔드포인트 URL 변경 주의
/jwt/login에서/login으로 변경됨에 따라, 프런트엔드 혹은 외부 연동부의 요청 경로 수정이 필요한지 확인해주세요.soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java (2)
116-118: 마지막 로그인 시각 업데이트 로직
updateLastLoginTime()메서드는 단순 시간 기록 수행으로 적절합니다. 별도의 문제가 없습니다.
120-126: 프로필 정보 갱신 메서드 구조
updateProfileImageUrl와updateNickname는 동일한 형식으로 작성되고 있어, 확장 가능성이 열려 있습니다. 문제 없이 사용 가능합니다.soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/JwtProvider.java (1)
36-38: JWT 토큰 생성 시 사용자 권한 추가
Role정보를 추가로 받아서 토큰을 생성하는 로직은 유용해 보입니다. 별다른 문제 없이 동작할 것으로 보입니다.soridam-auth/src/main/java/sorisoop/soridam/auth/config/SecurityConfig.java (3)
38-39: OAuth2 성공 핸들러와 OIDC 사용자 서비스 주입
OicdSuccessHandler와CustomOidcUserService필드 주입이 정상적으로 확인되었습니다. 별다른 문제 없습니다.
58-58: OAuth2 요청 경로 허용
OAUTH2_PATTERNS에 매칭되는 요청들을 허용하도록 설정하여 소셜 로그인을 지원하는 로직이 적절합니다.
97-101: OAuth2 경로 배열 추가
OAuth2 리다이렉트 및 인증 요청 경로 정의가 적절합니다. 추후 운영 정책에 따라 세분화할 수 있습니다.soridam-api/src/main/resources/application.yml (3)
2-3:application-oicd.yml파일명 확인 권장
import: application-oicd.yml설정이 실제로 필요한 맞춤 파일명인지 확인이 필요합니다. “OIDC” 표기가 일반적인데 이 부분이 오타(OICD vs OIDC)인지 검토해 주세요.
36-36: JWT 비밀 키 환경 변수 설정 점검
${JWT_SECRET_KEY_STRING}환경 변수가 실제로 정의되고 있는지 점검해 주세요.
38-46: Kakao/Google OIDC 설정 추가 확인
카카오와 구글에 대한issuer,jwk-set-uri등 필드가 정상적으로 적용된 것으로 보입니다. 환경 변수(KAKAO_CLIENT_ID,GOOGLE_CLIENT_ID)도 알맞게 설정되어 있는지 확인해 주세요.soridam-api/src/main/resources/application-oicd.yml (1)
1-25: OAuth2 Client Registration 설정 검토
카카오와 구글에 대한client-id,client-secret,scope등 설정이 잘 구성되어 있습니다. 단, 민감 정보인client-secret이 환경 변수로 관리되는지 다시 한번 확인 부탁드립니다.
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcProperties.java
Outdated
Show resolved
Hide resolved
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/request/OidcLoginRequest.java
Outdated
Show resolved
Hide resolved
soridam-api/src/main/java/sorisoop/soridam/api/auth/presentation/AuthApiController.java
Outdated
Show resolved
Hide resolved
soridam-api/src/main/java/sorisoop/soridam/api/auth/presentation/AuthApiController.java
Outdated
Show resolved
Hide resolved
soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java
Outdated
Show resolved
Hide resolved
soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java (4)
34-49: 예외 처리를 더욱 명확하게 분리하는 것을 고려해보세요.현재
validateAndDecodeIdToken에서 발생하는 예외를 모두RuntimeException으로 감싸 재Throw하고 있습니다. 이미OidcInvalidIssuerException,OidcInvalidAudienceException,OidcExpiredException등을 정의해두셨으므로, 가능한 경우에는 해당 예외를 직접 노출하거나 좀 더 구체적인 예외를 던지도록 하여 디버깅과 에러 추적을 용이하게 만들 수 있습니다.
68-82:validateExpiration메서드의 로직 통합을 고려해보세요.
Instant와Long두 가지 케이스를 분기 처리하고 있지만, Token 표준에서exp는 일반적으로 UNIX Timestamp 형식으로 들어옵니다. 내장 라이브러리를 활용해 만료 시간을 통일성 있게 검증할 수 있는지 검토해보세요.
84-96: 동시성(Concurrency) 이슈를 확인해보세요.
findOrCreateUser메서드에서 동일한 식별자(identifier)를 가진 요청이 동시에 들어올 경우, 중복 사용자가 생성될 가능성이 있습니다. JPA의 고유 컬럼 설정이나 DB 레벨의 유니크 제약 조건으로 이런 문제를 예방할 수 있으니, 추가 검토를 권장드립니다.
57-60:validateIssuer와validateAudience메서드에서 로깅을 추가해보세요.발급자나 클라이언트 식별자가 불일치할 경우 바로 예외만 발생시키는데, 원인 파악을 위해 검증 실패 시점에 디버그 혹은 정보성 로그를 남겨두는 것이 운영 환경에서 도움이 될 수 있습니다.
Also applies to: 62-66
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
soridam-api/src/main/java/sorisoop/soridam/api/auth/presentation/AuthApiController.java(3 hunks)soridam-api/src/main/resources/application.yml(2 hunks)soridam-auth/build.gradle(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OicdUserFactory.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/request/OidcLoginRequest.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/Provider.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/request/OidcLoginRequest.java
- soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/Provider.java
- soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcService.java
- soridam-auth/build.gradle
- soridam-api/src/main/java/sorisoop/soridam/api/auth/presentation/AuthApiController.java
- soridam-api/src/main/resources/application.yml
- soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OicdUserFactory.java
- soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java
🔇 Additional comments (3)
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java (2)
1-16: 클래스 구조가 잘 구성되어 있습니다!Spring Service 컴포넌트로 적절히 설정되어 있으며, 필요한 의존성들이 올바르게 import 되어 있습니다.
24-42: OIDC 제공자 메서드들이 깔끔하게 구현되었습니다!Provider enum을 활용한 구현이 잘 되어있으며, 모든 필수 메서드들이 적절하게 오버라이드 되어 있습니다.
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java (1)
30-33: JWK Set URI 구성 검증 방법을 확인해보세요.
buildDecoder에서 단순히jwkSetUri를 활용하고 있는데, URI가 유효하지 않은 경우 예외 처리가 적절히 이뤄지는지 또는 보완이 필요한지 검토해보세요.
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java
Outdated
Show resolved
Hide resolved
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/CustomOidcUserService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java(4 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/infrastructure/JpaUserRepository.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- soridam-domain/src/main/java/sorisoop/soridam/domain/user/infrastructure/JpaUserRepository.java
- soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/CustomOidcUserService.java
- soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java
🔇 Additional comments (1)
soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java (1)
95-103: Kakao 사용자 생성 시 이메일 처리 로직 추가 검토Google OAuth와의 일관성을 위해 Kakao 사용자 생성 시에도 이메일 정보를 처리하는 것이 좋습니다. Kakao API에서 이메일 정보를 제공받을 수 있다면, 이를 활용하여 사용자 데이터를 더 완성도 있게 구성할 수 있습니다.
soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java
Outdated
Show resolved
Hide resolved
| public void updateNickname(String nickname) { | ||
| this.nickname = nickname; | ||
| } |
There was a problem hiding this comment.
닉네임 업데이트 시 중복 검사 로직 필요
nickname 필드에 @Column(unique = true) 제약 조건이 있으므로, 닉네임 업데이트 시 중복 검사가 필요합니다. 이를 위해 UserRepository를 통한 검증 로직을 추가하는 것이 좋습니다.
public void updateNickname(String nickname, UserRepository userRepository) {
if (userRepository.existsByNickname(nickname)) {
throw new DuplicateNicknameException(nickname);
}
this.nickname = nickname;
}
soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcInvalidIdTokenException.java (1)
7-11: 예외 클래스 구현을 개선해 주세요.다음과 같은 개선사항을 제안드립니다:
- 클래스의 목적과 사용법을 설명하는 Javadoc 추가
- 사용자 정의 메시지를 지원하는 생성자 추가
- 예외 체이닝을 위한 cause 파라미터가 있는 생성자 추가
예시 코드:
+/** + * OpenID Connect ID 토큰이 유효하지 않을 때 발생하는 예외입니다. + * 이 예외는 토큰의 형식이 잘못되었거나, 서명이 유효하지 않은 경우 등에 사용됩니다. + */ public class OidcInvalidIdTokenException extends CustomException { public OidcInvalidIdTokenException() { super(OIDC_INVALID_ID_TOKEN); } + + public OidcInvalidIdTokenException(String message) { + super(OIDC_INVALID_ID_TOKEN, message); + } + + public OidcInvalidIdTokenException(String message, Throwable cause) { + super(OIDC_INVALID_ID_TOKEN, message, cause); + } }soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcProperties.java (2)
8-11: 클래스 수준의 문서화가 필요합니다.이 클래스의 목적과 사용법을 설명하는 Javadoc을 추가해 주세요.
+/** + * Kakao OIDC 인증에 필요한 설정값들을 관리하는 클래스입니다. + * application.yml 또는 application.properties 파일에서 oidc.kakao 접두사로 시작하는 속성들을 바인딩합니다. + */ @Getter @Setter @ConfigurationProperties(prefix = "oidc.kakao")
8-9: 불필요한 setter 제거를 고려해 주세요.설정 클래스는 애플리케이션 시작 시 한 번만 값이 설정되므로, @Setter 대신 생성자 주입 방식을 사용하는 것이 더 안전합니다.
@Getter -@Setter @ConfigurationProperties(prefix = "oidc.kakao") public final class KakaoOidcProperties { @NotBlank(message = "Kakao Client ID는 필수 값입니다") private final String clientId; + + public KakaoOidcProperties(String clientId) { + this.clientId = clientId; + } }soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcExceptionCode.java (2)
15-19: OIDC 관련 예외 상수 정의에 대한 제안현재 구현은 좋으나, 몇 가지 개선 사항을 제안드립니다:
- 메시지에 대한 다국어 지원을 고려해보시는 것이 좋을 것 같습니다.
- 각 상수에 대한 자세한 설명을 JavaDoc으로 추가하면 유지보수에 도움이 될 것 같습니다.
예시:
/** * ID 토큰의 발급자(issuer)가 유효하지 않은 경우 발생하는 예외 * 예상 발급자: https://kauth.kakao.com */ OIDC_INVALID_ISSUER(UNAUTHORIZED, "ID 토큰의 발급자가 유효하지 않습니다."),
21-27: getCode() 메서드 구현이 간단명료합니다!enum의 name()을 사용하여 코드값을 반환하는 방식이 적절합니다. 다만, 향후 확장성을 고려하여 다음 사항을 검토해보시면 좋을 것 같습니다:
- 에러 코드에 대한 체계적인 prefix 도입 (예: "OIDC_ERR_")
- 에러 코드의 국제화 지원
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java (1)
22-31: 보안 관련 메서드 접근 제어 강화 필요중요한 보안 관련 메서드들의 접근 제어를 강화해야 합니다.
다음과 같이 수정을 제안드립니다:
-public abstract class OidcService { +public sealed abstract class OidcService + permits GoogleOidcService, KakaoOidcService { private final JpaUserRepository jpaUserRepository; - protected abstract String getIssuer(); - protected abstract String getJwkSetUri(); - protected abstract String getClientId(); - protected abstract Provider getProvider(); + abstract String getIssuer(); + abstract String getJwkSetUri(); + abstract String getClientId(); + abstract Provider getProvider();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
soridam-auth/src/main/java/sorisoop/soridam/auth/config/OidcConfig.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcExceptionCode.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcInvalidIdTokenException.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcProperties.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcProperties.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcService.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcService.java
- soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java
🔇 Additional comments (5)
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcInvalidIdTokenException.java (1)
1-6: 패키지 구조와 임포트가 잘 구성되어 있습니다!패키지 구조가 명확하고 임포트가 깔끔하게 정리되어 있습니다.
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/exception/OidcExceptionCode.java (2)
1-11: 패키지 구조와 임포트가 잘 구성되어 있습니다!필요한 의존성들이 명확하게 임포트되어 있으며, 패키지 구조도 도메인 중심으로 잘 설계되어 있습니다.
12-14: Lombok 어노테이션 사용이 적절합니다!
@Getter와@AllArgsConstructor어노테이션을 통해 불필요한 보일러플레이트 코드를 제거하였습니다.ExceptionCode인터페이스 구현을 통해 일관된 예외 처리가 가능해졌습니다.soridam-auth/src/main/java/sorisoop/soridam/auth/config/OidcConfig.java (1)
10-16: 구성이 잘 되어있습니다!OIDC 설정이 깔끔하게 구성되어 있습니다.
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java (1)
42-49:⚠️ Potential issue사용자 정보 유효성 검증 강화 필요
필수 사용자 정보에 대한 null 체크와 유효성 검증이 필요합니다.
다음과 같이 수정을 제안드립니다:
@Override protected User createNewUser(String identifier, OidcUserInfo oidcUserInfo) { + if (oidcUserInfo == null) { + throw new IllegalArgumentException("사용자 정보가 없습니다"); + } + String name = oidcUserInfo.getFullName(); String email = oidcUserInfo.getEmail(); String profileImageUrl = oidcUserInfo.getPicture(); + if (email == null || email.isEmpty()) { + throw new IllegalArgumentException("이메일은 필수 정보입니다"); + } + + if (name == null || name.isEmpty()) { + name = "Unknown User"; + } + + if (profileImageUrl == null) { + profileImageUrl = ""; + } return User.googleOidcCreate(identifier, GOOGLE, name, email, profileImageUrl); }Likely invalid or redundant comment.
| public class KakaoOidcProperties { | ||
| private String clientId; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
필수 필드에 대한 유효성 검사와 불변성 보장이 필요합니다.
- clientId는 필수 값이므로 @notblank 어노테이션을 추가하여 유효성을 검증해야 합니다.
- 설정 클래스는 런타임에 변경되지 않도록 final로 선언하는 것이 좋습니다.
+import jakarta.validation.constraints.NotBlank;
+
-public class KakaoOidcProperties {
+public final class KakaoOidcProperties {
+ @NotBlank(message = "Kakao Client ID는 필수 값입니다")
private String clientId;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public class KakaoOidcProperties { | |
| private String clientId; | |
| } | |
| import jakarta.validation.constraints.NotBlank; | |
| public final class KakaoOidcProperties { | |
| @NotBlank(message = "Kakao Client ID는 필수 값입니다") | |
| private String clientId; | |
| } |
There was a problem hiding this comment.
record 타입으로 클래스를 생성해서 불변성을 유지하려 했으나, 빌드 과정에서 문제가 발생했습니다. record는 final 특성을 가지기 때문에 프록시 생성 과정에서 상속을 필요로 하는 CGLIB 기반의 프록시 생성 방식과 충돌이 발생한 것이 이유인데요. Spring Boot가 @ConfigurationProperties를 사용해 프록시 객체를 생성하려다 보니 이 과정에서 빌드 에러가 발생했습니다. 이 문제를 해결할 방법이 있을까요?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@minjo-on record 타입 대신 다음과 같은 방법으로 불변성을 유지하면서 프록시 생성 문제를 해결할 수 있습니다:
package sorisoop.soridam.auth.oauth.kakao;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.ConstructorBinding;
import jakarta.validation.constraints.NotBlank;
import lombok.Getter;
@Getter
@ConfigurationProperties(prefix = "oidc.kakao")
@ConstructorBinding
public class KakaoOidcProperties {
@NotBlank(message = "Kakao Client ID는 필수 값입니다")
private final String clientId;
public KakaoOidcProperties(@NotBlank(message = "Kakao Client ID는 필수 값입니다") String clientId) {
this.clientId = clientId;
}
}이 방식은:
@ConstructorBinding을 사용하여 생성자 기반 바인딩을 활성화- 필드를
final로 선언하여 불변성 보장 - 클래스는 non-final로 유지하여 프록시 생성 허용
- 생성자 매개변수에도 validation 제약조건 추가
✏️ Learnings added
Learnt from: minjo-on
PR: 9oormthon2024-T5-SoriSoop/soridam-server#54
File: soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcProperties.java:11-13
Timestamp: 2025-01-07T15:27:26.960Z
Learning: When using @ConfigurationProperties in Spring Boot, avoid using record types or final classes as they prevent CGLIB proxy creation. Instead, use @ConstructorBinding with non-final classes and final fields to achieve immutability while allowing proxy generation.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| package sorisoop.soridam.auth.oauth.exception; | ||
|
|
||
| import static org.springframework.http.HttpStatus.BAD_REQUEST; | ||
| import static org.springframework.http.HttpStatus.UNAUTHORIZED; | ||
|
|
||
| import org.springframework.http.HttpStatus; | ||
|
|
||
| import lombok.AllArgsConstructor; | ||
| import lombok.Getter; | ||
| import sorisoop.soridam.common.exception.ExceptionCode; | ||
|
|
||
| @Getter | ||
| @AllArgsConstructor | ||
| public enum OidcExceptionCode implements ExceptionCode { | ||
| OIDC_INVALID_ISSUER(UNAUTHORIZED, "ID 토큰의 발급자가 유효하지 않습니다."), | ||
| OIDC_INVALID_AUDIENCE(UNAUTHORIZED, "ID 토큰의 대상이 유효하지 않습니다."), | ||
| OIDC_EXPIRED(UNAUTHORIZED, "ID 토큰이 만료되었습니다."), | ||
| OIDC_INVALID_ID_TOKEN(BAD_REQUEST, "ID 토큰 검증에 실패하였습니다."), | ||
| ; | ||
|
|
||
| private final HttpStatus status; | ||
| private final String message; | ||
|
|
||
| @Override | ||
| public String getCode() { | ||
| return this.name(); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
보안 예외 처리 개선 필요
현재 구현된 OIDC 보안 검증은 Spring Security OAuth2를 통해 적절히 처리되고 있으나, 다음과 같은 예외 코드를 추가하여 더 세분화된 오류 처리를 제안드립니다:
OIDC_INVALID_SIGNATURE- JWT 서명 검증 실패 시OIDC_INVALID_ALG- 지원하지 않는 알고리즘 사용 시OIDC_INVALID_NONCE- nonce 검증 실패 시
이를 통해 보안 관련 문제 발생 시 더 명확한 오류 메시지를 제공할 수 있습니다.
🔗 Analysis chain
보안 관련 검증사항
OAuth2/OIDC 구현과 관련된 중요한 보안 예외들이 잘 정의되어 있습니다. 다음 사항들을 추가로 고려해보시면 좋을 것 같습니다:
- JWT 서명 검증 실패
- nonce 검증 실패
- 알고리즘 불일치
다음 스크립트로 관련 보안 검증 로직이 구현되어 있는지 확인해보겠습니다:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# JWT 검증 관련 코드 확인
rg -t java "verify|validate.*Token|checkSignature"
# nonce 검증 관련 코드 확인
rg -t java "nonce|state.*validate"
# 알고리즘 검증 관련 코드 확인
rg -t java "algorithm.*check|getAlgorithm"
Length of output: 1343
Script:
#!/bin/bash
# Check OidcService implementation
ast-grep --pattern 'class OidcService {
$$$
private OidcIdToken validateAndDecodeIdToken($_) {
$$$
}
$$$
}'
# Check if there's any algorithm-related configuration
rg -t java "algorithm|alg" -A 3
Length of output: 155
Script:
#!/bin/bash
# Check for OIDC configuration and decoder
rg -t java "OidcId.*Token|OAuth2|@Configuration.*OAuth|decode.*Token" -A 5
# Check for configuration files
fd -e yml -e yaml -e properties | xargs cat
Length of output: 12581
| @Getter | ||
| @Setter | ||
| @ConfigurationProperties(prefix = "oidc.google") | ||
| public class GoogleOidcProperties { | ||
| private String clientId; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
필수 속성 유효성 검증 추가 필요
clientId는 OIDC 인증에 필수적인 값이므로 유효성 검증이 필요합니다.
다음과 같이 수정을 제안드립니다:
@Getter
@Setter
@ConfigurationProperties(prefix = "oidc.google")
+@Validated
public class GoogleOidcProperties {
+ @NotBlank(message = "Google Client ID는 필수 값입니다")
private String clientId;
}📝 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.
| @Getter | |
| @Setter | |
| @ConfigurationProperties(prefix = "oidc.google") | |
| public class GoogleOidcProperties { | |
| private String clientId; | |
| } | |
| @Getter | |
| @Setter | |
| @ConfigurationProperties(prefix = "oidc.google") | |
| @Validated | |
| public class GoogleOidcProperties { | |
| @NotBlank(message = "Google Client ID는 필수 값입니다") | |
| private String clientId; | |
| } |
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java (2)
43-43: 🛠️ Refactor suggestion필수 필드에 대한 유효성 검증 어노테이션 추가 필요
OAuth 인증에 필요한 필드들에 대한 유효성 검증이 누락되었습니다:
-private String oauthIdentity; +@Column(nullable = false, unique = true) +private String oauthIdentity; -@Enumerated(STRING) -private Provider provider; +@Enumerated(STRING) +@Column(nullable = false) +private Provider provider; -@Enumerated(STRING) -private Role role; +@Enumerated(STRING) +@Column(nullable = false) +private Role role;Also applies to: 64-70
117-127:⚠️ Potential issue업데이트 메서드 보안 강화 필요
updateProfileImageUrl: URL 유효성 검증이 필요합니다.updateNickname: 이전 리뷰에서 언급된 중복 검사가 여전히 누락되었습니다.public void updateProfileImageUrl(String profileImageUrl) { + if (profileImageUrl != null && !profileImageUrl.matches("^https?://.*$")) { + throw new IllegalArgumentException("유효하지 않은 URL 형식입니다."); + } this.profileImageUrl = profileImageUrl; } -public void updateNickname(String nickname) { +public void updateNickname(String nickname, UserRepository userRepository) { + if (nickname == null || nickname.isBlank()) { + throw new IllegalArgumentException("닉네임은 필수값입니다."); + } + if (userRepository.existsByNickname(nickname)) { + throw new DuplicateNicknameException(nickname); + } this.nickname = nickname; }
🧹 Nitpick comments (5)
soridam-common/src/main/java/sorisoop/soridam/common/exception/ProviderNotFoundException.java (1)
5-9: 예외 처리 기능 개선 제안예외 발생 시 더 상세한 정보를 제공할 수 있도록 추가 생성자를 구현하는 것이 좋습니다.
다음과 같이 메시지를 포함하는 생성자를 추가하는 것을 고려해보세요:
public class ProviderNotFoundException extends CustomException { public ProviderNotFoundException() { super(PROVIDER_NOT_FOUND); } + + public ProviderNotFoundException(String provider) { + super(PROVIDER_NOT_FOUND, String.format("Provider not found: %s", provider)); + } }soridam-common/src/main/java/sorisoop/soridam/common/exception/GlobalExceptionCode.java (2)
16-18: 열거형 상수에 대한 문서화 제안각 열거형 상수에 대한 Javadoc 문서화를 추가하면 코드의 가독성과 유지보수성이 향상될 것 같습니다.
다음과 같이 각 상수에 대한 문서를 추가하는 것을 추천드립니다:
+ /** + * 서버 내부 오류가 발생했을 때 사용됩니다. + */ SERVER_ERROR(INTERNAL_SERVER_ERROR, "예상치 못한 문제가 발생했습니다."), + + /** + * 요청된 OAuth 제공자를 찾을 수 없을 때 사용됩니다. + */ PROVIDER_NOT_FOUND(NOT_FOUND, "해당 제공자를 찾을 수 없습니다."),
5-5: 정적 임포트 순서 개선
HttpStatus임포트문들을 알파벳 순서로 정렬하면 코드의 일관성이 향상될 것 같습니다.다음과 같이 정렬하는 것을 추천드립니다:
-import static org.springframework.http.HttpStatus.BAD_REQUEST; -import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; -import static org.springframework.http.HttpStatus.NOT_FOUND; +import static org.springframework.http.HttpStatus.BAD_REQUEST; +import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; +import static org.springframework.http.HttpStatus.NOT_FOUND;soridam-common/src/main/java/sorisoop/soridam/common/domain/Provider.java (1)
9-14: URI 값을 설정 파일로 분리하는 것을 고려해보세요.현재 구현은 잘 되어있지만, JWKS URI와 issuer 값을 application.yml이나 properties 파일로 분리하면 환경에 따라 쉽게 설정을 변경할 수 있습니다.
예시 설정 파일 구조:
oauth2: provider: kakao: jwks-uri: https://kauth.kakao.com/.well-known/jwks.json issuer: https://kauth.kakao.com google: jwks-uri: https://www.googleapis.com/oauth2/v3/certs issuer: https://accounts.google.comsoridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java (1)
Line range hint
31-127: 아키텍처 개선 제안
인증 방식별 사용자 생성 로직을 분리하는 것이 좋습니다:
UserBuilder인터페이스 도입OAuthUserBuilder와LocalUserBuilder구현체 분리OAuth 관련 메서드에 대한 문서화가 필요합니다:
- 각 소셜 로그인의 동작 방식
- 필수 필드와 선택 필드 구분
- 예외 처리 케이스
/** * OAuth 사용자 생성을 위한 팩토리 메서드 * @param oauthIdentity OAuth 제공자로부터 받은 고유 식별자 * @param provider 인증 제공자 (KAKAO, GOOGLE) * @param name 사용자 이름 * @param profileImageUrl 프로필 이미지 URL (null 가능) * @throws IllegalArgumentException 필수 파라미터가 null이거나 비어있는 경우 */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/CustomOidcUserService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OicdUserFactory.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcService.java(1 hunks)soridam-common/src/main/java/sorisoop/soridam/common/domain/Provider.java(1 hunks)soridam-common/src/main/java/sorisoop/soridam/common/exception/GlobalExceptionCode.java(2 hunks)soridam-common/src/main/java/sorisoop/soridam/common/exception/ProviderNotFoundException.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java(4 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/infrastructure/JpaUserRepository.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcService.java
- soridam-domain/src/main/java/sorisoop/soridam/domain/user/infrastructure/JpaUserRepository.java
- soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OicdUserFactory.java
- soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java
- soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/CustomOidcUserService.java
- soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java
🔇 Additional comments (3)
soridam-common/src/main/java/sorisoop/soridam/common/domain/Provider.java (2)
1-8: 패키지 구조와 임포트가 잘 구성되어 있습니다!공통 도메인 패키지에 위치하여 재사용성이 좋고, 필요한 임포트만 포함되어 있습니다.
16-17: 필드 선언이 적절합니다!private final 필드와 Lombok @Getter를 사용하여 불변성과 캡슐화가 잘 보장되어 있습니다.
soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java (1)
96-115:⚠️ Potential issueOAuth 팩토리 메서드 개선 필요
- 입력값 검증이 누락되었습니다.
point필드 초기화가 누락되었습니다.Role.USER하드코딩은 유연성을 제한합니다.public static User kakaoOidcCreate(String oauthIdentity, Provider provider, String name, String profileImageUrl){ + validateOAuthFields(oauthIdentity, provider, name); return User.builder() .oauthIdentity(oauthIdentity) .name(name) .profileImageUrl(profileImageUrl) .provider(provider) .role(Role.USER) + .point(0) .build(); } public static User googleOidcCreate(String oauthIdentity, Provider provider, String name, String email, String profileImageUrl){ + validateOAuthFields(oauthIdentity, provider, name); + validateEmail(email); return User.builder() .oauthIdentity(oauthIdentity) .name(name) .email(email) .profileImageUrl(profileImageUrl) .provider(provider) .role(Role.USER) + .point(0) .build(); } +private static void validateOAuthFields(String oauthIdentity, Provider provider, String name) { + if (oauthIdentity == null || oauthIdentity.isBlank()) { + throw new IllegalArgumentException("OAuth ID는 필수값입니다."); + } + if (provider == null) { + throw new IllegalArgumentException("제공자 정보는 필수값입니다."); + } + if (name == null || name.isBlank()) { + throw new IllegalArgumentException("이름은 필수값입니다."); + } +}Likely invalid or redundant comment.
soridam-common/src/main/java/sorisoop/soridam/common/domain/Provider.java
Show resolved
Hide resolved
minjo-on
changed the title
Summary
Tasks
idToken검증 (issuer, audience, expiration 등).idToken디코딩 및 검증.JwtProvider를 통해 Access 및 Refresh 토큰 발급 로직 추가.lastLoginTime을 업데이트하도록 처리.OidcInvalidIssuerException,OidcExpiredException) 추가.Summary by CodeRabbit
릴리즈 노트
새로운 기능
보안 개선
기술적 변경