refactor: Add dto for POST /credentials endpoint (#13306)

packages/@n8n/api-types/src/dto/credentials/__tests__/create-credential.dto.test.ts

@@ -0,0 +1,145 @@
+import { CreateCredentialDto } from '../create-credential.dto';
+
+describe('CreateCredentialDto', () => {
+	describe('Valid requests', () => {
+		test.each([
+			{
+				name: 'with required fields',
+				request: {
+					name: 'My API Credentials',
+					type: 'apiKey',
+					data: {},
+				},
+			},
+			{
+				name: 'with optional projectId',
+				request: {
+					name: 'My API Credentials',
+					type: 'apiKey',
+					data: {
+						apiKey: '123',
+						isAdmin: true,
+					},
+					projectId: 'project123',
+				},
+			},
+			{
+				name: 'with data object',
+				request: {
+					name: 'My API Credentials',
+					type: 'oauth2',
+					data: {
+						clientId: '123',
+						clientSecret: 'secret',
+					},
+				},
+			},
+		])('should validate $name', ({ request }) => {
+			const result = CreateCredentialDto.safeParse(request);
+			expect(result.success).toBe(true);
+		});
+
+		test('should not strip out properties from the data object', () => {
+			const result = CreateCredentialDto.safeParse({
+				name: 'My API Credentials',
+				type: 'apiKey',
+				data: {
+					apiKey: '123',
+					otherProperty: 'otherValue',
+				},
+			});
+
+			expect(result.success).toBe(true);
+			expect(result.data).toEqual({
+				name: 'My API Credentials',
+				type: 'apiKey',
+				data: {
+					apiKey: '123',
+					otherProperty: 'otherValue',
+				},
+			});
+		});
+	});
+
+	describe('Invalid requests', () => {
+		test.each([
+			{
+				name: 'missing name',
+				request: {
+					type: 'apiKey',
+					data: {},
+				},
+				expectedErrorPath: ['name'],
+			},
+			{
+				name: 'empty name',
+				request: {
+					name: '',
+					type: 'apiKey',
+					data: {},
+				},
+				expectedErrorPath: ['name'],
+			},
+			{
+				name: 'name too long',
+				request: {
+					name: 'a'.repeat(129),
+					type: 'apiKey',
+					data: {},
+				},
+				expectedErrorPath: ['name'],
+			},
+			{
+				name: 'missing type',
+				request: {
+					name: 'My API Credentials',
+					data: {},
+				},
+				expectedErrorPath: ['type'],
+			},
+			{
+				name: 'empty type',
+				request: {
+					name: 'My API Credentials',
+					type: '',
+					data: {},
+				},
+				expectedErrorPath: ['type'],
+			},
+			{
+				name: 'type too long',
+				request: {
+					name: 'My API Credentials',
+					type: 'a'.repeat(33),
+					data: {},
+				},
+				expectedErrorPath: ['type'],
+			},
+			{
+				name: 'missing data',
+				request: {
+					name: 'My API Credentials',
+					type: 'apiKey',
+				},
+				expectedErrorPath: ['data'],
+			},
+			{
+				name: 'invalid data type',
+				request: {
+					name: 'My API Credentials',
+					type: 'apiKey',
+					data: 'invalid',
+				},
+				expectedErrorPath: ['data'],
+			},
+		])('should fail validation for $name', ({ request, expectedErrorPath }) => {
+			const result = CreateCredentialDto.safeParse(request);
+
+			expect(result.success).toBe(false);
+
+			if (expectedErrorPath) {
+				expect(result.error?.issues[0].path).toEqual(expectedErrorPath);
+			}
+		});
+	});
+});

packages/@n8n/api-types/src/dto/credentials/create-credential.dto.ts

@@ -0,0 +1,9 @@
+import { z } from 'zod';
+import { Z } from 'zod-class';
+
+export class CreateCredentialDto extends Z.class({
+	name: z.string().min(1).max(128),
+	type: z.string().min(1).max(32),
+	data: z.record(z.string(), z.unknown()),
+	projectId: z.string().optional(),
+}) {}

packages/@n8n/api-types/src/dto/index.ts

@@ -39,6 +39,7 @@ export { CommunityRegisteredRequestDto } from './license/community-registered-re
 export { PullWorkFolderRequestDto } from './source-control/pull-work-folder-request.dto';
 export { PushWorkFolderRequestDto } from './source-control/push-work-folder-request.dto';
 
+export { CreateCredentialDto } from './credentials/create-credential.dto';
 export { VariableListRequestDto } from './variables/variables-list-request.dto';
 export { CredentialsGetOneRequestQuery } from './credentials/credentials-get-one-request.dto';
 export { CredentialsGetManyRequestQuery } from './credentials/credentials-get-many-request.dto';

packages/cli/src/controllers/ai.controller.ts

@@ -1,3 +1,4 @@
+import type { CreateCredentialDto } from '@n8n/api-types';
 import {
 	AiChatRequestDto,
 	AiApplySuggestionRequestDto,
@@ -14,7 +15,6 @@ import { FREE_AI_CREDITS_CREDENTIAL_NAME } from '@/constants';
 import { CredentialsService } from '@/credentials/credentials.service';
 import { Body, Post, RestController } from '@/decorators';
 import { InternalServerError } from '@/errors/response-errors/internal-server.error';
-import type { CredentialRequest } from '@/requests';
 import { AuthenticatedRequest } from '@/requests';
 import { AiService } from '@/services/ai.service';
 import { UserService } from '@/services/user.service';
@@ -84,18 +84,17 @@ export class AiController {
 		try {
 			const aiCredits = await this.aiService.createFreeAiCredits(req.user);
 
-			const credentialProperties: CredentialRequest.CredentialProperties = {
+			const credentialProperties: CreateCredentialDto = {
 				name: FREE_AI_CREDITS_CREDENTIAL_NAME,
 				type: OPEN_AI_API_CREDENTIAL_TYPE,
 				data: {
 					apiKey: aiCredits.apiKey,
 					url: aiCredits.url,
 				},
-				isManaged: true,
 				projectId: payload?.projectId,
 			};
 
-			const newCredential = await this.credentialsService.createCredential(
+			const newCredential = await this.credentialsService.createManagedCredential(
 				credentialProperties,
 				req.user,
 			);

packages/cli/src/credentials-helper.ts

@@ -487,7 +487,7 @@ export class CredentialsHelper extends ICredentialsHelper {
 }
 
 export function createCredentialsFromCredentialsEntity(
-	credential: CredentialsEntity,
+	credential: ICredentialsDb,
 	encrypt = false,
 ): Credentials {
 	const { id, name, type, data } = credential;

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

@@ -28,6 +28,7 @@ describe('CredentialsController', () => {
 	);
 
 	let req: AuthenticatedRequest;
+	let res = mock<Response>();
 	beforeAll(() => {
 		req = { user: { id: '123' } } as AuthenticatedRequest;
 	});
@@ -48,7 +49,7 @@ describe('CredentialsController', () => {
 				id: newCredentialsPayload.projectId,
 			});
 
-			credentialsService.createCredential.mockResolvedValue(createdCredentials);
+			credentialsService.createUnmanagedCredential.mockResolvedValue(createdCredentials);
 
 			sharedCredentialsRepository.findCredentialOwningProject.mockResolvedValue(
 				projectOwningCredentialData,
@@ -56,11 +57,15 @@ describe('CredentialsController', () => {
 
 			// Act
 
-			const newApiKey = await credentialsController.createCredentials(req);
+			const newApiKey = await credentialsController.createCredentials(
+				req,
+				res,
+				newCredentialsPayload,
+			);
 
 			// Assert
 
-			expect(credentialsService.createCredential).toHaveBeenCalledWith(
+			expect(credentialsService.createUnmanagedCredential).toHaveBeenCalledWith(
 				newCredentialsPayload,
 				req.user,
 			);

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

@@ -1,5 +1,4 @@
 import { mock } from 'jest-mock-extended';
-import { nanoId, date } from 'minifaker';
 import { CREDENTIAL_ERRORS, CredentialDataError, Credentials, type ErrorReporter } from 'n8n-core';
 import { CREDENTIAL_EMPTY_VALUE, type ICredentialType } from 'n8n-workflow';
 
@@ -7,11 +6,7 @@ import { CREDENTIAL_BLANKING_VALUE } from '@/constants';
 import type { CredentialTypes } from '@/credential-types';
 import { CredentialsService } from '@/credentials/credentials.service';
 import type { CredentialsEntity } from '@/databases/entities/credentials-entity';
-import type { AuthenticatedRequest } from '@/requests';
-
-import { createNewCredentialsPayload, credentialScopes } from './credentials.test-data';
-
-let req = { user: { id: '123' } } as AuthenticatedRequest;
+import type { CredentialsRepository } from '@/databases/repositories/credentials.repository';
 
 describe('CredentialsService', () => {
 	const credType = mock<ICredentialType>({
@@ -34,8 +29,9 @@ describe('CredentialsService', () => {
 
 	const errorReporter = mock<ErrorReporter>();
 	const credentialTypes = mock<CredentialTypes>();
+	const credentialsRepository = mock<CredentialsRepository>();
 	const service = new CredentialsService(
-		mock(),
+		credentialsRepository,
 		mock(),
 		mock(),
 		mock(),
@@ -81,69 +77,6 @@ describe('CredentialsService', () => {
 		});
 	});
 
-	describe('createCredential', () => {
-		it('it should create new credentials and return with scopes', async () => {
-			// Arrange
-
-			const encryptedData = 'encryptedData';
-
-			const newCredentialPayloadData = createNewCredentialsPayload();
-
-			const newCredential = mock<CredentialsEntity>({
-				name: newCredentialPayloadData.name,
-				data: JSON.stringify(newCredentialPayloadData.data),
-				type: newCredentialPayloadData.type,
-			});
-
-			const encryptedDataResponse = {
-				name: newCredentialPayloadData.name,
-				type: newCredentialPayloadData.type,
-				updatedAt: date(),
-				data: encryptedData,
-			};
-
-			const saveCredentialsResponse = {
-				id: nanoId.nanoid(),
-				name: newCredentialPayloadData.name,
-				type: newCredentialPayloadData.type,
-				updatedAt: encryptedDataResponse.updatedAt,
-				createdAt: date(),
-				data: encryptedDataResponse.data,
-				isManaged: false,
-				shared: undefined,
-			};
-
-			service.prepareCreateData = jest.fn().mockReturnValue(newCredential);
-			service.createEncryptedData = jest.fn().mockImplementation(() => encryptedDataResponse);
-			service.save = jest.fn().mockResolvedValue(saveCredentialsResponse);
-			service.getCredentialScopes = jest.fn().mockReturnValue(credentialScopes);
-
-			// Act
-
-			const createdCredential = await service.createCredential(newCredentialPayloadData, req.user);
-
-			// Assert
-
-			expect(service.prepareCreateData).toHaveBeenCalledWith(newCredentialPayloadData);
-			expect(service.createEncryptedData).toHaveBeenCalledWith(null, newCredential);
-			expect(service.save).toHaveBeenCalledWith(
-				newCredential,
-				encryptedDataResponse,
-				req.user,
-				newCredentialPayloadData.projectId,
-			);
-			expect(service.getCredentialScopes).toHaveBeenCalledWith(
-				req.user,
-				saveCredentialsResponse.id,
-			);
-
-			expect(createdCredential).toEqual({
-				...saveCredentialsResponse,
-				scopes: credentialScopes,
-			});
-		});
-	});
-
 	describe('decrypt', () => {
 		const data = {
 			clientId: 'abc123',

packages/cli/src/credentials/__tests__/credentials.test-data.ts

@@ -1,9 +1,8 @@
+import type { CreateCredentialDto } from '@n8n/api-types';
 import type { Scope } from '@n8n/permissions';
 import { nanoId, date } from 'minifaker';
 import { randomString } from 'n8n-workflow';
 
-import type { CredentialRequest } from '@/requests';
-
 type NewCredentialWithSCopes = {
 	scopes: Scope[];
 	name: string;
@@ -33,9 +32,7 @@ export const credentialScopes: Scope[] = [
 	'credential:update',
 ];
 
-export const createNewCredentialsPayload = (
-	payload?: Partial<CredentialRequest.CredentialProperties>,
-): CredentialRequest.CredentialProperties => {
+export const createNewCredentialsPayload = (payload?: Partial<CreateCredentialDto>) => {
 	return {
 		name,
 		type,

packages/cli/src/credentials/credentials.controller.ts

@@ -1,4 +1,5 @@
 import {
+	CreateCredentialDto,
 	CredentialsGetManyRequestQuery,
 	CredentialsGetOneRequestQuery,
 	GenerateCredentialNameRequestQuery,
@@ -7,6 +8,7 @@ import { GlobalConfig } from '@n8n/config';
 // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
 import { In } from '@n8n/typeorm';
 import { Logger } from 'n8n-core';
+import type { ICredentialDataDecryptedObject } from 'n8n-workflow';
 import { deepCopy } from 'n8n-workflow';
 import { z } from 'zod';
 
@@ -24,14 +26,14 @@ import {
 	RestController,
 	ProjectScope,
 } from '@/decorators';
-import { Param, Query } from '@/decorators/args';
+import { Body, Param, Query } from '@/decorators/args';
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
 import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
 import { NotFoundError } from '@/errors/response-errors/not-found.error';
 import { EventService } from '@/events/event.service';
 import { License } from '@/license';
 import { listQueryMiddleware } from '@/middlewares';
-import { CredentialRequest } from '@/requests';
+import { AuthenticatedRequest, CredentialRequest } from '@/requests';
 import { NamingService } from '@/services/naming.service';
 import { UserManagementMailer } from '@/user-management/email';
 import * as utils from '@/utils';
@@ -162,8 +164,15 @@ export class CredentialsController {
 	}
 
 	@Post('/')
-	async createCredentials(req: CredentialRequest.Create) {
-		const newCredential = await this.credentialsService.createCredential(req.body, req.user);
+	async createCredentials(
+		req: AuthenticatedRequest,
+		_: Response,
+		@Body payload: CreateCredentialDto,
+	) {
+		const newCredential = await this.credentialsService.createUnmanagedCredential(
+			payload,
+			req.user,
+		);
 
 		const project = await this.sharedCredentialsRepository.findCredentialOwningProject(
 			newCredential.id,
@@ -217,10 +226,12 @@ export class CredentialsController {
 			req.body,
 			decryptedData,
 		);
-		const newCredentialData = this.credentialsService.createEncryptedData(
-			credentialId,
-			preparedCredentialData,
-		);
+		const newCredentialData = this.credentialsService.createEncryptedData({
+			id: credential.id,
+			name: preparedCredentialData.name,
+			type: preparedCredentialData.type,
+			data: preparedCredentialData.data as unknown as ICredentialDataDecryptedObject,
+		});
 
 		const responseData = await this.credentialsService.update(credentialId, newCredentialData);
 

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

@@ -1,3 +1,4 @@
+import type { CreateCredentialDto } from '@n8n/api-types';
 import { Service } from '@n8n/di';
 import type { Scope } from '@n8n/permissions';
 // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
@@ -44,6 +45,10 @@ export type CredentialsGetSharedOptions =
 	| { allowGlobalScope: true; globalScope: Scope }
 	| { allowGlobalScope: false };
 
+type CreateCredentialOptions = CreateCredentialDto & {
+	isManaged: boolean;
+};
+
 @Service()
 export class CredentialsService {
 	constructor(
@@ -280,20 +285,6 @@ export class CredentialsService {
 		});
 	}
 
-	async prepareCreateData(
-		data: CredentialRequest.CredentialProperties,
-	): Promise<CredentialsEntity> {
-		const { id, ...rest } = data;
-
-		// This saves us a merge but requires some type casting. These
-		// types are compatible for this case.
-		const newCredentials = this.credentialsRepository.create(rest as ICredentialsDb);
-
-		await validateEntity(newCredentials);
-
-		return newCredentials;
-	}
-
 	async prepareUpdateData(
 		data: CredentialRequest.CredentialProperties,
 		decryptedData: ICredentialDataDecryptedObject,
@@ -318,10 +309,18 @@ export class CredentialsService {
 		return updateData;
 	}
 
-	createEncryptedData(credentialId: string | null, data: CredentialsEntity): ICredentialsDb {
-		const credentials = new Credentials({ id: credentialId, name: data.name }, data.type);
+	createEncryptedData(credential: {
+		id: string | null;
+		name: string;
+		type: string;
+		data: ICredentialDataDecryptedObject;
+	}): ICredentialsDb {
+		const credentials = new Credentials(
+			{ id: credential.id, name: credential.name },
+			credential.type,
+		);
 
-		credentials.setData(data.data as unknown as ICredentialDataDecryptedObject);
+		credentials.setData(credential.data);
 
 		const newCredentialData = credentials.getDataToSave() as ICredentialsDb;
 
@@ -673,16 +672,36 @@ export class CredentialsService {
 	 * Create a new credential in user's account and return it along the scopes
 	 * If a projectId is send, then it also binds the credential to that specific project
 	 */
-	async createCredential(credentialsData: CredentialRequest.CredentialProperties, user: User) {
-		const newCredential = await this.prepareCreateData(credentialsData);
+	async createUnmanagedCredential(dto: CreateCredentialDto, user: User) {
+		return await this.createCredential({ ...dto, isManaged: false }, user);
+	}
 
-		const encryptedData = this.createEncryptedData(null, newCredential);
+	/**
+	 * 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.
+	 */
+	async createManagedCredential(dto: CreateCredentialDto, user: User) {
+		return await this.createCredential({ ...dto, isManaged: true }, user);
+	}
+
+	private async createCredential(opts: CreateCredentialOptions, user: User) {
+		const encryptedCredential = this.createEncryptedData({
+			id: null,
+			name: opts.name,
+			type: opts.type,
+			data: opts.data as ICredentialDataDecryptedObject,
+		});
+
+		const credentialEntity = this.credentialsRepository.create({
+			...encryptedCredential,
+			isManaged: opts.isManaged,
+		});
 
 		const { shared, ...credential } = await this.save(
-			newCredential,
-			encryptedData,
+			credentialEntity,
+			encryptedCredential,
 			user,
-			credentialsData.projectId,
+			opts.projectId,
 		);
 
 		const scopes = await this.getCredentialScopes(user, credential.id);

packages/cli/src/requests.ts

@@ -147,8 +147,6 @@ export declare namespace CredentialRequest {
 		isManaged?: boolean;
 	}>;
 
-	type Create = AuthenticatedRequest<{}, {}, CredentialProperties>;
-
 	type Get = AuthenticatedRequest<{ credentialId: string }, {}, {}, Record<string, string>>;
 
 	type GetMany = AuthenticatedRequest<{}, {}, {}, ListQuery.Params & { includeScopes?: string }> & {

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

@@ -1,6 +1,7 @@
 import { GlobalConfig } from '@n8n/config';
 import { Container } from '@n8n/di';
 import type { Scope } from '@sentry/node';
+import * as a from 'assert';
 import { mock } from 'jest-mock-extended';
 import { Credentials } from 'n8n-core';
 import type { ICredentialDataDecryptedObject } from 'n8n-workflow';
@@ -17,6 +18,7 @@ import type { ListQuery } from '@/requests';
 import { CredentialsTester } from '@/services/credentials-tester.service';
 
 import {
+	decryptCredentialData,
 	getCredentialById,
 	saveCredential,
 	shareCredentialWithProjects,
@@ -777,11 +779,12 @@ describe('POST /credentials', () => {
 			].sort(),
 		);
 
-		const credential = await Container.get(CredentialsRepository).findOneByOrFail({ id });
+		const credential = await getCredentialById(id);
+		a.ok(credential);
 
 		expect(credential.name).toBe(payload.name);
 		expect(credential.type).toBe(payload.type);
-		expect(credential.data).not.toBe(payload.data);
+		expect(await decryptCredentialData(credential)).toStrictEqual(payload.data);
 
 		const sharedCredential = await Container.get(SharedCredentialsRepository).findOneOrFail({
 			relations: { project: true, credentials: true },

packages/cli/test/integration/shared/db/credentials.ts

@@ -23,6 +23,13 @@ export async function encryptCredentialData(
 	return Object.assign(credential, coreCredential.getDataToSave());
 }
 
+export async function decryptCredentialData(credential: ICredentialsDb): Promise<unknown> {
+	const { createCredentialsFromCredentialsEntity } = await import('@/credentials-helper');
+	const coreCredential = createCredentialsFromCredentialsEntity(credential);
+
+	return coreCredential.getData();
+}
+
 const emptyAttributes = {
 	name: 'test',
 	type: 'test',

packages/editor-ui/src/api/credentials.ts

@@ -12,6 +12,7 @@ import type {
 	INodeCredentialTestResult,
 } from 'n8n-workflow';
 import axios from 'axios';
+import type { CreateCredentialDto } from '@n8n/api-types';
 
 export async function getCredentialTypes(baseUrl: string): Promise<ICredentialType[]> {
 	const { data } = await axios.get(baseUrl + 'types/credentials.json', { withCredentials: true });
@@ -48,13 +49,9 @@ export async function getAllCredentialsForWorkflow(
 
 export async function createNewCredential(
 	context: IRestApiContext,
-	data: ICredentialsDecrypted,
-	projectId?: string,
+	payload: CreateCredentialDto,
 ): Promise<ICredentialsResponse> {
-	return await makeRestApiRequest(context, 'POST', '/credentials', {
-		...data,
-		projectId,
-	} as unknown as IDataObject);
+	return await makeRestApiRequest(context, 'POST', '/credentials', payload);
 }
 
 export async function deleteCredential(context: IRestApiContext, id: string): Promise<boolean> {

packages/editor-ui/src/stores/credentials.store.ts

@@ -300,11 +300,12 @@ export const useCredentialsStore = defineStore(STORES.CREDENTIALS, () => {
 		projectId?: string,
 	): Promise<ICredentialsResponse> => {
 		const settingsStore = useSettingsStore();
-		const credential = await credentialsApi.createNewCredential(
-			rootStore.restApiContext,
-			data,
+		const credential = await credentialsApi.createNewCredential(rootStore.restApiContext, {
+			name: data.name,
+			type: data.type,
+			data: data.data ?? {},
 			projectId,
-		);
+		});
 
 		if (data?.homeProject && !credential.homeProject) {
 			credential.homeProject = data.homeProject as ProjectSharingData;