Conversation
WalkthroughThis pull request standardizes and enhances the authentication and signup flows. Login methods in controllers and handler classes are updated to return unified response types (using Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
src/main/java/org/runimo/runimo/auth/jwt/JwtTokenFactory.java (1)
52-65:⚠️ Potential issueInconsistency between method name and token type claim
The method has been renamed from
generateRegisterTemporalTokentogenerateSignupTemporalTokenand now accepts akeyparameter for the JWT subject. However, on line 63, the "tokenType" claim still uses "register" instead of "signup", which is inconsistent with the method name change.- .withClaim("tokenType", "register") + .withClaim("tokenType", "signup")src/main/java/org/runimo/runimo/auth/service/apple/AppleTokenVerifier.java (2)
92-116: 🛠️ Refactor suggestionValidate HTTP status before parsing the response.
In
getAppleTokenPairFromAuthCode, consider checking if the response is a successful (2xx) status code before attempting to parse the JSON body. This prevents potential null-pointer or parsing issues when Apple’s endpoint returns non-2xx responses (such as 4xx or 5xx) with different payloads.ResponseEntity<String> response = restTemplate.postForEntity( "https://appleid.apple.com/auth/token", request, String.class); + if (!response.getStatusCode().is2xxSuccessful()) { + log.error("Apple returned non-2xx status: {}", response.getStatusCode()); + throw UserJwtException.of(UserHttpResponseCode.TOKEN_INVALID); + } try { JsonNode node = objectMapper.readTree(response.getBody()); ...
143-166: 🛠️ Refactor suggestionAddress potential concurrency issues with
publicKeysmap.The
publicKeysmap is refreshed by a scheduled task and is also accessed concurrently inverifyTokenAndExtractUserInfo. Using a plainHashMapcan lead to race conditions or partial updates. Consider switching to a thread-safe structure (e.g.,ConcurrentHashMap) or synchronizing the refresh logic to avoid potential concurrency errors.-private final Map<String, RSAPublicKey> publicKeys = new HashMap<>(); +private final Map<String, RSAPublicKey> publicKeys = new ConcurrentHashMap<>();
🧹 Nitpick comments (11)
src/main/java/org/runimo/runimo/common/CreatedAuditEntity.java (1)
1-21: Good implementation of creation timestamp trackingThis audit entity class provides a clean way to track creation timestamps across entities. It's properly annotated with JPA and Lombok annotations to reduce boilerplate.
Consider specifying a timezone for the
createdAtfield to ensure consistent timestamp handling, especially since you'll be using this timestamp for the 10-minute token expiration window mentioned in the PR objectives. TheGlobalConsts.TIME_ZONE_ID("Asia/Seoul") could be used for consistency.src/main/java/org/runimo/runimo/auth/service/EncryptUtil.java (1)
22-35: Improved error handling in encrypt methodThe encryption method now properly handles exceptions internally instead of propagating them. This approach is more robust and prevents callers from having to handle checked exceptions.
Consider applying the same pattern to the
decryptmethod for consistency:- public String decrypt(String cipherText) throws Exception { + public String decrypt(String cipherText) { + try { Cipher cipher = Cipher.getInstance(CIPHER_TRANS); SecretKeySpec keySpec = new SecretKeySpec(secretKey.getBytes(), ALGORITHM); IvParameterSpec ivSpec = new IvParameterSpec(iv.getBytes()); cipher.init(Cipher.DECRYPT_MODE, keySpec, ivSpec); byte[] decodedBytes = Base64.getDecoder().decode(cipherText); byte[] decrypted = cipher.doFinal(decodedBytes); return new String(decrypted); + } catch (Exception e) { + throw new RuntimeException("Error during decryption", e); + } }🧰 Tools
🪛 ast-grep (0.31.1)
[warning] 23-23: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: Cipher.getInstance(CIPHER_TRANS)
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html(use-of-aes-ecb-java)
[warning] 23-23: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: Cipher.getInstance(CIPHER_TRANS)
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA(desede-is-deprecated-java)
src/main/resources/sql/schema.sql (1)
80-87: Consider adding indexes to the signup_token tableThe
signup_tokentable looks good, but consider adding indexes to columns that will be frequently used in WHERE clauses, especially for theprovider_idandcreated_atfields since they'll likely be used for lookups and expiration checks.CREATE TABLE `signup_token` ( `token` VARCHAR(255) PRIMARY KEY NOT NULL UNIQUE , `provider_id` VARCHAR(255) NOT NULL, `refresh_token` VARCHAR(255), `provider` VARCHAR(255), `created_at` TIMESTAMP DEFAULT CURRENT_TIMESTAMP + INDEX `idx_signup_token_provider_id` (`provider_id`), + INDEX `idx_signup_token_created_at` (`created_at`) );src/main/java/org/runimo/runimo/auth/service/apple/AppleTokenVerifier.java (1)
44-44: Consider externalizing the URL.While defining
REVOKE_URLas astatic finalis clear, you may want to externalize it (e.g., via application configuration) to facilitate environment-based overrides and future changes without code modifications.src/main/java/org/runimo/runimo/auth/service/dtos/AuthResult.java (1)
1-42: Correct the spelling of the constructor parameter.In the constructor on line 13, the parameter
stausis likely a typo forstatus. Consider renaming it to maintain clarity and consistency.- public AuthResult(final AuthStatus staus, final User user, final TokenPair tokenPair, + public AuthResult(final AuthStatus status, final User user, final TokenPair tokenPair,src/main/java/org/runimo/runimo/auth/service/SignUpUsecaseImpl.java (2)
25-25: Consider externalizing the cutoff value.The
REGISTER_CUTOFF_MIN = 10is a magic number. Consider moving it to configuration (e.g., application properties) so it can be adjusted without a code change.
50-55: Prefer throwing a more descriptive exception.Currently,
orElseThrow(IllegalAccessError::new);is used.IllegalAccessErrortypically indicates reflection-level access violations. Consider using a more semantically relevant runtime exception (e.g.,NoSuchElementExceptionor a custom business exception).src/main/java/org/runimo/runimo/auth/controller/AuthController.java (2)
55-59: Consider validating the token parameter.
Although this login flow is presumably guarded by framework-level validations, adding a quick non-null or non-empty check forrequest.oidcToken()could help prevent potential misuse.
120-136: Re-examine using 404 for unregistered users.
While returning 404 can be valid if you treat an unregistered user as “resource not found,” you might also consider using 401 or 403 for better alignment with typical authentication semantics.src/main/java/org/runimo/runimo/auth/service/apple/AppleLoginHandler.java (2)
43-59: Add optional logging or null checks.
The overall flow is sound, but consider adding log statements to help diagnose errors if the token verification or user lookup fails unexpectedly.
61-76: Consider hashing refresh tokens instead of encryption.
If the system only needs to match tokens rather than decrypt them, storing a hashed version can reduce risk in the event of a data breach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
src/main/java/org/runimo/runimo/auth/controller/AuthController.java(4 hunks)src/main/java/org/runimo/runimo/auth/controller/AuthResponse.java(1 hunks)src/main/java/org/runimo/runimo/auth/domain/SignupToken.java(1 hunks)src/main/java/org/runimo/runimo/auth/jwt/JwtResolver.java(1 hunks)src/main/java/org/runimo/runimo/auth/jwt/JwtTokenFactory.java(1 hunks)src/main/java/org/runimo/runimo/auth/jwt/SignupTokenPayload.java(1 hunks)src/main/java/org/runimo/runimo/auth/repository/SignupTokenRepository.java(1 hunks)src/main/java/org/runimo/runimo/auth/service/EncryptUtil.java(1 hunks)src/main/java/org/runimo/runimo/auth/service/OidcService.java(2 hunks)src/main/java/org/runimo/runimo/auth/service/SignUpUsecaseImpl.java(1 hunks)src/main/java/org/runimo/runimo/auth/service/apple/AppleLoginHandler.java(2 hunks)src/main/java/org/runimo/runimo/auth/service/apple/AppleTokenVerifier.java(3 hunks)src/main/java/org/runimo/runimo/auth/service/dtos/AuthResponse.java(0 hunks)src/main/java/org/runimo/runimo/auth/service/dtos/AuthResult.java(1 hunks)src/main/java/org/runimo/runimo/auth/service/dtos/AuthStatus.java(1 hunks)src/main/java/org/runimo/runimo/auth/service/kakao/KakaoLoginHandler.java(2 hunks)src/main/java/org/runimo/runimo/common/CreatedAuditEntity.java(1 hunks)src/main/java/org/runimo/runimo/exceptions/GlobalExceptionHandler.java(0 hunks)src/main/java/org/runimo/runimo/exceptions/RegisterErrorResponse.java(1 hunks)src/main/resources/sql/schema.sql(2 hunks)src/test/java/org/runimo/runimo/auth/controller/AuthControllerTest.java(4 hunks)src/test/java/org/runimo/runimo/rewards/RewardTest.java(2 hunks)src/test/java/org/runimo/runimo/user/api/UserItemAcceptanceTest.java(2 hunks)src/test/resources/sql/schema.sql(2 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/org/runimo/runimo/exceptions/GlobalExceptionHandler.java
- src/main/java/org/runimo/runimo/auth/service/dtos/AuthResponse.java
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/main/java/org/runimo/runimo/auth/domain/SignupToken.java (1)
src/main/java/org/runimo/runimo/common/GlobalConsts.java (1)
NoArgsConstructor(8-21)
src/main/java/org/runimo/runimo/common/CreatedAuditEntity.java (1)
src/main/java/org/runimo/runimo/common/GlobalConsts.java (1)
NoArgsConstructor(8-21)
src/test/java/org/runimo/runimo/auth/controller/AuthControllerTest.java (1)
src/test/java/org/runimo/runimo/user/UserFixtures.java (1)
UserFixtures(6-27)
src/test/java/org/runimo/runimo/rewards/RewardTest.java (1)
src/main/java/org/runimo/runimo/auth/jwt/JwtTokenFactory.java (1)
JwtTokenFactory(10-72)
src/main/java/org/runimo/runimo/auth/controller/AuthController.java (1)
src/main/java/org/runimo/runimo/common/response/ErrorResponse.java (1)
ErrorResponse(5-22)
src/main/java/org/runimo/runimo/auth/service/SignUpUsecaseImpl.java (2)
src/main/java/org/runimo/runimo/auth/jwt/JwtResolver.java (1)
JwtResolver(11-44)src/main/java/org/runimo/runimo/auth/jwt/JwtTokenFactory.java (1)
JwtTokenFactory(10-72)
🪛 ast-grep (0.31.1)
src/main/java/org/runimo/runimo/auth/service/EncryptUtil.java
[warning] 23-23: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: Cipher.getInstance(CIPHER_TRANS)
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html
(use-of-aes-ecb-java)
[warning] 23-23: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: Cipher.getInstance(CIPHER_TRANS)
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
(desede-is-deprecated-java)
🔇 Additional comments (43)
src/main/java/org/runimo/runimo/auth/service/dtos/AuthStatus.java (1)
1-6: Clean enum implementation for authentication statusThis enum provides a clear way to represent authentication outcomes. The two states (
LOGIN_SUCCESSandSIGNUP_NEEDED) align well with the authentication flow changes described in the PR objectives, particularly for handling the scenario when users need to register.src/main/java/org/runimo/runimo/auth/controller/AuthResponse.java (1)
1-12: Well-structured response record with clear OpenAPI documentationThe
AuthResponserecord effectively encapsulates all necessary information for authentication responses. Good use of the@Schemaannotation to provide clear documentation for API consumers.src/main/java/org/runimo/runimo/auth/jwt/SignupTokenPayload.java (1)
5-7: Appropriate renaming and token field additionThe record has been renamed from
RegisterTokenPayloadtoSignupTokenPayloadfor consistency, and the newtokenfield has been added to support the UUID-based identification described in the PR objectives. These changes align well with the temporary token storage mechanism being implemented.src/main/java/org/runimo/runimo/auth/repository/SignupTokenRepository.java (1)
1-15: Well-structured repository implementation for token management.This repository interface is well-designed for managing signup tokens with proper expiration handling through the custom query method. The implementation correctly extends JpaRepository for standard CRUD operations and adds a specialized query to find unexpired tokens, which aligns with the PR's objective of implementing temporary token storage with expiration checks.
src/test/java/org/runimo/runimo/user/api/UserItemAcceptanceTest.java (2)
18-18: Correctly added UUID import.This import has been properly added to support the generation of random UUIDs for signup tokens.
139-141: Updated method call to match new implementation.This test has been correctly updated to use the renamed method
generateSignupTemporalTokenwith the additional UUID parameter, aligning with the changes in the authentication flow described in the PR objectives.src/main/java/org/runimo/runimo/auth/domain/SignupToken.java (5)
1-14: Appropriate imports for entity class.All necessary imports for JPA annotations, Lombok, and domain classes have been included correctly.
15-19: Well-designed entity class with proper annotations.The entity is correctly annotated with JPA and Lombok annotations, extending the audit entity for creation timestamp tracking.
21-23: Appropriate ID field configuration.The token field is correctly configured as the primary key with non-null and unique constraints, which is essential for secure token handling.
25-33: Well-structured entity fields with appropriate constraints.The fields are properly annotated with appropriate nullability constraints. The provider_id is correctly marked as non-nullable while refresh_token is nullable (as it's only needed for Apple login). The social provider is appropriately stored as a string enum.
35-42: Builder pattern implementation for flexible object creation.The builder pattern is appropriately implemented, making it easy to create instances with specific parameters. All fields are properly initialized in the constructor.
src/main/java/org/runimo/runimo/auth/jwt/JwtResolver.java (2)
29-30: Method renamed for consistency with signup flow.The method has been renamed from
getRegisterTokenPayloadtogetSignupTokenPayloadfor consistency with the overall signup token approach.
37-38: Updated return statement to include JWT subject as token identifier.The return statement now correctly includes the JWT subject as the first parameter to the SignupTokenPayload constructor. This aligns with the PR objective of using a UUID for identification in the signup process.
src/test/resources/sql/schema.sql (2)
3-3: LGTM: Properly added drop statement for the new tableThe addition of the
DROP TABLE IF EXISTS signup_token;statement ensures clean test database initialization.
80-87: Approve new signup_token table schemaThe new
signup_tokentable has been properly defined with appropriate columns to support the temporary storage of social login data during the signup process.src/main/java/org/runimo/runimo/auth/service/EncryptUtil.java (1)
15-16: LGTM: Good constant definitionsExtracting the cipher transformation and algorithm as constants improves maintainability.
src/main/resources/sql/schema.sql (2)
3-3: LGTM: Proper drop statement for the new tableAdding
DROP TABLE IF EXISTS signup_token;ensures clean database initialization.
69-78: LGTM: Appropriate placement of apple_user_token table definitionThe
apple_user_tokentable definition is properly structured.src/test/java/org/runimo/runimo/rewards/RewardTest.java (1)
8-9: Integration of SignupToken functionality looks good.The implementation correctly follows the new signup flow by generating a UUID key, creating a temporary token using
generateSignupTemporalToken, and storing the token details in the database throughsignupTokenRepository. This aligns with the PR objective of temporarily storing social data for the registration process.I particularly like that you're using
UUID.randomUUID()to generate unique keys for each test run, which ensures test isolation.Also applies to: 13-14, 15-16, 58-59, 63-76
src/test/java/org/runimo/runimo/auth/controller/AuthControllerTest.java (3)
9-10: Clean integration of ObjectMapper and new DTOs.The imports and autowired ObjectMapper support the updated authentication flow and JSON serialization needs.
Also applies to: 12-13, 18-21, 55-56
76-80: Great update to the unregistered user test case.The test now correctly reflects the new authentication flow where a signup token is returned instead of throwing an exception. This aligns with the PR objective of issuing a temporary token when a user is not found.
Also applies to: 85-86
96-103: Updated success case matches new response structure.Good job updating the test to use
AuthResult.successwith the proper token pair and user fixture. The expected values for nickname and image URL now match theUserFixturesimplementation.Also applies to: 111-113
src/main/java/org/runimo/runimo/exceptions/RegisterErrorResponse.java (1)
4-5: Good inheritance structure for RegisterErrorResponse.The class now properly extends the base
Responseclass and usesCustomResponseCodefor standardized error handling. This is a good refactoring that improves code organization and maintains consistency across the application's response types.Also applies to: 8-9, 12-14
src/main/java/org/runimo/runimo/auth/service/OidcService.java (1)
5-6: Consistent return type update to AuthResult.Both
kakaoLoginandappleLoginmethods now returnAuthResultinstead ofAuthResponse, which aligns with the standardized response structure mentioned in the PR objectives. This change ensures consistency across the authentication flow.Also applies to: 16-17, 20-21
src/main/java/org/runimo/runimo/auth/service/SignUpUsecaseImpl.java (8)
3-5: No concerns for updated imports.
8-9: No concerns for newly added imports.
12-13: No concerns for newly added imports.
15-15: No concerns for newly added import.
19-19: Transactional annotation is appropriate.Using
@Transactionalon the registration method ensures atomicity when creating users and tokens, preventing partial data persistence.
28-30: Fields injection check.The addition of
signupTokenRepository,appleUserTokenRepository, andjwtResolverfields looks good. Confirm that these beans are properly available and tested.
44-46: Apple-specific token logic is well-placed.Conditionally invoking
createAppleUserTokenis a straightforward approach for Apple logins. This separation ensures each provider’s unique flows are handled in an isolated manner.
57-63: Consider checking for null refresh tokens.In
createAppleUserToken,signupToken.getRefreshToken()may benullif not properly set. You may want to validate or handle a null refresh token scenario to avoid unexpected database states.src/main/java/org/runimo/runimo/auth/service/kakao/KakaoLoginHandler.java (5)
6-16: No concerns for added imports and new fields.
31-31:SignupTokenRepositoryfield injection recognized.
33-55: Enhanced authentication flow.The
validateAndLoginmethod returningAuthResultis more expressive and aligns with the uniform response approach used across the codebase. Nice improvement in clarity and handling signup-needed vs. login-success states.
57-68: Temporary signup token creation logic is coherent.Creating a random UUID for the DB entity and returning a signed JWT to the client is consistent. Ensure that the token is invalidated or removed if unused after 10 minutes to prevent buildup or possible misuse.
69-75: Improved error handling for JWT decoding.Decoding and throwing a dedicated exception for an invalid JWT helps maintain consistent error reporting. This approach is a solid way to handle malformed tokens.
src/main/java/org/runimo/runimo/auth/controller/AuthController.java (3)
18-19: No issues with the new imports.
23-23: No concerns about adding theResponseimport.
73-77: Apple login flow is consistent with the new return type.
No major concerns here, as the logic matches the Kakao login approach.src/main/java/org/runimo/runimo/auth/service/apple/AppleLoginHandler.java (3)
6-7: Added imports look good.
9-9: Repository introduction and DTO imports are consistent with the new signup token logic.Also applies to: 12-12, 14-15, 32-32
78-82:Details
❓ Verification inconclusive
Confirm actual signature verification outside this method.
This method only decodes the token without verifying its signature. It’s likely verified inappleTokenVerifier.verifyTokenAndExtractUserInfo, but let’s confirm proper usage.
🏁 Script executed:
#!/bin/bash # Checking all occurrences of decodeJwtOrThrow to ensure a subsequent signature verification step: rg -A 5 "decodeJwtOrThrow"Length of output: 2916
Ensure JWT Signature Verification in AppleLoginHandler Is Confirmed
The
decodeJwtOrThrowmethod is correctly limited to decoding the JWT. In theAppleLoginHandler, the decoded token is immediately passed toappleTokenVerifier.verifyTokenAndExtractUserInfo—mirroring the pattern seen in the Kakao handler—to handle signature verification. Please confirm thatverifyTokenAndExtractUserInfodoes perform complete JWT signature validation, and consider adding a brief in-line comment clarifying that responsibility.
| @Transactional | ||
| public SignupUserResponse register(UserSignupCommand command) { | ||
| RegisterTokenPayload payload = jwtResolver.getRegisterTokenPayload(command.registerToken()); | ||
| SignupTokenPayload payload = jwtResolver.getSignupTokenPayload(command.registerToken()); | ||
| SignupToken signupToken = findUnExpiredSignupToken(payload.token()); | ||
| User savedUser = userRegisterService.registerUser(new UserRegisterCommand( | ||
| command.nickname(), | ||
| command.imgUrl(), | ||
| command.gender(), | ||
| payload.providerId(), | ||
| payload.socialProvider()) | ||
| ); | ||
| if (payload.socialProvider() == SocialProvider.APPLE) { | ||
| createAppleUserToken(savedUser.getId(), signupToken); | ||
| } | ||
| return new SignupUserResponse(savedUser, jwtTokenFactory.generateTokenPair(savedUser)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle missing or invalid tokens more clearly.
In the register method, if command.registerToken() is null or the token is missing specific claims, it will throw. Consider returning a more meaningful exception or custom error response to facilitate client-side handling.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/org/runimo/runimo/auth/service/apple/AppleLoginHandler.java (2)
35-41: Comprehensive Javadoc but consider adding English translation.The Javadoc is detailed but written in Korean. Consider adding English translation to improve accessibility for international contributors or adding a bilingual comment.
/** * 애플 로그인 검증을 처리하고 사용자를 로그인 또는 회원가입으로 안내합니다. + * Validates Apple login and guides users to either login or signup. * * @param authCode iOS 앱에서 받은 oidc 인증 코드. + * @param authCode OIDC authentication code received from iOS app. * @param verifier iOS 앱에서 받은 PKCE용 Verifier. + * @param verifier PKCE verifier received from iOS app. * @return 인증 결과를 담은 객체; 로그인 성공 시 사용자 정보와 토큰 쌍을 포함합니다. 실패시 회원가입 토큰을 반환합니다. + * @return AuthResult object containing authentication results; includes user info and token pair on success, or signup token on failure. */
66-76: Well-implemented temporary signup token creation.This new method effectively implements the PR objective of temporarily storing social data in the database. It:
- Generates a UUID as required
- Properly encrypts the sensitive refresh token
- Saves provider_id and provider information
- Stores the token in the database via the repository
However, there's no explicit handling for token expiration or cleanup. According to the PR objectives, this will be addressed in future work, but consider adding a TODO comment to document this intention.
public SignupToken createTemporalSignupToken(AppleUserInfo appleUserInfo, TokenPair appleTokens) { String temporalTokenId = UUID.randomUUID().toString(); String encryptedRefreshToken = encryptUtil.encrypt(appleTokens.refreshToken()); SignupToken signupToken = new SignupToken( temporalTokenId, appleUserInfo.getProviderId(), encryptedRefreshToken, SocialProvider.APPLE ); + // TODO: Implement scheduled cleanup of expired tokens (see PR #54) return signupTokenRepository.save(signupToken); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/org/runimo/runimo/auth/service/apple/AppleLoginHandler.java(2 hunks)src/main/java/org/runimo/runimo/user/service/dtos/UserRegisterCommand.java(1 hunks)
🔇 Additional comments (7)
src/main/java/org/runimo/runimo/user/service/dtos/UserRegisterCommand.java (2)
3-3: Added import for validation annotationAdding the import for Jakarta validation's
@NotNullconstraint is necessary for implementing the field-level validations below.
11-12: Appropriate validation added to social authentication fieldsAdding the
@NotNullvalidation toproviderIdandsocialProviderfields aligns with the PR objective of standardizing authentication flows. This ensures that these critical fields for social authentication cannot be null during user registration, which is essential since these values are retrieved from the temporary token storage.Note that other fields (nickname, imgUrl, gender) remain without
@NotNullannotations, which suggests they're optional during registration. This appears to be intentional, but you may want to confirm this is the desired behavior.src/main/java/org/runimo/runimo/auth/service/apple/AppleLoginHandler.java (5)
6-7: Added imports for functional changes look appropriate.The addition of Optional and UUID imports align with the new functionality for handling signup tokens and proper nullability management.
9-16: Imports support the new authentication flow correctly.The new imports reflect the significant changes to the authentication flow, with SignupToken, SignupTokenRepository, and the new AuthResult/AuthStatus DTOs supporting the temporary token storage approach mentioned in the PR objectives.
32-32: Dependency injection for SignupTokenRepository correctly added.This is necessary for the new functionality that stores temporary social login data.
62-64: Refactored successful login flow aligns with PR objectives.This segment properly handles the successful login case when the user is already registered, generating a token pair and returning an appropriate AuthResult. The implementation correctly follows the design pattern established in the PR objectives.
78-84: Good error handling extraction improves code maintainability.Extracting the JWT decoding logic into a separate method improves code readability and maintainability. The method properly handles JWTDecodeException and throws a more specific UserJwtException with an appropriate error code.
2 participants
작업 내역
#53 이슈에서 논의한 바를 적용했습니다.
1. 회원 미가입으로 인한 실패 시, 회원가입을 위한 소셜 데이터를 데이터베이스에 임시저장
provider_id,provider를 공통으로 저장2. 미가입 시 404 에러와, 회원가입에 사용되는 임시 토큰을 발급
AuthResult객체를 반환3. 회원 가입 시, 임시로 저장했던 데이터를 활용
refresh_token을apple_user_token테이블에 저장.추후 작업할 내용
created_at을 활용하여 검증하고 있습니다.Summary by CodeRabbit
New Features
Refactor
Chores