Fix session cookie creation race: skip initial token refresh, wrap extension auth hooks (#6563)

Fixes a race causing “No auth header available for session creation”
during sign‑in, by skipping the initial token refresh event, and
wrapping extension auth hooks with async error handling.

Sentry:
https://comfy-org.sentry.io/issues/6990347926/?alert_rule_id=1614600&project=4509681221369857

Context
- Error surfaced as an unhandled rejection when session creation was
triggered without a valid auth header.
- Triggers: both onAuthUserResolved and onAuthTokenRefreshed fired
during initial login.
- Pre‑fix, onIdTokenChanged treated the very first token emission as a
“refresh” as well, so two concurrent createSession() calls ran
back‑to‑back.
- One of those calls could land before a Firebase ID token existed, so
getAuthHeader() returned null → createSession threw “No auth header
available for session creation”.

Exact pre‑fix failure path
- src/extensions/core/cloudSessionCookie.ts
  - onAuthUserResolved → useSessionCookie().createSession()
  - onAuthTokenRefreshed → useSessionCookie().createSession()
- src/stores/firebaseAuthStore.ts
- onIdTokenChanged increments tokenRefreshTrigger even for the initial
token (treated as a refresh)
- getAuthHeader() → getIdToken() may be undefined briefly during
initialization
- src/platform/auth/session/useSessionCookie.ts
- createSession(): calls authStore.getAuthHeader(); if falsy, throws
Error('No auth header available for session creation')

What this PR changes
1) Skip initial token “refresh”
- Track lastTokenUserId and ignore the first onIdTokenChanged for a
user; only subsequent token changes count as refresh events.
   - File: src/stores/firebaseAuthStore.ts
2) Wrap extension auth hooks with async error handling
- Use wrapWithErrorHandlingAsync for
onAuthUserResolved/onAuthTokenRefreshed/onAuthUserLogout callbacks to
avoid unhandled rejections.
   - File: src/services/extensionService.ts

Result
- Eliminates the timing window where createSession() runs before
getIdToken() returns a token.
- Ensures any remaining errors are caught and reported instead of
surfacing as unhandled promise rejections.

Notes
- Lint and typecheck run clean (pnpm lint:fix && pnpm typecheck).

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6563-Fix-session-cookie-creation-race-dedupe-calls-skip-initial-token-refresh-wrap-extensio-2a16d73d365081ef8c22c5ac8cb948aa)
by [Unito](https://www.unito.io)
This commit is contained in:
Benjamin Lu
2025-11-07 20:30:49 -08:00
committed by GitHub
parent ba100c4a04
commit b1050e3195
3 changed files with 105 additions and 4 deletions

View File

@@ -11,12 +11,17 @@ import { useMenuItemStore } from '@/stores/menuItemStore'
import { useWidgetStore } from '@/stores/widgetStore'
import { useBottomPanelStore } from '@/stores/workspace/bottomPanelStore'
import type { ComfyExtension } from '@/types/comfy'
import type { AuthUserInfo } from '@/types/authTypes'
export const useExtensionService = () => {
const extensionStore = useExtensionStore()
const settingStore = useSettingStore()
const keybindingStore = useKeybindingStore()
const { wrapWithErrorHandling } = useErrorHandling()
const {
wrapWithErrorHandling,
wrapWithErrorHandlingAsync,
toastErrorHandler
} = useErrorHandling()
/**
* Loads all extensions from the API into the window in parallel
@@ -77,22 +82,55 @@ export const useExtensionService = () => {
if (extension.onAuthUserResolved) {
const { onUserResolved } = useCurrentUser()
const handleUserResolved = wrapWithErrorHandlingAsync(
(user: AuthUserInfo) => extension.onAuthUserResolved?.(user, app),
(error) => {
console.error('[Extension Auth Hook Error]', {
extension: extension.name,
hook: 'onAuthUserResolved',
error
})
toastErrorHandler(error)
}
)
onUserResolved((user) => {
void extension.onAuthUserResolved?.(user, app)
void handleUserResolved(user)
})
}
if (extension.onAuthTokenRefreshed) {
const { onTokenRefreshed } = useCurrentUser()
const handleTokenRefreshed = wrapWithErrorHandlingAsync(
() => extension.onAuthTokenRefreshed?.(),
(error) => {
console.error('[Extension Auth Hook Error]', {
extension: extension.name,
hook: 'onAuthTokenRefreshed',
error
})
toastErrorHandler(error)
}
)
onTokenRefreshed(() => {
void extension.onAuthTokenRefreshed?.()
void handleTokenRefreshed()
})
}
if (extension.onAuthUserLogout) {
const { onUserLogout } = useCurrentUser()
const handleUserLogout = wrapWithErrorHandlingAsync(
() => extension.onAuthUserLogout?.(),
(error) => {
console.error('[Extension Auth Hook Error]', {
extension: extension.name,
hook: 'onAuthUserLogout',
error
})
toastErrorHandler(error)
}
)
onUserLogout(() => {
void extension.onAuthUserLogout?.()
void handleUserLogout()
})
}
}

View File

@@ -64,6 +64,11 @@ export const useFirebaseAuthStore = defineStore('firebaseAuth', () => {
// Token refresh trigger - increments when token is refreshed
const tokenRefreshTrigger = ref(0)
/**
* The user ID for which the initial ID token has been observed.
* When a token changes for the same user, that is a refresh.
*/
const lastTokenUserId = ref<string | null>(null)
const buildApiUrl = (path: string) => `${getComfyApiBaseUrl()}${path}`
@@ -95,6 +100,9 @@ export const useFirebaseAuthStore = defineStore('firebaseAuth', () => {
onAuthStateChanged(auth, (user) => {
currentUser.value = user
isInitialized.value = true
if (user === null) {
lastTokenUserId.value = null
}
// Reset balance when auth state changes
balance.value = null
@@ -104,6 +112,11 @@ export const useFirebaseAuthStore = defineStore('firebaseAuth', () => {
// Listen for token refresh events
onIdTokenChanged(auth, (user) => {
if (user && isCloud) {
// Skip initial token change
if (lastTokenUserId.value !== user.uid) {
lastTokenUserId.value = user.uid
return
}
tokenRefreshTrigger.value++
}
})

View File

@@ -83,6 +83,7 @@ vi.mock('@/services/dialogService')
describe('useFirebaseAuthStore', () => {
let store: ReturnType<typeof useFirebaseAuthStore>
let authStateCallback: (user: any) => void
let idTokenCallback: (user: any) => void
const mockAuth = {
/* mock Auth object */
@@ -143,6 +144,55 @@ describe('useFirebaseAuthStore', () => {
mockUser.getIdToken.mockResolvedValue('mock-id-token')
})
describe('token refresh events', () => {
beforeEach(async () => {
vi.resetModules()
vi.doMock('@/platform/distribution/types', () => ({
isCloud: true,
isDesktop: true
}))
vi.mocked(firebaseAuth.onIdTokenChanged).mockImplementation(
(_auth, callback) => {
idTokenCallback = callback as (user: any) => void
return vi.fn()
}
)
vi.mocked(vuefire.useFirebaseAuth).mockReturnValue(mockAuth as any)
setActivePinia(createPinia())
const storeModule = await import('@/stores/firebaseAuthStore')
store = storeModule.useFirebaseAuthStore()
})
it("should not increment tokenRefreshTrigger on the user's first ID token event", () => {
idTokenCallback?.(mockUser)
expect(store.tokenRefreshTrigger).toBe(0)
})
it('should increment tokenRefreshTrigger on subsequent ID token events for the same user', () => {
idTokenCallback?.(mockUser)
idTokenCallback?.(mockUser)
expect(store.tokenRefreshTrigger).toBe(1)
})
it('should not increment when ID token event is for a different user UID', () => {
const otherUser = { uid: 'other-user-id' }
idTokenCallback?.(mockUser)
idTokenCallback?.(otherUser)
expect(store.tokenRefreshTrigger).toBe(0)
})
it('should increment after switching to a new UID and receiving a second event for that UID', () => {
const otherUser = { uid: 'other-user-id' }
idTokenCallback?.(mockUser)
idTokenCallback?.(otherUser)
idTokenCallback?.(otherUser)
expect(store.tokenRefreshTrigger).toBe(1)
})
})
it('should initialize with the current user', () => {
expect(store.currentUser).toEqual(mockUser)
expect(store.isAuthenticated).toBe(true)