Add permissions details to credentials for User Management (#3863)

* ⚡ Open `GET /users` * ⚡ Add permissions to cred service * 🚚 Rename method * ⚡ Refactor cred controller * 🧪 Adjust test * ✏️ Improve comment * ✏️ Improve another comment * ⚡ Account for multiple sharings * 🐛 Fix access when user is editor * 📘 Expand interface * 📘 Relocate types * 📘 Exempt cred entity with service-injected fields * 📘 Adjust interface * ♻️ Add permissions only in `GET /credentials` * 🧪 Add expectations for `ownedBy` * 🧪 Add sharing details test * 🧪 Make `ownedBy` checks more granular * 📘 Adjust interface * 🚚 Rename cred getter * ♻️ Refactor cred getter * 🧪 Expand tests * ♻️ Refactor to use guard * 👕 Remove unneeded lint exception * 🔥 Remove unneeded relation * 🚚 Move relation to `GET /credentials/:id` * 📘 Consolidate typings * 🎨 Add multiline for readability * 🔥 Remove unneeded type * ✏️ Clarity comment * ✏️ Make comments consistent * 👕 Add exception to fix build * 👕 Add more lint exceptions to fix build * 🐛 Check for non-owner * 📘 Improve typings * 🧪 Temporarily skip tests * 🔥 Remove `@ts-ignore` * 👕 Move lint exceptions * ♻️ Refactor cred service and controller * ⚡ Simplify check
n8n-io · Aug 23, 2022 · 082e679 · 082e679
1 parent bcd0f91
commit 082e679
Show file tree

Hide file tree

Showing 11 changed files with 216 additions and 140 deletions.
diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts
@@ -28,16 +28,17 @@ import { Repository } from 'typeorm';
 
 import { ChildProcess } from 'child_process';
 import { Url } from 'url';
-import { Request } from 'express';
-import { WorkflowEntity } from './databases/entities/WorkflowEntity';
-import { TagEntity } from './databases/entities/TagEntity';
-import { Role } from './databases/entities/Role';
-import { User } from './databases/entities/User';
-import { SharedCredentials } from './databases/entities/SharedCredentials';
-import { SharedWorkflow } from './databases/entities/SharedWorkflow';
-import { Settings } from './databases/entities/Settings';
-import { InstalledPackages } from './databases/entities/InstalledPackages';
-import { InstalledNodes } from './databases/entities/InstalledNodes';
+
+import type { Request } from 'express';
+import type { WorkflowEntity } from './databases/entities/WorkflowEntity';
+import type { TagEntity } from './databases/entities/TagEntity';
+import type { Role } from './databases/entities/Role';
+import type { User } from './databases/entities/User';
+import type { SharedCredentials } from './databases/entities/SharedCredentials';
+import type { SharedWorkflow } from './databases/entities/SharedWorkflow';
+import type { Settings } from './databases/entities/Settings';
+import type { InstalledPackages } from './databases/entities/InstalledPackages';
+import type { InstalledNodes } from './databases/entities/InstalledNodes';
 
 export interface IActivationError {
 	time: number;
@@ -167,6 +168,7 @@ export interface ICredentialsBase {
 export interface ICredentialsDb extends ICredentialsBase, ICredentialsEncrypted {
 	id: number | string;
 	name: string;
+	shared?: SharedCredentials[];
 }
 
 export interface ICredentialsResponse extends ICredentialsDb {

diff --git a/packages/cli/src/UserManagement/controllers/AuthController.ts b/packages/cli/src/UserManagement/controllers/AuthController.ts
@@ -45,6 +45,7 @@ export class AuthController {
 			throw new Error('Unable to access database.');
 		}
 
+		// eslint-disable-next-line @typescript-eslint/prefer-optional-chain
 		if (!user || !user.password || !(await compareHash(password, user.password))) {
 			// password is empty until user signs up
 			const error = new Error('Wrong username or password. Do you have caps lock on?');
@@ -159,6 +160,7 @@ export class AuthController {
 
 		const inviter = users.find((user) => user.id === inviterId);
 
+		// eslint-disable-next-line @typescript-eslint/prefer-optional-chain
 		if (!inviter || !inviter.email || !inviter.firstName) {
 			Logger.error(
 				'Request to resolve signup token failed because inviter does not exist or is not set up',

diff --git a/packages/cli/src/UserManagement/controllers/PasswordResetController.ts b/packages/cli/src/UserManagement/controllers/PasswordResetController.ts
@@ -51,6 +51,7 @@ export class PasswordResetController {
 		// User should just be able to reset password if one is already present
 		const user = await Db.collections.User.findOne({ email, password: Not(IsNull()) });
 
+		// eslint-disable-next-line @typescript-eslint/prefer-optional-chain
 		if (!user || !user.password) {
 			Logger.debug(
 				'Request to send password reset email failed because no user was found for the provided email',

diff --git a/packages/cli/src/UserManagement/routes/index.ts b/packages/cli/src/UserManagement/routes/index.ts
@@ -79,7 +79,7 @@ export function addRoutes(this: N8nApp, ignoredEndpoints: string[], restEndpoint
 		}
 		// Not owner and user exists. We now protect restricted urls.
 		const postRestrictedUrls = [`/${this.restEndpoint}/users`, `/${this.restEndpoint}/owner`];
-		const getRestrictedUrls = [`/${this.restEndpoint}/users`];
+		const getRestrictedUrls: string[] = [];
 		const trimmedUrl = req.url.endsWith('/') ? req.url.slice(0, -1) : req.url;
 		if (
 			(req.method === 'POST' && postRestrictedUrls.includes(trimmedUrl)) ||

diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts
@@ -1,19 +1,19 @@
-/* eslint-disable @typescript-eslint/no-shadow */
+/* eslint-disable no-param-reassign */
 /* eslint-disable @typescript-eslint/no-unused-vars */
-/* eslint-disable no-restricted-syntax */
-/* eslint-disable @typescript-eslint/no-non-null-assertion */
 /* eslint-disable import/no-cycle */
 import express from 'express';
 import { INodeCredentialTestResult, LoggerProxy } from 'n8n-workflow';
-import { getLogger } from '../Logger';
-
-import { GenericHelpers, ICredentialsResponse, ResponseHelper } from '..';
 
-import type { CredentialRequest } from '../requests';
-import * as config from '../../config';
+import config from '../../config';
+import { getLogger } from '../Logger';
+import { GenericHelpers, ResponseHelper } from '..';
 import { CredentialsService } from './credentials.service';
 import { EECredentialsController } from './credentials.controller.ee';
 
+import type { ICredentialsDb, ICredentialsResponse } from '..';
+import type { CredentialRequest } from '../requests';
+import type { CredentialWithSharings } from './credentials.types';
+
 export const credentialsController = express.Router();
 
 /**
@@ -35,16 +35,42 @@ credentialsController.use('/', EECredentialsController);
  */
 credentialsController.get(
 	'/',
-	ResponseHelper.send(async (req: CredentialRequest.GetAll): Promise<ICredentialsResponse[]> => {
-		const filter = req.query.filter ? (JSON.parse(req.query.filter) as Record<string, string>) : {};
+	ResponseHelper.send(async (req: CredentialRequest.GetAll): Promise<CredentialWithSharings[]> => {
+		let allCredentials: ICredentialsDb[] | undefined;
+
+		try {
+			allCredentials = await CredentialsService.getAll(req.user, {
+				relations: ['shared', 'shared.role', 'shared.user'],
+			});
+
+			return allCredentials.map((credential: CredentialWithSharings) => {
+				credential.ownedBy = null;
+				credential.sharedWith = [];
+
+				credential.shared?.forEach((sharing) => {
+					const { id, email, firstName, lastName } = sharing.user;
+
+					if (sharing.role.name === 'owner') {
+						credential.ownedBy = { id, email, firstName, lastName };
+						return;
+					}
+
+					if (sharing.role.name !== 'owner') {
+						credential.sharedWith?.push({ id, email, firstName, lastName });
+					}
+				});
 
-		const credentials = await CredentialsService.getFiltered(req.user, filter);
+				delete credential.shared;
 
-		return credentials.map((credential) => {
-			// eslint-disable-next-line no-param-reassign
-			credential.id = credential.id.toString();
-			return credential as ICredentialsResponse;
-		});
+				// @TODO_TECH_DEBT: Stringify `id` with entity field transformer
+				credential.id = credential.id.toString();
+
+				return credential;
+			});
+		} catch (error) {
+			LoggerProxy.error('Request to list credentials failed', error as Error);
+			throw error;
+		}
 	}),
 );
 
@@ -90,12 +116,8 @@ credentialsController.post(
 	ResponseHelper.send(async (req: CredentialRequest.Create) => {
 		const newCredential = await CredentialsService.prepareCreateData(req.body);
 
-		const encryptionKey = await CredentialsService.getEncryptionKey();
-		const encryptedData = CredentialsService.createEncryptedData(
-			encryptionKey,
-			null,
-			newCredential,
-		);
+		const key = await CredentialsService.getEncryptionKey();
+		const encryptedData = CredentialsService.createEncryptedData(key, null, newCredential);
 		const { id, ...rest } = await CredentialsService.save(newCredential, encryptedData, req.user);
 
 		return { id: id.toString(), ...rest };
@@ -110,9 +132,9 @@ credentialsController.delete(
 	ResponseHelper.send(async (req: CredentialRequest.Delete) => {
 		const { id: credentialId } = req.params;
 
-		const shared = await CredentialsService.getShared(req.user, credentialId);
+		const sharing = await CredentialsService.getSharing(req.user, credentialId);
 
-		if (!shared) {
+		if (!sharing) {
 			LoggerProxy.info('Attempt to delete credential blocked due to lack of permissions', {
 				credentialId,
 				userId: req.user.id,
@@ -124,7 +146,9 @@ credentialsController.delete(
 			);
 		}
 
-		await CredentialsService.delete(shared.credentials);
+		const { credentials: credential } = sharing;
+
+		await CredentialsService.delete(credential);
 
 		return true;
 	}),
@@ -138,9 +162,9 @@ credentialsController.patch(
 	ResponseHelper.send(async (req: CredentialRequest.Update): Promise<ICredentialsResponse> => {
 		const { id: credentialId } = req.params;
 
-		const shared = await CredentialsService.getShared(req.user, credentialId);
+		const sharing = await CredentialsService.getSharing(req.user, credentialId);
 
-		if (!shared) {
+		if (!sharing) {
 			LoggerProxy.info('Attempt to update credential blocked due to lack of permissions', {
 				credentialId,
 				userId: req.user.id,
@@ -152,16 +176,16 @@ credentialsController.patch(
 			);
 		}
 
-		const { credentials: credential } = shared;
+		const { credentials: credential } = sharing;
 
-		const encryptionKey = await CredentialsService.getEncryptionKey();
-		const decryptedData = await CredentialsService.decrypt(encryptionKey, credential);
+		const key = await CredentialsService.getEncryptionKey();
+		const decryptedData = await CredentialsService.decrypt(key, credential);
 		const preparedCredentialData = await CredentialsService.prepareUpdateData(
 			req.body,
 			decryptedData,
 		);
 		const newCredentialData = CredentialsService.createEncryptedData(
-			encryptionKey,
+			key,
 			credentialId,
 			preparedCredentialData,
 		);
@@ -177,7 +201,7 @@ credentialsController.patch(
 		}
 
 		// Remove the encrypted data as it is not needed in the frontend
-		const { id, data, ...rest } = responseData;
+		const { id, data: _, ...rest } = responseData;
 
 		LoggerProxy.verbose('Credential updated', { credentialId });
 
@@ -195,37 +219,36 @@ credentialsController.get(
 	'/:id',
 	ResponseHelper.send(async (req: CredentialRequest.Get) => {
 		const { id: credentialId } = req.params;
+		const includeDecryptedData = req.query.includeData === 'true';
 
-		const shared = await CredentialsService.getShared(req.user, credentialId, ['credentials']);
+		const sharing = await CredentialsService.getSharing(req.user, credentialId, [
+			'credentials',
+			'role',
+		]);
 
-		if (!shared) {
+		if (!sharing) {
 			throw new ResponseHelper.ResponseError(
-				`Credentials with ID "${credentialId}" could not be found.`,
+				`Credential with ID "${credentialId}" could not be found.`,
 				undefined,
 				404,
 			);
 		}
 
-		const { credentials: credential } = shared;
+		const { credentials: credential } = sharing;
 
-		if (req.query.includeData !== 'true') {
-			const { data, id, ...rest } = credential;
+		if (!includeDecryptedData || sharing.role.name !== 'owner') {
+			const { id, data: _, ...rest } = credential;
 
-			return {
-				id: id.toString(),
-				...rest,
-			};
+			// @TODO_TECH_DEBT: Stringify `id` with entity field transformer
+			return { id: id.toString(), ...rest };
 		}
 
-		const { data, id, ...rest } = credential;
+		const { id, data: _, ...rest } = credential;
 
-		const encryptionKey = await CredentialsService.getEncryptionKey();
-		const decryptedData = await CredentialsService.decrypt(encryptionKey, credential);
+		const key = await CredentialsService.getEncryptionKey();
+		const decryptedData = await CredentialsService.decrypt(key, credential);
 
-		return {
-			id: id.toString(),
-			data: decryptedData,
-			...rest,
-		};
+		// @TODO_TECH_DEBT: Stringify `id` with entity field transformer
+		return { id: id.toString(), data: decryptedData, ...rest };
 	}),
 );
diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts
@@ -1,30 +1,33 @@
 /* eslint-disable import/no-cycle */
 import { Db } from '..';
 import { CredentialsService } from './credentials.service';
-import { CredentialsEntity } from '../databases/entities/CredentialsEntity';
-import { SharedCredentials } from '../databases/entities/SharedCredentials';
-import { User } from '../databases/entities/User';
 import { RoleService } from '../role/role.service';
 
+import type { CredentialsEntity } from '../databases/entities/CredentialsEntity';
+import type { SharedCredentials } from '../databases/entities/SharedCredentials';
+import type { User } from '../databases/entities/User';
+
 export class EECredentialsService extends CredentialsService {
 	static async isOwned(
 		user: User,
 		credentialId: string,
 	): Promise<{ ownsCredential: boolean; credential?: CredentialsEntity }> {
-		const sharing = await this.getShared(user, credentialId, ['credentials'], {
+		const sharing = await this.getSharing(user, credentialId, ['credentials'], {
 			allowGlobalOwner: false,
 		});
 
-		return sharing
-			? { ownsCredential: true, credential: sharing.credentials }
-			: { ownsCredential: false };
+		if (!sharing) return { ownsCredential: false };
+
+		const { credentials: credential } = sharing;
+
+		return { ownsCredential: true, credential };
 	}
 
-	static async share(credentials: CredentialsEntity, sharee: User): Promise<SharedCredentials> {
+	static async share(credential: CredentialsEntity, sharee: User): Promise<SharedCredentials> {
 		const role = await RoleService.get({ scope: 'credential', name: 'editor' });
 
 		return Db.collections.SharedCredentials.save({
-			credentials,
+			credentials: credential,
 			user: sharee,
 			role,
 		});