-
Notifications
You must be signed in to change notification settings - Fork 0
Feat: [FN-76][FN-77] 소셜 연동 #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. """ Walkthrough이 변경 사항은 OAuth2 소셜 계정 연동 기능을 대규모로 도입합니다. 주요 내용에는 OAuth2 인증/콜백 컨트롤러, 서비스, 레디스 토큰 저장소, 사용자-소셜 연동 엔티티 및 JPA 저장소, PKCE 및 쿠키 유틸리티, 관련 예외 및 설정 클래스 추가와 기존 설정의 리팩토링이 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OAuthController
participant OAuthService
participant OAuthApiClient
participant SocialLinkTokenRedisRepository
participant UserOAuthLinkRepository
Client->>OAuthController: GET /oauth2/authorization/{provider}
OAuthController->>OAuthService: getAuthorizationUri(provider, req, userId)
OAuthService->>OAuthApiClient: buildAuthorizeUri(...)
OAuthService->>SocialLinkTokenRedisRepository: saveToken(userId, stateToken)
OAuthService-->>OAuthController: AuthorizationRedirect(URI, Cookie)
OAuthController-->>Client: 302 Redirect + Set-Cookie
Client->>OAuthController: GET /oauth2/callback/{provider}?code=...&state=...
OAuthController->>OAuthService: linkSocialAccount(provider, code, state, codeVerifier, req)
OAuthService->>SocialLinkTokenRedisRepository: findUserIdByToken(state)
OAuthService->>OAuthApiClient: requestAccessToken(...)
OAuthService->>OAuthApiClient: requestUserInfo(...)
OAuthService->>UserOAuthLinkRepository: existsByUser_IdAndProviderId(userId, providerId)
alt 이미 연동된 계정
OAuthService-->>OAuthController: throw OAuthConflictException
else 신규 연동
OAuthService->>UserOAuthLinkRepository: save(UserOAuthLink)
OAuthService-->>OAuthController: 완료
end
OAuthController-->>Client: 302 Redirect (성공/실패/충돌)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (11)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
🧹 Nitpick comments (15)
src/main/java/project/flipnote/auth/exception/OAuthException.java (1)
3-4: OAuth 예외 계층 구조의 기본 클래스가 적절합니다.추상 클래스 설계가 OAuth 관련 예외들의 계층 구조를 만들기에 적합합니다.
예외 메시지 전달을 위한 생성자 추가를 고려해보세요:
public abstract class OAuthException extends RuntimeException { + + protected OAuthException() { + super(); + } + + protected OAuthException(String message) { + super(message); + } + + protected OAuthException(String message, Throwable cause) { + super(message, cause); + } }src/main/java/project/flipnote/auth/exception/OAuthConflictException.java (1)
3-4: OAuth 충돌 예외 클래스가 적절하게 구현되었습니다.
OAuthException을 상속받아 OAuth 충돌 시나리오를 나타내는 구체적인 예외 클래스입니다. 예외 계층 구조와 타입 구분을 위한 설계가 적절합니다.기본 클래스와 마찬가지로 생성자 추가를 고려해보세요:
public class OAuthConflictException extends OAuthException { + + public OAuthConflictException() { + super(); + } + + public OAuthConflictException(String message) { + super(message); + } + + public OAuthConflictException(String message, Throwable cause) { + super(message, cause); + } }src/main/java/project/flipnote/infra/oauth/model/OAuth2UserInfo.java (1)
3-8: OAuth2 사용자 정보를 위한 인터페이스가 잘 설계되었습니다.다양한 OAuth 제공자를 지원할 수 있는 깔끔한 추상화입니다. 그러나 메서드 문서화와 null 안전성을 개선할 수 있습니다.
+import org.springframework.lang.Nullable; + +/** + * OAuth2 제공자로부터 받은 사용자 정보에 대한 공통 인터페이스 + */ public interface OAuth2UserInfo { + /** + * OAuth 제공자에서의 사용자 고유 ID + */ String getProviderId(); + + /** + * OAuth 제공자명 (예: "google", "facebook") + */ String getProvider(); + + /** + * 사용자 이메일 주소 + */ + @Nullable String getEmail(); + + /** + * 사용자 이름 + */ + @Nullable String getName(); }src/main/java/project/flipnote/common/config/AppConfig.java (2)
14-17: RestClient 빈 설정 개선을 고려해보세요.기본
RestClient.create()를 사용하고 있는데, OAuth2 통신을 위해서는 타임아웃, 에러 핸들링, 로깅 등의 설정이 필요할 수 있습니다.다음과 같이 개선할 수 있습니다:
@Bean public RestClient restClient() { - return RestClient.create(); + return RestClient.builder() + .requestFactory(new SimpleClientHttpRequestFactory()) + .defaultHeader("User-Agent", "FlipNote-OAuth-Client") + .build(); }
19-22: ObjectMapper 설정 커스터마이징을 고려해보세요.OAuth2 API 응답 처리를 위해 ObjectMapper에 적절한 설정이 필요할 수 있습니다.
다음과 같은 설정을 고려해보세요:
@Bean public ObjectMapper objectMapper() { - return new ObjectMapper(); + ObjectMapper mapper = new ObjectMapper(); + mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); + mapper.configure(JsonParser.Feature.ALLOW_UNQUOTED_FIELD_NAMES, true); + return mapper; }src/main/java/project/flipnote/user/entity/UserOAuthLink.java (1)
35-39: 데이터 무결성을 위한 추가 제약조건을 고려해보세요.provider와 providerId 조합에 대한 유니크 제약조건 추가를 고려해보시기 바랍니다.
@Table( name = "user_oauth_link", indexes = { @Index(name = "idx_provider_provider_id", columnList = "provider, providerId") - } + }, + uniqueConstraints = { + @UniqueConstraint(name = "uk_provider_provider_id", columnNames = {"provider", "providerId"}) + } )src/main/java/project/flipnote/auth/repository/SocialLinkTokenRedisRepository.java (2)
18-23: Redis 연결 실패에 대한 예외 처리를 추가해보세요.Redis 연결 실패나 타임아웃 상황에 대한 예외 처리가 없어 OAuth 플로우가 중단될 수 있습니다.
public void saveToken(long userId, String token) { String key = AuthRedisKey.SOCIAL_LINK_TOKEN.key(token); Duration ttl = AuthRedisKey.SOCIAL_LINK_TOKEN.getTtl(); - - stringRedisTemplate.opsForValue().set(key, String.valueOf(userId), ttl); + + try { + stringRedisTemplate.opsForValue().set(key, String.valueOf(userId), ttl); + } catch (Exception e) { + throw new RuntimeException("Failed to save social link token", e); + } }
25-32: 토큰 조회 시 예외 처리를 개선해보세요.
Long.parseLong()에서 NumberFormatException이 발생할 수 있습니다.public Optional<Long> findUserIdByToken(String token) { String key = AuthRedisKey.SOCIAL_LINK_TOKEN.key(token); String userId = stringRedisTemplate.opsForValue().get(key); return Optional.ofNullable(userId) - .map(Long::parseLong); + .map(value -> { + try { + return Long.parseLong(value); + } catch (NumberFormatException e) { + return null; + } + }) + .filter(Objects::nonNull); }src/main/java/project/flipnote/common/util/PkceUtil.java (2)
14-14: 정적 SecureRandom 인스턴스의 스레드 안전성을 검토해보세요.정적
SecureRandom인스턴스는 멀티스레드 환경에서 동시 접근 시 성능 저하가 발생할 수 있습니다.다음과 같이 ThreadLocal 또는 인스턴스 변수로 개선할 수 있습니다:
-private static SecureRandom random = new SecureRandom(); +private final SecureRandom random = new SecureRandom();또는 ThreadLocal 사용:
-private static SecureRandom random = new SecureRandom(); +private static final ThreadLocal<SecureRandom> random = + ThreadLocal.withInitial(SecureRandom::new);그리고 사용부에서:
-random.nextBytes(codeVerifier); +random.get().nextBytes(codeVerifier);
25-25: 문자 인코딩을 검토해보세요.RFC 7636에서는 코드 검증자를 ASCII로 인코딩하도록 권장하지만, UTF-8 사용을 고려해볼 수 있습니다.
-byte[] hashed = digest.digest(codeVerifier.getBytes(StandardCharsets.US_ASCII)); +byte[] hashed = digest.digest(codeVerifier.getBytes(StandardCharsets.UTF_8));src/main/java/project/flipnote/common/config/OAuthProperties.java (1)
18-19: 중복된 Lombok 어노테이션 제거 필요
Provider클래스에 이미@Getter와@Setter가 선언되어 있으므로, 클래스 레벨의 중복된@Getter어노테이션을 제거하세요.-@Getter @Setter public static class Provider {src/main/java/project/flipnote/auth/controller/OAuthController.java (1)
63-66: 예외 로깅 시 민감 정보 노출 주의일반 Exception 처리 시 스택 트레이스에 민감한 정보가 포함될 수 있습니다.
} catch (Exception e) { redirectUri = clientProperties.buildUrl(ClientProperties.PathKey.SOCIAL_LINK_FAILURE); - log.error("소셜 계정 연동 콜백 처리 중 예상치 못한 오류 발생. provider: {}, state: {}", provider, state, e); + log.error("소셜 계정 연동 콜백 처리 중 예상치 못한 오류 발생. provider: {}", provider, e); }src/main/java/project/flipnote/infra/oauth/OAuthApiClient.java (1)
68-73: 프로바이더 확장성을 위한 팩토리 패턴 고려현재 switch 문은 새로운 프로바이더 추가 시 수정이 필요합니다.
Strategy 패턴이나 팩토리를 사용하여 확장성을 개선할 수 있습니다:
@Component public interface OAuth2UserInfoFactory { boolean supports(String provider); OAuth2UserInfo create(Map<String, Object> attributes); } // 각 프로바이더별 구현체를 별도로 생성 @Component public class GoogleUserInfoFactory implements OAuth2UserInfoFactory { public boolean supports(String provider) { return "google".equalsIgnoreCase(provider); } // ... }src/main/java/project/flipnote/auth/service/OAuthService.java (2)
81-87: 사용자 존재 여부 검증 추가 고려
getReferenceById는 실제 엔티티 조회 없이 프록시를 반환하므로, 존재하지 않는 사용자 ID의 경우 나중에 예외가 발생할 수 있습니다.+if (!userRepository.existsById(userId)) { + throw new OAuthFailException("사용자를 찾을 수 없습니다"); +} UserOAuthLink userOAuthLink = new UserOAuthLink( userInfo.getProvider(), userInfo.getProviderId(), userRepository.getReferenceById(userId) );
41-58: OAuth 인증 플로우 보안 강화 제안현재 구현은 기본적인 PKCE와 state 검증을 포함하고 있으나, 추가 보안 강화를 고려하세요:
- State 토큰에 대한 rate limiting 적용
- 동일 사용자의 동시 다중 OAuth 요청 제한
- State 토큰에 추가 컨텍스트(IP, User-Agent 등) 포함 고려
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
src/main/java/project/flipnote/auth/constants/AuthRedisKey.java(1 hunks)src/main/java/project/flipnote/auth/constants/OAuthConstants.java(1 hunks)src/main/java/project/flipnote/auth/controller/AuthController.java(4 hunks)src/main/java/project/flipnote/auth/controller/OAuthController.java(1 hunks)src/main/java/project/flipnote/auth/exception/OAuthConflictException.java(1 hunks)src/main/java/project/flipnote/auth/exception/OAuthException.java(1 hunks)src/main/java/project/flipnote/auth/exception/OAuthFailException.java(1 hunks)src/main/java/project/flipnote/auth/intecepter/OAuthCookieCleanupInterceptor.java(1 hunks)src/main/java/project/flipnote/auth/model/AuthorizationRedirect.java(1 hunks)src/main/java/project/flipnote/auth/repository/SocialLinkTokenRedisRepository.java(1 hunks)src/main/java/project/flipnote/auth/service/AuthService.java(1 hunks)src/main/java/project/flipnote/auth/service/OAuthService.java(1 hunks)src/main/java/project/flipnote/common/config/AppConfig.java(1 hunks)src/main/java/project/flipnote/common/config/AsyncProperties.java(0 hunks)src/main/java/project/flipnote/common/config/ClientProperties.java(2 hunks)src/main/java/project/flipnote/common/config/OAuthProperties.java(1 hunks)src/main/java/project/flipnote/common/config/WebConfig.java(1 hunks)src/main/java/project/flipnote/common/security/config/SecurityConfig.java(1 hunks)src/main/java/project/flipnote/common/util/CookieUtil.java(2 hunks)src/main/java/project/flipnote/common/util/PkceUtil.java(1 hunks)src/main/java/project/flipnote/infra/oauth/OAuthApiClient.java(1 hunks)src/main/java/project/flipnote/infra/oauth/model/GoogleUserInfo.java(1 hunks)src/main/java/project/flipnote/infra/oauth/model/OAuth2UserInfo.java(1 hunks)src/main/java/project/flipnote/user/entity/UserOAuthLink.java(1 hunks)src/main/java/project/flipnote/user/repository/UserOAuthLinkRepository.java(1 hunks)src/main/resources/application.yml(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/project/flipnote/common/config/AsyncProperties.java
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/project/flipnote/common/util/PkceUtil.java (1)
src/main/java/project/flipnote/common/util/CookieUtil.java (1)
Component(6-30)
src/main/java/project/flipnote/auth/service/OAuthService.java (6)
src/main/java/project/flipnote/auth/intecepter/OAuthCookieCleanupInterceptor.java (1)
RequiredArgsConstructor(14-30)src/main/java/project/flipnote/auth/repository/SocialLinkTokenRedisRepository.java (1)
RequiredArgsConstructor(12-37)src/main/java/project/flipnote/infra/oauth/OAuthApiClient.java (1)
RequiredArgsConstructor(23-98)src/main/java/project/flipnote/auth/controller/OAuthController.java (1)
Slf4j(26-72)src/main/java/project/flipnote/auth/exception/OAuthConflictException.java (1)
OAuthConflictException(3-4)src/main/java/project/flipnote/auth/exception/OAuthFailException.java (1)
OAuthFailException(3-4)
🔇 Additional comments (22)
src/main/java/project/flipnote/auth/service/AuthService.java (1)
107-107: buildUrl 메서드 및 PASSWORD_RESET PathKey 정상 확인됨
ClientProperties클래스에public String buildUrl(PathKey key, Object... uriVariables)메서드가 정의되어 있으며,
PathKeyenum에도PASSWORD_RESET항목이 존재합니다.
리팩토링된 URL 구성 방식이 의도한 대로 작동할 것으로 보입니다.src/main/java/project/flipnote/auth/constants/AuthRedisKey.java (1)
16-16: 소셜 링크 토큰 Redis 키 추가가 적절합니다.새로운
SOCIAL_LINK_TOKENenum 상수가 기존 패턴을 따르며 올바르게 추가되었습니다. 180초 TTL은 OAuth 플로우의 보안을 위해 적절한 짧은 유효기간입니다.src/main/java/project/flipnote/common/security/config/SecurityConfig.java (1)
68-70: OAuth2 콜백 엔드포인트 보안 설정이 올바르게 추가되었습니다.외부 OAuth 제공자로부터의 콜백을 받기 위해 인증 없이 접근할 수 있도록 허용하는 설정입니다. OAuth2 플로우에서 필수적인 설정이며, GET 메서드로 제한하고 provider 경로 변수를 사용하는 것이 적절합니다.
src/main/java/project/flipnote/auth/controller/AuthController.java (4)
34-34: 의존성 주입 패턴으로의 리팩토링이 잘 적용되었습니다.CookieUtil을 정적 클래스에서 Spring 관리 컴포넌트로 변경한 것은 좋은 개선사항입니다. 테스트 용이성과 유지보수성이 향상됩니다.
43-47: 쿠키 생성 로직이 올바르게 업데이트되었습니다.인스턴스 메서드 호출로 변경되었으며 기존 로직은 그대로 유지되어 안전합니다.
56-56: 로그아웃 시 쿠키 만료 처리가 올바릅니다.만료된 쿠키 생성 로직이 인스턴스 메서드로 적절히 변경되었습니다.
86-90: 토큰 갱신 시 쿠키 처리가 일관성 있게 구현되었습니다.다른 메서드들과 동일한 패턴으로 인스턴스 메서드를 사용하여 일관성이 유지됩니다.
src/main/java/project/flipnote/auth/model/AuthorizationRedirect.java (1)
5-9: OAuth 인가를 위한 데이터 전송 객체가 잘 설계되었습니다.Record 클래스를 사용한 불변 데이터 구조는 OAuth 인가 플로우의 DTO로 적합합니다. 필드명이 명확하고 타입이 적절합니다.
src/main/java/project/flipnote/user/repository/UserOAuthLinkRepository.java (1)
7-10: JPA 레포지토리가 올바르게 구현되었습니다.Spring Data JPA의 메서드 네이밍 규칙을 정확히 따르고 있으며, 사용자와 OAuth 제공자 간의 연결 존재 여부를 확인하는 목적이 명확합니다. 반환 타입과 매개변수 타입이 적절합니다.
src/main/java/project/flipnote/common/config/WebConfig.java (1)
16-20: 인터셉터 등록이 올바르게 구현되었습니다.OAuth 콜백 경로에만 적용되도록 경로 패턴이 적절히 설정되어 있고, Spring MVC 설정이 올바르게 구현되었습니다.
src/main/java/project/flipnote/infra/oauth/model/GoogleUserInfo.java (2)
23-25: null 안전성 검사를 추가해주세요.이메일 속성에 대한 null 체크가 필요합니다.
@Override public String getEmail() { - return (String) attributes.get("email"); + Object email = attributes.get("email"); + return email != null ? (String) email : null; }Likely an incorrect or invalid review comment.
13-15: null 안전성 검사를 추가해주세요.Google OAuth 응답에서 필수 속성이 누락될 경우를 대비한 null 체크가 필요합니다.
@Override public String getProviderId() { - return (String) attributes.get("sub"); + Object sub = attributes.get("sub"); + return sub != null ? (String) sub : null; }Likely an incorrect or invalid review comment.
src/main/java/project/flipnote/auth/intecepter/OAuthCookieCleanupInterceptor.java (1)
20-29: 인터셉터 구현이 올바르게 되어 있습니다.OAuth 콜백 완료 후 쿠키 정리 로직이 적절히 구현되어 있습니다.
src/main/java/project/flipnote/common/config/AppConfig.java (1)
10-10: 설정 프로퍼티 클래스 확인 완료
AsyncProperties, ClientProperties, OAuthProperties 클래스가 모두 존재하며 각각@ConfigurationProperties(prefix = "...")애노테이션이 올바르게 적용되어 있습니다. 별도 수정이 필요하지 않습니다.
- src/main/java/project/flipnote/common/config/AsyncProperties.java (
prefix="app.async")- src/main/java/project/flipnote/common/config/ClientProperties.java (
prefix="app.client")- src/main/java/project/flipnote/common/config/OAuthProperties.java (
prefix="app.oauth2")src/main/java/project/flipnote/user/entity/UserOAuthLink.java (1)
24-26: 인덱스 설계가 효율적입니다.
provider와providerId조합에 대한 복합 인덱스는 OAuth 제공업체별 사용자 조회에 최적화되어 있어 좋습니다.src/main/java/project/flipnote/common/util/PkceUtil.java (2)
16-20: PKCE 코드 검증자 생성이 RFC 7636 표준을 준수합니다.32바이트 길이의 안전한 랜덤 값을 생성하고 Base64 URL-safe 인코딩을 사용하는 것이 PKCE 표준에 맞습니다.
22-30: SHA-256 해시 알고리즘 사용이 적절합니다.PKCE 코드 챌린지 생성에 SHA-256을 사용하는 것은 보안상 안전하고 표준을 준수합니다. 예외 처리도 적절히 구현되어 있습니다.
src/main/java/project/flipnote/common/util/CookieUtil.java (2)
4-6: 정적 유틸리티에서 스프링 컴포넌트로의 리팩터링이 적절합니다.의존성 주입과 테스트 가능성 향상을 위한 좋은 변경입니다.
9-29: CookieUtil 인스턴스 메서드 호출 확인 완료
모든 정적 호출(CookieUtil.)은 제거되었으며,@Component로 등록된CookieUtil을 아래 클래스에서 인스턴스로 주입해 사용하고 있습니다. 추가 변경은 필요하지 않습니다.
- project/flipnote/auth/intecepter/OAuthCookieCleanupInterceptor.java
- project/flipnote/auth/service/OAuthService.java
- project/flipnote/auth/controller/AuthController.java
src/main/java/project/flipnote/common/config/ClientProperties.java (1)
17-37: 효과적인 리팩토링으로 확장성 향상다중 클라이언트 경로를 지원하도록 리팩토링이 잘 되었습니다:
EnumMap사용으로 타입 안전성과 성능 향상- 명확한 예외 처리로 설정 누락 시 즉시 감지 가능
- 가변 인자를 통한 유연한 URL 생성 지원
src/main/java/project/flipnote/auth/controller/OAuthController.java (1)
48-55: OAuth 콜백 엔드포인트 보안 검증 필요다음 항목을 꼭 확인 및 보완해 주세요:
@CookieValue(OAuthConstants.VERIFIER_COOKIE_NAME)에required = false속성 추가하여 쿠키가 없을 때 예외가 발생하지 않도록 처리- 서비스 레이어에서
state파라미터(CSRF 방지) 검증 로직이 실제로 구현되어 있는지 직접 점검참고:
ast-grep검색 결과 관련 검증 메서드를 찾지 못했습니다. 서비스 코드에서 수동으로 확인해 주세요.src/main/java/project/flipnote/infra/oauth/OAuthApiClient.java (1)
27-28: HTTP 클라이언트 타임아웃 설정 검증 필요OAuthApiClient에서 주입되는 RestClient가 외부 OAuth 프로바이더 호출 시 기본 타임아웃(무제한 또는 라이브러리 기본값)을 사용할 수 있습니다. RestClient 빈 설정을 확인하여 아래 사항을 검증해 주세요:
@Configuration클래스(예:RestClientConfiguration.java)에서
RestClient.builder()호출부에connectTimeout,readTimeout등을 명시적으로 설정했는지 확인src/main/resources/application.yml또는application.properties에서
connect-timeout,read-timeout,timeout등 관련 프로퍼티가 정의되어 있는지 검증필요 시 아래 스크립트로 전역 검색이 가능합니다:
# RestClient 빌더 및 타임아웃 설정 검색 rg -n "RestClient\\.builder" . rg -n "connectTimeout" . rg -n "readTimeout" . rg -g "*.yml" -n "timeout" src/main/resources rg -g "*.properties" -n "timeout" src/main/resources확인 후 적절한 타임아웃 값을 설정하거나 설정 위치를 문서화해 주세요.
📝 변경 내용
✅ 체크리스트
💬 기타 참고 사항
Summary by CodeRabbit
신규 기능
버그 수정
리팩터링 및 개선
설정