Feat/141 add apple oauth#3
Conversation
WalkthroughThis change introduces new OAuth client implementations and updates the authentication flow. Apple authentication support is added via a new client and secret provider, while the Kakao OAuth client has been refactored to use a dedicated properties class. DTOs have been reorganized and consolidated into a unified OAuth package, with legacy records and interfaces removed. The authentication controller and services now use standardized OAuth request and response types. Additional configuration properties, error codes, and utility adjustments (including JWT parsing) have been incorporated, and tests were updated to cover these changes. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant AC as AuthController
participant OS as OAuthService
participant OC as [Apple/Kakao]OAuthClient
participant DB as SiteUserRepository
participant SI as SignInService
participant ST as SignUpTokenProvider
C->>AC: Sends OAuthCodeRequest
AC->>OS: processOAuth(OAuthCodeRequest)
OS->>OC: getOAuthUserInfo(code)
OC-->>OS: OAuthUserInfoDto
OS->>DB: Check if user exists (by email)
alt User exists
OS->>SI: signIn(SiteUser)
SI-->>OS: SignInResponse (access & refresh tokens)
else New user
OS->>ST: Generate sign-up token
OS-->>C: SignUpPrepareResponse (token, user info)
end
OS-->>AC: OAuth response
AC-->>C: Return OAuthSignInResponse / SignUpPrepareResponse
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Here's the code health analysis summary for commits Analysis Summary
|
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Code Suggestions ✨Explore these optional code suggestions:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🔭 Outside diff range comments (1)
src/main/java/com/example/solidconnection/config/client/KakaoOAuthClientProperties.java (1)
1-13:Details
🧩 Analysis chain
Verify the configuration in application.yml.
Ensure that the Kakao OAuth configuration is properly set up in the application configuration files.
🏁 Script executed:
#!/bin/bash # Search for Kakao OAuth configuration in application files echo "Searching for Kakao OAuth configuration..." rg -l "oauth:\s*kakao:" --type yamlLength of output: 132
🏁 Script executed:
#!/bin/bash echo "Searching for Kakao OAuth configuration keys in application.yml..." rg -n "kakao:" application.ymlLength of output: 229
🏁 Script executed:
#!/bin/bash echo "Searching for Kakao OAuth configuration in all application config files (YML/YAML/PROPERTIES)..." find . -type f \( -iname "application*.yml" -o -iname "application*.yaml" -o -iname "application*.properties" \) -exec grep -H "oauth.kakao" {} \;Length of output: 354
Kakao OAuth configuration is missing in the application configuration files.
The Kakao OAuth client properties are defined in the code, but our searches across the configuration files (application.yml, YAML, and properties files) didn’t reveal any matching entries for keys like
oauth.kakaoor related parameters.
- No configuration was found for
oauth.kakaoin any of the searched config files.- Ensure the necessary properties (e.g.,
tokenUrl,userInfoUrl,redirectUrl,clientId) are provided under the correct prefix in your configuration file.
🧹 Nitpick comments (17)
src/main/java/com/example/solidconnection/util/JwtUtils.java (2)
62-67: Add JavaDoc documentation for the public method.Since this is a public method that's critical for JWT processing, please add JavaDoc documentation describing:
- Purpose of the method
- Parameters and their requirements
- Return value
- Possible exceptions
+ /** + * Parses a JWT token and returns its claims. + * + * @param token the JWT token to parse + * @param secretKey the key used to verify the token's signature + * @return the parsed Claims object + * @throws ExpiredJwtException if the token has expired + * @throws io.jsonwebtoken.JwtException for other JWT parsing errors + */ public static Claims parseClaims(String token, String secretKey) throws ExpiredJwtException {
49-67: Consider consolidating JWT parsing logic.The
isExpiredandparseClaimsmethods both parse the JWT token but handle exceptions differently. Consider refactoringisExpiredto useparseClaimsfor consistency:public static boolean isExpired(String token, String secretKey) { try { - Date expiration = Jwts.parser() - .setSigningKey(secretKey) - .parseClaimsJws(token) - .getBody() - .getExpiration(); + Date expiration = parseClaims(token, secretKey).getExpiration(); return expiration.before(new Date()); - } catch (Exception e) { + } catch (ExpiredJwtException e) { + return true; + } catch (Exception e) { return true; } }src/test/java/com/example/solidconnection/auth/service/SignInServiceTest.java (4)
23-25: Add class-level documentation and consider adding English translations.While the Korean display name is descriptive, consider adding:
- Class-level documentation explaining the test scope and purpose
- English translations in comments for better international collaboration
+/** + * Integration tests for SignInService. + * Tests user authentication flows including successful login and user reactivation. + */ @DisplayName("로그인 서비스 테스트") // Login Service Test @TestContainerSpringBootTest class SignInServiceTest {
42-47: Enhance test setup with cleanup and validation.Consider adding:
- @AfterEach cleanup to ensure test isolation
- Validation in setUp to ensure the test user was created successfully
@BeforeEach void setUp() { siteUser = createSiteUser(); siteUserRepository.save(siteUser); + assertThat(siteUser.getId()).isNotNull(); subject = siteUser.getId().toString(); } +@AfterEach +void tearDown() { + siteUserRepository.deleteAll(); +}
64-75: Enhance withdrawal history test with additional validations.The current test only verifies the quitedAt field. Consider:
- Verifying successful login after reactivation
- Checking other user attributes remain unchanged
- Testing multiple withdrawal/reactivation cycles
@Test void 탈퇴한_이력이_있으면_초기화한다() { // Resets if there is withdrawal history // given siteUser.setQuitedAt(LocalDate.now().minusDays(1)); siteUserRepository.save(siteUser); + String originalEmail = siteUser.getEmail(); + String originalNickname = siteUser.getNickname(); // when - signInService.signIn(siteUser); + SignInResponse signInResponse = signInService.signIn(siteUser); // then - assertThat(siteUser.getQuitedAt()).isNull(); + assertAll( + () -> assertThat(siteUser.getQuitedAt()).isNull(), + () -> assertThat(siteUser.getEmail()).isEqualTo(originalEmail), + () -> assertThat(siteUser.getNickname()).isEqualTo(originalNickname), + () -> assertThat(signInResponse.accessToken()).isNotNull(), + () -> assertThat(signInResponse.refreshToken()).isNotNull() + ); }
77-87: Improve test data creation flexibility.Consider implementing a builder pattern or test data factory for more flexible test data creation. This would make it easier to create different test scenarios.
+@Builder +@AllArgsConstructor +class TestSiteUserBuilder { + private String email = "test@example.com"; + private String nickname = "nickname"; + private String profileImageUrl = "profileImageUrl"; + private String birthDate = "1999-01-01"; + private PreparationStatus status = PreparationStatus.CONSIDERING; + private Role role = Role.MENTEE; + private Gender gender = Gender.MALE; + + public SiteUser build() { + return new SiteUser( + email, nickname, profileImageUrl, birthDate, + status, role, gender + ); + } +} -private SiteUser createSiteUser() { - return new SiteUser( - "test@example.com", - "nickname", - "profileImageUrl", - "1999-01-01", - PreparationStatus.CONSIDERING, - Role.MENTEE, - Gender.MALE - ); -} +private SiteUser createSiteUser() { + return TestSiteUserBuilder.builder().build(); +}src/main/java/com/example/solidconnection/auth/client/AppleOAuthClient.java (1)
76-77: Include the original exception when throwingCustomExceptionAdding the original exception as a cause will provide more context and aid in debugging.
Apply this diff to include the original exception:
} catch (Exception e) { - throw new CustomException(APPLE_ID_TOKEN_MISSING_EMAIL); + throw new CustomException(APPLE_ID_TOKEN_MISSING_EMAIL, e); }src/main/java/com/example/solidconnection/auth/service/oauth/OAuthSignUpService.java (1)
77-91: Refactor Methods to Reduce Code DuplicationThe methods
saveInterestedRegionandsaveInterestedCountryshare similar logic for saving user interests. Consider refactoring them into a single, generic method to reduce code duplication and improve maintainability.Here's an example of how you might refactor these methods:
private void saveInterestedEntities( List<String> interestedNames, Function<List<String>, List<? extends BaseEntity>> findEntitiesByNames, BiFunction<SiteUser, ? extends BaseEntity, ? extends BaseInterestedEntity> createInterestedEntity, Consumer<List<? extends BaseInterestedEntity>> saveAllInterestedEntities ) { List<? extends BaseEntity> entities = findEntitiesByNames.apply(interestedNames); List<? extends BaseInterestedEntity> interestedEntities = entities.stream() .map(entity -> createInterestedEntity.apply(savedSiteUser, entity)) .toList(); saveAllInterestedEntities.accept(interestedEntities); }You can then use this method for both regions and countries by passing the appropriate parameters.
src/main/java/com/example/solidconnection/auth/dto/oauth/OAuthUserInfoDto.java (1)
3-10: Consider adding validation and nullability annotations.The interface methods could benefit from additional annotations:
getEmail()@Nullableor@NotNullannotations to explicitly define nullability contracts+import javax.validation.constraints.Email; +import javax.annotation.Nullable; public interface OAuthUserInfoDto { + @Email String getEmail(); + @Nullable String getProfileImageUrl(); + @NotNull String getNickname(); }src/main/java/com/example/solidconnection/auth/dto/oauth/AppleTokenDto.java (1)
6-10: Add documentation and token validation.Consider adding:
- Javadoc to document the DTO's purpose and Apple's ID token format
- Validation to ensure the token is not blank
+import javax.validation.constraints.NotBlank; + @JsonIgnoreProperties(ignoreUnknown = true) +/** + * DTO for Apple OAuth ID token response. + * The ID token is a JWT containing user information from Apple. + */ public record AppleTokenDto( + @NotBlank(message = "ID token must not be blank") @JsonProperty("id_token") String idToken ) { }src/main/java/com/example/solidconnection/auth/dto/oauth/AppleUserInfoDto.java (1)
3-8: Add input validation and documentation.The record lacks email validation and documentation explaining why profile image and nickname are not supported for Apple OAuth.
+/** + * DTO for Apple OAuth user information. + * Note: Apple OAuth does not provide profile image or nickname information, + * only email is available. + */ public record AppleUserInfoDto( + @jakarta.validation.constraints.Email(message = "Invalid email format") + @jakarta.validation.constraints.NotNull(message = "Email cannot be null") String email) implements OAuthUserInfoDto {src/main/java/com/example/solidconnection/auth/dto/oauth/SignUpPrepareResponse.java (1)
10-18: Add validation and update factory method.The factory method should validate input parameters and be updated to support multiple providers.
- public static SignUpPrepareResponse of(OAuthUserInfoDto oAuthUserInfoDto, String kakaoOauthToken) { + /** + * Creates a SignUpPrepareResponse for the given OAuth provider. + * + * @param oAuthUserInfoDto User information from OAuth provider + * @param oauthToken OAuth token from the provider + * @param provider OAuth provider name (e.g., "KAKAO", "APPLE") + * @return SignUpPrepareResponse instance + * @throws IllegalArgumentException if required parameters are null + */ + public static SignUpPrepareResponse of( + @NotNull OAuthUserInfoDto oAuthUserInfoDto, + @NotNull String oauthToken, + @NotNull String provider) { + Objects.requireNonNull(oAuthUserInfoDto, "oAuthUserInfoDto must not be null"); + Objects.requireNonNull(oauthToken, "oauthToken must not be null"); + Objects.requireNonNull(provider, "provider must not be null"); return new SignUpPrepareResponse( false, oAuthUserInfoDto.getNickname(), oAuthUserInfoDto.getEmail(), oAuthUserInfoDto.getProfileImageUrl(), - kakaoOauthToken + oauthToken, + provider ); }src/main/java/com/example/solidconnection/auth/dto/oauth/KakaoUserInfoDto.java (1)
7-8: Add validation annotations and documentation.Consider adding validation annotations and documentation to clarify the structure and requirements.
+/** + * DTO for Kakao OAuth user information. + * Contains nested structures for account and profile information. + */ @JsonIgnoreProperties(ignoreUnknown = true) public record KakaoUserInfoDto( + @NotNull(message = "Kakao account information is required") @JsonProperty("kakao_account") KakaoAccountDto kakaoAccountDto) implements OAuthUserInfoDto { @JsonIgnoreProperties(ignoreUnknown = true) public record KakaoAccountDto( + @NotNull(message = "Profile information is required") @JsonProperty("profile") KakaoProfileDto profile, + @Email(message = "Invalid email format") String email) { @JsonIgnoreProperties(ignoreUnknown = true) public record KakaoProfileDto( + @NotNull(message = "Profile image URL is required") @JsonProperty("profile_image_url") String profileImageUrl, + @NotBlank(message = "Nickname is required") String nickname) {Also applies to: 11-13, 16-18
src/main/java/com/example/solidconnection/auth/service/oauth/AppleOAuthService.java (1)
22-24: Standardize OAuth client method names across implementations.There's an inconsistency in method naming between OAuth clients:
- KakaoOAuthClient uses
getUserInfo()- AppleOAuthClient uses
processOAuth()Consider standardizing these method names across all OAuth clients for better maintainability.
- protected OAuthUserInfoDto getOAuthUserInfo(String code) { - return appleOAuthClient.processOAuth(code); + protected OAuthUserInfoDto getOAuthUserInfo(String code) { + return appleOAuthClient.getUserInfo(code);src/main/java/com/example/solidconnection/auth/service/oauth/SignUpTokenProvider.java (2)
39-46: Consider additional JWT security measures.While the JWT implementation is functional, consider these security enhancements:
- Add a
jti(JWT ID) claim for token uniqueness- Include an
aud(audience) claim- Consider using RS256 instead of HS512 for better key management in distributed systems
65-70: Consider adding rate limiting for token validation.The Redis-based token validation could be susceptible to brute-force attacks. Consider implementing rate limiting:
- Track validation attempts per IP
- Implement exponential backoff for repeated failures
- Consider using Redis rate limiting modules or implementing a token bucket algorithm
src/test/java/com/example/solidconnection/auth/service/oauth/SignUpTokenProviderTest.java (1)
167-174: Consider improving test data and extracting constants.
- The token expiration time (1000ms) is a magic number
- Test data like "email" could be more realistic
Consider these improvements:
private String createExpiredToken() { + private static final long TOKEN_EXPIRATION_MS = 1000; + private static final String TEST_SUBJECT = "test@example.com"; return Jwts.builder() - .setSubject("subject") + .setSubject(TEST_SUBJECT) .setIssuedAt(new Date()) - .setExpiration(new Date(System.currentTimeMillis() - 1000)) + .setExpiration(new Date(System.currentTimeMillis() - TOKEN_EXPIRATION_MS)) .signWith(SignatureAlgorithm.HS256, jwtProperties.secret()) .compact(); }Also applies to: 176-181
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
src/main/java/com/example/solidconnection/auth/client/AppleOAuthClient.java(1 hunks)src/main/java/com/example/solidconnection/auth/client/AppleOAuthClientSecretProvider.java(1 hunks)src/main/java/com/example/solidconnection/auth/client/KakaoOAuthClient.java(3 hunks)src/main/java/com/example/solidconnection/auth/controller/AuthController.java(2 hunks)src/main/java/com/example/solidconnection/auth/dto/SignInResponse.java(1 hunks)src/main/java/com/example/solidconnection/auth/dto/SignUpRequest.java(3 hunks)src/main/java/com/example/solidconnection/auth/dto/SignUpResponse.java(0 hunks)src/main/java/com/example/solidconnection/auth/dto/kakao/FirstAccessResponse.java(0 hunks)src/main/java/com/example/solidconnection/auth/dto/kakao/KakaoCodeRequest.java(0 hunks)src/main/java/com/example/solidconnection/auth/dto/kakao/KakaoOauthResponse.java(0 hunks)src/main/java/com/example/solidconnection/auth/dto/oauth/AppleTokenDto.java(1 hunks)src/main/java/com/example/solidconnection/auth/dto/oauth/AppleUserInfoDto.java(1 hunks)src/main/java/com/example/solidconnection/auth/dto/oauth/KakaoTokenDto.java(1 hunks)src/main/java/com/example/solidconnection/auth/dto/oauth/KakaoUserInfoDto.java(2 hunks)src/main/java/com/example/solidconnection/auth/dto/oauth/OAuthCodeRequest.java(1 hunks)src/main/java/com/example/solidconnection/auth/dto/oauth/OAuthResponse.java(1 hunks)src/main/java/com/example/solidconnection/auth/dto/oauth/OAuthSignInResponse.java(1 hunks)src/main/java/com/example/solidconnection/auth/dto/oauth/OAuthUserInfoDto.java(1 hunks)src/main/java/com/example/solidconnection/auth/dto/oauth/SignUpPrepareResponse.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/SignInService.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/SignUpTokenProvider.java(0 hunks)src/main/java/com/example/solidconnection/auth/service/TokenValidator.java(0 hunks)src/main/java/com/example/solidconnection/auth/service/oauth/AppleOAuthService.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/oauth/KakaoOAuthService.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/oauth/OAuthService.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/oauth/OAuthSignUpService.java(3 hunks)src/main/java/com/example/solidconnection/auth/service/oauth/SignUpTokenProvider.java(1 hunks)src/main/java/com/example/solidconnection/config/client/AppleOAuthClientProperties.java(1 hunks)src/main/java/com/example/solidconnection/config/client/KakaoOAuthClientProperties.java(1 hunks)src/main/java/com/example/solidconnection/config/client/RestTemplateConfig.java(1 hunks)src/main/java/com/example/solidconnection/custom/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/util/JwtUtils.java(4 hunks)src/test/java/com/example/solidconnection/auth/service/SignInServiceTest.java(1 hunks)src/test/java/com/example/solidconnection/auth/service/SignUpTokenProviderTest.java(0 hunks)src/test/java/com/example/solidconnection/auth/service/oauth/SignUpTokenProviderTest.java(1 hunks)src/test/java/com/example/solidconnection/e2e/DynamicFixture.java(1 hunks)src/test/java/com/example/solidconnection/e2e/SignInTest.java(5 hunks)src/test/java/com/example/solidconnection/e2e/SignUpTest.java(6 hunks)
💤 Files with no reviewable changes (7)
- src/main/java/com/example/solidconnection/auth/dto/SignUpResponse.java
- src/test/java/com/example/solidconnection/auth/service/SignUpTokenProviderTest.java
- src/main/java/com/example/solidconnection/auth/dto/kakao/FirstAccessResponse.java
- src/main/java/com/example/solidconnection/auth/dto/kakao/KakaoOauthResponse.java
- src/main/java/com/example/solidconnection/auth/service/SignUpTokenProvider.java
- src/main/java/com/example/solidconnection/auth/dto/kakao/KakaoCodeRequest.java
- src/main/java/com/example/solidconnection/auth/service/TokenValidator.java
✅ Files skipped from review due to trivial changes (5)
- src/main/java/com/example/solidconnection/auth/dto/oauth/OAuthResponse.java
- src/main/java/com/example/solidconnection/auth/dto/oauth/KakaoTokenDto.java
- src/main/java/com/example/solidconnection/auth/dto/oauth/OAuthCodeRequest.java
- src/main/java/com/example/solidconnection/config/client/RestTemplateConfig.java
- src/test/java/com/example/solidconnection/e2e/DynamicFixture.java
🔇 Additional comments (17)
src/test/java/com/example/solidconnection/e2e/SignUpTest.java (4)
84-90: Response type standardization looks good.The change from
SignUpResponsetoSignInResponsealigns with the unified OAuth response types being implemented for both Kakao and Apple authentication.
77-77: Auth type specification enhances provider clarity.Adding explicit
AuthType.KAKAOparameter togenerateAndSaveSignUpTokencalls prepares the codebase for multi-provider support (Kakao and Apple).Also applies to: 129-129, 154-154
184-184: Improved OAuth-specific error handling.The change to
OAUTH_SIGN_UP_TOKEN_INVALIDerror code provides clearer, OAuth-specific error messaging compared to the generic JWT exception.
67-118: Verify Apple OAuth test coverage.While the Kakao OAuth sign-up test is well implemented, ensure equivalent test coverage exists for Apple OAuth sign-up flow.
Would you like me to help generate the corresponding Apple OAuth sign-up test case?
src/test/java/com/example/solidconnection/e2e/SignInTest.java (3)
4-7: LGTM! Good move towards standardized OAuth DTOs.The import changes reflect a well-structured move from Kakao-specific DTOs to standardized OAuth DTOs, which will better support multiple OAuth providers.
48-48: LGTM! Method name change improves clarity.The renaming from
processOauthtogetUserInfobetter describes the method's purpose and aligns with single responsibility principle.Also applies to: 78-78, 108-108
53-53: LGTM! Response type changes support multi-provider OAuth.The switch to generic OAuth response types (
OAuthSignInResponse,SignUpPrepareResponse) prepares the codebase for supporting multiple OAuth providers.Also applies to: 59-59, 86-86, 92-92, 119-119, 125-125
src/main/java/com/example/solidconnection/util/JwtUtils.java (3)
4-4: LGTM!The addition of the Claims import is appropriate for the new JWT claims parsing functionality.
31-47: LGTM! Good refactoring.The modifications to both methods improve code reuse by leveraging the new
parseClaimsmethod while maintaining appropriate error handling for each use case.
62-67: Verify Apple OAuth ID token parsing integration.Since this class is used for parsing Apple OAuth ID tokens, let's verify the integration:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for usage of parseClaims in Apple OAuth related files rg -l "parseClaims.*secretKey" | grep -i "apple" # Check if there are any Apple-specific JWT parsing requirements rg -i "apple.*jwt"Length of output: 264
No impact on Apple OAuth ID token parsing detected.
- Our search did not locate any direct references to
JwtUtils.parseClaimsin Apple OAuth–specific classes.- The only Apple-related reference found was an error message in
ErrorCode.java, which is unrelated to the JWT parsing logic inJwtUtils.- Thus, the changes in
JwtUtilsremain generic and do not appear to affect the Apple OAuth integration.src/main/java/com/example/solidconnection/auth/controller/AuthController.java (1)
52-56: Streamline Sign-Up Process by ReturningSignInResponseThe
signUpmethod now returns aSignInResponse, integrating the sign-up and sign-in processes. This enhances the user experience by automatically signing in the user after a successful registration.src/main/java/com/example/solidconnection/auth/dto/SignInResponse.java (1)
5-6: Verify Impact of RemovingisRegisteredField and Interface ImplementationThe
isRegisteredfield has been removed fromSignInResponse, and it no longer implementsKakaoOauthResponse. Please ensure that this change does not affect any parts of the codebase that rely on these features. Verify that all usages ofSignInResponse,isRegistered, andKakaoOauthResponsehave been updated accordingly.Run the following script to search for references:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Find all usages of `isRegistered` and `KakaoOauthResponse`. # Search for code accessing `isRegistered` in relation to `SignInResponse` rg 'SignInResponse' -A 5 | rg 'isRegistered' # Search for any direct references to `.isRegistered` rg '\.isRegistered' # Search for any references to `KakaoOauthResponse` rg 'KakaoOauthResponse'Length of output: 851
Based on the output, we see that:
- The removed field (
isRegistered) is no longer part ofSignInResponse(the file under review).- References to
isRegistered()still exist in tests (inSignInTest.java), but the only declaration ofisRegisteredwe found is inOAuthSignInResponse.java.- No references to
KakaoOauthResponseare found, meaning its removal has no systemic impact.This suggests that tests expecting an
isRegisteredmethod are now using the OAuth-specific response (i.e. the instance ofOAuthSignInResponse), and no other parts of the codebase rely on the removed field or interface.Therefore, the changes in
SignInResponse(removing theisRegisteredfield and implementingKakaoOauthResponse) do not appear to have adverse repercussions on the codebase, as usages have been adjusted accordingly.No further changes are necessary.
Action: Proceed with the PR as the changes are verified.
src/main/java/com/example/solidconnection/auth/service/oauth/KakaoOAuthService.java (1)
1-30: Well-structured OAuth service implementation!The implementation follows the template method pattern effectively, providing a clean separation of concerns and proper dependency injection.
src/main/java/com/example/solidconnection/auth/client/KakaoOAuthClient.java (2)
23-28: Great addition of documentation links!The added documentation links improve maintainability by providing quick access to Kakao's official API documentation.
34-34: Good refactoring to use properties class and descriptive method name!The changes improve the code by:
- Using
KakaoOAuthClientPropertiesfor centralized configuration management- Renaming
processOauthtogetUserInfofor better clarityAlso applies to: 36-36
src/main/java/com/example/solidconnection/custom/exception/ErrorCode.java (1)
14-18: Well-structured error codes with clear messages!The new error codes are:
- Logically grouped into sections (apple, oauth)
- Use appropriate HTTP status codes
- Provide clear, descriptive messages in Korean
Also applies to: 27-29
src/test/java/com/example/solidconnection/auth/service/oauth/SignUpTokenProviderTest.java (1)
64-136: Excellent test organization with comprehensive coverage!The nested test class structure and descriptive test names make the test cases clear and maintainable.
| // request - body 생성 및 요청 | ||
| KakaoCodeRequest kakaoCodeRequest = new KakaoCodeRequest(kakaoCode); | ||
| FirstAccessResponse response = RestAssured.given().log().all() | ||
| OAuthCodeRequest OAuthCodeRequest = new OAuthCodeRequest(kakaoCode); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix inconsistent variable casing.
The variable casing for OAuthCodeRequest is inconsistent across the test methods:
- Line 52:
OAuthCodeRequest OAuthCodeRequest - Line 85:
OAuthCodeRequest oAuthCodeRequest - Line 118:
OAuthCodeRequest OAuthCodeRequest
Please use camelCase consistently for variable names.
-OAuthCodeRequest OAuthCodeRequest = new OAuthCodeRequest(kakaoCode);
+OAuthCodeRequest oAuthCodeRequest = new OAuthCodeRequest(kakaoCode);Also applies to: 85-85, 118-118
| public SignInResponse signIn(SiteUser siteUser) { | ||
| resetQuitedAt(siteUser); | ||
| String accessToken = authTokenProvider.generateAccessToken(siteUser); | ||
| String refreshToken = authTokenProvider.generateAndSaveRefreshToken(siteUser); | ||
| return new SignInResponse(accessToken, refreshToken); |
There was a problem hiding this comment.
Add null check for siteUser parameter
To prevent a potential NullPointerException, consider adding a null check for the siteUser parameter at the beginning of the signIn method.
Apply this diff to include the null check:
public SignInResponse signIn(SiteUser siteUser) {
+ if (siteUser == null) {
+ throw new IllegalArgumentException("SiteUser must not be null");
+ }
resetQuitedAt(siteUser);
String accessToken = authTokenProvider.generateAccessToken(siteUser);
String refreshToken = authTokenProvider.generateAndSaveRefreshToken(siteUser);
return new SignInResponse(accessToken, refreshToken);
}📝 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 SignInResponse signIn(SiteUser siteUser) { | |
| resetQuitedAt(siteUser); | |
| String accessToken = authTokenProvider.generateAccessToken(siteUser); | |
| String refreshToken = authTokenProvider.generateAndSaveRefreshToken(siteUser); | |
| return new SignInResponse(accessToken, refreshToken); | |
| public SignInResponse signIn(SiteUser siteUser) { | |
| if (siteUser == null) { | |
| throw new IllegalArgumentException("SiteUser must not be null"); | |
| } | |
| resetQuitedAt(siteUser); | |
| String accessToken = authTokenProvider.generateAccessToken(siteUser); | |
| String refreshToken = authTokenProvider.generateAndSaveRefreshToken(siteUser); | |
| return new SignInResponse(accessToken, refreshToken); | |
| } |
| @Test | ||
| void 성공적으로_로그인한다() { | ||
| // when | ||
| SignInResponse signInResponse = signInService.signIn(siteUser); | ||
|
|
||
| // then | ||
| String accessTokenSubject = JwtUtils.parseSubject(signInResponse.accessToken(), jwtProperties.secret()); | ||
| String refreshTokenSubject = JwtUtils.parseSubject(signInResponse.refreshToken(), jwtProperties.secret()); | ||
| Optional<String> savedRefreshToken = authTokenProvider.findRefreshToken(subject); | ||
| assertAll( | ||
| () -> assertThat(accessTokenSubject).isEqualTo(subject), | ||
| () -> assertThat(refreshTokenSubject).isEqualTo(subject), | ||
| () -> assertThat(savedRefreshToken).hasValue(signInResponse.refreshToken())); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance login test coverage with additional scenarios.
While the happy path is well tested, consider adding:
- Token expiration validation
- Negative test cases (e.g., invalid user)
- Token format validation
@Test
void 성공적으로_로그인한다() { // Successfully logs in
// when
SignInResponse signInResponse = signInService.signIn(siteUser);
// then
String accessTokenSubject = JwtUtils.parseSubject(signInResponse.accessToken(), jwtProperties.secret());
String refreshTokenSubject = JwtUtils.parseSubject(signInResponse.refreshToken(), jwtProperties.secret());
Optional<String> savedRefreshToken = authTokenProvider.findRefreshToken(subject);
assertAll(
() -> assertThat(accessTokenSubject).isEqualTo(subject),
() -> assertThat(refreshTokenSubject).isEqualTo(subject),
- () -> assertThat(savedRefreshToken).hasValue(signInResponse.refreshToken()));
+ () -> assertThat(savedRefreshToken).hasValue(signInResponse.refreshToken()),
+ () -> assertThat(JwtUtils.isTokenExpired(signInResponse.accessToken(), jwtProperties.secret())).isFalse(),
+ () -> assertThat(JwtUtils.isTokenExpired(signInResponse.refreshToken(), jwtProperties.secret())).isFalse());
}
+@Test
+void 존재하지_않는_사용자_로그인_실패() { // Non-existent user login fails
+ // given
+ SiteUser nonExistentUser = createSiteUser();
+
+ // when/then
+ assertThatThrownBy(() -> signInService.signIn(nonExistentUser))
+ .isInstanceOf(IllegalArgumentException.class);
+}📝 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 | |
| void 성공적으로_로그인한다() { | |
| // when | |
| SignInResponse signInResponse = signInService.signIn(siteUser); | |
| // then | |
| String accessTokenSubject = JwtUtils.parseSubject(signInResponse.accessToken(), jwtProperties.secret()); | |
| String refreshTokenSubject = JwtUtils.parseSubject(signInResponse.refreshToken(), jwtProperties.secret()); | |
| Optional<String> savedRefreshToken = authTokenProvider.findRefreshToken(subject); | |
| assertAll( | |
| () -> assertThat(accessTokenSubject).isEqualTo(subject), | |
| () -> assertThat(refreshTokenSubject).isEqualTo(subject), | |
| () -> assertThat(savedRefreshToken).hasValue(signInResponse.refreshToken())); | |
| } | |
| @Test | |
| void 성공적으로_로그인한다() { | |
| // when | |
| SignInResponse signInResponse = signInService.signIn(siteUser); | |
| // then | |
| String accessTokenSubject = JwtUtils.parseSubject(signInResponse.accessToken(), jwtProperties.secret()); | |
| String refreshTokenSubject = JwtUtils.parseSubject(signInResponse.refreshToken(), jwtProperties.secret()); | |
| Optional<String> savedRefreshToken = authTokenProvider.findRefreshToken(subject); | |
| assertAll( | |
| () -> assertThat(accessTokenSubject).isEqualTo(subject), | |
| () -> assertThat(refreshTokenSubject).isEqualTo(subject), | |
| () -> assertThat(savedRefreshToken).hasValue(signInResponse.refreshToken()), | |
| () -> assertThat(JwtUtils.isTokenExpired(signInResponse.accessToken(), jwtProperties.secret())).isFalse(), | |
| () -> assertThat(JwtUtils.isTokenExpired(signInResponse.refreshToken(), jwtProperties.secret())).isFalse()); | |
| } | |
| @Test | |
| void 존재하지_않는_사용자_로그인_실패() { | |
| // given | |
| SiteUser nonExistentUser = createSiteUser(); | |
| // when/then | |
| assertThatThrownBy(() -> signInService.signIn(nonExistentUser)) | |
| .isInstanceOf(IllegalArgumentException.class); | |
| } |
| @Transactional | ||
| public OAuthResponse processOAuth(OAuthCodeRequest oauthCodeRequest) { | ||
| OAuthUserInfoDto userInfoDto = getOAuthUserInfo(oauthCodeRequest.code()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider moving external service calls outside transactional boundaries
The processOAuth method is annotated with @Transactional, but it immediately calls getOAuthUserInfo, which likely involves network I/O to the OAuth provider. Including external service calls within a transaction can lead to prolonged transaction durations and potential database locking issues. It's recommended to move external service calls outside the transactional scope to improve performance and avoid unnecessary locking.
Apply this diff to adjust the transactional boundaries:
- @Transactional
public OAuthResponse processOAuth(OAuthCodeRequest oauthCodeRequest) {
OAuthUserInfoDto userInfoDto = getOAuthUserInfo(oauthCodeRequest.code());
+ return processUserInfo(userInfoDto);
}
+ @Transactional
+ private OAuthResponse processUserInfo(OAuthUserInfoDto userInfoDto) {
String email = userInfoDto.getEmail();
Optional<SiteUser> optionalSiteUser = siteUserRepository.findByEmailAndAuthType(email, getAuthType());
if (optionalSiteUser.isPresent()) {
SiteUser siteUser = optionalSiteUser.get();
return getSignInResponse(siteUser);
}
return getSignUpPrepareResponse(userInfoDto);
}📝 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.
| @Transactional | |
| public OAuthResponse processOAuth(OAuthCodeRequest oauthCodeRequest) { | |
| OAuthUserInfoDto userInfoDto = getOAuthUserInfo(oauthCodeRequest.code()); | |
| public OAuthResponse processOAuth(OAuthCodeRequest oauthCodeRequest) { | |
| OAuthUserInfoDto userInfoDto = getOAuthUserInfo(oauthCodeRequest.code()); | |
| return processUserInfo(userInfoDto); | |
| } | |
| @Transactional | |
| private OAuthResponse processUserInfo(OAuthUserInfoDto userInfoDto) { | |
| String email = userInfoDto.getEmail(); | |
| Optional<SiteUser> optionalSiteUser = siteUserRepository.findByEmailAndAuthType(email, getAuthType()); | |
| if (optionalSiteUser.isPresent()) { | |
| SiteUser siteUser = optionalSiteUser.get(); | |
| return getSignInResponse(siteUser); | |
| } | |
| return getSignUpPrepareResponse(userInfoDto); | |
| } |
| public class AppleOAuthClientSecretProvider { | ||
|
|
||
| private static final String KEY_ID_HEADER = "kid"; | ||
| private static final long TOKEN_DURATION = 1000 * 60; // 10min |
There was a problem hiding this comment.
Fix the discrepancy between TOKEN_DURATION value and the comment
The TOKEN_DURATION is set to 1000 * 60, which equals 60,000 milliseconds (1 minute), but the comment indicates // 10min. This inconsistency may lead to unintended behavior. Please update either the value or the comment to reflect the correct token duration.
Apply this diff to correct the value:
- private static final long TOKEN_DURATION = 1000 * 60; // 10min
+ private static final long TOKEN_DURATION = 1000 * 60 * 10; // 10min📝 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.
| private static final long TOKEN_DURATION = 1000 * 60; // 10min | |
| private static final long TOKEN_DURATION = 1000 * 60 * 10; // 10min |
| public record SignUpPrepareResponse( | ||
| boolean isRegistered, | ||
| String nickname, | ||
| String email, | ||
| String profileImageUrl, | ||
| String kakaoOauthToken) implements OAuthResponse { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to make the DTO provider-agnostic.
The response contains a Kakao-specific field (kakaoOauthToken) but the class name suggests a generic signup response. Consider making it provider-agnostic.
public record SignUpPrepareResponse(
boolean isRegistered,
String nickname,
String email,
String profileImageUrl,
- String kakaoOauthToken) implements OAuthResponse {
+ String oauthToken,
+ String provider) implements OAuthResponse {📝 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 record SignUpPrepareResponse( | |
| boolean isRegistered, | |
| String nickname, | |
| String email, | |
| String profileImageUrl, | |
| String kakaoOauthToken) implements OAuthResponse { | |
| public record SignUpPrepareResponse( | |
| boolean isRegistered, | |
| String nickname, | |
| String email, | |
| String profileImageUrl, | |
| String oauthToken, | |
| String provider) implements OAuthResponse { |
| @Override | ||
| public String getEmail() { | ||
| return kakaoAccountDto.email; | ||
| } | ||
|
|
||
| @Override | ||
| public String getProfileImageUrl() { | ||
| return kakaoAccountDto.profile.profileImageUrl; | ||
| } | ||
|
|
||
| @Override | ||
| public String getNickname() { | ||
| return kakaoAccountDto.profile.nickname; | ||
| } |
There was a problem hiding this comment.
Add null checks in getter methods.
The getter methods don't handle potential null values in the nested objects, which could lead to NullPointerException.
@Override
public String getEmail() {
+ if (kakaoAccountDto == null) {
+ return "";
+ }
return kakaoAccountDto.email;
}
@Override
public String getProfileImageUrl() {
+ if (kakaoAccountDto == null || kakaoAccountDto.profile == null) {
+ return "";
+ }
return kakaoAccountDto.profile.profileImageUrl;
}
@Override
public String getNickname() {
+ if (kakaoAccountDto == null || kakaoAccountDto.profile == null) {
+ return "";
+ }
return kakaoAccountDto.profile.nickname;
}📝 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 kakaoAccountDto.email; | |
| } | |
| @Override | |
| public String getProfileImageUrl() { | |
| return kakaoAccountDto.profile.profileImageUrl; | |
| } | |
| @Override | |
| public String getNickname() { | |
| return kakaoAccountDto.profile.nickname; | |
| } | |
| @Override | |
| public String getEmail() { | |
| if (kakaoAccountDto == null) { | |
| return ""; | |
| } | |
| return kakaoAccountDto.email; | |
| } | |
| @Override | |
| public String getProfileImageUrl() { | |
| if (kakaoAccountDto == null || kakaoAccountDto.profile == null) { | |
| return ""; | |
| } | |
| return kakaoAccountDto.profile.profileImageUrl; | |
| } | |
| @Override | |
| public String getNickname() { | |
| if (kakaoAccountDto == null || kakaoAccountDto.profile == null) { | |
| return ""; | |
| } | |
| return kakaoAccountDto.profile.nickname; | |
| } |
| public SiteUser toSiteUser(String email, AuthType authType) { | ||
| return new SiteUser( | ||
| email, | ||
| this.nickname, | ||
| this.profileImageUrl, | ||
| this.birth, | ||
| this.preparationStatus, | ||
| role, | ||
| this.gender | ||
| Role.MENTEE, | ||
| this.gender, | ||
| authType | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hardcoding user roles.
The role is hardcoded to Role.MENTEE, which limits the flexibility of the sign-up process. Consider making the role configurable:
- public SiteUser toSiteUser(String email, AuthType authType) {
+ public SiteUser toSiteUser(String email, AuthType authType, Role role) {
return new SiteUser(
email,
this.nickname,
this.profileImageUrl,
this.birth,
this.preparationStatus,
- Role.MENTEE,
+ role,
this.gender,
authType
);
}📝 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 SiteUser toSiteUser(String email, AuthType authType) { | |
| return new SiteUser( | |
| email, | |
| this.nickname, | |
| this.profileImageUrl, | |
| this.birth, | |
| this.preparationStatus, | |
| role, | |
| this.gender | |
| Role.MENTEE, | |
| this.gender, | |
| authType | |
| ); | |
| } | |
| public SiteUser toSiteUser(String email, AuthType authType, Role role) { | |
| return new SiteUser( | |
| email, | |
| this.nickname, | |
| this.profileImageUrl, | |
| this.birth, | |
| this.preparationStatus, | |
| role, | |
| this.gender, | |
| authType | |
| ); | |
| } |
| public AuthType parseAuthType(String token) { | ||
| Claims claims = parseClaims(token, jwtProperties.secret()); | ||
| String authTypeStr = claims.get(AUTH_TYPE_CLAIM_KEY, String.class); | ||
| return AuthType.valueOf(authTypeStr); | ||
| } |
There was a problem hiding this comment.
Add null checks for JWT claims.
The parseAuthType method should validate the claim exists before attempting to parse it:
public AuthType parseAuthType(String token) {
Claims claims = parseClaims(token, jwtProperties.secret());
- String authTypeStr = claims.get(AUTH_TYPE_CLAIM_KEY, String.class);
+ String authTypeStr = Optional.ofNullable(claims.get(AUTH_TYPE_CLAIM_KEY, String.class))
+ .orElseThrow(() -> new CustomException(OAUTH_SIGN_UP_TOKEN_MISSING_CLAIMS));
return AuthType.valueOf(authTypeStr);
}📝 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 AuthType parseAuthType(String token) { | |
| Claims claims = parseClaims(token, jwtProperties.secret()); | |
| String authTypeStr = claims.get(AUTH_TYPE_CLAIM_KEY, String.class); | |
| return AuthType.valueOf(authTypeStr); | |
| } | |
| public AuthType parseAuthType(String token) { | |
| Claims claims = parseClaims(token, jwtProperties.secret()); | |
| String authTypeStr = Optional.ofNullable(claims.get(AUTH_TYPE_CLAIM_KEY, String.class)) | |
| .orElseThrow(() -> new CustomException(OAUTH_SIGN_UP_TOKEN_MISSING_CLAIMS)); | |
| return AuthType.valueOf(authTypeStr); | |
| } |
| private void validateFormatAndExpiration(String token) { | ||
| try { | ||
| Claims claims = parseClaims(token, jwtProperties.secret()); | ||
| Objects.requireNonNull(claims.getSubject()); | ||
| String serializedAuthType = claims.get(AUTH_TYPE_CLAIM_KEY, String.class); | ||
| AuthType.valueOf(serializedAuthType); | ||
| } catch (Exception e) { | ||
| throw new CustomException(OAUTH_SIGN_UP_TOKEN_INVALID); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance exception handling specificity.
The current implementation catches all exceptions and maps them to a generic OAUTH_SIGN_UP_TOKEN_INVALID. Consider handling specific exceptions separately:
private void validateFormatAndExpiration(String token) {
try {
Claims claims = parseClaims(token, jwtProperties.secret());
Objects.requireNonNull(claims.getSubject());
String serializedAuthType = claims.get(AUTH_TYPE_CLAIM_KEY, String.class);
AuthType.valueOf(serializedAuthType);
- } catch (Exception e) {
+ } catch (ExpiredJwtException e) {
+ throw new CustomException(OAUTH_SIGN_UP_TOKEN_EXPIRED);
+ } catch (MalformedJwtException | SignatureException e) {
+ throw new CustomException(OAUTH_SIGN_UP_TOKEN_INVALID);
+ } catch (IllegalArgumentException e) {
+ throw new CustomException(OAUTH_SIGN_UP_TOKEN_MISSING_CLAIMS);
+ } catch (Exception e) {
throw new CustomException(OAUTH_SIGN_UP_TOKEN_INVALID);
}
}📝 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.
| private void validateFormatAndExpiration(String token) { | |
| try { | |
| Claims claims = parseClaims(token, jwtProperties.secret()); | |
| Objects.requireNonNull(claims.getSubject()); | |
| String serializedAuthType = claims.get(AUTH_TYPE_CLAIM_KEY, String.class); | |
| AuthType.valueOf(serializedAuthType); | |
| } catch (Exception e) { | |
| throw new CustomException(OAUTH_SIGN_UP_TOKEN_INVALID); | |
| } | |
| } | |
| private void validateFormatAndExpiration(String token) { | |
| try { | |
| Claims claims = parseClaims(token, jwtProperties.secret()); | |
| Objects.requireNonNull(claims.getSubject()); | |
| String serializedAuthType = claims.get(AUTH_TYPE_CLAIM_KEY, String.class); | |
| AuthType.valueOf(serializedAuthType); | |
| } catch (ExpiredJwtException e) { | |
| throw new CustomException(OAUTH_SIGN_UP_TOKEN_EXPIRED); | |
| } catch (MalformedJwtException | SignatureException e) { | |
| throw new CustomException(OAUTH_SIGN_UP_TOKEN_INVALID); | |
| } catch (IllegalArgumentException e) { | |
| throw new CustomException(OAUTH_SIGN_UP_TOKEN_MISSING_CLAIMS); | |
| } catch (Exception e) { | |
| throw new CustomException(OAUTH_SIGN_UP_TOKEN_INVALID); | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
src/main/java/com/example/solidconnection/config/client/AppleOAuthClientProperties.java (1)
5-14: 🛠️ Refactor suggestionAdd validation annotations to ensure required configuration properties
The fields in
AppleOAuthClientPropertieslack validation annotations. Adding validations like@NotBlankhelps ensure that all required properties are provided, preventing misconfigurations at runtime.Apply this diff to include validation:
@ConfigurationProperties(prefix = "oauth.apple") public record AppleOAuthClientProperties( + @NotBlank(message = "Token URL must not be blank") String tokenUrl, + @NotBlank(message = "Client Secret Audience URL must not be blank") String clientSecretAudienceUrl, + @NotBlank(message = "Redirect URL must not be blank") String redirectUrl, + @NotBlank(message = "Public Key URL must not be blank") String publicKeyUrl, + @NotBlank(message = "Client ID must not be blank") String clientId, + @NotBlank(message = "Team ID must not be blank") String teamId, + @NotBlank(message = "Key ID must not be blank") String keyId ) { }src/main/java/com/example/solidconnection/auth/client/AppleOAuthClient.java (1)
56-56: 🛠️ Refactor suggestionHandle potential null response body to prevent
NullPointerExceptionThe
response.getBody()might returnnullif the response does not contain a body. AccessingidToken()onnullwill cause aNullPointerException. Incorporate a null check to handle this scenario gracefully.Apply this diff to improve null handling:
); - return Objects.requireNonNull(response.getBody()).idToken(); + AppleTokenDto tokenDto = response.getBody(); + if (tokenDto == null || tokenDto.idToken() == null) { + throw new CustomException(APPLE_AUTHORIZATION_FAILED, "Failed to retrieve id_token from Apple"); + } + return tokenDto.idToken(); } catch (Exception e) { throw new CustomException(APPLE_AUTHORIZATION_FAILED, e.getMessage()); }
🧹 Nitpick comments (8)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java (3)
35-41: Consider adding API documentation.The Apple OAuth endpoint is well-implemented. Consider adding OpenAPI/Swagger annotations to document the endpoint's purpose, request/response models, and possible error scenarios.
Add these annotations:
+ @Operation(summary = "Process Apple OAuth authentication", + description = "Handles the OAuth code from Apple and returns authentication tokens") + @ApiResponses(value = { + @ApiResponse(responseCode = "200", description = "Successfully authenticated"), + @ApiResponse(responseCode = "400", description = "Invalid OAuth code"), + @ApiResponse(responseCode = "401", description = "Authentication failed") + }) @PostMapping("/apple") public ResponseEntity<OAuthResponse> processAppleOAuth(
43-49: Add matching API documentation for consistency.For consistency with the Apple OAuth endpoint, add similar OpenAPI/Swagger annotations here.
Add these annotations:
+ @Operation(summary = "Process Kakao OAuth authentication", + description = "Handles the OAuth code from Kakao and returns authentication tokens") + @ApiResponses(value = { + @ApiResponse(responseCode = "200", description = "Successfully authenticated"), + @ApiResponse(responseCode = "400", description = "Invalid OAuth code"), + @ApiResponse(responseCode = "401", description = "Authentication failed") + }) @PostMapping("/kakao") public ResponseEntity<OAuthResponse> processKakaoOAuth(
51-57: Update API documentation to reflect the new response type.The change to return
SignInResponseis a good UX improvement. Please update the existing API documentation to reflect this change.Update the documentation:
+ @Operation(summary = "Sign up a new user", + description = "Creates a new user account and returns authentication tokens") + @ApiResponses(value = { + @ApiResponse(responseCode = "200", description = "Successfully signed up and authenticated", + content = @Content(schema = @Schema(implementation = SignInResponse.class))), + @ApiResponse(responseCode = "400", description = "Invalid sign-up data") + }) @PostMapping("/sign-up") public ResponseEntity<SignInResponse> signUp(src/main/java/com/example/solidconnection/auth/dto/oauth/SignUpPrepareResponse.java (1)
3-8: Consider adding field validation.While the record structure is clean and provider-agnostic, consider adding validation for:
- Email format
- Required fields (especially email and signUpToken)
This can be achieved using Jakarta Validation annotations:
public record SignUpPrepareResponse( boolean isRegistered, + @NotBlank(message = "Nickname is required") String nickname, + @NotBlank(message = "Email is required") + @Email(message = "Invalid email format") String email, String profileImageUrl, + @NotBlank(message = "SignUpToken is required") String signUpToken) implements OAuthResponse {src/main/java/com/example/solidconnection/auth/dto/oauth/OAuthCodeRequest.java (1)
5-9: Consider internationalizing the error message.The record is well-designed for reuse across different OAuth providers. However, the hardcoded Korean error message might need internationalization for better maintainability and global usage.
Consider using message properties for internationalization:
- @NotBlank(message = "인증 코드를 입력해주세요.") + @NotBlank(message = "{validation.oauth.code.required}")src/main/java/com/example/solidconnection/auth/client/ApplePublicKeyProvider.java (1)
26-26: Preferjava.util.Base64over Apache'sBase64for standard decodingImporting
Base64fromorg.apache.tomcat.util.codec.binaryis unnecessary when Java's built-inBase64provides the required functionality. Using standard libraries reduces dependencies and potential issues with library versions.Apply this diff to use
java.util.Base64:-import static org.apache.tomcat.util.codec.binary.Base64.decodeBase64URLSafe; +import java.util.Base64; ... private PublicKey generatePublicKey(JsonNode key) throws Exception { - BigInteger modulus = new BigInteger(1, decodeBase64URLSafe(key.get("n").asText())); - BigInteger exponent = new BigInteger(1, decodeBase64URLSafe(key.get("e").asText())); + BigInteger modulus = new BigInteger(1, Base64.getUrlDecoder().decode(key.get("n").asText())); + BigInteger exponent = new BigInteger(1, Base64.getUrlDecoder().decode(key.get("e").asText())); RSAPublicKeySpec spec = new RSAPublicKeySpec(modulus, exponent); return KeyFactory.getInstance("RSA").generatePublic(spec); }src/main/java/com/example/solidconnection/custom/exception/ErrorCode.java (2)
14-22: LGTM! Consider adding documentation for error scenarios.The Apple OAuth error codes are well-structured with appropriate HTTP status codes and clear messages. They comprehensively cover both client-side validation errors (400) and server-side infrastructure issues (500).
Consider adding code comments to document specific scenarios when each error code should be used, especially for infrastructure-related errors like
APPLE_PUBLIC_KEY_NOT_FOUNDandAPPLE_CLIENT_SECRET_GENERATION_FAILED.
31-33: Consider adding error codes for additional OAuth sign-up scenarios.While the current error codes cover basic token validation, consider adding codes for other common OAuth sign-up scenarios:
- Token expiration
- Missing or malformed token data
- Rate limiting
Example additions:
OAUTH_SIGN_UP_TOKEN_INVALID(HttpStatus.BAD_REQUEST.value(), "유효하지 않은 회원가입 토큰입니다."), OAUTH_SIGN_UP_TOKEN_NOT_ISSUED_BY_SERVER(HttpStatus.BAD_REQUEST.value(), "회원가입 토큰이 우리 서버에서 발급되지 않았습니다."), + OAUTH_SIGN_UP_TOKEN_EXPIRED(HttpStatus.BAD_REQUEST.value(), "회원가입 토큰이 만료되었습니다."), + OAUTH_SIGN_UP_TOKEN_MALFORMED(HttpStatus.BAD_REQUEST.value(), "회원가입 토큰의 형식이 잘못되었습니다."), + OAUTH_SIGN_UP_RATE_LIMIT_EXCEEDED(HttpStatus.TOO_MANY_REQUESTS.value(), "회원가입 요청이 너무 많습니다. 잠시 후 다시 시도해주세요."),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/example/solidconnection/auth/client/AppleOAuthClient.java(1 hunks)src/main/java/com/example/solidconnection/auth/client/AppleOAuthClientSecretProvider.java(1 hunks)src/main/java/com/example/solidconnection/auth/client/ApplePublicKeyProvider.java(1 hunks)src/main/java/com/example/solidconnection/auth/controller/AuthController.java(2 hunks)src/main/java/com/example/solidconnection/auth/dto/oauth/OAuthCodeRequest.java(1 hunks)src/main/java/com/example/solidconnection/auth/dto/oauth/SignUpPrepareResponse.java(1 hunks)src/main/java/com/example/solidconnection/config/client/AppleOAuthClientProperties.java(1 hunks)src/main/java/com/example/solidconnection/custom/exception/ErrorCode.java(1 hunks)src/test/java/com/example/solidconnection/e2e/SignInTest.java(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/example/solidconnection/auth/client/AppleOAuthClientSecretProvider.java
- src/test/java/com/example/solidconnection/e2e/SignInTest.java
🔇 Additional comments (3)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java (1)
4-11: LGTM! Clean OAuth service integration.The new imports and service fields are well-structured and follow Spring's dependency injection pattern.
Also applies to: 31-33
src/main/java/com/example/solidconnection/auth/dto/oauth/OAuthCodeRequest.java (2)
1-4: LGTM! Clean package structure and imports.The package structure follows Java conventions, and the import is appropriate for the validation needs.
5-9: Verify usage across OAuth providers.Let's confirm that this DTO is properly integrated with both Apple and Kakao OAuth controllers.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for controller usage of OAuthCodeRequest ast-grep --pattern 'class $_ { $$$ $_($$_) { $$$ OAuthCodeRequest $$$ $$$ } $$$ }'Length of output: 104
Let's further verify the integration by searching for references to
OAuthCodeRequestin the auth package (including controllers). Run the following shell script to see if both the Apple and Kakao OAuth controllers (or other relevant controllers) properly reference this DTO:
🏁 Script executed:
#!/bin/bash # Search for usage of OAuthCodeRequest in all Java files within the auth package rg "OAuthCodeRequest" src/main/java/com/example/solidconnection/auth/ -t javaLength of output: 923
Let's gather additional information on the provider-specific integration. We'll search the auth package for keywords "apple" and "kakao" to ensure that the OAuthCodeRequest is used in both Apple and Kakao OAuth flows:
🏁 Script executed:
#!/bin/bash echo "Searching for 'apple' in auth package" rg -i "apple" src/main/java/com/example/solidconnection/auth/ -t java echo "Searching for 'kakao' in auth package" rg -i "kakao" src/main/java/com/example/solidconnection/auth/ -t javaLength of output: 17331
Integration Confirmation: OAuthCodeRequest is Properly Referenced in OAuth Providers
- AuthController Usage: The
OAuthCodeRequestDTO is imported and used inAuthController(both for Apple and Kakao endpoints).- Service Integration: It is passed to the respective OAuth services (
AppleOAuthServiceandKakaoOAuthService) via the commonOAuthServicemethod, confirming proper integration.
| public static SignUpPrepareResponse of(OAuthUserInfoDto oAuthUserInfoDto, String signUpToken) { | ||
| return new SignUpPrepareResponse( | ||
| false, | ||
| oAuthUserInfoDto.getNickname(), | ||
| oAuthUserInfoDto.getEmail(), | ||
| oAuthUserInfoDto.getProfileImageUrl(), | ||
| signUpToken | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and null checks.
The factory method should validate its input parameters to prevent NullPointerException:
public static SignUpPrepareResponse of(OAuthUserInfoDto oAuthUserInfoDto, String signUpToken) {
+ Objects.requireNonNull(oAuthUserInfoDto, "OAuthUserInfoDto must not be null");
+ Objects.requireNonNull(signUpToken, "SignUpToken must not be null");
+
return new SignUpPrepareResponse(
false,
oAuthUserInfoDto.getNickname(),
oAuthUserInfoDto.getEmail(),
oAuthUserInfoDto.getProfileImageUrl(),
signUpToken
);
}📝 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 static SignUpPrepareResponse of(OAuthUserInfoDto oAuthUserInfoDto, String signUpToken) { | |
| return new SignUpPrepareResponse( | |
| false, | |
| oAuthUserInfoDto.getNickname(), | |
| oAuthUserInfoDto.getEmail(), | |
| oAuthUserInfoDto.getProfileImageUrl(), | |
| signUpToken | |
| ); | |
| } | |
| public static SignUpPrepareResponse of(OAuthUserInfoDto oAuthUserInfoDto, String signUpToken) { | |
| Objects.requireNonNull(oAuthUserInfoDto, "OAuthUserInfoDto must not be null"); | |
| Objects.requireNonNull(signUpToken, "SignUpToken must not be null"); | |
| return new SignUpPrepareResponse( | |
| false, | |
| oAuthUserInfoDto.getNickname(), | |
| oAuthUserInfoDto.getEmail(), | |
| oAuthUserInfoDto.getProfileImageUrl(), | |
| signUpToken | |
| ); | |
| } |
| private String getKeyIdFromTokenHeader(String idToken) throws JsonProcessingException { | ||
| String[] jwtParts = idToken.split("\\."); | ||
| if (jwtParts.length < 2) { | ||
| throw new CustomException(INVALID_APPLE_ID_TOKEN); | ||
| } | ||
| String headerJson = new String(Base64.getUrlDecoder().decode(jwtParts[0]), StandardCharsets.UTF_8); | ||
| return new ObjectMapper().readTree(headerJson).get("kid").asText(); | ||
| } |
There was a problem hiding this comment.
Handle the absence of kid in the token header to prevent NullPointerException
In the getKeyIdFromTokenHeader method, if the kid field is missing in the token header, calling asText() on null will cause a NullPointerException. Ensure you check for null before accessing kid.
Apply this diff to handle missing kid:
private String getKeyIdFromTokenHeader(String idToken) throws JsonProcessingException {
String[] jwtParts = idToken.split("\\.");
if (jwtParts.length < 2) {
throw new CustomException(INVALID_APPLE_ID_TOKEN);
}
String headerJson = new String(Base64.getUrlDecoder().decode(jwtParts[0]), StandardCharsets.UTF_8);
- return new ObjectMapper().readTree(headerJson).get("kid").asText();
+ JsonNode kidNode = new ObjectMapper().readTree(headerJson).get("kid");
+ if (kidNode == null) {
+ throw new CustomException(INVALID_APPLE_ID_TOKEN);
+ }
+ return kidNode.asText();
}📝 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.
| private String getKeyIdFromTokenHeader(String idToken) throws JsonProcessingException { | |
| String[] jwtParts = idToken.split("\\."); | |
| if (jwtParts.length < 2) { | |
| throw new CustomException(INVALID_APPLE_ID_TOKEN); | |
| } | |
| String headerJson = new String(Base64.getUrlDecoder().decode(jwtParts[0]), StandardCharsets.UTF_8); | |
| return new ObjectMapper().readTree(headerJson).get("kid").asText(); | |
| } | |
| private String getKeyIdFromTokenHeader(String idToken) throws JsonProcessingException { | |
| String[] jwtParts = idToken.split("\\."); | |
| if (jwtParts.length < 2) { | |
| throw new CustomException(INVALID_APPLE_ID_TOKEN); | |
| } | |
| String headerJson = new String(Base64.getUrlDecoder().decode(jwtParts[0]), StandardCharsets.UTF_8); | |
| JsonNode kidNode = new ObjectMapper().readTree(headerJson).get("kid"); | |
| if (kidNode == null) { | |
| throw new CustomException(INVALID_APPLE_ID_TOKEN); | |
| } | |
| return kidNode.asText(); | |
| } |
| public PublicKey getApplePublicKey(String idToken) { | ||
| try { | ||
| String kid = getKeyIdFromTokenHeader(idToken); | ||
| if (applePublicKeyCache.containsKey(kid)) { | ||
| return applePublicKeyCache.get(kid); | ||
| } | ||
|
|
||
| fetchApplePublicKeys(); | ||
| if (applePublicKeyCache.containsKey(kid)) { | ||
| return applePublicKeyCache.get(kid); | ||
| } else { | ||
| throw new CustomException(APPLE_PUBLIC_KEY_NOT_FOUND); | ||
| } | ||
| } catch (ExpiredJwtException e) { | ||
| throw new CustomException(APPLE_ID_TOKEN_EXPIRED); | ||
| } catch (Exception e) { | ||
| throw new CustomException(INVALID_APPLE_ID_TOKEN); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid catching generic Exception; catch specific exceptions instead
In the getApplePublicKey method, catching the generic Exception can mask unexpected errors and make debugging difficult. It's better to catch specific exceptions to handle known error cases explicitly.
Apply this diff to improve exception handling:
} catch (ExpiredJwtException e) {
throw new CustomException(APPLE_ID_TOKEN_EXPIRED);
- } catch (Exception e) {
+ } catch (JsonProcessingException e) {
+ throw new CustomException(INVALID_APPLE_ID_TOKEN);
+ } catch (CustomException e) {
+ throw e; // Re-throw custom exceptions
+ } catch (Exception e) {
+ // Log unexpected exceptions
+ // logger.error("Unexpected exception", e);
throw new CustomException(INVALID_APPLE_ID_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.
| public PublicKey getApplePublicKey(String idToken) { | |
| try { | |
| String kid = getKeyIdFromTokenHeader(idToken); | |
| if (applePublicKeyCache.containsKey(kid)) { | |
| return applePublicKeyCache.get(kid); | |
| } | |
| fetchApplePublicKeys(); | |
| if (applePublicKeyCache.containsKey(kid)) { | |
| return applePublicKeyCache.get(kid); | |
| } else { | |
| throw new CustomException(APPLE_PUBLIC_KEY_NOT_FOUND); | |
| } | |
| } catch (ExpiredJwtException e) { | |
| throw new CustomException(APPLE_ID_TOKEN_EXPIRED); | |
| } catch (Exception e) { | |
| throw new CustomException(INVALID_APPLE_ID_TOKEN); | |
| } | |
| public PublicKey getApplePublicKey(String idToken) { | |
| try { | |
| String kid = getKeyIdFromTokenHeader(idToken); | |
| if (applePublicKeyCache.containsKey(kid)) { | |
| return applePublicKeyCache.get(kid); | |
| } | |
| fetchApplePublicKeys(); | |
| if (applePublicKeyCache.containsKey(kid)) { | |
| return applePublicKeyCache.get(kid); | |
| } else { | |
| throw new CustomException(APPLE_PUBLIC_KEY_NOT_FOUND); | |
| } | |
| } catch (ExpiredJwtException e) { | |
| throw new CustomException(APPLE_ID_TOKEN_EXPIRED); | |
| } catch (JsonProcessingException e) { | |
| throw new CustomException(INVALID_APPLE_ID_TOKEN); | |
| } catch (CustomException e) { | |
| throw e; // Re-throw custom exceptions | |
| } catch (Exception e) { | |
| // Log unexpected exceptions | |
| // logger.error("Unexpected exception", e); | |
| throw new CustomException(INVALID_APPLE_ID_TOKEN); | |
| } | |
| } |
| private void fetchApplePublicKeys() throws Exception { | ||
| ObjectMapper objectMapper = new ObjectMapper(); | ||
| ResponseEntity<String> response = restTemplate.getForEntity(properties.publicKeyUrl(), String.class); | ||
| JsonNode jsonNode = objectMapper.readTree(response.getBody()).get("keys"); | ||
|
|
||
| applePublicKeyCache.clear(); | ||
| for (JsonNode key : jsonNode) { | ||
| applePublicKeyCache.put(key.get("kid").asText(), generatePublicKey(key)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Ensure thread safety when updating the public key cache
In the fetchApplePublicKeys method, clearing and updating the applePublicKeyCache without proper synchronization can lead to race conditions in a multi-threaded environment. Consider updating the cache atomically.
Apply this diff to synchronize cache updates:
private void fetchApplePublicKeys() throws Exception {
ObjectMapper objectMapper = new ObjectMapper();
ResponseEntity<String> response = restTemplate.getForEntity(properties.publicKeyUrl(), String.class);
JsonNode jsonNode = objectMapper.readTree(response.getBody()).get("keys");
- applePublicKeyCache.clear();
- for (JsonNode key : jsonNode) {
- applePublicKeyCache.put(key.get("kid").asText(), generatePublicKey(key));
- }
+ Map<String, PublicKey> newKeys = new ConcurrentHashMap<>();
+ for (JsonNode key : jsonNode) {
+ newKeys.put(key.get("kid").asText(), generatePublicKey(key));
+ }
+ applePublicKeyCache.clear();
+ applePublicKeyCache.putAll(newKeys);
}📝 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.
| private void fetchApplePublicKeys() throws Exception { | |
| ObjectMapper objectMapper = new ObjectMapper(); | |
| ResponseEntity<String> response = restTemplate.getForEntity(properties.publicKeyUrl(), String.class); | |
| JsonNode jsonNode = objectMapper.readTree(response.getBody()).get("keys"); | |
| applePublicKeyCache.clear(); | |
| for (JsonNode key : jsonNode) { | |
| applePublicKeyCache.put(key.get("kid").asText(), generatePublicKey(key)); | |
| } | |
| } | |
| private void fetchApplePublicKeys() throws Exception { | |
| ObjectMapper objectMapper = new ObjectMapper(); | |
| ResponseEntity<String> response = restTemplate.getForEntity(properties.publicKeyUrl(), String.class); | |
| JsonNode jsonNode = objectMapper.readTree(response.getBody()).get("keys"); | |
| Map<String, PublicKey> newKeys = new ConcurrentHashMap<>(); | |
| for (JsonNode key : jsonNode) { | |
| newKeys.put(key.get("kid").asText(), generatePublicKey(key)); | |
| } | |
| applePublicKeyCache.clear(); | |
| applePublicKeyCache.putAll(newKeys); | |
| } |
| ); | ||
| return Objects.requireNonNull(response.getBody()).idToken(); | ||
| } catch (Exception e) { | ||
| throw new CustomException(APPLE_AUTHORIZATION_FAILED, e.getMessage()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid exposing internal exception messages to clients
Passing e.getMessage() directly to CustomException might reveal sensitive information to the client. Instead, log the exception internally and provide a generic error message.
Apply this diff to prevent information leakage:
} catch (Exception e) {
- throw new CustomException(APPLE_AUTHORIZATION_FAILED, e.getMessage());
+ // Log the exception message internally
+ // logger.error("Failed to request ID token from Apple", e);
+ throw new CustomException(APPLE_AUTHORIZATION_FAILED);
}📝 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.
| throw new CustomException(APPLE_AUTHORIZATION_FAILED, e.getMessage()); | |
| } catch (Exception e) { | |
| - throw new CustomException(APPLE_AUTHORIZATION_FAILED, e.getMessage()); | |
| + // Log the exception message internally | |
| + // logger.error("Failed to request ID token from Apple", e); | |
| + throw new CustomException(APPLE_AUTHORIZATION_FAILED); | |
| } |
1 participant
User description
관련 이슈
작업 내용
특이 사항
리뷰 요구사항 (선택)
PR Type
Enhancement, Tests
Description
Added Apple OAuth integration, including client and service implementations.
Refactored existing OAuth structure for better abstraction and reusability.
Introduced unified DTOs and token handling for multiple OAuth providers.
Updated and added tests for OAuth services and token validation.
Changes walkthrough 📝
18 files
Implement Apple OAuth client for token handlingAdd Apple OAuth client secret generationRefactor Kakao OAuth client to use propertiesAdd Apple and Kakao OAuth endpointsSimplify SignInResponse DTOUpdate SignUpRequest to support multiple OAuth typesRemove unused SignUpResponse DTOAdd DTO for Apple token responseAdd DTO for Apple user infoMove KakaoTokenDto to unified OAuth packageRefactor KakaoUserInfoDto to implement OAuthUserInfoDtoAdd DTO for OAuth code requestAdd OAuthSignInResponse DTOAdd interface for unified OAuth user infoImplement Apple OAuth serviceImplement Kakao OAuth serviceAdd abstract OAuth service for common logicRefactor sign-up service to support OAuth2 files
Add tests for SignInServiceAdd tests for SignUpTokenProvider18 files
Summary by CodeRabbit
New Features
Refactor