Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add permissions details to credentials for User Management #3863

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
22daabc
:zap: Open `GET /users`
ivov Aug 10, 2022
22e783f
:zap: Add permissions to cred service
ivov Aug 10, 2022
a49bf44
:truck: Rename method
ivov Aug 10, 2022
7aa1715
:zap: Refactor cred controller
ivov Aug 10, 2022
065d45c
:test_tube: Adjust test
ivov Aug 10, 2022
49a3df8
:pencil2: Improve comment
ivov Aug 10, 2022
2c661e5
:pencil2: Improve another comment
ivov Aug 10, 2022
d98b776
:zap: Account for multiple sharings
ivov Aug 10, 2022
4b74e28
:bug: Fix access when user is editor
ivov Aug 10, 2022
966e0a1
:blue_book: Expand interface
ivov Aug 10, 2022
60c37d9
:blue_book: Relocate types
ivov Aug 10, 2022
980fb09
:blue_book: Exempt cred entity with service-injected fields
ivov Aug 10, 2022
a3189d8
:blue_book: Adjust interface
ivov Aug 19, 2022
3e69d16
:recycle: Add permissions only in `GET /credentials`
ivov Aug 19, 2022
84898a6
:test_tube: Add expectations for `ownedBy`
ivov Aug 19, 2022
176e6f2
:test_tube: Add sharing details test
ivov Aug 19, 2022
fb78f51
:test_tube: Make `ownedBy` checks more granular
ivov Aug 19, 2022
a85a299
:blue_book: Adjust interface
ivov Aug 23, 2022
9bf5703
:truck: Rename cred getter
ivov Aug 23, 2022
8f17f31
:recycle: Refactor cred getter
ivov Aug 23, 2022
ceb25b2
:test_tube: Expand tests
ivov Aug 23, 2022
83d764d
:recycle: Refactor to use guard
ivov Aug 23, 2022
e2944df
:shirt: Remove unneeded lint exception
ivov Aug 23, 2022
4b55777
:fire: Remove unneeded relation
ivov Aug 23, 2022
ee5a295
:truck: Move relation to `GET /credentials/:id`
ivov Aug 23, 2022
d0a3e3b
:blue_book: Consolidate typings
ivov Aug 23, 2022
de04add
:art: Add multiline for readability
ivov Aug 23, 2022
9bf2696
:fire: Remove unneeded type
ivov Aug 23, 2022
75aa1eb
:pencil2: Clarity comment
ivov Aug 23, 2022
215459b
:pencil2: Make comments consistent
ivov Aug 23, 2022
001e4b0
:shirt: Add exception to fix build
ivov Aug 23, 2022
c5689c0
:shirt: Add more lint exceptions to fix build
ivov Aug 23, 2022
e2be428
:bug: Check for non-owner
ivov Aug 23, 2022
766e594
:blue_book: Improve typings
ivov Aug 23, 2022
7e9df3d
:test_tube: Temporarily skip tests
ivov Aug 23, 2022
777a148
:fire: Remove `@ts-ignore`
ivov Aug 23, 2022
b6cd595
:shirt: Move lint exceptions
ivov Aug 23, 2022
a20efa0
:recycle: Refactor cred service and controller
ivov Aug 23, 2022
e026f84
:zap: Simplify check
ivov Aug 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions packages/cli/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/UserManagement/controllers/AuthController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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?');
Expand Down Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/UserManagement/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) ||
Expand Down
123 changes: 73 additions & 50 deletions packages/cli/src/credentials/credentials.controller.ts
Original file line number Diff line number Diff line change
@@ -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();

/**
Expand All @@ -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;
}
}),
);

Expand Down Expand Up @@ -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 };
Expand All @@ -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,
Expand All @@ -124,7 +146,9 @@ credentialsController.delete(
);
}

await CredentialsService.delete(shared.credentials);
const { credentials: credential } = sharing;

await CredentialsService.delete(credential);

return true;
}),
Expand All @@ -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,
Expand All @@ -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,
);
Expand All @@ -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 });

Expand All @@ -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 };
}),
);
21 changes: 12 additions & 9 deletions packages/cli/src/credentials/credentials.service.ee.ts
Original file line number Diff line number Diff line change
@@ -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,
});
Expand Down
Loading