From 0ab043c8bb3c0ccdb21c57a885d454eabaacbb35 Mon Sep 17 00:00:00 2001 From: Yuliia Pominchuk <31064937+yuliia-pominchuk@users.noreply.github.com> Date: Wed, 10 Jun 2026 09:11:48 +0200 Subject: [PATCH] fix(API): Make conditional credential fields optional instead of forbidden (#32010) --- packages/cli/src/public-api/types.ts | 4 +- .../__tests__/credentials.service.test.ts | 77 ++++++++++++++++++- .../credentials/credentials.service.ts | 26 ++++--- 3 files changed, 91 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/public-api/types.ts b/packages/cli/src/public-api/types.ts index e65204211c1..d6e3f698b9d 100644 --- a/packages/cli/src/public-api/types.ts +++ b/packages/cli/src/public-api/types.ts @@ -238,12 +238,10 @@ export type OffsetPagination = PaginationBase & { offset: number; numberOfTotalR export type CursorPagination = PaginationBase & { lastId: string; numberOfNextRecords: number }; export interface IRequired { required?: string[]; - not?: { required?: string[] }; } export interface IDependency { - if?: { properties: {} }; + if?: { properties: {}; required?: string[] }; then?: { allOf: IRequired[] }; - else?: { allOf: IRequired[] }; } export interface IJsonSchema { diff --git a/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.service.test.ts b/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.service.test.ts index a4fbabb98b3..efcf92b610f 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.service.test.ts +++ b/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.service.test.ts @@ -2,6 +2,7 @@ import type { CredentialsEntity, Project, SharedCredentials, User } from '@n8n/d import { CredentialsRepository, GLOBAL_OWNER_ROLE, GLOBAL_MEMBER_ROLE } from '@n8n/db'; import { Container } from '@n8n/di'; import { mock } from 'jest-mock-extended'; +import { validate, type Schema } from 'jsonschema'; import { Cipher, CipherAes256GCM, CipherAes256CBC, EncryptionKeyProxy } from 'n8n-core'; import type { InstanceSettings } from 'n8n-core'; import type { GenericValue, IDataObject, INodeProperties } from 'n8n-workflow'; @@ -120,6 +121,7 @@ describe('CredentialsService', () => { { name: 'secret', type: 'string', + required: true, displayName: 'Secret', default: '', displayOptions: { @@ -131,6 +133,7 @@ describe('CredentialsService', () => { { name: 'privateKey', type: 'string', + required: true, displayName: 'Private Key', default: '', displayOptions: { @@ -142,6 +145,7 @@ describe('CredentialsService', () => { { name: 'publicKey', type: 'string', + required: true, displayName: 'Public Key', default: '', displayOptions: { @@ -235,6 +239,7 @@ describe('CredentialsService', () => { { name: 'username', type: 'string', + required: true, displayName: 'Username', default: '', displayOptions: { @@ -246,6 +251,7 @@ describe('CredentialsService', () => { { name: 'password', type: 'string', + required: true, displayName: 'Password', default: '', displayOptions: { @@ -257,6 +263,7 @@ describe('CredentialsService', () => { { name: 'clientId', type: 'string', + required: true, displayName: 'Client ID', default: '', displayOptions: { @@ -297,6 +304,7 @@ describe('CredentialsService', () => { { name: 'createField', type: 'string', + required: true, displayName: 'Create Field', default: '', displayOptions: { @@ -308,6 +316,7 @@ describe('CredentialsService', () => { { name: 'updateField', type: 'string', + required: true, displayName: 'Update Field', default: '', displayOptions: { @@ -319,6 +328,7 @@ describe('CredentialsService', () => { { name: 'deleteField', type: 'string', + required: true, displayName: 'Delete Field', default: '', displayOptions: { @@ -369,6 +379,7 @@ describe('CredentialsService', () => { { name: 'field3', type: 'string', + required: true, displayName: 'Field 3', default: '', displayOptions: { @@ -408,10 +419,68 @@ describe('CredentialsService', () => { // then block requires field3 when field2 === false expect(condition.then?.allOf.some((req: any) => req.required?.includes('field3'))).toBe(true); - // else block forbids field3 when field2 !== false - expect( - condition.else?.allOf.some((notReq: any) => notReq.not?.required?.includes('field3')), - ).toBe(true); + // no else block: hidden fields are optional, not forbidden + expect((condition as any).else).toBeUndefined(); + }); + + it('should not forbid conditional fields belonging to inactive conditions', () => { + // Mirrors the mongoDb scenario: two fields depend on the same options field + // but with different values. A payload that includes both the active + // and the inactive conditional field must validate. + const properties: INodeProperties[] = [ + { + name: 'configurationType', + type: 'options', + options: [ + { value: 'connectionString', name: 'Connection String' }, + { value: 'values', name: 'Values' }, + ], + displayName: 'Configuration Type', + default: 'values', + }, + { + name: 'connectionString', + type: 'string', + required: true, + displayName: 'Connection String', + default: '', + displayOptions: { show: { configurationType: ['connectionString'] } }, + }, + { + name: 'host', + type: 'string', + required: true, + displayName: 'Host', + default: '', + displayOptions: { show: { configurationType: ['values'] } }, + }, + ]; + + const schema = toJsonSchema(properties); + + // No `not: { required }` anywhere in the generated schema + expect(JSON.stringify(schema)).not.toContain('"not"'); + + // A payload using connectionString but also carrying the inactive `host` field + // must be accepted. + const connectionStringPayload = { + configurationType: 'connectionString', + connectionString: 'mongodb://localhost:27017/mydb', + host: 'localhost', + }; + expect(validate(connectionStringPayload, schema as unknown as Schema).valid).toBe(true); + + // And the reverse direction. + const valuesPayload = { + configurationType: 'values', + host: 'localhost', + connectionString: 'mongodb://localhost:27017/mydb', + }; + expect(validate(valuesPayload, schema as unknown as Schema).valid).toBe(true); + + // Required fields are still enforced via the `then` block. + const missingRequired = { configurationType: 'values' }; + expect(validate(missingRequired, schema as unknown as Schema).valid).toBe(false); }); }); diff --git a/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts b/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts index 61e8f749170..92108b44b66 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts +++ b/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts @@ -439,28 +439,36 @@ export function toJsonSchema(properties: INodeProperties[]): IDataObject { properties: { [dependantName]: conditionalValue, }, + // Require the controlling field in the `if` so the condition only + // matches when it is actually present and equal. Without this, an + // absent controlling field makes `properties` vacuously true and the + // `then` block would fire unexpectedly. + required: [dependantName], }, then: { allOf: [], }, - else: { - allOf: [], - }, }; resolveProperties.push(dependencyKey); } - propertyRequiredDependencies[dependencyKey].then?.allOf.push({ required: [property.name] }); - propertyRequiredDependencies[dependencyKey].else?.allOf.push({ - not: { required: [property.name] }, - }); - // remove global required + // Only enforce a field as required when the credential actually marks it `required`. + if (property.required) { + propertyRequiredDependencies[dependencyKey].then?.allOf.push({ + required: [property.name], + }); + } + // Requiredness is now conditional, so drop it from the global required list. requiredFields = requiredFields.filter((field) => field !== property.name); } }); Object.assign(jsonSchema, { required: requiredFields }); - jsonSchema.allOf = Object.values(propertyRequiredDependencies); + // Drop conditionals that ended up with no required fields, so credentials whose + // conditional fields are all optional produce no `allOf` constraints. + jsonSchema.allOf = Object.values(propertyRequiredDependencies).filter( + (dependency) => (dependency.then?.allOf.length ?? 0) > 0, + ); if (!jsonSchema.allOf.length) { delete jsonSchema.allOf;