MOSU-341 refactor: 카카오 회원 중복 시 에러 처리#342
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughImplements duplicate OAuth user detection with a new OAuthException, expands user update with phone, birth, and marketing flags, and revises OAuth2 login failure handling to emit OAuth-specific error responses using OAuthErrorType and OAuthFailureResponse. Removes an unused LoginResponse factory. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant OAuthProvider as OAuth Provider
participant Server as Mosu Server
User->>App: Start OAuth login
App->>OAuthProvider: Redirect to authorize
OAuthProvider-->>App: Redirect back (error or code)
App->>Server: /oauth2/callback (error or code)
alt Error present
Server->>Server: OAuth2LoginFailureHandler.onAuthenticationFailure()
Server->>Server: Map message -> OAuthErrorType
Server->>App: Redirect with data=OAuthFailureResponse(JSON)
else Code present
Server->>Server: Persist/Update OAuth user
alt Existing Mosu user detected as duplicate
Server->>Server: throw OAuthException("DUPLICATE")
Server->>App: Redirect with data=OAuthFailureResponse("DUPLICATE")
else New or updatable user
Server-->>App: Proceed with normal success flow
end
end
sequenceDiagram
participant Processor as OAuthUserPersistenceProcessor
participant Repo as User Repository
participant Entity as UserJpaEntity
Processor->>Repo: findByProviderId(...)
alt Existing user
Processor->>Entity: updateOAuthUser(gender, name, phone, birth, marketing)
else No user
Processor->>Repo: save(new OAuth user)
end
alt existingUser.isMosuUser()
Processor->>Processor: throw OAuthException("DUPLICATE")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Summary of Changes
Hello @wlgns12370, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
이 PR은 OAuth 인증 과정에서 발생하는 오류 처리를 리팩토링하여, 특히 카카오 회원 중복 가입 시 발생하는 오류를 명확하게 처리하고 OAuth 관련 오류 응답을 표준화합니다. 이를 통해 사용자에게 더 구체적인 오류 정보를 제공하고, 오류 처리 로직의 일관성을 높입니다.
Highlights
- 카카오 회원 중복 에러 처리: 기존에 주석 처리되어 있던 카카오 회원 중복 가입 로직을 활성화하고, IllegalArgumentException 대신 새로 정의된 OAuthException을 사용하여 'DUPLICATE' 오류를 발생시키도록 변경했습니다.
- OAuth 전용 예외 도입: OAuth 인증 과정에서 발생하는 특정 오류를 처리하기 위해 AuthenticationException을 상속받는 OAuthException 클래스를 새로 추가했습니다.
- OAuth 오류 타입 및 응답 표준화: OAuth 오류를 'CANCELED', 'DUPLICATE', 'UNKNOWN' 등으로 분류하는 OAuthErrorType 열거형과, 표준화된 오류 응답 형식을 제공하는 OAuthFailureResponse 레코드를 도입하여 오류 처리의 일관성을 확보했습니다.
- OAuth 로그인 실패 핸들러 개선: OAuth2LoginFailureHandler에서 기존의 일반적인 LoginResponse 대신 새로 정의된 OAuthErrorType과 OAuthFailureResponse를 사용하여 로그인 실패 시 더 구체적인 오류 정보를 클라이언트에 전달하도록 로직을 수정했습니다.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
안녕하세요. 카카오 회원 중복 및 OAuth 관련 에러 처리 리팩토링 PR 잘 보았습니다.
전반적으로 OAuth 인증 실패 시 에러를 공통으로 처리하기 위한 구조를 잘 설계하셨습니다. OAuthException, OAuthErrorType, OAuthFailureResponse를 도입하여 에러 처리 로직을 명확하게 분리한 점이 인상적입니다.
다만, 코드의 안정성과 유지보수성을 높이기 위해 몇 가지 개선점을 제안합니다.
주요 제안 내용은 다음과 같습니다:
OAuth2LoginFailureHandler에서OAuth2AuthenticationException의 에러 코드를 직접 사용하여, 외부 OAuth 제공자의 에러 메시지 변경에 대한 의존성을 제거하는 것을 제안합니다.- 하드코딩된 에러 메시지 문자열을
OAuthErrorTypeenum으로 대체하여 타입 안정성을 확보하고 코드 일관성을 높이는 방안을 제안합니다.
자세한 내용은 각 파일에 남긴 리뷰 코멘트를 참고해주세요.
| OAuthErrorType errorType = OAuthErrorType.from(exception.getMessage()); | ||
| OAuthFailureResponse oAuthFailureResponse = OAuthFailureResponse.from( | ||
| errorType.getMessage()); |
There was a problem hiding this comment.
exception.getMessage()를 직접 사용하여 에러 유형을 확인하는 것은 OAuth 제공자의 에러 메시지 변경에 취약할 수 있습니다.
AuthenticationException이 OAuth2AuthenticationException의 인스턴스인 경우, getError().getErrorCode()를 통해 보다 안정적으로 에러 코드를 추출할 수 있습니다. 이렇게 하면 "access_denied"와 같은 표준 에러 코드를 사용하여 OAuthErrorType에서 분기 처리를 할 수 있어 코드가 더 견고해집니다.
이 변경을 적용하려면 org.springframework.security.oauth2.core.OAuth2AuthenticationException를 import해야 합니다.
OAuthErrorType.java의 관련 수정 제안도 함께 확인해주세요.
String errorCode = exception.getMessage();
if (exception instanceof org.springframework.security.oauth2.core.OAuth2AuthenticationException e) {
errorCode = e.getError().getErrorCode();
}
OAuthErrorType errorType = OAuthErrorType.from(errorCode);
OAuthFailureResponse oAuthFailureResponse = OAuthFailureResponse.from(
errorType.getMessage());| return switch (text) { | ||
| case "DUPLICATE" -> DUPLICATE; | ||
| case "[access_denied] User denied access" -> CANCELED; | ||
| default -> UNKNOWN; | ||
| }; |
There was a problem hiding this comment.
에러 메시지 문자열 "[access_denied] User denied access"에 직접 의존하는 것은 좋지 않은 방법입니다. OAuth 제공자가 에러 메시지 형식을 변경하면 코드가 오작동할 수 있습니다.
OAuth2LoginFailureHandler에서 exception 객체가 OAuth2AuthenticationException의 인스턴스인지 확인하고, 그렇다면 getError().getErrorCode()를 사용하여 "access_denied"와 같은 표준 오류 코드를 가져오는 것이 더 안정적입니다.
관련하여 OAuth2LoginFailureHandler.java 파일에도 수정 제안을 남겼습니다. 이 제안과 함께 적용하는 것을 권장합니다.
아래와 같이 수정할 수 있습니다.
| return switch (text) { | |
| case "DUPLICATE" -> DUPLICATE; | |
| case "[access_denied] User denied access" -> CANCELED; | |
| default -> UNKNOWN; | |
| }; | |
| return switch (text) { | |
| case "DUPLICATE" -> DUPLICATE; | |
| case "access_denied" -> CANCELED; | |
| default -> UNKNOWN; | |
| }; |
| // throw new IllegalArgumentException("이미 모수 회원입니다."); | ||
| // } | ||
| if (existingUser.isMosuUser()) { | ||
| throw new OAuthException("DUPLICATE"); |
There was a problem hiding this comment.
에러 메시지로 하드코딩된 문자열 "DUPLICATE"을 사용하는 대신, OAuthErrorType enum을 직접 사용하여 타입 안정성을 높이고 유지보수를 용이하게 하는 것이 좋습니다. OAuthErrorType.DUPLICATE.getMessage()를 사용하면, 향후 에러 코드 문자열이 변경되더라도 한 곳(OAuthErrorType enum)에서만 수정하면 되므로 코드 관리가 더 수월해집니다. 이 변경을 적용하려면 life.mosu.mosuserver.global.handler.OAuthErrorType를 import해야 합니다.
| throw new OAuthException("DUPLICATE"); | |
| throw new OAuthException(OAuthErrorType.DUPLICATE.getMessage()); |
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Bug Fixes