Skip to content

Commit b1050e3

Browse files
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)
1 parent ba100c4 commit b1050e3

File tree

3 files changed

+105
-4
lines changed

3 files changed

+105
-4
lines changed

src/services/extensionService.ts

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,17 @@ import { useMenuItemStore } from '@/stores/menuItemStore'
1111
import { useWidgetStore } from '@/stores/widgetStore'
1212
import { useBottomPanelStore } from '@/stores/workspace/bottomPanelStore'
1313
import type { ComfyExtension } from '@/types/comfy'
14+
import type { AuthUserInfo } from '@/types/authTypes'
1415

1516
export const useExtensionService = () => {
1617
const extensionStore = useExtensionStore()
1718
const settingStore = useSettingStore()
1819
const keybindingStore = useKeybindingStore()
19-
const { wrapWithErrorHandling } = useErrorHandling()
20+
const {
21+
wrapWithErrorHandling,
22+
wrapWithErrorHandlingAsync,
23+
toastErrorHandler
24+
} = useErrorHandling()
2025

2126
/**
2227
* Loads all extensions from the API into the window in parallel
@@ -77,22 +82,55 @@ export const useExtensionService = () => {
7782

7883
if (extension.onAuthUserResolved) {
7984
const { onUserResolved } = useCurrentUser()
85+
const handleUserResolved = wrapWithErrorHandlingAsync(
86+
(user: AuthUserInfo) => extension.onAuthUserResolved?.(user, app),
87+
(error) => {
88+
console.error('[Extension Auth Hook Error]', {
89+
extension: extension.name,
90+
hook: 'onAuthUserResolved',
91+
error
92+
})
93+
toastErrorHandler(error)
94+
}
95+
)
8096
onUserResolved((user) => {
81-
void extension.onAuthUserResolved?.(user, app)
97+
void handleUserResolved(user)
8298
})
8399
}
84100

85101
if (extension.onAuthTokenRefreshed) {
86102
const { onTokenRefreshed } = useCurrentUser()
103+
const handleTokenRefreshed = wrapWithErrorHandlingAsync(
104+
() => extension.onAuthTokenRefreshed?.(),
105+
(error) => {
106+
console.error('[Extension Auth Hook Error]', {
107+
extension: extension.name,
108+
hook: 'onAuthTokenRefreshed',
109+
error
110+
})
111+
toastErrorHandler(error)
112+
}
113+
)
87114
onTokenRefreshed(() => {
88-
void extension.onAuthTokenRefreshed?.()
115+
void handleTokenRefreshed()
89116
})
90117
}
91118

92119
if (extension.onAuthUserLogout) {
93120
const { onUserLogout } = useCurrentUser()
121+
const handleUserLogout = wrapWithErrorHandlingAsync(
122+
() => extension.onAuthUserLogout?.(),
123+
(error) => {
124+
console.error('[Extension Auth Hook Error]', {
125+
extension: extension.name,
126+
hook: 'onAuthUserLogout',
127+
error
128+
})
129+
toastErrorHandler(error)
130+
}
131+
)
94132
onUserLogout(() => {
95-
void extension.onAuthUserLogout?.()
133+
void handleUserLogout()
96134
})
97135
}
98136
}

src/stores/firebaseAuthStore.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ export const useFirebaseAuthStore = defineStore('firebaseAuth', () => {
6464

6565
// Token refresh trigger - increments when token is refreshed
6666
const tokenRefreshTrigger = ref(0)
67+
/**
68+
* The user ID for which the initial ID token has been observed.
69+
* When a token changes for the same user, that is a refresh.
70+
*/
71+
const lastTokenUserId = ref<string | null>(null)
6772

6873
const buildApiUrl = (path: string) => `${getComfyApiBaseUrl()}${path}`
6974

@@ -95,6 +100,9 @@ export const useFirebaseAuthStore = defineStore('firebaseAuth', () => {
95100
onAuthStateChanged(auth, (user) => {
96101
currentUser.value = user
97102
isInitialized.value = true
103+
if (user === null) {
104+
lastTokenUserId.value = null
105+
}
98106

99107
// Reset balance when auth state changes
100108
balance.value = null
@@ -104,6 +112,11 @@ export const useFirebaseAuthStore = defineStore('firebaseAuth', () => {
104112
// Listen for token refresh events
105113
onIdTokenChanged(auth, (user) => {
106114
if (user && isCloud) {
115+
// Skip initial token change
116+
if (lastTokenUserId.value !== user.uid) {
117+
lastTokenUserId.value = user.uid
118+
return
119+
}
107120
tokenRefreshTrigger.value++
108121
}
109122
})

tests-ui/tests/store/firebaseAuthStore.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ vi.mock('@/services/dialogService')
8383
describe('useFirebaseAuthStore', () => {
8484
let store: ReturnType<typeof useFirebaseAuthStore>
8585
let authStateCallback: (user: any) => void
86+
let idTokenCallback: (user: any) => void
8687

8788
const mockAuth = {
8889
/* mock Auth object */
@@ -143,6 +144,55 @@ describe('useFirebaseAuthStore', () => {
143144
mockUser.getIdToken.mockResolvedValue('mock-id-token')
144145
})
145146

147+
describe('token refresh events', () => {
148+
beforeEach(async () => {
149+
vi.resetModules()
150+
vi.doMock('@/platform/distribution/types', () => ({
151+
isCloud: true,
152+
isDesktop: true
153+
}))
154+
155+
vi.mocked(firebaseAuth.onIdTokenChanged).mockImplementation(
156+
(_auth, callback) => {
157+
idTokenCallback = callback as (user: any) => void
158+
return vi.fn()
159+
}
160+
)
161+
162+
vi.mocked(vuefire.useFirebaseAuth).mockReturnValue(mockAuth as any)
163+
164+
setActivePinia(createPinia())
165+
const storeModule = await import('@/stores/firebaseAuthStore')
166+
store = storeModule.useFirebaseAuthStore()
167+
})
168+
169+
it("should not increment tokenRefreshTrigger on the user's first ID token event", () => {
170+
idTokenCallback?.(mockUser)
171+
expect(store.tokenRefreshTrigger).toBe(0)
172+
})
173+
174+
it('should increment tokenRefreshTrigger on subsequent ID token events for the same user', () => {
175+
idTokenCallback?.(mockUser)
176+
idTokenCallback?.(mockUser)
177+
expect(store.tokenRefreshTrigger).toBe(1)
178+
})
179+
180+
it('should not increment when ID token event is for a different user UID', () => {
181+
const otherUser = { uid: 'other-user-id' }
182+
idTokenCallback?.(mockUser)
183+
idTokenCallback?.(otherUser)
184+
expect(store.tokenRefreshTrigger).toBe(0)
185+
})
186+
187+
it('should increment after switching to a new UID and receiving a second event for that UID', () => {
188+
const otherUser = { uid: 'other-user-id' }
189+
idTokenCallback?.(mockUser)
190+
idTokenCallback?.(otherUser)
191+
idTokenCallback?.(otherUser)
192+
expect(store.tokenRefreshTrigger).toBe(1)
193+
})
194+
})
195+
146196
it('should initialize with the current user', () => {
147197
expect(store.currentUser).toEqual(mockUser)
148198
expect(store.isAuthenticated).toBe(true)

0 commit comments

Comments
 (0)