diff --git a/src/composables/auth/useAuthActions.test.ts b/src/composables/auth/useAuthActions.test.ts index ecf89d8cd33..a4c7c8892f0 100644 --- a/src/composables/auth/useAuthActions.test.ts +++ b/src/composables/auth/useAuthActions.test.ts @@ -1,7 +1,9 @@ import { FirebaseError } from 'firebase/app' +import { AuthErrorCodes } from 'firebase/auth' import { beforeEach, describe, expect, it, vi } from 'vitest' import { useAuthActions } from '@/composables/auth/useAuthActions' +import enLocale from '@/locales/en/main.json' import type { ComfyWorkflow } from '@/platform/workflow/management/stores/workflowStore' type ModifiedWorkflow = Pick @@ -44,18 +46,30 @@ const mockBillingState = vi.hoisted(() => ({ })) const mockClearAllWorkflowStorage = vi.hoisted(() => vi.fn()) -const knownAuthErrorCodes = new Set([ - 'auth/invalid-credential', - 'auth/email-already-in-use', - 'auth/user-not-found' -]) +const authErrorMessages: Record = enLocale.auth.errors + +const firebaseCodesWithOwnMessage = Object.keys(authErrorMessages).filter( + (key) => key.startsWith('auth/') +) + +const popupPermissionCodes = [ + AuthErrorCodes.POPUP_CLOSED_BY_USER, + AuthErrorCodes.EXPIRED_POPUP_REQUEST, + AuthErrorCodes.POPUP_BLOCKED +] + +const accessErrorCodes = [ + 'auth/unauthorized-domain', + 'auth/invalid-dynamic-link-domain', + 'auth/unauthorized-continue-uri' +] vi.mock('@/i18n', () => ({ - t: (key: string, values?: { workflow?: string }) => - values?.workflow ? `${key}:${values.workflow}` : key, + t: (key: string, values?: Record) => + values ? `${key}:${Object.values(values).join(':')}` : key, st: (key: string, fallback: string) => { const code = key.replace('auth.errors.', '') - return knownAuthErrorCodes.has(code) ? key : fallback + return code in authErrorMessages ? key : fallback } })) @@ -412,17 +426,33 @@ describe('useAuthActions auth flow error telemetry', () => { }) describe('useAuthActions.reportError', () => { - it('shows the friendly message for a known Firebase auth code', () => { - const { reportError } = useAuthActions() + it.for(firebaseCodesWithOwnMessage)( + 'maps %s to its own message rather than the generic fallback', + (code) => { + const { reportError } = useAuthActions() - reportError(new FirebaseError('auth/invalid-credential', 'raw firebase')) + reportError(new FirebaseError(code, 'raw firebase')) - expect(mockToastStore.add).toHaveBeenCalledWith({ - severity: 'error', - summary: 'g.error', - detail: 'auth.errors.auth/invalid-credential' - }) - expect(mockToastErrorHandler).not.toHaveBeenCalled() + expect(mockToastStore.add).toHaveBeenCalledWith( + expect.objectContaining({ detail: `auth.errors.${code}` }) + ) + expect(mockToastErrorHandler).not.toHaveBeenCalled() + } + ) + + it('gives every Firebase code a message distinct from the generic one', () => { + const generic = authErrorMessages['generic'] + const collisions = firebaseCodesWithOwnMessage.filter( + (code) => authErrorMessages[code] === generic + ) + + expect(collisions).toEqual([]) + }) + + it('covers every popup-permission code with its own message', () => { + expect(firebaseCodesWithOwnMessage).toEqual( + expect.arrayContaining(popupPermissionCodes) + ) }) it('shows the signupBlocked message when the error carries the signup_blocked token', () => { @@ -481,4 +511,59 @@ describe('useAuthActions.reportError', () => { expect(mockToastErrorHandler).toHaveBeenCalledWith(networkError) expect(mockToastStore.add).not.toHaveBeenCalled() }) + + it.for(popupPermissionCodes)( + 'warns rather than errors for %s, since the user or browser caused it', + (code) => { + const { reportError } = useAuthActions() + + reportError(new FirebaseError(code, 'raw firebase')) + + expect(mockToastStore.add).toHaveBeenCalledWith({ + severity: 'warn', + summary: 'g.warning', + detail: `auth.errors.${code}` + }) + expect(mockToastErrorHandler).not.toHaveBeenCalled() + } + ) + + it('reports an account collision as an error, not a popup warning', () => { + const { reportError, accessError } = useAuthActions() + + reportError( + new FirebaseError('auth/account-exists-with-different-credential', 'raw') + ) + + expect(mockToastStore.add).toHaveBeenCalledWith({ + severity: 'error', + summary: 'g.error', + detail: 'auth.errors.auth/account-exists-with-different-credential' + }) + expect(accessError.value).toBe(false) + }) + + it.for(accessErrorCodes)( + 'interpolates the domain and flips accessError for %s', + (code) => { + const { reportError, accessError } = useAuthActions() + + reportError(new FirebaseError(code, 'raw firebase')) + + expect(accessError.value).toBe(true) + expect(mockToastStore.add).toHaveBeenCalledWith({ + severity: 'error', + summary: 'g.error', + detail: `toastMessages.unauthorizedDomain:${window.location.hostname}:support@comfy.org` + }) + } + ) + + it('leaves accessError false for auth codes outside the domain group', () => { + const { reportError, accessError } = useAuthActions() + + reportError(new FirebaseError('auth/popup-blocked', 'raw firebase')) + + expect(accessError.value).toBe(false) + }) }) diff --git a/src/composables/auth/useAuthActions.ts b/src/composables/auth/useAuthActions.ts index 6eaf58a8414..ad8adb4edae 100644 --- a/src/composables/auth/useAuthActions.ts +++ b/src/composables/auth/useAuthActions.ts @@ -18,6 +18,13 @@ import { useAuthStore } from '@/stores/authStore' import type { BillingPortalTargetTier } from '@/stores/authStore' import { usdToMicros } from '@/utils/formatUtil' +/** Popup outcomes the user or their browser caused, not app faults. */ +const POPUP_PERMISSION_ERROR_CODES: readonly string[] = [ + AuthErrorCodes.POPUP_CLOSED_BY_USER, + AuthErrorCodes.EXPIRED_POPUP_REQUEST, + AuthErrorCodes.POPUP_BLOCKED +] + /** * Service for Firebase Auth actions. * All actions are wrapped with error handling. @@ -71,6 +78,15 @@ export const useAuthActions = () => { summary: t('g.error'), detail: t('auth.errors.signupBlocked') }) + } else if ( + error instanceof FirebaseError && + POPUP_PERMISSION_ERROR_CODES.includes(error.code) + ) { + toastStore.add({ + severity: 'warn', + summary: t('g.warning'), + detail: st(`auth.errors.${error.code}`, t('auth.errors.generic')) + }) } else if (error instanceof FirebaseError) { toastStore.add({ severity: 'error', diff --git a/src/locales/en/main.json b/src/locales/en/main.json index a45bed8cc4d..308768bea2a 100644 --- a/src/locales/en/main.json +++ b/src/locales/en/main.json @@ -2576,8 +2576,10 @@ "auth/operation-not-allowed": "This sign-in method is not currently supported.", "auth/invalid-credential": "Invalid login credentials. Please check your email and password.", "auth/network-request-failed": "Network error. Please check your connection and try again.", - "auth/popup-closed-by-user": "Sign-in was cancelled. Please try again.", - "auth/cancelled-popup-request": "Sign-in was cancelled. Please try again.", + "auth/popup-closed-by-user": "The sign-in window closed before sign-in finished. Please try again.", + "auth/cancelled-popup-request": "Another sign-in window was already open, so this one was cancelled. Please try again.", + "auth/popup-blocked": "Your browser blocked the sign-in window. Please allow pop-ups for this site and try again.", + "auth/account-exists-with-different-credential": "An account already exists with this email address but uses a different sign-in method. Please sign in the way you did originally.", "generic": "Something went wrong while signing you in. Please try again.", "signupBlocked": "We couldn't create your account right now. Please try again later. If this keeps happening, email support@comfy.org." },