From 1935bb4d5e320a9a00eaef08bb13ad655582d00e Mon Sep 17 00:00:00 2001 From: kishore Date: Mon, 11 May 2026 22:06:51 -0700 Subject: [PATCH] refactor: extract OAuth post-login resume into a composable Both CloudLoginView and CloudSignupView had identical post-success OAuth-resume blocks (capture request id, create session, navigate or surface error). Extract into useOAuthPostLoginRedirect so the logic is unit-testable in isolation without spinning up the full login views. Also gives codecov diff coverage on the try/catch error path. --- .../oauth/useOAuthPostLoginRedirect.test.ts | 89 +++++++++++++++++++ .../cloud/oauth/useOAuthPostLoginRedirect.ts | 50 +++++++++++ .../cloud/onboarding/CloudLoginView.vue | 28 ++---- .../cloud/onboarding/CloudSignupView.vue | 28 ++---- 4 files changed, 151 insertions(+), 44 deletions(-) create mode 100644 src/platform/cloud/oauth/useOAuthPostLoginRedirect.test.ts create mode 100644 src/platform/cloud/oauth/useOAuthPostLoginRedirect.ts diff --git a/src/platform/cloud/oauth/useOAuthPostLoginRedirect.test.ts b/src/platform/cloud/oauth/useOAuthPostLoginRedirect.test.ts new file mode 100644 index 0000000000..1935223b78 --- /dev/null +++ b/src/platform/cloud/oauth/useOAuthPostLoginRedirect.test.ts @@ -0,0 +1,89 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { useOAuthPostLoginRedirect } from '@/platform/cloud/oauth/useOAuthPostLoginRedirect' + +const VALID_REQUEST_ID = '550e8400-e29b-41d4-a716-446655440000' + +const routerPush = vi.fn().mockResolvedValue(undefined) +const createSessionOrThrow = vi.fn().mockResolvedValue(undefined) + +vi.mock('vue-router', () => ({ + useRouter: () => ({ push: routerPush }) +})) + +vi.mock('@/platform/auth/session/useSessionCookie', () => ({ + useSessionCookie: () => ({ createSessionOrThrow }) +})) + +describe('useOAuthPostLoginRedirect', () => { + beforeEach(() => { + sessionStorage.clear() + routerPush.mockClear() + createSessionOrThrow.mockReset().mockResolvedValue(undefined) + }) + + it('returns no-oauth when neither query nor sessionStorage holds a request id', async () => { + const { resumeOAuthIfNeeded } = useOAuthPostLoginRedirect() + + const result = await resumeOAuthIfNeeded({}) + + expect(result).toEqual({ kind: 'no-oauth' }) + expect(createSessionOrThrow).not.toHaveBeenCalled() + expect(routerPush).not.toHaveBeenCalled() + }) + + it('establishes session and navigates to consent when oauth_request_id is in the query', async () => { + const { resumeOAuthIfNeeded } = useOAuthPostLoginRedirect() + + const result = await resumeOAuthIfNeeded({ + oauth_request_id: VALID_REQUEST_ID + }) + + expect(createSessionOrThrow).toHaveBeenCalledOnce() + expect(routerPush).toHaveBeenCalledWith({ + name: 'cloud-oauth-consent', + query: { oauth_request_id: VALID_REQUEST_ID } + }) + expect(result).toEqual({ kind: 'resumed' }) + }) + + it('resumes using a stashed sessionStorage id when the query is empty (multi-step flows)', async () => { + sessionStorage.setItem('Comfy.OAuthRequestId', VALID_REQUEST_ID) + const { resumeOAuthIfNeeded } = useOAuthPostLoginRedirect() + + const result = await resumeOAuthIfNeeded({}) + + expect(result).toEqual({ kind: 'resumed' }) + expect(routerPush).toHaveBeenCalledWith({ + name: 'cloud-oauth-consent', + query: { oauth_request_id: VALID_REQUEST_ID } + }) + }) + + it('returns an error with the thrown message when session creation fails', async () => { + createSessionOrThrow.mockRejectedValue(new Error('Unauthorized')) + const { resumeOAuthIfNeeded } = useOAuthPostLoginRedirect() + + const result = await resumeOAuthIfNeeded({ + oauth_request_id: VALID_REQUEST_ID + }) + + expect(result).toEqual({ kind: 'error', message: 'Unauthorized' }) + expect(routerPush).not.toHaveBeenCalled() + }) + + it('falls back to a generic message when session creation rejects with a non-Error value', async () => { + createSessionOrThrow.mockRejectedValue('boom') + const { resumeOAuthIfNeeded } = useOAuthPostLoginRedirect() + + const result = await resumeOAuthIfNeeded({ + oauth_request_id: VALID_REQUEST_ID + }) + + expect(result.kind).toBe('error') + if (result.kind === 'error') { + expect(result.message).toMatch(/try again/i) + } + expect(routerPush).not.toHaveBeenCalled() + }) +}) diff --git a/src/platform/cloud/oauth/useOAuthPostLoginRedirect.ts b/src/platform/cloud/oauth/useOAuthPostLoginRedirect.ts new file mode 100644 index 0000000000..e672843551 --- /dev/null +++ b/src/platform/cloud/oauth/useOAuthPostLoginRedirect.ts @@ -0,0 +1,50 @@ +import type { LocationQuery } from 'vue-router' +import { useRouter } from 'vue-router' + +import { useSessionCookie } from '@/platform/auth/session/useSessionCookie' +import { + captureOAuthRequestId, + getOAuthRequestId +} from '@/platform/cloud/oauth/oauthState' + +export type OAuthResumeResult = + | { kind: 'no-oauth' } + | { kind: 'resumed' } + | { kind: 'error'; message: string } + +const FALLBACK_ERROR_MESSAGE = 'Failed to establish session. Please try again.' + +/** + * Post-login OAuth resume. If the current login flow originated from an OAuth + * authorize request, establishes the Cloud session cookie and navigates to the + * consent route. Used by both `CloudLoginView` and `CloudSignupView`. + */ +export function useOAuthPostLoginRedirect() { + const router = useRouter() + const sessionCookie = useSessionCookie() + + async function resumeOAuthIfNeeded( + query: LocationQuery + ): Promise { + captureOAuthRequestId(query) + const oauthRequestId = getOAuthRequestId() + if (!oauthRequestId) return { kind: 'no-oauth' } + + try { + await sessionCookie.createSessionOrThrow() + } catch (error) { + return { + kind: 'error', + message: error instanceof Error ? error.message : FALLBACK_ERROR_MESSAGE + } + } + + await router.push({ + name: 'cloud-oauth-consent', + query: { oauth_request_id: oauthRequestId } + }) + return { kind: 'resumed' } + } + + return { resumeOAuthIfNeeded } +} diff --git a/src/platform/cloud/onboarding/CloudLoginView.vue b/src/platform/cloud/onboarding/CloudLoginView.vue index 131f2a378e..e369acd0f6 100644 --- a/src/platform/cloud/onboarding/CloudLoginView.vue +++ b/src/platform/cloud/onboarding/CloudLoginView.vue @@ -116,11 +116,7 @@ import { useRoute, useRouter } from 'vue-router' import Button from '@/components/ui/button/Button.vue' import { useAuthActions } from '@/composables/auth/useAuthActions' -import { useSessionCookie } from '@/platform/auth/session/useSessionCookie' -import { - captureOAuthRequestId, - getOAuthRequestId -} from '@/platform/cloud/oauth/oauthState' +import { useOAuthPostLoginRedirect } from '@/platform/cloud/oauth/useOAuthPostLoginRedirect' import CloudSignInForm from '@/platform/cloud/onboarding/components/CloudSignInForm.vue' import { useFreeTierOnboarding } from '@/platform/cloud/onboarding/composables/useFreeTierOnboarding' import { getSafePreviousFullPath } from '@/platform/cloud/onboarding/utils/previousFullPath' @@ -132,7 +128,7 @@ const { t } = useI18n() const router = useRouter() const route = useRoute() const authActions = useAuthActions() -const sessionCookie = useSessionCookie() +const { resumeOAuthIfNeeded } = useOAuthPostLoginRedirect() const isSecureContext = globalThis.isSecureContext const authError = ref('') const toastStore = useToastStore() @@ -159,24 +155,12 @@ const onSuccess = async () => { life: 2000 }) - captureOAuthRequestId(route.query) - const oauthRequestId = getOAuthRequestId() - if (oauthRequestId) { - try { - await sessionCookie.createSessionOrThrow() - } catch (error) { - authError.value = - error instanceof Error - ? error.message - : 'Failed to establish session. Please try again.' - return - } - await router.push({ - name: 'cloud-oauth-consent', - query: { oauth_request_id: oauthRequestId } - }) + const oauthResume = await resumeOAuthIfNeeded(route.query) + if (oauthResume.kind === 'error') { + authError.value = oauthResume.message return } + if (oauthResume.kind === 'resumed') return const previousFullPath = getSafePreviousFullPath(route.query) if (previousFullPath) { diff --git a/src/platform/cloud/onboarding/CloudSignupView.vue b/src/platform/cloud/onboarding/CloudSignupView.vue index 8a5a6b0892..3daf1b1168 100644 --- a/src/platform/cloud/onboarding/CloudSignupView.vue +++ b/src/platform/cloud/onboarding/CloudSignupView.vue @@ -141,11 +141,7 @@ import { useRoute, useRouter } from 'vue-router' import SignUpForm from '@/components/dialog/content/signin/SignUpForm.vue' import Button from '@/components/ui/button/Button.vue' import { useAuthActions } from '@/composables/auth/useAuthActions' -import { useSessionCookie } from '@/platform/auth/session/useSessionCookie' -import { - captureOAuthRequestId, - getOAuthRequestId -} from '@/platform/cloud/oauth/oauthState' +import { useOAuthPostLoginRedirect } from '@/platform/cloud/oauth/useOAuthPostLoginRedirect' import { useFreeTierOnboarding } from '@/platform/cloud/onboarding/composables/useFreeTierOnboarding' import { getSafePreviousFullPath } from '@/platform/cloud/onboarding/utils/previousFullPath' import { isCloud } from '@/platform/distribution/types' @@ -159,7 +155,7 @@ const { t } = useI18n() const router = useRouter() const route = useRoute() const authActions = useAuthActions() -const sessionCookie = useSessionCookie() +const { resumeOAuthIfNeeded } = useOAuthPostLoginRedirect() const isSecureContext = globalThis.isSecureContext const authError = ref('') const userIsInChina = ref(false) @@ -185,24 +181,12 @@ const onSuccess = async () => { life: 2000 }) - captureOAuthRequestId(route.query) - const oauthRequestId = getOAuthRequestId() - if (oauthRequestId) { - try { - await sessionCookie.createSessionOrThrow() - } catch (error) { - authError.value = - error instanceof Error - ? error.message - : 'Failed to establish session. Please try again.' - return - } - await router.push({ - name: 'cloud-oauth-consent', - query: { oauth_request_id: oauthRequestId } - }) + const oauthResume = await resumeOAuthIfNeeded(route.query) + if (oauthResume.kind === 'error') { + authError.value = oauthResume.message return } + if (oauthResume.kind === 'resumed') return const previousFullPath = getSafePreviousFullPath(route.query) if (previousFullPath) {