[#251] 여러 계정을 링크한 이후 각 공급자에 대한 실질적인 회원탈퇴가 이루어지지 않는 이슈를 해결한다#252
[#251] 여러 계정을 링크한 이후 각 공급자에 대한 실질적인 회원탈퇴가 이루어지지 않는 이슈를 해결한다#252
Conversation
opficdev
commented
Mar 11, 2026
- closed 여러 계정을 링크한 이후 각 공급자에 대한 실질적인 회원탈퇴가 이루어지지 않는 이슈를 해결한다 #251
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 여러 소셜 로그인 계정이 연결된 사용자가 회원 탈퇴를 할 때, 각 연결된 제공자에 대한 실제적인 탈퇴 처리가 누락되던 문제를 해결합니다. 기존에는 각 소셜 로그인 서비스가 개별적으로 탈퇴 관련 로직을 처리했으나, 이제는 Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
이 PR은 여러 소셜 계정 연동 시 회원 탈퇴가 제대로 이루어지지 않던 문제를 해결하며, AuthenticationRepositoryImpl의 delete 메서드를 수정하여 모든 연동된 제공자에 대해 토큰을 해지하고 사용자 데이터를 삭제하도록 개선했습니다. 관련 로직을 AuthService와 AuthenticationRepositoryImpl로 중앙화하여 코드 구조를 개선한 점도 좋습니다. 하지만, 두 가지 보안 관련 문제가 확인되었습니다: 클라우드 함수에 중복된 UID를 전달하여 발생할 수 있는 잠재적인 Insecure Direct Object Reference (IDOR) 취약점과, 제공자 토큰 해지 중 부분적인 실패가 발생할 경우 계정이 영구적으로 '정지' 상태가 될 수 있는 삭제 흐름의 로직 결함입니다. 이 외에도 코드의 안정성을 더 높일 수 있는 몇 가지 제안을 드립니다.
| logger.info("Deleting Firestore user data. uid: \(uid)") | ||
|
|
||
| let deleteFunction = functions.httpsCallable("deleteUserFirestoreData") | ||
| _ = try await deleteFunction.call(["uid": uid]) |
There was a problem hiding this comment.
The deleteFirestoreUserData function passes the user's UID to the deleteUserFirestoreData cloud function. This is redundant because the authenticated user's UID is already available to the cloud function via the auth context (request.auth.uid). More importantly, this pattern leads to an Insecure Direct Object Reference (IDOR) vulnerability because the corresponding cloud function (as seen in Firebase/functions/src/user/delete.ts) trusts the provided uid without verifying it against the authenticated session. An attacker could potentially call this cloud function directly with another user's UID to delete their data.
| for provider in providers { | ||
| switch provider { | ||
| case .apple: | ||
| try await appleAuthService.deleteAuth(uid) | ||
| case .github: | ||
| try await githubAuthService.deleteAuth(uid) | ||
| case .google: | ||
| try await googleAuthService.deleteAuth(uid) | ||
| } | ||
| } |
There was a problem hiding this comment.
현재 delete 메서드는 연결된 모든 제공자를 반복하며 deleteAuth를 호출합니다. 이 호출 중 하나라도 실패하면 (예: 네트워크 오류 또는 만료된 세션으로 인해) 전체 삭제 프로세스가 중단됩니다. 이로 인해 계정이 부분적으로만 삭제된 상태로 남게 됩니다. 일부 제공자(예: Apple)는 deleteAuth 중에 토큰을 해지하므로, 계정을 다시 삭제하려는 시도는 해지된 제공자가 이제 일관되게 오류를 반환하기 때문에 영구적으로 실패할 수 있습니다. 이는 사용자가 계정과 Firestore 데이터를 완전히 삭제할 수 없게 만드는 로직 결함으로 이어질 수 있습니다. TaskGroup을 사용하여 각 프로바이더의 인증 해제 작업을 병렬로 실행하고, 개별 작업의 실패가 다른 작업에 영향을 주지 않도록 하는 것이 더 안정적입니다. 이렇게 하면 모든 프로바이더에 대한 해지를 시도하고, 일부가 실패하더라도 최종적인 사용자 삭제는 계속 진행할 수 있으며, 탈퇴 과정의 속도도 개선될 수 있습니다.
await withTaskGroup(of: Void.self) { group in
for provider in providers {
group.addTask {
do {
switch provider {
case .apple:
try await self.appleAuthService.deleteAuth(uid)
case .github:
try await self.githubAuthService.deleteAuth(uid)
case .google:
try await self.googleAuthService.deleteAuth(uid)
}
} catch {
// 각 프로바이더 인증 해제 실패 시 로그를 남기는 것이 좋습니다.
print("Failed to de-auth provider \(provider): \(error)")
}
}
}
}| try await messaging.deleteToken() | ||
| try Auth.auth().signOut() |
There was a problem hiding this comment.
clearCurrentSession 함수에서 messaging.deleteToken() 호출이 실패할 경우, Auth.auth().signOut()이 실행되지 않고 함수가 종료됩니다. FCM 토큰 삭제는 중요하지만, 이 작업의 실패가 사용자 로그아웃 자체를 막아서는 안 됩니다.
messaging.deleteToken() 호출을 do-catch 블록으로 감싸서 오류가 발생하더라도 로그만 남기고, signOut()은 계속 실행되도록 하여 세션 정리 로직의 안정성을 높이는 것이 좋겠습니다.
do {
try await messaging.deleteToken()
} catch {
logger.error("Failed to delete FCM token during session clearing", error: error)
}
try Auth.auth().signOut()