mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-26 10:59:53 +00:00
[auth] handle auth/requires-recent-login for account deletion and password updates (#6109)
## Summary Implemented error recovery system to handle Firebase `auth/requires-recent-login` errors when deleting accounts or updating passwords. https://github.com/user-attachments/assets/92a79e2a-cff5-4b18-b529-dbaf5f2303f2 ## Changes - **What**: Added [ErrorRecoveryStrategy pattern](https://firebase.google.com/docs/auth/web/manage-users#re-authenticate_a_user) to `useErrorHandling` composable with automatic retry logic for sensitive Firebase operations - **Breaking**: None - recovery strategies are optional, all existing code unchanged ## Technical Details Firebase enforces [reauthentication](https://firebase.google.com/docs/reference/js/auth#autherrorcodes) for security-sensitive operations (account deletion, password changes) after ~5 minutes of inactivity. Previously these operations failed with cryptic error messages. New flow: 1. Operation throws `auth/requires-recent-login` 2. Recovery strategy shows confirmation dialog 3. User logs out and re-authenticates 4. Operation automatically retries ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6109-auth-handle-auth-requires-recent-login-for-account-deletion-and-password-updates-28f6d73d36508119abf4ce30eecea976) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -1,9 +1,12 @@
|
||||
import { FirebaseError } from 'firebase/app'
|
||||
import { AuthErrorCodes } from 'firebase/auth'
|
||||
import { ref } from 'vue'
|
||||
|
||||
import { useErrorHandling } from '@/composables/useErrorHandling'
|
||||
import type { ErrorRecoveryStrategy } from '@/composables/useErrorHandling'
|
||||
import { t } from '@/i18n'
|
||||
import { useToastStore } from '@/platform/updates/common/toastStore'
|
||||
import { useDialogService } from '@/services/dialogService'
|
||||
import { useFirebaseAuthStore } from '@/stores/firebaseAuthStore'
|
||||
import { usdToMicros } from '@/utils/formatUtil'
|
||||
|
||||
@@ -122,6 +125,47 @@ export const useFirebaseAuthActions = () => {
|
||||
reportError
|
||||
)
|
||||
|
||||
/**
|
||||
* Recovery strategy for Firebase auth/requires-recent-login errors.
|
||||
* Prompts user to reauthenticate and retries the operation after successful login.
|
||||
*/
|
||||
const createReauthenticationRecovery = <
|
||||
TArgs extends unknown[],
|
||||
TReturn
|
||||
>(): ErrorRecoveryStrategy<TArgs, TReturn> => {
|
||||
const dialogService = useDialogService()
|
||||
|
||||
return {
|
||||
shouldHandle: (error: unknown) =>
|
||||
error instanceof FirebaseError &&
|
||||
error.code === AuthErrorCodes.CREDENTIAL_TOO_OLD_LOGIN_AGAIN,
|
||||
|
||||
recover: async (
|
||||
_error: unknown,
|
||||
retry: (...args: TArgs) => Promise<TReturn> | TReturn,
|
||||
args: TArgs
|
||||
) => {
|
||||
const confirmed = await dialogService.confirm({
|
||||
title: t('auth.reauthRequired.title'),
|
||||
message: t('auth.reauthRequired.message'),
|
||||
type: 'default'
|
||||
})
|
||||
|
||||
if (!confirmed) {
|
||||
return
|
||||
}
|
||||
|
||||
await authStore.logout()
|
||||
|
||||
const signedIn = await dialogService.showSignInDialog()
|
||||
|
||||
if (signedIn) {
|
||||
await retry(...args)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const updatePassword = wrapWithErrorHandlingAsync(
|
||||
async (newPassword: string) => {
|
||||
await authStore.updatePassword(newPassword)
|
||||
@@ -132,18 +176,25 @@ export const useFirebaseAuthActions = () => {
|
||||
life: 5000
|
||||
})
|
||||
},
|
||||
reportError
|
||||
reportError,
|
||||
undefined,
|
||||
[createReauthenticationRecovery<[string], void>()]
|
||||
)
|
||||
|
||||
const deleteAccount = wrapWithErrorHandlingAsync(async () => {
|
||||
await authStore.deleteAccount()
|
||||
toastStore.add({
|
||||
severity: 'success',
|
||||
summary: t('auth.deleteAccount.success'),
|
||||
detail: t('auth.deleteAccount.successDetail'),
|
||||
life: 5000
|
||||
})
|
||||
}, reportError)
|
||||
const deleteAccount = wrapWithErrorHandlingAsync(
|
||||
async () => {
|
||||
await authStore.deleteAccount()
|
||||
toastStore.add({
|
||||
severity: 'success',
|
||||
summary: t('auth.deleteAccount.success'),
|
||||
detail: t('auth.deleteAccount.successDetail'),
|
||||
life: 5000
|
||||
})
|
||||
},
|
||||
reportError,
|
||||
undefined,
|
||||
[createReauthenticationRecovery<[], void>()]
|
||||
)
|
||||
|
||||
return {
|
||||
logout,
|
||||
|
||||
@@ -1,6 +1,54 @@
|
||||
import { t } from '@/i18n'
|
||||
import { useToastStore } from '@/platform/updates/common/toastStore'
|
||||
|
||||
/**
|
||||
* Strategy for recovering from specific error conditions.
|
||||
* Allows operations to be retried after resolving the error condition.
|
||||
*
|
||||
* @template TArgs - The argument types of the operation to be retried
|
||||
* @template TReturn - The return type of the operation
|
||||
*
|
||||
* @example
|
||||
* ```typescript
|
||||
* const networkRecovery: ErrorRecoveryStrategy = {
|
||||
* shouldHandle: (error) => error instanceof NetworkError,
|
||||
* recover: async (error, retry) => {
|
||||
* await waitForNetwork()
|
||||
* await retry()
|
||||
* }
|
||||
* }
|
||||
* ```
|
||||
*/
|
||||
export interface ErrorRecoveryStrategy<
|
||||
TArgs extends unknown[] = unknown[],
|
||||
TReturn = unknown
|
||||
> {
|
||||
/**
|
||||
* Determines if this strategy should handle the given error.
|
||||
* @param error - The error to check
|
||||
* @returns true if this strategy can handle the error
|
||||
*/
|
||||
shouldHandle: (error: unknown) => boolean
|
||||
|
||||
/**
|
||||
* Attempts to recover from the error and retry the operation.
|
||||
* This function is responsible for:
|
||||
* 1. Resolving the error condition (e.g., reauthentication, network reconnect)
|
||||
* 2. Calling retry() to re-execute the original operation
|
||||
* 3. Handling the retry result (success or failure)
|
||||
*
|
||||
* @param error - The error that occurred
|
||||
* @param retry - Function to retry the original operation
|
||||
* @param args - Original arguments passed to the operation
|
||||
* @returns Promise that resolves when recovery completes (whether successful or not)
|
||||
*/
|
||||
recover: (
|
||||
error: unknown,
|
||||
retry: (...args: TArgs) => Promise<TReturn> | TReturn,
|
||||
args: TArgs
|
||||
) => Promise<void>
|
||||
}
|
||||
|
||||
export function useErrorHandling() {
|
||||
const toast = useToastStore()
|
||||
const toastErrorHandler = (error: unknown) => {
|
||||
@@ -13,9 +61,9 @@ export function useErrorHandling() {
|
||||
}
|
||||
|
||||
const wrapWithErrorHandling =
|
||||
<TArgs extends any[], TReturn>(
|
||||
<TArgs extends unknown[], TReturn>(
|
||||
action: (...args: TArgs) => TReturn,
|
||||
errorHandler?: (error: any) => void,
|
||||
errorHandler?: (error: unknown) => void,
|
||||
finallyHandler?: () => void
|
||||
) =>
|
||||
(...args: TArgs): TReturn | undefined => {
|
||||
@@ -29,15 +77,27 @@ export function useErrorHandling() {
|
||||
}
|
||||
|
||||
const wrapWithErrorHandlingAsync =
|
||||
<TArgs extends any[], TReturn>(
|
||||
<TArgs extends unknown[], TReturn>(
|
||||
action: (...args: TArgs) => Promise<TReturn> | TReturn,
|
||||
errorHandler?: (error: any) => void,
|
||||
finallyHandler?: () => void
|
||||
errorHandler?: (error: unknown) => void,
|
||||
finallyHandler?: () => void,
|
||||
recoveryStrategies: ErrorRecoveryStrategy<TArgs, TReturn>[] = []
|
||||
) =>
|
||||
async (...args: TArgs): Promise<TReturn | undefined> => {
|
||||
try {
|
||||
return await action(...args)
|
||||
} catch (e) {
|
||||
for (const strategy of recoveryStrategies) {
|
||||
if (strategy.shouldHandle(e)) {
|
||||
try {
|
||||
await strategy.recover(e, action, args)
|
||||
return
|
||||
} catch (recoveryError) {
|
||||
console.error('Recovery strategy failed:', recoveryError)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
;(errorHandler ?? toastErrorHandler)(e)
|
||||
} finally {
|
||||
finallyHandler?.()
|
||||
|
||||
@@ -1864,6 +1864,12 @@
|
||||
"success": "Account Deleted",
|
||||
"successDetail": "Your account has been successfully deleted."
|
||||
},
|
||||
"reauthRequired": {
|
||||
"title": "Re-authentication Required",
|
||||
"message": "For security reasons, this action requires you to sign in again. Would you like to proceed?",
|
||||
"confirm": "Sign In Again",
|
||||
"cancel": "Cancel"
|
||||
},
|
||||
"loginButton": {
|
||||
"tooltipHelp": "Login to be able to use \"API Nodes\"",
|
||||
"tooltipLearnMore": "Learn more..."
|
||||
|
||||
@@ -54,7 +54,7 @@ export interface TreeExplorerNode<T = any> extends TreeNode {
|
||||
event: MouseEvent
|
||||
) => void | Promise<void>
|
||||
/** Function to handle errors */
|
||||
handleError?: (this: TreeExplorerNode<T>, error: Error) => void
|
||||
handleError?: (this: TreeExplorerNode<T>, error: unknown) => void
|
||||
/** Extra context menu items */
|
||||
contextMenuItems?:
|
||||
| MenuItem[]
|
||||
|
||||
353
tests-ui/tests/composables/useErrorHandling.test.ts
Normal file
353
tests-ui/tests/composables/useErrorHandling.test.ts
Normal file
@@ -0,0 +1,353 @@
|
||||
import { createPinia, setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { ErrorRecoveryStrategy } from '@/composables/useErrorHandling'
|
||||
import { useErrorHandling } from '@/composables/useErrorHandling'
|
||||
|
||||
describe('useErrorHandling', () => {
|
||||
let errorHandler: ReturnType<typeof useErrorHandling>
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
setActivePinia(createPinia())
|
||||
errorHandler = useErrorHandling()
|
||||
})
|
||||
|
||||
describe('wrapWithErrorHandlingAsync', () => {
|
||||
it('should execute action successfully', async () => {
|
||||
const action = vi.fn(async () => 'success')
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(action)
|
||||
|
||||
const result = await wrapped()
|
||||
|
||||
expect(result).toBe('success')
|
||||
expect(action).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('should call error handler when action throws', async () => {
|
||||
const testError = new Error('test error')
|
||||
const action = vi.fn(async () => {
|
||||
throw testError
|
||||
})
|
||||
const customErrorHandler = vi.fn()
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
customErrorHandler
|
||||
)
|
||||
|
||||
await wrapped()
|
||||
|
||||
expect(customErrorHandler).toHaveBeenCalledWith(testError)
|
||||
})
|
||||
|
||||
it('should call finally handler after success', async () => {
|
||||
const action = vi.fn(async () => 'success')
|
||||
const finallyHandler = vi.fn()
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
undefined,
|
||||
finallyHandler
|
||||
)
|
||||
|
||||
await wrapped()
|
||||
|
||||
expect(finallyHandler).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('should call finally handler after error', async () => {
|
||||
const action = vi.fn(async () => {
|
||||
throw new Error('test error')
|
||||
})
|
||||
const finallyHandler = vi.fn()
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
vi.fn(),
|
||||
finallyHandler
|
||||
)
|
||||
|
||||
await wrapped()
|
||||
|
||||
expect(finallyHandler).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
describe('error recovery', () => {
|
||||
it('should not use recovery strategy when no error occurs', async () => {
|
||||
const action = vi.fn(async () => 'success')
|
||||
const recoveryStrategy: ErrorRecoveryStrategy = {
|
||||
shouldHandle: vi.fn(() => true),
|
||||
recover: vi.fn()
|
||||
}
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
undefined,
|
||||
undefined,
|
||||
[recoveryStrategy]
|
||||
)
|
||||
|
||||
await wrapped()
|
||||
|
||||
expect(recoveryStrategy.shouldHandle).not.toHaveBeenCalled()
|
||||
expect(recoveryStrategy.recover).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should use recovery strategy when it matches error', async () => {
|
||||
const testError = new Error('test error')
|
||||
const action = vi.fn(async () => {
|
||||
throw testError
|
||||
})
|
||||
const recoveryStrategy: ErrorRecoveryStrategy = {
|
||||
shouldHandle: vi.fn((error) => error === testError),
|
||||
recover: vi.fn(async () => {
|
||||
// Recovery succeeds, does nothing
|
||||
})
|
||||
}
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
vi.fn(),
|
||||
undefined,
|
||||
[recoveryStrategy]
|
||||
)
|
||||
|
||||
await wrapped()
|
||||
|
||||
expect(recoveryStrategy.shouldHandle).toHaveBeenCalledWith(testError)
|
||||
expect(recoveryStrategy.recover).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should pass action and args to recovery strategy', async () => {
|
||||
const testError = new Error('test error')
|
||||
const action = vi.fn(async (_arg1: string, _arg2: number) => {
|
||||
throw testError
|
||||
})
|
||||
const recoveryStrategy: ErrorRecoveryStrategy<[string, number], void> =
|
||||
{
|
||||
shouldHandle: vi.fn(() => true),
|
||||
recover: vi.fn()
|
||||
}
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
vi.fn(),
|
||||
undefined,
|
||||
[recoveryStrategy]
|
||||
)
|
||||
|
||||
await wrapped('test', 123)
|
||||
|
||||
expect(recoveryStrategy.recover).toHaveBeenCalledWith(
|
||||
testError,
|
||||
action,
|
||||
['test', 123]
|
||||
)
|
||||
})
|
||||
|
||||
it('should retry operation when recovery succeeds', async () => {
|
||||
let attemptCount = 0
|
||||
const action = vi.fn(async (value: string) => {
|
||||
attemptCount++
|
||||
if (attemptCount === 1) {
|
||||
throw new Error('first attempt failed')
|
||||
}
|
||||
return `success: ${value}`
|
||||
})
|
||||
|
||||
const recoveryStrategy: ErrorRecoveryStrategy<[string], string> = {
|
||||
shouldHandle: vi.fn(() => true),
|
||||
recover: vi.fn(async (_error, retry, args) => {
|
||||
await retry(...args)
|
||||
})
|
||||
}
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
vi.fn(),
|
||||
undefined,
|
||||
[recoveryStrategy]
|
||||
)
|
||||
|
||||
await wrapped('test-value')
|
||||
|
||||
expect(action).toHaveBeenCalledTimes(2)
|
||||
expect(recoveryStrategy.recover).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('should not call error handler when recovery succeeds', async () => {
|
||||
const action = vi.fn(async () => {
|
||||
throw new Error('test error')
|
||||
})
|
||||
const customErrorHandler = vi.fn()
|
||||
const recoveryStrategy: ErrorRecoveryStrategy = {
|
||||
shouldHandle: vi.fn(() => true),
|
||||
recover: vi.fn(async () => {
|
||||
// Recovery succeeds
|
||||
})
|
||||
}
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
customErrorHandler,
|
||||
undefined,
|
||||
[recoveryStrategy]
|
||||
)
|
||||
|
||||
await wrapped()
|
||||
|
||||
expect(customErrorHandler).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should call error handler when recovery fails', async () => {
|
||||
const originalError = new Error('original error')
|
||||
const recoveryError = new Error('recovery error')
|
||||
const action = vi.fn(async () => {
|
||||
throw originalError
|
||||
})
|
||||
const customErrorHandler = vi.fn()
|
||||
const recoveryStrategy: ErrorRecoveryStrategy = {
|
||||
shouldHandle: vi.fn(() => true),
|
||||
recover: vi.fn(async () => {
|
||||
throw recoveryError
|
||||
})
|
||||
}
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
customErrorHandler,
|
||||
undefined,
|
||||
[recoveryStrategy]
|
||||
)
|
||||
|
||||
await wrapped()
|
||||
|
||||
expect(customErrorHandler).toHaveBeenCalledWith(originalError)
|
||||
})
|
||||
|
||||
it('should try multiple recovery strategies in order', async () => {
|
||||
const testError = new Error('test error')
|
||||
const action = vi.fn(async () => {
|
||||
throw testError
|
||||
})
|
||||
|
||||
const strategy1: ErrorRecoveryStrategy = {
|
||||
shouldHandle: vi.fn(() => false),
|
||||
recover: vi.fn()
|
||||
}
|
||||
|
||||
const strategy2: ErrorRecoveryStrategy = {
|
||||
shouldHandle: vi.fn(() => true),
|
||||
recover: vi.fn(async () => {
|
||||
// Recovery succeeds
|
||||
})
|
||||
}
|
||||
|
||||
const strategy3: ErrorRecoveryStrategy = {
|
||||
shouldHandle: vi.fn(() => true),
|
||||
recover: vi.fn()
|
||||
}
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
vi.fn(),
|
||||
undefined,
|
||||
[strategy1, strategy2, strategy3]
|
||||
)
|
||||
|
||||
await wrapped()
|
||||
|
||||
expect(strategy1.shouldHandle).toHaveBeenCalledWith(testError)
|
||||
expect(strategy1.recover).not.toHaveBeenCalled()
|
||||
|
||||
expect(strategy2.shouldHandle).toHaveBeenCalledWith(testError)
|
||||
expect(strategy2.recover).toHaveBeenCalled()
|
||||
|
||||
// Strategy 3 should not be checked because strategy 2 handled it
|
||||
expect(strategy3.shouldHandle).not.toHaveBeenCalled()
|
||||
expect(strategy3.recover).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should fall back to error handler when no strategy matches', async () => {
|
||||
const testError = new Error('test error')
|
||||
const action = vi.fn(async () => {
|
||||
throw testError
|
||||
})
|
||||
const customErrorHandler = vi.fn()
|
||||
|
||||
const strategy: ErrorRecoveryStrategy = {
|
||||
shouldHandle: vi.fn(() => false),
|
||||
recover: vi.fn()
|
||||
}
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
customErrorHandler,
|
||||
undefined,
|
||||
[strategy]
|
||||
)
|
||||
|
||||
await wrapped()
|
||||
|
||||
expect(strategy.shouldHandle).toHaveBeenCalledWith(testError)
|
||||
expect(strategy.recover).not.toHaveBeenCalled()
|
||||
expect(customErrorHandler).toHaveBeenCalledWith(testError)
|
||||
})
|
||||
|
||||
it('should work with synchronous actions', async () => {
|
||||
const testError = new Error('test error')
|
||||
const action = vi.fn(() => {
|
||||
throw testError
|
||||
})
|
||||
const recoveryStrategy: ErrorRecoveryStrategy = {
|
||||
shouldHandle: vi.fn(() => true),
|
||||
recover: vi.fn(async () => {
|
||||
// Recovery succeeds
|
||||
})
|
||||
}
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
vi.fn(),
|
||||
undefined,
|
||||
[recoveryStrategy]
|
||||
)
|
||||
|
||||
await wrapped()
|
||||
|
||||
expect(recoveryStrategy.recover).toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('backward compatibility', () => {
|
||||
it('should work without recovery strategies parameter', async () => {
|
||||
const action = vi.fn(async () => 'success')
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(action)
|
||||
|
||||
const result = await wrapped()
|
||||
|
||||
expect(result).toBe('success')
|
||||
})
|
||||
|
||||
it('should work with empty recovery strategies array', async () => {
|
||||
const testError = new Error('test error')
|
||||
const action = vi.fn(async () => {
|
||||
throw testError
|
||||
})
|
||||
const customErrorHandler = vi.fn()
|
||||
|
||||
const wrapped = errorHandler.wrapWithErrorHandlingAsync(
|
||||
action,
|
||||
customErrorHandler,
|
||||
undefined,
|
||||
[]
|
||||
)
|
||||
|
||||
await wrapped()
|
||||
|
||||
expect(customErrorHandler).toHaveBeenCalledWith(testError)
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user