[WTH-64] 애플로 로그인 기능 추가 #226
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (1)
Tip You can get early access to new features in CodeRabbit.Enable the WalkthroughApple OAuth 인증 경로(로그인·회원가입)와 관련 엔티티·DTO·예외·OAuth2 서버 구성 및 변환기/그랜트 타입을 추가하고, User 엔티티에 appleId 도입과 관련 저장소/서비스/컨트롤러/매퍼/유스케이스를 확장했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant UCtrl as UserController
participant UCase as UserUseCase
participant AppleAuth as AppleAuthService
participant Repo as UserRepository
participant Mapper as UserMapper
User->>UCtrl: POST /api/v1/users/apple/register (appleAuthCode)
UCtrl->>UCase: appleRegister(Register)
UCase->>AppleAuth: getAppleToken(authCode)
AppleAuth-->>UCase: AppleTokenResponse(id_token)
UCase->>AppleAuth: verifyAndDecodeIdToken(id_token)
AppleAuth-->>UCase: AppleUserInfo(appleId)
UCase->>Repo: save(User with appleId)
Repo-->>UCase: User
UCase-->>UCtrl: success
UCtrl-->>User: 200 OK
User->>UCtrl: POST /api/v1/users/apple/login (appleAuthCode)
UCtrl->>UCase: appleLogin(Login)
UCase->>AppleAuth: getAppleToken(authCode)
AppleAuth-->>UCase: AppleTokenResponse
UCase->>AppleAuth: verifyAndDecodeIdToken(id_token)
AppleAuth-->>UCase: AppleUserInfo
UCase->>Repo: findByAppleId(appleId)
alt user exists
UCase->>Mapper: toAppleLoginResponse(User, JwtDto)
Mapper-->>UCase: SocialLoginResponse (LOGIN)
UCase-->>UCtrl: token response
UCtrl-->>User: 200 + tokens
else user missing
UCase-->>UCtrl: INTEGRATE response
UCtrl-->>User: 200 INTEGRATE
end
sequenceDiagram
participant Client as Apple 클라이언트
participant AuthCtrl as AuthController
participant AuthUse as AuthUsecase
participant AppleAuth as AppleAuthService
participant UserSvc as UserGetService
participant Security as OAuth2AuthorizationServerConfig
Client->>AuthCtrl: GET /apple/oauth?code=...&id_token=...
AuthCtrl->>AuthUse: appleLogin(code, id_token)
AuthUse->>AppleAuth: getAppleToken(code)
AppleAuth-->>AuthUse: AppleTokenResponse
AuthUse->>AppleAuth: verifyAndDecodeIdToken(id_token)
AppleAuth-->>AuthUse: AppleUserInfo
AuthUse->>UserSvc: findByAppleId(appleId)
UserSvc-->>AuthUse: User
AuthCtrl->>Security: save SecurityContext + redirect
AuthCtrl-->>Client: Redirect (성공)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/leets/weeth/domain/user/application/dto/request/UserRequestDto.java (1)
30-42:RegisterDTO에 상호 배타성 검증이 없으므로 비즈니스 로직에 검증 로직을 추가해야 합니다.코드 검토 결과,
UserUseCaseImpl의validate(Register dto)메서드(약 229-234줄)는 학번과 연락처 중복만 확인하고kakaoId와appleAuthCode중 정확히 하나만 제공되는지는 검증하지 않습니다.
appleRegister는 라인 279에서dto.appleAuthCode()를 직접 사용하고,socialRegister는kakaoId검증이 명시적이지 않습니다. 따라서 다음 시나리오가 모두 가능합니다:
- 둘 다 null
- 둘 다 null이 아님
- 하나만 값을 가짐 (예상된 경우)
validate(Register dto)메서드에 다음과 같은 검증을 추가하세요:if ((dto.kakaoId() == null && dto.appleAuthCode() == null) || (dto.kakaoId() != null && dto.appleAuthCode() != null)) { throw new InvalidAuthenticationMethodException(); }src/main/java/leets/weeth/domain/user/application/dto/response/UserResponseDto.java (1)
13-21:toIntegrateResponse매퍼에 필드 매핑 누락이 있습니다.appleIdToken이 추가되면서
SocialLoginResponse의 시그니처가 변경된 후,toIntegrateResponse(Long kakaoId)메서드에서 필요한 필드 매핑이 불완전합니다.현재 매핑 (UserMapper.java, 56-62줄):
@Mappings({ @Mapping(target = "status", expression = "java(LoginStatus.INTEGRATE)"), @Mapping(target = "appleIdToken", expression = "java(null)"), @Mapping(target = "accessToken", expression = "java(null)"), @Mapping(target = "refreshToken", expression = "java(null)") }) SocialLoginResponse toIntegrateResponse(Long kakaoId);문제점:
id와kakaoId필드에 대한 매핑이 없어서 두 필드가 모두 null이 됩니다.
toAppleIntegrateResponse처럼 다음과 같이 수정되어야 합니다:@Mappings({ @Mapping(target = "status", expression = "java(LoginStatus.INTEGRATE)"), @Mapping(target = "id", expression = "java(null)"), @Mapping(target = "kakaoId", source = "kakaoId"), @Mapping(target = "appleIdToken", expression = "java(null)"), @Mapping(target = "accessToken", expression = "java(null)"), @Mapping(target = "refreshToken", expression = "java(null)") })
🧹 Nitpick comments (3)
src/main/java/leets/weeth/global/config/SecurityConfig.java (1)
77-77: Apple 엔드포인트가 올바르게 화이트리스트에 추가되었습니다.Apple 로그인/회원가입 엔드포인트가
permitAll에 적절히 추가되었습니다.참고: 이 라인이 매우 길어 가독성이 떨어집니다. 향후 엔드포인트 목록을 별도 상수 배열로 추출하는 것을 고려해보세요.
src/main/java/leets/weeth/global/auth/apple/dto/ApplePublicKey.java (1)
3-3: 사용하지 않는 import를 제거하세요.
JsonProperty어노테이션이 코드에서 사용되지 않고 있습니다. Record의 컴포넌트들이 JSON 필드명과 동일하다면 Jackson이 자동으로 매핑하므로 이 import는 불필요합니다.-import com.fasterxml.jackson.annotation.JsonProperty; - public record ApplePublicKey(src/main/java/leets/weeth/global/sas/config/grant/AppleIdentityTokenAuthenticationToken.java (1)
19-20: AppleGrantType 상수를 재사용해 주세요.동일한 문자열로
AuthorizationGrantType를 새로 생성하지 말고AppleGrantType.APPLE_IDENTITY_TOKEN을 사용하면 비교 로직과 설정 간 일관성이 유지됩니다.- super(new AuthorizationGrantType("apple_identity_token"), clientPrincipal, additionalParameters); + super(AppleGrantType.APPLE_IDENTITY_TOKEN, clientPrincipal, additionalParameters);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
src/main/java/leets/weeth/domain/user/application/dto/request/UserRequestDto.java(2 hunks)src/main/java/leets/weeth/domain/user/application/dto/response/UserResponseDto.java(1 hunks)src/main/java/leets/weeth/domain/user/application/mapper/UserMapper.java(2 hunks)src/main/java/leets/weeth/domain/user/application/usecase/UserUseCase.java(1 hunks)src/main/java/leets/weeth/domain/user/application/usecase/UserUseCaseImpl.java(3 hunks)src/main/java/leets/weeth/domain/user/domain/entity/User.java(2 hunks)src/main/java/leets/weeth/domain/user/domain/repository/UserRepository.java(1 hunks)src/main/java/leets/weeth/domain/user/domain/service/UserGetService.java(1 hunks)src/main/java/leets/weeth/domain/user/presentation/UserController.java(1 hunks)src/main/java/leets/weeth/global/auth/apple/AppleAuthService.java(1 hunks)src/main/java/leets/weeth/global/auth/apple/dto/ApplePublicKey.java(1 hunks)src/main/java/leets/weeth/global/auth/apple/dto/ApplePublicKeys.java(1 hunks)src/main/java/leets/weeth/global/auth/apple/dto/AppleTokenResponse.java(1 hunks)src/main/java/leets/weeth/global/auth/apple/dto/AppleUserInfo.java(1 hunks)src/main/java/leets/weeth/global/config/SecurityConfig.java(1 hunks)src/main/java/leets/weeth/global/sas/application/exception/AppleLoginException.java(1 hunks)src/main/java/leets/weeth/global/sas/application/exception/ErrorMessage.java(1 hunks)src/main/java/leets/weeth/global/sas/application/mapper/OAuth2AuthorizationConverter.java(3 hunks)src/main/java/leets/weeth/global/sas/application/usecase/AuthUsecase.java(3 hunks)src/main/java/leets/weeth/global/sas/config/OAuth2AuthorizationServerConfig.java(5 hunks)src/main/java/leets/weeth/global/sas/config/grant/AppleAuthenticationProvider.java(1 hunks)src/main/java/leets/weeth/global/sas/config/grant/AppleGrantType.java(1 hunks)src/main/java/leets/weeth/global/sas/config/grant/AppleIdentityTokenAuthenticationConverter.java(1 hunks)src/main/java/leets/weeth/global/sas/config/grant/AppleIdentityTokenAuthenticationToken.java(1 hunks)src/main/java/leets/weeth/global/sas/domain/service/RedisOAuth2AuthorizationService.java(1 hunks)src/main/java/leets/weeth/global/sas/presentation/AuthController.java(1 hunks)src/main/resources/application.yml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/main/java/leets/weeth/global/sas/config/grant/AppleIdentityTokenAuthenticationConverter.java (1)
src/main/java/leets/weeth/global/sas/config/grant/AppleGrantType.java (1)
AppleGrantType(5-10)
src/main/java/leets/weeth/global/sas/application/mapper/OAuth2AuthorizationConverter.java (1)
src/main/java/leets/weeth/global/sas/config/grant/AppleGrantType.java (1)
AppleGrantType(5-10)
src/main/java/leets/weeth/global/sas/config/grant/AppleAuthenticationProvider.java (2)
src/main/java/leets/weeth/global/sas/application/exception/AppleLoginException.java (1)
AppleLoginException(6-10)src/main/java/leets/weeth/global/sas/config/grant/AppleGrantType.java (1)
AppleGrantType(5-10)
src/main/java/leets/weeth/global/sas/config/OAuth2AuthorizationServerConfig.java (1)
src/main/java/leets/weeth/global/sas/config/grant/AppleGrantType.java (1)
AppleGrantType(5-10)
🔇 Additional comments (15)
src/main/java/leets/weeth/global/sas/domain/service/RedisOAuth2AuthorizationService.java (2)
40-41: LGTM!refresh token이 존재할 때 해당 토큰의 만료 시간을 사용하는 로직이 올바르게 구현되어 있습니다.
42-44: LGTM!refresh token이 없을 때 access token의 만료 시간을 사용하는 명확한 fallback 로직입니다. 주석도 의도를 잘 설명하고 있습니다.
src/main/resources/application.yml (1)
30-37: Apple OAuth 설정이 올바르게 추가되었습니다.Apple OAuth 프로바이더 설정이 기존 Kakao 구성과 일관된 패턴으로 잘 구현되었습니다. 민감한 값들이 환경 변수로 적절히 외부화되어 있습니다.
src/main/java/leets/weeth/global/config/SecurityConfig.java (1)
79-79: Apple OAuth 경로가 적절히 추가되었습니다.기존 Kakao OAuth 패턴과 일관되게
/apple/oauth경로가 추가되었습니다.src/main/java/leets/weeth/domain/user/domain/repository/UserRepository.java (1)
21-22: Apple ID 조회 메서드가 올바르게 추가되었습니다.기존
findByKakaoId와 일관된 패턴으로 구현되었으며, Spring Data JPA 규칙을 올바르게 따릅니다.src/main/java/leets/weeth/domain/user/domain/entity/User.java (1)
99-101: Apple ID 설정 메서드가 적절히 구현되었습니다.기존
addKakaoId메서드와 일관된 패턴으로 구현되었습니다.src/main/java/leets/weeth/domain/user/domain/service/UserGetService.java (1)
36-38: Apple ID 조회 서비스 메서드가 올바르게 추가되었습니다.기존 Kakao ID 조회 메서드와 일관된 구조로 구현되었습니다.
src/main/java/leets/weeth/global/auth/apple/dto/ApplePublicKeys.java (1)
5-8: Apple 공개 키 DTO가 적절히 구현되었습니다.Apple의 JWKS 엔드포인트 응답을 역직렬화하기 위한 단순하고 적절한 레코드 구조입니다.
src/main/java/leets/weeth/domain/user/application/usecase/UserUseCase.java (1)
40-42: 인터페이스 확장이 깔끔합니다.기존 Kakao 로그인 메서드들과 일관된 패턴으로 Apple 로그인 메서드들이 추가되었습니다. 메서드 시그니처와 네이밍이 적절합니다.
src/main/java/leets/weeth/global/auth/apple/dto/AppleUserInfo.java (1)
5-11: 구조가 명확하고 적절합니다.Apple 사용자 정보를 담는 DTO로 적절하며,
@Builder패턴을 사용하여 유연한 객체 생성을 지원합니다.src/main/java/leets/weeth/global/auth/apple/dto/AppleTokenResponse.java (1)
3-10: OAuth2 토큰 응답 구조가 적절합니다.Apple의 토큰 응답 형식을 정확하게 반영하고 있으며, snake_case 필드명이 Apple API 응답과 일치합니다.
src/main/java/leets/weeth/domain/user/presentation/UserController.java (1)
71-76: Apple 로그인 엔드포인트 구현이 적절합니다.기존 Kakao 로그인과 일관된 패턴으로 구현되어 있습니다.
src/main/java/leets/weeth/global/sas/config/grant/AppleGrantType.java (1)
5-10: Grant 타입 상수 클래스가 올바르게 구현되었습니다.
private생성자로 인스턴스화를 방지하고,public static final상수로 Apple identity token grant 타입을 정의하는 표준 패턴을 따르고 있습니다.src/main/java/leets/weeth/global/sas/application/exception/ErrorMessage.java (1)
12-13: 에러 메시지 추가가 적절합니다.기존
KAKAO_AUTH_ERROR와 일관된 패턴으로APPLE_AUTH_ERROR가 추가되었으며, 에러 코드 넘버링도 순차적으로 잘 관리되고 있습니다.src/main/java/leets/weeth/global/sas/application/exception/AppleLoginException.java (1)
6-10: OAuth2 예외 처리가 올바르게 구현되었습니다.
OAuth2AuthenticationException을 확장하여 Apple 로그인 전용 예외를 정의하고,ErrorMessage.APPLE_AUTH_ERROR를 사용하여 일관된 에러 코드 관리를 하고 있습니다.
src/main/java/leets/weeth/domain/user/application/usecase/UserUseCaseImpl.java
Outdated
Show resolved
Hide resolved
| @PostMapping("/apple/register") | ||
| @Operation(summary = "애플 소셜 회원가입 (dev 전용 - 바로 ACTIVE)") | ||
| public CommonResponse<Void> appleRegister(@RequestBody @Valid Register dto) { | ||
| userUseCase.appleRegister(dto); | ||
| return CommonResponse.createSuccess(USER_APPLY_SUCCESS.getMessage()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
개발 전용 자동 승인 로직의 프로덕션 사용을 검증하세요.
엔드포인트 설명에 "dev 전용 - 바로 ACTIVE"라고 명시되어 있으나, 코드 상에서는 환경에 따른 분기 처리가 보이지 않습니다. 앱 심사를 위한 의도적인 구현이라고 명시되어 있지만, 프로덕션 환경에서 이 엔드포인트가 노출되지 않도록 하거나 환경별로 승인 로직이 다르게 동작하는지 확인이 필요합니다.
다음 스크립트로 환경별 분기 처리 여부를 확인해주세요:
🏁 Script executed:
#!/bin/bash
# Description: Apple 회원가입 관련 환경별 분기 처리 확인
# Apple register 구현체에서 환경 변수나 프로파일 기반 분기 확인
rg -n "appleRegister" --type=java -A 20 | rg -i "profile|environment|active|dev|prod"
# 사용자 승인 관련 로직 확인
rg -n "setStatus|ACTIVE|PENDING" --type=java -g "*User*Impl.java" -C 5Length of output: 1174
환경별 분기 처리를 더 자세히 확인하겠습니다.
추가 정보를 수집하겠습니다. 전체 메서드 구현과 클래스 레벨의 조건부 설정을 확인하겠습니다.
프로덕션 환경에서 자동 승인 로직이 보호되지 않습니다.
실행 결과 확인 결과, UserUseCaseImpl.java의 appleRegister 메서드(293-294줄)에서 user.accept()가 환경별 조건 없이 무조건 실행되고 있습니다. 코드에 "dev 전용"이라는 주석이 있지만, 실제 환경 기반 분기 처리(예: @Profile, Environment 인젝션을 통한 조건 체크)가 없으므로 프로덕션 환경에서도 동일하게 자동 승인됩니다.
필수 조치:
appleRegister메서드에 환경 확인 로직 추가 (예:Environment.getActiveProfiles()로 "dev" 프로파일만 검사)- 또는
@Profile("dev")를appleRegister메서드에 적용하여 dev 환경에서만 등록되도록 제한 - 또는 설정 속성(
@ConditionalOnProperty)을 활용하여 제어
🤖 Prompt for AI Agents
In src/main/java/leets/weeth/domain/user/presentation/UserController.java around
lines 78-83, the controller endpoint is labeled "dev 전용" but the actual
dangerous logic is in
src/main/java/leets/weeth/domain/user/application/UserUseCaseImpl.java around
lines 293-294 where user.accept() is invoked unconditionally; fix by restricting
automatic acceptance to dev only — either inject Spring Environment and check
Environment.getActiveProfiles() includes "dev" before calling user.accept(), or
annotate the method with @Profile("dev") (or use @ConditionalOnProperty and a
feature flag) so that user.accept() runs only when the dev profile/property is
active; ensure tests/beans still work by updating configuration and adding a
clear log/error path for non-dev flows.
| private final RestClient restClient = RestClient.create(); | ||
|
|
||
| /** | ||
| * Authorization code로 애플 토큰 요청 | ||
| * client_secret은 JWT로 생성 (ES256 알고리즘) | ||
| */ | ||
| public AppleTokenResponse getAppleToken(String authCode) { | ||
| String clientSecret = generateClientSecret(); | ||
|
|
||
| MultiValueMap<String, String> body = new LinkedMultiValueMap<>(); | ||
| body.add("grant_type", "authorization_code"); | ||
| body.add("client_id", appleClientId); | ||
| body.add("client_secret", clientSecret); | ||
| body.add("code", authCode); | ||
| body.add("redirect_uri", redirectUri); | ||
|
|
||
| return restClient.post() | ||
| .uri(tokenUri) | ||
| .body(body) | ||
| .contentType(MediaType.APPLICATION_FORM_URLENCODED) | ||
| .retrieve() | ||
| .body(AppleTokenResponse.class); | ||
| } | ||
|
|
||
| /** | ||
| * ID Token 검증 및 사용자 정보 추출 | ||
| * 애플은 별도 userInfo 엔드포인트가 없고 ID Token에 정보가 포함됨 | ||
| */ | ||
| public AppleUserInfo verifyAndDecodeIdToken(String idToken) { | ||
| try { | ||
| // 1. ID Token의 헤더에서 kid 추출 | ||
| String[] tokenParts = idToken.split("\\."); | ||
| String header = new String(Base64.getUrlDecoder().decode(tokenParts[0])); | ||
| Map<String, Object> headerMap = parseJson(header); | ||
| String kid = (String) headerMap.get("kid"); | ||
|
|
||
| // 2. 애플 공개키 가져오기 | ||
| ApplePublicKeys publicKeys = restClient.get() | ||
| .uri(keysUri) | ||
| .retrieve() | ||
| .body(ApplePublicKeys.class); | ||
|
|
There was a problem hiding this comment.
HTTP 타임아웃을 반드시 설정해야 합니다.
지금 RestClient.create() 기본값으로 Apple 토큰/키 엔드포인트를 호출하면 connect/read 타임아웃이 무제한입니다. 애플 쪽이 지연되거나 응답을 닫지 않으면 SAS 요청 스레드가 끝없이 대기하면서 서비스 전체가 묶일 수 있습니다. 최소한 수 초 단위의 타임아웃을 명시해 주세요.
아래처럼 RestClient 생성 구간을 수정하고, java.time.Duration, org.springframework.http.client.SimpleClientHttpRequestFactory 임포트를 추가하면 됩니다.
-import org.springframework.web.client.RestClient;
+import org.springframework.http.client.SimpleClientHttpRequestFactory;
+import org.springframework.web.client.RestClient;
@@
-import java.util.Map;
+import java.time.Duration;
+import java.util.Map;
@@
- private final RestClient restClient = RestClient.create();
+ private final RestClient restClient = RestClient.builder()
+ .requestFactory(() -> {
+ SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory();
+ factory.setConnectTimeout(Duration.ofSeconds(5));
+ factory.setReadTimeout(Duration.ofSeconds(5));
+ return factory;
+ })
+ .build();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/leets/weeth/global/auth/apple/AppleAuthService.java around
lines 59 to 100, the RestClient is created with RestClient.create() which leaves
connect/read timeouts unlimited; replace this with a RestClient/RestTemplate
instance configured with a SimpleClientHttpRequestFactory (setConnectTimeout and
setReadTimeout) using java.time.Duration for values (e.g., 3–5 seconds) so calls
to Apple's token/keys endpoints cannot hang indefinitely, and add imports for
java.time.Duration and
org.springframework.http.client.SimpleClientHttpRequestFactory; ensure the new
client instance is used where restClient is referenced.
| if (entity.getAccessToken() == null && entity instanceof OAuth2AuthorizationCodeGrantAuthorization codeGrant) { | ||
| entity.updateExpire(calculateTtlSeconds(((OAuth2AuthorizationCodeGrantAuthorization) entity).getAuthorizationCode().getExpiresAt())); | ||
| } else { | ||
| entity.updateExpire(calculateTtlSeconds(codeGrant.getAuthorizationCode().getExpiresAt())); |
There was a problem hiding this comment.
🧩 Analysis chain
인가 코드의 null 안전성 검증 고려.
패턴 매칭을 사용한 타입 체크는 깔끔합니다. 다만, codeGrant.getAuthorizationCode()가 null일 가능성이 있는지 확인이 필요합니다. 만약 null일 수 있다면 NPE(NullPointerException)가 발생할 수 있습니다.
다음 스크립트로 OAuth2AuthorizationCodeGrantAuthorization의 authorizationCode 필드가 null일 수 있는지 확인해주세요:
🏁 Script executed:
#!/bin/bash
# Description: OAuth2AuthorizationCodeGrantAuthorization 엔티티에서 authorizationCode가 null 가능한지 확인
# authorizationCode 필드의 선언과 사용 패턴 검색
ast-grep --pattern 'class OAuth2AuthorizationCodeGrantAuthorization {
$$$
}'
# authorizationCode가 null로 설정되거나 체크되는 패턴 검색
rg -nP --type=java -C3 'authorizationCode.*null|null.*authorizationCode' Length of output: 3695
Based on the verification:
Per RFC 6749 section 5.1 the expires_in in the response is recommended, but not required. Thus, OAuth2AccessToken.getExpiresAt() can be null. Additionally, the script output confirmed that authorizationCode itself can be null (as shown in OAuth2AuthorizationMapper.java line 61 which explicitly checks if (authorizationCode == null)).
codeGrant.getAuthorizationCode() 또는 getExpiresAt()이 null일 가능성이 있습니다.
lines 38-39에서 codeGrant.getAuthorizationCode().getExpiresAt()를 직접 호출하기 전에 null 안전성 검사가 필요합니다. 다음과 같이 수정하세요:
if (entity.getAccessToken() == null && entity instanceof OAuth2AuthorizationCodeGrantAuthorization codeGrant) {
AuthorizationCode authCode = codeGrant.getAuthorizationCode();
if (authCode != null && authCode.getExpiresAt() != null) {
entity.updateExpire(calculateTtlSeconds(authCode.getExpiresAt()));
}
}또는 null 값에 대한 기본값을 설정하세요.
🤖 Prompt for AI Agents
In
src/main/java/leets/weeth/global/sas/domain/service/RedisOAuth2AuthorizationService.java
around lines 38-39, prevent potential NPE by not calling
codeGrant.getAuthorizationCode().getExpiresAt() directly; extract
AuthorizationCode authCode = codeGrant.getAuthorizationCode() and only call
updateExpire(calculateTtlSeconds(...)) if authCode != null and
authCode.getExpiresAt() != null, or alternatively provide a sensible default TTL
when either value is null.
| } else { | ||
| // access token도 없으면 기본값으로 1시간 설정 | ||
| entity.updateExpire(3600L); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
매직 넘버를 상수로 추출 권장.
기본 TTL 값인 3600L(1시간)이 하드코딩되어 있습니다. 유지보수성과 가독성을 위해 클래스 레벨의 상수로 추출하는 것을 권장합니다.
다음과 같이 상수로 추출할 수 있습니다:
+ private static final long DEFAULT_AUTHORIZATION_TTL_SECONDS = 3600L;
+
@Override
public void save(OAuth2Authorization authorization) {그리고 사용 부분을 수정:
} else {
// access token도 없으면 기본값으로 1시간 설정
- entity.updateExpire(3600L);
+ entity.updateExpire(DEFAULT_AUTHORIZATION_TTL_SECONDS);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/leets/weeth/global/sas/domain/service/RedisOAuth2AuthorizationService.java
around lines 45-47, the TTL literal 3600L is a magic number; extract it to a
class-level constant (e.g., private static final long DEFAULT_TTL_SECONDS =
3600L) and replace the hardcoded 3600L with that constant where updateExpire is
called; ensure the constant name is clear and, if appropriate, add a brief
comment or JavaDoc describing it.
|
|
||
| @Mappings({ | ||
| @Mapping(target = "status", expression = "java(LoginStatus.INTEGRATE)"), | ||
| @Mapping(target = "appleIdToken", expression = "java(null)"), |
There was a problem hiding this comment.
appleIdToken는 왜 null값이 들어가나요? kakaoId는 null값이 아닌거로 이해했어요
There was a problem hiding this comment.
현재 해당 메서드가 카카오 연동 과정에서만 쓰이고 있어요! 애플은 toAppleIntegrateResponse로 별도로 채워서 보여주고 있습니당
| com.fasterxml.jackson.databind.ObjectMapper objectMapper = new com.fasterxml.jackson.databind.ObjectMapper(); | ||
| return objectMapper.readValue(json, Map.class); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("JSON 파싱 실패: " + e.getMessage()); |
There was a problem hiding this comment.
보안상 에러메세지는 서버 로그로 남기거나 없애는 편이 좋을거같아요
위에 있는 에러메세지들도요!
src/main/java/leets/weeth/global/sas/application/exception/AppleLoginException.java
Outdated
Show resolved
Hide resolved
src/main/java/leets/weeth/global/sas/config/grant/AppleAuthenticationProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/leets/weeth/global/sas/config/grant/AppleGrantType.java
Outdated
Show resolved
Hide resolved
...main/java/leets/weeth/global/sas/config/grant/AppleIdentityTokenAuthenticationConverter.java
Outdated
Show resolved
Hide resolved
src/main/java/leets/weeth/global/sas/config/grant/AppleIdentityTokenAuthenticationToken.java
Outdated
Show resolved
Hide resolved
|
|
||
| if (optionalUser.isEmpty()) { | ||
| throw new UserNotFoundException(); // -> Weeth 회원가입 페이지로 리다이렉트 | ||
| } |
There was a problem hiding this comment.
UserNotFoundException예외를 던지면 프론트 측에서 리다이렉트를 처리하는건가요 ?
|
|
||
| // SAS용 SecurityFilterChain | ||
| @Bean | ||
| @Order(1) // 우선순위를 기본 filter보다 높게 설정 |
There was a problem hiding this comment.
order 어노테이션의 필터 우선순위를 조절할 수 있군요! 배워갑니다!
seokjun01
left a comment
seokjun01
left a comment
There was a problem hiding this comment.
고생하셨습니다 .. 코드 양이 많고 로직이 복잡해 이해가 쉽지 않아서 우선 승인부터 해두고, 차차 다시 읊어보겠습니다!
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/main/java/leets/weeth/global/auth/apple/AppleAuthService.java (1)
60-60: HTTP 타임아웃 설정이 누락되었습니다.이전 리뷰에서 지적된 대로,
RestClient.create()는 무제한 타임아웃을 가지고 있어 Apple 서버 응답 지연 시 서비스 전체가 블로킹될 수 있습니다. 반드시 connect/read 타임아웃을 설정해야 합니다.이전 리뷰에서 제안된 수정사항을 적용해 주세요.
🧹 Nitpick comments (3)
src/main/java/leets/weeth/domain/user/application/usecase/UserUseCaseImpl.java (1)
299-314: isDevEnvironment 메서드를 최적화할 수 있습니다.매번 호출 시 배열을 순회하는 것보다 Arrays.asList()와 contains()를 사용하면 더 간결합니다.
다음과 같이 리팩토링할 수 있습니다:
+ import java.util.Arrays; + /** * 현재 환경이 dev 프로파일인지 확인 * @return dev 프로파일이 활성화되어 있으면 true */ private boolean isDevEnvironment() { String[] activeProfiles = environment.getActiveProfiles(); - for (String profile : activeProfiles) { - if ("dev".equals(profile)) { - return true; - } - if ("local".equals(profile)) { - return true; - } - } - return false; + return Arrays.asList(activeProfiles).contains("dev") + || Arrays.asList(activeProfiles).contains("local"); }src/main/java/leets/weeth/global/auth/apple/AppleAuthService.java (2)
66-82: authCode 검증을 추가하세요.Apple 토큰 요청 전에 authCode가 null이거나 비어있는지 검증하는 것이 좋습니다.
메서드 시작 부분에 검증 로직 추가:
public AppleTokenResponse getAppleToken(String authCode) { + if (authCode == null || authCode.isBlank()) { + throw new AppleAuthenticationException(); + } String clientSecret = generateClientSecret();
140-178: Apple 클라이언트 시크릿 JWT 만료 기간 검토 (보안 권장사항)현재 코드의 5개월(라인 161:
plusMonths(5)) 설정은 Apple의 최대 허용 기간인 6개월 이내로 규정을 준수합니다. 다만 보안을 강화하기 위해 만료 기간을 더 단축(예: 1-3개월)하여 클라이언트 시크릿 갱신 주기를 단축하는 것을 고려하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/leets/weeth/domain/user/application/usecase/UserUseCaseImpl.java(3 hunks)src/main/java/leets/weeth/domain/user/domain/entity/User.java(4 hunks)src/main/java/leets/weeth/global/auth/apple/AppleAuthService.java(1 hunks)src/main/java/leets/weeth/global/auth/apple/exception/AppleAuthenticationException.java(1 hunks)src/main/java/leets/weeth/global/sas/application/exception/AppleLoginException.java(1 hunks)src/main/java/leets/weeth/global/sas/config/grant/AppleAuthenticationProvider.java(1 hunks)src/main/java/leets/weeth/global/sas/config/grant/AppleGrantType.java(1 hunks)src/main/java/leets/weeth/global/sas/config/grant/AppleIdentityTokenAuthenticationConverter.java(1 hunks)src/main/java/leets/weeth/global/sas/config/grant/AppleIdentityTokenAuthenticationToken.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/java/leets/weeth/global/sas/application/exception/AppleLoginException.java
- src/main/java/leets/weeth/global/sas/config/grant/AppleGrantType.java
- src/main/java/leets/weeth/global/sas/config/grant/AppleIdentityTokenAuthenticationToken.java
- src/main/java/leets/weeth/domain/user/domain/entity/User.java
- src/main/java/leets/weeth/global/sas/config/grant/AppleIdentityTokenAuthenticationConverter.java
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/leets/weeth/global/auth/apple/AppleAuthService.java (1)
src/main/java/leets/weeth/global/auth/apple/exception/AppleAuthenticationException.java (1)
AppleAuthenticationException(5-9)
src/main/java/leets/weeth/global/sas/config/grant/AppleAuthenticationProvider.java (2)
src/main/java/leets/weeth/global/sas/application/exception/AppleLoginException.java (1)
AppleLoginException(6-10)src/main/java/leets/weeth/global/sas/config/grant/AppleGrantType.java (1)
AppleGrantType(5-10)
src/main/java/leets/weeth/domain/user/application/usecase/UserUseCaseImpl.java (4)
src/main/java/leets/weeth/domain/user/domain/service/UserGetService.java (1)
Service(16-91)src/main/java/leets/weeth/domain/user/domain/service/CardinalGetService.java (1)
Service(12-55)src/main/java/leets/weeth/global/auth/jwt/application/usecase/JwtManageUseCase.java (1)
Service(14-59)src/main/java/leets/weeth/domain/user/domain/service/UserSaveService.java (1)
Service(8-17)
🔇 Additional comments (3)
src/main/java/leets/weeth/global/auth/apple/exception/AppleAuthenticationException.java (1)
1-9: 예외 클래스 구현이 적절합니다.애플 인증 실패에 대한 전용 예외 클래스로, 401 상태 코드와 명확한 에러 메시지를 제공하고 있습니다.
src/main/java/leets/weeth/domain/user/application/usecase/UserUseCaseImpl.java (1)
244-269: 애플 로그인 로직을 검토해 주세요.로그인 흐름 자체는 합리적이지만, 이전 리뷰에서 지적된 보안 문제(appleIdToken 로깅)가 아직 해결되지 않았습니다. 민감한 토큰 정보는 로그에 남기지 말아야 합니다.
src/main/java/leets/weeth/global/sas/config/grant/AppleAuthenticationProvider.java (1)
1-73: 커스텀 인증 프로바이더 구현이 올바릅니다.Apple ID 토큰 검증 및 사용자 로드 로직이 적절하게 구현되어 있습니다. 사용자의 비활성 상태도 올바르게 검증하고 있습니다.
참고: 이전 리뷰에서 파일 끝에 개행 추가를 요청했습니다.
src/main/java/leets/weeth/domain/user/application/usecase/UserUseCaseImpl.java
Show resolved
Hide resolved
| private void validateClaims(Claims claims) { | ||
| String iss = claims.getIssuer(); | ||
| String aud = claims.getAudience(); | ||
|
|
||
| if (!iss.equals("https://appleid.apple.com")) { | ||
| throw new RuntimeException("유효하지 않은 발급자(issuer)입니다."); | ||
| } | ||
|
|
||
| if (!aud.equals(appleClientId)) { | ||
| throw new RuntimeException("유효하지 않은 수신자(audience)입니다."); | ||
| } | ||
|
|
||
| Date expiration = claims.getExpiration(); | ||
| if (expiration.before(new Date())) { | ||
| throw new RuntimeException("만료된 ID Token입니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
일관된 예외 타입을 사용하세요.
validateClaims 메서드에서 RuntimeException 대신 AppleAuthenticationException을 던져야 합니다. 이는 다른 메서드들과 일관성을 유지하고, 클라이언트가 적절한 에러 응답을 받을 수 있게 합니다.
다음과 같이 수정하세요:
private void validateClaims(Claims claims) {
String iss = claims.getIssuer();
String aud = claims.getAudience();
if (!iss.equals("https://appleid.apple.com")) {
- throw new RuntimeException("유효하지 않은 발급자(issuer)입니다.");
+ throw new AppleAuthenticationException();
}
if (!aud.equals(appleClientId)) {
- throw new RuntimeException("유효하지 않은 수신자(audience)입니다.");
+ throw new AppleAuthenticationException();
}
Date expiration = claims.getExpiration();
if (expiration.before(new Date())) {
- throw new RuntimeException("만료된 ID Token입니다.");
+ throw new AppleAuthenticationException();
}
}📝 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 validateClaims(Claims claims) { | |
| String iss = claims.getIssuer(); | |
| String aud = claims.getAudience(); | |
| if (!iss.equals("https://appleid.apple.com")) { | |
| throw new RuntimeException("유효하지 않은 발급자(issuer)입니다."); | |
| } | |
| if (!aud.equals(appleClientId)) { | |
| throw new RuntimeException("유효하지 않은 수신자(audience)입니다."); | |
| } | |
| Date expiration = claims.getExpiration(); | |
| if (expiration.before(new Date())) { | |
| throw new RuntimeException("만료된 ID Token입니다."); | |
| } | |
| } | |
| private void validateClaims(Claims claims) { | |
| String iss = claims.getIssuer(); | |
| String aud = claims.getAudience(); | |
| if (!iss.equals("https://appleid.apple.com")) { | |
| throw new AppleAuthenticationException(); | |
| } | |
| if (!aud.equals(appleClientId)) { | |
| throw new AppleAuthenticationException(); | |
| } | |
| Date expiration = claims.getExpiration(); | |
| if (expiration.before(new Date())) { | |
| throw new AppleAuthenticationException(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/leets/weeth/global/auth/apple/AppleAuthService.java around
lines 217 to 233, the validateClaims method currently throws RuntimeException
for invalid issuer, audience, and expired token; replace those with
AppleAuthenticationException to keep exception types consistent with other
methods and ensure proper client error handling — update each throw to
instantiate and throw AppleAuthenticationException with the same message (and
add/import the class if missing).
| private Map<String, Object> parseJson(String json) { | ||
| try { | ||
| com.fasterxml.jackson.databind.ObjectMapper objectMapper = new com.fasterxml.jackson.databind.ObjectMapper(); | ||
| return objectMapper.readValue(json, Map.class); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("JSON 파싱 실패"); | ||
| } | ||
| } |
There was a problem hiding this comment.
parseJson 메서드의 예외 처리를 개선하세요.
RuntimeException 대신 AppleAuthenticationException을 사용하여 일관된 예외 처리를 유지하세요.
다음과 같이 수정하세요:
@SuppressWarnings("unchecked")
private Map<String, Object> parseJson(String json) {
try {
com.fasterxml.jackson.databind.ObjectMapper objectMapper = new com.fasterxml.jackson.databind.ObjectMapper();
return objectMapper.readValue(json, Map.class);
} catch (Exception e) {
- throw new RuntimeException("JSON 파싱 실패");
+ log.error("JSON 파싱 실패", e);
+ throw new AppleAuthenticationException();
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/leets/weeth/global/auth/apple/AppleAuthService.java around
lines 239 to 246, the parseJson method currently catches Exception and throws a
generic RuntimeException; replace this with throwing an
AppleAuthenticationException to preserve consistent error handling by changing
the catch to wrap the original exception (pass the caught exception as the
cause) into a new AppleAuthenticationException with the message "JSON 파싱 실패" so
the original exception stack and message are preserved for debugging; also add
the necessary import if missing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/leets/weeth/domain/user/application/usecase/UserUseCaseImpl.java (2)
244-269: Apple 인증 정보 추출 로직을 헬퍼 메서드로 분리하는 것을 고려하세요.
appleLogin과appleRegister메서드 모두 동일한 패턴으로 Apple 토큰을 가져오고 ID 토큰을 검증하여 사용자 정보를 추출합니다(lines 248-249, 277-278). 이 로직을 별도의 헬퍼 메서드로 추출하면 코드 중복을 줄이고 유지보수성을 높일 수 있습니다.예시:
+ private AppleUserInfo getAppleUserInfo(String authCode) { + AppleTokenResponse tokenResponse = appleAuthService.getAppleToken(authCode); + return appleAuthService.verifyAndDecodeIdToken(tokenResponse.id_token()); + } + + private String getAppleIdToken(String authCode) { + AppleTokenResponse tokenResponse = appleAuthService.getAppleToken(authCode); + return tokenResponse.id_token(); + }그런 다음 두 메서드에서 활용:
public SocialLoginResponse appleLogin(Login dto) { - AppleTokenResponse tokenResponse = appleAuthService.getAppleToken(dto.authCode()); - AppleUserInfo userInfo = appleAuthService.verifyAndDecodeIdToken(tokenResponse.id_token()); - - String appleIdToken = tokenResponse.id_token(); - String appleId = userInfo.appleId(); + String appleIdToken = getAppleIdToken(dto.authCode()); + AppleUserInfo userInfo = appleAuthService.verifyAndDecodeIdToken(appleIdToken); + String appleId = userInfo.appleId();
298-313: isDevEnvironment 메서드를 간소화할 수 있습니다.현재 반복문으로 프로파일을 확인하는 로직을 Stream API나 Collection 메서드로 더 간결하게 작성할 수 있습니다.
다음과 같이 리팩터링할 수 있습니다:
private boolean isDevEnvironment() { - String[] activeProfiles = environment.getActiveProfiles(); - for (String profile : activeProfiles) { - if ("dev".equals(profile)) { - return true; - } - if ("local".equals(profile)) { - return true; - } - } - return false; + return Arrays.stream(environment.getActiveProfiles()) + .anyMatch(profile -> "dev".equals(profile) || "local".equals(profile)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/leets/weeth/domain/user/application/usecase/UserUseCaseImpl.java(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/leets/weeth/domain/user/application/usecase/UserUseCaseImpl.java (4)
src/main/java/leets/weeth/domain/user/domain/service/UserGetService.java (1)
Service(16-91)src/main/java/leets/weeth/domain/user/domain/service/CardinalGetService.java (1)
Service(12-55)src/main/java/leets/weeth/global/auth/jwt/application/usecase/JwtManageUseCase.java (1)
Service(14-59)src/main/java/leets/weeth/domain/user/domain/service/UserSaveService.java (1)
Service(8-17)
🔇 Additional comments (1)
src/main/java/leets/weeth/domain/user/application/usecase/UserUseCaseImpl.java (1)
271-296: Apple 회원가입 로직이 잘 구현되었습니다.트랜잭션 범위 내에서 사용자 엔티티를 저장하고 dev/local 환경에서 자동 승인 처리하는 로직이 적절합니다.
user.accept()호출이 같은 트랜잭션 내에서 이루어져 상태 변경이 올바르게 영속화됩니다.
📌 PR 내용
🔍 PR 세부사항
애플 로그인, 회원가입 추가
앱 심사를 위해 애플 로그인으로 회원가입시 바로 가입 승인 처리
모바일 로그인을 위한 custom grant 설정
애플 로그인의 경우는 기본적인 OIDC 프로토콜을 따르지만, 약간 특이한 부분은 인증 서버로 /oauth/token 요청을 보낼 때 client_secret을 정적 키로 넣어주는게 아니라 발급받은 key를 jwt로 wrapping해서 보내준다는 특이점이 있습니다 (generateClientSecret 참고)
또 OIDC 프로토콜을 사용하기 때문에 id_token에 유저 정보 (sub: apple user id)를 담아줍니다. 따라서 애플 인증 서버에서 제공하는 공개키를 가져와서 디코딩해 유저 정보를 추출 -> 디비에서 유저를 찾아서 로그인 / 회원가입 / 연동 처리
📸 관련 스크린샷
📝 주의사항
✅ 체크리스트
Summary by CodeRabbit