mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
fix(editor): Wait for OAuth sign-in before the assistant credential card continues (#39998)
This commit is contained in:
parent
93c0a5c0ff
commit
0a1ed4e09b
|
|
@ -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<typeof useCredentialsStore>,
|
||||
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<typeof vi.spyOn>) {
|
||||
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[] = [
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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<Record<string, boolean | undefined>>({});
|
||||
|
||||
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"
|
||||
/>
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
|
|
@ -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<ICredentialsResponse, 'isResolvable' | 'connectedByMe' | 'scopes'> | 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[];
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user