Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough사용자 엔티티의 OAuth 필드(provider, provider_id, apple_refresh_token)를 제거하고 별도 엔티티 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OAuthProvider
participant OAuthHandler as OAuth2LoginSuccessHandler
participant UserOAuthService as UserOAuthAccountService
participant UserOAuthRepo as UserOAuthAccountRepository
participant UserRepo as UserRepository
participant DB
Client->>OAuthProvider: OAuth 인증 요청
OAuthProvider->>OAuthHandler: 콜백 + 사용자정보 (+appleRefreshToken)
OAuthHandler->>UserOAuthService: linkOAuthAccount(userId, provider, providerId, oauthEmail, appleRefreshToken)
alt providerId present
UserOAuthService->>UserOAuthRepo: findByProviderAndProviderId(provider, providerId)
UserOAuthRepo->>UserOAuthService: existingAccount?
alt existingAccount.owner == userId
UserOAuthService->>UserOAuthRepo: updateAppleRefreshToken(account, token)
UserOAuthRepo->>DB: save
else
UserOAuthService->>UserOAuthRepo: throw OAUTH_ACCOUNT_ALREADY_LINKED
end
else no providerId / email match
UserOAuthService->>UserOAuthRepo: findAccountByProviderAndOauthEmail(provider, oauthEmail)
UserOAuthRepo->>UserOAuthService: maybeAccount
alt account exists && account.user.deletedAt IS NOT NULL
UserOAuthService->>UserRepo: restore user
UserRepo->>DB: update
else
UserOAuthService->>UserRepo: findOrCreateUser(oauthEmail,...)
UserRepo->>DB: insert
end
UserOAuthService->>UserOAuthRepo: save(new UserOAuthAccount(..., appleRefreshToken))
UserOAuthRepo->>DB: insert
end
OAuthHandler->>Client: 로그인 완료 응답
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java (1)
175-180: 🧹 Nitpick | 🔵 Trivial[LEVEL: low] saveAndFlush 사용으로 단순화 가능
문제:
save()후 별도로entityManager.flush()호출 → 영향: 코드 복잡도 증가 → 제안:UserOAuthAccountService.saveWithConflictHandling처럼saveAndFlush()사용으로 일관성 확보.♻️ 수정 제안
try { - userOAuthAccountRepository.save(UserOAuthAccount.of(user, provider, providerId, oauthEmail, null)); - entityManager.flush(); + userOAuthAccountRepository.saveAndFlush(UserOAuthAccount.of(user, provider, providerId, oauthEmail, null)); } catch (DataIntegrityViolationException e) { throw CustomException.of(ApiResponseCode.OAUTH_PROVIDER_ALREADY_LINKED); }이 방식으로 변경 시
EntityManager의존성도 제거 가능합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java` around lines 175 - 180, Replace the separate userOAuthAccountRepository.save(...) followed by entityManager.flush() with a single userOAuthAccountRepository.saveAndFlush(...) call (or delegate to a service method like UserOAuthAccountService.saveWithConflictHandling that calls saveAndFlush) and keep the existing DataIntegrityViolationException catch throwing CustomException.of(ApiResponseCode.OAUTH_PROVIDER_ALREADY_LINKED); this lets you remove the EntityManager dependency from OAuthLoginHelper and simplifies the flow while preserving conflict handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/gg/agit/konect/domain/user/model/UserOAuthAccount.java`:
- Around line 60-61: The appleRefreshToken field in UserOAuthAccount is stored
in plaintext; change it to use JPA field-level encryption by implementing an
AttributeConverter (e.g., AppleTokenAttributeConverter) that encrypts on
convertToDatabaseColumn and decrypts on convertToEntityAttribute, then annotate
the appleRefreshToken field with `@Convert`(converter =
AppleTokenAttributeConverter.class); ensure the converter uses a secure key
management source (vault/KMS or environment-provided key) and handles nulls and
exceptions safely so tokens are encrypted at rest and transparently decrypted
for application use.
In
`@src/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.java`:
- Around line 49-59: The current repository method
UserOAuthAccountRepository.findUserByOauthEmailAndProvider(...) returns
Optional<User> while the underlying user_oauth_account table does not enforce
uniqueness on (provider, oauth_email), causing possible ambiguous matches; fix
by either (A) switching to a providerId-based lookup: change the query and
method signature to use the providerId field (e.g.,
findUserByProviderAndProviderId(`@Param`("provider") Provider provider,
`@Param`("providerId") String providerId)) and update callers accordingly, or (B)
preserve oauthEmail lookup but make it safe: change the repository method to
return List<User> (e.g., findUsersByOauthEmailAndProvider) and handle multiple
results, or enforce DB uniqueness by adding a unique constraint on (provider,
oauth_email) and then keep the Optional<User> signature—apply one of these
remedies and update tests/consuming code to match.
In
`@src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java`:
- Around line 179-182: The getPrimaryOAuthAccount method is nondeterministic
because userOAuthAccountRepository.findAllByUserId lacks an ORDER BY; update
data access to guarantee the earliest-linked account is returned by either
adding a repository method like findFirstByUserIdOrderByIdAsc(Integer userId) or
modifying the existing query in UserOAuthAccountRepository to include ORDER BY
id ASC and then call that ordered method from getPrimaryOAuthAccount so it
returns the first (oldest) account deterministically.
In `@src/main/java/gg/agit/konect/domain/user/service/UserService.java`:
- Around line 223-238: Replace the duplicate calls to
userOAuthAccountService.getPrimaryOAuthAccount by reusing the first result: keep
the initial UserOAuthAccount primaryAccount from the first call, use its
appleRefreshToken for appleTokenRevocationService.revoke if present, and derive
providerName from that same primaryAccount (falling back to "UNKNOWN") when
creating UserWithdrawnEvent.from; remove the second getPrimaryOAuthAccount call
and the primaryAccountForEvent variable so only one DB lookup occurs.
In
`@src/main/resources/db/migration/V45__add_apple_refresh_token_to_user_oauth_account.sql`:
- Around line 15-24: The INSERT in
V45__add_apple_refresh_token_to_user_oauth_account.sql currently only checks for
duplicates by (user_id, provider) which can violate the user_oauth_account
unique constraint on (provider, provider_id) for re-registered users; fix by
narrowing the backfill to active users or by excluding existing (provider,
provider_id) pairs — e.g., add AND u.deleted_at IS NULL to the WHERE clause or
change the NOT EXISTS subquery to also check ua.provider_id = u.provider_id
(WHERE ua.user_id = u.id AND ua.provider = u.provider AND ua.provider_id =
u.provider_id) so no existing (provider, provider_id) rows are inserted.
---
Outside diff comments:
In `@src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java`:
- Around line 175-180: Replace the separate userOAuthAccountRepository.save(...)
followed by entityManager.flush() with a single
userOAuthAccountRepository.saveAndFlush(...) call (or delegate to a service
method like UserOAuthAccountService.saveWithConflictHandling that calls
saveAndFlush) and keep the existing DataIntegrityViolationException catch
throwing CustomException.of(ApiResponseCode.OAUTH_PROVIDER_ALREADY_LINKED); this
lets you remove the EntityManager dependency from OAuthLoginHelper and
simplifies the flow while preserving conflict handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 228a4101-89e3-49a3-8777-627fb462115e
📒 Files selected for processing (15)
src/main/java/gg/agit/konect/domain/user/model/User.javasrc/main/java/gg/agit/konect/domain/user/model/UserOAuthAccount.javasrc/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.javasrc/main/java/gg/agit/konect/domain/user/repository/UserRepository.javasrc/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.javasrc/main/java/gg/agit/konect/domain/user/service/UserService.javasrc/main/java/gg/agit/konect/global/auth/oauth/OAuth2LoginSuccessHandler.javasrc/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.javasrc/main/java/gg/agit/konect/infrastructure/oauth/AppleOAuthServiceImpl.javasrc/main/java/gg/agit/konect/infrastructure/oauth/GoogleOAuthServiceImpl.javasrc/main/java/gg/agit/konect/infrastructure/oauth/KakaoOAuthServiceImpl.javasrc/main/java/gg/agit/konect/infrastructure/oauth/NaverOAuthServiceImpl.javasrc/main/resources/db/migration/V45__add_apple_refresh_token_to_user_oauth_account.sqlsrc/main/resources/db/migration/V46__remove_oauth_columns_from_users.sqlsrc/test/java/gg/agit/konect/support/fixture/UserFixture.java
💤 Files with no reviewable changes (1)
- src/test/java/gg/agit/konect/support/fixture/UserFixture.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/user/service/UserService.javasrc/main/java/gg/agit/konect/global/auth/oauth/OAuth2LoginSuccessHandler.javasrc/main/java/gg/agit/konect/domain/user/model/UserOAuthAccount.javasrc/main/java/gg/agit/konect/domain/user/model/User.javasrc/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.javasrc/main/java/gg/agit/konect/infrastructure/oauth/GoogleOAuthServiceImpl.javasrc/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.javasrc/main/java/gg/agit/konect/infrastructure/oauth/KakaoOAuthServiceImpl.javasrc/main/java/gg/agit/konect/infrastructure/oauth/NaverOAuthServiceImpl.javasrc/main/java/gg/agit/konect/domain/user/repository/UserRepository.javasrc/main/java/gg/agit/konect/infrastructure/oauth/AppleOAuthServiceImpl.javasrc/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/domain/user/service/UserService.javasrc/main/resources/db/migration/V45__add_apple_refresh_token_to_user_oauth_account.sqlsrc/main/java/gg/agit/konect/global/auth/oauth/OAuth2LoginSuccessHandler.javasrc/main/resources/db/migration/V46__remove_oauth_columns_from_users.sqlsrc/main/java/gg/agit/konect/domain/user/model/UserOAuthAccount.javasrc/main/java/gg/agit/konect/domain/user/model/User.javasrc/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.javasrc/main/java/gg/agit/konect/infrastructure/oauth/GoogleOAuthServiceImpl.javasrc/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.javasrc/main/java/gg/agit/konect/infrastructure/oauth/KakaoOAuthServiceImpl.javasrc/main/java/gg/agit/konect/infrastructure/oauth/NaverOAuthServiceImpl.javasrc/main/java/gg/agit/konect/domain/user/repository/UserRepository.javasrc/main/java/gg/agit/konect/infrastructure/oauth/AppleOAuthServiceImpl.javasrc/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java
src/main/resources/db/migration/**/*.sql
⚙️ CodeRabbit configuration file
src/main/resources/db/migration/**/*.sql: Flyway 마이그레이션 리뷰 규칙:
- 버전 파일명 규칙(V{number}__{description}.sql) 위반 여부를 우선 확인한다.
- 이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다.
- 파괴적 변경(drop, rename 등)은 롤백 가능성과 운영 영향 관점에서 검토한다.
Files:
src/main/resources/db/migration/V45__add_apple_refresh_token_to_user_oauth_account.sqlsrc/main/resources/db/migration/V46__remove_oauth_columns_from_users.sql
🔇 Additional comments (2)
src/main/java/gg/agit/konect/domain/user/model/User.java (1)
80-81: LGTM!
@OneToMany관계가LAZYfetch로 적절히 설정되었고, 서비스 레이어에서UserOAuthAccountRepository를 통해 명시적으로 조회하는 패턴과 일관성이 있습니다.final키워드로 컬렉션 재할당 방지도 좋습니다.src/main/java/gg/agit/konect/domain/user/repository/UserRepository.java (1)
19-25: LGTM!
findById쿼리가id와deletedAt IS NULL조건으로 정확히 구성되어 있습니다. Provider 관련 쿼리 제거는 OAuth 계정 분리 리팩토링 목적에 부합합니다.
src/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.java
Show resolved
Hide resolved
src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java
Show resolved
Hide resolved
src/main/java/gg/agit/konect/domain/user/service/UserService.java
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sql
Show resolved
Hide resolved
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)
src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java (1)
50-72:⚠️ Potential issue | 🔴 Critical[LEVEL: high] Line 72는 이메일 fallback에서
restoreWithdrawnByEmail을 우회해 빈 이메일도 그대로 조회합니다.
영향: Apple처럼 이메일이 누락될 수 있는 로그인에서 잘못된 사용자 매칭이나 조회 예외가 날 수 있고, 복구 가능 기간의 탈퇴 계정도 정상 복구되지 않습니다.
제안:StringUtils.hasText(email)를 먼저 확인하고restoreWithdrawnByEmail(provider, email)을 선호한 뒤, 없을 때만 이메일 기반 조회를 수행하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java` around lines 50 - 72, The fallback path in findUserByProvider currently queries userOAuthAccountRepository.findUserByOauthEmailAndProvider(email, provider) even when email is blank and skips restoreWithdrawnByEmail; update findUserByProvider to first check StringUtils.hasText(email), and if true call restoreWithdrawnByEmail(provider, email) and if that returns a user call ensureLinkedAccount(...) and return it; only if restoreWithdrawnByEmail returns empty then fall back to userOAuthAccountRepository.findUserByOauthEmailAndProvider(email, provider). Ensure you keep the existing linked-account logic (ensureLinkedAccount and restoreOrCleanupWithdrawnByLinkedProvider) unchanged.src/main/resources/db/migration/V43__add_user_oauth_account_table.sql (1)
1-35:⚠️ Potential issue | 🔴 Critical[LEVEL: high] V43 버전 파일을 직접 수정하면 Flyway checksum과 환경별 스키마가 갈라집니다.
이미 V43가 적용된 환경에서는 이번 변경이 재실행되지 않아provider_idNULL 허용 여부와 backfill 조건이 환경마다 달라질 수 있습니다.
이 변경은 V43 수정 대신 새 버전 마이그레이션으로ALTER TABLE과 보정 DML을 분리해 추가하세요.
As per coding guidelines,src/main/resources/db/migration/**/*.sql: 이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V43__add_user_oauth_account_table.sql` around lines 1 - 35, Do not modify the existing V43__add_user_oauth_account_table.sql; instead create a new numbered migration that performs the schema change and a separate migration (or same new migration with ordered statements) that runs the backfill DML. Specifically, leave the current V43 (which creates table user_oauth_account and constraints fk_user_oauth_account_user, uq_user_oauth_account_provider_provider_id, uq_user_oauth_account_user_provider) untouched, and add a new migration (e.g., next version) that runs ALTER TABLE user_oauth_account ... to change provider_id nullable/constraints as needed and a following DML that inserts missing rows (the SELECT ... FROM users logic) so already-applied environments get the ALTER + backfill applied uniformly rather than editing V43. Ensure the new migration includes both the ALTER TABLE and the corrective INSERT in the correct order and references the same table/constraint names.
♻️ Duplicate comments (3)
src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java (1)
190-192:⚠️ Potential issue | 🟠 Major[LEVEL: medium] Line 191-192는 정렬 없는 목록의 첫 원소를 primary로 반환해 결과가 비결정적입니다.
영향: DB 실행 계획이나 데이터 적재 순서에 따라 다른 OAuth 계정이 대표 계정으로 선택되어 토큰 처리나 이벤트 기준이 흔들릴 수 있습니다.
제안: Repository에findFirstByUserIdOrderByIdAsc같은 메서드를 추가하거나 조회 쿼리에ORDER BY uoa.id ASC를 명시하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java` around lines 190 - 192, getPrimaryOAuthAccount currently calls userOAuthAccountRepository.findAllByUserId and returns the first element of an unordered list, making the primary selection non-deterministic; change this to use an ordered, single-row lookup such as adding a repository method like findFirstByUserIdOrderByIdAsc (or a query with ORDER BY uoa.id ASC LIMIT 1) in UserOAuthAccountRepository and update getPrimaryOAuthAccount to call that new method and return null when absent so the primary OAuth account selection is deterministic.src/main/resources/db/migration/V45__add_apple_refresh_token_to_user_oauth_account.sql (1)
15-23:⚠️ Potential issue | 🔴 Critical[LEVEL: high] Line 15-23의 보정 INSERT는
(user_id, provider)만 보고 중복을 걸러서(provider, provider_id)유니크 제약과 충돌할 수 있습니다.
탈퇴 후 같은 OAuth 계정으로 재가입한 이력이 있으면 V45가 중간에 실패하고 뒤의 V46까지 막힙니다.
보정 대상에서 이미 존재하는(provider, provider_id)조합을 추가로 제외하거나, 정책상 유지할 행을 먼저 정리한 뒤 INSERT 하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V45__add_apple_refresh_token_to_user_oauth_account.sql` around lines 15 - 23, The INSERT in V45 into user_oauth_account only checks NOT EXISTS by (user_id, provider) and can violate the unique constraint on (provider, provider_id); update the INSERT (in src/main/resources/db/migration/V45__add_apple_refresh_token_to_user_oauth_account.sql) to also exclude rows where the (provider, provider_id) pair already exists (e.g., add an AND NOT EXISTS subquery that checks ua.provider = u.provider AND ua.provider_id = u.provider_id), or alternately add a pre-INSERT cleanup step that deterministically removes/archives conflicting rows for the (provider, provider_id) combination before running the INSERT to avoid failing the migration.src/main/java/gg/agit/konect/domain/user/model/UserOAuthAccount.java (1)
60-61:⚠️ Potential issue | 🟠 Major[LEVEL: medium] Line 60-61은 Apple refresh token을 평문으로 저장해 DB 유출 시 바로 재사용 가능한 자격증명이 노출됩니다.
영향: 토큰 탈취가 곧 계정 재연동이나 세션 악용으로 이어질 수 있어 사고 범위가 커집니다.
제안: 저장 시 암호화하거나 별도 비밀 저장소로 분리해 at-rest 보호를 적용하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/domain/user/model/UserOAuthAccount.java` around lines 60 - 61, The appleRefreshToken field in UserOAuthAccount is stored in plaintext; update persistence to protect it at-rest by either encrypting the value with a reversible encryption layer or moving the secret to a secure secret store and persisting only a reference. Implement this by adding a JPA AttributeConverter (e.g., AppleTokenEncryptor) and annotate the appleRefreshToken field with `@Convert`(converter = AppleTokenEncryptor.class) or change setAppleRefreshToken/getAppleRefreshToken to call the encryption/decryption utility, or alternatively store a secretId from your secret manager instead of the raw token and add logic to fetch the token at runtime; ensure encryption keys or secret-store credentials are managed outside source control.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java`:
- Around line 128-144: validatePrimaryProviderOwnership currently throws
OAUTH_PROVIDER_ALREADY_LINKED even when the existingAccount.getProviderId() is
null/blank, blocking the auto-fill logic; update the second check
(userOAuthAccountRepository.findByUserIdAndProvider(...).ifPresent(existingAccount
-> { ... })) to only throw when existingAccount.getProviderId() is non-empty and
not equal to the incoming providerId (i.e., skip the mismatch exception if
existingAccount.getProviderId() is null/blank so Line 108-119 can perform the
auto-fill). Reference: validatePrimaryProviderOwnership,
userOAuthAccountRepository.findByUserIdAndProvider, existingAccount,
getProviderId(), providerId.
---
Outside diff comments:
In `@src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java`:
- Around line 50-72: The fallback path in findUserByProvider currently queries
userOAuthAccountRepository.findUserByOauthEmailAndProvider(email, provider) even
when email is blank and skips restoreWithdrawnByEmail; update findUserByProvider
to first check StringUtils.hasText(email), and if true call
restoreWithdrawnByEmail(provider, email) and if that returns a user call
ensureLinkedAccount(...) and return it; only if restoreWithdrawnByEmail returns
empty then fall back to
userOAuthAccountRepository.findUserByOauthEmailAndProvider(email, provider).
Ensure you keep the existing linked-account logic (ensureLinkedAccount and
restoreOrCleanupWithdrawnByLinkedProvider) unchanged.
In `@src/main/resources/db/migration/V43__add_user_oauth_account_table.sql`:
- Around line 1-35: Do not modify the existing
V43__add_user_oauth_account_table.sql; instead create a new numbered migration
that performs the schema change and a separate migration (or same new migration
with ordered statements) that runs the backfill DML. Specifically, leave the
current V43 (which creates table user_oauth_account and constraints
fk_user_oauth_account_user, uq_user_oauth_account_provider_provider_id,
uq_user_oauth_account_user_provider) untouched, and add a new migration (e.g.,
next version) that runs ALTER TABLE user_oauth_account ... to change provider_id
nullable/constraints as needed and a following DML that inserts missing rows
(the SELECT ... FROM users logic) so already-applied environments get the ALTER
+ backfill applied uniformly rather than editing V43. Ensure the new migration
includes both the ALTER TABLE and the corrective INSERT in the correct order and
references the same table/constraint names.
---
Duplicate comments:
In `@src/main/java/gg/agit/konect/domain/user/model/UserOAuthAccount.java`:
- Around line 60-61: The appleRefreshToken field in UserOAuthAccount is stored
in plaintext; update persistence to protect it at-rest by either encrypting the
value with a reversible encryption layer or moving the secret to a secure secret
store and persisting only a reference. Implement this by adding a JPA
AttributeConverter (e.g., AppleTokenEncryptor) and annotate the
appleRefreshToken field with `@Convert`(converter = AppleTokenEncryptor.class) or
change setAppleRefreshToken/getAppleRefreshToken to call the
encryption/decryption utility, or alternatively store a secretId from your
secret manager instead of the raw token and add logic to fetch the token at
runtime; ensure encryption keys or secret-store credentials are managed outside
source control.
In
`@src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java`:
- Around line 190-192: getPrimaryOAuthAccount currently calls
userOAuthAccountRepository.findAllByUserId and returns the first element of an
unordered list, making the primary selection non-deterministic; change this to
use an ordered, single-row lookup such as adding a repository method like
findFirstByUserIdOrderByIdAsc (or a query with ORDER BY uoa.id ASC LIMIT 1) in
UserOAuthAccountRepository and update getPrimaryOAuthAccount to call that new
method and return null when absent so the primary OAuth account selection is
deterministic.
In
`@src/main/resources/db/migration/V45__add_apple_refresh_token_to_user_oauth_account.sql`:
- Around line 15-23: The INSERT in V45 into user_oauth_account only checks NOT
EXISTS by (user_id, provider) and can violate the unique constraint on
(provider, provider_id); update the INSERT (in
src/main/resources/db/migration/V45__add_apple_refresh_token_to_user_oauth_account.sql)
to also exclude rows where the (provider, provider_id) pair already exists
(e.g., add an AND NOT EXISTS subquery that checks ua.provider = u.provider AND
ua.provider_id = u.provider_id), or alternately add a pre-INSERT cleanup step
that deterministically removes/archives conflicting rows for the (provider,
provider_id) combination before running the INSERT to avoid failing the
migration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 461115f0-3a6a-4cbc-98b0-c2bbcd8a66aa
📒 Files selected for processing (5)
src/main/java/gg/agit/konect/domain/user/model/UserOAuthAccount.javasrc/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.javasrc/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.javasrc/main/resources/db/migration/V43__add_user_oauth_account_table.sqlsrc/main/resources/db/migration/V45__add_apple_refresh_token_to_user_oauth_account.sql
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
src/main/resources/db/migration/**/*.sql
⚙️ CodeRabbit configuration file
src/main/resources/db/migration/**/*.sql: Flyway 마이그레이션 리뷰 규칙:
- 버전 파일명 규칙(V{number}__{description}.sql) 위반 여부를 우선 확인한다.
- 이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다.
- 파괴적 변경(drop, rename 등)은 롤백 가능성과 운영 영향 관점에서 검토한다.
Files:
src/main/resources/db/migration/V45__add_apple_refresh_token_to_user_oauth_account.sqlsrc/main/resources/db/migration/V43__add_user_oauth_account_table.sql
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/resources/db/migration/V45__add_apple_refresh_token_to_user_oauth_account.sqlsrc/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.javasrc/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.javasrc/main/java/gg/agit/konect/domain/user/model/UserOAuthAccount.javasrc/main/resources/db/migration/V43__add_user_oauth_account_table.sql
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.javasrc/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.javasrc/main/java/gg/agit/konect/domain/user/model/UserOAuthAccount.java
src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sql (1)
15-23:⚠️ Potential issue | 🔴 Critical[LEVEL: high] 탈퇴 유저 보정 INSERT가 새 유니크 키와 다시 충돌할 수 있습니다.
문제: Line 15-23은 중복 여부를
(user_id, provider)로만 확인해서, 탈퇴 후 같은 OAuth 계정으로 재가입한 이력이 있으면(provider, provider_id)유니크 제약을 피하지 못합니다. 영향: 운영 데이터에 재가입 케이스가 있으면 이 마이그레이션이 중간에 실패하고 이후 버전 적용도 함께 막힙니다. 제안: 보정 대상에서 기존(provider, provider_id)조합을 추가로 제외하거나, 충돌 데이터를 먼저 정리한 뒤 INSERT 하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sql` around lines 15 - 23, The INSERT checks duplicates only by (user_id, provider) which can still violate the (provider, provider_id) unique constraint for re-registered users; update the migration so the SELECT excludes rows where a user_oauth_account already exists with the same (provider, provider_id) (add a NOT EXISTS subquery referencing ua.provider = u.provider AND ua.provider_id = u.provider_id) or alternatively delete/merge conflicting user_oauth_account rows for those (provider, provider_id) before running the INSERT to prevent unique-key collisions; target the user_oauth_account INSERT/SELECT block and the users columns provider and provider_id when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/resources/db/migration/V43__add_user_oauth_account_table.sql`:
- Around line 33-34: Revert the changes made to
V43__add_user_oauth_account_table.sql so the original migration (including the
original WHERE clause) is restored rather than editing an already-applied
migration; then create a new migration file (e.g., V44__...) that implements the
modified condition (the change to "WHERE ua.user_id = u.id AND ua.provider =
u.provider") so environments that have already run V43 keep a consistent
checksum and new installs get the updated behavior—do not modify V43 in-place.
In `@src/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sql`:
- Around line 1-2: The migration file
V45__make_provider_id_nullable_in_users.sql must not be inserted between
existing V44/V45 versions; instead revert this change and create a new migration
with the next sequential Flyway version (e.g., V46__) that makes provider_id
nullable, so you never alter or insert migrations into an already-deployed
version chain; update the PR to remove the mid-chain insertion and add the new
final-version migration file (reference:
V45__make_provider_id_nullable_in_users.sql and the new V46__... file).
In
`@src/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sql`:
- Around line 7-12: The UPDATE and the corresponding INSERT must be restricted
to Apple provider rows: modify the UPDATE on user_oauth_account (aliases uoa and
u) to include a provider filter (e.g., AND u.provider = 'APPLE' and/or
uoa.provider = 'APPLE' as appropriate) so only Apple accounts get
u.apple_refresh_token copied, and likewise change the INSERT ... SELECT that
inserts apple_refresh_token (the later block referencing users and
user_oauth_account) to only select/insert rows where provider = 'APPLE'; target
the table aliases user_oauth_account (uoa) and users (u) in both statements to
ensure only Apple-provider records are updated/inserted.
In `@src/main/resources/db/migration/V47__remove_oauth_columns_from_users.sql`:
- Line 3: V47 currently issues ALTER TABLE users DROP INDEX
uq_users_provider_provider_id_active which duplicates the removal already
performed in V45__make_provider_id_nullable_in_users.sql; remove that DROP INDEX
from V47 to avoid failing in environments where V45 has run, or alternatively
replace it with a guarded drop (e.g., DROP INDEX IF EXISTS) for the index
uq_users_provider_provider_id_active so the migration no longer errors when the
index is absent.
In
`@src/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sql`:
- Around line 14-27: The DELETE currently joins user_oauth_account AS dup to the
subquery aliased ranked (which selects provider, oauth_email, id AS keep_id and
ROW_NUMBER() AS rn) using dup.id != ranked.keep_id, causing the wrong rows to be
removed; change the join condition so you delete only rows whose id corresponds
to ranked.id for rows with ranked.rn > 1 (i.e., select id in the ranked subquery
and join/compare dup.id = ranked.id or delete using WHERE id IN (SELECT id FROM
(...) WHERE rn > 1)) so that only duplicate rows (rn > 1) are deleted and the
intended keep_id (rn = 1) is preserved.
---
Duplicate comments:
In
`@src/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sql`:
- Around line 15-23: The INSERT checks duplicates only by (user_id, provider)
which can still violate the (provider, provider_id) unique constraint for
re-registered users; update the migration so the SELECT excludes rows where a
user_oauth_account already exists with the same (provider, provider_id) (add a
NOT EXISTS subquery referencing ua.provider = u.provider AND ua.provider_id =
u.provider_id) or alternatively delete/merge conflicting user_oauth_account rows
for those (provider, provider_id) before running the INSERT to prevent
unique-key collisions; target the user_oauth_account INSERT/SELECT block and the
users columns provider and provider_id when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1f69ffa5-cef9-474a-ab41-805a2bddf10b
📒 Files selected for processing (6)
src/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.javasrc/main/resources/db/migration/V43__add_user_oauth_account_table.sqlsrc/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sqlsrc/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sqlsrc/main/resources/db/migration/V47__remove_oauth_columns_from_users.sqlsrc/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sql
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
src/main/resources/db/migration/**/*.sql
⚙️ CodeRabbit configuration file
src/main/resources/db/migration/**/*.sql: Flyway 마이그레이션 리뷰 규칙:
- 버전 파일명 규칙(V{number}__{description}.sql) 위반 여부를 우선 확인한다.
- 이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다.
- 파괴적 변경(drop, rename 등)은 롤백 가능성과 운영 영향 관점에서 검토한다.
Files:
src/main/resources/db/migration/V43__add_user_oauth_account_table.sqlsrc/main/resources/db/migration/V47__remove_oauth_columns_from_users.sqlsrc/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sqlsrc/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sqlsrc/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sql
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/resources/db/migration/V43__add_user_oauth_account_table.sqlsrc/main/resources/db/migration/V47__remove_oauth_columns_from_users.sqlsrc/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sqlsrc/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sqlsrc/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sqlsrc/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.java
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.java
src/main/resources/db/migration/V43__add_user_oauth_account_table.sql
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sql
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sql
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/V47__remove_oauth_columns_from_users.sql
Show resolved
Hide resolved
src/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/main/resources/db/migration/V43__add_user_oauth_account_table.sql (1)
1-19:⚠️ Potential issue | 🔴 Critical[LEVEL: high] 배포 이력이 있는
V43__...파일을 직접 수정하면 안 됩니다.문제: 이 파일의 SQL이 바뀌면 이미 V43를 적용한 환경과 신규 환경의 Flyway checksum/실행 결과가 달라집니다. 영향: 배포 시
validate실패가 나거나, 같은 버전 번호가 환경마다 다른 스키마 상태를 뜻하게 됩니다. 제안: V43은 원복하고, 필요한 제약/백필 조정은 새 버전 마이그레이션으로 분리하세요. As per coding guidelines, "이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V43__add_user_oauth_account_table.sql` around lines 1 - 19, Revert all changes to V43__add_user_oauth_account_table.sql so the originally-applied migration is unchanged, do not modify this deployed migration file; instead create a new Flyway migration (e.g., V44__add_oauth_constraints_and_backfill.sql) that applies the additional constraints/unique indexes and any backfill/alter-table statements for the user_oauth_account table and performs safe backfill of oauth_email or provider_id data if needed; reference the existing table name user_oauth_account and the constraints (uq_user_oauth_account_provider_provider_id, uq_user_oauth_account_user, fk_user_oauth_account_user) in the new migration so all schema changes are applied forward-only.src/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sql (1)
1-2:⚠️ Potential issue | 🔴 Critical[LEVEL: high] 새 변경을
V45__...로 끼워 넣으면 기존 Flyway version chain이 깨집니다.문제: 이 파일 추가 때문에 기존 V45 이후 마이그레이션이 재번호 부여되어, 이미 적용된 환경과 신규 환경의 버전 이력이 달라집니다. 영향: 배포 시
validate/migrate실패나 환경별 스키마 분기가 발생할 수 있습니다. 제안: 이 변경은 현재 마지막 버전 다음 번호의 새 마이그레이션으로 추가하고, 기존 버전 파일명은 유지하세요. As per coding guidelines, "이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sql` around lines 1 - 2, You added a migration named V45__make_provider_id_nullable_in_users.sql which rewrites the Flyway version chain and will break existing deployments; instead leave any existing V45 file unchanged and create a new migration with the next sequential Flyway version (e.g., V46__) that contains the ALTER TABLE user_oauth_account MODIFY COLUMN provider_id VARCHAR(255) NULL; change, and then run Flyway validate/migrate locally to confirm the sequence is correct before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java`:
- Around line 53-62: Both public methods linkOAuthAccount and
linkPrimaryOAuthAccount drop the Apple refresh token by passing null into
linkAccount, so user_oauth_account.apple_refresh_token never gets updated;
change the signatures of linkOAuthAccount(Integer userId, Provider provider,
String providerId, String oauthEmail) and linkPrimaryOAuthAccount(User user,
Provider provider, String providerId, String oauthEmail) to accept an additional
String appleRefreshToken parameter, pass that parameter into linkAccount(...)
instead of null, and ensure linkAccount updates existing OAuth records (and
persists the new appleRefreshToken) when re-linking so deleteUser() can revoke
the correct token.
- Around line 86-98: The current logic only calls
restoreOrCleanupWithdrawnByLinkedProvider(provider, providerId) when providerId
is present, leaving withdrawn accounts with NULL providerId unhandled and
causing unique-constraint conflicts; add a branch in UserOAuthAccountService
(around the block using providerId, requireProviderId, oauthEmail,
validatePrimaryProviderOwnership) that when providerId is not present calls a
new or existing restore/cleanup routine based on oauthEmail (e.g.,
restoreOrCleanupWithdrawnByOauthEmail(provider, oauthEmail) or an overloaded
restoreOrCleanupWithdrawnByLinkedProvider accepting oauthEmail) before
proceeding, and ensure validatePrimaryProviderOwnership still runs appropriately
or is adjusted to accept the oauthEmail flow so NULL-providerId withdrawn
accounts are recovered/cleaned prior to uniqueness checks.
In `@src/main/java/gg/agit/konect/domain/user/service/UserService.java`:
- Around line 223-228: The current logic in UserService using
userOAuthAccountService.getPrimaryOAuthAccount(userId) only revokes Apple tokens
on the primary account, which misses Apple tokens on non-primary accounts;
update the code to fetch all OAuth accounts for the user (via
userOAuthAccountService.getByUserId or equivalent) or query for accounts where
getProvider() == Provider.APPLE, iterate over each UserOAuthAccount, call
getAppleRefreshToken() and if non-null invoke
appleTokenRevocationService.revoke(...) for each, ensuring all Apple refresh
tokens are revoked rather than only the primary account.
In
`@src/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sql`:
- Around line 17-25: The current dedupe SELECT in
V48__add_unique_constraint_provider_oauth_email.sql ranks rows by created_at
DESC and may keep recently backfilled deleted accounts; modify the query that
builds the ranked set (the subquery selecting id, ROW_NUMBER() OVER (PARTITION
BY provider, oauth_email ORDER BY created_at DESC) AS rn FROM
user_oauth_account) to join the users table and prefer active accounts by
including users.deleted_at IS NULL in the ORDER BY (so active accounts sort
before deleted), and add a deterministic tiebreaker such as id DESC as a final
ORDER BY key; ensure the ROW_NUMBER() partition remains PARTITION BY provider,
oauth_email and only rows with rn > 1 are returned for deletion.
---
Duplicate comments:
In `@src/main/resources/db/migration/V43__add_user_oauth_account_table.sql`:
- Around line 1-19: Revert all changes to V43__add_user_oauth_account_table.sql
so the originally-applied migration is unchanged, do not modify this deployed
migration file; instead create a new Flyway migration (e.g.,
V44__add_oauth_constraints_and_backfill.sql) that applies the additional
constraints/unique indexes and any backfill/alter-table statements for the
user_oauth_account table and performs safe backfill of oauth_email or
provider_id data if needed; reference the existing table name user_oauth_account
and the constraints (uq_user_oauth_account_provider_provider_id,
uq_user_oauth_account_user, fk_user_oauth_account_user) in the new migration so
all schema changes are applied forward-only.
In `@src/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sql`:
- Around line 1-2: You added a migration named
V45__make_provider_id_nullable_in_users.sql which rewrites the Flyway version
chain and will break existing deployments; instead leave any existing V45 file
unchanged and create a new migration with the next sequential Flyway version
(e.g., V46__) that contains the ALTER TABLE user_oauth_account MODIFY COLUMN
provider_id VARCHAR(255) NULL; change, and then run Flyway validate/migrate
locally to confirm the sequence is correct before committing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e97b1cea-7803-412a-af65-425ab8910aa4
📒 Files selected for processing (6)
src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.javasrc/main/java/gg/agit/konect/domain/user/service/UserService.javasrc/main/resources/db/migration/V43__add_user_oauth_account_table.sqlsrc/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sqlsrc/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sqlsrc/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sql
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
src/main/resources/db/migration/**/*.sql
⚙️ CodeRabbit configuration file
src/main/resources/db/migration/**/*.sql: Flyway 마이그레이션 리뷰 규칙:
- 버전 파일명 규칙(V{number}__{description}.sql) 위반 여부를 우선 확인한다.
- 이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다.
- 파괴적 변경(drop, rename 등)은 롤백 가능성과 운영 영향 관점에서 검토한다.
Files:
src/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sqlsrc/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sqlsrc/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sqlsrc/main/resources/db/migration/V43__add_user_oauth_account_table.sql
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sqlsrc/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sqlsrc/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sqlsrc/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.javasrc/main/java/gg/agit/konect/domain/user/service/UserService.javasrc/main/resources/db/migration/V43__add_user_oauth_account_table.sql
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.javasrc/main/java/gg/agit/konect/domain/user/service/UserService.java
src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java
Outdated
Show resolved
Hide resolved
src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java
Show resolved
Hide resolved
src/main/java/gg/agit/konect/domain/user/service/UserService.java
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sql
Show resolved
Hide resolved
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)
src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java (1)
83-115:⚠️ Potential issue | 🟡 Minor[LEVEL: medium] 사용되지 않는 메서드 제거 필요
restoreWithdrawnByProviderId(83-97줄)와restoreWithdrawnByEmail(99-115줄) 메서드가 코드베이스 어디에서도 호출되지 않습니다.restoreOrCleanupWithdrawnByLinkedProvider(121줄)와restoreOrCleanupWithdrawnByOauthEmail(144줄)이 이 역할을 담당하고 있어 데드 코드로 유지보수 부담을 증가시킵니다. 두 메서드를 삭제하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java` around lines 83 - 115, Remove the dead helper methods restoreWithdrawnByProviderId and restoreWithdrawnByEmail from OAuthLoginHelper: delete their entire method definitions (including their internals) and any now-unused imports or fields only referenced by them; before committing, run a project-wide search to confirm no callers remain (restoreOrCleanupWithdrawnByLinkedProvider and restoreOrCleanupWithdrawnByOauthEmail already cover this functionality) and run tests/build to ensure no compilation issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.java`:
- Around line 74-83: Remove the unused duplicate repository method
findByProviderAndProviderId from UserOAuthAccountRepository; instead rely on the
existing findAccountByProviderAndProviderId as the single access point for
looking up accounts by provider and providerId. Ensure any imports/annotations
only used by the removed method are cleaned up and run tests/compile to verify
no references remain to findByProviderAndProviderId.
In
`@src/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sql`:
- Around line 6-10: Remove or comment out the diagnostic SELECT statement that
returns duplicate provider/oauth_email rows in the migration (the SELECT
provider, oauth_email, COUNT(*) ... HAVING cnt > 1 query) so the Flyway
migration doesn't print unnecessary results; keep any needed UNIQUE CONSTRAINT
creation logic intact and ensure only non-query DDL/cleanup statements remain in
V48__add_unique_constraint_provider_oauth_email.sql.
---
Outside diff comments:
In `@src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java`:
- Around line 83-115: Remove the dead helper methods
restoreWithdrawnByProviderId and restoreWithdrawnByEmail from OAuthLoginHelper:
delete their entire method definitions (including their internals) and any
now-unused imports or fields only referenced by them; before committing, run a
project-wide search to confirm no callers remain
(restoreOrCleanupWithdrawnByLinkedProvider and
restoreOrCleanupWithdrawnByOauthEmail already cover this functionality) and run
tests/build to ensure no compilation issues.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e7d10a38-1d11-4881-bd5f-b285ec96f115
📒 Files selected for processing (7)
src/main/java/gg/agit/konect/domain/user/controller/UserController.javasrc/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.javasrc/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.javasrc/main/java/gg/agit/konect/domain/user/service/UserService.javasrc/main/java/gg/agit/konect/global/auth/oauth/OAuth2LoginSuccessHandler.javasrc/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.javasrc/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sql
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/global/auth/oauth/OAuth2LoginSuccessHandler.javasrc/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.javasrc/main/java/gg/agit/konect/domain/user/service/UserService.javasrc/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.javasrc/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.javasrc/main/java/gg/agit/konect/domain/user/controller/UserController.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/global/auth/oauth/OAuth2LoginSuccessHandler.javasrc/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.javasrc/main/java/gg/agit/konect/domain/user/service/UserService.javasrc/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.javasrc/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sqlsrc/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.javasrc/main/java/gg/agit/konect/domain/user/controller/UserController.java
src/main/resources/db/migration/**/*.sql
⚙️ CodeRabbit configuration file
src/main/resources/db/migration/**/*.sql: Flyway 마이그레이션 리뷰 규칙:
- 버전 파일명 규칙(V{number}__{description}.sql) 위반 여부를 우선 확인한다.
- 이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다.
- 파괴적 변경(drop, rename 등)은 롤백 가능성과 운영 영향 관점에서 검토한다.
Files:
src/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sql
🔇 Additional comments (17)
src/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sql (1)
12-28: [LEVEL: low] 중복 제거 로직이 이전 리뷰 피드백을 반영하여 개선되었습니다.이전 리뷰에서 지적된 두 가지 문제가 해결되었습니다:
rn > 1인 행의id만 별도로 추출하여dup.id = to_delete.id기준으로 삭제users테이블을 조인하여deleted_at IS NULL활성 계정을 우선 보존하고,created_at DESC, id DESC로 결정적 정렬src/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.java (3)
42-52: [LEVEL: low] LGTM - V48 마이그레이션의 유니크 제약 조건과 함께 사용됩니다.
findAccountByProviderAndOauthEmail메서드가Optional<UserOAuthAccount>를 반환하며, V48 마이그레이션에서(provider, oauth_email)유니크 제약 조건이 추가되어 단건 조회가 보장됩니다.
62-72: [LEVEL: low] LGTM - 유니크 제약 조건이 단건 조회를 보장합니다.이전 리뷰에서
(provider, oauth_email)유니크 제약이 없어 문제가 될 수 있다고 지적되었으나, V48 마이그레이션에서 해당 제약 조건이 추가되어Optional<User>반환이 안전합니다.
58-58: [LEVEL: low] LGTM - 결정적 정렬이 추가되었습니다.
ORDER BY uoa.id ASC가 추가되어getPrimaryOAuthAccount메서드의 결과가 결정적으로 반환됩니다.src/main/java/gg/agit/konect/domain/user/service/UserService.java (3)
229-234: [LEVEL: low] LGTM - 모든 Apple OAuth 계정의 토큰을 철회합니다.이전 리뷰에서 지적된 문제(primary account만 철회)가 해결되었습니다. 이제 사용자의 모든 OAuth 계정을 순회하며 Apple provider인 경우 토큰을 철회합니다.
239-242: [LEVEL: low] LGTM - 중복 호출이 제거되었습니다.이전 리뷰에서 지적된
getPrimaryOAuthAccount중복 호출 문제가 해결되었습니다. 이미 조회한oauthAccounts리스트의 첫 번째 항목을 재사용합니다.
105-111: [LEVEL: low] LGTM - Apple refresh token이 올바르게 전달됩니다.
tempUser.getAppleRefreshToken()을 통해 Apple refresh token이linkPrimaryOAuthAccount로 전달되어, 신규 가입 시 토큰이 저장됩니다.src/main/java/gg/agit/konect/domain/user/controller/UserController.java (1)
115-115: [LEVEL: low] LGTM - 수동 연동 시 Apple refresh token이 없는 것은 정상입니다.REST API를 통한 수동 OAuth 연동 시에는 Apple refresh token이 없으므로
null을 전달하는 것이 올바릅니다.UserOAuthAccountService.linkAccount에서StringUtils.hasText(appleRefreshToken)로 검증 후 처리하므로 기존 토큰이 덮어씌워지지 않습니다.src/main/java/gg/agit/konect/global/auth/oauth/OAuth2LoginSuccessHandler.java (2)
182-188: [LEVEL: low] LGTM - OAuth 연동 모드에서 Apple refresh token이 올바르게 전달됩니다.
handleOAuthLinkMode에서 추출된appleRefreshToken이linkOAuthAccount로 전달되어, 연동 시 토큰이 저장됩니다.
360-364: [LEVEL: low] LGTM - 기존 사용자의 Apple refresh token이 올바르게 갱신됩니다.
findByUserIdAndProvider로 조회 후ifPresent로 토큰을 갱신합니다. 계정이 없는 경우(신규 가입 전)는saveAppleRefreshTokenForUnRegisteredUser(Line 367-391)에서 별도로 처리됩니다.src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java (4)
54-62: [LEVEL: low] LGTM - Apple refresh token 전달 경로가 추가되었습니다.이전 리뷰에서 지적된 토큰 전달 누락 문제가 해결되었습니다.
linkOAuthAccount와linkPrimaryOAuthAccount모두appleRefreshToken파라미터를 받아linkAccount로 전달합니다.
106-111: [LEVEL: low] LGTM - providerId 없는 계정의 복구 경로가 추가되었습니다.이전 리뷰에서 지적된 문제가 해결되었습니다.
providerId가 없을 때oauthEmail기반의restoreOrCleanupWithdrawnByOauthEmail을 호출하여 탈퇴 계정 복구/정리를 수행합니다.
121-133: [LEVEL: low] LGTM - providerId NULL/blank 처리가 개선되었습니다.이전 리뷰에서 지적된 문제가 해결되었습니다. 기존 계정의
providerId가 NULL이거나 blank인 경우에만 업데이트하고, 이미 다른 값이 있으면 충돌 예외를 던집니다.
224-227: [LEVEL: low] LGTM - 결정적 정렬이 보장됩니다.이전 리뷰에서 지적된 문제가 해결되었습니다.
findAllByUserId쿼리에ORDER BY uoa.id ASC가 추가되어 항상 동일한 계정이 primary로 반환됩니다.src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java (3)
72-80: [LEVEL: low] LGTM - 이메일 기반 복구 흐름이 추가되었습니다.
providerId로 사용자를 찾지 못한 경우,oauthEmail기반의restoreOrCleanupWithdrawnByOauthEmail을 호출하여 탈퇴 계정을 복구하거나 정리합니다.
144-164: [LEVEL: low] LGTM - 이메일 기반 탈퇴 계정 복구/정리 로직이 추가되었습니다.
restoreOrCleanupWithdrawnByLinkedProvider와 동일한 패턴으로oauthEmail기반의 복구/정리를 수행합니다. 복구 가능 기간 내이면 복구하고, 아니면 OAuth 계정을 삭제합니다.
193-218: [LEVEL: low] LGTM - providerId 없이도 OAuth 계정 연동이 가능합니다.
providerId가 없는 경우에도oauthEmail기반으로 계정을 연동할 수 있도록 개선되었습니다. 기존 계정의providerId가 NULL/blank인 경우에만 업데이트하여 충돌을 방지합니다.
src/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.java
Show resolved
Hide resolved
src/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sql
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
user_oauth_account 테이블을 중심으로 OAuth 연동 정보를 분리/정규화하여 users 테이블의 OAuth 관련 컬럼을 제거하고, 로그인/연동/탈퇴 로직이 새 구조를 사용하도록 전환하는 PR입니다.
Changes:
users테이블의 OAuth 컬럼 제거 및user_oauth_account로 마이그레이션/제약조건(유니크) 보강- OAuth 로그인/계정 연동 흐름에서 유저 조회·복구·토큰 저장을
user_oauth_account기반으로 변경 - 도메인 모델/리포지토리/테스트 픽스처에서 제거된 컬럼(provider/providerId/appleRefreshToken) 의존성 정리
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/gg/agit/konect/support/fixture/UserFixture.java | User 생성 픽스처에서 provider/providerId 설정 제거 |
| src/main/resources/db/migration/V43__add_user_oauth_account_table.sql | user_oauth_account 테이블 생성 및 초기 데이터 이관 |
| src/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sql | user_oauth_account.provider_id NULL 허용으로 변경 |
| src/main/resources/db/migration/V46__add_apple_refresh_token_to_user_oauth_account.sql | apple_refresh_token 컬럼 추가 및 데이터 이관/보정 |
| src/main/resources/db/migration/V47__remove_oauth_columns_from_users.sql | users의 OAuth 컬럼 및 관련 유니크 인덱스 제거 |
| src/main/resources/db/migration/V48__add_unique_constraint_provider_oauth_email.sql | (provider, oauth_email) 유니크 추가 및 중복 데이터 정리 |
| src/main/java/gg/agit/konect/infrastructure/oauth/GoogleOAuthServiceImpl.java | OAuth 등록 유저 판별을 oauth_email 기반으로 변경 |
| src/main/java/gg/agit/konect/infrastructure/oauth/KakaoOAuthServiceImpl.java | OAuth 등록 유저 판별을 oauth_email 기반으로 변경 |
| src/main/java/gg/agit/konect/infrastructure/oauth/NaverOAuthServiceImpl.java | OAuth 등록 유저 판별을 oauth_email 기반으로 변경 |
| src/main/java/gg/agit/konect/infrastructure/oauth/AppleOAuthServiceImpl.java | providerId/email 존재 여부 확인을 user_oauth_account 기반으로 변경 |
| src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java | 로그인 시 계정 복구/정리/연동 보장 로직을 user_oauth_account 중심으로 변경 |
| src/main/java/gg/agit/konect/global/auth/oauth/OAuth2LoginSuccessHandler.java | Apple refresh token 저장 및 link-mode 연동 플로우를 user_oauth_account 기반으로 변경 |
| src/main/java/gg/agit/konect/domain/user/service/UserService.java | 회원가입/탈퇴 시 OAuth 계정 연결 및 Apple 토큰 revoke 처리 변경 |
| src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java | 연동 상태 조회/링크 로직을 user_oauth_account 중심으로 변경 및 refresh token 저장 지원 |
| src/main/java/gg/agit/konect/domain/user/repository/UserRepository.java | provider/providerId 기반 조회 제거 및 최소 API로 정리 |
| src/main/java/gg/agit/konect/domain/user/repository/UserOAuthAccountRepository.java | oauthEmail 기반 조회/정렬/존재확인 쿼리 추가 |
| src/main/java/gg/agit/konect/domain/user/model/UserOAuthAccount.java | providerId nullable 및 appleRefreshToken 필드/팩토리 확장 |
| src/main/java/gg/agit/konect/domain/user/model/User.java | OAuth 필드 제거 및 oauthAccounts 연관관계 추가 |
| src/main/java/gg/agit/konect/domain/user/controller/UserController.java | OAuth 링크 API에서 서비스 시그니처 변경 반영 |
Comments suppressed due to low confidence (2)
src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java:97
restoreWithdrawnByProviderId는 현재 클래스 내에서 호출되지 않는(검색 결과 단일 정의만 존재) 미사용 메서드입니다. 동작이restoreOrCleanupWithdrawnByLinkedProvider로 대체된 것으로 보이므로, 혼동을 줄이기 위해 메서드를 제거하거나 실제 사용하도록 정리해 주세요.
private Optional<User> restoreWithdrawnByProviderId(Provider provider, String providerId) {
if (isStageProfile()) {
return Optional.empty();
}
return userOAuthAccountRepository
.findAccountByProviderAndProviderId(provider, providerId)
.filter(account -> account.getUser().getDeletedAt() != null)
.map(UserOAuthAccount::getUser)
.filter(user -> user.canRestore(LocalDateTime.now(), RESTORE_WINDOW_DAYS))
.map(user -> {
user.restore();
return userRepository.save(user);
});
}
src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java:115
restoreWithdrawnByEmail도 현재 클래스 내에서 호출되지 않는 미사용 메서드입니다.restoreOrCleanupWithdrawnByOauthEmail로 로직이 이동한 상태라면 중복/사이드이펙트 방지를 위해 삭제하거나 호출부를 명확히 해 주세요.
private Optional<User> restoreWithdrawnByEmail(Provider provider, String email) {
if (isStageProfile()) {
return Optional.empty();
}
if (!StringUtils.hasText(email)) {
return Optional.empty();
}
return userOAuthAccountRepository.findUserByOauthEmailAndProvider(email, provider)
.filter(user -> user.getDeletedAt() != null)
.filter(user -> user.canRestore(LocalDateTime.now(), RESTORE_WINDOW_DAYS))
.map(user -> {
user.restore();
return userRepository.save(user);
});
}
src/main/java/gg/agit/konect/domain/user/service/UserOAuthAccountService.java
Show resolved
Hide resolved
src/main/resources/db/migration/V45__make_provider_id_nullable_in_users.sql
Show resolved
Hide resolved
src/main/java/gg/agit/konect/domain/user/repository/UserRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/gg/agit/konect/global/auth/oauth/OAuthLoginHelper.java
Outdated
Show resolved
Hide resolved
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔍 개요
user_oauth_account테이블로 인해 사용자에 대한 OAuth 연동 상태를 명확히 관리할 수 있게 되어 유저 테이블을 정리한다.🚀 주요 변경 내용
users테이블에 있는 OAuth 관련 컬럼provider,provider_id,apple_refresh_token을user_oauth_account테이블로 옮겼습니다.(provider, provider_id)유니크와(user_id, provider)유니크를 동시에 적용하여 한 OAuth 계정은 한 사용자에게만, 한 사용자는 provider별 1개 연동만 가능하도록 제한했습니다.💬 참고 사항
✅ Checklist (완료 조건)