Conversation
WalkthroughOAuth 인증 흐름이 재구성되어 (provider, providerUid) 우선 조회 후 이메일 기반 확인 및 신규 사용자 생성·링크 로직이 추가되었고, UserOauthAccountRepository에 existsByProviderAndUser 메서드가 추가되었으며 TokenEncryptor 기본 키값이 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant OAuth as OAuth Provider
participant Service as CustomOAuth2UserService
participant OAuthRepo as UserOauthAccountRepository
participant UserRepo as UserRepository
participant DB as Database
OAuth->>Service: 로그인 응답 (provider, providerUid, email?, name?)
rect rgb(200,220,240)
Note over Service: 1) provider+providerUid로 기존 링크 조회
Service->>OAuthRepo: findByProviderAndProviderUid(provider, uid)
OAuthRepo->>DB: 쿼리
DB-->>OAuthRepo: 결과
OAuthRepo-->>Service: Optional<UserOauthAccount>
end
alt 기존 링크가 존재
Service-->>Service: 연관 User 로드 및 반환 (DefaultOAuth2User)
else 링크 없음
rect rgb(220,240,200)
Note over Service: 2) 이메일로 사용자 조회
Service->>UserRepo: findByEmail(email 또는 가상 이메일)
UserRepo->>DB: 쿼리
DB-->>UserRepo: 결과
UserRepo-->>Service: Optional<User>
end
alt 이메일로 사용자 존재
rect rgb(240,240,200)
Note over Service: 3) 존재 사용자에 OAuth 링크 생성 시도
Service->>OAuthRepo: save(new UserOauthAccount)
OAuthRepo->>DB: insert
alt 삽입 성공
DB-->>OAuthRepo: 저장 완료
OAuthRepo-->>Service: UserOauthAccount
else DataIntegrityViolationException
DB-->>OAuthRepo: 예외
OAuthRepo-->>Service: 예외 전달
Service->>OAuthRepo: existsByProviderAndUser(provider, user)
OAuthRepo-->>Service: true/false
Service-->>Service: 기존 링크 재확인 후 사용자 로드
end
end
else 신규 사용자
rect rgb(240,220,240)
Note over Service: 4) 신규 User 생성 및 OAuth 링크 생성
Service->>UserRepo: save(new User)
UserRepo->>DB: insert
DB-->>UserRepo: User 저장
UserRepo-->>Service: User
Service->>OAuthRepo: save(new UserOauthAccount)
OAuthRepo->>DB: insert
DB-->>OAuthRepo: UserOauthAccount 저장
OAuthRepo-->>Service: UserOauthAccount
end
end
end
Service-->>OAuth: DefaultOAuth2User (provider, providerUid, email, name, userId, ROLE_TEAM_MEMBER)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20-25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/TokenEncryptor.java (1)
24-30: 하드코딩된 기본 암호화 키는 프로덕션 보안 사고로 직결됩니다 (기본값 제거 권장).환경변수가 누락되면 누구나 아는 키로 토큰을 복호화할 수 있습니다. 또한
key.length() == 16은 “문자” 기준이라 UTF-8 바이트 길이가 16이 아닐 수 있습니다(비-ASCII).
아래처럼 기본값을 빈 값으로 두고(byte 길이로) fail-fast 하는 쪽이 안전합니다.- public TokenEncryptor(@Value("${TOKEN_ENCRYPTION_KEY:mySecretKey12345}") String key) { - if (key == null || key.length() != 16) { + public TokenEncryptor(@Value("${TOKEN_ENCRYPTION_KEY:}") String key) { + byte[] keyBytes = (key == null) ? null : key.getBytes(StandardCharsets.UTF_8); + if (keyBytes == null || keyBytes.length != 16) { throw new IllegalStateException( "유효한 16바이트 토큰 암호화 키가 설정되지 않았습니다. 환경변수 TOKEN_ENCRYPTION_KEY를 확인하세요."); } - this.secretKey = new SecretKeySpec(key.getBytes(StandardCharsets.UTF_8), ALGORITHM); + this.secretKey = new SecretKeySpec(keyBytes, ALGORITHM); }backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java (1)
51-70: providerUid/email 추출에서 NPE/정책 공백(특히 email=null)을 명확히 처리해 주세요.
attrs.get("id").toString()/attrs.get("sub")는 provider 응답이 예상과 다르면 NPE가 날 수 있습니다.- GitHub/Kakao는
email == null이 현실적으로 자주 발생하는데, 아래 신규 생성 로직에서email(null)로User를 만들지/막을지 정책이 필요합니다(현재는 “DB 제약 있으면 정책 필요” 주석만 존재).가능하면 “email이 없는 신규 유저 생성”을 명시적으로 차단하고 OAuth 인증 실패로 돌려 UX/운영 이슈를 줄이는 게 안전합니다(또는 GitHub의 경우 별도 이메일 API/스코프 요구).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/src/main/java/org/sejongisc/backend/auth/dao/UserOauthAccountRepository.java(1 hunks)backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java(3 hunks)backend/src/main/java/org/sejongisc/backend/common/auth/jwt/TokenEncryptor.java(1 hunks)
🔇 Additional comments (3)
backend/src/main/java/org/sejongisc/backend/auth/dao/UserOauthAccountRepository.java (1)
3-14: 메서드 추가는 적절합니다. 다만 (provider, user) 인덱스/유니크 제약은 확인해 주세요.
existsByProviderAndUser(...)는 로그인 핫패스에서 호출될 가능성이 높아서, DB에(provider, user_id)인덱스(또는 유니크)가 없으면 성능이 흔들릴 수 있습니다. 또한 중복 링크 방지는 궁극적으로 유니크 제약이 보장해주는지 확인이 필요합니다.backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java (2)
24-38: delegate/Optional 정리는 깔끔합니다.가독성 개선 관점에서 변경 방향 좋습니다.
151-165: 반환 attributes에 providerUid/userId를 노출하는 의도가 맞는지 확인해 주세요.
providerUid는 외부 계정 식별자라, 프론트/클라이언트로 내려보내는 것이 요구사항이 아니라면 불필요한 노출이 될 수 있습니다. 또한DefaultOAuth2User의 name key를"userId"로 두는 변경은 downstream(Spring Security에서 principal name 사용처)이"userId"기준으로 동작하도록 바뀌는 점만 확인하면 좋겠습니다.
backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java (2)
98-106: 이전 리뷰에서 지적된 TOCTOU 레이스 조건이 여전히 남아있습니다.
existsByProviderAndUser확인 후save사이에 동시 요청이 들어오면 DB 유니크 제약 위반으로DataIntegrityViolationException이 발생할 수 있습니다. 이 부분에 예외 처리가 필요합니다.다음과 같이 수정하세요:
boolean alreadyLinked = oauthAccountRepository.existsByProviderAndUser(authProvider, user); if (!alreadyLinked) { UserOauthAccount oauth = UserOauthAccount.builder() .user(user) .provider(authProvider) .providerUid(providerUid) .build(); - oauthAccountRepository.save(oauth); + try { + oauthAccountRepository.save(oauth); + } catch (org.springframework.dao.DataIntegrityViolationException ignore) { + // 동시성으로 이미 링크가 생성된 경우 무시 + } }
133-140: catch 블록 내부의 OAuth 링크 저장도 동시성 예외가 발생할 수 있습니다.133-140번 라인의
existsByProviderAndUser+save패턴도 동일한 TOCTOU 레이스 조건을 가지고 있습니다. 다른 동시 요청이 같은 OAuth 링크를 먼저 생성하면DataIntegrityViolationException이 발생하는데, 이것이 catch 블록 내부이므로 처리되지 않고 상위로 전파됩니다.다음과 같이 수정하세요:
if (!oauthAccountRepository.existsByProviderAndUser(authProvider, existing)) { UserOauthAccount oauth = UserOauthAccount.builder() .user(existing) .provider(authProvider) .providerUid(providerUid) .build(); - oauthAccountRepository.save(oauth); + try { + oauthAccountRepository.save(oauth); + } catch (org.springframework.dao.DataIntegrityViolationException ignore) { + // 동시성으로 이미 링크가 생성된 경우 무시 + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java(2 hunks)
🔇 Additional comments (2)
backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java (2)
23-23: 개선된 코드 구조입니다.Optional 임포트 추가와 delegate 인스턴스 생성 간소화는 적절하며, 코드 가독성이 향상되었습니다.
Also applies to: 29-29, 37-37
60-62: Null-safe 처리가 적절히 추가되었습니다.Kakao와 GitHub 제공자의 null 가능성에 대한 방어 코드와 주석이 잘 작성되었습니다.
Also applies to: 66-67
| // 2) 링크가 없으면 email로 기존 로컬/회원가입 유저를 찾는다 (핵심!) | ||
| Optional<User> userByEmailOpt = Optional.empty(); | ||
| if (!email.isBlank()) { | ||
| userByEmailOpt = userRepository.findUserByEmail(email); | ||
| } |
There was a problem hiding this comment.
중복된 blank 체크입니다.
74-77번 라인에서 이미 email이 null이거나 blank일 경우 가상 이메일로 대체했으므로, 90번 라인의 !email.isBlank() 체크는 항상 true입니다. 이 체크는 불필요하며 로직 흐름을 불명확하게 만듭니다.
다음과 같이 수정하세요:
- // 2) 링크가 없으면 email로 기존 로컬/회원가입 유저를 찾는다 (핵심!)
- Optional<User> userByEmailOpt = Optional.empty();
- if (!email.isBlank()) {
- userByEmailOpt = userRepository.findUserByEmail(email);
- }
+ // 2) 링크가 없으면 email로 기존 로컬/회원가입 유저를 찾는다 (핵심!)
+ Optional<User> userByEmailOpt = userRepository.findUserByEmail(email);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 2) 링크가 없으면 email로 기존 로컬/회원가입 유저를 찾는다 (핵심!) | |
| Optional<User> userByEmailOpt = Optional.empty(); | |
| if (!email.isBlank()) { | |
| userByEmailOpt = userRepository.findUserByEmail(email); | |
| } | |
| // 2) 링크가 없으면 email로 기존 로컬/회원가입 유저를 찾는다 (핵심!) | |
| Optional<User> userByEmailOpt = userRepository.findUserByEmail(email); |
🤖 Prompt for AI Agents
In
backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java
around lines 88 to 92, the check `!email.isBlank()` is redundant because earlier
(lines 74-77) email is already replaced with a non-blank virtual email; remove
the if block and directly call the repository so userByEmailOpt is assigned from
userRepository.findUserByEmail(email) unconditionally, simplifying the code and
clarifying control flow.
| return new DefaultOAuth2User( | ||
| List.of(new SimpleGrantedAuthority("ROLE_TEAM_MEMBER")), | ||
| attributes, | ||
| "userId" // 또는 "email" -> email null 이면 id가 더 안전 | ||
| "userId" | ||
| ); |
There was a problem hiding this comment.
하드코딩된 권한이 사용자의 실제 역할과 불일치합니다.
154번 라인에서 모든 사용자에게 ROLE_TEAM_MEMBER 권한을 부여하고 있지만, 실제 데이터베이스의 사용자는 다른 역할(예: ADMIN)을 가질 수 있습니다. 이는 권한 기반 접근 제어에서 잘못된 권한 검사를 초래할 수 있습니다.
사용자의 실제 역할을 사용하도록 수정하세요:
return new DefaultOAuth2User(
- List.of(new SimpleGrantedAuthority("ROLE_TEAM_MEMBER")),
+ List.of(new SimpleGrantedAuthority("ROLE_" + user.getRole().name())),
attributes,
"userId"
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return new DefaultOAuth2User( | |
| List.of(new SimpleGrantedAuthority("ROLE_TEAM_MEMBER")), | |
| attributes, | |
| "userId" // 또는 "email" -> email null 이면 id가 더 안전 | |
| "userId" | |
| ); | |
| return new DefaultOAuth2User( | |
| List.of(new SimpleGrantedAuthority("ROLE_" + user.getRole().name())), | |
| attributes, | |
| "userId" | |
| ); |
🤖 Prompt for AI Agents
In
backend/src/main/java/org/sejongisc/backend/common/auth/config/CustomOAuth2UserService.java
around lines 153 to 157, the code currently hardcodes a single
SimpleGrantedAuthority("ROLE_TEAM_MEMBER") for every user; replace this by
loading the user's actual roles from your User entity (or user repository) and
mapping them to a List<GrantedAuthority> (e.g., prefixing with "ROLE_" if
needed), then pass that list into the DefaultOAuth2User constructor; ensure you
handle null/empty roles by providing a safe default and convert any role
DTOs/strings into SimpleGrantedAuthority instances before returning the
DefaultOAuth2User.
1 participant
Summary by CodeRabbit
출시 노트
새로운 기능
버그 수정
보안
✏️ Tip: You can customize this high-level summary in your review settings.