Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions src/scripts/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1303,8 +1303,7 @@ export class ComfyApp {
const executionStore = useExecutionStore()
executionStore.lastNodeErrors = null

let comfyOrgAuthToken =
(await useFirebaseAuthStore().getIdToken()) ?? undefined
let comfyOrgAuthToken = await useFirebaseAuthStore().getIdToken()
let comfyOrgApiKey = useApiKeyAuthStore().getApiKey()

try {
Expand Down
27 changes: 23 additions & 4 deletions src/stores/firebaseAuthStore.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { FirebaseError } from 'firebase/app'
import {
type Auth,
AuthErrorCodes,
GithubAuthProvider,
GoogleAuthProvider,
type User,
Expand All @@ -20,6 +22,7 @@ import { useFirebaseAuth } from 'vuefire'

import { COMFY_API_BASE_URL } from '@/config/comfyApi'
import { t } from '@/i18n'
import { useDialogService } from '@/services/dialogService'
import { useApiKeyAuthStore } from '@/stores/apiKeyAuthStore'
import { type AuthHeader } from '@/types/authTypes'
import { operations } from '@/types/comfyRegistryTypes'
Expand Down Expand Up @@ -88,11 +91,27 @@ export const useFirebaseAuthStore = defineStore('firebaseAuth', () => {
lastBalanceUpdateTime.value = null
})

const getIdToken = async (): Promise<string | null> => {
if (currentUser.value) {
return currentUser.value.getIdToken()
const getIdToken = async (): Promise<string | undefined> => {
if (!currentUser.value) return
try {
return await currentUser.value.getIdToken()
} catch (error: unknown) {
if (
error instanceof FirebaseError &&
error.code === AuthErrorCodes.NETWORK_REQUEST_FAILED
) {
console.warn(
'Could not authenticate with Firebase. Features requiring authentication might not work.'
)
return
}

useDialogService().showErrorDialog(error, {
title: t('errorDialog.defaultTitle'),
reportType: 'authenticationError'
})
console.error(error)
}
return null
}

/**
Expand Down
119 changes: 97 additions & 22 deletions tests-ui/tests/store/firebaseAuthStore.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { FirebaseError } from 'firebase/app'
import * as firebaseAuth from 'firebase/auth'
import { createPinia, setActivePinia } from 'pinia'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import * as vuefire from 'vuefire'

import { useDialogService } from '@/services/dialogService'
import { useFirebaseAuthStore } from '@/stores/firebaseAuthStore'

// Mock fetch
Expand Down Expand Up @@ -46,21 +48,24 @@ vi.mock('vue-i18n', () => ({
})
}))

vi.mock('firebase/auth', () => ({
signInWithEmailAndPassword: vi.fn(),
createUserWithEmailAndPassword: vi.fn(),
signOut: vi.fn(),
onAuthStateChanged: vi.fn(),
signInWithPopup: vi.fn(),
GoogleAuthProvider: class {
setCustomParameters = vi.fn()
},
GithubAuthProvider: class {
setCustomParameters = vi.fn()
},
browserLocalPersistence: 'browserLocalPersistence',
setPersistence: vi.fn().mockResolvedValue(undefined)
}))
vi.mock('firebase/auth', async (importOriginal) => {
const actual = await importOriginal<typeof import('firebase/auth')>()
return {
...actual,
signInWithEmailAndPassword: vi.fn(),
createUserWithEmailAndPassword: vi.fn(),
signOut: vi.fn(),
onAuthStateChanged: vi.fn(),
signInWithPopup: vi.fn(),
GoogleAuthProvider: class {
setCustomParameters = vi.fn()
},
GithubAuthProvider: class {
setCustomParameters = vi.fn()
},
setPersistence: vi.fn().mockResolvedValue(undefined)
}
})

// Mock useToastStore
vi.mock('@/stores/toastStore', () => ({
Expand All @@ -70,11 +75,7 @@ vi.mock('@/stores/toastStore', () => ({
}))

// Mock useDialogService
vi.mock('@/services/dialogService', () => ({
useDialogService: () => ({
showSettingsDialog: vi.fn()
})
}))
vi.mock('@/services/dialogService')

describe('useFirebaseAuthStore', () => {
let store: ReturnType<typeof useFirebaseAuthStore>
Expand All @@ -93,6 +94,12 @@ describe('useFirebaseAuthStore', () => {
beforeEach(() => {
vi.resetAllMocks()

// Setup dialog service mock
vi.mocked(useDialogService, { partial: true }).mockReturnValue({
showSettingsDialog: vi.fn(),
showErrorDialog: vi.fn()
})

// Mock useFirebaseAuth to return our mock auth object
vi.mocked(vuefire.useFirebaseAuth).mockReturnValue(mockAuth as any)

Expand Down Expand Up @@ -297,7 +304,7 @@ describe('useFirebaseAuthStore', () => {

const token = await store.getIdToken()

expect(token).toBeNull()
expect(token).toBeUndefined()
})

it('should return null for token after login and logout sequence', async () => {
Expand Down Expand Up @@ -329,7 +336,75 @@ describe('useFirebaseAuthStore', () => {

// Verify token is null after logout
const tokenAfterLogout = await store.getIdToken()
expect(tokenAfterLogout).toBeNull()
expect(tokenAfterLogout).toBeUndefined()
})

it('should handle network errors gracefully when offline (reproduces issue #4468)', async () => {
// This test reproduces the issue where Firebase Auth makes network requests when offline
// and fails without graceful error handling, causing toast error messages

// Simulate a user with an expired token that requires network refresh
mockUser.getIdToken.mockReset()

// Mock network failure (auth/network-request-failed error from Firebase)
const networkError = new FirebaseError(
firebaseAuth.AuthErrorCodes.NETWORK_REQUEST_FAILED,
'mock error'
)

mockUser.getIdToken.mockRejectedValue(networkError)

const token = await store.getIdToken()
expect(token).toBeUndefined() // Should return undefined instead of throwing
})

it('should show error dialog when getIdToken fails with non-network error', async () => {
// This test verifies that non-network errors trigger the error dialog
mockUser.getIdToken.mockReset()

// Mock a non-network error using actual Firebase Auth error code
const authError = new FirebaseError(
firebaseAuth.AuthErrorCodes.USER_DISABLED,
'User account is disabled.'
)

mockUser.getIdToken.mockRejectedValue(authError)

// Should call the error dialog instead of throwing
const token = await store.getIdToken()
const dialogService = useDialogService()

expect(dialogService.showErrorDialog).toHaveBeenCalledWith(authError, {
title: 'errorDialog.defaultTitle',
reportType: 'authenticationError'
})
expect(token).toBeUndefined()
})
})

describe('getAuthHeader', () => {
it('should handle network errors gracefully when getting Firebase token (reproduces issue #4468)', async () => {
// This test reproduces the issue where getAuthHeader fails due to network errors
// when Firebase Auth tries to refresh tokens offline

// Mock useApiKeyAuthStore to return null (no API key fallback)
const mockApiKeyStore = {
getAuthHeader: vi.fn().mockReturnValue(null)
}
vi.doMock('@/stores/apiKeyAuthStore', () => ({
useApiKeyAuthStore: () => mockApiKeyStore
}))

// Setup user with network error on token refresh
mockUser.getIdToken.mockReset()
const networkError = new FirebaseError(
firebaseAuth.AuthErrorCodes.NETWORK_REQUEST_FAILED,
'mock error'
)
mockUser.getIdToken.mockRejectedValue(networkError)

const authHeader = await store.getAuthHeader()
expect(authHeader).toBeNull() // Should fallback gracefully
})
})

Expand Down