From 0a1ed4e09b0e16cd55a29ada08b2641447a5f815 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20G=C3=B3mez=20Morales?= Date: Fri, 2 Oct 2026 09:56:20 +0000 Subject: [PATCH] fix(editor): Wait for OAuth sign-in before the assistant credential card continues (#39998) --- .../InstanceAiCredentialSetup.test.ts | 189 +++++++++++++++++- .../components/InstanceAiCredentialSetup.vue | 61 +++++- .../setupPanel/InstanceAiSetupCredential.vue | 16 +- .../credentials/composables/oauthCallback.ts | 25 +++ 4 files changed, 273 insertions(+), 18 deletions(-) diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiCredentialSetup.test.ts b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiCredentialSetup.test.ts index 742cc026356..3422962d4de 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiCredentialSetup.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiCredentialSetup.test.ts @@ -2,9 +2,10 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { createTestingPinia } from '@pinia/testing'; import { setActivePinia } from 'pinia'; import userEvent from '@testing-library/user-event'; -import { defineComponent, h, nextTick } from 'vue'; +import { defineComponent, h, nextTick, ref } from 'vue'; import { createThreadComponentRenderer } from './createThreadComponentRenderer'; import type { InstanceAiCredentialRequest } from '@n8n/api-types'; +import type { ICredentialType } from 'n8n-workflow'; import InstanceAiCredentialSetup from '../components/InstanceAiCredentialSetup.vue'; import { useInstanceAiStore, type ThreadRuntime } from '../instanceAi.store'; import { useInstanceAiSettingsStore } from '../instanceAiSettings.store'; @@ -37,6 +38,7 @@ vi.mock('@/features/credentials/quickConnect/composables/useQuickConnect', () => vi.mock('@/features/credentials/composables/useCredentialOAuth', () => ({ useCredentialOAuth: () => ({ canOAuthCredentialQuickConnect: () => false, + isOAuthCredentialType: (type: string) => type.endsWith('OAuth2Api'), }), })); @@ -150,6 +152,16 @@ function stubUsableCredentials( }); } +function stubCredentialTypes( + store: ReturnType, + getType: (name: string) => ICredentialType | undefined, +) { + Object.defineProperty(store, 'getCredentialTypeByName', { + configurable: true, + get: () => getType, + }); +} + /** Creates requests with no existing credentials (shows setup button) */ function makeCredentialRequests(count: number): InstanceAiCredentialRequest[] { return Array.from({ length: count }, (_, i) => ({ @@ -194,6 +206,7 @@ describe('InstanceAiCredentialSetup', () => { vi.spyOn(credentialsStore, 'fetchAllCredentials').mockResolvedValue([]); vi.spyOn(credentialsStore, 'fetchUsableCredentials').mockResolvedValue([]); vi.spyOn(credentialsStore, 'fetchCredentialTypes').mockResolvedValue(undefined); + stubCredentialTypes(credentialsStore, (name) => ({ name, displayName: name, properties: [] })); // The card renders the NodeCredentials picker when the store has a usable // credential of the type; default to one so the picker-based tests render it. stubUsableCredentials(credentialsStore, () => [ @@ -660,6 +673,180 @@ describe('InstanceAiCredentialSetup', () => { }); }); + describe('OAuth credential', () => { + const oauthRequests: InstanceAiCredentialRequest[] = [ + { credentialType: 'linearMcpOAuth2Api', reason: 'Linear MCP', existingCredentials: [] }, + ]; + const created = { + id: 'oauth-1', + name: 'Linear MCP account', + type: 'linearMcpOAuth2Api', + updatedAt: '2026-09-30T10:00:00.000Z', + } as ICredentialsResponse; + const payload = { id: '', name: created.name, type: created.type }; + const signedIn = { ...created, data: { oauthTokenData: { access_token: 'token' } } }; + + beforeEach(() => { + vi.spyOn(credentialsApi, 'createNewCredential').mockResolvedValue(created); + }); + + function renderOAuthCard(credentialRequests = oauthRequests) { + return renderComponent({ + props: { requestId: 'req-1', credentialRequests, message: 'Set up credentials' }, + }); + } + + function expectSubmitted(confirmSpy: ReturnType) { + return vi.waitFor(() => + expect(confirmSpy).toHaveBeenCalledWith('req-1', { + kind: 'credentialSelection', + credentials: { linearMcpOAuth2Api: 'oauth-1' }, + }), + ); + } + + it('submits a new credential only after the sign-in completes', async () => { + const credentialsStore = useCredentialsStore(); + const getDataSpy = vi + .spyOn(credentialsStore, 'getCredentialData') + .mockResolvedValue({ ...created, data: {} } as never); + const confirmSpy = vi.spyOn(thread, 'confirmAction').mockResolvedValue(true); + const { getByTestId } = renderOAuthCard(); + + // The credential modal saves the credential before the sign-in popup completes. + await credentialsStore.createNewCredential({ ...payload, data: {} }); + await vi.waitFor(() => expect(getDataSpy).toHaveBeenCalled()); + await nextTick(); + expect(confirmSpy).not.toHaveBeenCalled(); + expect(getByTestId('instance-ai-credential-continue-button')).toBeDisabled(); + + // The popup saves the token. Then the modal refreshes the store. + getDataSpy.mockResolvedValue(signedIn as never); + credentialsStore.upsertCredential({ ...created, updatedAt: '2026-09-30T10:01:00.000Z' }); + + await expectSubmitted(confirmSpy); + }); + + it('does not submit an existing credential that is not connected', async () => { + const credentialsStore = useCredentialsStore(); + credentialsStore.upsertCredential(created); + const getDataSpy = vi + .spyOn(credentialsStore, 'getCredentialData') + .mockResolvedValue({ ...created, data: {} } as never); + const confirmSpy = vi.spyOn(thread, 'confirmAction').mockResolvedValue(true); + const { getByTestId } = renderOAuthCard([ + { ...oauthRequests[0], existingCredentials: [{ id: created.id, name: created.name }] }, + ]); + + await vi.waitFor(() => expect(getDataSpy).toHaveBeenCalledWith({ id: 'oauth-1' })); + await nextTick(); + expect(confirmSpy).not.toHaveBeenCalled(); + expect(getByTestId('instance-ai-credential-continue-button')).toBeDisabled(); + }); + + it('submits an existing credential that is connected', async () => { + const credentialsStore = useCredentialsStore(); + credentialsStore.upsertCredential(created); + vi.spyOn(credentialsStore, 'getCredentialData').mockResolvedValue(signedIn as never); + const confirmSpy = vi.spyOn(thread, 'confirmAction').mockResolvedValue(true); + renderOAuthCard([ + { ...oauthRequests[0], existingCredentials: [{ id: created.id, name: created.name }] }, + ]); + + await expectSubmitted(confirmSpy); + }); + + it('keeps Continue disabled while a selected credential waits for its sign-in', async () => { + const credentialsStore = useCredentialsStore(); + credentialsStore.upsertCredential(created); + const getDataSpy = vi + .spyOn(credentialsStore, 'getCredentialData') + .mockResolvedValue({ ...created, data: {} } as never); + const confirmSpy = vi.spyOn(thread, 'confirmAction').mockResolvedValue(true); + const { getByTestId } = renderOAuthCard([ + { + credentialType: 'slackApi', + reason: 'Slack', + existingCredentials: [{ id: 'api-1', name: 'Slack' }], + }, + { ...oauthRequests[0], existingCredentials: [{ id: created.id, name: created.name }] }, + ]); + + await vi.waitFor(() => expect(getDataSpy).toHaveBeenCalledWith({ id: 'oauth-1' })); + await nextTick(); + expect(confirmSpy).not.toHaveBeenCalled(); + expect(getByTestId('instance-ai-credential-continue-button')).toBeDisabled(); + + getDataSpy.mockResolvedValue(signedIn as never); + credentialsStore.upsertCredential({ ...created, updatedAt: '2026-09-30T10:01:00.000Z' }); + + await vi.waitFor(() => + expect(confirmSpy).toHaveBeenCalledWith('req-1', { + kind: 'credentialSelection', + credentials: { slackApi: 'api-1', linearMcpOAuth2Api: 'oauth-1' }, + }), + ); + }); + + it('submits a credential right away when its grant type needs no sign-in', async () => { + const credentialsStore = useCredentialsStore(); + vi.spyOn(credentialsStore, 'getCredentialData').mockResolvedValue({ + ...created, + data: { grantType: 'clientCredentials' }, + } as never); + const confirmSpy = vi.spyOn(thread, 'confirmAction').mockResolvedValue(true); + renderOAuthCard(); + + await credentialsStore.createNewCredential({ ...payload, data: {} }); + + await expectSubmitted(confirmSpy); + }); + + it('ignores a credential that a connection flow creates before authorization', async () => { + const credentialsStore = useCredentialsStore(); + const confirmSpy = vi.spyOn(thread, 'confirmAction').mockResolvedValue(true); + const { queryByTestId } = renderOAuthCard(); + + await credentialsStore.createNewCredential({ ...payload, data: {} }, undefined, undefined, { + skipStoreUpdate: true, + pendingAuthorization: true, + }); + await nextTick(); + await nextTick(); + + expect(confirmSpy).not.toHaveBeenCalled(); + expect(queryByTestId('instance-ai-credential-step-check')).not.toBeInTheDocument(); + }); + }); + + it('auto-continues only after credential types load', async () => { + const typesLoaded = ref(false); + stubCredentialTypes(useCredentialsStore(), (name) => + typesLoaded.value ? { name, displayName: name, properties: [] } : undefined, + ); + const confirmSpy = vi.spyOn(thread, 'confirmAction').mockResolvedValue(true); + + renderComponent({ + props: { + requestId: 'req-1', + credentialRequests: makeCredentialRequestsWithSingleExisting(1), + message: 'Set up credentials', + }, + }); + await nextTick(); + await nextTick(); + expect(confirmSpy).not.toHaveBeenCalled(); + + typesLoaded.value = true; + + await vi.waitFor(() => + expect(confirmSpy).toHaveBeenCalledWith('req-1', { + kind: 'credentialSelection', + credentials: { type1: 'existing-1' }, + }), + ); + }); + it('keeps a sole generic auth credential preselected but does not auto-submit', async () => { const requests: InstanceAiCredentialRequest[] = [ { diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiCredentialSetup.vue b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiCredentialSetup.vue index ae5b85ae2ef..39666c9e67d 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiCredentialSetup.vue +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiCredentialSetup.vue @@ -2,6 +2,7 @@ import { useUIStore } from '@/app/stores/ui.store'; import { getAppNameFromCredType } from '@/app/utils/nodeTypesUtils'; import { useWizardNavigation } from '@/features/ai/shared/composables/useWizardNavigation'; +import { isOAuthCredentialConnected } from '@/features/credentials/composables/oauthCallback'; import { useCredentialOAuth } from '@/features/credentials/composables/useCredentialOAuth'; import CredentialIcon from '@/features/credentials/components/CredentialIcon.vue'; import { deriveServiceName } from '@/features/credentials/templatedAuth.utils'; @@ -54,7 +55,7 @@ const isBrowserCredentialSetupEnabled = computed( () => INSTANCE_AI_BROWSER_CREDENTIAL_SETUP_ENABLED && settingsStore.isBrowserUseAvailable, ); const { getQuickConnectOptionByCredentialTypes } = useQuickConnect(); -const { canOAuthCredentialQuickConnect } = useCredentialOAuth(); +const { canOAuthCredentialQuickConnect, isOAuthCredentialType } = useCredentialOAuth(); // --------------------------------------------------------------------------- // Navigation @@ -114,8 +115,11 @@ const stopDeleteListener = credentialsStore.$onAction(({ name, after, args }) => // Listen for credential creation to auto-select newly created credentials // when using the button path (no NodeCredentials rendered) -const stopCreateListener = credentialsStore.$onAction(({ name, after }) => { +const stopCreateListener = credentialsStore.$onAction(({ name, after, args }) => { if (name !== 'createNewCredential') return; + // A connection flow creates its credential before authorization and selects + // it through NodeCredentials once it succeeds. + if (args[3]?.skipStoreUpdate) return; after((newCred) => { if (!newCred || typeof newCred !== 'object' || !('id' in newCred)) return; const req = currentRequest.value; @@ -136,8 +140,48 @@ onBeforeUnmount(() => { // Completion // --------------------------------------------------------------------------- +/** Selected OAuth credential id → whether it can authenticate now. */ +const oauthConnections = ref>({}); + +const selectedOAuthIds = computed(() => + props.credentialRequests.flatMap(({ credentialType }) => { + const id = selections.value[credentialType]; + return id && isOAuthCredentialType(credentialType) ? [id] : []; + }), +); + +// Check again when the stored record changes: the OAuth callback saves the +// token and updates `updatedAt`, and the credential modal then fetches again. +watch( + () => + selectedOAuthIds.value + .map((id) => { + const stored = credentialsStore.getCredentialById(id); + return [id, stored?.updatedAt, stored?.connectedByMe, stored?.scopes?.join(',')].join(':'); + }) + .join('|'), + async (_key, _previous, onCleanup) => { + let stale = false; + onCleanup(() => { + stale = true; + }); + await Promise.all( + selectedOAuthIds.value.map(async (id) => { + const stored = credentialsStore.getCredentialById(id); + const fetched = await credentialsStore.getCredentialData({ id }).catch(() => undefined); + if (!stale) oauthConnections.value[id] = isOAuthCredentialConnected(stored, fetched); + }), + ); + }, + { immediate: true }, +); + function isStepComplete(credentialType: string): boolean { - return selections.value[credentialType] !== null; + const selected = selections.value[credentialType]; + if (!selected) return false; + // Credential types load on mount. Until then, the card cannot tell if a sign-in is needed. + if (!credentialsStore.getCredentialTypeByName(credentialType)) return false; + return !isOAuthCredentialType(credentialType) || oauthConnections.value[selected] === true; } /** A step is handled once it has a selection or the user explicitly skipped it — either way, nothing left to do there. */ @@ -153,6 +197,15 @@ const anySelected = computed(() => props.credentialRequests.some((r) => isStepComplete(r.credentialType)), ); +/** Continue must not drop a selected credential that is still waiting for its sign-in. */ +const canContinue = computed( + () => + anySelected.value && + props.credentialRequests.every( + (r) => !selections.value[r.credentialType] || isStepComplete(r.credentialType), + ), +); + /** The submitted-state label: finalize has its own copy; otherwise distinguish a full submit from a mixed skip/select one. */ const submittedLabelKey = computed(() => { if (isFinalize.value) return 'instanceAi.credential.finalize.applied'; @@ -760,7 +813,7 @@ async function handleSetupAutomatically() { size="medium" :class="$style.actionButton" :label="i18n.baseText('instanceAi.credential.continueButton')" - :disabled="!anySelected" + :disabled="!canContinue" data-test-id="instance-ai-credential-continue-button" @click="handleContinue" /> diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/setupPanel/InstanceAiSetupCredential.vue b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/setupPanel/InstanceAiSetupCredential.vue index 435000fbdfc..ce347dedbb3 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/setupPanel/InstanceAiSetupCredential.vue +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/setupPanel/InstanceAiSetupCredential.vue @@ -2,7 +2,6 @@ import { computed, onMounted, onScopeDispose, ref, watch } from 'vue'; import isEqual from 'lodash/isEqual'; import { TEMPLATED_CUSTOM_AUTH_CREDENTIAL_TYPE } from '@n8n/api-types'; -import { getResourcePermissions } from '@n8n/permissions'; import { N8nButton, N8nCallout, @@ -29,7 +28,7 @@ import { useExternalHooks } from '@/app/composables/useExternalHooks'; import type { InstanceAiCredentialContext } from '@/app/composables/useInstanceAiEditorCapability'; import { useCredentialForm } from '@/features/credentials/composables/useCredentialForm'; import { useCredentialOAuth } from '@/features/credentials/composables/useCredentialOAuth'; -import { hasOAuthTokenData } from '@/features/credentials/composables/oauthCallback'; +import { isOAuthCredentialConnected } from '@/features/credentials/composables/oauthCallback'; import { useQuickConnect } from '@/features/credentials/quickConnect/composables/useQuickConnect'; import { useCredentialsStore } from '@/features/credentials/credentials.store'; import { groupCredentialSetupFields } from '@/features/credentials/credentialSetupFields'; @@ -186,25 +185,16 @@ watch( onCleanup(() => { stale = true; }); - const permissions = getResourcePermissions(storedCredential.value?.scopes).credential; - oauthConnection.value = storedCredential.value?.isResolvable - ? storedCredential.value.connectedByMe - : permissions.read === true && !permissions.update; + oauthConnection.value = isOAuthCredentialConnected(storedCredential.value); oauthMode.value = 'unknown'; loadingOAuth.value = Boolean(id); if (!id) return; try { const credential = await credentialsStore.getCredentialData({ id }); if (stale) return; + oauthConnection.value = isOAuthCredentialConnected(storedCredential.value, credential); const data = credential?.data; - if (credential?.isResolvable) oauthConnection.value = credential.connectedByMe; if (data && typeof data === 'object') { - if (!credential?.isResolvable) { - oauthConnection.value = - Boolean( - data.grantType && !['authorizationCode', 'pkce'].includes(String(data.grantType)), - ) || hasOAuthTokenData(credential); - } // Stored client fields identify a custom app even when managed OAuth is available. const customClient = Boolean( (data.clientId && data.clientSecret) || (data.consumerKey && data.consumerSecret), diff --git a/packages/frontend/editor-ui/src/features/credentials/composables/oauthCallback.ts b/packages/frontend/editor-ui/src/features/credentials/composables/oauthCallback.ts index afe02970516..8c32a4617a0 100644 --- a/packages/frontend/editor-ui/src/features/credentials/composables/oauthCallback.ts +++ b/packages/frontend/editor-ui/src/features/credentials/composables/oauthCallback.ts @@ -1,4 +1,6 @@ +import { getResourcePermissions } from '@n8n/permissions'; import { CREDENTIAL_EMPTY_VALUE } from 'n8n-workflow'; +import type { ICredentialsDecryptedResponse, ICredentialsResponse } from '../credentials.types'; export const OAUTH_CALLBACK_SUCCESS = 'success'; export const OAUTH_CALLBACK_ERROR = 'error'; @@ -71,6 +73,29 @@ export function hasOAuthTokenData(credential: unknown): boolean { return isOAuthTokenDataSet(credential.data); } +/** + * Whether a saved OAuth credential can authenticate now. `fetched` is the + * credential read with its data. Without data, fall back to the listing: a + * user who can read but not edit a shared credential counts as connected. + */ +export function isOAuthCredentialConnected( + stored: Pick | undefined, + fetched?: ICredentialsDecryptedResponse | ICredentialsResponse, +): boolean | undefined { + if (fetched?.isResolvable) return fetched.connectedByMe; + const data = fetched?.data; + if (data && typeof data === 'object') { + // Only the authorization code and PKCE grants need the user to sign in. + return ( + Boolean(data.grantType && !['authorizationCode', 'pkce'].includes(String(data.grantType))) || + isOAuthTokenDataSet(data) + ); + } + if (stored?.isResolvable) return stored.connectedByMe; + const permissions = getResourcePermissions(stored?.scopes).credential; + return permissions.read === true && !permissions.update; +} + export interface WaitForOAuthCallbackOptions { popup: Window; trustedOrigins: string[];