feat : 앱 전용 카카오 로그인 구현 #335
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughKakao 앱 로그인 엔드포인트와 관련 DTO/서비스를 추가하고, OAuth2 사용자 로딩을 서비스로 위임하며 토큰 관리 로직을 TokenService에서 신규 AuthTokenService로 대체했습니다. 일부 클래스 이동/삭제와 MenuService의 import 정리도 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant KakaoAppAuthController
participant KakaoAppLoginService
participant CustomOAuth2UserService
participant OAuthUserService
participant UserRepository
participant AuthTokenService
participant TokenRepository
Client->>KakaoAppAuthController: POST /v2/app/oauth/kakao/login (kakaoAccessToken)
KakaoAppAuthController->>KakaoAppLoginService: login(request)
KakaoAppLoginService->>KakaoAppLoginService: clientRegistration 조회 및 OAuth2UserRequest 생성
KakaoAppLoginService->>CustomOAuth2UserService: loadUser(OAuth2UserRequest)
CustomOAuth2UserService->>OAuthUserService: loadOrCreateUser(OAuth2Response)
OAuthUserService->>UserRepository: findByEmail(email)
alt exists
UserRepository-->>OAuthUserService: User
else not exists
OAuthUserService->>UserRepository: save(new User)
UserRepository-->>OAuthUserService: new User
end
OAuthUserService-->>CustomOAuth2UserService: OAuthUserResult(user, newUser)
CustomOAuth2UserService-->>KakaoAppLoginService: CustomOAuth2User
KakaoAppLoginService->>AuthTokenService: issueTokens(user)
AuthTokenService->>AuthTokenService: access/refresh 토큰 생성
AuthTokenService->>TokenRepository: refresh 토큰 저장/갱신
AuthTokenService-->>KakaoAppLoginService: AuthenticationResponse(accessToken, refreshToken)
KakaoAppLoginService-->>KakaoAppAuthController: KakaoAppLoginResponse
KakaoAppAuthController-->>Client: 200 ApiUtils.success(response)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45-60분
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 (8)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/app/KakaoAppLoginRequest.java (1)
8-12: 요청 DTO에 토큰 유효성 검증 애너테이션을 추가하는 것을 고려해 보세요.
kakaoAccessToken이null/빈 문자열인 경우도 그대로 서비스까지 전달되므로,@NotBlank같은 Bean Validation 애너테이션을 붙이고 컨트롤러에서@Valid를 사용하면, 잘못된 요청을 초기에 400으로 걸러내기 좋습니다.nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/controller/KakaoAppAuthController.java (1)
17-34: 컨트롤러 시그니처를 조금 더 타입 안전하게 만들 수 있습니다.
- 메서드 시그니처를
ResponseEntity<ApiUtils.ApiResult<KakaoAppLoginResponse>>와 같이 구체적인 제네릭 타입으로 바꾸면, 컨트롤러/서비스 간 계약이 더 명확해집니다.@RequestBody에@Valid를 추가하고, DTO 쪽에 Bean Validation 애너테이션을 붙이면 요청 검증도 일관되게 처리할 수 있습니다.필수는 아니지만 유지보수성 측면에서 고려해 볼 만한 부분입니다.
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/global/OAuthUserService.java (1)
21-43: 소셜 타입 하드코딩은 향후 다른 OAuth 제공자 추가 시 확장성이 떨어질 수 있습니다.현재
createUser에서socialType(SocialType.KAKAO)로 고정되어 있어서, 나중에 다른 OAuth2 제공자(예: 네이버, 구글 등)를 같은 서비스로 처리하려면 이 부분을 다시 손봐야 합니다.
OAuth2Response.getProvider()값을 SocialType 과 매핑하는 헬퍼를 두는 방식 등으로 일반화해 두면 추후 확장이 훨씬 수월할 것 같습니다.nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/app/KakaoAppLoginService.java (1)
30-68: 로그인 플로우는 명확하지만, 입력 토큰 검증과 캐스팅 방어 로직을 추가하면 더 안전합니다.
kakaoAccessTokenValue가null이나 빈 문자열이어도 바로OAuth2AccessToken을 만들고 카카오 API 호출까지 진행됩니다. 초기에StringUtils.hasText(...)등으로 체크해서 바로 400 에 해당하는 예외를 던지면 디버깅과 트래픽 낭비 측면에서 이득이 있습니다.OAuth2User oAuth2User = customOAuth2UserService.loadUser(userRequest);이후 바로(CustomOAuth2User)로 캐스팅하고 있어서, 향후 다른 구현으로 교체되면ClassCastException이 날 수 있습니다.instanceof검사 후, 아닐 때는 의미 있는 비즈니스 예외를 던지는 쪽이 운영 시 원인 파악에 더 도움이 됩니다.핵심 로직은 잘 잡혀 있어서, 위 두 부분은 운영 안정성을 위한 보강 관점에서만 보시면 될 것 같습니다.
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/app/KakaoAppLoginResponse.java (1)
8-21: 불리언 필드 네이밍을 API 전반에서 한 번만 정리해 두면 좋습니다.이 DTO 안에서
phoneEntered/marketingAgree/isNewUser처럼 boolean 필드 네이밍 규칙이 섞여 있어서, JSON 응답 키를 기준으로 보면 약간 헷갈릴 수 있습니다.
아직 앱 클라이언트와의 계약이 굳지 않았다면,newUser등으로 통일할지,isNewUser스타일을 전반에 맞출지 한 번만 정리해 두면 이후 프런트/문서화에서 혼선을 줄일 수 있을 것 같습니다.nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/global/AuthTokenService.java (3)
35-35: 가독성을 위해 만료 시간 계산식을 명확하게 수정하세요.현재
60 * 60 * 2000L은 수학적으로는 정확하지만 의도를 파악하기 어렵습니다. 2시간을 나타내는 것이 명확하도록 수정하는 것이 좋습니다.다음 diff를 적용하여 계산식을 명확하게 만드세요:
- 60 * 60 * 2000L + 2 * 60 * 60 * 1000L
24-55: 토큰 발급 실패 시 에러 핸들링을 추가하는 것을 고려하세요.현재
jwtUtil.createAccessToken,jwtUtil.createRefreshToken,tokenRepository.save등의 작업에서 예외가 발생할 수 있지만 별도의 에러 핸들링이 없습니다. 토큰 발급은 인증의 핵심 단계이므로 실패 시 적절한 예외 처리나 로깅을 추가하는 것이 좋습니다.
57-68: Lombok 애노테이션을 사용하여 일관성을 유지하세요.프로젝트의 다른 부분에서 Lombok을 활용하고 있지만,
TokenResult클래스는 수동으로 getter를 구현하고 있습니다.@Getter애노테이션을 사용하면 코드 간결성과 일관성을 높일 수 있습니다.다음 diff를 적용하세요:
+ @Getter public static class TokenResult { private final String accessToken; private final String refreshToken; public TokenResult(String accessToken, String refreshToken) { this.accessToken = accessToken; this.refreshToken = refreshToken; } - - public String getAccessToken() { return accessToken; } - public String getRefreshToken() { return refreshToken; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuService.java(2 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/config/security/SecurityConfig.java(2 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/controller/KakaoAppAuthController.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/app/KakaoAppLoginRequest.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/app/KakaoAppLoginResponse.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/web/KaKaoResponse.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/web/OAuth2Response.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/app/KakaoAppLoginService.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/global/AuthTokenService.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/global/OAuthUserService.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/web/CustomOAuth2UserService.java(3 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/web/OAuth2LoginSuccessHandler.java(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/global/OAuthUserService.java (1)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/global/AuthTokenService.java (1)
Service(17-69)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/global/AuthTokenService.java (1)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/global/OAuthUserService.java (1)
Service(16-44)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/app/KakaoAppLoginResponse.java (2)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerLoginResponseDto.java (1)
AllArgsConstructor(10-34)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/app/KakaoAppLoginRequest.java (1)
Getter(8-13)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/app/KakaoAppLoginRequest.java (2)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerLoginResponseDto.java (1)
AllArgsConstructor(10-34)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/app/KakaoAppLoginResponse.java (1)
Getter(8-22)
🔇 Additional comments (8)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuService.java (1)
5-5: Collectors 임포트 방식 변경은 동작 영향 없이 가독성만 개선됩니다.
import java.util.stream.Collectors;추가 후Collectors.toMap(...)으로 사용하는 변경은 기존 FQCN(java.util.stream.Collectors.toMap)을 단순화한 것이라 기능·예외 동작은 완전히 동일합니다. 서비스 로직이나 정렬 검증 플로우에는 영향 없으므로 그대로 가셔도 될 것 같습니다.Also applies to: 160-161
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/web/OAuth2Response.java (1)
1-19: OAuth2 웹 DTO 패키지로의 이동은 구조적으로 적절합니다.기존 인터페이스를 변경하지 않고
dto.web패키지로만 이동해서, 웹 계층 DTO와의 책임 분리가 명확해진 것 같네요.nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/web/KaKaoResponse.java (1)
1-40: 패키지 이동은 괜찮지만, 카카오 응답 필드 누락 시 NPE 가능성을 한 번 확인해 주세요.
kakao_account/properties/profile_image에 바로toString()을 호출하고 있어서, 카카오 쪽 설정 변경이나 일부 계정에서 해당 값이 없을 경우 NPE가 발생할 수 있습니다.
현재 카카오 Developers에서 이메일/프로필 정보 제공을 “항상 필수”로 강제하고 있는지, 그리고 누락 시 어떤 동작(예: 명시적인 예외/에러 응답)을 원하는지 검토해 보시면 좋겠습니다.nowait-app-user-api/src/main/java/com/nowait/applicationuser/config/security/SecurityConfig.java (1)
17-18: 새 카카오 앱 로그인 엔드포인트 보안 설정이 매핑과 잘 맞습니다.
/v2/app/oauth/kakao+/login컨트롤러 매핑과"/v2/app/oauth/kakao/login"permitAll설정이 정확히 일치하고, OAuth2 관련 빈도oauth2.web패키지로 잘 정리되어 있어서 설정 측면에서는 문제 없어 보입니다.Also applies to: 56-62
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/web/OAuth2LoginSuccessHandler.java (2)
1-1: 패키지 재구성이 잘 되었습니다.OAuth2 관련 클래스들을
web패키지로 분리한 것은 관심사의 분리 측면에서 좋은 아키텍처 결정입니다.Also applies to: 11-11
29-29: 토큰 발급 로직 중앙화가 잘 적용되었습니다.
AuthTokenService를 사용하도록 리팩토링하여 결합도를 낮추고 단일 책임 원칙을 잘 따르고 있습니다.nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/web/CustomOAuth2UserService.java (2)
1-1: 패키지 재구성과 import 업데이트가 일관성 있게 적용되었습니다.OAuth2 관련 클래스들의 패키지 구조가 명확하게 정리되었습니다 (web, global 분리). DTO와 서비스 레이어의 분리가 잘 되어 있습니다.
Also applies to: 9-11
23-23: 사용자 조회/생성 로직의 중앙화가 잘 적용되었습니다.
OAuthUserService로 사용자 로딩 및 생성 로직을 위임하여 책임을 명확히 분리했습니다. 이전의 조건부 로직을 제거하고 단순화된 흐름으로 개선되었습니다.Based on learnings, 서비스 계층 분리 원칙을 잘 따르고 있습니다.
Also applies to: 42-44
...-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/global/AuthTokenService.java
Outdated
Show resolved
Hide resolved
...api/src/main/java/com/nowait/applicationuser/oauth/oauth2/web/OAuth2LoginSuccessHandler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/KakaoAppLoginResponse.java (1)
12-22: 필드 네이밍 일관성 및 LombokisNewUsergetter 이슈 확인 필요
네이밍 불일치:
nickName이 camelCase로 되어 있으나,ManagerLoginResponseDto에서는nickname으로 사용됩니다. 프로젝트 내 일관성을 위해 확인이 필요합니다.
isNewUsergetter 문제: Lombok@Getter는boolean isNewUser필드에 대해isNewUser()메서드를 생성합니다. JSON 직렬화 시 Jackson은 이를newUser로 인식할 수 있어 의도와 다른 결과가 나올 수 있습니다.- private String nickName; + private String nickname;
isNewUser필드의 경우, 명확한 직렬화를 위해 다음 중 하나를 고려하세요:
@JsonProperty("isNewUser")어노테이션 추가- 또는 필드명을
newUser로 변경nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/controller/TokenController.java (1)
58-76: 토큰 발급 로직 중복 가능성 검토
OAuth2LoginSuccessHandler와KakaoAppLoginService에서는authTokenService.issueTokens(user)를 사용하여 토큰을 발급하는 반면, 이 컨트롤러에서는jwtUtil을 직접 호출하여 토큰을 생성합니다.리프레시 토큰 갱신 시에도
AuthTokenService에 유사한 메서드를 추가하여 일관성을 유지하는 것을 고려해 주세요.예를 들어,
AuthTokenService에reissueTokens(User user, String oldRefreshToken)메서드를 추가하면 토큰 생성 로직이 한 곳에서 관리됩니다.nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/OAuth2LoginSuccessHandler.java (1)
40-56: 리다이렉트 URL 외부 설정 권장
AuthTokenService를 활용한 리팩토링이 잘 적용되었습니다. 다만 두 가지 개선점을 제안드립니다:
하드코딩된 리다이렉트 URL:
https://app.nowait.co.kr/login/success가 코드에 직접 작성되어 있어 환경별 배포 시 문제가 될 수 있습니다.@Value를 통해 설정 파일에서 주입받는 것을 권장합니다.
secure(false)설정: 주석에 명시된 대로 운영 환경에서는 반드시true로 변경해야 합니다. 환경 변수로 관리하면 실수를 방지할 수 있습니다.@Component @RequiredArgsConstructor @Slf4j public class OAuth2LoginSuccessHandler extends SimpleUrlAuthenticationSuccessHandler { private final AuthTokenService authTokenService; + @Value("${oauth2.login-success-url}") + private String loginSuccessUrl; + @Value("${cookie.secure:true}") + private boolean cookieSecure;- .secure(false) // 운영환경에서는 true + .secure(cookieSecure) ... - String targetUrl = "https://app.nowait.co.kr/login/success?accessToken=" + authenticationResponse.getAccessToken(); + String targetUrl = loginSuccessUrl + "?accessToken=" + authenticationResponse.getAccessToken();nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/OAuthUserService.java (1)
21-25: OAuth 신규 유저 생성 시 이메일 중복/기본값 정책 한 번 더 점검하는 것이 좋겠습니다
loadOrCreateUser가 이메일로 조회 후 없으면createUser를 타는 구조라, 동일 이메일로 거의 동시에 첫 로그인 요청이 들어오면 두 트랜잭션이 모두 새 유저를 만들려고 시도할 수 있습니다. 이메일에 unique 제약이 걸려 있다면 500 (DataIntegrityViolation 등) 으로 터질 수 있고, 제약이 없다면 중복 유저가 생길 수 있으니 DB 제약과 기대 동작을 한 번 더 확인해 보시는 게 좋겠습니다.또한
phoneNumber를 빈 문자열,storeId를0L로 두는 것이 도메인 상 “값이 없다”는 의미로 합의된 값인지도 점검해 보시는 걸 권장드립니다. 가능하다면 도메인 단에 상수/enum 등을 두거나, 진짜 “없음”은null로 두고 비즈니스 로직에서 분기하는 편이 가독성과 유지보수에 유리합니다.Also applies to: 27-42
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/KakaoAppLoginRequest.java (1)
8-12: 단순 요청 DTO 구성은 무난합니다
@Getter,@NoArgsConstructor(PROTECTED),@AllArgsConstructor조합으로 직렬화/역직렬화 및 테스트 작성에 모두 무리가 없어 보입니다. 추후 필드가 늘어날 가능성이 있다면, 생성자 오버로드 대신@Builder를 추가해서 가독성을 높이는 것도 선택지입니다.nowait-app-user-api/src/main/java/com/nowait/applicationuser/user/service/UserService.java (1)
21-23: AuthTokenService 연동은 일관적이지만, JWT 파싱 책임을 함께 모아두면 더 명확할 것 같습니다
putOptional이AuthenticationResponse를 반환하고 토큰 발급은authTokenService.issueTokens(user)로 위임한 부분은 새 토큰 설계와 잘 맞습니다. 다만 여전히JwtUtil.getUserId(accessToken)으로 컨트롤러/서비스 단에서 직접 JWT 를 파싱하고 있어서, 토큰 관련 책임이JwtUtil,AuthTokenService, 서비스들에 흩어져 있습니다.향후 유지보수성을 위해, userId 추출까지
AuthTokenService(또는 별도의 TokenFacade) 로 모아 두는 것을 고려해 보시면 좋겠습니다. 그러면 토큰 생성/검증/파싱 정책 변경 시 수정 범위를 한 곳으로 국한할 수 있습니다.Also applies to: 25-39
nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/service/AuthTokenService.java (2)
29-54:validateToken이 리프레시 토큰 전용임이 드러나지 않아 의미가 다소 모호합니다현재 구현은
userId로 DB 에서 리프레시 토큰을 찾고, 전달받은token과 일치 여부 및 만료 여부를 확인하는 로직이라 사실상 “리프레시 토큰 검증” 전용입니다. 메서드 이름이 단순validateToken이라, 액세스 토큰 검증까지 포함하는 것처럼 읽힐 수 있습니다.
- 사용처가 모두 리프레시 토큰이라면
validateRefreshToken정도로 명시적으로 바꾸거나,- 주석으로 “refresh token 용도임” 을 명확히 적어두면 이후 리팩토링 시 혼선을 줄일 수 있습니다.
- 이 메서드는 DB 읽기만 수행하므로
@Transactional(readOnly = true)로 의도를 드러내는 것도 고려할 만합니다.메서드 시그니처를 변경하면 호출부 수정이 필요하니, 실제 사용처 범위를 한 번 확인해 보시는 것을 권장드립니다.
56-78: 토큰 만료 시간 계산/상수는 공통 상수로 추출하면 가독성과 일관성이 더 좋아집니다
issueTokens,newUpdateRefreshToken,updateRefreshToken에서 리프레시 토큰 30일 만료를 여러 번 반복하고, 액세스 토큰 만료도60 * 60 * 2000L같은 수식으로 표현되어 있어 의도가 바로 눈에 들어오지 않습니다.예를 들어 다음처럼 상수/헬퍼로 빼면 의도가 명확해집니다.
+ private static final long ACCESS_TOKEN_EXPIRY_MILLIS = 1000L * 60 * 60 * 2; // 2시간 + private static final long REFRESH_TOKEN_EXPIRY_DAYS = 30L; + public AuthenticationResponse issueTokens(User user) { @@ - String accessToken = jwtUtil.createAccessToken( + String accessToken = jwtUtil.createAccessToken( "accessToken", userId, role, Boolean.TRUE.equals(user.getPhoneEntered()), Boolean.TRUE.equals(user.getIsMarketingAgree()), - 60 * 60 * 2000L - ); // 2시간 + ACCESS_TOKEN_EXPIRY_MILLIS + ); @@ - String refreshToken = jwtUtil.createRefreshToken( + String refreshToken = jwtUtil.createRefreshToken( "refreshToken", userId, - 30L * 24 * 60 * 60 * 1000L - ); // 30일 + REFRESH_TOKEN_EXPIRY_DAYS * 24 * 60 * 60 * 1000L + ); @@ - token.updateRefreshToken(refreshToken, LocalDateTime.now().plusDays(30)); + token.updateRefreshToken(refreshToken, LocalDateTime.now().plusDays(REFRESH_TOKEN_EXPIRY_DAYS)); @@ - Token token = Token.toEntity(user, refreshToken, LocalDateTime.now().plusDays(30)); + Token token = Token.toEntity(user, refreshToken, LocalDateTime.now().plusDays(REFRESH_TOKEN_EXPIRY_DAYS)); @@ - token.updateRefreshToken(newRefreshToken, LocalDateTime.now().plusDays(30)); + token.updateRefreshToken(newRefreshToken, LocalDateTime.now().plusDays(REFRESH_TOKEN_EXPIRY_DAYS));이렇게 하면 만료 정책을 바꿀 때 한 곳만 수정하면 되고, 코드를 읽을 때도 시간이 얼마인지 바로 파악할 수 있습니다.
Also applies to: 80-90, 92-103
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/config/security/SecurityConfig.java(2 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/controller/KakaoAppAuthController.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/KakaoAppLoginRequest.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/KakaoAppLoginResponse.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/OAuth2LoginSuccessHandler.java(4 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/CustomOAuth2UserService.java(3 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/KakaoAppLoginService.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/OAuthUserService.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/controller/TokenController.java(2 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/dto/NewAccessTokenResponse.java(0 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/service/AuthTokenService.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/service/TokenService.java(0 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/user/controller/UserController.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/user/service/UserService.java(3 hunks)
💤 Files with no reviewable changes (2)
- nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/dto/NewAccessTokenResponse.java
- nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/service/TokenService.java
🚧 Files skipped from review as they are similar to previous changes (2)
- nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/controller/KakaoAppAuthController.java
- nowait-app-user-api/src/main/java/com/nowait/applicationuser/config/security/SecurityConfig.java
🧰 Additional context used
🧬 Code graph analysis (3)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/OAuthUserService.java (2)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/CustomOAuth2UserService.java (1)
Service(18-45)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/KaKaoResponse.java (1)
RequiredArgsConstructor(8-40)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/KakaoAppLoginResponse.java (2)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerLoginResponseDto.java (1)
AllArgsConstructor(10-34)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/KakaoAppLoginRequest.java (1)
Getter(8-13)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/KakaoAppLoginRequest.java (2)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerLoginResponseDto.java (1)
AllArgsConstructor(10-34)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/KakaoAppLoginResponse.java (1)
Getter(8-22)
🔇 Additional comments (2)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/user/controller/UserController.java (1)
29-36: LGTM!
AuthenticationResponse로의 타입 변경이 다른 파일들과 일관성 있게 적용되었습니다.nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/KakaoAppLoginService.java (1)
64-66:isNewUser로직 및Boolean→boolean변환 확인 필요
isNewUser가 항상false: 앱에서도 신규 가입자가 있을 수 있는데, 항상false로 설정하면 프론트엔드에서 신규 사용자 플로우를 처리할 수 없습니다.CustomOAuth2UserService나OAuthUserService에서 신규 사용자 여부를 판단하여 전달하는 것이 좋습니다.
Boolean필드의 null 처리:user.getPhoneEntered()와user.getIsMarketingAgree()가Boolean타입이라면 null일 때NullPointerException이 발생할 수 있습니다.- .phoneEntered(user.getPhoneEntered()) - .marketingAgree(user.getIsMarketingAgree()) - .isNewUser(false) + .phoneEntered(Boolean.TRUE.equals(user.getPhoneEntered())) + .marketingAgree(Boolean.TRUE.equals(user.getIsMarketingAgree())) + .isNewUser(customUser.isNewUser()) // CustomOAuth2User에서 신규 여부 전달 필요
| @Slf4j | ||
| public class CustomOAuth2UserService extends DefaultOAuth2UserService { | ||
| private final UserRepository userRepository; | ||
| private final OAuthUserService oAuthUserService; |
There was a problem hiding this comment.
OAuth 사용자/속성 전체를 INFO 레벨로 로그에 남기는 것은 PII 노출 위험이 있습니다
log.info("CustomOAuth2UserService :: {}", oAuth2User); 와 log.info("oAuthUser.getAttributes :: {}", oAuth2User.getAttributes()); 는 이메일, 프로필 이미지 등 개인정보가 포함된 전체 OAuth 응답을 그대로 애플리케이션 로그에 남깁니다. 운영 환경에서 INFO 로그는 수집/보관 범위가 넓어서, 개인정보 보호/컴플라이언스 관점에서 리스크가 큽니다.
- 운영 기준으로는 최소한 DEBUG 이하로 내리거나,
- provider, providerId 정도의 최소 정보만 로그로 남기고 raw attributes 는 남기지 않는 방향을 추천드립니다.
예시:
- log.info("CustomOAuth2UserService :: {}", oAuth2User);
- log.info("oAuthUser.getAttributes :: {}", oAuth2User.getAttributes());
+ log.debug("CustomOAuth2UserService loaded user. provider={}, attributeKeys={}",
+ registrationId,
+ oAuth2User.getAttributes().keySet());이렇게 하면 디버깅에 필요한 정보는 유지하면서, 실제 PII/토큰 값 노출은 피할 수 있습니다.
Also applies to: 28-29, 40-44
There was a problem hiding this comment.
해당 로그 삭제 여부 검토 필요하니 이슈 등록해줘
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
gh issue create \
--repo GTable/NoWait_BE_MultiModule \
--title "CustomOAuth2UserService의 개인정보 로그 노출 검토 필요" \
--body "## 개요
CustomOAuth2UserService에서 OAuth 사용자 정보를 INFO 레벨로 로그에 기록하고 있어 개인정보(이메일, 프로필 이미지 등) 노출 위험이 있습니다.
## 위치
\`nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/CustomOAuth2UserService.java\`
## 검토 사항
- \`log.info(\"CustomOAuth2UserService :: {}\", oAuth2User);\` 삭제 또는 DEBUG 레벨로 변경
- \`log.info(\"oAuthUser.getAttributes :: {}\", oAuth2User.getAttributes());\` 삭제 또는 DEBUG 레벨로 변경
- 필요시 최소한의 정보(provider, providerId 등)만 로그에 남기는 방안 검토
## 관련 링크
- PR: https://github.com/GTable/NoWait_BE_MultiModule/pull/335
- 코멘트: https://github.com/GTable/NoWait_BE_MultiModule/pull/335#discussion_r2619398338
요청자: @Jjiggu" \
--assignee Jjiggu
Length of output: 58
@Jjiggu, 이슈를 생성했습니다!
✅ 이슈 #336: CustomOAuth2UserService의 개인정보 로그 노출 검토 필요
해당 이슈에 로그 삭제 또는 DEBUG 레벨 변경 검토 사항을 정리해두었으며, 이 PR과 코멘트 링크도 포함했습니다. 담당자로 배정되었으니 검토 후 처리 부탁드립니다.
🧬 Code Graph Analysis Results
filename:nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/KaKaoResponse.java
At lines 8-40:
@RequiredArgsConstructor
public class KaKaoResponse implements OAuth2Response {
private final Map<String, Object> attributes;
@Override
public String getProvider() {
return "kakao";
}
@Override
public String getProviderId() {
return attributes.get("id").toString();
}
@Override
public String getEmail() {
Map<String, Object> kakaoAccount = (Map<String, Object>)attributes.get("kakao_account");
return kakaoAccount.get("email").toString();
}
@Override
public String getNickName() {
Map<String, Object> properties = (Map<String, Object>)attributes.get("properties");
return properties.get("nickname").toString();
}
@Override
public String getProfileImage() {
Map<String, Object> properties = (Map<String, Object>)attributes.get("properties");
return properties.get("profile_image").toString();
}
}
filename:nowait-domain/domain-user-rdb/src/main/java/com/nowait/domainuserrdb/oauth/dto/CustomOAuth2User.java
At lines 15-64:
@RequiredArgsConstructor
@Getter
public class CustomOAuth2User implements OAuth2User {
private final User user;
private final boolean newUser;
@Override
public Map<String, Object> getAttributes() {
return null;
}
// 사용자가 가지는 권한 설정
@Override
public Collection<? extends GrantedAuthority> getAuthorities() {
Collection<GrantedAuthority> authorities = new ArrayList<>();
authorities.add(new GrantedAuthority() {
@Override
public String getAuthority() {
return user.getRole().getName(); // 유저의 권한 리턴
}
});
return authorities;
}
// JWT 인증 시 (항상 기존 유저 취급)
public CustomOAuth2User(User user) {
this.user = user;
this.newUser = false;
}
@Override
public String getName() {
return user.getEmail();
}
public User getUser() {
return user;
}
public Long getUserId() {
return user.getId();
}
public String getNickname() {
return user.getNickname();
}
}
filename:nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/OAuthUserService.java
At lines 17-49:
@Service
@RequiredArgsConstructor
public class OAuthUserService {
private final UserRepository userRepository;
@Transactional
public OAuthUserResult loadOrCreateUser(OAuth2Response oAuth2Response) {
return userRepository.findByEmail(oAuth2Response.getEmail())
.map(user -> new OAuthUserResult(user, false)) // 기존 유저
.orElseGet(() -> {
User created = createUser(oAuth2Response);
return new OAuthUserResult(created, true); // 신규 유저
});
}
private User createUser(OAuth2Response oAuth2Response) {
User user = User.builder()
.email(oAuth2Response.getEmail())
.phoneNumber("")
.nickname(oAuth2Response.getNickName())
.profileImage(oAuth2Response.getProfileImage())
.socialType(SocialType.KAKAO)
.role(Role.USER) // 일반 유저 설정
.storeId(0L)
.phoneEntered(false)
.isMarketingAgree(false)
.createdAt(LocalDateTime.now())
.updatedAt(LocalDateTime.now())
.build();
return userRepository.save(user);
}
}
filename:nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/KakaoAppLoginService.java
At lines 22-69:
@Service
@RequiredArgsConstructor
public class KakaoAppLoginService {
private final CustomOAuth2UserService customOAuth2UserService;
private final ClientRegistrationRepository clientRegistrationRepository;
private final AuthTokenService authTokenService;
public KakaoAppLoginResponse login(KakaoAppLoginRequest request) {
String kakaoAccessTokenValue = request.getKakaoAccessToken();
ClientRegistration kakaoRegistration = clientRegistrationRepository.findByRegistrationId("kakao");
if (kakaoRegistration == null) {
throw new OAuth2AuthenticationException("Kakao Client Registration not found.");
}
OAuth2AccessToken kakaoAccessToken = new OAuth2AccessToken(
OAuth2AccessToken.TokenType.BEARER,
kakaoAccessTokenValue,
Instant.now(),
null
);
OAuth2UserRequest userRequest = new OAuth2UserRequest(
kakaoRegistration,
kakaoAccessToken
);
OAuth2User oAuth2User = customOAuth2UserService.loadUser(userRequest);
CustomOAuth2User customUser = (CustomOAuth2User) oAuth2User;
User user = customUser.getUser();
AuthenticationResponse authenticationResponse = authTokenService.issueTokens(user);
return KakaoAppLoginResponse.builder()
.accessToken(authenticationResponse.getAccessToken())
.refreshToken(authenticationResponse.getRefreshToken())
.userId(user.getId())
.email(user.getEmail())
.nickName(user.getNickname())
.profileImage(user.getProfileImage())
.phoneEntered(user.getPhoneEntered())
.marketingAgree(user.getIsMarketingAgree())
.isNewUser(customUser.isNewUser())
.build();
}
}
...pp-user-api/src/main/java/com/nowait/applicationuser/oauth/service/KakaoAppLoginService.java
Show resolved
Hide resolved
| OAuth2User oAuth2User = customOAuth2UserService.loadUser(userRequest); | ||
| CustomOAuth2User customUser = (CustomOAuth2User) oAuth2User; | ||
| User user = customUser.getUser(); |
There was a problem hiding this comment.
ClassCastException 발생 가능성
loadUser가 반환하는 OAuth2User를 CustomOAuth2User로 직접 캐스팅하고 있습니다. 예상치 못한 구현체가 반환될 경우 런타임 예외가 발생합니다.
OAuth2User oAuth2User = customOAuth2UserService.loadUser(userRequest);
-CustomOAuth2User customUser = (CustomOAuth2User) oAuth2User;
+if (!(oAuth2User instanceof CustomOAuth2User customUser)) {
+ throw new OAuth2AuthenticationException("Unexpected OAuth2User type");
+}
User user = customUser.getUser();📝 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.
| OAuth2User oAuth2User = customOAuth2UserService.loadUser(userRequest); | |
| CustomOAuth2User customUser = (CustomOAuth2User) oAuth2User; | |
| User user = customUser.getUser(); | |
| OAuth2User oAuth2User = customOAuth2UserService.loadUser(userRequest); | |
| if (!(oAuth2User instanceof CustomOAuth2User customUser)) { | |
| throw new OAuth2AuthenticationException("Unexpected OAuth2User type"); | |
| } | |
| User user = customUser.getUser(); |
🤖 Prompt for AI Agents
In
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/KakaoAppLoginService.java
around lines 51 to 53, the code directly casts OAuth2User to CustomOAuth2User
which can throw ClassCastException; update the method to safely handle other
implementations by checking if oAuth2User is an instance of CustomOAuth2User
before casting, and if not either adapt the OAuth2User to your User domain
(extract required attributes and build a User) or throw a clear, specific
exception with contextual details; also add a null check for oAuth2User and
ensure any created/adapted User is validated before further processing.
There was a problem hiding this comment.
@Jjiggu, 알겠습니다! 나중에 개선하시는 것으로 하겠습니다.
필요하시면 추후 개선 작업을 추적하기 위한 이슈를 생성해 드릴 수 있습니다. 원하시면 말씀해 주세요! 😊
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/CustomOAuth2UserService.java (1)
29-30: OAuth2 응답 전체를 INFO 로그로 남기는 것은 PII/보안 리스크가 큽니다 (이전 코멘트 재강조)
log.info("CustomOAuth2UserService :: {}", oAuth2User);와
log.info("oAuthUser.getAttributes :: {}", oAuth2User.getAttributes());는 이메일, 닉네임, 프로필 이미지 URL 등 개인정보를 포함한 전체 OAuth 응답을 그대로 INFO 로그에 남깁니다. 운영 환경에서 INFO 로그는 수집/보관 범위가 넓어 개인정보 보호/컴플라이언스 측면에서 리스크가 큽니다.
- 운영용 프로파일에서는 최소 DEBUG 이하로 내리거나,
- provider, attribute key 정도의 최소 메타 정보만 남기고 실제 값은 로그에 남기지 않는 쪽을 권장드립니다.
예시:
- log.info("CustomOAuth2UserService :: {}", oAuth2User); - log.info("oAuthUser.getAttributes :: {}", oAuth2User.getAttributes()); + log.debug("CustomOAuth2UserService loaded user. provider={}, attributeKeys={}", + registrationId, + oAuth2User.getAttributes().keySet());이전 리뷰에서도 동일한 포인트가 언급되었는데, 아직 수정되지 않아 한 번 더 짚고 갑니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/OAuthUserResult.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/CustomOAuth2UserService.java(4 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/KakaoAppLoginService.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/OAuthUserService.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/dto/RefreshTokenRequest.java(1 hunks)nowait-domain/domain-user-rdb/src/main/java/com/nowait/domainuserrdb/oauth/dto/CustomOAuth2User.java(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/dto/RefreshTokenRequest.java
- nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/OAuthUserResult.java
🚧 Files skipped from review as they are similar to previous changes (2)
- nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/OAuthUserService.java
- nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/KakaoAppLoginService.java
🧰 Additional context used
🧬 Code graph analysis (1)
nowait-domain/domain-user-rdb/src/main/java/com/nowait/domainuserrdb/oauth/dto/CustomOAuth2User.java (1)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/dto/OAuthUserResult.java (1)
Getter(8-13)
🔇 Additional comments (4)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/service/CustomOAuth2UserService.java (1)
23-47: OAuthUserService 위임 + OAuthUserResult 사용 구조는 이해하기 쉽고 확장에도 좋아 보입니다
CustomOAuth2UserService가OAuthUserService에 사용자 조회/생성 책임을 위임하고,OAuthUserResult로User와newUser여부를 함께 받는 구조 덕분에:
- 이 클래스는 OAuth Provider → 도메인 DTO 변환에만 집중할 수 있고,
- 앱 측에서 신규 유저 분기(온보딩 등)를 해야 할 때도
newUser플래그로 간단히 처리할 수 있어 보입니다.현재 형태 그대로도 충분히 깔끔해서, 이 부분은 그대로 가셔도 될 것 같습니다.
nowait-domain/domain-user-rdb/src/main/java/com/nowait/domainuserrdb/oauth/dto/CustomOAuth2User.java (3)
18-19: LGTM!필드를
final로 선언하여 불변성을 보장하는 것은 좋은 설계입니다. 이는OAuthUserResult클래스의 패턴과도 일치합니다.
41-45: JWT 인증 시나리오를 위한 편의 생성자 추가 확인됨.JWT 인증 시 기존 유저를 나타내기 위해
newUser를false로 기본 설정하는 로직은 적절합니다. 이는 OAuth 로그인(신규/기존 유저 구분 필요)과 JWT 인증(항상 기존 유저) 시나리오를 명확히 구분합니다.
56-62: 위임 메서드 추가 확인됨.
getUserId()와getNickname()메서드는 하위 서비스에서 사용하기 편리한 위임 메서드입니다. 이는 OAuth 관련 서비스(Kakao 앱 로그인 플로우 등)에서CustomOAuth2User를 통해 사용자 정보에 쉽게 접근할 수 있도록 합니다.
| @RequiredArgsConstructor | ||
| @Getter |
There was a problem hiding this comment.
Lombok 어노테이션 사용 검증 필요.
@RequiredArgsConstructor와 @Getter가 클래스 레벨에 적용되어 있어 다음이 자동 생성됩니다:
CustomOAuth2User(User user, boolean newUser)생성자getUser(),isNewUser()메서드
그러나 커스텀 생성자(lines 42-45)와 명시적 getter 메서드들(lines 52-62)이 이미 존재합니다. 특히 getUser() 메서드가 line 52-54에 명시적으로 정의되어 있어 Lombok이 생성하는 메서드와 중복됩니다.
다음 중 하나를 선택하여 일관성을 개선하세요:
- Lombok 어노테이션을 제거하고 명시적 구현 유지
- 명시적 getter를 제거하고 Lombok에 의존 (단,
getUserId(),getNickname()은 유지)
-@RequiredArgsConstructor
@Getter
public class CustomOAuth2User implements OAuth2User {
private final User user;
private final boolean newUser;
+
+ public CustomOAuth2User(User user, boolean newUser) {
+ this.user = user;
+ this.newUser = newUser;
+ }
// ...중략...
- public User getUser() {
- return user;
- }
-
public Long getUserId() {
return user.getId();
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
nowait-domain/domain-user-rdb/src/main/java/com/nowait/domainuserrdb/oauth/dto/CustomOAuth2User.java
around lines 15-16 (and referencing the custom constructor at lines 42-45 and
explicit getters at lines 52-62), remove the Lombok annotations
@RequiredArgsConstructor and @Getter to avoid duplicate constructor/getter
generation; also remove any now-unused Lombok imports. Keep the existing
explicit constructor and all manually implemented getters (including getUserId()
and getNickname()) as-is so there are no conflicting/generated methods.
작업 요약
Issue Link
#334
문제점 및 어려움
해결 방안
Reference
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
제거/변경
✏️ Tip: You can customize this high-level summary in your review settings.