feat: 회원 탈퇴 시 소셜 연동 해제(unlink) 지원#99
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughOAuth2 로그인 성공 시 제공자 토큰을 저장하고, 탈퇴 시 저장된 토큰으로 소셜 연동 해제를 먼저 수행한 뒤 소셜 계정과 제공자 토큰을 삭제하도록 흐름이 확장됐다. Changes소셜 토큰 저장과 탈퇴 정리
Sequence Diagram(s)OAuth2 성공 시 제공자 토큰 저장sequenceDiagram
participant OAuth2SuccessHandler
participant OAuth2ProviderTokenService
participant OAuth2AuthorizedClientService
participant SocialProviderTokenRepository
OAuth2SuccessHandler->>OAuth2ProviderTokenService: saveProviderToken(authentication, userId)
OAuth2ProviderTokenService->>OAuth2AuthorizedClientService: loadAuthorizedClient(registrationId, principalName)
OAuth2AuthorizedClientService-->>OAuth2ProviderTokenService: OAuth2AuthorizedClient
OAuth2ProviderTokenService->>SocialProviderTokenRepository: save(userId, provider, SocialProviderToken)
탈퇴 시 소셜 연동 해제와 정리sequenceDiagram
participant AccountWithdrawalService
participant SocialAccountUnlinkService
participant SocialAccountReader
participant SocialProviderTokenRepository
participant SocialAccountUnlinkClient
AccountWithdrawalService->>SocialAccountUnlinkService: unlinkAll(userId)
SocialAccountUnlinkService->>SocialAccountReader: findAllByUserId(userId)
loop each SocialAccount
SocialAccountUnlinkService->>SocialProviderTokenRepository: find(userId, provider)
alt client and token exist
SocialAccountUnlinkService->>SocialAccountUnlinkClient: unlink(socialAccount, token)
SocialAccountUnlinkClient-->>SocialAccountUnlinkService: completed
else missing client or token
SocialAccountUnlinkService-->>SocialAccountUnlinkService: skip
end
end
AccountWithdrawalService->>SocialProviderTokenRepository: deleteAllByUserId(userId)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 8
🧹 Nitpick comments (1)
src/main/java/com/zimdugo/auth/application/SocialAccountUnlinkService.java (1)
30-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win동일 provider 클라이언트를 조용히 덮어쓰지 마세요.
같은
AuthProvider구현체가 둘 이상 주입되면 마지막 빈이 앞선 빈을 덮어써서 잘못된 unlink 클라이언트가 선택될 수 있습니다.put()의 반환값을 확인해서 중복 등록 시 바로 실패시키는 편이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/zimdugo/auth/application/SocialAccountUnlinkService.java` around lines 30 - 33, The SocialAccountUnlinkService constructor currently builds the unlinkClients map by calling put() for each SocialAccountUnlinkClient, which can silently overwrite an existing entry when the same AuthProvider appears more than once. Update the constructor logic to detect duplicate provider registration while iterating over unlinkClients, using the put() return value in SocialAccountUnlinkService and failing immediately when a provider is already present so the wrong client cannot be selected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/zimdugo/auth/application/AccountWithdrawalService.java`:
- Around line 47-53: Move the side-effecting cleanup in AccountWithdrawalService
out of the DB transaction: unlinkAll() on socialAccountUnlinkService and
deleteAllByUserId() on socialProviderTokenRepository should run only after the
userStore/store and socialAccountStore changes are successfully committed.
Refactor the withdrawal flow so the transactional part only persists the
anonymized user and local DB state, then trigger the external unlink and
Redis/token cleanup via an after-commit event or outbox-based handler.
In `@src/main/java/com/zimdugo/auth/entrypoint/oauth2/OAuth2SuccessHandler.java`:
- Line 70: The provider token persistence in OAuth2 success flow should not
block login completion when it fails. Update the OAuth2 success handler around
saveProviderToken so failures from loadAuthorizedClient, AuthProvider.valueOf,
or Redis storage are caught and logged as warn, then continue with session
creation and the rest of the success path. Keep the failure isolated inside the
token-saving path in OAuth2SuccessHandler so missing provider tokens do not stop
authentication success.
In
`@src/main/java/com/zimdugo/auth/infrastructure/client/GoogleSocialAccountUnlinkClient.java`:
- Around line 32-38: The Google social unlink revocation request is sending the
token in the URL query string via GoogleSocialAccountUnlinkClient and should
instead send it in the POST body. Update the revoke call in the client to submit
`token` as `application/x-www-form-urlencoded` content rather than using
`uriBuilder.queryParam`, keeping the `/revoke` endpoint and the token resolution
logic in `resolveRevocationToken` intact.
In
`@src/main/java/com/zimdugo/auth/infrastructure/client/KakaoSocialAccountUnlinkClient.java`:
- Around line 49-59: 외부 unlink 실패가 `SocialAccountUnlinkService`의 내부 탈퇴 흐름을 중단시키고
있습니다. `KakaoSocialAccountUnlinkClient.unlink(...)`에서 던지는 `BusinessException`을
호출자에서 그대로 전파하지 말고, provider별 호출부에서 try/catch로 격리해 실패해도 사용자 익명화, 소셜 계정 삭제,
provider token 삭제는 계속 진행되도록 `SocialAccountUnlinkService`와 관련 호출 흐름을 수정하세요.
`unlinkClient.unlink(...)`, `SocialAccountUnlinkService`,
`KakaoSocialAccountUnlinkClient.unlink(...)`를 기준으로 실패를 기록만 하고 다음 단계로 넘어가거나, 실패
결과를 반환하는 계약으로 변경해 내부 탈퇴가 중단되지 않게 하세요.
In
`@src/main/java/com/zimdugo/auth/infrastructure/client/NaverSocialAccountUnlinkClient.java`:
- Around line 43-59: 외부 unlink 실패가 내부 탈퇴를 막고 있으므로, SocialAccountUnlinkService에서
unlinkClient.unlink(...) 호출을 provider별로 try/catch로 감싸 BusinessException이 전파되지 않게
격리하세요. NaverSocialAccountUnlinkClient.unlink에서 발생하는 실패는 로그만 남기고, 이어서 사용자 익명화, 소셜
계정 삭제, provider token 삭제가 계속 실행되도록 흐름을 유지해야 합니다. SocialAccountUnlinkService의 각
provider 분기와 unlinkClient, BusinessException 처리 지점을 함께 수정해 주세요.
- Around line 45-52: The Naver unlink/token request currently exposes sensitive
credentials by putting client_secret and tokens into the URL query in
NaverSocialAccountUnlinkClient. Update the request-building logic in the
unlink/token flow to send client_secret, access_token, and refresh_token in an
application/x-www-form-urlencoded POST body instead of uriBuilder.queryParam,
keeping the existing request methods and token-resolving helpers but moving the
payload construction into the body handling.
In
`@src/main/java/com/zimdugo/auth/infrastructure/persistence/RedisSocialProviderTokenRepository.java`:
- Around line 25-28: The RedisSocialProviderTokenRepository.save method is
storing SocialProviderToken without any retention limit, so provider credentials
can remain indefinitely. Update save to apply an appropriate TTL when calling
stringRedisTemplate.opsForValue().set for the key(userId, provider), or encrypt
the serialized token before persisting if TTL cannot be used. Keep the fix
localized to RedisSocialProviderTokenRepository and its save/key/serialize flow.
- Around line 39-46: The deleteAllByUserId method in
RedisSocialProviderTokenRepository currently uses stringRedisTemplate.keys(),
which blocks Redis on large keyspaces; replace it with a cursor-based SCAN flow
that iterates matching keys by KEY_PREFIX + userId + ":*" and deletes them in
batches. Apply the same pattern in RedisRefreshTokenRepository as well so both
repositories avoid KEYS and use non-blocking key iteration.
---
Nitpick comments:
In `@src/main/java/com/zimdugo/auth/application/SocialAccountUnlinkService.java`:
- Around line 30-33: The SocialAccountUnlinkService constructor currently builds
the unlinkClients map by calling put() for each SocialAccountUnlinkClient, which
can silently overwrite an existing entry when the same AuthProvider appears more
than once. Update the constructor logic to detect duplicate provider
registration while iterating over unlinkClients, using the put() return value in
SocialAccountUnlinkService and failing immediately when a provider is already
present so the wrong client cannot be selected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 450bf4e4-4f4e-4909-8da4-88e9754e9069
📒 Files selected for processing (10)
src/main/java/com/zimdugo/auth/application/AccountWithdrawalService.javasrc/main/java/com/zimdugo/auth/application/SocialAccountUnlinkService.javasrc/main/java/com/zimdugo/auth/domain/SocialAccountUnlinkClient.javasrc/main/java/com/zimdugo/auth/domain/SocialProviderToken.javasrc/main/java/com/zimdugo/auth/domain/SocialProviderTokenRepository.javasrc/main/java/com/zimdugo/auth/entrypoint/oauth2/OAuth2SuccessHandler.javasrc/main/java/com/zimdugo/auth/infrastructure/client/GoogleSocialAccountUnlinkClient.javasrc/main/java/com/zimdugo/auth/infrastructure/client/KakaoSocialAccountUnlinkClient.javasrc/main/java/com/zimdugo/auth/infrastructure/client/NaverSocialAccountUnlinkClient.javasrc/main/java/com/zimdugo/auth/infrastructure/persistence/RedisSocialProviderTokenRepository.java
Comment on lines
+47
to
+53
| socialAccountUnlinkService.unlinkAll(userId); | ||
|
|
||
| user.anonymizeForWithdrawal(); | ||
| userStore.store(user); | ||
|
|
||
| socialAccountStore.deleteAllByUserId(userId); | ||
| socialProviderTokenRepository.deleteAllByUserId(userId); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
외부 unlink와 Redis 삭제를 DB 트랜잭션 안에서 실행하면 부분 실패가 남습니다.
unlinkAll()은 외부 side effect이고 socialProviderTokenRepository.deleteAllByUserId()도 DB 트랜잭션에 묶이지 않는 삭제입니다. 지금 순서에서는 뒤쪽 userStore/socialAccountStore/커밋이 실패해도 외부 연동 해제나 Redis 토큰 삭제만 먼저 확정될 수 있어서, 내부 탈퇴 상태와 외부 연동 상태가 어긋납니다. 이 단계는 after-commit 이벤트나 아웃박스처럼 커밋 이후에 실행되도록 분리하는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/zimdugo/auth/application/AccountWithdrawalService.java`
around lines 47 - 53, Move the side-effecting cleanup in
AccountWithdrawalService out of the DB transaction: unlinkAll() on
socialAccountUnlinkService and deleteAllByUserId() on
socialProviderTokenRepository should run only after the userStore/store and
socialAccountStore changes are successfully committed. Refactor the withdrawal
flow so the transactional part only persists the anonymized user and local DB
state, then trigger the external unlink and Redis/token cleanup via an
after-commit event or outbox-based handler.
Comment on lines
+49
to
+59
| public void unlink(SocialAccount socialAccount, SocialProviderToken token) { | ||
| try { | ||
| apiRestClient.post() | ||
| .uri("/v1/user/unlink") | ||
| .header(HttpHeaders.AUTHORIZATION, "Bearer " + resolveAccessToken(token)) | ||
| .retrieve() | ||
| .toBodilessEntity(); | ||
| } catch (RestClientException exception) { | ||
| log.error("카카오 연동 해제에 실패했습니다. userId={}", socialAccount.getUser().getId(), exception); | ||
| throw new BusinessException(ErrorCode.EXTERNAL_API_ERROR, exception); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
외부 unlink 실패가 내부 탈퇴를 중단하지 않게 격리하세요.
제공된 SocialAccountUnlinkService 흐름은 unlinkClient.unlink(...) 예외를 잡지 않으므로, Line 58/67/84의 BusinessException이 전파되면 사용자 익명화, 소셜 계정 삭제, provider token 삭제가 실행되지 않습니다. PR 목표처럼 내부 탈퇴는 계속되어야 한다면 caller에서 provider별 try/catch로 계속 진행시키거나 실패 결과를 반환하는 계약으로 바꿔 주세요.
Also applies to: 62-87
🧰 Tools
🪛 PMD (7.25.0)
[Low] 57-57: InvalidLogMessageFormat (Error Prone): Too many arguments, expected 1 argument but found 2
(InvalidLogMessageFormat (Error Prone))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/zimdugo/auth/infrastructure/client/KakaoSocialAccountUnlinkClient.java`
around lines 49 - 59, 외부 unlink 실패가 `SocialAccountUnlinkService`의 내부 탈퇴 흐름을
중단시키고 있습니다. `KakaoSocialAccountUnlinkClient.unlink(...)`에서 던지는
`BusinessException`을 호출자에서 그대로 전파하지 말고, provider별 호출부에서 try/catch로 격리해 실패해도 사용자
익명화, 소셜 계정 삭제, provider token 삭제는 계속 진행되도록 `SocialAccountUnlinkService`와 관련 호출
흐름을 수정하세요. `unlinkClient.unlink(...)`, `SocialAccountUnlinkService`,
`KakaoSocialAccountUnlinkClient.unlink(...)`를 기준으로 실패를 기록만 하고 다음 단계로 넘어가거나, 실패
결과를 반환하는 계약으로 변경해 내부 탈퇴가 중단되지 않게 하세요.
Comment on lines
+43
to
+59
| public void unlink(SocialAccount socialAccount, SocialProviderToken token) { | ||
| try { | ||
| restClient.get() | ||
| .uri(uriBuilder -> uriBuilder | ||
| .path("/oauth2.0/token") | ||
| .queryParam("grant_type", "delete") | ||
| .queryParam("client_id", clientId) | ||
| .queryParam("client_secret", clientSecret) | ||
| .queryParam("access_token", resolveAccessToken(token)) | ||
| .queryParam("service_provider", "NAVER") | ||
| .build()) | ||
| .retrieve() | ||
| .body(NaverTokenResponse.class); | ||
| } catch (RestClientException exception) { | ||
| log.error("네이버 연동 해제에 실패했습니다. userId={}", socialAccount.getUser().getId(), exception); | ||
| throw new BusinessException(ErrorCode.EXTERNAL_API_ERROR, exception); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
외부 unlink 실패가 내부 탈퇴를 중단하지 않게 격리하세요.
제공된 SocialAccountUnlinkService 흐름은 unlinkClient.unlink(...) 예외를 잡지 않으므로, Line 58/67/82의 BusinessException이 전파되면 사용자 익명화, 소셜 계정 삭제, provider token 삭제가 실행되지 않습니다. 내부 탈퇴는 계속 진행되도록 caller에서 provider별 try/catch로 격리해 주세요.
Also applies to: 66-82
🧰 Tools
🪛 PMD (7.25.0)
[Low] 57-57: InvalidLogMessageFormat (Error Prone): Too many arguments, expected 1 argument but found 2
(InvalidLogMessageFormat (Error Prone))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/zimdugo/auth/infrastructure/client/NaverSocialAccountUnlinkClient.java`
around lines 43 - 59, 외부 unlink 실패가 내부 탈퇴를 막고 있으므로, SocialAccountUnlinkService에서
unlinkClient.unlink(...) 호출을 provider별로 try/catch로 감싸 BusinessException이 전파되지 않게
격리하세요. NaverSocialAccountUnlinkClient.unlink에서 발생하는 실패는 로그만 남기고, 이어서 사용자 익명화, 소셜
계정 삭제, provider token 삭제가 계속 실행되도록 흐름을 유지해야 합니다. SocialAccountUnlinkService의 각
provider 분기와 unlinkClient, BusinessException 처리 지점을 함께 수정해 주세요.
Comment on lines
+25
to
+28
| @Override | ||
| public void save(Long userId, AuthProvider provider, SocialProviderToken token) { | ||
| stringRedisTemplate.opsForValue().set(key(userId, provider), serialize(token)); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Provider 토큰에 보존 한계를 두세요.
Line 27은 access/refresh token을 TTL 없이 JSON으로 저장합니다. 탈퇴 전까지 장기 휴면 계정의 bearer credential이 계속 남을 수 있으니, provider refresh token 유효기간/보존 정책에 맞춘 TTL 또는 저장 전 암호화를 적용해 주세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/zimdugo/auth/infrastructure/persistence/RedisSocialProviderTokenRepository.java`
around lines 25 - 28, The RedisSocialProviderTokenRepository.save method is
storing SocialProviderToken without any retention limit, so provider credentials
can remain indefinitely. Update save to apply an appropriate TTL when calling
stringRedisTemplate.opsForValue().set for the key(userId, provider), or encrypt
the serialized token before persisting if TTL cannot be used. Keep the fix
localized to RedisSocialProviderTokenRepository and its save/key/serialize flow.
Comment on lines
+39
to
+46
| @Override | ||
| public void deleteAllByUserId(Long userId) { | ||
| Set<String> keys = stringRedisTemplate.keys(KEY_PREFIX + userId + ":*"); | ||
| if (keys == null || keys.isEmpty()) { | ||
| return; | ||
| } | ||
| stringRedisTemplate.delete(keys); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
sed -n '1,220p' src/main/java/com/zimdugo/auth/infrastructure/persistence/RedisSocialProviderTokenRepository.javaRepository: JECT-Study/Zimdugo-4th-Server
Length of output: 2597
🏁 Script executed:
sed -n '1,220p' src/main/java/com/zimdugo/auth/infrastructure/persistence/RedisSocialProviderTokenRepository.javaRepository: JECT-Study/Zimdugo-4th-Server
Length of output: 2597
🏁 Script executed:
sed -n '1,220p' src/main/java/com/zimdugo/auth/infrastructure/persistence/RedisSocialProviderTokenRepository.javaRepository: JECT-Study/Zimdugo-4th-Server
Length of output: 2597
🏁 Script executed:
sed -n '1,220p' src/main/java/com/zimdugo/auth/infrastructure/persistence/RedisSocialProviderTokenRepository.javaRepository: JECT-Study/Zimdugo-4th-Server
Length of output: 2597
🏁 Script executed:
sed -n '1,220p' src/main/java/com/zimdugo/auth/infrastructure/persistence/RedisSocialProviderTokenRepository.javaRepository: JECT-Study/Zimdugo-4th-Server
Length of output: 2597
🏁 Script executed:
rg -n "deleteAllByUserId\\(" -S src/main/javaRepository: JECT-Study/Zimdugo-4th-Server
Length of output: 1612
🏁 Script executed:
rg -n "deleteAllByUserId\\(" -S src/main/javaRepository: JECT-Study/Zimdugo-4th-Server
Length of output: 1612
🏁 Script executed:
rg -n "deleteAllByUserId\\(" -S src/main/javaRepository: JECT-Study/Zimdugo-4th-Server
Length of output: 1612
🏁 Script executed:
sed -n '1,220p' src/main/java/com/zimdugo/auth/infrastructure/persistence/RedisRefreshTokenRepository.javaRepository: JECT-Study/Zimdugo-4th-Server
Length of output: 2604
🏁 Script executed:
sed -n '1,220p' src/main/java/com/zimdugo/auth/infrastructure/persistence/RedisRefreshTokenRepository.javaRepository: JECT-Study/Zimdugo-4th-Server
Length of output: 2604
🏁 Script executed:
sed -n '1,220p' src/main/java/com/zimdugo/auth/infrastructure/persistence/RedisRefreshTokenRepository.javaRepository: JECT-Study/Zimdugo-4th-Server
Length of output: 2604
🏁 Script executed:
sed -n '1,220p' src/main/java/com/zimdugo/auth/infrastructure/persistence/RedisRefreshTokenRepository.javaRepository: JECT-Study/Zimdugo-4th-Server
Length of output: 2604
KEYS 대신 cursor 기반 SCAN으로 바꾸세요.
stringRedisTemplate.keys(...)는 Redis 전체 keyspace를 블로킹하므로, 탈퇴 요청이 많거나 키가 늘면 응답 지연을 유발할 수 있습니다. 같은 패턴이 RedisRefreshTokenRepository에도 있으니 함께 교체하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/zimdugo/auth/infrastructure/persistence/RedisSocialProviderTokenRepository.java`
around lines 39 - 46, The deleteAllByUserId method in
RedisSocialProviderTokenRepository currently uses stringRedisTemplate.keys(),
which blocks Redis on large keyspaces; replace it with a cursor-based SCAN flow
that iterates matching keys by KEY_PREFIX + userId + ":*" and deletes them in
batches. Apply the same pattern in RedisRefreshTokenRepository as well so both
repositories avoid KEYS and use non-blocking key iteration.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/java/com/zimdugo/auth/entrypoint/oauth2/OAuth2SuccessHandlerTest.java (1)
51-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
saveProviderToken호출 검증을 추가하면 좋습니다.새로 주입한
providerTokenService가 mock으로만 전달되고 실제 호출 여부는 검증되지 않습니다. 토큰 저장이 성공 경로에서 호출되는지 확인하는verify(providerTokenService).saveProviderToken(any(), eq(42L));를 추가하면 회귀를 방지할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/zimdugo/auth/entrypoint/oauth2/OAuth2SuccessHandlerTest.java` around lines 51 - 61, The success-path test for OAuth2 authentication is missing verification that the newly injected providerTokenService is actually used. Update OAuth2SuccessHandlerTest to assert that providerTokenService.saveProviderToken is invoked during handler.onAuthenticationSuccess, alongside the existing sessionService and callbackUrlCookieManager checks, so the test covers the token-save behavior for the authenticated principal.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/zimdugo/auth/application/OAuth2ProviderTokenService.java`:
- Around line 43-51: `OAuth2ProviderTokenService`에서
`AuthProvider.valueOf(registrationId.toUpperCase())`가 알 수 없는 registrationId에 대해
예외를 던져 로그인 흐름을 중단할 수 있습니다. `save` 호출 직전에 `registrationId`를 안전하게 검증하거나 enum 매칭
실패를 처리해, 매칭되지 않으면 토큰 저장만 건너뛰고 예외를 밖으로 전파하지 않도록 `save`, `AuthProvider.valueOf`,
`OAuth2ProviderTokenService` 흐름을 수정하세요.
---
Nitpick comments:
In
`@src/test/java/com/zimdugo/auth/entrypoint/oauth2/OAuth2SuccessHandlerTest.java`:
- Around line 51-61: The success-path test for OAuth2 authentication is missing
verification that the newly injected providerTokenService is actually used.
Update OAuth2SuccessHandlerTest to assert that
providerTokenService.saveProviderToken is invoked during
handler.onAuthenticationSuccess, alongside the existing sessionService and
callbackUrlCookieManager checks, so the test covers the token-save behavior for
the authenticated principal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a18ba064-f463-45a2-bc54-492c671db427
📒 Files selected for processing (6)
src/main/java/com/zimdugo/auth/application/OAuth2ProviderTokenService.javasrc/main/java/com/zimdugo/auth/application/SocialAccountUnlinkService.javasrc/main/java/com/zimdugo/auth/entrypoint/oauth2/OAuth2SuccessHandler.javasrc/main/java/com/zimdugo/auth/infrastructure/client/GoogleSocialAccountUnlinkClient.javasrc/main/java/com/zimdugo/auth/infrastructure/client/NaverSocialAccountUnlinkClient.javasrc/test/java/com/zimdugo/auth/entrypoint/oauth2/OAuth2SuccessHandlerTest.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/zimdugo/auth/infrastructure/client/GoogleSocialAccountUnlinkClient.java
- src/main/java/com/zimdugo/auth/application/SocialAccountUnlinkService.java
- src/main/java/com/zimdugo/auth/infrastructure/client/NaverSocialAccountUnlinkClient.java
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
1 participant
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.
✨ 작업 내용 한 줄 요약
🛠️ 작업 내용
🧠 기술적 의사결정
📚 참고 자료 (선택)
📷 스크린샷 (선택)
Summary by CodeRabbit