fix(core): Prevent credentials saving if missing required properties

packages/@n8n/backend-test-utils/src/random.ts

@@ -48,7 +48,7 @@ export const randomCredentialPayload = ({
 	isGlobal,
 }: { isManaged?: boolean; isGlobal?: boolean } = {}): CredentialPayload => ({
 	name: randomName(),
-	type: randomName(),
+	type: 'githubApi',
 	data: { accessToken: randomString(6, 16) },
 	isManaged,
 	isGlobal,

packages/cli/src/credentials/__tests__/credentials.service.test.ts

@@ -1,3 +1,4 @@
+import type { Logger } from '@n8n/backend-common';
 import type {
 	CredentialsEntity,
 	CredentialsRepository,
@@ -10,17 +11,17 @@ import { GLOBAL_OWNER_ROLE, GLOBAL_MEMBER_ROLE } from '@n8n/db';
 import { mock } from 'jest-mock-extended';
 import { CREDENTIAL_ERRORS, CredentialDataError, Credentials, type ErrorReporter } from 'n8n-core';
 import { CREDENTIAL_EMPTY_VALUE, type ICredentialType } from 'n8n-workflow';
-import type { Logger } from '@n8n/backend-common';
 
 import { CREDENTIAL_BLANKING_VALUE } from '@/constants';
 import type { CredentialTypes } from '@/credential-types';
-import { CredentialsService } from '@/credentials/credentials.service';
 import type { CredentialsFinderService } from '@/credentials/credentials-finder.service';
+import { CredentialsService } from '@/credentials/credentials.service';
+import type { CredentialsHelper } from '@/credentials-helper';
+import type { ExternalHooks } from '@/external-hooks';
+import type { CredentialsTester } from '@/services/credentials-tester.service';
 import type { OwnershipService } from '@/services/ownership.service';
 import type { ProjectService } from '@/services/project.service.ee';
 import type { RoleService } from '@/services/role.service';
-import type { CredentialsTester } from '@/services/credentials-tester.service';
-import type { ExternalHooks } from '@/external-hooks';
 
 describe('CredentialsService', () => {
 	const credType = mock<ICredentialType>({
@@ -54,6 +55,7 @@ describe('CredentialsService', () => {
 	const roleService = mock<RoleService>();
 	const userRepository = mock<UserRepository>();
 	const credentialsFinderService = mock<CredentialsFinderService>();
+	const credentialsHelper = mock<CredentialsHelper>();
 
 	const service = new CredentialsService(
 		credentialsRepository,
@@ -69,6 +71,7 @@ describe('CredentialsService', () => {
 		roleService,
 		userRepository,
 		credentialsFinderService,
+		credentialsHelper,
 	);
 
 	beforeEach(() => jest.resetAllMocks());
@@ -1383,6 +1386,7 @@ describe('CredentialsService', () => {
 				id: 'project-1',
 			} as any);
 			projectService.getProjectRelationsForUser.mockResolvedValue([]);
+			credentialsHelper.getCredentialsProperties.mockReturnValue([]);
 		});
 
 		it('should allow owner to create global credential', async () => {
@@ -1476,4 +1480,96 @@ describe('CredentialsService', () => {
 			expect(savedCredential.isGlobal).toBeUndefined();
 		});
 	});
+
+	describe('createManagedCredential', () => {
+		const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
+
+		const credentialData = {
+			name: 'Managed Credential',
+			type: 'oauth2',
+			oauthProvider: 'google',
+			projectId: 'project-1',
+			data: { someData: 'value' },
+		};
+
+		beforeEach(async () => {
+			// Mock the save chain
+			roleService.addScopes.mockImplementation(
+				(c) =>
+					({
+						...c,
+						scopes: ['credential:read', 'credential:update'],
+					}) as any,
+			);
+			roleService.combineResourceScopes.mockReturnValue([
+				'credential:read',
+				'credential:update',
+			] as any);
+			sharedCredentialsRepository.findOne.mockResolvedValue({ role: 'credential:owner' } as any);
+			sharedCredentialsRepository.create.mockImplementation((data) => data as any);
+			sharedCredentialsRepository.find.mockResolvedValue([]);
+			externalHooks.run.mockResolvedValue();
+			projectService.getProjectWithScope.mockResolvedValue({
+				id: 'project-1',
+			} as any);
+			projectService.getProjectRelationsForUser.mockResolvedValue([]);
+		});
+
+		it('should throw BadRequestError when credential is missing required properties', async () => {
+			// ARRANGE
+			const payload = { ...credentialData };
+			credentialsHelper.getCredentialsProperties.mockReturnValue([
+				{
+					displayName: 'required prop',
+					name: 'requiredProp',
+					type: 'string',
+					required: true,
+					default: null,
+				},
+			]);
+
+			// ACT
+			await expect(service.createManagedCredential(payload, ownerUser)).rejects.toThrow(
+				'The field "requiredProp" is mandatory for credentials of type "oauth2"',
+			);
+		});
+
+		it('should create managed credential when all required properties are provided', async () => {
+			// ARRANGE
+			const payload = {
+				...credentialData,
+				data: {
+					requiredProp: 'some-value',
+				},
+			};
+			credentialsHelper.getCredentialsProperties.mockReturnValue([
+				{
+					displayName: 'required prop',
+					name: 'requiredProp',
+					type: 'string',
+					required: true,
+					default: null,
+				},
+			]);
+			credentialsRepository.create.mockImplementation((data) => ({ ...data }) as any);
+			// @ts-expect-error - Mocking manager for testing
+			credentialsRepository.manager = {
+				transaction: jest.fn().mockImplementation(async (callback) => {
+					const mockManager = {
+						save: jest.fn().mockImplementation(async (entity) => {
+							return { ...entity, id: 'new-managed-cred-id' };
+						}),
+					};
+					return await callback(mockManager);
+				}),
+			};
+
+			// ACT
+			const result = await service.createManagedCredential(payload, ownerUser);
+
+			// ASSERT
+			expect(result).toHaveProperty('id', 'new-managed-cred-id');
+			expect(result).toHaveProperty('name', 'Managed Credential');
+		});
+	});
 });

packages/cli/src/credentials/credentials.service.ts

@@ -39,7 +39,7 @@ import { CredentialsFinderService } from './credentials-finder.service';
 
 import { CREDENTIAL_BLANKING_VALUE } from '@/constants';
 import { CredentialTypes } from '@/credential-types';
-import { createCredentialsFromCredentialsEntity } from '@/credentials-helper';
+import { createCredentialsFromCredentialsEntity, CredentialsHelper } from '@/credentials-helper';
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
 import { NotFoundError } from '@/errors/response-errors/not-found.error';
 import { ExternalHooks } from '@/external-hooks';
@@ -76,6 +76,7 @@ export class CredentialsService {
 		private readonly roleService: RoleService,
 		private readonly userRepository: UserRepository,
 		private readonly credentialsFinderService: CredentialsFinderService,
+		private readonly credentialsHelper: CredentialsHelper,
 	) {}
 
 	private async addGlobalCredentials(
@@ -863,6 +864,23 @@ export class CredentialsService {
 		return await this.createCredential({ ...dto, isManaged: false }, user);
 	}
 
+	private checkCredentialData(type: string, data: ICredentialDataDecryptedObject) {
+		// check mandatory fields are present
+		const credentialProperties = this.credentialsHelper.getCredentialsProperties(type);
+		for (const property of credentialProperties) {
+			if (property.required) {
+				const value = data[property.name];
+				if (value === undefined || value === null || value === '') {
+					throw new BadRequestError(
+						`The field "${property.name}" is mandatory for credentials of type "${type}"`,
+					);
+				}
+			}
+		}
+
+		// TODO: add further validation if needed
+	}
+
 	/**
 	 * Create a new managed credential in user's account and return it along the scopes.
 	 * Managed credentials are managed by n8n and cannot be edited by the user.
@@ -872,6 +890,7 @@ export class CredentialsService {
 	}
 
 	private async createCredential(opts: CreateCredentialOptions, user: User) {
+		this.checkCredentialData(opts.type, opts.data as ICredentialDataDecryptedObject);
 		const encryptedCredential = this.createEncryptedData({
 			id: null,
 			name: opts.name,

packages/cli/test/integration/access-control/built-in-roles.test.ts

@@ -60,6 +60,8 @@ describe('Built-in Role Matrix Testing', () => {
 			passwordReset: jest.fn(),
 		});
 
+		await utils.initCredentialsTypes();
+
 		// Create standard users
 		owner = await createOwner();
 		member1 = await createMember();

packages/cli/test/integration/access-control/cross-project-access.test.ts

@@ -63,6 +63,8 @@ describe('Cross-Project Access Control Tests', () => {
 			passwordReset: jest.fn(),
 		});
 
+		await utils.initCredentialsTypes();
+
 		// Create standard users
 		owner = await createOwner();
 		member1 = await createMember();

packages/cli/test/integration/access-control/custom-roles-functionality.test.ts

@@ -67,6 +67,8 @@ describe('Custom Role Functionality Tests', () => {
 			passwordReset: jest.fn(),
 		});
 
+		await utils.initCredentialsTypes();
+
 		// Create standard users
 		owner = await createOwner();
 		member1 = await createMember();

packages/cli/test/integration/access-control/resource-access-matrix.test.ts

@@ -64,6 +64,8 @@ describe('Resource Access Control Matrix Tests', () => {
 		ownerAgent = testServer.authAgentFor(owner);
 		testUserAgent = testServer.authAgentFor(testUser);
 
+		await utils.initCredentialsTypes();
+
 		// Create custom roles with specific scopes
 		workflowReadOnlyRole = await createCustomRoleWithScopeSlugs(
 			['workflow:read', 'workflow:list'],

packages/cli/test/integration/ai/ai.api.test.ts

@@ -16,7 +16,7 @@ import { AiService } from '@/services/ai.service';
 
 import { createOwner } from '../shared/db/users';
 import type { SuperAgentTest } from '../shared/types';
-import { setupTestServer } from '../shared/utils';
+import { initCredentialsTypes, setupTestServer } from '../shared/utils';
 
 const createAiCreditsResponse = {
 	apiKey: randomUUID(),
@@ -37,6 +37,10 @@ let ownerPersonalProject: Project;
 
 let authOwnerAgent: SuperAgentTest;
 
+beforeAll(async () => {
+	await initCredentialsTypes();
+});
+
 beforeEach(async () => {
 	await testDb.truncate(['SharedCredentials', 'CredentialsEntity']);
 

packages/cli/test/integration/credentials/credentials.api.ee.test.ts

@@ -62,6 +62,8 @@ let projectRepository: ProjectRepository;
 
 beforeAll(async () => {
 	await Container.get(RoleCacheService).refreshCache();
+
+	await utils.initCredentialsTypes();
 });
 
 beforeEach(async () => {

packages/cli/test/integration/credentials/credentials.api.test.ts

@@ -20,6 +20,7 @@ import { randomString } from 'n8n-workflow';
 
 import { CREDENTIAL_BLANKING_VALUE } from '@/constants';
 import { CredentialsService } from '@/credentials/credentials.service';
+import { createCredentialsFromCredentialsEntity } from '@/credentials-helper';
 import { CredentialsTester } from '@/services/credentials-tester.service';
 
 import {
@@ -31,11 +32,13 @@ import {
 } from '../shared/db/credentials';
 import { createAdmin, createManyUsers, createMember, createOwner } from '../shared/db/users';
 import type { SuperAgentTest } from '../shared/types';
-import { setupTestServer } from '../shared/utils';
+import { initCredentialsTypes, setupTestServer } from '../shared/utils';
 
 const { any } = expect;
 
-const testServer = setupTestServer({ endpointGroups: ['credentials'] });
+const testServer = setupTestServer({
+	endpointGroups: ['credentials'],
+});
 
 let owner: User;
 let member: User;
@@ -52,6 +55,10 @@ let authAdminAgent: SuperAgentTest;
 let projectRepository: ProjectRepository;
 let sharedCredentialsRepository: SharedCredentialsRepository;
 
+beforeAll(async () => {
+	await initCredentialsTypes();
+});
+
 beforeEach(async () => {
 	await testDb.truncate(['SharedCredentials', 'CredentialsEntity']);
 
@@ -792,7 +799,6 @@ describe('POST /credentials', () => {
 		const payload = randomCredentialPayload();
 
 		const response = await authMemberAgent.post('/credentials').send(payload);
-
 		expect(response.statusCode).toBe(200);
 
 		const { id, name, type, data: encryptedData, scopes } = response.body.data;
@@ -1301,15 +1307,16 @@ describe('PATCH /credentials/:id', () => {
 			.query({ includeData: true })
 			.expect(200);
 
-		const { id, data } = response.body.data;
+		const { id } = response.body.data;
 
 		expect(id).toBe(savedCredential.id);
-		// was overwritten
-		expect(data.accessToken).toBe(patchPayload.data.accessToken);
 		// was not overwritten
 		const dbCredential = await getCredentialById(savedCredential.id);
-		const unencryptedData = Container.get(CredentialsService).decrypt(dbCredential!);
-		expect(unencryptedData.oauthTokenData).toEqual(credential.data.oauthTokenData);
+		const unencryptedData = createCredentialsFromCredentialsEntity(dbCredential!);
+		expect(unencryptedData.getData().oauthTokenData).toEqual(credential.data.oauthTokenData);
+
+		// was overwritten
+		expect(unencryptedData.getData().accessToken).toBe(patchPayload.data.accessToken);
 	});
 
 	test('should fail with invalid inputs', async () => {

packages/cli/test/integration/shared/utils/index.ts

@@ -12,6 +12,9 @@ import {
 } from 'n8n-core';
 import { Ftp } from 'n8n-nodes-base/credentials/Ftp.credentials';
 import { GithubApi } from 'n8n-nodes-base/credentials/GithubApi.credentials';
+import { HttpBasicAuth } from 'n8n-nodes-base/credentials/HttpBasicAuth.credentials';
+import { HttpHeaderAuth } from 'n8n-nodes-base/credentials/HttpHeaderAuth.credentials';
+import { OpenAiApi } from 'n8n-nodes-base/credentials/OpenAiApi.credentials';
 import { Cron } from 'n8n-nodes-base/nodes/Cron/Cron.node';
 import { FormTrigger } from 'n8n-nodes-base/nodes/Form/FormTrigger.node';
 import { ScheduleTrigger } from 'n8n-nodes-base/nodes/Schedule/ScheduleTrigger.node';
@@ -63,6 +66,18 @@ export async function initCredentialsTypes(): Promise<void> {
 			type: new Ftp(),
 			sourcePath: '',
 		},
+		openAiApi: {
+			type: new OpenAiApi(),
+			sourcePath: '',
+		},
+		httpHeaderAuth: {
+			type: new HttpHeaderAuth(),
+			sourcePath: '',
+		},
+		httpBasicAuth: {
+			type: new HttpBasicAuth(),
+			sourcePath: '',
+		},
 	};
 }
 

packages/frontend/editor-ui/src/features/credentials/components/CredentialEdit/CredentialEdit.vue

@@ -690,6 +690,7 @@ async function saveCredential(): Promise<ICredentialsResponse | null> {
 	if (!requiredPropertiesFilled.value) {
 		showValidationWarning.value = true;
 		scrollToTop();
+		return null;
 	} else {
 		showValidationWarning.value = false;
 	}