[RELEASE] V2.1.7 동일 이메일의 경우 로그인 통합 구현#340
Conversation
chore: Login V2관련 조건문과 주석 변경
Co-authored-by: 나용준 <141994188+youngJun99@users.noreply.github.com>
Summary of ChangesHello @yongjun0511, 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! 이 Pull Request는 동일한 이메일 주소를 가진 사용자 계정의 로그인 경험을 통합하고 개선하는 데 중점을 둡니다. 특히, OAuth 로그인 흐름에서 기존 이메일 기반 계정을 우선적으로 연결하도록 하여 계정 중복 생성을 방지하고, 사용자 기기 타입 업데이트 로직을 간소화했습니다. 또한, 개발 환경에서의 JWT 토큰 유효 기간을 조정하여 개발 및 테스트 편의성을 높였습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR implements the functionality to integrate multiple social logins using the same email. However, the current implementation introduces critical security flaws: the Kakao login flow lacks server-side token verification, allowing for account squatting on any email address, and the authentication logic in CustomUserDetailsService can lead to a denial of service for users with multiple provider accounts. Beyond these security concerns, a severe functional bug exists where authentication fails when attempting to access an existing account with a new social login. Additionally, there is code duplication across several service methods that requires refactoring, and some comments do not adhere to the style guide.
| .orElseGet(() -> userRepository.findFirstByEmailOrderByIdAsc(request.email()) | ||
| .orElseGet(() -> userService.join(request.email(), KAKAO, request.providerId()))); |
There was a problem hiding this comment.
The kakaoLogin and kakaoLoginV2 methods accept email and providerId directly from the client request without server-side verification. This trusts unvalidated user input, allowing an attacker to 'squat' on any unassigned email address by providing an existing user's email, preventing legitimate owners from using the service with a different provider. All OAuth-based logins must verify the provider's token on the backend to retrieve a verified email and provider ID. Additionally, a functional bug exists: when attempting a different social login with the same email, authentication fails. Even if a user is found with findFirstByEmailOrderByIdAsc, their credentials are stored with previous social login information. generateOauthJwtTokens then attempts authentication with new social login info, causing a BadCredentialsException. To resolve this, when a user is found by email, the logic needs to update their provider, providerId, and credentials with the new social login information. This issue affects kakaoLogin, kakaoLoginV2, appleLogin, and appleLoginV2 methods.
| @Override | ||
| public UserDetails loadUserByUsername(String username) { | ||
| User user = userRepository.findByEmail(username) | ||
| User user = userRepository.findFirstByEmailOrderByIdAsc(username) |
There was a problem hiding this comment.
The change to return the first user found by email (findFirstByEmailOrderByIdAsc) breaks authentication for users who have multiple accounts with the same email but different providers (e.g., one Apple account and one Kakao account). When such a user attempts to log in with their newer account, Spring Security's AuthenticationManager will retrieve the oldest account's details and attempt to verify the current provider's credentials against the oldest account's stored credentials (which are provider-specific, e.g., APPLE + appleId vs KAKAO + kakaoId). This will result in a BadCredentialsException, effectively locking users out of all but their oldest account. A proper integration should either allow multiple credentials per user or handle provider-specific lookups during authentication.
|
|
||
| private final UserRepository userRepository; | ||
|
|
||
| // OrderByIdAsc로 해줌으로써 처음 생성된 계정으로 연결된다. |
There was a problem hiding this comment.
리포지토리 스타일 가이드 규칙 #8에 따라 주석에 작성자 이름과 날짜를 포함해야 합니다.
References
- Each comment must include the author's name and the date. (link)
| User user = userRepository.findByProviderId(request.providerId()) | ||
| .orElseGet(() -> userService.joinV2(request.email(), KAKAO, request.providerId(),request.deviceType())); | ||
| .orElseGet(() -> userRepository.findFirstByEmailOrderByIdAsc(request.email()) | ||
| .orElseGet(() -> userService.joinV2(request.email(), KAKAO, request.providerId(),request.deviceType()))); |
There was a problem hiding this comment.
사용자 조회 및 가입 로직이 kakaoLogin, kakaoLoginV2, appleLogin, appleLoginV2 네 개의 메소드에 걸쳐 중복되고 있습니다. 이 로직을 별도의 private 메소드로 추출하여 코드 중복을 줄이고 유지보수성을 향상시키는 것을 고려해보세요.
| } | ||
|
|
||
| // 회원 가입 v2 마이그레이션을 위한 메서드 (원래는 가입시 DeviceType을 받아야 합니다.) | ||
| // 추후에 기종이 바뀌더라도 업데이트합니다. + V1-> V2 마이그레이션을 수행하는 역할을 하기도 합니다. |
There was a problem hiding this comment.
리포지토리 스타일 가이드 규칙 #8에 따라 주석에 작성자 이름과 날짜를 포함해야 합니다.
References
- Each comment must include the author's name and the date. (link)
2 participants
No description provided.