Skip to content

Commit

Permalink
✨ sharing/unsharing a credential (#3601)
Browse files Browse the repository at this point in the history
* 🎉 initial design

* ✨ sharing/unsharing of credentials

* ✅ add tests for EE credentials controller

* 💪 implement review comments

* 🛠 refactor agent creation and credential role locking

* 👕 linting adjustments (#3691)

* 👕 Adjust rule `naming-convention`

* 👕 Fix `naming-convention` config value

* 👕 Disregard casing for EE-prefixed vars

Co-authored-by: Iván Ovejero <ivov.src@gmail.com>
  • Loading branch information
BHesseldieck and ivov authored Jul 13, 2022
1 parent f525b9a commit b9d2301
Show file tree
Hide file tree
Showing 22 changed files with 539 additions and 80 deletions.
13 changes: 13 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,19 @@ module.exports = {
leadingUnderscore: 'allowSingleOrDouble',
trailingUnderscore: 'allowSingleOrDouble',
},
{
selector: 'variable',
filter: {
regex: '^EE',
match: true,
},
format: null,
},
{
selector: 'parameter',
format: ['camelCase'],
leadingUnderscore: 'allowSingleOrDouble',
},
{
selector: 'property',
format: ['camelCase', 'snake_case'],
Expand Down
6 changes: 6 additions & 0 deletions packages/cli/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,12 @@ export const schema = {
default: 'default',
env: 'N8N_DEPLOYMENT_TYPE',
},
paid: {
doc: 'Whether paid features are enabled.',
format: Boolean,
default: true,
env: 'N8N_PAID',
},
},

hiringBanner: {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/CredentialsHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ export async function getCredentialWithoutUser(
return credential;
}

export function createCredentiasFromCredentialsEntity(
export function createCredentialsFromCredentialsEntity(
credential: CredentialsEntity,
encrypt = false,
): Credentials {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/Server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ import { DEFAULT_EXECUTIONS_GET_ALL_LIMIT, validateEntity } from './GenericHelpe
import { ExecutionEntity } from './databases/entities/ExecutionEntity';
import { SharedWorkflow } from './databases/entities/SharedWorkflow';
import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES } from './constants';
import { credentialsController } from './api/credentials.api';
import { credentialsController } from './credentials/credentials.controller';
import {
getInstanceBaseUrl,
isEmailSetUp,
Expand Down
73 changes: 73 additions & 0 deletions packages/cli/src/credentials/credentials.controller.ee.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/* eslint-disable import/no-cycle */
import express from 'express';

import config from '../../config';
import type { CredentialRequest } from '../requests';
import { UserService } from '../user/user.service';
import { EECredentialsService as EECredentials } from './credentials.service.ee';

export const EECredentialsController = express.Router();

EECredentialsController.use((_req, _res, next) => {
if (!config.getEnv('deployment.paid')) {
// skip ee router and use free one
next('router');
return;
}
// use ee router
next();
});

/**
* (EE) POST /credentials/:id/share
*
* Grant a user access to a credential.
*/

EECredentialsController.post('/:id/share', async (req: CredentialRequest.Share, res) => {
const { id } = req.params;
const { shareeId } = req.body;

const isOwned = EECredentials.isOwned(req.user.id, id);
const getSharee = UserService.get({ id: shareeId });

// parallelize DB requests and destructure results
const [{ ownsCredential, credential }, sharee] = await Promise.all([isOwned, getSharee]);

if (!ownsCredential || !credential) {
return res.status(403).send();
}

if (!sharee || sharee.isPending) {
return res.status(400).send('Bad Request');
}

await EECredentials.share(credential, sharee);

return res.status(200).send();
});

/**
* (EE) DELETE /credentials/:id/share
*
* Revoke a user's access to a credential.
*/

EECredentialsController.delete('/:id/share', async (req: CredentialRequest.Share, res) => {
const { id } = req.params;
const { shareeId } = req.body;

const { ownsCredential } = await EECredentials.isOwned(req.user.id, id);

if (!ownsCredential) {
return res.status(403).send();
}

if (!shareeId || typeof shareeId !== 'string') {
return res.status(400).send('Bad Request');
}

await EECredentials.unshare(id, shareeId);

return res.status(200).send();
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ import { RESPONSE_ERROR_MESSAGES } from '../constants';
import { CredentialsEntity } from '../databases/entities/CredentialsEntity';
import { SharedCredentials } from '../databases/entities/SharedCredentials';
import { validateEntity } from '../GenericHelpers';
import { createCredentiasFromCredentialsEntity } from '../CredentialsHelper';
import { createCredentialsFromCredentialsEntity } from '../CredentialsHelper';
import type { CredentialRequest } from '../requests';
import * as config from '../../config';
import { externalHooks } from '../Server';
import { CredentialsService } from './credentials.service';
import { EECredentialsController } from './credentials.controller.ee';

export const credentialsController = express.Router();

Expand All @@ -42,6 +44,8 @@ credentialsController.use((req, res, next) => {
next();
});

credentialsController.use('/', EECredentialsController);

/**
* GET /credentials
*/
Expand Down Expand Up @@ -166,7 +170,7 @@ credentialsController.post(
}

// Encrypt the data
const coreCredential = createCredentiasFromCredentialsEntity(newCredential, true);
const coreCredential = createCredentialsFromCredentialsEntity(newCredential, true);

// @ts-ignore
coreCredential.setData(newCredential.data, encryptionKey);
Expand Down Expand Up @@ -298,7 +302,7 @@ credentialsController.patch(
);
}

const coreCredential = createCredentiasFromCredentialsEntity(credential);
const coreCredential = createCredentialsFromCredentialsEntity(credential);

const decryptedData = coreCredential.getData(encryptionKey);

Expand Down Expand Up @@ -361,14 +365,7 @@ credentialsController.get(
ResponseHelper.send(async (req: CredentialRequest.Get) => {
const { id: credentialId } = req.params;

const shared = await Db.collections.SharedCredentials.findOne({
relations: ['credentials'],
where: whereClause({
user: req.user,
entityType: 'credentials',
entityId: credentialId,
}),
});
const shared = await CredentialsService.getSharing(req.user.id, credentialId, ['credentials']);

if (!shared) {
throw new ResponseHelper.ResponseError(
Expand Down Expand Up @@ -402,7 +399,7 @@ credentialsController.get(
);
}

const coreCredential = createCredentiasFromCredentialsEntity(credential);
const coreCredential = CredentialsService.createCredentialsFromCredentialsEntity(credential);

return {
id: id.toString(),
Expand Down
37 changes: 37 additions & 0 deletions packages/cli/src/credentials/credentials.service.ee.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/* 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';

export class EECredentialsService extends CredentialsService {
static async isOwned(
userId: string,
credentialId: string,
): Promise<{ ownsCredential: boolean; credential?: CredentialsEntity }> {
const sharing = await this.getSharing(userId, credentialId, ['credentials']);

return sharing
? { ownsCredential: true, credential: sharing.credentials }
: { ownsCredential: false };
}

static async share(credentials: CredentialsEntity, sharee: User): Promise<SharedCredentials> {
const role = await RoleService.get({ scope: 'credential', name: 'editor' });

return Db.collections.SharedCredentials.save({
credentials,
user: sharee,
role,
});
}

static async unshare(credentialId: string, shareeId: string): Promise<void> {
return Db.collections.SharedCredentials.delete({
credentials: { id: credentialId },
user: { id: shareeId },
});
}
}
39 changes: 39 additions & 0 deletions packages/cli/src/credentials/credentials.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/* eslint-disable import/no-cycle */
import { Credentials } from 'n8n-core';
import { FindOneOptions } from 'typeorm';

import { Db } from '..';
import { CredentialsEntity } from '../databases/entities/CredentialsEntity';
import { SharedCredentials } from '../databases/entities/SharedCredentials';

export class CredentialsService {
static async getSharing(
userId: string,
credentialId: number | string,
relations?: string[],
): Promise<SharedCredentials | undefined> {
const options: FindOneOptions = {
where: {
user: { id: userId },
credentials: { id: credentialId },
},
};

if (relations) {
options.relations = relations;
}

return Db.collections.SharedCredentials.findOne(options);
}

static createCredentialsFromCredentialsEntity(
credential: CredentialsEntity,
encrypt = false,
): Credentials {
const { id, name, type, nodesAccess, data } = credential;
if (encrypt) {
return new Credentials({ id: null, name }, type, nodesAccess);
}
return new Credentials({ id: id.toString(), name }, type, nodesAccess, data);
}
}
Empty file.
Empty file.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MigrationInterface, QueryRunner } from 'typeorm';
import config = require('../../../../config');
import * as config from '../../../../config';
import { logMigrationEnd, logMigrationStart } from '../../utils/migrationHelpers';

export class LowerCaseUserEmail1648740597343 implements MigrationInterface {
Expand All @@ -8,7 +8,7 @@ export class LowerCaseUserEmail1648740597343 implements MigrationInterface {
public async up(queryRunner: QueryRunner): Promise<void> {
logMigrationStart(this.name);

const tablePrefix = config.get('database.tablePrefix');
const tablePrefix = config.getEnv('database.tablePrefix');

await queryRunner.query(`
UPDATE "${tablePrefix}user"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { MigrationInterface, QueryRunner } from 'typeorm';
import * as config from '../../../../config';
import { logMigrationEnd, logMigrationStart } from '../../utils/migrationHelpers';

export class CreateCredentialsEditorRole1657062385367 implements MigrationInterface {
name = 'CreateCredentialsEditorRole1657062385367';

public async up(queryRunner: QueryRunner): Promise<void> {
logMigrationStart(this.name);

const tablePrefix = config.getEnv('database.tablePrefix');

await queryRunner.query(`
INSERT INTO "${tablePrefix}role" (name, scope)
VALUES ("editor", "credential");
`);

logMigrationEnd(this.name);
}

public async down(queryRunner: QueryRunner): Promise<void> {
const tablePrefix = config.getEnv('database.tablePrefix');

await queryRunner.query(`
DELETE FROM "${tablePrefix}role" WHERE name='editor' AND scope='credential';
`);
}
}
2 changes: 2 additions & 0 deletions packages/cli/src/databases/migrations/sqlite/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { CreateUserManagement1646992772331 } from './1646992772331-CreateUserMan
import { LowerCaseUserEmail1648740597343 } from './1648740597343-LowerCaseUserEmail';
import { AddUserSettings1652367743993 } from './1652367743993-AddUserSettings';
import { AddAPIKeyColumn1652905585850 } from './1652905585850-AddAPIKeyColumn';
import { CreateCredentialsEditorRole1657062385367 } from './1657062385367-CreateCredentialsEditorRole';

const sqliteMigrations = [
InitialMigration1588102412422,
Expand All @@ -28,6 +29,7 @@ const sqliteMigrations = [
LowerCaseUserEmail1648740597343,
AddUserSettings1652367743993,
AddAPIKeyColumn1652905585850,
CreateCredentialsEditorRole1657062385367,
];

export { sqliteMigrations };
2 changes: 2 additions & 0 deletions packages/cli/src/requests.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ export declare namespace CredentialRequest {
type NewName = WorkflowRequest.NewName;

type Test = AuthenticatedRequest<{}, {}, INodeCredentialTestRequest>;

type Share = AuthenticatedRequest<{ id: string }, {}, { shareeId: string }>;
}

// ----------------------------------
Expand Down
9 changes: 9 additions & 0 deletions packages/cli/src/role/role.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/* eslint-disable import/no-cycle */
import { Db } from '..';
import { Role } from '../databases/entities/Role';

export class RoleService {
static async get(role: Partial<Role>): Promise<Role | undefined> {
return Db.collections.Role.findOne(role);
}
}
9 changes: 9 additions & 0 deletions packages/cli/src/user/user.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/* eslint-disable import/no-cycle */
import { Db } from '..';
import { User } from '../databases/entities/User';

export class UserService {
static async get(user: Partial<User>): Promise<User | undefined> {
return Db.collections.User.findOne(user);
}
}
Loading

0 comments on commit b9d2301

Please sign in to comment.