Skip to content

[#251] 여러 계정을 링크한 이후 각 공급자에 대한 실질적인 회원탈퇴가 이루어지지 않는 이슈를 해결한다 #252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
opficdev merged 4 commits into from
Mar 11, 2026
Merged

[#251] 여러 계정을 링크한 이후 각 공급자에 대한 실질적인 회원탈퇴가 이루어지지 않는 이슈를 해결한다 #252

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5d155e2
fix: 여러 공급자가 링크된 유저인 경우 providers를 순회해서 링크를 끉도록 해결
opficdev Mar 11, 2026
81f0c99
refactor: providerIDs가 currentUser가 nil인 경우 빈 배열을 반환하도록 개선
opficdev Mar 11, 2026
77fbb23
refactor: uid를 클라이언트 쪽에서 보내지 않도록 개선
opficdev Mar 11, 2026
21a6518
refactor: FCM 토큰을 삭제하는데 실패해도 로그아웃은 되도록 개선
opficdev Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DevLog/Data/Repository/AuthDataRepositoryImpl.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ final class AuthDataRepositoryImpl: AuthDataRepository {
}

func fetchAllProviders() async throws -> [AuthProvider] {
let providerStrings = authService.providerIDs ?? []
let providerStrings = authService.providerIDs
return providerStrings.compactMap { AuthProvider(rawValue: $0) }
}

Expand Down
27 changes: 16 additions & 11 deletions DevLog/Data/Repository/AuthenticationRepositoryImpl.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,25 @@ final class AuthenticationRepositoryImpl: AuthenticationRepository {
}

func delete() async throws {
guard let uid = authService.uid,
let providerID = try await authService.getProviderID(),
let provider = AuthProvider(rawValue: providerID)
else {
guard let uid = authService.uid else {
throw AuthError.notAuthenticated
}

switch provider {
case .apple:
try await appleAuthService.deleteAuth(uid)
case .github:
try await githubAuthService.deleteAuth(uid)
case .google:
try await googleAuthService.deleteAuth(uid)
let providers = authService.providerIDs.compactMap { AuthProvider(rawValue: $0) }

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)
}
}
Comment on lines +72 to 81
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

현재 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 authService.deleteFirestoreUserData(uid)
try await authService.deleteCurrentUser()
try await authService.clearCurrentSession()
}
}
33 changes: 31 additions & 2 deletions DevLog/Infra/Service/AuthService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,21 @@

import FirebaseAuth
import FirebaseFirestore
import FirebaseFunctions
import FirebaseMessaging

final class AuthService {
private let store = Firestore.firestore()
private let functions = Functions.functions(region: "asia-northeast3")
private let messaging = Messaging.messaging()
private let logger = Logger(category: "AuthService")

var uid: String? {
Auth.auth().currentUser?.uid
}

var providerIDs: [String]? {
Auth.auth().currentUser?.providerData.map { $0.providerID }
var providerIDs: [String] {
Auth.auth().currentUser?.providerData.map { $0.providerID } ?? []
}

func getProviderID() async throws -> String? {
Expand All @@ -42,4 +46,29 @@ final class AuthService {
throw error
}
}

func deleteFirestoreUserData(_ uid: String) async throws {
logger.info("Deleting Firestore user data. uid: \(uid)")

let deleteFunction = functions.httpsCallable("deleteUserFirestoreData")
_ = try await deleteFunction.call(["uid": uid])
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

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.

}

func deleteCurrentUser() async throws {
logger.info("Deleting FirebaseAuth current user")

guard let currentUser = Auth.auth().currentUser else {
logger.warning("No current user to delete")
throw AuthError.notAuthenticated
}

try await currentUser.delete()
}

func clearCurrentSession() async throws {
logger.info("Clearing current auth session")

try await messaging.deleteToken()
try Auth.auth().signOut()
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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()

}
}
8 changes: 0 additions & 8 deletions DevLog/Infra/Service/SocialLogin/AppleAuthenticationService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,6 @@ final class AppleAuthenticationService: AuthenticationService {
let token = try await refreshAppleAccessToken()

try await revokeAppleAccessToken(token: token)

let deleteFunction = functions.httpsCallable("deleteUserFirestoreData")

_ = try await deleteFunction.call(["uid": uid])

try await user?.delete()
try await messaging.deleteToken()
try Auth.auth().signOut()
}

func link(uid: String, email: String) async throws {
Expand Down
8 changes: 0 additions & 8 deletions DevLog/Infra/Service/SocialLogin/GithubAuthenticationService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,6 @@ final class GithubAuthenticationService: NSObject, AuthenticationService {

func deleteAuth(_ uid: String) async throws {
try await revokeAccessToken()

let deleteFunction = functions.httpsCallable("deleteUserFirestoreData")

_ = try await deleteFunction.call(["uid": uid])

try await user?.delete()
try await messaging.deleteToken()
try Auth.auth().signOut()
}

func link(uid: String, email: String) async throws {
Expand Down
7 changes: 0 additions & 7 deletions DevLog/Infra/Service/SocialLogin/GoogleAuthenticationService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,8 @@ final class GoogleAuthenticationService: AuthenticationService {
}

func deleteAuth(_ uid: String) async throws {
let deleteFunction = functions.httpsCallable("deleteUserFirestoreData")

_ = try await deleteFunction.call(["uid": uid])

try await user?.delete()
GIDSignIn.sharedInstance.signOut()
try await GIDSignIn.sharedInstance.disconnect()
try await messaging.deleteToken()
try Auth.auth().signOut()
}

func link(uid: String, email: String) async throws {
Expand Down