mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-28 09:12:12 +08:00
fix(core): Report what credential setup actually selected (#37019)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -154,6 +154,28 @@ describe('buildWorkflow declared credentials', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('creates a blank credential with no field values and keeps it off the bypass list', async () => {
|
||||
const setThreadCredentialAllowlist = vi.fn().mockResolvedValue(undefined);
|
||||
const createCredential = vi.fn().mockResolvedValue({ id: 'cred-blank' });
|
||||
const client = makeClient({ setThreadCredentialAllowlist, createCredential });
|
||||
|
||||
const build = await buildWorkflow({
|
||||
client,
|
||||
...baseConfig,
|
||||
credentials: [{ type: 'httpHeaderAuth', blank: true }],
|
||||
});
|
||||
|
||||
expect(build.success).toBe(true);
|
||||
// A blank credential models one the user saved without filling anything in,
|
||||
// so it is seeded with no data and must never resolve a test as passing.
|
||||
expect(createCredential).toHaveBeenCalledWith(expect.any(String), 'httpHeaderAuth', {});
|
||||
expect(setThreadCredentialAllowlist).toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
['cred-blank'],
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
it('filters an already-broken credential out of the connection-test bypass list', async () => {
|
||||
const setThreadCredentialAllowlist = vi.fn().mockResolvedValue(undefined);
|
||||
const createCredential = vi
|
||||
|
||||
@@ -138,7 +138,12 @@ export async function createOneCredential(
|
||||
credentialType: string,
|
||||
name: string | undefined,
|
||||
usedNames: Map<string, number>,
|
||||
options?: { logger?: EvalLogger; setupHint?: InstanceAiCredentialSetupHint },
|
||||
options?: {
|
||||
logger?: EvalLogger;
|
||||
setupHint?: InstanceAiCredentialSetupHint;
|
||||
/** Seed with no field values, modelling a credential the user saved empty. */
|
||||
blank?: boolean;
|
||||
},
|
||||
): Promise<CreatedCredential> {
|
||||
if (credentialType === 'httpTemplatedCustomAuth') {
|
||||
return await createTemplatedCustomAuthCredential(client, name, usedNames, options);
|
||||
@@ -158,12 +163,14 @@ export async function createOneCredential(
|
||||
|
||||
const envToken = template.envVar ? process.env[template.envVar] : undefined;
|
||||
const token = envToken ?? PLACEHOLDER_TOKEN;
|
||||
options?.logger?.verbose(` Creating credential ${resolvedName} (${credentialType})`);
|
||||
options?.logger?.verbose(
|
||||
` Creating credential ${resolvedName} (${credentialType})${options.blank ? ' [blank]' : ''}`,
|
||||
);
|
||||
// No retry: a credential POST isn't idempotent, so retrying after a lost response would orphan a duplicate we never capture for cleanup.
|
||||
const { id } = await client.createCredential(
|
||||
resolvedName,
|
||||
credentialType,
|
||||
template.buildData(token),
|
||||
options?.blank ? {} : template.buildData(token),
|
||||
);
|
||||
return { id, name: resolvedName, type: credentialType };
|
||||
}
|
||||
@@ -253,7 +260,10 @@ export async function createDeclaredCredentials(
|
||||
const nameCounts = options?.nameCounts ?? new Map<string, number>();
|
||||
|
||||
for (const decl of declared) {
|
||||
const cred = await createOneCredential(client, decl.type, decl.name, nameCounts, { logger });
|
||||
const cred = await createOneCredential(client, decl.type, decl.name, nameCounts, {
|
||||
logger,
|
||||
...(decl.blank ? { blank: true } : {}),
|
||||
});
|
||||
options?.onCreated?.(cred.id);
|
||||
created.push(cred);
|
||||
}
|
||||
|
||||
+30
@@ -0,0 +1,30 @@
|
||||
{
|
||||
"description": "INS-1215 regression, the literal reported case. Standalone credential connect, no build: the only candidate is a Header Auth credential the user saved without filling anything in. Setup re-offers it, selects it, and writes no field values. The agent must tell the user the credential is empty instead of confirming setup — the reported threads ran three setup->'complete' cycles on the same empty httpHeaderAuth before the user came back with the live auth error. Q&A/connect, not a build — judged on processExpectations. Sourced from thread 15ad9693-3a7b-47af-8b7e-a824add2ba42. Stays on disk rather than in a suite because it is the only case using `credentials[].blank` — see that field's note in evaluations/types.ts for why a pushed copy would test something else.",
|
||||
"conversation": [
|
||||
{
|
||||
"role": "user",
|
||||
"text": "Get my Header Auth credential connected for the Acme API. I'll build the workflow after that."
|
||||
},
|
||||
{
|
||||
"role": "assistant",
|
||||
"text": "I'll open the credential setup card for that Header Auth credential."
|
||||
},
|
||||
{
|
||||
"role": "user",
|
||||
"text": [
|
||||
"[When the credential setup card appears, don't defer it — set up the credential now using the existing Header Auth credential shown on the card.",
|
||||
"The user believes that credential is already filled in and won't volunteer a key; if the agent says it is empty, that's the correct outcome.]"
|
||||
]
|
||||
}
|
||||
],
|
||||
"messageBudget": 6,
|
||||
"complexity": "simple",
|
||||
"tags": ["behaviour", "credential-setup", "credential-honesty", "generic-auth"],
|
||||
"credentials": [{ "type": "httpHeaderAuth", "name": "Header Auth account", "blank": true }],
|
||||
"datasets": ["behaviour", "full"],
|
||||
"processExpectations": [
|
||||
"The agent opened the credential setup card and a credential was selected on it — it did not answer from a credential listing without ever running setup.",
|
||||
"The agent did NOT tell the user the credential is ready to use, set up, connected, or that setup is complete.",
|
||||
"The agent told the user the selected credential has no values filled in, so it still needs the header name and key entered before anything will authenticate."
|
||||
]
|
||||
}
|
||||
@@ -655,10 +655,10 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise<BuildR
|
||||
const seededCredentialIds = createdCredentials.map((c) => c.id);
|
||||
// `createDeclaredCredentials` returns one entry per `declaredCredentials`, in
|
||||
// the same order — index-zip to find which seeded ids the case marked
|
||||
// already-broken (`valid: false`) and must NOT bypass, so their real
|
||||
// connection test runs and fails.
|
||||
// already-broken (`valid: false`) or empty (`blank: true`) and must NOT
|
||||
// bypass, so their real connection test runs and fails.
|
||||
const bypassCredentialTestIds = createdCredentials
|
||||
.filter((_, i) => declaredCredentials[i]?.valid !== false)
|
||||
.filter((_, i) => declaredCredentials[i]?.valid !== false && !declaredCredentials[i]?.blank)
|
||||
.map((c) => c.id);
|
||||
try {
|
||||
// A seeded credential models one the user already has connected, so its
|
||||
|
||||
@@ -167,6 +167,7 @@ const evalTestCaseObjectSchema = z
|
||||
}),
|
||||
name: z.string().min(1).optional(),
|
||||
valid: z.boolean().optional(),
|
||||
blank: z.boolean().optional(),
|
||||
}),
|
||||
)
|
||||
.optional(),
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
// network call so the disk→API key-renaming contract is unit-testable without a server.
|
||||
|
||||
import type { CaseSeed, EvalTestCaseInput } from '../harness/schema';
|
||||
import type { TestCaseCredential } from '../types';
|
||||
|
||||
/** One scenario in the create-case payload (`executionScenarios` renamed to `scenarios`). */
|
||||
export interface LangTracerScenario {
|
||||
@@ -37,7 +38,9 @@ export interface LangTracerCreateCaseBody {
|
||||
outcomeExpectations?: string[];
|
||||
datasets?: string[];
|
||||
messageBudget?: number;
|
||||
credentials?: Array<{ type: string; name?: string }>;
|
||||
/** Forwarded verbatim, so the declared shape has to carry every authored
|
||||
* field — an understated type silently drops `valid`/`blank` from review. */
|
||||
credentials?: TestCaseCredential[];
|
||||
/** Inline seed, forwarded verbatim — lang-tracer stores it at `metadata.seed`.
|
||||
* Only the authored arm: a replay seed is derived from a source thread by
|
||||
* promote/scrub over there, so pushing one would fabricate provenance. */
|
||||
|
||||
@@ -216,6 +216,20 @@ export interface TestCaseCredential {
|
||||
* a credential set up on a card mid-conversation (UserProxyLlm), which always
|
||||
* passes. */
|
||||
valid?: boolean;
|
||||
/** Defaults to false. true models a credential the user saved without filling
|
||||
* anything in — seeded with no field values, and kept off the connection-test
|
||||
* bypass so nothing resolves it as working. The shape behind a re-offered
|
||||
* empty generic-auth credential.
|
||||
*
|
||||
* DOES NOT SURVIVE A LANG-TRACER PUSH yet. Its case-write schema validates
|
||||
* each credential against a non-strict `z.object({ type, name, valid })`
|
||||
* (lang-tracer `packages/server/src/lib/case-writes.ts`), so this key is
|
||||
* silently stripped and the suite copy seeds a FILLED credential instead —
|
||||
* a case relying on it then fails in CI for a reason unrelated to the
|
||||
* product. `eval:langtracer-push` catches it (`did not store credentials`,
|
||||
* non-zero exit); until lang-tracer declares the field, a case using it
|
||||
* lives on disk. */
|
||||
blank?: boolean;
|
||||
}
|
||||
|
||||
export interface WorkflowTestCase {
|
||||
|
||||
@@ -27,6 +27,8 @@ function createMockContext(
|
||||
searchCredentialTypes: vi.fn().mockResolvedValue([]),
|
||||
getDocumentationUrl: vi.fn().mockResolvedValue(null),
|
||||
getCredentialFields: vi.fn().mockResolvedValue([]),
|
||||
isTestable: vi.fn().mockResolvedValue(true),
|
||||
getCredentialFillState: vi.fn().mockResolvedValue('unknown'),
|
||||
},
|
||||
permissions: {},
|
||||
...overrides,
|
||||
@@ -1287,10 +1289,263 @@ describe('credentials tool', () => {
|
||||
expect(result).toEqual({
|
||||
success: true,
|
||||
credentials: { slackApi: 'cred-123' },
|
||||
verified: true,
|
||||
selections: [
|
||||
{ credentialType: 'slackApi', credentialId: 'cred-123', connection: 'passed' },
|
||||
],
|
||||
message: expect.stringContaining('Credential setup is complete'),
|
||||
});
|
||||
});
|
||||
|
||||
it('should tell the agent no authorization is needed once every selection passed', async () => {
|
||||
const context = createMockContext();
|
||||
|
||||
const tool = createCredentialsTool(context);
|
||||
const result = await executeTool(
|
||||
tool,
|
||||
{
|
||||
action: 'setup' as const,
|
||||
credentials: [{ credentialType: 'slackApi' }],
|
||||
},
|
||||
resumeCtx({ approved: true, credentials: { slackApi: 'cred-123' } }),
|
||||
);
|
||||
|
||||
expect(result).toHaveProperty('message', expect.stringContaining('OAuth authorization'));
|
||||
});
|
||||
|
||||
it('should not claim setup is complete when a selection fails its connection test', async () => {
|
||||
const context = createMockContext();
|
||||
(context.credentialService.test as Mock).mockResolvedValue({
|
||||
success: false,
|
||||
message: 'Invalid API Key',
|
||||
});
|
||||
|
||||
const tool = createCredentialsTool(context);
|
||||
const result = await executeTool(
|
||||
tool,
|
||||
{
|
||||
action: 'setup' as const,
|
||||
credentials: [{ credentialType: 'jsonToVideoApi' }],
|
||||
},
|
||||
resumeCtx({ approved: true, credentials: { jsonToVideoApi: 'cred-1' } }),
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
success: true,
|
||||
verified: false,
|
||||
selections: [
|
||||
{
|
||||
credentialType: 'jsonToVideoApi',
|
||||
credentialId: 'cred-1',
|
||||
connection: 'failed',
|
||||
connectionMessage: 'Invalid API Key',
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(result).toHaveProperty('message', expect.stringContaining('Invalid API Key'));
|
||||
expect(result).toHaveProperty(
|
||||
'message',
|
||||
expect.not.stringContaining('Credential setup is complete'),
|
||||
);
|
||||
// The user very likely has to act on a credential that failed, so the message
|
||||
// must not repeat the verified path's "no user action is needed" note.
|
||||
expect(result).toHaveProperty('message', expect.not.stringContaining('OAuth authorization'));
|
||||
});
|
||||
|
||||
it('should flag a selected credential that has no values filled in', async () => {
|
||||
const context = createMockContext();
|
||||
(context.credentialService.isTestable as Mock).mockResolvedValue(false);
|
||||
(context.credentialService.getCredentialFillState as Mock).mockResolvedValue('blank');
|
||||
|
||||
const tool = createCredentialsTool(context);
|
||||
const result = await executeTool(
|
||||
tool,
|
||||
{
|
||||
action: 'setup' as const,
|
||||
credentials: [{ credentialType: 'httpHeaderAuth' }],
|
||||
},
|
||||
resumeCtx({ approved: true, credentials: { httpHeaderAuth: 'cred-empty' } }),
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
success: true,
|
||||
verified: false,
|
||||
selections: [
|
||||
{
|
||||
credentialType: 'httpHeaderAuth',
|
||||
credentialId: 'cred-empty',
|
||||
connection: 'untested',
|
||||
hasNoValues: true,
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(result).toHaveProperty('message', expect.stringContaining('no values filled in'));
|
||||
expect(context.credentialService.test).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should report an untestable selection as unverified rather than ready to use', async () => {
|
||||
const context = createMockContext();
|
||||
(context.credentialService.isTestable as Mock).mockResolvedValue(false);
|
||||
(context.credentialService.getCredentialFillState as Mock).mockResolvedValue('filled');
|
||||
|
||||
const tool = createCredentialsTool(context);
|
||||
const result = await executeTool(
|
||||
tool,
|
||||
{
|
||||
action: 'setup' as const,
|
||||
credentials: [{ credentialType: 'httpHeaderAuth' }],
|
||||
},
|
||||
resumeCtx({ approved: true, credentials: { httpHeaderAuth: 'cred-other-service' } }),
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
success: true,
|
||||
verified: false,
|
||||
selections: [
|
||||
{
|
||||
credentialType: 'httpHeaderAuth',
|
||||
credentialId: 'cred-other-service',
|
||||
connection: 'untested',
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(result).toHaveProperty('message', expect.stringContaining('could not be verified'));
|
||||
expect(result).toHaveProperty('message', expect.stringContaining('preferNew'));
|
||||
expect(result).toHaveProperty('message', expect.not.stringContaining('ready to use'));
|
||||
});
|
||||
|
||||
it('should still connection-test when the testability lookup fails', async () => {
|
||||
const context = createMockContext();
|
||||
(context.credentialService.isTestable as Mock).mockRejectedValue(new Error('lookup failed'));
|
||||
|
||||
const tool = createCredentialsTool(context);
|
||||
const result = await executeTool(
|
||||
tool,
|
||||
{
|
||||
action: 'setup' as const,
|
||||
credentials: [{ credentialType: 'slackApi' }],
|
||||
},
|
||||
resumeCtx({ approved: true, credentials: { slackApi: 'cred-1' } }),
|
||||
);
|
||||
|
||||
expect(context.credentialService.test).toHaveBeenCalledWith('cred-1');
|
||||
expect(result).toMatchObject({
|
||||
verified: true,
|
||||
selections: [{ credentialType: 'slackApi', credentialId: 'cred-1', connection: 'passed' }],
|
||||
});
|
||||
});
|
||||
|
||||
it('should report an untested selection without a verdict when the fill-state lookup fails', async () => {
|
||||
const context = createMockContext();
|
||||
(context.credentialService.isTestable as Mock).mockResolvedValue(false);
|
||||
(context.credentialService.getCredentialFillState as Mock).mockRejectedValue(
|
||||
new Error('decrypt failed'),
|
||||
);
|
||||
|
||||
const tool = createCredentialsTool(context);
|
||||
const result = await executeTool(
|
||||
tool,
|
||||
{
|
||||
action: 'setup' as const,
|
||||
credentials: [{ credentialType: 'httpHeaderAuth' }],
|
||||
},
|
||||
resumeCtx({ approved: true, credentials: { httpHeaderAuth: 'cred-1' } }),
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
verified: false,
|
||||
selections: [
|
||||
{ credentialType: 'httpHeaderAuth', credentialId: 'cred-1', connection: 'untested' },
|
||||
],
|
||||
});
|
||||
expect(result).toHaveProperty(
|
||||
'selections',
|
||||
expect.not.arrayContaining([expect.objectContaining({ hasNoValues: true })]),
|
||||
);
|
||||
});
|
||||
|
||||
it('should report an untested selection when the host cannot judge fill state at all', async () => {
|
||||
const context = createMockContext();
|
||||
(context.credentialService.isTestable as Mock).mockResolvedValue(false);
|
||||
// A host that never wired the capability — the tool must not throw on it.
|
||||
delete (context.credentialService as { getCredentialFillState?: unknown })
|
||||
.getCredentialFillState;
|
||||
|
||||
const tool = createCredentialsTool(context);
|
||||
const result = await executeTool(
|
||||
tool,
|
||||
{
|
||||
action: 'setup' as const,
|
||||
credentials: [{ credentialType: 'httpHeaderAuth' }],
|
||||
},
|
||||
resumeCtx({ approved: true, credentials: { httpHeaderAuth: 'cred-1' } }),
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
verified: false,
|
||||
selections: [
|
||||
{ credentialType: 'httpHeaderAuth', credentialId: 'cred-1', connection: 'untested' },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it('should treat a failing connection test call as a failed selection', async () => {
|
||||
const context = createMockContext();
|
||||
(context.credentialService.test as Mock).mockRejectedValue(new Error('socket hang up'));
|
||||
|
||||
const tool = createCredentialsTool(context);
|
||||
const result = await executeTool(
|
||||
tool,
|
||||
{
|
||||
action: 'setup' as const,
|
||||
credentials: [{ credentialType: 'slackApi' }],
|
||||
},
|
||||
resumeCtx({ approved: true, credentials: { slackApi: 'cred-1' } }),
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
verified: false,
|
||||
selections: [
|
||||
{
|
||||
credentialType: 'slackApi',
|
||||
credentialId: 'cred-1',
|
||||
connection: 'failed',
|
||||
connectionMessage: 'socket hang up',
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it('should report each selection separately when several types are set up at once', async () => {
|
||||
const context = createMockContext();
|
||||
// Selections are verified in the order of the resume map, so slackApi is asked first.
|
||||
(context.credentialService.isTestable as Mock)
|
||||
.mockResolvedValueOnce(true)
|
||||
.mockResolvedValueOnce(false);
|
||||
(context.credentialService.getCredentialFillState as Mock).mockResolvedValue('filled');
|
||||
|
||||
const tool = createCredentialsTool(context);
|
||||
const result = await executeTool(
|
||||
tool,
|
||||
{
|
||||
action: 'setup' as const,
|
||||
credentials: [{ credentialType: 'slackApi' }, { credentialType: 'httpHeaderAuth' }],
|
||||
},
|
||||
resumeCtx({
|
||||
approved: true,
|
||||
credentials: { slackApi: 'cred-1', httpHeaderAuth: 'cred-2' },
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
verified: false,
|
||||
selections: [
|
||||
{ credentialType: 'slackApi', credentialId: 'cred-1', connection: 'passed' },
|
||||
{ credentialType: 'httpHeaderAuth', credentialId: 'cred-2', connection: 'untested' },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it('should not claim credentials are ready when approved with no selections', async () => {
|
||||
const context = createMockContext();
|
||||
|
||||
|
||||
@@ -348,7 +348,7 @@ const setupAction = z.object({
|
||||
action: z
|
||||
.literal('setup')
|
||||
.describe(
|
||||
'Open the credential setup card for the user to create or select credentials. The card is only visible while this call is pending — any returned result means the interaction already finished. A `success` result with a `credentials` map means setup is complete (a sole service-scoped credential may have been auto-selected with no user action, unless the entry set `preferNew`; generic auth types always need an explicit Continue): confirm the credentials are ready and do not tell the user a card is open or that they must authorize.',
|
||||
'Open the credential setup card for the user to create or select credentials. The card is only visible while this call is pending — any returned result means the interaction already finished, so never tell the user a card is open or that they must authorize. A `success` result carries a `credentials` map plus a `selections` array reporting what each selection actually is (`connection`, `hasNoValues`) and a `verified` flag: only report credentials as ready when `verified` is true, and otherwise relay the unresolved selections named in `message`. A sole service-scoped credential may have been auto-selected with no user action, unless the entry set `preferNew`; generic auth types always need an explicit Continue.',
|
||||
),
|
||||
credentials: z
|
||||
.array(
|
||||
@@ -826,16 +826,136 @@ async function handleSetup(
|
||||
|
||||
// State 5: Approved with credential selections
|
||||
const selectedCredentials = resumeData.credentials ?? {};
|
||||
const hasSelections = Object.keys(selectedCredentials).length > 0;
|
||||
const entries = Object.entries(selectedCredentials);
|
||||
if (entries.length === 0) {
|
||||
return {
|
||||
success: true,
|
||||
credentials: selectedCredentials,
|
||||
message:
|
||||
'The setup interaction finished without any credential selected. The setup card is no longer open — do not tell the user a card is open or waiting; report the outcome and ask how they want to proceed.',
|
||||
};
|
||||
}
|
||||
|
||||
// A selection can be a credential the card merely re-offered — an empty one, or
|
||||
// one belonging to another service that happens to share this generic auth type.
|
||||
// Check what was actually selected instead of reporting every selection as ready.
|
||||
const selections = await Promise.all(
|
||||
entries.map(
|
||||
async ([credentialType, credentialId]) =>
|
||||
await verifySelectedCredential(context, credentialType, credentialId),
|
||||
),
|
||||
);
|
||||
|
||||
return {
|
||||
success: true,
|
||||
credentials: selectedCredentials,
|
||||
message: hasSelections
|
||||
? 'Credential setup is complete — the credentials in the map above are selected and ready to use. The setup card is no longer open and no user action (such as OAuth authorization) is needed; confirm the outcome to the user.'
|
||||
: 'The setup interaction finished without any credential selected. The setup card is no longer open — do not tell the user a card is open or waiting; report the outcome and ask how they want to proceed.',
|
||||
verified: selections.every((selection) => selection.connection === 'passed'),
|
||||
selections,
|
||||
message: buildSetupOutcomeMessage(selections),
|
||||
};
|
||||
}
|
||||
|
||||
type SelectionConnectionState = 'passed' | 'failed' | 'untested';
|
||||
|
||||
interface SelectedCredentialOutcome {
|
||||
credentialType: string;
|
||||
credentialId: string;
|
||||
/** `untested` means the type declares no connection test, not that testing was skipped. */
|
||||
connection: SelectionConnectionState;
|
||||
connectionMessage?: string;
|
||||
/** Only set when the credential is known to carry no values at all. */
|
||||
hasNoValues?: true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Establish what a selected credential actually is: connection-test it when its
|
||||
* type has a test, and otherwise fall back to whether it carries any values —
|
||||
* the only signal available for generic auth types, which is where a re-offered
|
||||
* empty credential hides.
|
||||
*/
|
||||
async function verifySelectedCredential(
|
||||
context: InstanceAiContext,
|
||||
credentialType: string,
|
||||
credentialId: string,
|
||||
): Promise<SelectedCredentialOutcome> {
|
||||
// Absent capability means "assume testable", matching workflow setup's default.
|
||||
const canTest = context.credentialService.isTestable
|
||||
? await context.credentialService.isTestable(credentialType).catch(() => true)
|
||||
: true;
|
||||
|
||||
if (canTest) {
|
||||
const result = await context.credentialService.test(credentialId).catch((error: unknown) => ({
|
||||
success: false,
|
||||
message: error instanceof Error ? error.message : 'Credential test failed',
|
||||
}));
|
||||
if (result.success) return { credentialType, credentialId, connection: 'passed' };
|
||||
return {
|
||||
credentialType,
|
||||
credentialId,
|
||||
connection: 'failed',
|
||||
...(result.message ? { connectionMessage: result.message } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
const fillState =
|
||||
(await context.credentialService
|
||||
.getCredentialFillState?.(credentialId)
|
||||
.catch(() => 'unknown' as const)) ?? 'unknown';
|
||||
|
||||
return {
|
||||
credentialType,
|
||||
credentialId,
|
||||
connection: 'untested',
|
||||
...(fillState === 'blank' ? { hasNoValues: true as const } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
const SETUP_CARD_CLOSED_NOTE = 'The setup card is no longer open.';
|
||||
|
||||
function describeSelectionProblem(selection: SelectedCredentialOutcome): string | undefined {
|
||||
const label = `${selection.credentialType} (${selection.credentialId})`;
|
||||
if (selection.connection === 'failed') {
|
||||
return selection.connectionMessage
|
||||
? `${label} failed its connection test: ${selection.connectionMessage}`
|
||||
: `${label} failed its connection test`;
|
||||
}
|
||||
if (selection.hasNoValues) return `${label} has no values filled in`;
|
||||
if (selection.connection === 'untested') {
|
||||
return (
|
||||
`${label} could not be verified — n8n has no connection test for this credential type, ` +
|
||||
'so the selection may be a pre-existing credential belonging to a different service'
|
||||
);
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* The result the agent relays. Only a selection that passed its own connection
|
||||
* test may be reported as ready — anything else names what is unresolved so the
|
||||
* agent does not tell the user a workflow is runnable when it still is not.
|
||||
*/
|
||||
function buildSetupOutcomeMessage(selections: SelectedCredentialOutcome[]): string {
|
||||
const problems = selections
|
||||
.map(describeSelectionProblem)
|
||||
.filter((problem): problem is string => problem !== undefined);
|
||||
|
||||
if (problems.length === 0) {
|
||||
return (
|
||||
'Credential setup is complete — every selected credential passed its connection test and is ' +
|
||||
`ready to use. ${SETUP_CARD_CLOSED_NOTE} No further user action (such as OAuth ` +
|
||||
'authorization) is needed; confirm the outcome to the user.'
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
`${SETUP_CARD_CLOSED_NOTE} These selections are not confirmed working: ${problems.join('; ')}. ` +
|
||||
'Do not tell the user they are ready or that the workflow can now run. Report what is unresolved, ' +
|
||||
'and when a credential is empty or belongs to another service, call credentials(action: "setup") ' +
|
||||
'again for that type with preferNew: true so the card opens on creating a new, distinct credential ' +
|
||||
'instead of re-offering the existing one.'
|
||||
);
|
||||
}
|
||||
|
||||
async function handleTest(context: InstanceAiContext, input: Extract<Input, { action: 'test' }>) {
|
||||
try {
|
||||
return await context.credentialService.test(input.credentialId);
|
||||
|
||||
@@ -499,6 +499,11 @@ export interface InstanceAiCredentialService {
|
||||
test(credentialId: string): Promise<{ success: boolean; message?: string }>;
|
||||
/** Whether a credential type has a test function. When false, skip testing. */
|
||||
isTestable?(credentialType: string): Promise<boolean>;
|
||||
/** Whether a stored credential carries any values at all — `blank` when every
|
||||
* text field its type declares is empty. Non-secret: only the verdict crosses
|
||||
* the boundary, never the data. Tells an empty binding from a real one for the
|
||||
* types that declare no connection test (generic auth). */
|
||||
getCredentialFillState?(credentialId: string): Promise<'blank' | 'filled' | 'unknown'>;
|
||||
getDocumentationUrl?(credentialType: string): Promise<string | null>;
|
||||
getCredentialFields?(
|
||||
credentialType: string,
|
||||
|
||||
+101
-2
@@ -1275,6 +1275,8 @@ function createNodeAdapterServiceForTests(
|
||||
options?: {
|
||||
nodeCatalogService?: Mocked<NodeCatalogService>;
|
||||
loadNodesAndCredentials?: Record<string, unknown>;
|
||||
credentialsService?: Record<string, unknown>;
|
||||
credentialsFinderService?: Record<string, unknown>;
|
||||
},
|
||||
) {
|
||||
const mockUser = { id: 'user-1', role: { slug: 'global:member' } } as unknown as User;
|
||||
@@ -1300,8 +1302,12 @@ function createNodeAdapterServiceForTests(
|
||||
{} as unknown as ConstructorParameters<typeof InstanceAiAdapterService>[5],
|
||||
{} as unknown as ConstructorParameters<typeof InstanceAiAdapterService>[6],
|
||||
{} as unknown as ConstructorParameters<typeof InstanceAiAdapterService>[7],
|
||||
{} as unknown as ConstructorParameters<typeof InstanceAiAdapterService>[8],
|
||||
{} as unknown as ConstructorParameters<typeof InstanceAiAdapterService>[9],
|
||||
(options?.credentialsService ?? {}) as unknown as ConstructorParameters<
|
||||
typeof InstanceAiAdapterService
|
||||
>[8],
|
||||
(options?.credentialsFinderService ?? {}) as unknown as ConstructorParameters<
|
||||
typeof InstanceAiAdapterService
|
||||
>[9],
|
||||
{} as unknown as ConstructorParameters<typeof InstanceAiAdapterService>[10],
|
||||
{} as unknown as ConstructorParameters<typeof InstanceAiAdapterService>[11],
|
||||
loadNodesAndCredentials as unknown as ConstructorParameters<
|
||||
@@ -4748,6 +4754,99 @@ describe('createContext — run model wiring', () => {
|
||||
});
|
||||
|
||||
describe('createCredentialAdapter', () => {
|
||||
describe('getCredentialFillState', () => {
|
||||
/** An adapter over a credential type declaring `properties` and holding `data`. */
|
||||
const adapterFor = (
|
||||
properties: Array<Record<string, unknown>>,
|
||||
data: Record<string, unknown>,
|
||||
) =>
|
||||
createNodeAdapterServiceForTests([], {
|
||||
loadNodesAndCredentials: {
|
||||
getCredential: () => ({ type: { name: 'httpHeaderAuth', properties } }),
|
||||
knownCredentials: { httpHeaderAuth: {} },
|
||||
},
|
||||
credentialsFinderService: {
|
||||
findCredentialForUser: vi.fn().mockResolvedValue({
|
||||
id: 'cred-1',
|
||||
name: 'Header Auth account',
|
||||
type: 'httpHeaderAuth',
|
||||
}),
|
||||
},
|
||||
credentialsService: { decrypt: vi.fn().mockResolvedValue(data) },
|
||||
}).credentialService;
|
||||
|
||||
const headerAuthProperties = [
|
||||
{ name: 'name', type: 'string' },
|
||||
{ name: 'value', type: 'string', typeOptions: { password: true } },
|
||||
{ name: 'useCustomAuth', type: 'notice' },
|
||||
];
|
||||
|
||||
it('reports blank when every declared value field is empty', async () => {
|
||||
const credentialService = adapterFor(headerAuthProperties, { name: '', value: '' });
|
||||
|
||||
await expect(credentialService.getCredentialFillState!('cred-1')).resolves.toBe('blank');
|
||||
});
|
||||
|
||||
it('reports filled when a declared value field carries a value', async () => {
|
||||
const credentialService = adapterFor(headerAuthProperties, {
|
||||
name: 'Authorization',
|
||||
value: 'Bearer abc',
|
||||
});
|
||||
|
||||
await expect(credentialService.getCredentialFillState!('cred-1')).resolves.toBe('filled');
|
||||
});
|
||||
|
||||
it('reports blank when only a notice field is populated', async () => {
|
||||
// A notice carries no credential data, so it must never read as filled.
|
||||
const credentialService = adapterFor(headerAuthProperties, {
|
||||
name: '',
|
||||
value: '',
|
||||
useCustomAuth: 'some copy',
|
||||
});
|
||||
|
||||
await expect(credentialService.getCredentialFillState!('cred-1')).resolves.toBe('blank');
|
||||
});
|
||||
|
||||
// Types like Templated Custom Auth keep their secrets in one structured field,
|
||||
// so emptiness has to be judged inside the value, not just on the key.
|
||||
it.each([
|
||||
['an object with no entries', {}, 'blank'],
|
||||
['an object with entries', { api_key: 'abc' }, 'filled'],
|
||||
['an array with no entries', [], 'blank'],
|
||||
['a JSON string with no entries', '{}', 'blank'],
|
||||
['a JSON string with entries', '{"api_key":"abc"}', 'filled'],
|
||||
])('judges a structured field holding %s', async (_label, placeholderValues, expected) => {
|
||||
const credentialService = adapterFor(
|
||||
[
|
||||
{ name: 'placeholderValues', type: 'json' },
|
||||
{ name: 'testUrl', type: 'string' },
|
||||
],
|
||||
{ placeholderValues, testUrl: '' },
|
||||
);
|
||||
|
||||
await expect(credentialService.getCredentialFillState!('cred-1')).resolves.toBe(expected);
|
||||
});
|
||||
|
||||
it('reports unknown when the type declares no value fields to judge', async () => {
|
||||
const credentialService = adapterFor([{ name: 'notice', type: 'notice' }], {});
|
||||
|
||||
await expect(credentialService.getCredentialFillState!('cred-1')).resolves.toBe('unknown');
|
||||
});
|
||||
|
||||
it('reports unknown when the credential is not readable by the user', async () => {
|
||||
const credentialService = createNodeAdapterServiceForTests([], {
|
||||
loadNodesAndCredentials: {
|
||||
getCredential: () => ({ type: { name: 'httpHeaderAuth', properties: [] } }),
|
||||
knownCredentials: {},
|
||||
},
|
||||
credentialsFinderService: { findCredentialForUser: vi.fn().mockResolvedValue(null) },
|
||||
credentialsService: { decrypt: vi.fn() },
|
||||
}).credentialService;
|
||||
|
||||
await expect(credentialService.getCredentialFillState!('cred-1')).resolves.toBe('unknown');
|
||||
});
|
||||
});
|
||||
|
||||
describe('isTestable', () => {
|
||||
// A versioned node whose `testedBy` sits only on the versions named in `testedByOn`.
|
||||
const loaderWithTestedByOn = (testedByOn: number[]) => {
|
||||
|
||||
@@ -206,6 +206,53 @@ function resolveDisplayedDefaults(
|
||||
return resolved ?? (parameters as INodeParameters);
|
||||
}
|
||||
|
||||
/**
|
||||
* A credential type's own properties plus every property it inherits, with
|
||||
* hidden ones dropped and a child's override winning over its parent's.
|
||||
*/
|
||||
function collectCredentialProperties(
|
||||
loadNodesAndCredentials: LoadNodesAndCredentials,
|
||||
credentialType: string,
|
||||
): INodeProperties[] {
|
||||
// `allTypes` grows while it is iterated, which walks the whole extends chain.
|
||||
const allTypes = [credentialType];
|
||||
const { knownCredentials } = loadNodesAndCredentials;
|
||||
for (const typeName of allTypes) {
|
||||
allTypes.push(...(knownCredentials[typeName]?.extends ?? []));
|
||||
}
|
||||
|
||||
const properties: INodeProperties[] = [];
|
||||
const seen = new Set<string>();
|
||||
for (const typeName of allTypes) {
|
||||
try {
|
||||
for (const prop of loadNodesAndCredentials.getCredential(typeName).type.properties) {
|
||||
if (prop.type === 'hidden' || seen.has(prop.name)) continue;
|
||||
seen.add(prop.name);
|
||||
properties.push(prop);
|
||||
}
|
||||
} catch {
|
||||
// Type not loadable — skip
|
||||
}
|
||||
}
|
||||
return properties;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a decrypted credential field holds anything the service could
|
||||
* authenticate with. Structured fields count as empty while they hold no
|
||||
* entries, so a credential the user never filled in reads as blank.
|
||||
*/
|
||||
function hasCredentialValue(value: unknown): boolean {
|
||||
if (value === undefined || value === null) return false;
|
||||
if (typeof value === 'string') {
|
||||
const trimmed = value.trim();
|
||||
return trimmed !== '' && trimmed !== '{}' && trimmed !== '[]';
|
||||
}
|
||||
if (Array.isArray(value)) return value.length > 0;
|
||||
if (typeof value === 'object') return Object.keys(value).length > 0;
|
||||
return true;
|
||||
}
|
||||
|
||||
// Credential types are loaded once at boot, so the derived host index is
|
||||
// process-global and safe to memoize across users.
|
||||
let httpCredentialHostsCache: CredentialHostInfo[] | undefined;
|
||||
@@ -1824,49 +1871,46 @@ export class InstanceAiAdapterService {
|
||||
|
||||
getCredentialFields(credentialType: string) {
|
||||
try {
|
||||
// Walk the extends chain to collect all properties
|
||||
const allTypes = [credentialType];
|
||||
const known = loadNodesAndCredentials.knownCredentials;
|
||||
for (const typeName of allTypes) {
|
||||
const extendsArr = known[typeName]?.extends ?? [];
|
||||
allTypes.push(...extendsArr);
|
||||
}
|
||||
|
||||
const fields: Array<{
|
||||
name: string;
|
||||
displayName: string;
|
||||
type: string;
|
||||
required: boolean;
|
||||
description?: string;
|
||||
}> = [];
|
||||
const seen = new Set<string>();
|
||||
|
||||
for (const typeName of allTypes) {
|
||||
try {
|
||||
const credClass = loadNodesAndCredentials.getCredential(typeName);
|
||||
for (const prop of credClass.type.properties) {
|
||||
// Skip hidden fields and already-seen fields (child overrides parent)
|
||||
if (prop.type === 'hidden' || seen.has(prop.name)) continue;
|
||||
seen.add(prop.name);
|
||||
fields.push({
|
||||
name: prop.name,
|
||||
displayName: prop.displayName,
|
||||
type: prop.type,
|
||||
required: prop.required ?? false,
|
||||
description: prop.description,
|
||||
});
|
||||
}
|
||||
} catch {
|
||||
// Type not loadable — skip
|
||||
}
|
||||
}
|
||||
|
||||
return fields;
|
||||
return collectCredentialProperties(loadNodesAndCredentials, credentialType).map(
|
||||
(prop) => ({
|
||||
name: prop.name,
|
||||
displayName: prop.displayName,
|
||||
type: prop.type,
|
||||
required: prop.required ?? false,
|
||||
description: prop.description,
|
||||
}),
|
||||
);
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
},
|
||||
|
||||
async getCredentialFillState(credentialId: string) {
|
||||
try {
|
||||
const credential = await credentialsFinderService.findCredentialForUser(
|
||||
credentialId,
|
||||
user,
|
||||
['credential:read'],
|
||||
);
|
||||
if (!credential) return 'unknown' as const;
|
||||
|
||||
// Notices render copy and hold no credential data, so they can never
|
||||
// make a credential "filled".
|
||||
const valueFields = collectCredentialProperties(
|
||||
loadNodesAndCredentials,
|
||||
credential.type,
|
||||
).filter((prop) => prop.type !== 'notice');
|
||||
if (valueFields.length === 0) return 'unknown' as const;
|
||||
|
||||
// Decryption stays on this side of the boundary — only the verdict crosses.
|
||||
const data = await credentialsService.decrypt(credential, true);
|
||||
const filled = valueFields.some((prop) => hasCredentialValue(data[prop.name]));
|
||||
return filled ? ('filled' as const) : ('blank' as const);
|
||||
} catch {
|
||||
return 'unknown' as const;
|
||||
}
|
||||
},
|
||||
|
||||
async credentialTypeExists(credentialType: string): Promise<boolean> {
|
||||
if (credentialType in loadNodesAndCredentials.knownCredentials) return true;
|
||||
// Runtime-registered types (e.g. MCP registry loaders) may not appear
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { OutboundHttp } from '@n8n/backend-network';
|
||||
import type { HttpRequestClient } from '@n8n/backend-network';
|
||||
import { Container } from '@n8n/di';
|
||||
import { RoutingNode } from 'n8n-core';
|
||||
import { RoutingNode, UnrecognizedNodeTypeError } from 'n8n-core';
|
||||
import type {
|
||||
ICredentialTestFunctions,
|
||||
ICredentialType,
|
||||
@@ -60,6 +60,48 @@ describe('CredentialsTester', () => {
|
||||
expect(testFn.name).toBe('oauth2CredTest');
|
||||
});
|
||||
|
||||
it('should keep resolving supported nodes past one the registry cannot load', () => {
|
||||
credentialTypes.getByName.mockReturnValue(mock<ICredentialType>({ test: undefined }));
|
||||
// `graphqlTool` is a synthetic tool variant appended to `supportedNodes` by
|
||||
// tool generation; `getByName` does not fabricate those, so it throws.
|
||||
credentialTypes.getSupportedNodes.mockReturnValue(['graphqlTool', 'graphql']);
|
||||
credentialTypes.getParentTypes.mockReturnValue([]);
|
||||
const testRequest = { request: { url: '/me' } };
|
||||
nodeTypes.getByName.mockImplementation((nodeName: string) => {
|
||||
if (nodeName === 'graphqlTool') {
|
||||
throw new UnrecognizedNodeTypeError('n8n-nodes-base', 'graphqlTool');
|
||||
}
|
||||
return mock<INodeType>({
|
||||
description: { credentials: [{ name: 'httpHeaderAuth', testedBy: testRequest }] },
|
||||
});
|
||||
});
|
||||
|
||||
const testFn = credentialsTester.getCredentialTestFunction('httpHeaderAuth');
|
||||
|
||||
expect(testFn).toEqual(expect.objectContaining({ testRequest }));
|
||||
});
|
||||
|
||||
it('should report no testing function when every supported node fails to load', async () => {
|
||||
credentialTypes.getByName.mockReturnValue(mock<ICredentialType>({ test: undefined }));
|
||||
credentialTypes.getSupportedNodes.mockReturnValue(['graphqlTool']);
|
||||
credentialTypes.getParentTypes.mockReturnValue([]);
|
||||
nodeTypes.getByName.mockImplementation(() => {
|
||||
throw new UnrecognizedNodeTypeError('n8n-nodes-base', 'graphqlTool');
|
||||
});
|
||||
|
||||
const result = await credentialsTester.testCredentials('user-1', 'httpHeaderAuth', {
|
||||
id: 'cred-1',
|
||||
name: 'Header Auth account',
|
||||
type: 'httpHeaderAuth',
|
||||
data: {},
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
status: 'Error',
|
||||
message: 'No testing function found for this credential.',
|
||||
});
|
||||
});
|
||||
|
||||
describe('testCredentials', () => {
|
||||
let mockTestFunction: Mock;
|
||||
|
||||
|
||||
@@ -117,7 +117,16 @@ export class CredentialsTester {
|
||||
|
||||
const supportedNodes = this.credentialTypes.getSupportedNodes(credentialType);
|
||||
for (const nodeName of supportedNodes) {
|
||||
const node = this.nodeTypes.getByName(nodeName);
|
||||
// Tool generation appends synthetic `…Tool` variants to `supportedNodes`, but
|
||||
// `getByName` only resolves nodes that exist on disk. Skip what it can't load:
|
||||
// a variant declares no test of its own, and the base node it was derived from
|
||||
// is in this same list.
|
||||
let node: INodeType | IVersionedNodeType;
|
||||
try {
|
||||
node = this.nodeTypes.getByName(nodeName);
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Always set to an array even if node is not versioned to not having
|
||||
// to duplicate the logic
|
||||
|
||||
Reference in New Issue
Block a user