๐ Fix: ์์ ๋ก๊ทธ์ธ ์ด๋ฉ์ผ ์ค๋ณต ์ฒดํฌ ๋ก์ง ์ถ๊ฐ - #166
๐ Fix: ์์
๋ก๊ทธ์ธ ์ด๋ฉ์ผ ์ค๋ณต ์ฒดํฌ ๋ก์ง ์ถ๊ฐ#166jihwankim128 wants to merge 2 commits into
Conversation
* ๊ฐ์ ํฌ์ธํธ๊ฐ ๋ง์
WalkthroughGoogle/Kakao/Naver ์์ ๋ก๊ทธ์ธ ์ ๋ต์ UserValidator ์์กด์ฑ ๋ฐ ์ด๋ฉ์ผ ์ค๋ณต ๊ฒ์ฆ์ ์ถ๊ฐํ๊ณ , AuthService์๋ import ์ ๋ฆฌ์ TODO ์ฃผ์์ ์ถ๊ฐํ์ผ๋ฉฐ UserValidator์ ์ธ๋ผ์ธ ์ฃผ์์ ๋ณด๊ฐํ๊ณ UserErrorCode์ USER_DUPLICATE_EMAIL ๋ฉ์์ง๋ฅผ ์์ ํ์ต๋๋ค. Changes
Estimated code review effort๐ฏ 3 (Moderate) | โฑ๏ธ ~25 minutes Tip ๐ Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. โจ 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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and canโt be posted inline due to platform limitations.
โ ๏ธ Outside diff range comments (1)
insty-api/src/main/java/insty/domain/auth/strategy/KakaoStrategy.java (1)
57-67: ์ด๋ฉ์ผ ์ค๋ณต ๊ฒ์ฆ ์์น๊ฐ ์๋ชป๋์ด ๊ธฐ์กด ํ์ ๋ก๊ทธ์ธ ์คํจ ๊ฐ๋ฅ โ ๊ฐ์ ์ผ์ด์ค์์๋ง ๊ฒ์ฆํ๋๋ก ์ด๋ ํ์ํ์ฌ๋ ๋ก๊ทธ์ธ ํ๋ฆ ์ด๋ฐ์
validateDuplicateEmail(email)์ ํญ์ ํธ์ถํฉ๋๋ค. ์ด๋ฏธ ๊ฐ์ ๋ ์นด์นด์ค ์ฌ์ฉ์๊ฐ ์ฌ๋ก๊ทธ์ธํ๋ ๊ฒฝ์ฐ์๋ ์ด๋ฉ์ผ์ด DB์ ์กด์ฌํ๋ฏ๋ก ์ค๋ณต ์์ธ๊ฐ ๋ฐ์ํ ์ ์์ต๋๋ค. ์ค๋ณต ๊ฒ์ฌ๋ โ์ ๊ท ํ์ ์์ฑโ ๋ถ๊ธฐ(ํ์๊ฐ์ )์์๋ง ์ํํด์ผ ํฉ๋๋ค. ๋๋ค์ ์์ฑ๋ ๋์ผํ๊ฒ ๊ฐ์ ์์๋ง ์ํํ์ธ์.์๋์ ๊ฐ์ด ์์ ์ ์๋๋ฆฝ๋๋ค.
@@ public User loginBySocial(String code, UserType userType) { - String nickname = NicknameGenerator.generateNickname(); - - userValidator.validateDuplicateEmail(email); + // ์ด๋ฉ์ผ ์ค๋ณต ๊ฒ์ฌ๋ ์ ๊ท ๊ฐ์ ์ผ์ด์ค์์๋ง ์ํ log.info("์นด์นด์ค ๋ก๊ทธ์ธ : ์ฌ์ฉ์ ์ ๋ณด ์กฐํ ์๋ฃ , ์์ ID : {}", socialId); return userRepository.findBySocialIdAndSocialType(String.valueOf(socialId), SocialType.KAKAO) .orElseGet(() -> { // ์กด์ฌ X โ ํ์๊ฐ์ - User newUser = User.createBySocial(String.valueOf(socialId), SocialType.KAKAO, email, nickname, userType); + if (email != null && !email.isBlank()) { + userValidator.validateDuplicateEmail(email); + } + String nickname = NicknameGenerator.generateNickname(); + User newUser = User.createBySocial(String.valueOf(socialId), SocialType.KAKAO, email, nickname, userType); return userRepository.save(newUser); });
๐งน Nitpick comments (7)
insty-api/src/main/java/insty/domain/user/implement/UserValidator.java (1)
28-32: [์์ ๊ฐ์ ์ ์ฉ] ์ค๋ณต ์ฒดํฌ ์๋๋ฅผ ๋ฉ์๋ ์๊ทธ๋์ฒ/๋ค์ด๋ฐ์ผ๋ก ๋ช ํํํ์ฌ validateDuplicateEmail์ โ์กด์ฌํ๋ฉด ์์ธโ๋ง ์ํํฉ๋๋ค. ์์ ๋ก๊ทธ์ธ ํ๋ฆ์์ โ๊ธฐ์กด ์ฌ์ฉ์ ๋ก๊ทธ์ธโ ๋จ๊ณ์์ ์ด ๋ฉ์๋๋ฅผ ํธ์ถํ๋ฉด ์ ์ ์ฌ์ฉ์๋ ๋งํ๋ ์ค์ฉ ๋ฆฌ์คํฌ๊ฐ ํฝ๋๋ค(์ ๋ต ํ์ผ ์ฝ๋ฉํธ ์ฐธ๊ณ ). ์๋์ฒ๋ผ โ์ ๊ท ์์ฑ ์์๋งโ ํธ์ถํ๋๋ก ์๋๋ฅผ ๋๋ฌ๋ด๋ ๋ฉ์๋๋ก ๋ถ๋ฆฌ/๋ค์ด๋ฐํ๋ฉด ์ค์ฉ์ ์ค์ผ ์ ์์ต๋๋ค.
์:
- validateDuplicateEmailForCreate(String email)
- validateDuplicateEmailExcluding(String email, Long excludeUserId) // ์ํฉ์ ๋ฐ๋ผ
ํ์ํ์๋ค๋ฉด ๋ณ๊ฒฝ ํจ์น ์ ์ ๋๋ฆฌ๊ฒ ์ต๋๋ค.
insty-api/src/main/java/insty/domain/auth/service/AuthService.java (1)
127-128: ์์ ๋ก๊ทธ์ธ ์ฒ๋ฆฌ ์์ ์ฌ์ ๋ ฌ ์ ์: โ์กฐํ โ ์์ผ๋ฉด ๋ก๊ทธ์ธ โ ์์ผ๋ฉด ์ค๋ณต๊ฒ์ฌ ํ ์์ฑโํ์ฌ ์ ๋ต ๋ด๋ถ์์ โ์ด๋ฉ์ผ ์ค๋ณต ๊ฒ์ฌโ๋ฅผ ๋จผ์ ์ํํ๋ฉด ๊ธฐ์กด ์์ ์ฌ์ฉ์์ ์ ์ ๋ก๊ทธ์ธ๊น์ง ์ฐจ๋จ๋๋ ๋ฌธ์ ๊ฐ ์๊น๋๋ค. ๊ถ์ฅ ํ๋ก์ฐ:
- ์์ ํ ํฐ/ํ๋กํ ์กฐํ
- socialId+provider๋ก ์ฌ์ฉ์ ์กฐํ
- ์กด์ฌํ๋ฉด ๊ทธ๋๋ก ๋ก๊ทธ์ธ
- ์กด์ฌํ์ง ์์ผ๋ฉด โ์ ๊ท ์์ฑโ ์ง์ ์ ์ด๋ฉ์ผ ์ค๋ณต ๊ฒ์ฌ โ ์ค๋ณต ์ 409 ๋ฐํ or ๊ณ์ ์ฐ๊ฒฐ(migration) ํ๋ก์ฐ ์ ๋
๋ํ 4)์์ DB Unique ์ ์ฝ(์ด๋ฉ์ผ)์ ์์กดํด ๋์์ฑ(TOCTOU) ๋ฐฉ์ดํ๊ณ , ์๋ฐ ์ DataIntegrityViolationException์ ์ก์ ๋์ผ ์๋ฌ์ฝ๋๋ก ๋งคํํ๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.
insty-api/src/main/java/insty/domain/auth/strategy/KakaoStrategy.java (5)
5-5: NicknameGenerator import ์ถ๊ฐ OK์๋ ์ ์๋๋ก ๋๋ค์ ์์ฑ์ โ๊ฐ์ ์โ์๋ง ์ํํ๋ฉด ๋ ๊น๋ํด์ง๋๋ค.
55-56: ์ด๋ฉ์ผ null ๊ฐ๋ฅ์ฑ ๋ฐฉ์ด ์ฝ๋ ์ถ๊ฐ ๊ถ์ฅ์นด์นด์ค ๊ณ์ ์ ์ฝ๊ด ๋์ ์ฌ๋ถ์ ๋ฐ๋ผ ์ด๋ฉ์ผ์ด ์ ๊ณต๋์ง ์์ ์ ์์ต๋๋ค. NPE ๋ฐฉ์ง๋ฅผ ์ํด null-safe ํ ๋น์ ๊ถ์ฅํฉ๋๋ค(๊ฒ์ฆ ๋ก์ง์ ์ด๋ฏธ null ์ฒดํฌ ํ ํธ์ถ๋ก ๋ณด์ ์์ ).
- String email = userProfile.kakaoAccount().email(); // ์ด๋ฉ์ผ + String email = userProfile.kakaoAccount() != null ? userProfile.kakaoAccount().email() : null; // ์ด๋ฉ์ผ(๋ฏธ์ ๊ณต ๊ฐ๋ฅ์ฑ ๊ณ ๋ ค)์ด๋ฉ์ผ ๋ฏธ์ ๊ณต ์์ ์ ์ฑ (๊ฐ์ ํ์ฉ/์ฐจ๋จ, ๋์ฒด ์๋ณ์ ์ฌ์ฉ ๋ฑ)์ด ์ ํด์ ธ ์๋ค๋ฉด ๊ณต์ ๋ถํ๋๋ฆฝ๋๋ค. ์ ์ฑ ์ ๋ง์ถฐ ์์ธ ์ฒ๋ฆฌ๋ ๋ถ๊ธฐ ๋ก์ง๊น์ง ๋ฐ์ํด๋๋ฆฌ๊ฒ ์ต๋๋ค.
61-61: PII(์์ ID) ๋ก๊ทธ ๋ ธ์ถ ์ต์ํ ๊ถ์ฅINFO ๋ ๋ฒจ์ ์์ ์๋ณ์๋ฅผ ๊ทธ๋๋ก ๊ธฐ๋กํ๋ ๊ฒ์ ๊ณผ๋ํ ์ ์์ต๋๋ค. ๋๋ฒ๊ทธ๋ก ๋ด๋ฆฌ๊ฑฐ๋ ๋ง์คํน์ ๊ณ ๋ คํด์ฃผ์ธ์.
- log.info("์นด์นด์ค ๋ก๊ทธ์ธ : ์ฌ์ฉ์ ์ ๋ณด ์กฐํ ์๋ฃ , ์์ ID : {}", socialId); + log.debug("์นด์นด์ค ๋ก๊ทธ์ธ : ์ฌ์ฉ์ ์ ๋ณด ์กฐํ ์๋ฃ , ์์ ID : {}", socialId);
46-54: ํธ๋์ญ์ ๊ฒฝ๊ณ ์ต์ํ ๊ณ ๋ ค์ธ๋ถ API ํธ์ถ(ํ ํฐ/ํ๋กํ ์กฐํ)์ ํธ๋์ญ์ ๋ฐ๊นฅ์์ ์ํํ๊ณ , DB I/O๊ฐ ํ์ํ ๊ตฌ๊ฐ๋ง ํธ๋์ญ์ ์ผ๋ก ๊ฐ์ธ๋ ํธ์ด ํจ์จ์ ์ ๋๋ค. ํ์ฌ ์ํฅ์ ํฌ์ง ์์ง๋ง, ์ ์ฌ์ ์ธ ์ปค๋ฅ์ ์ ์ ์๊ฐ์ ์ค์ผ ์ ์์ต๋๋ค.
ํธ๋์ญ์ ์ค์ ์ ์ฒด ์ ์ฑ ์ ๋ฐ๋ผ ์ ์ง๊ฐ ํ์ํ๋ค๋ฉด ๊ทธ๋๋ก ๊ฐ์ ๋ ๋ฉ๋๋ค. ํ์ธ๋ง ๋ถํ๋๋ฆฝ๋๋ค.
57-67: ๊ฒฝํฉ ์ํฉ์์์ ์ต์ข ์ผ๊ด์ฑ ํ๋ณด(๊ณ ์ ์ธ๋ฑ์ค/์์ธ ์ ํ) ์ ์์ค๋ณต ๊ฒ์ฌ โ ์ ์ฅ ์ฌ์ด์ ๋ค๋ฅธ ํธ๋์ญ์ ์ด ๋์ผ ์ด๋ฉ์ผ๋ก ๊ฐ์ ํ ์ ์์ต๋๋ค. ์ด๋ฉ์ผ ์ปฌ๋ผ์ ์ ๋ํฌ ์ธ๋ฑ์ค๊ฐ ์๋ค๋ฉด DB๊ฐ ์ต์ข ๋ฐฉ์ด๋ฅผ ํด์ฃผ๊ณ , ์ด๋
DataIntegrityViolationException์ ์ก์ ๋๋ฉ์ธ ์์ธ๋ก ์ ํํ๋ ๊ฒ์ด ์์ ํฉ๋๋ค. ์คํค๋ง๊ฐ ์ด๋ฏธ ๋ณด์ฅํ๋ค๋ฉด ๋ฌด์ํ์ ๋ ๋ฉ๋๋ค.์คํค๋ง์ ์ด๋ฉ์ผ ์ ๋ํฌ ์ ์ฝ์ด ์๋์ง ํ์ธ ๋ถํ๋๋ฆฝ๋๋ค. ํ์ ์ ์์ธ ์ ํ ๋ก์ง ์ถ๊ฐ๋ ๋์๋๋ฆฌ๊ฒ ์ต๋๋ค.
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
๐ก Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
๐ Files selected for processing (6)
insty-api/src/main/java/insty/domain/auth/service/AuthService.java(2 hunks)insty-api/src/main/java/insty/domain/auth/strategy/GoogleStrategy.java(3 hunks)insty-api/src/main/java/insty/domain/auth/strategy/KakaoStrategy.java(3 hunks)insty-api/src/main/java/insty/domain/auth/strategy/NaverStrategy.java(3 hunks)insty-api/src/main/java/insty/domain/user/implement/UserValidator.java(1 hunks)insty-common/src/main/java/insty/error/UserErrorCode.java(1 hunks)
๐งฐ Additional context used
๐งฌ Code Graph Analysis (1)
insty-api/src/main/java/insty/domain/auth/strategy/KakaoStrategy.java (1)
insty-common/src/main/java/insty/generator/NicknameGenerator.java (1)
NicknameGenerator(5-48)
๐ Additional comments (6)
insty-common/src/main/java/insty/error/UserErrorCode.java (1)
8-8: ๋ฉ์์ง ๋ณ๊ฒฝ OK. ๋ค๋ง ํด๋ผ์ด์ธํธ(์น/์ฑ) ๋ฌธ์์ด ์์กด ์ฌ๋ถ ์ ๊ฒ ๊ถ์ฅ๋ฌธ์์ด์ด ๋ฐ๋๋ฉด E2E/FE ๋จ์์ ๋ฌธ์์ด ๋งค์นญ/์ค๋ ์ท ํ ์คํธ๊ฐ ๊นจ์ง ์ ์์ต๋๋ค. ๊ฐ๋ฅํ๋ฉด FE๋ code(=USER_005)์๋ง ์์กดํ๋๋ก ์ ๊ฒ ๋ถํ๋๋ฆฝ๋๋ค.
insty-api/src/main/java/insty/domain/auth/service/AuthService.java (1)
3-4: ๋น๊ธฐ๋ฅ ๋ณ๊ฒฝ(import ์ ๋ฆฌ) โ ๋ฌธ์ ์์insty-api/src/main/java/insty/domain/auth/strategy/KakaoStrategy.java (4)
23-23: UserValidator ์์ฑ์ ์ฃผ์ ์ถ๊ฐ ์ ์ ์ ๋ต ๋ ๋ฒจ์์ ๊ฐ์ ์ ๊ฒ์ฆ ์ฑ ์์ ๋ถ์ฌํ๋ ๋ฐฉํฅ์ด ์ผ๊ด๋๊ณ ์ข์ต๋๋ค.
3-3: UserValidator import ์ถ๊ฐ OK์์กด์ฑ ์ฃผ์ ๊ณผ ์ผ์นํฉ๋๋ค.
9-11: ์นด์นด์ค ์ด๋ํฐ/DTO import ์ ๋ฆฌ ์ด์ ์์์์กด์ฑ ์ฌ์ฉ๊ณผ ์ผ์นํฉ๋๋ค.
23-23: ์๋ ์ธ์คํด์คํ ๋ฐ ๊ฒ์ฆ ์์น ์ ์ ํ์ธ๋จ
ํ๋ก์ ํธ ๋ดnew KakaoStrategy(...)ํธ์ถ์ ์์๊ณ , Google/Naver ์ ๋ต์์๋validateDuplicateEmail๊ฐ ํ์๊ฐ์ ์์ (orElseGet ์ง์ )์ ํธ์ถ๋๊ณ ์์ต๋๋ค.
| userValidator.validateDuplicateEmail(email); | ||
|
|
There was a problem hiding this comment.
๐ก Verification agent
๐งฉ Analysis chain
์ค๋ํ ๋ฒ๊ทธ: ๊ธฐ์กด ์์ ์ฌ์ฉ์์ ์ ์ ๋ก๊ทธ์ธ๊น์ง 409(USER_DUPLICATE_EMAIL)๋ก ์ฐจ๋จ๋ฉ๋๋ค
์ด ์ค์์ ์ด๋ฉ์ผ ์ค๋ณต์ ๋จผ์ ๊ฒ์ฆํ๋ฉด, ์ด๋ฏธ ๋ฑ๋ก๋ ๊ตฌ๊ธ ์ฌ์ฉ์(๋์ผ email)๋ ๋ก๊ทธ์ธ ๋จ๊ณ์์ ์์ธ๊ฐ ๋ฐ์ํฉ๋๋ค. ์ฌ๋ฐ๋ฅธ ์์น๋ โํด๋น socialId/provider๋ก ์ฌ์ฉ์๊ฐ ์์ด์ ์๋ก ๋ง๋ค๋ ค๋ ์์ โ์ ๋๋ค.
์ ์ฉ ์ ์:
- ์ฌ์ ์ค๋ณต๊ฒ์ฌ ํธ์ถ ์ ๊ฑฐ
- userValidator.validateDuplicateEmail(email);- ์ ๊ท ์์ฑ ๋ถ๊ธฐ ๋ด์์๋ง ์ค๋ณต ๊ฒ์ฌ ์ํ(์์ ์ฝ๋)
return userRepository.findBySocialIdAndSocialType(socialId, SocialType.GOOGLE)
.orElseGet(() -> {
// ์ ๊ท ์์ฑ ์ ์๋ง ์ด๋ฉ์ผ ์ค๋ณต ์ฒดํฌ
userValidator.validateDuplicateEmail(email);
User newUser = User.createBySocial(socialId, SocialType.GOOGLE, email, nickname, userType);
return userRepository.save(newUser);
});์ถ๊ฐ ๊ถ์ฅ:
- ์ด๋ฉ์ผ์ Unique ์ธ๋ฑ์ค๊ฐ ์์ด์ผ ๋์ ๊ฐ์ ๊ฒฝํฉ ์ ๋ฌด๊ฒฐ์ฑ์ด ๋ณด์ฅ๋ฉ๋๋ค. ์๋ฐ ์์ธ๋ฅผ ์ก์ USER_DUPLICATE_EMAIL๋ก ๋งคํํด์ฃผ์ธ์.
๊ฒ์ฆ ์คํฌ๋ฆฝํธ(๋ค๋ฅธ ์ ๋ต์์๋ ๋์ผ ๋ฌธ์ ์ฌ๋ถ ํ์ธ):
๐ Script executed:
#!/bin/bash
# ์ ๋ต๋ค์์ validateDuplicateEmail ํธ์ถ ์์น ํ์ธ
rg -n -C3 --type=java '\bvalidateDuplicateEmail\s*\(' insty-api/src/main/java/insty/domain/auth/strategyLength of output: 2422
์ค๋ํ ๋ฒ๊ทธ: ์์ ๋ก๊ทธ์ธ ํ๋ฆ์์ ์ด๋ฉ์ผ ์ค๋ณต ๊ฒ์ฌ ์์น ์ค๋ฅ๋ก ๊ธฐ์กด ์ฌ์ฉ์ 409 ์ฐจ๋จ
ํ์ฌ Naver/Google/Kakao ์ ๋ต ๋ชจ๋ ๋ก๊ทธ์ธ ์ด๊ธฐ์ userValidator.validateDuplicateEmail(email);์ ํธ์ถํ๊ณ ์์ด, ์ด๋ฏธ ๊ฐ์
๋ ์์
์ฌ์ฉ์๊ฐ ๋ก๊ทธ์ธํ ๋๋ 409(USER_DUPLICATE_EMAIL) ์์ธ๊ฐ ๋ฐ์ํฉ๋๋ค.
ํด๊ฒฐ์ ์ํด ์๋ ์์
์ ๋ฐ๋์ ์ ์ฉํด์ฃผ์ธ์.
โข ์ํฅ ํ์ผ ๋ฐ ์์น
- insty-api/src/main/java/insty/domain/auth/strategy/NaverStrategy.java:57
- insty-api/src/main/java/insty/domain/auth/strategy/GoogleStrategy.java:58
- insty-api/src/main/java/insty/domain/auth/strategy/KakaoStrategy.java:59
โข 1) ๋ก๊ทธ์ธ ์ ์ค๋ณต๊ฒ์ฌ ํธ์ถ ์ ๊ฑฐ
- userValidator.validateDuplicateEmail(email);โข 2) ์ ๊ท ์ฌ์ฉ์ ์์ฑ ๋ถ๊ธฐ ๋ด์์๋ง ์ค๋ณต๊ฒ์ฌ ์ํ
return userRepository.findBySocialIdAndSocialType(socialId, SocialType.GOOGLE)
.orElseGet(() -> {
// ์ ๊ท ์์ฑ ์ ์๋ง ์ด๋ฉ์ผ ์ค๋ณต ์ฒดํฌ
userValidator.validateDuplicateEmail(email);
User newUser = User.createBySocial(socialId, SocialType.GOOGLE, email, nickname, userType);
return userRepository.save(newUser);
});(๊ฐ ์ ๋ต์์ findBySocialIdAndSocialType ํธ์ถ๋ถ๋ฅผ ์ ํํ๋ก ์์ )
โข 3) ์ด๋ฉ์ผ ์ปฌ๋ผ์ Unique ์ธ๋ฑ์ค ์ถ๊ฐ
๋ฐ์ดํฐ๋ฒ ์ด์ค์ ์ด๋ฉ์ผ ๊ณ ์ ์ ์ฝ์กฐ๊ฑด์ ๊ฑธ๊ณ , ConstraintViolation(DataIntegrityViolationException ๋ฑ)๋ฅผ ์ก์ USER_DUPLICATE_EMAIL๋ก ๋งคํํด ์ฃผ์ธ์.
์ ์ธ ๊ฐ์ง๋ฅผ ๋ฐ์ํ๋ฉด ๊ธฐ์กด ์์ ์ฌ์ฉ์์ ์ ์ ๋ก๊ทธ์ธ์ด 409๋ก ์ฐจ๋จ๋๋ ๋ฌธ์ ๋ฅผ ํด๊ฒฐํ ์ ์์ต๋๋ค.
๐ค Prompt for AI Agents
insty-api/src/main/java/insty/domain/auth/strategy/GoogleStrategy.java around
lines 58-59: currently calling userValidator.validateDuplicateEmail(email)
before checking for existing social user causes already-registered social users
to be blocked with 409; remove that pre-login duplicate-email check, change the
user lookup to use findBySocialIdAndSocialType(...).orElseGet(() -> {
userValidator.validateDuplicateEmail(email); create new User via
User.createBySocial(...); save and return it; }) so the email uniqueness check
runs only when creating a new user, and additionally add a UNIQUE
constraint/index on the email column in the DB and catch
constraint-violation/DataIntegrityViolationException to map it to
USER_DUPLICATE_EMAIL.
| userValidator.validateDuplicateEmail(email); | ||
|
|
There was a problem hiding this comment.
์ค๋ํ ๋ฒ๊ทธ: ๊ธฐ์กด ๋ค์ด๋ฒ ์์ ์ฌ์ฉ์์ ์ ์ ๋ก๊ทธ์ธ๊น์ง 409(USER_DUPLICATE_EMAIL)๋ก ์ฐจ๋จ๋ฉ๋๋ค
์ด๋ฉ์ผ ์ค๋ณต ๊ฒ์ฌ๋ โ์ ๊ท ์ฌ์ฉ์ ์์ฑโ ๊ฒฝ๋ก์์๋ง ์ํํด์ผ ํฉ๋๋ค. ํ์ฌ ์์น์์ ์ด๋ฏธ ๋ฑ๋ก๋ ์ฌ์ฉ์๋ ์์ธ๊ฐ ๋ฐ์ํฉ๋๋ค.
์ ์ฉ ์ ์:
- ์ฌ์ ์ค๋ณต๊ฒ์ฌ ํธ์ถ ์ ๊ฑฐ
- userValidator.validateDuplicateEmail(email);- ์ ๊ท ์์ฑ ๋ถ๊ธฐ ๋ด๋ก ์ด๋(์์ ์ฝ๋)
return userRepository.findBySocialIdAndSocialType(socialId, SocialType.NAVER)
.orElseGet(() -> {
userValidator.validateDuplicateEmail(email); // ์ ๊ท ์์ฑ ์ง์
User newUser = User.createBySocial(socialId, SocialType.NAVER, email, nickname, userType);
return userRepository.save(newUser);
});์ถ๊ฐ ๊ถ์ฅ:
- ์ด๋ฉ์ผ Unique ์ ์ฝ + ์๋ฐ ์์ธ๋ฅผ USER_DUPLICATE_EMAIL๋ก ๋ณํํ์ฌ ๊ฒฝํฉ/์ฌ์๋ ์ํฉ์ ์์ ํ๊ฒ ์ฒ๋ฆฌํ์ธ์.
๐ค Prompt for AI Agents
In insty-api/src/main/java/insty/domain/auth/strategy/NaverStrategy.java around
lines 57-58, the call to userValidator.validateDuplicateEmail(email) is
currently executed for all lookups causing existing Naver social users to be
blocked with 409; remove that pre-check and instead invoke
validateDuplicateEmail(email) only immediately before creating a new User inside
the orElseGet (or equivalent "create new" branch). Ensure the code finds by
socialId+SocialType, orElseGet does the duplicate-email validation then creates
and saves the new User; additionally add/ensure mapping from DB unique
constraint violations on email to the USER_DUPLICATE_EMAIL error to handle race
conditions safely.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and canโt be posted inline due to platform limitations.
โ ๏ธ Outside diff range comments (1)
insty-api/src/main/java/insty/domain/auth/strategy/KakaoStrategy.java (1)
55-67: ์ด๋ฉ์ผ ์ค๋ณต ๊ฒ์ฆ์ ์ค๋ณต ํธ์ถ๋ก ๊ธฐ์กด ์ฌ์ฉ์ ๋ก๊ทธ์ธ ์ฐจ๋จ ์ํ
- Line 59์์ ์ (ๅ ) ์ค๋ณต ๊ฒ์ฆ์ ์ํํ๊ณ , Line 65์์๋ ํ์๊ฐ์ ๊ฒฝ๋ก์์ ๋์ผ ๊ฒ์ฆ์ ๋ค์ ์ํํฉ๋๋ค.
- ๋ง์ฝ
validateDuplicateEmail์ด โํด๋น ์ด๋ฉ์ผ์ด ํ๋๋ผ๋ ์กด์ฌํ๋ฉด ์์ธโ๋ก ๋์ํ๋ค๋ฉด, ์ด๋ฏธ ๊ฐ์ ๋ ๋์ผ ์ฌ์ฉ์์ ์ผ๋ฐ ๋ก๊ทธ์ธ(= ๊ธฐ์กด ์์ ID ๋งค์นญ)๋ Line 59์์ ๋ถํ์ํ๊ฒ ์ฐจ๋จ๋ ์ ์์ต๋๋ค.- ์ค๋ณต ๊ฒ์ฆ์ โ์ ๊ท ์ฌ์ฉ์ ์์ฑ ๊ฒฝ๋ก(orElseGet ๋ด๋ถ)โ์์๋ง ์ํํ๋ ๊ฒ์ด ์์ ํฉ๋๋ค. ๋ํ ๋๋ค์ ์์ฑ๋ ์ ๊ท ๊ฐ์ ๊ฒฝ๋ก์์๋ง ์ํํ๋ฉด ๋ถํ์ํ ์ฐ์ฐ์ ์ค์ผ ์ ์์ต๋๋ค.
์ ์ ์์ ์:
Long socialId = userProfile.id(); // ์์ ํ์ ID String email = userProfile.kakaoAccount().email(); // ์ด๋ฉ์ผ - String nickname = NicknameGenerator.generateNickname(); - - userValidator.validateDuplicateEmail(email); + // ์ด๋ฉ์ผ/๋๋ค์ ๊ฒ์ฌ๋ ์ ๊ท ํ์๊ฐ์ ๊ฒฝ๋ก์์๋ง ์ํ log.info("์นด์นด์ค ๋ก๊ทธ์ธ : ์ฌ์ฉ์ ์ ๋ณด ์กฐํ ์๋ฃ , ์์ ID : {}", socialId); return userRepository.findBySocialIdAndSocialType(String.valueOf(socialId), SocialType.KAKAO) .orElseGet(() -> { // ์กด์ฌ X โ ํ์๊ฐ์ userValidator.validateDuplicateEmail(email); - User newUser = User.createBySocial(String.valueOf(socialId), SocialType.KAKAO, email, nickname, userType); + String nickname = NicknameGenerator.generateNickname(); + User newUser = User.createBySocial(String.valueOf(socialId), SocialType.KAKAO, email, nickname, userType); return userRepository.save(newUser); });
๐งน Nitpick comments (2)
insty-api/src/main/java/insty/domain/auth/strategy/KakaoStrategy.java (2)
46-54: ์ธ๋ถ HTTP ํธ์ถ์ ํธ๋์ญ์ ๋ฒ์ ๋ฐ์ผ๋ก ์ด๋ ๊ณ ๋ ค
@Transactional๋ฉ์๋ ์์ ์งํ ์ธ๋ถ API ํธ์ถ(kakao ํ ํฐ/ํ๋กํ ์กฐํ)์ด ์ํ๋๊ณ ์์ต๋๋ค. ๋ถํ์ํ๊ฒ ํธ๋์ญ์ ์ด ๊ธธ์ด์ง ์ ์์ผ๋ฏ๋ก:
- ์ธ๋ถ ํธ์ถ์ ํธ๋์ญ์ ๋ฐ์์ ์ํํ๊ณ ,
- DB ์ฐ๊ธฐ๊ฐ ํ์ํ ๊ตฌ๊ฐ(์ ๊ท ์ ์ ์์ฑ/์ ์ฅ)๋ง ํธ๋์ญ์ ์ผ๋ก ๊ฐ์ธ๋ ๊ตฌ์กฐ(๋ณ๋ ๋ฉ์๋ ๋ถ๋ฆฌ ๋ฑ)๋ฅผ ๊ณ ๋ คํด ์ฃผ์ธ์.
61-61: ๋ก๊ทธ ๋ฏผ๊ฐ๋ ์ ๊ฒ (์์ ID ์ถ๋ ฅ)์์ ID๋ ์ธ๋ถ ์๋ณ์๋ผ ๊ฐ์ธ ์๋ณ ์ ๋ณด๋ก ๊ฐ์ฃผ๋ ์ ์์ต๋๋ค. ์ด์ ํ๊ฒฝ์์๋ debug ๋ ๋ฒจ๋ก ๋ฎ์ถ๊ฑฐ๋ ๋ง์คํน/ํด์ฑ์ ๊ณ ๋ คํด ์ฃผ์ธ์.
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
๐ก Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
๐ Files selected for processing (3)
insty-api/src/main/java/insty/domain/auth/strategy/GoogleStrategy.java(3 hunks)insty-api/src/main/java/insty/domain/auth/strategy/KakaoStrategy.java(3 hunks)insty-api/src/main/java/insty/domain/auth/strategy/NaverStrategy.java(3 hunks)
๐ง Files skipped from review as they are similar to previous changes (2)
- insty-api/src/main/java/insty/domain/auth/strategy/NaverStrategy.java
- insty-api/src/main/java/insty/domain/auth/strategy/GoogleStrategy.java
๐งฐ Additional context used
๐งฌ Code Graph Analysis (1)
insty-api/src/main/java/insty/domain/auth/strategy/KakaoStrategy.java (1)
insty-common/src/main/java/insty/generator/NicknameGenerator.java (1)
NicknameGenerator(5-48)
๐ Additional comments (2)
insty-api/src/main/java/insty/domain/auth/strategy/KakaoStrategy.java (2)
23-23: UserValidator ์์กด์ฑ ์ฃผ์ ๐์์ ๋ก๊ทธ์ธ ์ ์ด๋ฉ์ผ ์ ํจ์ฑ ๊ณตํต ๊ฒ์ฆ์ ์ํด Validator๋ฅผ ์ฃผ์ ํ ๋ฐฉํฅ์ฑ ์ข์ต๋๋ค. ๋ค๋ฅธ ์ ๋ต๋ค๊ณผ์ ์ผ๊ด์ฑ๋ ํ๋ณด๋ ๋ฏํฉ๋๋ค.
56-56: Kakao ํ๋กํ์์ ์ด๋ฉ์ผ์ด null/๋ฏธ๋์์ผ ์ ์์ โ NPE/๊ฒ์ฆ ํ๋ฆ ํ์ธ ํ์Kakao๋ ๊ณ์ ์ค์ /๋์์ ๋ฐ๋ผ
kakaoAccount()ํน์email()์ด null์ผ ์ ์์ต๋๋ค. ์ด ๊ฒฝ์ฐ:
validateDuplicateEmail(email)๋ด๋ถ์์ NPE๊ฐ ๋ฐ์ํ๊ฑฐ๋,createBySocial(..., email, ...)์ ์ฅ ์ ์ ์ฝ ์กฐ๊ฑด ์๋ฐ์ด ๋ฐ์ํ ์ ์์ต๋๋ค.ํ์ฌ
UserValidator.validateDuplicateEmail์ด null/blank๋ฅผ ์์ ํ๊ฒ ์ฒ๋ฆฌํ๋์ง, ๋๋ ์ฌ๊ธฐ์ ์ ํ ๊ฐ๋๋ฅผ ๋์ด์ผ ํ๋์ง ํ์ธ ๋ถํ๋๋ฆฝ๋๋ค. ํ์ ์ ๋ค์ ์ค ํ๋๋ฅผ ๊ณ ๋ คํด ์ฃผ์ธ์:
- ์ด๋ฉ์ผ ๋ฏธ์ ๊ณต ์ ๋ช ํํ ์๋ฌ ์ฝ๋๋ก ๊ฑฐ์ ์ฒ๋ฆฌ
- ์ด๋ฉ์ผ ์๋ ์์ ๊ฐ์ ์ ํ์ฉํ๋ ์ ์ฑ ์ด๋ผ๋ฉด, ํด๋น ์ผ์ด์ค๋ ์ค๋ณต ๊ฒ์ฌ ์คํต ๋ฐ DB ์ ์ฝ๊ณผ์ ์ผ๊ด์ฑ ํ๋ณด
| return userRepository.findBySocialIdAndSocialType(String.valueOf(socialId), SocialType.KAKAO) | ||
| .orElseGet(() -> { // ์กด์ฌ X โ ํ์๊ฐ์ | ||
| userValidator.validateDuplicateEmail(email); | ||
| User newUser = User.createBySocial(String.valueOf(socialId), SocialType.KAKAO, email, nickname, userType); | ||
| return userRepository.save(newUser); | ||
| }); |
There was a problem hiding this comment.
๐ก Verification agent
๐งฉ Analysis chain
TOCTOU ๊ฒฝ์ ์กฐ๊ฑด ๊ฐ๋ฅ์ฑ โ DB ์ ๋ํฌ ์ ์ฝ/์์ธ ์ฒ๋ฆฌ ํ์ธ ๊ถ์ฅ
์ค๋ณต ์ด๋ฉ์ผ ๊ฒ์ฌ ํ ์ ์ฅ ์ฌ์ด์ ๊ฒฝ์ ์กฐ๊ฑด์ด ๋ฐ์ํ๋ฉด(๋์ ๊ฐ์ ), ๋์ผ ์ด๋ฉ์ผ์ ๊ณ์ ์ด ์ค๋ณต ์์ฑ๋ ์ ์์ต๋๋ค. ์ค์ง์ ์ธ ์์ ์ฅ์น๋ DB์ ์ ๋ํฌ ์ ์ฝ์ ๋๋ค. ๋ค์์ ํ์ธํด ์ฃผ์ธ์:
- user.email์ ์ ๋ํฌ ์ธ๋ฑ์ค ์กด์ฌ ์ฌ๋ถ
- (์ ํ) social_id + social_type ์กฐํฉ์ ๋ํ ์ ๋ํฌ ์ ์ฝ ์กด์ฌ ์ฌ๋ถ
- ์๋ฐ ์
DataIntegrityViolationException๋ฑ์ ์บ์นํ์ฌ ๋๋ฉ์ธ ์ค๋ฅ๋ก ๋ณํํ๋ ์ฒ๋ฆฌ
์๋ ์คํฌ๋ฆฝํธ๋ก ์ํฐํฐ/๋ง์ด๊ทธ๋ ์ด์ ์์ ์ ๋ํฌ ์ ์ฝ์ ๋น ๋ฅด๊ฒ ์ ๊ฒํ ์ ์์ต๋๋ค.
๐ Script executed:
#!/bin/bash
set -euo pipefail
echo "== JPA ์ํฐํฐ์์ ์ ๋ํฌ ์ ์ฝ ์ด๋
ธํ
์ด์
๊ฒ์ =="
rg -n -C2 -P '(?i)@Table\([^)]*uniqueConstraints|@Column\([^)]*unique\s*=\s*true' --type=java
echo
echo "== User ์ํฐํฐ ๋ด email ํ๋/์ ์ฝ ๊ด๋ จ ๋จ์ ๊ฒ์ =="
rg -n -C3 -P '(?i)class\s+User\b|@Column|email' --type=java
echo
echo "== SQL/๋ง์ด๊ทธ๋ ์ด์
์์ ์ ๋ํฌ ์ ์ฝ ๊ฒ์ =="
rg -n -C2 -P '(?i)create\s+table|unique\s+index|unique\s*\(|constraint' --type=sqlLength of output: 211263
DB ์ ๋ํฌ ์ ์ฝ ๋๋ฝ ๋ฐ TOCTOU ๋ฐฉ์ง ๋ณด๊ฐ ํ์
ํ์ฌ users.email์ ๋ํ DBยทJPA ๋ ๋ฒจ ์ ๋ํฌ ์ ์ฝ์ด ์์ด ๋์ ๊ฐ์
์ ์ค๋ณต ๊ณ์ ์ด ์์ฑ๋ ์ ์์ต๋๋ค. ์๋๋ฅผ ๋ฐ๋์ ์ ์ฉํด์ฃผ์ธ์:
โข insty-domain/src/main/java/insty/model/user/User.java
โ @Column(nullable = false, length = 100) โ @Column(nullable = false, length = 100, unique = true) ์ถ๊ฐ
โข ์ค์ด์ DB ๋ง์ด๊ทธ๋ ์ด์
(๋๋ schema.sql)
โ users.email์ UNIQUE INDEX ์์ฑ
โ (์ ํ) (social_id, social_type) ๋ณตํฉ ์ ๋ํฌ ์ ์ฝ ์ถ๊ฐ
โข ์์ธ ์ฒ๋ฆฌ ๋ณด๊ฐ
โ ํ์๊ฐ์
๋ก์ง(์: KakaoStrategy)์์ userRepository.save(...) ํธ์ถ์ try { โฆ } catch (DataIntegrityViolationException e)๋ก ๊ฐ์ธ๊ณ
UserErrorCode.USER_DUPLICATE_EMAIL(409) ๋ฑ์ผ๋ก ๋ณํํ๋ ํธ๋ค๋ฌ ์ถ๊ฐ
๐ค Prompt for AI Agents
In insty-api/src/main/java/insty/domain/auth/strategy/KakaoStrategy.java around
lines 63 to 68, the current signup flow can create duplicate users under
concurrent requests because there's no DB/JPA UNIQUE constraint on users.email
and the save() is not handling integrity violations; add a DB/JPA unique
constraint and catch integrity exceptions: 1) update
insty-domain/src/main/java/insty/model/user/User.java to change @Column(nullable
= false, length = 100) to @Column(nullable = false, length = 100, unique =
true); 2) add a DB migration (or update schema.sql) to create a UNIQUE INDEX on
users.email (and optionally a composite UNIQUE on (social_id, social_type)); 3)
wrap userRepository.save(...) in KakaoStrategy with try { ... } catch
(DataIntegrityViolationException e) and translate it into your application error
(e.g., throw a UserErrorCode.USER_DUPLICATE_EMAIL mapped to 409) so TOCTOU race
conditions result in a controlled, meaningful error.
Summary by CodeRabbit