[FEATURE] OAuth 소셜 로그인 전략 패턴 적용 및 공통 로직 분리#27
Conversation
WalkthroughThis update migrates the authentication system from a legacy, custom, and controller-based OAuth2 login flow to a strategy-based, modular OAuth architecture using Feign clients for Google, Kakao, and Naver. Legacy interfaces, controllers, DTOs, and services for OAuth2 and member management are removed and replaced with new strategy interfaces, record-based DTOs, a centralized OAuth service, and dedicated controllers for token and OAuth flows. Supporting constants, configuration, and exception handling are also introduced or updated, and package structures are reorganized for clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OAuthController
participant OAuthService
participant OAuthStrategyFactory
participant OAuthStrategy
participant OAuthMemberProcessor
participant TokenService
Client->>OAuthController: GET /api/oauth/redirect-url/{type}
OAuthController->>OAuthService: redirectURL(type)
OAuthService->>OAuthStrategyFactory: getStrategy(type)
OAuthStrategyFactory->>OAuthStrategy: (returns strategy)
OAuthService->>OAuthStrategy: getOauthRedirectURL()
OAuthService->>OAuthController: (returns redirect URL)
OAuthController->>Client: (redirect URL)
Client->>OAuthController: GET /api/oauth/callback/{type}?code=...
OAuthController->>OAuthService: loginOrSignUp(type, code)
OAuthService->>OAuthStrategyFactory: getStrategy(type)
OAuthStrategyFactory->>OAuthStrategy: (returns strategy)
OAuthService->>OAuthStrategy: toMember(code)
OAuthService->>OAuthMemberProcessor: processOAuthMember(type, member)
OAuthMemberProcessor->>TokenService: (issue tokens)
OAuthMemberProcessor->>OAuthService: (returns sign-in DTO)
OAuthService->>OAuthController: (returns sign-in DTO)
OAuthController->>Client: (sign-in DTO)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes 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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 24
🔭 Outside diff range comments (1)
src/main/java/com/example/spot/auth/presentation/controller/legacy/AuthController.java (1)
253-256:logout()is still unimplemented – avoid shipping dead endpoints
POST /spot/logoutreturnsnull, which will 500 at runtime. Either implement the logic or temporarily remove/annotate with@Deprecatedto prevent accidental use.
🧹 Nitpick comments (24)
src/main/java/com/example/spot/auth/application/refactor/TokenService.java (1)
5-9: Consider marking the one-method interface with@FunctionalInterface.Since
TokenServicecurrently exposes a single behaviour, adding the annotation documents the intent and enables compiler checks should additional abstract methods be introduced inadvertently.public interface TokenService { + @FunctionalInterface + // 리프레시 토큰을 사용하여 새로운 액세스 토큰을 발급 TokenResponseDTO.TokenDTO reissueToken(String refreshToken); }build.gradle (1)
39-44: Consider moving H2 totestRuntimeOnly.H2 is typically required only for tests; shipping it in production artifacts inflates size and may open unwanted JDBC URLs.
src/main/java/com/example/spot/auth/application/refactor/impl/JwtTokenService.java (2)
3-4: Class name now hides the implementation intent.The previous
JwtTokenServiceImplmade the bean’s role obvious. After the interface rename, keeping the “Impl” suffix maintains clarity and avoids cognitive collision withJwtTokenProvider.-public class JwtTokenService implements TokenService { +public class JwtTokenServiceImpl implements TokenService {Remember to adjust the
@Servicename or component scans accordingly.
54-56: Redundant self-comparison check.
memberIdByTokenis fetched from the same member that was just loaded. Comparing the two adds no value and can be removed, or it should compare againsttokenInDB.getMemberId()if the intent was repository consistency.-// 회원의 리프레시 토큰과 요청된 리프레시 토큰 비교 -if (!Objects.equals(member.getId(), memberIdByToken)) { - throw new GeneralException(ErrorStatus._INVALID_JWT); -} +// 토큰에 기록된 회원과 DB에 저장된 토큰의 회원이 일치하는지 검증 +if (!Objects.equals(tokenInDB.getMemberId(), memberIdByToken)) { + throw new GeneralException(ErrorStatus._INVALID_JWT); +}src/main/java/com/example/spot/auth/infrastructure/constants/JwtConstants.java (1)
3-9: Make the constants holder non-instantiable and final
JwtConstantsis a pure utility class. Marking itfinaland adding a private constructor prevents accidental subclassing or instantiation.-public class JwtConstants { +public final class JwtConstants { + + private JwtConstants() { + /* static utility class – do not instantiate */ + }src/main/java/com/example/spot/auth/presentation/controller/legacy/AuthController.java (1)
36-40: Consider restoring@Deprecatedon the legacy controllerThe package path already says
legacy, but an explicit@Deprecatedhelps IDEs and static-analysis tools flag callers.src/main/java/com/example/spot/auth/presentation/dto/oauth/kakao/KaKaoOAuthTokenDTO.java (1)
3-9: Consider numeric types for expiration fields.The record structure is appropriate for Kakao's OAuth token response. However, consider using
IntegerorLongtypes forexpires_inandrefresh_token_expires_infields to facilitate time-based calculations and validations.public record KaKaoOAuthTokenDTO( String token_type, String access_token, String refresh_token, - String expires_in, - String refresh_token_expires_in + Integer expires_in, + Integer refresh_token_expires_in ) {src/main/java/com/example/spot/auth/application/refactor/strategy/OAuthStrategy.java (2)
12-12: Use English for code comments.Consider using English for code comments to maintain consistency across the codebase.
- Member toMember(String code); // 전략별 구현에서 Member 객체 생성 + Member toMember(String code); // Convert authorization code to Member object via strategy-specific implementation
10-12: Consider adding exception declarations.The methods may throw exceptions during OAuth operations (network issues, invalid codes, etc.). Consider adding
throwsdeclarations or documenting expected exceptions.- String getOauthRedirectURL(); + String getOauthRedirectURL() throws OAuthException; - Member toMember(String code); + Member toMember(String code) throws OAuthException;src/main/java/com/example/spot/auth/presentation/dto/oauth/naver/NaverOAuthTokenDTO.java (1)
3-10: Good error handling in OAuth token DTO.The record appropriately includes error fields (
error,error_description) for robust error handling. The structure aligns well with Naver's OAuth API response format.Consider using
Integertype forexpires_into facilitate time-based operations.public record NaverOAuthTokenDTO( String access_token, String token_type, String refresh_token, - String expires_in, + Integer expires_in, String error, String error_description ) {src/main/java/com/example/spot/auth/application/refactor/strategy/OAuthStrategyFactory.java (1)
17-20: Consider handling potential duplicate LoginType conflicts.The
Collectors.toMapwill throwIllegalStateExceptionif multiple strategies return the sameLoginType. While this indicates a configuration error, consider adding explicit validation or usingtoMapwith a merge function for clearer error handling.public OAuthStrategyFactory(List<OAuthStrategy> strategies) { - this.strategyMap = strategies.stream() - .collect(Collectors.toMap(OAuthStrategy::getType, s -> s)); + this.strategyMap = strategies.stream() + .collect(Collectors.toMap( + OAuthStrategy::getType, + s -> s, + (existing, replacement) -> { + throw new IllegalStateException("Duplicate LoginType: " + existing.getType()); + })); }src/main/java/com/example/spot/auth/presentation/dto/oauth/naver/NaverUser.java (1)
8-14: Consider adding @JsonProperty annotations for explicit field mapping.While the field names appear to match Naver's API response format, adding
@JsonPropertyannotations would make the mapping explicit and prevent issues if field names change. Also, consider naming consistency -thumbnail_imageuses snake_case while other fields use camelCase.+import com.fasterxml.jackson.annotation.JsonProperty; + public record NaverPropertiesDTO( + @JsonProperty("id") String id, + @JsonProperty("name") String name, + @JsonProperty("email") String email, + @JsonProperty("thumbnail_image") String thumbnail_image ) {src/main/java/com/example/spot/auth/infrastructure/client/google/GoogleAuthClient.java (1)
14-22: Consider using constants for request parameter names.While the implementation is functionally correct, consider using constants from
AuthConstantsfor request parameter names to maintain consistency across the codebase.@PostMapping("/token") GoogleOAuthToken getGoogleAccessToken( @RequestHeader(HEADER_CONTENT_TYPE) String contentType, - @RequestParam String code, - @RequestParam String clientId, - @RequestParam String clientSecret, - @RequestParam String redirectUri, - @RequestParam String grantType + @RequestParam("code") String code, + @RequestParam(CLIENT_ID) String clientId, + @RequestParam(CLIENT_SECRET) String clientSecret, + @RequestParam(REDIRECT_URI) String redirectUri, + @RequestParam(GRANT_TYPE) String grantType );You'll need to add the missing imports:
+import static com.example.spot.auth.infrastructure.constants.AuthConstants.CLIENT_SECRET; +import static com.example.spot.auth.infrastructure.constants.AuthConstants.REDIRECT_URI; +import static com.example.spot.auth.infrastructure.constants.AuthConstants.GRANT_TYPE;src/main/java/com/example/spot/auth/infrastructure/client/naver/NaverAuthClient.java (2)
16-22: Use constants consistently for all parameter names.Some request parameters use constants from
AuthConstantswhile others use string literals. For consistency, consider using constants for all parameters.NaverOAuthTokenDTO getNaverAccessToken( @RequestParam(GRANT_TYPE) String grantType, @RequestParam(CLIENT_ID) String clientId, @RequestParam(CLIENT_SECRET) String clientSecret, - @RequestParam String code, - @RequestParam String state + @RequestParam("code") String code, + @RequestParam(STATE) String state );You'll need to add the missing import:
+import static com.example.spot.auth.infrastructure.constants.AuthConstants.STATE;
24-25: Remove unnecessary blank lines.); - - }src/main/java/com/example/spot/auth/infrastructure/oauth/KaKaoOauth.java (1)
41-52: Consider using UriComponentsBuilder for URL construction.The manual URL building with string concatenation is error-prone and less readable than using Spring's UriComponentsBuilder.
public String getOauthRedirectURL() { - Map<String, Object> params = new HashMap<>(); - params.put(CLIENT_ID, KAKAO_SNS_CLIENT_ID); - params.put(REDIRECT_URI, KAKAO_SNS_CALLBACK_LOGIN_URL); - params.put(RESPONSE_TYPE, RESPONSE_TYPE_CODE); - - String parameterString = params.entrySet().stream() - .map(x -> x.getKey() + KEY_VALUE_DELIMITER + x.getValue()) - .collect(Collectors.joining(QUERY_DELIMITER)); - - return KAKAO_SNS_URL + QUERY_PREFIX + parameterString; + return UriComponentsBuilder.fromHttpUrl(KAKAO_SNS_URL) + .queryParam(CLIENT_ID, KAKAO_SNS_CLIENT_ID) + .queryParam(REDIRECT_URI, KAKAO_SNS_CALLBACK_LOGIN_URL) + .queryParam(RESPONSE_TYPE, RESPONSE_TYPE_CODE) + .build() + .toUriString(); }src/main/java/com/example/spot/auth/presentation/dto/oauth/kakao/KaKaoUser.java (1)
10-17: Consider adding validation annotations for email fields.Since email validation is critical for user authentication, consider adding validation to ensure data integrity.
public record KaKaoAccountDTO( Boolean has_email, Boolean email_needs_agreement, Boolean is_email_valid, Boolean is_email_verified, + @Email(message = "Invalid email format") String email ) { }src/main/java/com/example/spot/auth/presentation/controller/refactor/TokenController.java (1)
38-38: Consider using _OK instead of _CREATED for token reissuance.Token reissuance is typically considered an update operation rather than resource creation. The HTTP 200 OK status might be more semantically appropriate.
-return ApiResponse.onSuccess(SuccessStatus._CREATED, tokenService.reissueToken(refreshToken)); +return ApiResponse.onSuccess(SuccessStatus._OK, tokenService.reissueToken(refreshToken));src/main/java/com/example/spot/auth/infrastructure/constants/AuthConstants.java (1)
19-19: Consider using a more descriptive state value.The hardcoded
STATE_STRINGappears to be a placeholder. OAuth2 state parameter should typically be a random value to prevent CSRF attacks.Consider generating the state value dynamically or using a more descriptive constant name if this is intentionally static:
- public static final String STATE_STRING = "STATE_STRING"; + public static final String DEFAULT_STATE = "SPOT_OAUTH_STATE";src/main/java/com/example/spot/member/domain/Member.java (1)
107-108: Consider implications of generated phone number and current date birth.Setting birth date to current date and generating a random phone number may not be appropriate for OAuth users. Consider using null values or explicit placeholders.
- .phone(MemberUtils.generatePhoneNumber()) - .birth(LocalDate.now()) + .phone(null) // Let user provide later + .birth(null) // Let user provide latersrc/main/java/com/example/spot/auth/presentation/controller/refactor/OAuthController.java (1)
26-26: Consider using @PathVariable validation.Instead of manual string conversion, consider using Spring's built-in enum conversion with proper validation.
-@GetMapping("/redirect-url/{type}") -public String getRedirectUrl(@PathVariable("type") String type) { +@GetMapping("/redirect-url/{type}") +public String getRedirectUrl(@PathVariable("type") LoginType type) {src/main/java/com/example/spot/auth/infrastructure/oauth/GoogleOauth.java (1)
44-57: Improve URL construction robustness.Consider using UriComponentsBuilder for more robust and readable URL construction, and add URL encoding for parameter values.
+import org.springframework.web.util.UriComponentsBuilder; public String getOauthRedirectURL() { - Map<String, String> params = new HashMap<>(); - - params.put(SCOPE, GOOGLE_DATA_ACCESS_SCOPE); - params.put(RESPONSE_TYPE, RESPONSE_TYPE_CODE); - params.put(CLIENT_ID, GOOGLE_SNS_CLIENT_ID); - params.put(REDIRECT_URI, GOOGLE_SNS_CALLBACK_LOGIN_URL); - - String parameterString = params.entrySet().stream() - .map(x -> x.getKey() + KEY_VALUE_DELIMITER + x.getValue()) - .collect(Collectors.joining(QUERY_DELIMITER)); - - return GOOGLE_SNS_URL + QUERY_PREFIX + parameterString; + return UriComponentsBuilder.fromHttpUrl(GOOGLE_SNS_URL) + .queryParam(SCOPE, GOOGLE_DATA_ACCESS_SCOPE) + .queryParam(RESPONSE_TYPE, RESPONSE_TYPE_CODE) + .queryParam(CLIENT_ID, GOOGLE_SNS_CLIENT_ID) + .queryParam(REDIRECT_URI, GOOGLE_SNS_CALLBACK_LOGIN_URL) + .build() + .toUriString(); }src/main/java/com/example/spot/auth/application/refactor/impl/OAuthMemberProcessor.java (1)
88-93: Consider optimizing with a single queryThe current implementation makes three separate database queries. For better performance, consider creating a custom repository method that checks all three conditions in a single query.
Example approach:
// In MemberRepository @Query("SELECT COUNT(DISTINCT t.type) = 3 FROM (" + "SELECT 'theme' as type FROM PreferredTheme WHERE memberId = :memberId " + "UNION SELECT 'region' FROM PreferredRegion WHERE memberId = :memberId " + "UNION SELECT 'reason' FROM StudyJoinReason WHERE memberId = :memberId" + ") t") boolean isSpotMember(@Param("memberId") Long memberId);src/main/java/com/example/spot/auth/infrastructure/oauth/NaverOauth.java (1)
64-64: Remove unnecessary empty line- - + public NaverUser requestUserInfo(NaverOAuthTokenDTO naverOAuthTokenDTO) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (70)
build.gradle(2 hunks)src/main/java/com/example/spot/auth/application/legacy/AuthService.java(1 hunks)src/main/java/com/example/spot/auth/application/legacy/AuthServiceImpl.java(1 hunks)src/main/java/com/example/spot/auth/application/legacy/NaverOAuthService.java(0 hunks)src/main/java/com/example/spot/auth/application/refactor/KakaoAuthService.java(0 hunks)src/main/java/com/example/spot/auth/application/refactor/OAuthService.java(1 hunks)src/main/java/com/example/spot/auth/application/refactor/TokenService.java(1 hunks)src/main/java/com/example/spot/auth/application/refactor/impl/JwtTokenService.java(2 hunks)src/main/java/com/example/spot/auth/application/refactor/impl/KakaoAuthServiceImpl.java(0 hunks)src/main/java/com/example/spot/auth/application/refactor/impl/OAuthMemberProcessor.java(1 hunks)src/main/java/com/example/spot/auth/application/refactor/strategy/OAuthStrategy.java(1 hunks)src/main/java/com/example/spot/auth/application/refactor/strategy/OAuthStrategyFactory.java(1 hunks)src/main/java/com/example/spot/auth/application/refactor/strategy/provider/GoogleOAuthStrategy.java(1 hunks)src/main/java/com/example/spot/auth/application/refactor/strategy/provider/KakaoOAuthStrategy.java(1 hunks)src/main/java/com/example/spot/auth/application/refactor/strategy/provider/NaverOAuthStrategy.java(1 hunks)src/main/java/com/example/spot/auth/exception/UnsupportedSocialLoginTypeException.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/client/google/GoogleApiClient.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/client/google/GoogleAuthClient.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/client/kakao/KaKaoApiClient.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/client/kakao/KaKaoAuthClient.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/client/naver/NaverApiClient.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/client/naver/NaverAuthClient.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/constants/AuthConstants.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/constants/JwtConstants.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/kakao/KakaoOAuthClient.java(0 hunks)src/main/java/com/example/spot/auth/infrastructure/oauth/GoogleOauth.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/oauth/KaKaoOauth.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/oauth/NaverOauth.java(1 hunks)src/main/java/com/example/spot/auth/presentation/controller/legacy/AuthController.java(12 hunks)src/main/java/com/example/spot/auth/presentation/controller/refactor/JwtTokenController.java(0 hunks)src/main/java/com/example/spot/auth/presentation/controller/refactor/KakaoAuthController.java(0 hunks)src/main/java/com/example/spot/auth/presentation/controller/refactor/OAuthController.java(1 hunks)src/main/java/com/example/spot/auth/presentation/controller/refactor/TokenController.java(1 hunks)src/main/java/com/example/spot/auth/presentation/dto/google/GoogleExampleResponse.java(0 hunks)src/main/java/com/example/spot/auth/presentation/dto/kakao/KaKaoOAuthToken.java(0 hunks)src/main/java/com/example/spot/auth/presentation/dto/kakao/KaKaoUser.java(0 hunks)src/main/java/com/example/spot/auth/presentation/dto/naver/NaverCallback.java(0 hunks)src/main/java/com/example/spot/auth/presentation/dto/naver/NaverMember.java(0 hunks)src/main/java/com/example/spot/auth/presentation/dto/naver/NaverOAuthToken.java(0 hunks)src/main/java/com/example/spot/auth/presentation/dto/oauth/google/GoogleOAuthToken.java(1 hunks)src/main/java/com/example/spot/auth/presentation/dto/oauth/google/GoogleUser.java(1 hunks)src/main/java/com/example/spot/auth/presentation/dto/oauth/kakao/KaKaoOAuthTokenDTO.java(1 hunks)src/main/java/com/example/spot/auth/presentation/dto/oauth/kakao/KaKaoUser.java(1 hunks)src/main/java/com/example/spot/auth/presentation/dto/oauth/naver/NaverOAuthTokenDTO.java(1 hunks)src/main/java/com/example/spot/auth/presentation/dto/oauth/naver/NaverUser.java(1 hunks)src/main/java/com/example/spot/common/api/exception/base/UnsupportedException.java(1 hunks)src/main/java/com/example/spot/common/config/FeignConfig.java(1 hunks)src/main/java/com/example/spot/common/config/WebSecurity.java(5 hunks)src/main/java/com/example/spot/common/security/filters/JwtAuthenticationFilter.java(3 hunks)src/main/java/com/example/spot/common/security/oauth/CustomOAuth2UserService.java(0 hunks)src/main/java/com/example/spot/common/security/oauth/CustomOAuthSuccessHandler.java(0 hunks)src/main/java/com/example/spot/common/security/oauth/OAuthUserInfoFactory.java(0 hunks)src/main/java/com/example/spot/common/security/oauth/adpter/CustomOAuth2User.java(0 hunks)src/main/java/com/example/spot/common/security/oauth/adpter/OAuth2UserInfo.java(0 hunks)src/main/java/com/example/spot/common/security/oauth/adpter/google/GoogleUserInfo.java(0 hunks)src/main/java/com/example/spot/member/application/MemberInfoService.java(1 hunks)src/main/java/com/example/spot/member/application/MemberPreferenceService.java(1 hunks)src/main/java/com/example/spot/member/application/MemberTestSupportService.java(1 hunks)src/main/java/com/example/spot/member/application/impl/MemberInfoServiceImpl.java(1 hunks)src/main/java/com/example/spot/member/application/impl/MemberPreferenceServiceImpl.java(1 hunks)src/main/java/com/example/spot/member/application/impl/MemberTestSupportServiceImpl.java(2 hunks)src/main/java/com/example/spot/member/application/legacy/MemberService.java(0 hunks)src/main/java/com/example/spot/member/application/legacy/MemberServiceImpl.java(0 hunks)src/main/java/com/example/spot/member/application/refactor/MemberPreferenceService.java(0 hunks)src/main/java/com/example/spot/member/application/refactor/MemberTestSupportService.java(0 hunks)src/main/java/com/example/spot/member/domain/Member.java(3 hunks)src/main/java/com/example/spot/member/presentation/controller/legacy/MemberController.java(0 hunks)src/main/java/com/example/spot/member/presentation/controller/refactor/MemberInfoController.java(1 hunks)src/main/java/com/example/spot/member/presentation/controller/refactor/MemberPreferenceController.java(1 hunks)src/main/java/com/example/spot/member/presentation/controller/refactor/MemberTestSupportController.java(1 hunks)
💤 Files with no reviewable changes (23)
- src/main/java/com/example/spot/common/security/oauth/adpter/google/GoogleUserInfo.java
- src/main/java/com/example/spot/member/application/legacy/MemberService.java
- src/main/java/com/example/spot/auth/presentation/dto/google/GoogleExampleResponse.java
- src/main/java/com/example/spot/common/security/oauth/adpter/OAuth2UserInfo.java
- src/main/java/com/example/spot/auth/application/refactor/KakaoAuthService.java
- src/main/java/com/example/spot/common/security/oauth/adpter/CustomOAuth2User.java
- src/main/java/com/example/spot/common/security/oauth/OAuthUserInfoFactory.java
- src/main/java/com/example/spot/auth/presentation/dto/kakao/KaKaoOAuthToken.java
- src/main/java/com/example/spot/common/security/oauth/CustomOAuthSuccessHandler.java
- src/main/java/com/example/spot/member/application/legacy/MemberServiceImpl.java
- src/main/java/com/example/spot/member/application/refactor/MemberTestSupportService.java
- src/main/java/com/example/spot/auth/presentation/dto/naver/NaverCallback.java
- src/main/java/com/example/spot/member/application/refactor/MemberPreferenceService.java
- src/main/java/com/example/spot/common/security/oauth/CustomOAuth2UserService.java
- src/main/java/com/example/spot/auth/presentation/dto/naver/NaverMember.java
- src/main/java/com/example/spot/auth/presentation/controller/refactor/JwtTokenController.java
- src/main/java/com/example/spot/member/presentation/controller/legacy/MemberController.java
- src/main/java/com/example/spot/auth/presentation/dto/naver/NaverOAuthToken.java
- src/main/java/com/example/spot/auth/application/legacy/NaverOAuthService.java
- src/main/java/com/example/spot/auth/presentation/dto/kakao/KaKaoUser.java
- src/main/java/com/example/spot/auth/application/refactor/impl/KakaoAuthServiceImpl.java
- src/main/java/com/example/spot/auth/presentation/controller/refactor/KakaoAuthController.java
- src/main/java/com/example/spot/auth/infrastructure/kakao/KakaoOAuthClient.java
🧰 Additional context used
🧬 Code Graph Analysis (12)
src/main/java/com/example/spot/auth/exception/UnsupportedSocialLoginTypeException.java (1)
src/main/java/com/example/spot/common/api/exception/base/UnsupportedException.java (1)
UnsupportedException(7-12)
src/main/java/com/example/spot/member/application/impl/MemberTestSupportServiceImpl.java (1)
src/main/java/com/example/spot/auth/presentation/dto/token/TokenResponseDTO.java (1)
TokenResponseDTO(5-25)
src/main/java/com/example/spot/member/presentation/controller/refactor/MemberInfoController.java (2)
src/main/java/com/example/spot/member/presentation/controller/refactor/MemberTestSupportController.java (1)
RestController(19-54)src/main/java/com/example/spot/member/presentation/controller/refactor/MemberPreferenceController.java (1)
RestController(21-131)
src/main/java/com/example/spot/auth/presentation/controller/refactor/TokenController.java (1)
src/main/java/com/example/spot/auth/presentation/dto/token/TokenResponseDTO.java (1)
TokenResponseDTO(5-25)
src/main/java/com/example/spot/auth/infrastructure/client/google/GoogleApiClient.java (1)
src/main/java/com/example/spot/auth/infrastructure/constants/AuthConstants.java (1)
AuthConstants(3-27)
src/main/java/com/example/spot/auth/infrastructure/client/kakao/KaKaoAuthClient.java (1)
src/main/java/com/example/spot/auth/infrastructure/constants/AuthConstants.java (1)
AuthConstants(3-27)
src/main/java/com/example/spot/auth/infrastructure/client/google/GoogleAuthClient.java (1)
src/main/java/com/example/spot/auth/infrastructure/constants/AuthConstants.java (1)
AuthConstants(3-27)
src/main/java/com/example/spot/auth/application/refactor/strategy/OAuthStrategyFactory.java (4)
src/main/java/com/example/spot/auth/exception/UnsupportedSocialLoginTypeException.java (1)
UnsupportedSocialLoginTypeException(6-11)src/main/java/com/example/spot/auth/application/refactor/strategy/provider/GoogleOAuthStrategy.java (1)
Component(12-34)src/main/java/com/example/spot/auth/application/refactor/strategy/provider/NaverOAuthStrategy.java (1)
Component(12-36)src/main/java/com/example/spot/auth/application/refactor/strategy/provider/KakaoOAuthStrategy.java (1)
Component(12-35)
src/main/java/com/example/spot/auth/infrastructure/oauth/NaverOauth.java (5)
src/main/java/com/example/spot/auth/infrastructure/constants/AuthConstants.java (1)
AuthConstants(3-27)src/main/java/com/example/spot/auth/infrastructure/constants/JwtConstants.java (1)
JwtConstants(3-9)src/main/java/com/example/spot/auth/application/refactor/strategy/provider/NaverOAuthStrategy.java (1)
Component(12-36)src/main/java/com/example/spot/auth/infrastructure/oauth/KaKaoOauth.java (1)
Component(25-70)src/main/java/com/example/spot/auth/infrastructure/oauth/GoogleOauth.java (1)
Component(26-69)
src/main/java/com/example/spot/auth/application/legacy/AuthService.java (1)
src/main/java/com/example/spot/auth/presentation/dto/token/TokenResponseDTO.java (1)
TokenResponseDTO(5-25)
src/main/java/com/example/spot/auth/application/refactor/impl/OAuthMemberProcessor.java (1)
src/main/java/com/example/spot/auth/presentation/dto/token/TokenResponseDTO.java (1)
TokenResponseDTO(5-25)
src/main/java/com/example/spot/auth/infrastructure/client/naver/NaverAuthClient.java (1)
src/main/java/com/example/spot/auth/infrastructure/constants/AuthConstants.java (1)
AuthConstants(3-27)
🔇 Additional comments (48)
src/main/java/com/example/spot/member/application/MemberInfoService.java (1)
1-1: LGTM: Clean package refactoringThe package relocation from
.refactorto the main.applicationpackage aligns with the PR's goal of consolidating member services.src/main/java/com/example/spot/member/application/impl/MemberInfoServiceImpl.java (1)
1-1: LGTM: Consistent package refactoringThe package declaration and import statement have been correctly updated to reflect the new package structure while maintaining all existing functionality.
Also applies to: 4-4
src/main/java/com/example/spot/member/application/impl/MemberPreferenceServiceImpl.java (1)
1-1: LGTM: Package refactoring maintains functionalityThe package declaration and import statement updates are consistent with the broader refactoring effort. All existing preference management logic remains intact.
Also applies to: 8-8
src/main/java/com/example/spot/member/application/impl/MemberTestSupportServiceImpl.java (1)
1-1: LGTM: Consistent refactoring with minor improvementsPackage declaration and import updates align with the refactoring effort. The minor formatting improvements in the
saveRefreshTokenmethod enhance readability.Also applies to: 9-9
src/main/java/com/example/spot/member/application/MemberTestSupportService.java (1)
1-13: LGTM: Well-designed interface completes refactoringThe interface provides a clean API for test support functionality with appropriate method signatures and return types. This successfully consolidates the test support service into the main application package.
src/main/java/com/example/spot/member/presentation/controller/refactor/MemberTestSupportController.java (3)
6-6: Package restructuring looks good.The import update reflects the consolidation of service interfaces from the
refactorsubpackage to the mainapplicationpackage, which aligns with the broader refactoring effort.
13-17: Consistent import formatting applied.The import statements have been properly reordered and formatted consistently across the file.
25-53: Method formatting standardized without functional changes.The indentation has been consistently changed from tabs to spaces, and the method logic remains unchanged. The controller continues to properly delegate to the service layer.
src/main/java/com/example/spot/member/presentation/controller/refactor/MemberPreferenceController.java (3)
6-6: Package restructuring consistently applied.The
MemberPreferenceServiceimport has been correctly updated to reference the consolidated package structure, maintaining consistency with other member-related controllers.
14-19: Import organization improved.The Spring Web and validation imports have been properly grouped and formatted for better readability.
27-130: API endpoints maintain functionality with improved formatting.All controller methods retain their original functionality while benefiting from consistent indentation and spacing improvements. The delegation to the service layer and use of
SecurityUtils.getCurrentUserId()remains proper.src/main/java/com/example/spot/member/presentation/controller/refactor/MemberInfoController.java (3)
6-6: Service import consistently updated.The
MemberInfoServiceimport follows the same package consolidation pattern as other member controllers, ensuring consistency across the refactored codebase.
14-18: Import formatting standardized.The import statements are properly organized and formatted consistently with the other refactored controllers.
26-42: Controller method maintains functionality with improved formatting.The
updateMemberInfomethod retains its original logic while benefiting from consistent spacing and indentation improvements.src/main/java/com/example/spot/member/application/MemberPreferenceService.java (2)
1-9: Clean package structure and appropriate imports.The interface is properly placed in the main
applicationpackage and uses static imports for response DTOs effectively, improving readability.
10-22: Well-designed service interface with consistent method signatures.The interface provides a clean separation of concerns for member preference management with:
- Consistent use of
Long memberIdparameter across all methods- Appropriate pairing of update/get operations for each preference type (themes, regions, study reasons)
- Proper use of request/response DTOs for each operation
src/main/java/com/example/spot/auth/application/refactor/TokenService.java (1)
5-9: All injections updated to TokenService
- The only remaining occurrence of
JwtTokenServiceis the implementation class in
src/main/java/com/example/spot/auth/application/refactor/impl/JwtTokenService.java, which now implements the renamedTokenServiceinterface.- No other
@Autowiredinjections orimplements JwtTokenServicereferences were found.src/main/java/com/example/spot/common/api/exception/base/UnsupportedException.java (1)
7-12: Good abstraction layer for “unsupported” scenarios.A dedicated base exception keeps the domain-level variants tidy. No issues spotted.
src/main/java/com/example/spot/common/security/filters/JwtAuthenticationFilter.java (3)
3-3: Good: dropped unusedMemberServicedependency.Removes an unnecessary constructor parameter and tightens the filter’s responsibility.
63-66: Behaviour unchanged, brackets improve readability.Adding braces around the
if (isValidToken(token))body prevents future maintenance slip-ups.
90-105:authenticate(String)mixes “token” and “principal”.
tempAuthenticateUserpasses an email toauthenticate, which then forwards it as the token argument tojwtTokenProvider.getAuthentication. UnlessJwtTokenProvidertolerates raw emails, this may break temp-token flows.Would you confirm that
JwtTokenProvider.getAuthentication(String, UserDetails)works when the first parameter is an email rather than a JWT? If not, we should overloadgetAuthenticationor supply the actual temp token.src/main/java/com/example/spot/auth/exception/UnsupportedSocialLoginTypeException.java (1)
6-11: Looks good – exception wiring is clearThe subclass cleanly delegates to
UnsupportedException; no issues found.src/main/java/com/example/spot/common/config/FeignConfig.java (1)
6-9: Configuration is minimal and correct
@EnableFeignClientson the root package will pick up every Feign client you introduced. Nothing to add.src/main/java/com/example/spot/auth/presentation/dto/oauth/google/GoogleOAuthToken.java (1)
1-9: No Jackson naming strategy found and annotations are redundant
- No
spring.jackson.property-naming-strategy,PropertyNamingStrategyor@JsonNamingdefinitions detected.- Jackson’s built-in record support uses component names as JSON property names, so your snake-case record components (e.g.
access_token) will bind correctly by default.- You can safely omit the
@JsonPropertyannotations here.src/main/java/com/example/spot/auth/presentation/dto/oauth/google/GoogleUser.java (1)
3-12: Well-structured OAuth user data record.The record design is clean and follows immutable data carrier patterns appropriately. Field types match Google's OAuth API response format correctly.
Consider adding validation annotations if input validation is required at this layer, though it may be handled elsewhere in the OAuth flow.
src/main/java/com/example/spot/auth/application/refactor/strategy/OAuthStrategy.java (1)
6-13: Excellent strategy pattern implementation.The interface design effectively abstracts OAuth provider differences and follows the strategy pattern correctly. Method signatures are clear and purposeful.
src/main/java/com/example/spot/auth/infrastructure/client/naver/NaverApiClient.java (2)
10-16: Clean Feign client implementation.The declarative approach using Feign client effectively replaces legacy HTTP handling. The configuration is correct for Naver's OpenAPI, and the method signature appropriately handles authorization via header parameter.
3-3: Good use of constants for header names.Using the imported
HEADER_AUTHORIZATIONconstant promotes consistency and reduces magic strings.src/main/java/com/example/spot/auth/infrastructure/client/google/GoogleApiClient.java (2)
10-15: LGTM! Clean Feign client implementation.The interface correctly targets Google's OAuth2 userinfo endpoint with proper annotations and constant usage. The method signature is clear and follows Feign conventions.
13-14: Authorization header correctly includes “Bearer ” prefixThe
requestUserInfomethod inGoogleOauth.javaprependsBEARER_PREFIX(“Bearer ”) to the access token when calling
googleApiClient.getGoogleUserInfo, ensuring the header is properly formatted. No further changes needed.src/main/java/com/example/spot/auth/application/refactor/strategy/OAuthStrategyFactory.java (1)
12-26: LGTM! Well-implemented factory pattern.The factory correctly implements the strategy pattern with:
- Clean constructor injection of all strategies
- Efficient map-based strategy lookup
- Proper error handling with custom exceptions
- Thread-safe immutable design
src/main/java/com/example/spot/auth/infrastructure/client/kakao/KaKaoApiClient.java (1)
11-18: LGTM! Proper Feign client for Kakao user info.The interface correctly targets Kakao's user info endpoint with appropriate headers and follows Feign conventions well.
src/main/java/com/example/spot/auth/presentation/dto/oauth/naver/NaverUser.java (1)
3-15: LGTM! Clean record-based DTO structure.The record structure appropriately models Naver's OAuth user response with proper nesting and immutable design. The field names align with Naver's API conventions.
src/main/java/com/example/spot/auth/application/refactor/strategy/provider/GoogleOAuthStrategy.java (1)
14-34: LGTM! Clean strategy implementation following the pattern.The
GoogleOAuthStrategycorrectly implements theOAuthStrategyinterface with proper delegation toGoogleOauthfor OAuth operations. ThetoMembermethod appropriately converts the OAuth response to a domainMemberobject using the factory method.src/main/java/com/example/spot/auth/application/refactor/strategy/provider/NaverOAuthStrategy.java (1)
14-36: LGTM! Correct implementation of Naver OAuth strategy.The
NaverOAuthStrategyproperly implements theOAuthStrategyinterface with appropriate field mapping in thetoMembermethod, correctly using separate fields for name (user.response().name()) and email (user.response().email()).src/main/java/com/example/spot/auth/infrastructure/oauth/KaKaoOauth.java (1)
65-67: LGTM! Good use of constants for token formatting.The private helper method correctly uses JwtConstants.BEARER_PREFIX for consistent token formatting across the application.
src/main/java/com/example/spot/auth/presentation/dto/oauth/kakao/KaKaoUser.java (1)
3-25: LGTM! Clean record-based DTO design.The use of Java records for immutable DTOs is excellent. The nested structure properly models Kakao's API response format, and the snake_case field naming aligns with the external API contract.
src/main/java/com/example/spot/auth/presentation/controller/refactor/TokenController.java (1)
27-34: Excellent API documentation.The OpenAPI documentation clearly describes the token reissuance process, including the conditions and behavior. This will help API consumers understand the endpoint properly.
src/main/java/com/example/spot/auth/application/legacy/AuthService.java (3)
4-4: LGTM! Proper import addition for TokenResponseDTO.The new import is correctly added to support the return type of the
verifyEmailmethod that returnsTokenResponseDTO.TempTokenDTO.
11-11: Good use of @deprecated annotation.The deprecation annotation properly signals that this interface is legacy code and should be migrated to the new OAuth strategy pattern.
1-39: All AuthService implementations have removed Naver OAuth methodsI’ve confirmed that
AuthServiceImpl.javais the only class implementingAuthServiceand contains no references to “naver.” No compilation issues should arise from the interface changes.src/main/java/com/example/spot/common/config/WebSecurity.java (4)
4-4: LGTM! Correct import update for refactored UserDetailsService.The import correctly points to the new
UserDetailsServiceCustomin the refactor package, aligning with the architectural changes.
38-38: LGTM! Updated CSRF configuration syntax.The lambda-based CSRF configuration follows modern Spring Security patterns and is correctly implemented.
82-82: LGTM! Proper JWT filter configuration.The JWT authentication filter is correctly instantiated with the updated UserDetailsService from the refactor package.
51-57: Confirm and clean up deprecated OAuth endpoint matchersI didn’t find any controller or handler mappings for these OAuth endpoints in the codebase. Please verify whether they’re still required under the new strategy and remove their corresponding
requestMatchersfromWebSecurity.javaif they’re unused:
.requestMatchers(new AntPathRequestMatcher("/spot/members/sign-in/naver", "POST")).permitAll().requestMatchers(new AntPathRequestMatcher("/spot/members/sign-in/naver/redirect", "GET")).permitAll().requestMatchers(new AntPathRequestMatcher("/spot/members/sign-in/naver/authorize/test", "GET")).permitAll().requestMatchers(new AntPathRequestMatcher("/spot/login/kakao", "GET")).permitAll().requestMatchers(new AntPathRequestMatcher("/spot/members/sign-in/kakao", "GET")).permitAll().requestMatchers(new AntPathRequestMatcher("/spot/members/sign-in/kakao/redirect", "GET")).permitAll()src/main/java/com/example/spot/member/domain/Member.java (1)
33-33: LGTM - Good encapsulation with private constructor.Restricting the constructor to private access and providing a static factory method improves encapsulation and ensures controlled object creation.
src/main/java/com/example/spot/auth/application/refactor/impl/OAuthMemberProcessor.java (1)
79-86: LGTM!The refresh token management logic is clean and ensures only one active refresh token per member.
src/main/java/com/example/spot/auth/infrastructure/oauth/NaverOauth.java (1)
65-72: LGTM!The user info retrieval implementation is clean and follows the same pattern as other OAuth providers.
src/main/java/com/example/spot/auth/application/legacy/AuthServiceImpl.java
Show resolved
Hide resolved
src/main/java/com/example/spot/auth/application/refactor/impl/OAuthMemberProcessor.java
Show resolved
Hide resolved
src/main/java/com/example/spot/auth/application/refactor/impl/OAuthMemberProcessor.java
Show resolved
Hide resolved
src/main/java/com/example/spot/auth/application/refactor/impl/OAuthMemberProcessor.java
Show resolved
Hide resolved
src/main/java/com/example/spot/auth/infrastructure/oauth/NaverOauth.java
Show resolved
Hide resolved
src/main/java/com/example/spot/auth/presentation/controller/refactor/OAuthController.java
Show resolved
Hide resolved
src/main/java/com/example/spot/auth/presentation/controller/refactor/OAuthController.java
Show resolved
Hide resolved
src/main/java/com/example/spot/auth/presentation/controller/refactor/TokenController.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/example/spot/common/infrastructure/feign/SafeFeignExecutor.java (1)
16-17: Consider internationalization for error messages.The error message is hardcoded in Korean ("Feign API 호출 실패"), which may not be suitable for international deployments or future localization needs.
Consider using a message properties file or constants for error messages:
- throw new ExternalApiException( - "Feign API 호출 실패: " + extractMessage(e), e); + throw new ExternalApiException( + ErrorMessages.FEIGN_API_CALL_FAILED + ": " + extractMessage(e), e);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/example/spot/auth/infrastructure/constants/AuthConstants.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/constants/JwtConstants.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/oauth/GoogleOauth.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/oauth/KaKaoOauth.java(1 hunks)src/main/java/com/example/spot/auth/infrastructure/oauth/NaverOauth.java(1 hunks)src/main/java/com/example/spot/common/api/exception/base/ExternalApiException.java(1 hunks)src/main/java/com/example/spot/common/infrastructure/feign/SafeFeignExecutor.java(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/main/java/com/example/spot/common/api/exception/base/ExternalApiException.java
- src/main/java/com/example/spot/auth/infrastructure/constants/JwtConstants.java
- src/main/java/com/example/spot/auth/infrastructure/constants/AuthConstants.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/example/spot/auth/infrastructure/oauth/GoogleOauth.java
- src/main/java/com/example/spot/auth/infrastructure/oauth/NaverOauth.java
- src/main/java/com/example/spot/auth/infrastructure/oauth/KaKaoOauth.java
🔇 Additional comments (2)
src/main/java/com/example/spot/common/infrastructure/feign/SafeFeignExecutor.java (2)
7-11: LGTM! Proper utility class design.The class follows utility class best practices with a final modifier and private constructor to prevent instantiation. This is appropriate for a class containing only static utility methods.
12-19: LGTM! Well-structured exception handling pattern.The generic method design allows for flexible usage across different Feign client calls while maintaining consistent exception handling. The approach of wrapping
FeignExceptioninto a customExternalApiExceptionprovides better abstraction for the application layer.
#️⃣ 연관된 이슈
🔎 작업 내용
문제점
해결 방안
OAuthStrategy인터페이스 도입 및LoginType기반 전략 선택 로직 분리KakaoOAuthStrategy,GoogleOAuthStrategy,NaverOAuthStrategy전략 클래스 생성OAuthStrategyFactory 추가→ 전략 주입 및 매핑 처리OAuthMemberProcessor로 분리OAuthMemberProcessor내부 로직은 리팩토링 X, 추후 진행 예정OAuthService는 전략 위임 및 결과 조합 역할만 수행기대 효과
📷 스크린샷 (선택)
💬리뷰 요구사항 (선택)
application.yml파일 변동사항이 있어서 해당 내용은 슬랙으로 추후에 전달 드릴게요!Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Removals
Bug Fixes
Chores