Skip to content
119 changes: 102 additions & 17 deletions src/composables/auth/useAuthActions.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { FirebaseError } from 'firebase/app'
import { AuthErrorCodes } from 'firebase/auth'
import { createPinia, setActivePinia } from 'pinia'
import { beforeEach, describe, expect, it, vi } from 'vitest'

import { useAuthActions } from '@/composables/auth/useAuthActions'
import enLocale from '@/locales/en/main.json'
Comment thread
MaanilVerma marked this conversation as resolved.
import type { ComfyWorkflow } from '@/platform/workflow/management/stores/workflowStore'

type ModifiedWorkflow = Pick<ComfyWorkflow, 'path' | 'isModified'>
Expand Down Expand Up @@ -36,18 +38,30 @@ const mockTrackAuthFailed = vi.hoisted(() => vi.fn())
const mockDistributionState = vi.hoisted(() => ({ isCloud: false }))
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<string, string> = 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<string, string>) =>
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
}
}))

Expand Down Expand Up @@ -366,17 +380,33 @@ describe('useAuthActions.reportError', () => {
vi.clearAllMocks()
})

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', () => {
Expand Down Expand Up @@ -435,4 +465,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)
})
})
16 changes: 16 additions & 0 deletions src/composables/auth/useAuthActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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',
Expand Down
6 changes: 4 additions & 2 deletions src/locales/en/main.json
Original file line number Diff line number Diff line change
Expand Up @@ -2568,8 +2568,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."
},
Expand Down
Loading