Repository navigation
[Feat/#600] 애플 계정 탈퇴 후 특정 기간 경과 시 Apple 서버 연동 해제 구현 - #601
Conversation
📝 WalkthroughWalkthroughApple authentication now exchanges authorization codes for tokens and stores Apple refresh tokens in Redis. Expired-user cleanup revokes those tokens through Apple and removes them. Withdrawal documentation reflects Apple disconnection after seven days. ChangesApple token lifecycle
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AuthCallback
participant AppleAuthService
participant AppleTokenEndpoint
participant Redis
AuthCallback->>AppleAuthService: submit identity token, user data, and code
AppleAuthService->>AppleAuthService: generateAppleClientSecret()
AppleAuthService->>AppleTokenEndpoint: exchange code
AppleTokenEndpoint-->>AppleAuthService: return refresh token
AppleAuthService->>Redis: store refresh token with retention TTL
sequenceDiagram
participant ExpiredUserCleanup
participant Redis
participant AppleAuthService
participant AppleRevokeEndpoint
ExpiredUserCleanup->>Redis: retrieve Apple refresh token
ExpiredUserCleanup->>AppleAuthService: generateAppleClientSecret()
ExpiredUserCleanup->>AppleRevokeEndpoint: revoke refresh token
AppleRevokeEndpoint-->>ExpiredUserCleanup: return revocation result
ExpiredUserCleanup->>Redis: delete token after success
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (2)
src/services/auth.service.js (2)
190-192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the full Apple error response for easier debugging.
The Kakao unlink failure above logs
err.response?.data ?? err.message(Line 168), but the Apple revoke failure only logserr.message(Line 191). Apple's error body (for exampleinvalid_grant) is more useful for diagnosing recurring revoke failures than the generic axios message.🩹 Proposed fix
} catch(err) { - console.warn(`애플 연동 해제 실패 (userId: ${user.id})`, err.message); + console.warn(`애플 연동 해제 실패 (userId: ${user.id})`, err.response?.data ?? err.message); }🤖 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/services/auth.service.js` around lines 190 - 192, Update the Apple revoke failure handler in the catch block to log the full error response body when available, falling back to err.message otherwise, matching the existing Kakao unlink logging behavior while preserving the user ID context.
171-195: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid regenerating the Apple client secret per user in the cleanup loop.
new AppleAuthService()andgenerateAppleClientSecret()run once per Apple user found inusers. The client secret does not depend on the user, so this repeats the ES256 signing work needlessly for every user in the same cleanup run. Generate it once perdeleteExpiredUsersinvocation instead.♻️ Proposed refactor: generate the client secret once per run
const tokenKeysMap = new Map(); let cursor = "0"; + let appleClientSecretPromise = null; + const getAppleClientSecret = () => { + if (!appleClientSecretPromise) { + appleClientSecretPromise = new AppleAuthService().generateAppleClientSecret(); + } + return appleClientSecretPromise; + }; do {if (appleRefreshToken) { try { - const appleService = new AppleAuthService(); - const clientSecret = await appleService.generateAppleClientSecret(); + const clientSecret = await getAppleClientSecret(); await axios.post(🤖 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/services/auth.service.js` around lines 171 - 195, Update deleteExpiredUsers to generate the Apple client secret once before iterating through users, then reuse that value in the Apple token revocation logic instead of constructing AppleAuthService and calling generateAppleClientSecret for each user. Keep the per-user revocation and cleanup behavior unchanged.
🤖 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/services/auth.service.js`:
- Around line 171-195: Update the Apple withdrawal flow in withdrawAppleUser to
reset the Redis key’s TTL when the user withdraws, using the configured
deletion-window duration so the token remains available to the cleanup block
that revokes it. Ensure the related token-storage logic does not expire the key
before that post-withdrawal cleanup window; preserve deletion after successful
revocation.
---
Nitpick comments:
In `@src/services/auth.service.js`:
- Around line 190-192: Update the Apple revoke failure handler in the catch
block to log the full error response body when available, falling back to
err.message otherwise, matching the existing Kakao unlink logging behavior while
preserving the user ID context.
- Around line 171-195: Update deleteExpiredUsers to generate the Apple client
secret once before iterating through users, then reuse that value in the Apple
token revocation logic instead of constructing AppleAuthService and calling
generateAppleClientSecret for each user. Keep the per-user revocation and
cleanup behavior unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 69be9be7-0857-45cc-9c69-51e5c47f219a
📒 Files selected for processing (3)
src/controllers/auth.controller.jssrc/services/auth.service.jssrc/swagger/swagger.yml
yangwooyoung123
changed the title
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 (4)
src/services/auth.service.js (4)
528-545: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not complete login when refresh-token capture fails.
The
catchlogs exchange or Redis failures and allows a successful session to return. A response withoutappleTokens.refresh_tokenalso skips storage without an error. The cleanup job then has no token to revoke. Apple’s authorization-code response includes a refresh token, and the refresh token is required for the revoke flow. (developer.apple.com)Treat a missing token or Redis write failure as an explicit failure. Alternatively, persist a durable retry state before marking the login complete.
🤖 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/services/auth.service.js` around lines 528 - 545, Update the login flow around exchangeCodeForTokens and the redis.set call so a missing appleTokens.refresh_token or any token-storage failure is treated as an explicit login failure. Do not continue to successful session completion after the catch; propagate or return the established failure response, ensuring the login is not marked complete without a stored refresh token.
528-545: 🗄️ Data Integrity & Integration | 🟠 MajorRetain the token until the withdrawal window completes.
This change resets the TTL only when the key still exists. Line 535-540 still starts the TTL at login, so a user who withdraws after
(DELETE_AFTER_DAYS + 1)days without logging in loses the key before Line 570 can reset it. Cleanup then cannot revoke the Apple authorization. The separateGETandEXPIREcalls also leave a race if the key expires between them. Keep the token through the full pre-withdrawal period or until successful revocation, and make the reset outcome observable. Apple requires a valid token for revocation. (developer.apple.com)Also applies to: 569-577
🤖 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/services/auth.service.js` around lines 528 - 545, The Apple refresh-token persistence in the login flow expires too early and uses a race-prone TTL reset during withdrawal cleanup. Update the token storage around exchangeCodeForTokens and the withdrawal cleanup flow to retain the token through the complete pre-withdrawal window, then atomically preserve or renew it until revocation succeeds and remove it afterward. Make the Redis renewal result observable and ensure cleanup can still access a valid token for Apple revocation.
171-195: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd bounded timeouts to both Apple HTTP requests.
Neither
auth/tokennorauth/revokehas a request-level timeout, and no shared Axios timeout exists. Set a finitetimeoutand handle timeout errors as retryable failures.🤖 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/services/auth.service.js` around lines 171 - 195, Add a finite Axios request timeout to both Apple `auth/token` and `auth/revoke` calls, including the revoke request in the Apple disconnect flow and the corresponding token request in its existing implementation. Reuse a shared timeout value where available, and classify timeout errors as retryable while preserving the current error handling behavior for other failures.
171-195: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftKeep failed Apple revocations retryable before deleting the user.
The daily cleanup cron logs the failure and still deletes
user. The Apple refresh token can remain active for the remaining Redis TTL, but no user record remains for a retry. Persist a retryable revocation state or job, and delete the user only after successful revocation or a confirmed terminal state.🤖 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/services/auth.service.js` around lines 171 - 195, Update the Apple unlink flow around AppleAuthService, the Apple revoke request, and user deletion so a revocation failure persists a retryable state or job instead of allowing user deletion to continue. Delete the user only after successful revocation or an explicitly confirmed terminal outcome, while preserving the existing Redis token for retries.
🤖 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/services/auth.service.js`:
- Around line 569-577: Update the withdrawal flow in the surrounding
user-withdrawal method so Redis failures after prisma.user.update do not leave
recovery impossible: make the apple refresh-token TTL restoration idempotently
retryable or enqueue a durable revocation job before returning failure. Preserve
the committed deletedAt state and ensure retries can restore the token TTL
without being blocked by ALREADY_WITHDRAWN.
---
Outside diff comments:
In `@src/services/auth.service.js`:
- Around line 528-545: Update the login flow around exchangeCodeForTokens and
the redis.set call so a missing appleTokens.refresh_token or any token-storage
failure is treated as an explicit login failure. Do not continue to successful
session completion after the catch; propagate or return the established failure
response, ensuring the login is not marked complete without a stored refresh
token.
- Around line 528-545: The Apple refresh-token persistence in the login flow
expires too early and uses a race-prone TTL reset during withdrawal cleanup.
Update the token storage around exchangeCodeForTokens and the withdrawal cleanup
flow to retain the token through the complete pre-withdrawal window, then
atomically preserve or renew it until revocation succeeds and remove it
afterward. Make the Redis renewal result observable and ensure cleanup can still
access a valid token for Apple revocation.
- Around line 171-195: Add a finite Axios request timeout to both Apple
`auth/token` and `auth/revoke` calls, including the revoke request in the Apple
disconnect flow and the corresponding token request in its existing
implementation. Reuse a shared timeout value where available, and classify
timeout errors as retryable while preserving the current error handling behavior
for other failures.
- Around line 171-195: Update the Apple unlink flow around AppleAuthService, the
Apple revoke request, and user deletion so a revocation failure persists a
retryable state or job instead of allowing user deletion to continue. Delete the
user only after successful revocation or an explicitly confirmed terminal
outcome, while preserving the existing Redis token for retries.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 58bc02ee-6d60-4406-b975-542b2ad3183b
📒 Files selected for processing (1)
src/services/auth.service.js
yangwooyoung123
changed the title
📌 관련 이슈번호
(Closes 키워드가 있어야 PR이 머지되었을 때 이슈가 자동으로 닫힌다)
📌 PR 유형
어떤 변경 사항이 있나요?
📌 PR 요약
애플 계정 탈퇴 후 특정 기간 경과 시 Apple 서버 연동 해제 구현
📌 작업 세부 내용
📸 스크린샷 (선택)
🔗 참고 자료
Summary by CodeRabbit
New Features
Bug Fixes
Documentation