fix(API): Make conditional credential fields optional instead of forbidden (#32010)

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 {

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);
 		});
 	});
 

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;