[배포]: 카카오 중복 판단 로직 추가 및 로그 제거 #351
Conversation
MOSU-308 feat: 본인인증 회원가입 실패시 에러 로직 분기
Refactor/mosu 348 비밀번호 정규식 수정, admin application NPE 해결
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ 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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
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은 사용자 등록 및 인증 관련 로직을 개선하고 시스템 로깅을 최적화하는 데 중점을 둡니다. 특히, 카카오 계정의 중복 가입을 보다 정교하게 처리하는 로직이 추가되었으며, 불필요한 로그를 제거하여 시스템의 가독성과 성능을 향상시켰습니다. 또한, 비밀번호 유효성 검사 패턴이 업데이트되었고, 토큰 관련 도메인 모델이 재구성되어 코드의 일관성과 유지보수성이 강화되었습니다. 마지막으로, 관리자 애플리케이션 조회 쿼리가 개선되어 데이터 정확성과 효율성이 증대되었습니다.
Highlights
- 카카오 중복 가입 로직 강화: 기존 전화번호 기반 사용자 존재 여부 확인 로직에 카카오 계정의 PENDING 상태를 고려한 중복 판단 로직이 추가되었습니다. 이를 통해 카카오를 통한 가입 시 이미 PENDING 상태인 계정이 존재할 경우 KAKAO_DUPLICATED 오류를 발생시켜 중복 가입을 방지합니다.
- 불필요한 로그 제거: SignUpService, NotifyService, OAuthUserService, RefundQueryRepositoryImpl 등 여러 서비스에서 개발 및 디버깅 목적으로 사용되던 과도한 log.info 및 log.warn 구문이 제거되어 운영 환경에서의 로그 노이즈를 줄이고 성능을 개선했습니다.
- 비밀번호 패턴 특수문자 업데이트: 비밀번호 유효성 검사에 사용되는 정규 표현식의 허용 특수문자 집합이 [@$!%?&]에서 [!@#$%^&_/+=]로 변경되어 비밀번호 보안 요구사항을 업데이트했습니다.
- 토큰 도메인 모듈 리팩토링: domain.auth.signup 패키지에 있던 Token, SignUpTokenRedisEntity, SignUpTokenRedisRepository, TokenRepository 등의 클래스들이 domain.auth.token 패키지로 이동 및 이름이 변경되었습니다. 이는 토큰 관련 도메인 모델의 명확성과 일관성을 높이기 위한 구조적 개선입니다.
- 관리자 애플리케이션 조회 쿼리 개선: ApplicationQueryRepositoryImpl에서 애플리케이션 조회 시 profile.userName 대신 user.name을 사용하도록 변경하여 사용자 이름 조회 로직을 일관되게 만들었습니다. 또한, 결제 상태(PaymentStatus.DONE, PaymentStatus.ABORTED)에 따라 애플리케이션을 필터링하는 조건이 추가되었고, DTO 매핑 시 null 값에 대한 안전한 처리가 적용되었습니다.
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
안녕하세요. 코드 리뷰를 맡게 되어 기쁩니다. 이번 PR은 카카오 계정 중복 확인 로직 추가, 로그 제거, 비밀번호 정책 변경 등 중요한 개선 사항을 포함하고 있네요. 전체적으로 코드의 목적이 명확하고 구조가 잘 정리되어 있습니다.
몇 가지 추가 개선을 위해 아래에 리뷰 의견을 남겼습니다.
SignUpAccountStepProcessor의 사용자 중복 검증 로직이 일부 케이스를 놓칠 수 있는 부분을 발견했습니다. 이로 인해 예기치 않은 오류가 발생할 수 있어 수정이 필요해 보입니다.OAuthUserPersistenceProcessor의switch문에default케이스를 추가하여 코드의 안정성을 높이는 것을 제안합니다.TokenService의 Javadoc이 실제 코드와 일치하지 않는 부분을 수정하여 코드의 가독성을 높일 수 있습니다.
자세한 내용은 각 파일의 주석을 확인해주세요. 감사합니다!
| userRepository.findByPhoneNumber(user.getOriginPhoneNumber()) | ||
| .ifPresent(existingUser -> { | ||
| if (existingUser.isPendingUser()) { | ||
| switch (existingUser.getProvider()) { | ||
| case MOSU: | ||
| throw new CustomRuntimeException(ErrorCode.USER_ALREADY_EXISTS); | ||
| case KAKAO: | ||
| throw new CustomRuntimeException(ErrorCode.KAKAO_DUPLICATED); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
validateForNewSignUp 메서드의 중복 사용자 검증 로직이 모든 케이스를 처리하지 못하고 있습니다. 현재 코드는 PENDING 상태의 사용자에 대해서만 중복을 검사하고, 이미 가입이 완료된 사용자에 대해서는 아무런 처리를 하지 않아 DataIntegrityViolationException이 발생할 수 있습니다.
또한, switch 문에 default 케이스가 없어 새로운 AuthProvider가 추가될 경우를 대비하지 못하고 있습니다.
아래와 같이 수정하여 모든 중복 케이스를 처리하고 코드를 단순화하는 것을 제안합니다. 이 로직은 카카오 중복 가입 시도에 대해 특정 에러를 발생시키고, 그 외 모든 중복 상황에서는 일반적인 "사용자 존재" 에러를 발생시킵니다.
userRepository.findByPhoneNumber(user.getOriginPhoneNumber())
.ifPresent(existingUser -> {
if (existingUser.isPendingUser() && existingUser.getProvider() == life.mosu.mosuserver.domain.user.entity.AuthProvider.KAKAO) {
throw new CustomRuntimeException(ErrorCode.KAKAO_DUPLICATED);
}
throw new CustomRuntimeException(ErrorCode.USER_ALREADY_EXISTS);
});| switch (existingUser.getProvider()) { | ||
| case MOSU: | ||
| if (existingUser.isPendingUser()) { | ||
| throw new OAuthException("DUPLICATE"); | ||
| } | ||
| break; | ||
| case KAKAO: | ||
| if (existingUser.isPendingUser()) { | ||
| throw new OAuthException("KAKAO_DUPLICATE"); | ||
| } | ||
| break; | ||
| } |
There was a problem hiding this comment.
switch (existingUser.getProvider()) 문에 default 케이스가 없어 새로운 AuthProvider가 추가될 경우를 대비하지 못하고 있습니다. 새로운 Provider가 추가되면 아무런 검증 없이 updateOAuthUser가 호출될 수 있습니다.
향후 확장성을 고려하여 default 케이스를 추가하는 것이 좋습니다. 예를 들어, 지원하지 않는 Provider에 대해 예외를 발생시키거나 기본 동작을 정의할 수 있습니다.
switch (existingUser.getProvider()) {
case MOSU:
if (existingUser.isPendingUser()) {
throw new OAuthException("DUPLICATE");
}
break;
case KAKAO:
if (existingUser.isPendingUser()) {
throw new OAuthException("KAKAO_DUPLICATE");
}
break;
default:
// 새로운 Provider에 대한 처리를 명시적으로 하거나, 아무것도 하지 않음을 주석으로 남기는 것이 좋습니다.
break;
}|
|
||
| /** | ||
| * 회원가입 토큰을 검증하고, 유효하면 SignUpToken 객체를 반환합니다. | ||
| * 회원가입 토큰을 검증하고, 유효하면 SignUpInfo 객체를 반환합니다. |
There was a problem hiding this comment.
validateToken 메서드의 Javadoc 설명이 실제 동작과 다릅니다. 메서드는 void를 반환하지만, Javadoc에는 "SignUpInfo 객체를 반환합니다"라고 명시되어 있습니다.
메서드의 실제 동작(토큰 유효성 검증)에 맞게 Javadoc을 수정하는 것이 좋겠습니다.
| * 회원가입 토큰을 검증하고, 유효하면 SignUpInfo 객체를 반환합니다. | |
| * 회원가입 토큰의 유효성을 검증합니다. |
wlgns12370
changed the title
Co-authored-by: 장보형 <jbh010204@naver.com>
Co-authored-by: 장보형 <jbh010204@naver.com>
Co-authored-by: 장보형 <jbh010204@naver.com>"
Co-authored-by: 장보형 <jbh010204@naver.com>
|
/gemini review |
...n/java/life/mosu/mosuserver/infra/notify/dto/discord/DiscordExceptionNotifyEventRequest.java
Show resolved
Hide resolved
| userRepository.findByPhoneNumber(user.getOriginPhoneNumber()) | ||
| .ifPresent(existingUser -> { | ||
| if (existingUser.isPendingUser()) { | ||
| switch (existingUser.getProvider()) { | ||
| case MOSU: | ||
| throw new CustomRuntimeException(ErrorCode.USER_ALREADY_EXISTS); | ||
| case KAKAO: | ||
| throw new CustomRuntimeException(ErrorCode.KAKAO_DUPLICATED); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
회원가입 시 사용자를 검증하는 로직에 잠재적인 문제가 있습니다. 현재 코드는 전화번호로 사용자를 조회한 후, 해당 사용자가 isPendingUser()일 경우에만 중복 여부를 판단합니다. 만약 이미 가입이 완료된 사용자가 동일한 전화번호로 다시 회원가입을 시도할 경우, isPendingUser()가 false를 반환하여 아무런 예외도 발생시키지 않고 넘어가게 됩니다. 이 경우 이후 userRepository.save(user) 호출 시 데이터베이스의 유니크 제약 조건 위반으로 인해 처리되지 않은 예외가 발생하여 사용자에게 좋지 않은 경험을 줄 수 있습니다.
기존 사용자가 PENDING 상태가 아니더라도 이미 존재하는 사용자이므로, USER_ALREADY_EXISTS 예외를 발생시켜야 합니다.
userRepository.findByPhoneNumber(user.getOriginPhoneNumber())
.ifPresent(existingUser -> {
if (existingUser.isPendingUser()) {
switch (existingUser.getProvider()) {
case KAKAO:
throw new CustomRuntimeException(ErrorCode.KAKAO_DUPLICATED);
case MOSU:
default:
throw new CustomRuntimeException(ErrorCode.USER_ALREADY_EXISTS);
}
}
// 사용자가 존재하지만 PENDING 상태가 아닌 경우에도 중복으로 처리합니다.
throw new CustomRuntimeException(ErrorCode.USER_ALREADY_EXISTS);
});|
|
||
| /** | ||
| * 회원가입 토큰을 검증하고, 유효하면 SignUpToken 객체를 반환합니다. | ||
| * 회원가입 토큰을 검증하고, 유효하면 SignUpInfo 객체를 반환합니다. |
There was a problem hiding this comment.
메서드에 대한 Javadoc 설명이 실제 동작과 일치하지 않습니다. 이 메서드는 토큰의 유효성을 검증할 뿐, SignUpInfo 객체를 반환하지 않습니다. 부정확한 문서는 다른 개발자에게 혼란을 줄 수 있으므로, 메서드의 실제 기능을 정확하게 설명하도록 수정하는 것이 좋습니다.
| * 회원가입 토큰을 검증하고, 유효하면 SignUpInfo 객체를 반환합니다. | |
| * 회원가입 토큰을 검증합니다. |
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타