feat: 소셜 로그인 방식 변경 #310
feat: 소셜 로그인 방식 변경 #310
Conversation
Walkthrough소셜 로그인 기능이 전면적으로 리팩터링되고 확장되었습니다. Google, Kakao, Naver에 대한 소셜 로그인 서비스가 각각 분리된 클래스로 구현되었고, 토큰 타입(Access/ID)별 분기 처리 및 팩토리 패턴이 도입되었습니다. 다양한 테스트 코드와 빌드 설정도 함께 추가 및 수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UserAuthController
participant UserAuthServiceImpl
participant SocialLoginFactory
participant SocialLoginService
participant UserRepository
participant JwtProvider
Client->>UserAuthController: POST /oauth/login/{provider}
UserAuthController->>UserAuthServiceImpl: socialLogin(provider, requestDTO)
UserAuthServiceImpl->>SocialLoginFactory: getService(provider)
SocialLoginFactory-->>UserAuthServiceImpl: SocialLoginService
alt ID_TOKEN
UserAuthServiceImpl->>SocialLoginService: getUserInfoFromIdToken(token)
else ACCESS_TOKEN
UserAuthServiceImpl->>SocialLoginService: getUserInfoFromAccessToken(token)
end
SocialLoginService-->>UserAuthServiceImpl: OAuthUserInfo
UserAuthServiceImpl->>UserRepository: findByProviderAndProviderId(...)
alt User exists
UserRepository-->>UserAuthServiceImpl: User
else New user
UserAuthServiceImpl->>UserRepository: save(User)
UserRepository-->>UserAuthServiceImpl: User
end
UserAuthServiceImpl->>JwtProvider: issueJwtToken(User)
JwtProvider-->>UserAuthServiceImpl: JwtResponse
UserAuthServiceImpl-->>UserAuthController: JwtResponse
UserAuthController-->>Client: JwtResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (1)
src/main/java/UMC_7th/Closit/security/SecurityConfig.java (1)
13-32:CustomOAuth2UserService주입만 하고 미사용 – OAuth2 로그인 설정에 포함 필요
customOAuth2UserService를 주입했지만http.oauth2Login()설정이 없어 실제로 사용되지 않습니다.
의도라면 다음과 같이 연결해 주세요.+ .oauth2Login(oauth -> oauth + .userInfoEndpoint(userInfo -> userInfo + .userService(customOAuth2UserService)))미사용 상태가 계속되면 스프링이 “unused injection” 을 경고하고, 기대했던 OAuth2 유저 매핑 로직이 동작하지 않으므로 반드시 확인 바랍니다.
🧹 Nitpick comments (13)
src/main/java/UMC_7th/Closit/domain/user/dto/JwtResponse.java (1)
4-11:@Data사용 시 불변성/보안 이슈 발생 가능 – 최소@Getter로 한정 권장
@Data는 setter·equals·hashCode 까지 모두 생성해 DTO 가 가변 객체가 됩니다.
JWT 응답처럼 변경되면 안 되는 값을 담는 DTO 는 불변성을 유지하는 편이 안전합니다.-@Data -@AllArgsConstructor -@NoArgsConstructor -@Builder +@Getter +@Builder +@AllArgsConstructor +@NoArgsConstructor또는 필드를
final로 선언해 실제 불변 객체로 만드는 것도 고려해 주세요.src/test/java/UMC_7th/Closit/domain/user/service/social/SocialLoginFactoryTest.java (2)
35-37: lenient() 사용의 적절성 확인
lenient()스터빙은 모든 테스트에서 해당 모킹이 사용되지 않을 때 경고를 방지합니다. 현재 사용이 적절해 보이지만, 특정 테스트에서만 필요한 스터빙인지 확인해주세요.
74-81: 예외 테스트 시나리오 보완 제안현재는 빈 팩토리로 예외를 테스트하고 있는데, 기존 팩토리에서 지원하지 않는 새로운 소셜 로그인 타입을 추가한 시나리오도 고려해볼 수 있습니다.
@Test @DisplayName("새로운 소셜 로그인 타입 추가 시 예외 발생") void getService_NewSocialLoginType() { // given - 새로운 타입이 enum에 추가되었지만 서비스는 없는 경우를 가정 // when & then assertThatThrownBy(() -> socialLoginFactory.getService(/* 새로운 타입 */)) .isInstanceOf(GeneralException.class); }src/main/java/UMC_7th/Closit/domain/user/entity/NaverUserInfo.java (1)
14-14: 중복 코드 리팩터링 제안
userInfoJson.get("response")호출이 여러 메서드에서 반복됩니다. private 메서드로 추출하여 중복을 제거하고 가독성을 향상시킬 수 있습니다.+ private JsonNode getResponseNode() { + return userInfoJson.get("response"); + } + @Override public String providerId() { - JsonNode response = userInfoJson.get("response"); + JsonNode response = getResponseNode(); // ... 나머지 로직 }Also applies to: 23-23, 32-32
src/main/java/UMC_7th/Closit/domain/user/service/social/SocialLoginService.java (1)
18-18: 예외 처리 전략 문서화 제안메서드들이 토큰 검증 실패나 사용자 정보 추출 실패 시 어떤 예외를 던지는지 Javadoc에 명시하면 구현체들의 일관성을 높일 수 있습니다.
/** * ID Token을 검증하고 사용자 정보를 반환 * @param idToken 소셜 플랫폼에서 발급받은 ID Token * @return 검증된 사용자 정보 + * @throws GeneralException 토큰 검증 실패 시 + * @throws IllegalStateException 사용자 정보 추출 실패 시 */Also applies to: 25-25
src/main/java/UMC_7th/Closit/domain/user/service/social/KakaoSocialLoginService.java (1)
29-30: 의존성 주입 방식 개선 제안RestTemplate과 ObjectMapper를 new 연산자로 직접 생성하고 있습니다. Spring Bean으로 주입받는 방식으로 변경하면 테스트 용이성과 설정 관리가 향상됩니다.
- private final RestTemplate restTemplate = new RestTemplate(); - private final ObjectMapper objectMapper = new ObjectMapper(); + private final RestTemplate restTemplate; + private final ObjectMapper objectMapper;그리고 생성자에서 주입받도록 수정하거나, @Autowired를 사용하세요.
src/main/java/UMC_7th/Closit/domain/user/service/social/NaverSocialLoginService.java (1)
40-74: Access Token 검증 로직이 잘 구현되어 있지만 예외 처리 개선이 필요합니다.전반적인 구조는 좋으나, 더 구체적인 예외 처리를 통해 디버깅을 개선할 수 있습니다.
예외 처리를 더 세분화하여 디버깅을 개선할 수 있습니다:
} catch (Exception e) { - log.error("Failed to get user info from Naver access token", e); + if (e instanceof UserHandler) { + throw e; // UserHandler 예외는 그대로 재던지기 + } + log.error("Failed to get user info from Naver access token: {}", e.getMessage(), e); throw new UserHandler(ErrorStatus.INVALID_TOKEN); }src/test/java/UMC_7th/Closit/domain/user/service/social/GoogleSocialLoginServiceTest.java (1)
96-96: 라인 96에 코드 포맷팅 문제가 있습니다.
when(verifier.verify(mockIdToken)).thenReturn(googleIdToken);문이 이전 라인과 합쳐져 있습니다.다음과 같이 수정하세요:
mockedVerifier.when(() -> new GoogleIdTokenVerifier.Builder(any(), any())).thenReturn(mockBuilder); - when(verifier.verify(mockIdToken)).thenReturn(googleIdToken); + + when(verifier.verify(mockIdToken)).thenReturn(googleIdToken);src/main/java/UMC_7th/Closit/domain/user/service/social/GoogleSocialLoginService.java (3)
28-41: 의존성 주입 방식을 개선하는 것을 고려해보세요.
RestTemplate과ObjectMapper를 인스턴스 필드로 직접 생성하는 대신 Spring의 의존성 주입을 사용하는 것이 좋습니다. 이렇게 하면 테스트 시 목킹이 용이하고 Spring의 설정을 통한 커스터마이징이 가능합니다.- private final RestTemplate restTemplate = new RestTemplate(); - private final ObjectMapper objectMapper = new ObjectMapper(); + private final RestTemplate restTemplate; + private final ObjectMapper objectMapper;그리고 이들을 생성자나 설정 클래스에서 주입받도록 변경하세요.
47-83: ID 토큰 검증 로직이 보안적으로 잘 구현되어 있습니다.Google의 공식 라이브러리를 사용한 토큰 검증, issuer 및 audience 검증이 적절히 수행되고 있습니다. 하지만 보안상 민감한 토큰 정보를 로그에 남기는 것은 피해야 합니다.
- log.debug("Invalid ID token: {}", idToken); + log.debug("Invalid ID token provided");토큰 값 자체를 로그에 남기지 않도록 수정하는 것이 보안상 안전합니다.
85-114: 액세스 토큰을 통한 사용자 정보 조회가 적절히 구현되었습니다.Bearer 토큰 설정, HTTP 상태 코드 검증, JSON 파싱이 올바르게 수행되고 있습니다.
RestTemplate에 타임아웃 설정을 추가하는 것을 고려해보세요:
@Bean public RestTemplate restTemplate() { RestTemplate restTemplate = new RestTemplate(); restTemplate.setRequestFactory(new SimpleClientHttpRequestFactory()); ((SimpleClientHttpRequestFactory) restTemplate.getRequestFactory()) .setConnectTimeout(5000); ((SimpleClientHttpRequestFactory) restTemplate.getRequestFactory()) .setReadTimeout(10000); return restTemplate; }src/main/java/UMC_7th/Closit/domain/user/entity/GoogleUserInfo.java (2)
13-23: 생성자 설계가 두 가지 데이터 소스를 적절히 처리하고 있습니다.ID 토큰과 액세스 토큰에 대한 별도 생성자를 제공하는 것은 합리적인 접근입니다. 하지만 입력 검증을 추가하는 것을 고려해보세요.
// ID Token용 생성자 public GoogleUserInfo(GoogleIdToken.Payload payload) { + if (payload == null) { + throw new IllegalArgumentException("Payload cannot be null"); + } this.payload = payload; this.userInfoJson = null; } // Access Token용 생성자 public GoogleUserInfo(JsonNode userInfoJson) { + if (userInfoJson == null) { + throw new IllegalArgumentException("UserInfo JSON cannot be null"); + } this.payload = null; this.userInfoJson = userInfoJson; }
25-31: providerId 메서드가 올바르게 구현되었습니다.두 데이터 소스에서 사용자 ID를 적절히 추출하고 있습니다. 하지만 JSON 필드 접근 시 null 안전성을 개선할 수 있습니다.
- return userInfoJson.get("id").asText(); + JsonNode idNode = userInfoJson.get("id"); + return idNode != null ? idNode.asText() : null;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
build.gradle(2 hunks)src/main/java/UMC_7th/Closit/domain/user/controller/UserAuthController.java(1 hunks)src/main/java/UMC_7th/Closit/domain/user/dto/JwtResponse.java(1 hunks)src/main/java/UMC_7th/Closit/domain/user/dto/OAuthLoginRequestDTO.java(1 hunks)src/main/java/UMC_7th/Closit/domain/user/entity/GoogleUserInfo.java(1 hunks)src/main/java/UMC_7th/Closit/domain/user/entity/KakaoUserInfo.java(1 hunks)src/main/java/UMC_7th/Closit/domain/user/entity/NaverUserInfo.java(1 hunks)src/main/java/UMC_7th/Closit/domain/user/service/GoogleOAuthService.java(0 hunks)src/main/java/UMC_7th/Closit/domain/user/service/UserAuthService.java(1 hunks)src/main/java/UMC_7th/Closit/domain/user/service/UserAuthServiceImpl.java(4 hunks)src/main/java/UMC_7th/Closit/domain/user/service/social/GoogleSocialLoginService.java(1 hunks)src/main/java/UMC_7th/Closit/domain/user/service/social/KakaoSocialLoginService.java(1 hunks)src/main/java/UMC_7th/Closit/domain/user/service/social/NaverSocialLoginService.java(1 hunks)src/main/java/UMC_7th/Closit/domain/user/service/social/SocialLoginFactory.java(1 hunks)src/main/java/UMC_7th/Closit/domain/user/service/social/SocialLoginService.java(1 hunks)src/main/java/UMC_7th/Closit/global/common/SocialLoginType.java(1 hunks)src/main/java/UMC_7th/Closit/security/SecurityConfig.java(2 hunks)src/test/java/UMC_7th/Closit/domain/user/service/UserAuthServiceImplTest.java(2 hunks)src/test/java/UMC_7th/Closit/domain/user/service/UserAuthServiceSocialLoginTest.java(1 hunks)src/test/java/UMC_7th/Closit/domain/user/service/social/GoogleSocialLoginServiceTest.java(1 hunks)src/test/java/UMC_7th/Closit/domain/user/service/social/KakaoSocialLoginServiceTest.java(1 hunks)src/test/java/UMC_7th/Closit/domain/user/service/social/NaverSocialLoginServiceTest.java(1 hunks)src/test/java/UMC_7th/Closit/domain/user/service/social/SocialLoginFactoryTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/UMC_7th/Closit/domain/user/service/GoogleOAuthService.java
🧰 Additional context used
🧬 Code Graph Analysis (7)
src/main/java/UMC_7th/Closit/domain/user/entity/NaverUserInfo.java (1)
src/main/java/UMC_7th/Closit/domain/user/entity/KakaoUserInfo.java (1)
RequiredArgsConstructor(7-48)
src/main/java/UMC_7th/Closit/domain/user/service/social/SocialLoginFactory.java (2)
src/main/java/UMC_7th/Closit/domain/user/entity/KakaoUserInfo.java (1)
RequiredArgsConstructor(7-48)src/main/java/UMC_7th/Closit/domain/user/entity/NaverUserInfo.java (1)
RequiredArgsConstructor(7-48)
src/main/java/UMC_7th/Closit/domain/user/entity/KakaoUserInfo.java (1)
src/main/java/UMC_7th/Closit/domain/user/entity/NaverUserInfo.java (1)
RequiredArgsConstructor(7-48)
src/main/java/UMC_7th/Closit/domain/user/service/social/NaverSocialLoginService.java (4)
src/main/java/UMC_7th/Closit/global/apiPayload/exception/handler/UserHandler.java (1)
UserHandler(6-11)src/main/java/UMC_7th/Closit/domain/user/entity/NaverUserInfo.java (1)
RequiredArgsConstructor(7-48)src/main/java/UMC_7th/Closit/domain/user/service/social/KakaoSocialLoginService.java (1)
Service(22-73)src/main/java/UMC_7th/Closit/domain/user/service/social/GoogleSocialLoginService.java (1)
Service(28-115)
src/main/java/UMC_7th/Closit/domain/user/dto/OAuthLoginRequestDTO.java (1)
src/main/java/UMC_7th/Closit/domain/user/dto/JwtResponse.java (1)
Data(8-16)
src/main/java/UMC_7th/Closit/domain/user/dto/JwtResponse.java (1)
src/main/java/UMC_7th/Closit/domain/user/dto/RegisterResponseDTO.java (1)
Data(8-17)
src/main/java/UMC_7th/Closit/domain/user/service/social/KakaoSocialLoginService.java (4)
src/main/java/UMC_7th/Closit/global/apiPayload/exception/handler/UserHandler.java (1)
UserHandler(6-11)src/main/java/UMC_7th/Closit/domain/user/entity/KakaoUserInfo.java (1)
RequiredArgsConstructor(7-48)src/main/java/UMC_7th/Closit/domain/user/service/social/GoogleSocialLoginService.java (1)
Service(28-115)src/main/java/UMC_7th/Closit/domain/user/service/social/NaverSocialLoginService.java (1)
Service(19-75)
🔇 Additional comments (40)
src/main/java/UMC_7th/Closit/global/common/SocialLoginType.java (1)
3-5: 열거형 순서 변경 시 기존 데이터 호환성 확인 필요
GOOGLE, NAVER, KAKAO로 순서를 변경하면 JPA@Enumerated(EnumType.ORDINAL)또는 직렬화 시 ordinal 값을 사용하는 곳에서 기존 데이터와 매핑이 달라질 수 있습니다.
DB 컬럼이 문자열이 아닌 정수(ordinal)로 저장돼 있다면 마이그레이션 또는 문자열(enum name) 저장 방식으로의 전환을 검토해 주세요.build.gradle (1)
31-32: OAuth2 Client 의존성 추가 👍
spring-boot-starter-oauth2-client추가는 새로운 소셜 로그인 플로우에 필수이므로 적절합니다.src/main/java/UMC_7th/Closit/domain/user/service/UserAuthService.java (1)
22-23: 파라미터 명세 변경 확인인터페이스 시그니처가 바뀌었으므로
UserAuthServiceImpl및 모든 테스트 코드가 동일하게 수정됐는지 다시 한 번 확인해 주세요.
변경 자체는 의미가 명확해져 👍src/test/java/UMC_7th/Closit/domain/user/service/UserAuthServiceImplTest.java (1)
48-53: 원본 리뷰 코멘트 부정확:registerUser메서드가 실제로 호출하는 의존성은userRepository.existsByEmail/existsByClositId와passwordEncoder.encode뿐이며, 이 둘 모두 테스트에서@Mock처리되어 있습니다. 주석 처리된emailTokenService호출과 사용되지 않는 나머지 의존성은 실행되지 않아 NPE나 실제 호출 누락 우려가 없습니다.Likely an incorrect or invalid review comment.
src/test/java/UMC_7th/Closit/domain/user/service/social/SocialLoginFactoryTest.java (1)
42-81: 팩토리 테스트 커버리지가 우수함모든 소셜 로그인 타입에 대한 정상 케이스와 지원하지 않는 타입에 대한 예외 케이스를 모두 테스트하고 있어 좋습니다. 테스트 메서드명도 명확하고 한국어로 잘 작성되었습니다.
src/main/java/UMC_7th/Closit/domain/user/service/social/SocialLoginFactory.java (1)
35-44: 팩토리 메서드 구현이 우수함
getService()메서드가 명확한 Javadoc과 함께 적절한 예외 처리를 하고 있어 좋습니다. 지연 초기화 패턴도 적절히 구현되어 있습니다.src/main/java/UMC_7th/Closit/domain/user/entity/NaverUserInfo.java (2)
13-19: JSON 응답 구조 처리가 적절함네이버의 중첩된 JSON 응답 구조(
response노드)를 올바르게 처리하고 있으며, 필수 필드 누락 시 명확한 한국어 예외 메시지를 제공합니다.
30-42: 이름 필드 처리 로직이 견고함이름 추출에서
name→nickname→ 기본값 순서의 폴백 로직이 잘 구현되어 있습니다. 네이버 API의 다양한 응답 형태를 적절히 처리하고 있습니다.src/main/java/UMC_7th/Closit/domain/user/service/social/SocialLoginService.java (1)
6-26: 우수한 인터페이스 설계소셜 로그인 서비스의 계약을 명확하게 정의하고 있습니다:
- ID Token과 Access Token 모두 지원 (PR 목표와 일치)
- 명확한 Javadoc 문서화
- 일관된 반환 타입 (
OAuthUserInfo)- 각 구현체의 지원 타입 식별 가능
src/main/java/UMC_7th/Closit/domain/user/service/UserAuthServiceImpl.java (3)
13-14: 팩토리 패턴 도입으로 확장성 개선SocialLoginFactory와 SocialLoginService 인터페이스 도입으로 다중 소셜 로그인 제공자를 깔끔하게 지원할 수 있게 되었습니다. 확장 가능한 설계입니다.
Also applies to: 42-42
118-131: 토큰 타입별 분기 처리 구현이 우수합니다ID_TOKEN과 ACCESS_TOKEN을 구분하여 처리하는 로직이 명확하고, 지원하지 않는 토큰 타입에 대한 적절한 예외 처리가 되어 있습니다.
162-162: registerNewUser 메서드 접근 제한자(package-private) 유지해도 됩니다.현재 registerNewUser 메서드는 같은 패키지 내 서비스 구현과 테스트(
UserAuthServiceSocialLoginTest)에서만 호출되며 외부 노출이 없습니다. Mockito 스파이를 사용해 private 메서드를 목킹할 수 없어 접근 제한자를 package-private으로 변경한 것으로 보이며, 이 정의를 유지해도 무방합니다.src/test/java/UMC_7th/Closit/domain/user/service/social/KakaoSocialLoginServiceTest.java (3)
55-60: ReflectionTestUtils 사용이 적절합니다private final 필드들을 테스트를 위해 Mock으로 교체하는 방식이 적절합니다. setUp 메서드에서 깔끔하게 처리되었습니다.
80-114: 포괄적인 성공 시나리오 테스트실제 Kakao API 응답 형식을 모방한 Mock 데이터와 RestTemplate, ObjectMapper에 대한 적절한 Mocking으로 완전한 테스트 시나리오를 구현했습니다.
116-148: 예외 처리 시나리오가 잘 커버되었습니다유효하지 않은 토큰(401 응답)과 API 호출 중 예외 발생 상황에 대한 테스트가 적절히 구현되어 있습니다.
src/main/java/UMC_7th/Closit/domain/user/service/social/KakaoSocialLoginService.java (2)
37-41: ID Token 미지원 처리가 명확합니다카카오가 ID Token을 지원하지 않는다는 것을 명시적으로 처리하여 API 사용자에게 명확한 피드백을 제공합니다.
68-71: 예외 처리와 로깅이 적절합니다모든 예외를 catch하여 적절한 로그를 남기고 일관된 예외 타입으로 변환하는 것이 좋습니다. 보안상 민감한 토큰 정보는 로그에 포함되지 않도록 잘 처리되었습니다.
src/test/java/UMC_7th/Closit/domain/user/service/UserAuthServiceSocialLoginTest.java (2)
70-93: 기존 회원 로그인 시나리오가 잘 구현되었습니다Access Token을 사용한 기존 회원의 소셜 로그인 플로우가 완전히 테스트되어 있습니다. Mock 설정과 검증이 적절합니다.
113-116: Spy 패턴 사용이 적절합니다package-private 메서드인 registerNewUser를 테스트하기 위해 spy를 사용한 것이 적절한 접근 방식입니다.
src/main/java/UMC_7th/Closit/domain/user/controller/UserAuthController.java (2)
55-79: 상세한 API 문서화가 우수합니다소셜 로그인 API에 대한 매우 상세하고 실용적인 문서화가 되어 있습니다. 지원 플랫폼별 토큰 타입, 요청 예시, 처리 과정이 명확하게 설명되었습니다.
80-80: 엔드포인트 경로 변경이 RESTful 합니다
/oauth/{provider}/login에서/oauth/login/{provider}로 변경된 것이 더 명확하고 RESTful한 설계입니다.src/main/java/UMC_7th/Closit/domain/user/service/social/NaverSocialLoginService.java (2)
24-27: API URL 상수와 의존성 주입 패턴이 일관성 있게 구현되어 있습니다.네이버 API URL이 올바르게 상수로 정의되어 있고, RestTemplate과 ObjectMapper의 인스턴스 생성 방식이 다른 소셜 로그인 서비스들과 일치합니다.
34-37: ID 토큰 미지원에 대한 올바른 예외 처리입니다.네이버는 실제로 ID 토큰을 지원하지 않으므로, 적절한 예외를 던지는 것이 맞습니다. 다른 소셜 로그인 서비스(카카오)와도 일관된 패턴을 보여줍니다.
src/main/java/UMC_7th/Closit/domain/user/entity/KakaoUserInfo.java (3)
17-24: 이메일 조회 로직이 안전하게 구현되어 있습니다.null 체크와 적절한 예외 처리가 포함되어 있어 안전합니다.
26-42: 닉네임 조회의 다단계 폴백 로직이 잘 구현되어 있습니다.카카오 API의 두 가지 닉네임 소스("properties.nickname", "kakao_account.profile.nickname")를 모두 처리하고 적절한 기본값을 제공합니다.
44-47: SocialLoginType 반환이 올바르게 구현되어 있습니다.카카오 프로바이더 타입을 정확히 반환합니다.
src/main/java/UMC_7th/Closit/domain/user/dto/OAuthLoginRequestDTO.java (4)
17-27: 토큰 타입과 토큰 필드의 검증 어노테이션이 적절히 설정되어 있습니다.
@NotNull과@NotBlank어노테이션을 통해 필수 필드 검증이 올바르게 구현되어 있고, 한국어 메시지도 적절합니다.
32-35: TokenType 열거형이 명확하게 정의되어 있습니다.ID Token과 Access Token의 구분이 명확하고, 주석으로 각각의 표준(OpenID Connect, OAuth 2.0)을 명시한 것이 좋습니다.
37-48: 하위 호환성 지원이 잘 구현되어 있습니다.기존 코드와의 호환성을 위한 deprecated 생성자와 getter가 적절히 제공되어 있습니다.
@Deprecated어노테이션 사용도 올바릅니다.
7-12: Builder 패턴 추가가 유용합니다.
@Builder어노테이션 추가로 객체 생성의 유연성이 향상되었습니다.src/test/java/UMC_7th/Closit/domain/user/service/social/GoogleSocialLoginServiceTest.java (3)
79-108: ID Token 테스트가 포괄적으로 구현되어 있습니다.Google ID Token 검증의 모든 단계(토큰 검증, payload 추출, 필드 검증)가 적절히 모킹되고 테스트되어 있습니다. MockedStatic 사용도 올바릅니다.
127-161: Access Token 테스트가 잘 구현되어 있습니다.REST API 호출 모킹, 응답 파싱, 검증 로직이 모두 포함되어 있고, verify를 통한 호출 검증도 적절합니다.
163-195: 에러 시나리오 테스트 커버리지가 우수합니다.HTTP 오류 상태와 런타임 예외 모두를 테스트하여 견고한 오류 처리를 검증하고 있습니다.
src/test/java/UMC_7th/Closit/domain/user/service/social/NaverSocialLoginServiceTest.java (5)
66-74: 소셜 로그인 타입 테스트가 간단하고 명확합니다.지원하는 소셜 로그인 타입을 올바르게 검증하고 있습니다.
76-82: ID Token 미지원 테스트가 적절합니다.네이버가 ID Token을 지원하지 않는다는 비즈니스 로직이 올바르게 테스트되어 있습니다.
84-118: Access Token 성공 시나리오 테스트가 포괄적입니다.실제 네이버 API 응답 구조를 모방한 테스트 데이터를 사용하고, 모든 사용자 정보 필드를 검증하고 있습니다.
138-157: 네이버 API 특화 오류 테스트가 우수합니다.HTTP 성공 상태이지만 네이버 API
resultcode가 오류인 경우를 별도로 테스트하여, 네이버 API의 특수한 오류 응답 구조를 적절히 다루고 있습니다.
175-244: 사용자 이름 폴백 로직의 엣지 케이스 테스트가 훌륭합니다.닉네임만 있는 경우와 이름/닉네임이 모두 없는 경우를 별도로 테스트하여,
NaverUserInfo의 이름 추출 로직을 완전히 검증하고 있습니다.src/main/java/UMC_7th/Closit/domain/user/service/social/GoogleSocialLoginService.java (2)
1-26: 패키지 및 임포트 구조가 적절합니다.필요한 의존성들이 명확하게 임포트되어 있고, Google API 클라이언트 라이브러리와 Spring 컴포넌트들이 적절히 포함되어 있습니다.
42-45: 지원 타입 반환 메서드가 올바르게 구현되었습니다.인터페이스 계약을 정확히 이행하고 있습니다.
| testImplementation 'org.mockitox:mockito-core:5.3.1' | ||
| testImplementation 'org.mockito:mockito-junit-jupiter:5.3.1' |
There was a problem hiding this comment.
잘못된 Maven Group ID ‑ org.mockitox → org.mockito
org.mockitox:mockito-core 는 존재하지 않아 빌드 실패가 발생합니다.
-testImplementation 'org.mockitox:mockito-core:5.3.1'
+testImplementation 'org.mockito:mockito-core:5.3.1'📝 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.
| testImplementation 'org.mockitox:mockito-core:5.3.1' | |
| testImplementation 'org.mockito:mockito-junit-jupiter:5.3.1' | |
| testImplementation 'org.mockito:mockito-core:5.3.1' | |
| testImplementation 'org.mockito:mockito-junit-jupiter:5.3.1' |
🤖 Prompt for AI Agents
In build.gradle at lines 43 to 44, the Maven group ID for the mockito-core
dependency is incorrect as 'org.mockitox'. Change it to the correct group ID
'org.mockito' to fix the build failure caused by the non-existent dependency.
| @Override | ||
| public String getEmail () { | ||
| return payload.getEmail(); | ||
| public String getEmail() { | ||
| if (payload != null) { | ||
| return payload.getEmail(); | ||
| } | ||
| return userInfoJson.get("email").asText(); | ||
| } | ||
|
|
||
| @Override | ||
| public String getName () { | ||
| return payload.get("name").toString(); | ||
| public String getName() { | ||
| if (payload != null) { | ||
| return payload.get("name").toString(); | ||
| } | ||
| return userInfoJson.get("name").asText(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
getEmail과 getName 메서드에서 일관성 및 안전성을 개선해야 합니다.
메서드 구현에서 몇 가지 문제점이 있습니다:
- getName에서 payload와 JSON 처리 방식이 다름 (toString() vs asText())
- JSON 필드 접근 시 null 체크 부재
@Override
public String getEmail() {
if (payload != null) {
return payload.getEmail();
}
- return userInfoJson.get("email").asText();
+ JsonNode emailNode = userInfoJson.get("email");
+ return emailNode != null ? emailNode.asText() : null;
}
@Override
public String getName() {
if (payload != null) {
- return payload.get("name").toString();
+ Object name = payload.get("name");
+ return name != null ? name.toString() : null;
}
- return userInfoJson.get("name").asText();
+ JsonNode nameNode = userInfoJson.get("name");
+ return nameNode != null ? nameNode.asText() : null;
}📝 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.
| @Override | |
| public String getEmail () { | |
| return payload.getEmail(); | |
| public String getEmail() { | |
| if (payload != null) { | |
| return payload.getEmail(); | |
| } | |
| return userInfoJson.get("email").asText(); | |
| } | |
| @Override | |
| public String getName () { | |
| return payload.get("name").toString(); | |
| public String getName() { | |
| if (payload != null) { | |
| return payload.get("name").toString(); | |
| } | |
| return userInfoJson.get("name").asText(); | |
| } | |
| @Override | |
| public String getEmail() { | |
| if (payload != null) { | |
| return payload.getEmail(); | |
| } | |
| JsonNode emailNode = userInfoJson.get("email"); | |
| return emailNode != null ? emailNode.asText() : null; | |
| } | |
| @Override | |
| public String getName() { | |
| if (payload != null) { | |
| Object name = payload.get("name"); | |
| return name != null ? name.toString() : null; | |
| } | |
| JsonNode nameNode = userInfoJson.get("name"); | |
| return nameNode != null ? nameNode.asText() : null; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/UMC_7th/Closit/domain/user/entity/GoogleUserInfo.java between
lines 33 and 47, the getEmail and getName methods lack consistency and null
safety. To fix this, ensure both methods access payload and JSON fields
uniformly by using asText() for string extraction. Also, add null checks when
accessing JSON fields to prevent potential NullPointerExceptions by verifying
the field exists before calling asText().
| @Override | ||
| public String providerId() { | ||
| return userInfoJson.get("id").asText(); | ||
| } |
There was a problem hiding this comment.
providerId() 메서드에서 null 체크가 누락되어 있습니다.
JSON에서 "id" 필드가 없거나 null인 경우 NullPointerException이 발생할 수 있습니다. NaverUserInfo와 달리 null 체크가 없습니다.
다음과 같이 수정하여 안전성을 개선하세요:
@Override
public String providerId() {
- return userInfoJson.get("id").asText();
+ JsonNode idNode = userInfoJson.get("id");
+ if (idNode != null) {
+ return idNode.asText();
+ }
+ throw new IllegalStateException("카카오 응답에서 사용자 ID를 가져올 수 없습니다.");
}📝 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.
| @Override | |
| public String providerId() { | |
| return userInfoJson.get("id").asText(); | |
| } | |
| @Override | |
| public String providerId() { | |
| JsonNode idNode = userInfoJson.get("id"); | |
| if (idNode != null) { | |
| return idNode.asText(); | |
| } | |
| throw new IllegalStateException("카카오 응답에서 사용자 ID를 가져올 수 없습니다."); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/UMC_7th/Closit/domain/user/entity/KakaoUserInfo.java at lines
12 to 15, the providerId() method lacks a null check for the "id" field in
userInfoJson, which can cause a NullPointerException if the field is missing or
null. Modify the method to first check if userInfoJson.get("id") is not null and
contains a valid value before calling asText(), returning a safe default or null
if absent, similar to the approach used in NaverUserInfo.
| @RequiredArgsConstructor | ||
| public class NaverUserInfo implements OAuthUserInfo { | ||
|
|
||
| private final JsonNode userInfoJson; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
생성자 null 체크 추가 권장
userInfoJson 파라미터에 대한 null 체크가 없습니다. 모든 메서드에서 이 필드에 접근하므로 생성자에서 null 체크를 추가하는 것이 좋겠습니다.
@RequiredArgsConstructor
public class NaverUserInfo implements OAuthUserInfo {
- private final JsonNode userInfoJson;
+ private final JsonNode userInfoJson;
+
+ public NaverUserInfo(JsonNode userInfoJson) {
+ this.userInfoJson = Objects.requireNonNull(userInfoJson, "사용자 정보 JSON은 null일 수 없습니다.");
+ }그리고 import 추가:
import java.util.Objects;🤖 Prompt for AI Agents
In src/main/java/UMC_7th/Closit/domain/user/entity/NaverUserInfo.java at line
10, the constructor lacks a null check for the userInfoJson parameter, which is
accessed by all methods. Add a null check in the constructor using
Objects.requireNonNull(userInfoJson) to ensure it is not null. Also, add the
import statement for java.util.Objects at the top of the file.
| if (!"00".equals(userInfoJson.get("resultcode").asText())) { | ||
| log.debug("Naver API returned error: {}", userInfoJson.get("message").asText()); | ||
| throw new UserHandler(ErrorStatus.INVALID_TOKEN); | ||
| } |
There was a problem hiding this comment.
네이버 API 응답 코드 검증에서 잠재적인 NullPointerException 위험이 있습니다.
네이버 API의 resultcode 필드가 없거나 null인 경우 NullPointerException이 발생할 수 있습니다.
다음과 같이 수정하여 안전성을 개선하세요:
- // 네이버 API 응답 코드 확인
- if (!"00".equals(userInfoJson.get("resultcode").asText())) {
- log.debug("Naver API returned error: {}", userInfoJson.get("message").asText());
- throw new UserHandler(ErrorStatus.INVALID_TOKEN);
- }
+ // 네이버 API 응답 코드 확인
+ JsonNode resultcodeNode = userInfoJson.get("resultcode");
+ if (resultcodeNode == null || !"00".equals(resultcodeNode.asText())) {
+ JsonNode messageNode = userInfoJson.get("message");
+ String errorMessage = messageNode != null ? messageNode.asText() : "Unknown error";
+ log.debug("Naver API returned error: {}", errorMessage);
+ throw new UserHandler(ErrorStatus.INVALID_TOKEN);
+ }📝 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.
| if (!"00".equals(userInfoJson.get("resultcode").asText())) { | |
| log.debug("Naver API returned error: {}", userInfoJson.get("message").asText()); | |
| throw new UserHandler(ErrorStatus.INVALID_TOKEN); | |
| } | |
| // 네이버 API 응답 코드 확인 | |
| JsonNode resultcodeNode = userInfoJson.get("resultcode"); | |
| if (resultcodeNode == null || !"00".equals(resultcodeNode.asText())) { | |
| JsonNode messageNode = userInfoJson.get("message"); | |
| String errorMessage = messageNode != null ? messageNode.asText() : "Unknown error"; | |
| log.debug("Naver API returned error: {}", errorMessage); | |
| throw new UserHandler(ErrorStatus.INVALID_TOKEN); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/UMC_7th/Closit/domain/user/service/social/NaverSocialLoginService.java
around lines 64 to 67, the code directly calls asText() on
userInfoJson.get("resultcode") without checking if the "resultcode" field exists
or is null, which can cause a NullPointerException. To fix this, first check if
userInfoJson.has("resultcode") and userInfoJson.get("resultcode") is not null
before calling asText(), or use a safe method to retrieve the text value with a
default fallback. This ensures the code safely handles missing or null
"resultcode" fields and avoids exceptions.
| private final List<SocialLoginService> socialLoginServices; | ||
| private Map<SocialLoginType, SocialLoginService> serviceMap; | ||
|
|
||
| public void init() { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
메서드 접근 제한자 수정 필요
init() 메서드가 public으로 선언되어 있어 외부에서 직접 호출할 수 있습니다. 내부적으로만 사용되는 메서드이므로 private으로 변경해야 합니다.
-public void init() {
+private void init() {📝 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 void init() { | |
| private void init() { |
🤖 Prompt for AI Agents
In
src/main/java/UMC_7th/Closit/domain/user/service/social/SocialLoginFactory.java
at line 20, the init() method is declared as public but should be private since
it is intended for internal use only. Change the method's access modifier from
public to private to restrict its visibility and prevent external calls.
| import static org.mockito.ArgumentMatchers.anyString; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
MockitoExtension 사용으로 중복 제거 필요
@ExtendWith(MockitoExtension.class) 어노테이션을 사용하면 Mockito가 자동으로 초기화되므로 MockitoAnnotations.openMocks(this)는 불필요합니다.
@BeforeEach
void setup() {
- MockitoAnnotations.openMocks(this);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/test/java/UMC_7th/Closit/domain/user/service/UserAuthServiceImplTest.java
at line 29, remove the manual initialization call to
MockitoAnnotations.openMocks(this) since the @ExtendWith(MockitoExtension.class)
annotation already handles Mockito initialization automatically, eliminating
redundancy.
| @InjectMocks | ||
| private UserCommandServiceImpl userCommandService; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
@Injectmocks 의존성 주입 문제 확인 필요
UserCommandServiceImpl이 @InjectMocks로 주입되고 있지만, 이 클래스의 의존성들이 모두 모킹되어 있는지 확인이 필요합니다.
다음 스크립트로 UserCommandServiceImpl의 의존성을 확인해주세요:
🏁 Script executed:
#!/bin/bash
# UserCommandServiceImpl의 생성자나 필드 의존성 확인
ast-grep --pattern 'class UserCommandServiceImpl {
$$$
}'Length of output: 61
ast-grep로 클래스가 검색되지 않아, ripgrep을 사용해 선언 위치와 생성자/필드 의존성을 재확인해 주세요.
🏁 Script executed:
#!/bin/bash
# 1. 클래스 선언 위치 확인
rg -n "class UserCommandServiceImpl" --glob "*.java"
# 2. 선언부 포함 상위 10줄 출력하여 생성자 및 필드 의존성 확인
rg -A10 "class UserCommandServiceImpl" --glob "*.java"Length of output: 2819
UserCommandServiceImpl 테스트: 모든 의존성에 대해 @mock 추가 필요
현재 UserCommandServiceImpl에는 아래와 같이 8개의 final 필드가 있지만, 테스트에서는 UserRepository와 SecurityUtil만 모킹되어 있습니다. 나머지 의존성들이 모킹되지 않으면 NullPointerException 등 예기치 않은 오류가 발생할 수 있습니다.
– src/main/java/UMC_7th/Closit/domain/user/service/UserCommandServiceImpl.java
• userRepository
• blockRepository
• followRepository
• emailTokenService
• passwordEncoder
• securityUtil
• s3Service
• userUtil
– src/test/java/UMC_7th/Closit/domain/user/service/UserCommandServiceImplTest.java
@mock 로 선언된 의존성:
• UserRepository
• SecurityUtil
→ 아래 의존성을 @Mock으로 추가해주세요:
@Mock
private UserRepository userRepository;
+@Mock
+private BlockRepository blockRepository;
+@Mock
+private FollowRepository followRepository;
+@Mock
+private EmailTokenService emailTokenService;
+@Mock
+private PasswordEncoder passwordEncoder;
@Mock
private SecurityUtil securityUtil;
+@Mock
+private S3Service s3Service;
+@Mock
+private UserUtil userUtil;
@InjectMocks
private UserCommandServiceImpl userCommandService;🤖 Prompt for AI Agents
In src/test/java/UMC_7th/Closit/domain/user/service/UserAuthServiceImplTest.java
around lines 41 to 42, the UserCommandServiceImpl has 8 final dependencies but
only UserRepository and SecurityUtil are mocked in the test. To prevent
NullPointerException and other errors, add @Mock annotations for the remaining
dependencies: blockRepository, followRepository, emailTokenService,
passwordEncoder, s3Service, and userUtil.
| @AfterEach | ||
| void afterEach () { | ||
| userRepository.deleteByClositId("testuser"); | ||
| } |
There was a problem hiding this comment.
모킹된 Repository에서 실제 삭제 호출 문제
afterEach에서 userRepository.deleteByClositId()를 호출하고 있지만, userRepository는 @Mock으로 선언된 모킹 객체입니다. 실제 데이터 정리가 이루어지지 않을 수 있습니다.
통합 테스트가 아닌 단위 테스트에서는 실제 데이터 정리가 필요하지 않거나, 테스트 데이터베이스를 사용하는 경우 @TestMethodOrder나 @DirtiesContext 등을 고려해보세요.
🤖 Prompt for AI Agents
In src/test/java/UMC_7th/Closit/domain/user/service/UserAuthServiceImplTest.java
around lines 56 to 59, the afterEach method calls
userRepository.deleteByClositId on a mocked userRepository, which does not
perform actual data deletion. To fix this, remove or modify the afterEach method
to avoid calling delete on the mock, and instead manage test data cleanup
appropriately using test database setup or annotations like @TestMethodOrder or
@DirtiesContext if integration testing is intended.
| @Test | ||
| @DisplayName("잘못된 토큰 타입으로 소셜 로그인 시 예외 발생") | ||
| void socialLogin_InvalidTokenType() { | ||
| // given | ||
| OAuthLoginRequestDTO invalidRequest = new OAuthLoginRequestDTO(); | ||
| invalidRequest.setTokenType(null); | ||
| invalidRequest.setToken("mock_token"); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> userAuthService.socialLogin(SocialLoginType.GOOGLE, invalidRequest)) | ||
| .isInstanceOf(Exception.class); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
잘못된 토큰 타입 테스트 개선 필요
null 토큰 타입에 대한 테스트는 있지만, 구체적인 예외 타입을 검증하지 않고 있습니다. 더 구체적인 예외 검증을 추가하는 것이 좋겠습니다.
- assertThatThrownBy(() -> userAuthService.socialLogin(SocialLoginType.GOOGLE, invalidRequest))
- .isInstanceOf(Exception.class);
+ assertThatThrownBy(() -> userAuthService.socialLogin(SocialLoginType.GOOGLE, invalidRequest))
+ .isInstanceOf(GeneralException.class)
+ .hasMessageContaining("INVALID_TOKEN");📝 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.
| @Test | |
| @DisplayName("잘못된 토큰 타입으로 소셜 로그인 시 예외 발생") | |
| void socialLogin_InvalidTokenType() { | |
| // given | |
| OAuthLoginRequestDTO invalidRequest = new OAuthLoginRequestDTO(); | |
| invalidRequest.setTokenType(null); | |
| invalidRequest.setToken("mock_token"); | |
| // when & then | |
| assertThatThrownBy(() -> userAuthService.socialLogin(SocialLoginType.GOOGLE, invalidRequest)) | |
| .isInstanceOf(Exception.class); | |
| } | |
| @Test | |
| @DisplayName("잘못된 토큰 타입으로 소셜 로그인 시 예외 발생") | |
| void socialLogin_InvalidTokenType() { | |
| // given | |
| OAuthLoginRequestDTO invalidRequest = new OAuthLoginRequestDTO(); | |
| invalidRequest.setTokenType(null); | |
| invalidRequest.setToken("mock_token"); | |
| // when & then | |
| assertThatThrownBy(() -> userAuthService.socialLogin(SocialLoginType.GOOGLE, invalidRequest)) | |
| .isInstanceOf(GeneralException.class) | |
| .hasMessageContaining("INVALID_TOKEN"); | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/UMC_7th/Closit/domain/user/service/UserAuthServiceSocialLoginTest.java
around lines 130 to 141, the test for invalid token type currently only checks
for a generic Exception. Update the test to assert the specific exception type
that should be thrown when the token type is null, replacing the generic
Exception.class with the precise exception class expected from the socialLogin
method.
🔗 연관된 이슈
📝 작업 내용
참고
https://devtalk.kakao.com/t/topic/134310/7
Google
https://www.googleapis.com/oauth2/v2/userinfoKakao
https://kapi.kakao.com/v2/user/meNaver
https://openapi.naver.com/v1/nid/me📸 스크린샷
💬 리뷰 요구사항
Summary by CodeRabbit
신규 기능
버그 수정
문서화
리팩터링
테스트