diff --git a/packages/cli/config/schema.ts b/packages/cli/config/schema.ts index 78b7f9d59d..f8fc82fa2d 100644 --- a/packages/cli/config/schema.ts +++ b/packages/cli/config/schema.ts @@ -882,6 +882,12 @@ export const schema = { default: false, }, }, + // This is a temporary flag (acting as feature toggle) + // Will be removed when feature goes live + workflowSharingEnabled: { + format: Boolean, + default: false, + }, }, hiringBanner: { diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 3420054755..9892d3f8e4 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -518,6 +518,7 @@ export interface IN8nUISettings { isNpmAvailable: boolean; enterprise: { sharing: boolean; + workflowSharing: boolean; }; } diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 12ec80f31e..66ca14a204 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -90,7 +90,7 @@ import { WEBHOOK_METHODS } from './WebhookHelpers'; import { getSharedWorkflowIds, whereClause } from './WorkflowHelpers'; import { nodesController } from './api/nodes.api'; -import { workflowsController } from './api/workflows.api'; +import { workflowsController } from './workflows/workflows.controller'; import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES } from './constants'; import { credentialsController } from './credentials/credentials.controller'; import { oauth2CredentialController } from './credentials/oauth2Credential.api'; @@ -108,12 +108,12 @@ import { resolveJwt } from './UserManagement/auth/jwt'; import { executionsController } from './api/executions.api'; import { nodeTypesController } from './api/nodeTypes.api'; import { tagsController } from './api/tags.api'; -import { isCredentialsSharingEnabled } from './credentials/helpers'; import { loadPublicApiVersions } from './PublicApi'; import * as telemetryScripts from './telemetry/scripts'; import { getInstanceBaseUrl, isEmailSetUp, + isSharingEnabled, isUserManagementEnabled, } from './UserManagement/UserManagementHelper'; import { @@ -330,6 +330,7 @@ class App { isNpmAvailable: false, enterprise: { sharing: false, + workflowSharing: false, }, }; } @@ -357,7 +358,8 @@ class App { // refresh enterprise status Object.assign(this.frontendSettings.enterprise, { - sharing: isCredentialsSharingEnabled(), + sharing: isSharingEnabled(), + workflowSharing: config.getEnv('enterprise.workflowSharingEnabled'), }); if (config.get('nodes.packagesMissing').length > 0) { diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index f8099ff699..dbefaa27f7 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -39,6 +39,10 @@ export function isUserManagementEnabled(): boolean { ); } +export function isSharingEnabled(): boolean { + return isUserManagementEnabled() && config.getEnv('enterprise.features.sharing'); +} + export function isUserManagementDisabled(): boolean { return ( config.getEnv('userManagement.disabled') && @@ -276,3 +280,24 @@ export async function compareHash(plaintext: string, hashed: string): Promise( + [arr1, keyExtractor1]: [T1[], (item: T1) => string], + [arr2, keyExtractor2]: [T2[], (item: T2) => string], +): T2[] { + // create map { itemKey => true } for fast lookup for diff + const keyMap = arr1.reduce<{ [key: string]: true }>((map, item) => { + // eslint-disable-next-line no-param-reassign + map[keyExtractor1(item)] = true; + return map; + }, {}); + + // diff against map + return arr2.reduce((acc, item) => { + if (!keyMap[keyExtractor2(item)]) { + acc.push(item); + } + return acc; + }, []); +} diff --git a/packages/cli/src/credentials/credentials.controller.ee.ts b/packages/cli/src/credentials/credentials.controller.ee.ts index 54e7d28491..9e3966cfda 100644 --- a/packages/cli/src/credentials/credentials.controller.ee.ts +++ b/packages/cli/src/credentials/credentials.controller.ee.ts @@ -5,15 +5,15 @@ import { Db, InternalHooksManager, ResponseHelper } from '..'; import type { CredentialsEntity } from '../databases/entities/CredentialsEntity'; import type { CredentialRequest } from '../requests'; +import { isSharingEnabled, rightDiff } from '../UserManagement/UserManagementHelper'; import { EECredentialsService as EECredentials } from './credentials.service.ee'; import type { CredentialWithSharings } from './credentials.types'; -import { isCredentialsSharingEnabled, rightDiff } from './helpers'; // eslint-disable-next-line @typescript-eslint/naming-convention export const EECredentialsController = express.Router(); EECredentialsController.use((req, res, next) => { - if (!isCredentialsSharingEnabled()) { + if (!isSharingEnabled()) { // skip ee router and use free one next('router'); return; diff --git a/packages/cli/src/credentials/helpers.ee.ts b/packages/cli/src/credentials/helpers.ee.ts deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/packages/cli/src/credentials/helpers.ts b/packages/cli/src/credentials/helpers.ts deleted file mode 100644 index ec04d53d13..0000000000 --- a/packages/cli/src/credentials/helpers.ts +++ /dev/null @@ -1,28 +0,0 @@ -/* eslint-disable import/no-cycle */ -import config from '../../config'; -import { isUserManagementEnabled } from '../UserManagement/UserManagementHelper'; - -export function isCredentialsSharingEnabled(): boolean { - return isUserManagementEnabled() && config.getEnv('enterprise.features.sharing'); -} - -// return the difference between two arrays -export function rightDiff( - [arr1, keyExtractor1]: [T1[], (item: T1) => string], - [arr2, keyExtractor2]: [T2[], (item: T2) => string], -): T2[] { - // create map { itemKey => true } for fast lookup for diff - const keyMap = arr1.reduce<{ [key: string]: true }>((map, item) => { - // eslint-disable-next-line no-param-reassign - map[keyExtractor1(item)] = true; - return map; - }, {}); - - // diff against map - return arr2.reduce((acc, item) => { - if (!keyMap[keyExtractor2(item)]) { - acc.push(item); - } - return acc; - }, []); -} diff --git a/packages/cli/src/databases/entities/Role.ts b/packages/cli/src/databases/entities/Role.ts index f47d65090b..9107737e41 100644 --- a/packages/cli/src/databases/entities/Role.ts +++ b/packages/cli/src/databases/entities/Role.ts @@ -17,7 +17,7 @@ import { User } from './User'; import { SharedWorkflow } from './SharedWorkflow'; import { SharedCredentials } from './SharedCredentials'; -type RoleNames = 'owner' | 'member' | 'user'; +type RoleNames = 'owner' | 'member' | 'user' | 'editor'; type RoleScopes = 'global' | 'workflow' | 'credential'; // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types diff --git a/packages/cli/src/databases/migrations/mysqldb/1663755770894-CreateWorkflowsEditorRole.ts b/packages/cli/src/databases/migrations/mysqldb/1663755770894-CreateWorkflowsEditorRole.ts new file mode 100644 index 0000000000..61ace16951 --- /dev/null +++ b/packages/cli/src/databases/migrations/mysqldb/1663755770894-CreateWorkflowsEditorRole.ts @@ -0,0 +1,26 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; +import { getTablePrefix, logMigrationEnd, logMigrationStart } from '../../utils/migrationHelpers'; + +export class CreateWorkflowsEditorRole1663755770894 implements MigrationInterface { + name = 'CreateWorkflowsEditorRole1663755770894'; + + async up(queryRunner: QueryRunner) { + logMigrationStart(this.name); + const tablePrefix = getTablePrefix(); + + await queryRunner.query(` + INSERT IGNORE INTO ${tablePrefix}role (name, scope) + VALUES ("editor", "workflow") + `); + + logMigrationEnd(this.name); + } + + async down(queryRunner: QueryRunner) { + const tablePrefix = getTablePrefix(); + + await queryRunner.query(` + DELETE FROM ${tablePrefix}role WHERE name='user' AND scope='workflow'; + `); + } +} diff --git a/packages/cli/src/databases/migrations/mysqldb/index.ts b/packages/cli/src/databases/migrations/mysqldb/index.ts index 691009b2e1..bd5875ee42 100644 --- a/packages/cli/src/databases/migrations/mysqldb/index.ts +++ b/packages/cli/src/databases/migrations/mysqldb/index.ts @@ -20,6 +20,7 @@ import { IntroducePinData1654090101303 } from './1654090101303-IntroducePinData' import { AddNodeIds1658932910559 } from './1658932910559-AddNodeIds'; import { AddJsonKeyPinData1659895550980 } from './1659895550980-AddJsonKeyPinData'; import { CreateCredentialsUserRole1660062385367 } from './1660062385367-CreateCredentialsUserRole'; +import { CreateWorkflowsEditorRole1663755770894 } from './1663755770894-CreateWorkflowsEditorRole'; export const mysqlMigrations = [ InitialMigration1588157391238, @@ -44,4 +45,5 @@ export const mysqlMigrations = [ AddNodeIds1658932910559, AddJsonKeyPinData1659895550980, CreateCredentialsUserRole1660062385367, + CreateWorkflowsEditorRole1663755770894, ]; diff --git a/packages/cli/src/databases/migrations/postgresdb/1663755770893-CreateWorkflowsEditorRole.ts b/packages/cli/src/databases/migrations/postgresdb/1663755770893-CreateWorkflowsEditorRole.ts new file mode 100644 index 0000000000..a54192fa59 --- /dev/null +++ b/packages/cli/src/databases/migrations/postgresdb/1663755770893-CreateWorkflowsEditorRole.ts @@ -0,0 +1,27 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; +import { getTablePrefix, logMigrationEnd, logMigrationStart } from '../../utils/migrationHelpers'; + +export class CreateWorkflowsEditorRole1663755770893 implements MigrationInterface { + name = 'CreateWorkflowsEditorRole1663755770893'; + + async up(queryRunner: QueryRunner) { + logMigrationStart(this.name); + const tablePrefix = getTablePrefix(); + + await queryRunner.query(` + INSERT INTO ${tablePrefix}role (name, scope) + VALUES ('editor', 'workflow') + ON CONFLICT DO NOTHING; + `); + + logMigrationEnd(this.name); + } + + async down(queryRunner: QueryRunner) { + const tablePrefix = getTablePrefix(); + + await queryRunner.query(` + DELETE FROM ${tablePrefix}role WHERE name='user' AND scope='workflow'; + `); + } +} diff --git a/packages/cli/src/databases/migrations/postgresdb/index.ts b/packages/cli/src/databases/migrations/postgresdb/index.ts index 491393ea9c..d71905464e 100644 --- a/packages/cli/src/databases/migrations/postgresdb/index.ts +++ b/packages/cli/src/databases/migrations/postgresdb/index.ts @@ -18,6 +18,7 @@ import { IntroducePinData1654090467022 } from './1654090467022-IntroducePinData' import { AddNodeIds1658932090381 } from './1658932090381-AddNodeIds'; import { AddJsonKeyPinData1659902242948 } from './1659902242948-AddJsonKeyPinData'; import { CreateCredentialsUserRole1660062385367 } from './1660062385367-CreateCredentialsUserRole'; +import { CreateWorkflowsEditorRole1663755770893 } from './1663755770893-CreateWorkflowsEditorRole'; export const postgresMigrations = [ InitialMigration1587669153312, @@ -40,4 +41,5 @@ export const postgresMigrations = [ CreateCredentialsUserRole1660062385367, AddNodeIds1658932090381, AddJsonKeyPinData1659902242948, + CreateWorkflowsEditorRole1663755770893, ]; diff --git a/packages/cli/src/databases/migrations/sqlite/1663755770892-CreateWorkflowsUserRole.ts b/packages/cli/src/databases/migrations/sqlite/1663755770892-CreateWorkflowsUserRole.ts new file mode 100644 index 0000000000..75f5a4837f --- /dev/null +++ b/packages/cli/src/databases/migrations/sqlite/1663755770892-CreateWorkflowsUserRole.ts @@ -0,0 +1,28 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; +import { getTablePrefix, logMigrationEnd, logMigrationStart } from '../../utils/migrationHelpers'; + +export class CreateWorkflowsEditorRole1663755770892 implements MigrationInterface { + name = 'CreateWorkflowsEditorRole1663755770892'; + + async up(queryRunner: QueryRunner) { + logMigrationStart(this.name); + + const tablePrefix = getTablePrefix(); + + await queryRunner.query(` + INSERT INTO "${tablePrefix}role" (name, scope) + VALUES ("editor", "workflow") + ON CONFLICT DO NOTHING; + `); + + logMigrationEnd(this.name); + } + + async down(queryRunner: QueryRunner) { + const tablePrefix = getTablePrefix(); + + await queryRunner.query(` + DELETE FROM "${tablePrefix}role" WHERE name='user' AND scope='workflow'; + `); + } +} diff --git a/packages/cli/src/databases/migrations/sqlite/index.ts b/packages/cli/src/databases/migrations/sqlite/index.ts index 4e14548d19..4d0b177832 100644 --- a/packages/cli/src/databases/migrations/sqlite/index.ts +++ b/packages/cli/src/databases/migrations/sqlite/index.ts @@ -17,6 +17,7 @@ import { IntroducePinData1654089251344 } from './1654089251344-IntroducePinData' import { AddNodeIds1658930531669 } from './1658930531669-AddNodeIds'; import { AddJsonKeyPinData1659888469333 } from './1659888469333-AddJsonKeyPinData'; import { CreateCredentialsUserRole1660062385367 } from './1660062385367-CreateCredentialsUserRole'; +import { CreateWorkflowsEditorRole1663755770892 } from './1663755770892-CreateWorkflowsUserRole'; const sqliteMigrations = [ InitialMigration1588102412422, @@ -38,6 +39,7 @@ const sqliteMigrations = [ AddNodeIds1658930531669, AddJsonKeyPinData1659888469333, CreateCredentialsUserRole1660062385367, + CreateWorkflowsEditorRole1663755770892, ]; export { sqliteMigrations }; diff --git a/packages/cli/src/requests.d.ts b/packages/cli/src/requests.d.ts index 03cd9ec6f8..53ad893ec6 100644 --- a/packages/cli/src/requests.d.ts +++ b/packages/cli/src/requests.d.ts @@ -77,6 +77,8 @@ export declare namespace WorkflowRequest { destinationNode?: string; } >; + + type Share = AuthenticatedRequest<{ workflowId: string }, {}, { shareWithIds: string[] }>; } // ---------------------------------- diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts new file mode 100644 index 0000000000..6ed3e6a91d --- /dev/null +++ b/packages/cli/src/workflows/workflows.controller.ee.ts @@ -0,0 +1,60 @@ +import express from 'express'; +import { Db } from '..'; +import config from '../../config'; +import type { WorkflowRequest } from '../requests'; +import { isSharingEnabled, rightDiff } from '../UserManagement/UserManagementHelper'; +import { EEWorkflowsService as EEWorkflows } from './workflows.services.ee'; + +// eslint-disable-next-line @typescript-eslint/naming-convention +export const EEWorkflowController = express.Router(); + +EEWorkflowController.use((req, res, next) => { + if (!isSharingEnabled() || !config.getEnv('enterprise.workflowSharingEnabled')) { + // skip ee router and use free one + next('router'); + return; + } + // use ee router + next(); +}); + +/** + * (EE) PUT /workflows/:id/share + * + * Grant or remove users' access to a workflow. + */ + +EEWorkflowController.put('/:workflowId/share', async (req: WorkflowRequest.Share, res) => { + const { workflowId } = req.params; + const { shareWithIds } = req.body; + + if (!Array.isArray(shareWithIds) || !shareWithIds.every((userId) => typeof userId === 'string')) { + return res.status(400).send('Bad Request'); + } + + const { ownsWorkflow, workflow } = await EEWorkflows.isOwned(req.user, workflowId); + + if (!ownsWorkflow || !workflow) { + return res.status(403).send(); + } + + let newShareeIds: string[] = []; + await Db.transaction(async (trx) => { + // remove all sharings that are not supposed to exist anymore + await EEWorkflows.pruneSharings(trx, workflowId, [req.user.id, ...shareWithIds]); + + const sharings = await EEWorkflows.getSharings(trx, workflowId); + + // extract the new sharings that need to be added + newShareeIds = rightDiff( + [sharings, (sharing) => sharing.userId], + [shareWithIds, (shareeId) => shareeId], + ); + + if (newShareeIds.length) { + await EEWorkflows.share(trx, workflow, newShareeIds); + } + }); + + return res.status(200).send(); +}); diff --git a/packages/cli/src/api/workflows.api.ts b/packages/cli/src/workflows/workflows.controller.ts similarity index 97% rename from packages/cli/src/api/workflows.api.ts rename to packages/cli/src/workflows/workflows.controller.ts index c6d92965cf..737662b07c 100644 --- a/packages/cli/src/api/workflows.api.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -29,12 +29,28 @@ import { WorkflowEntity } from '../databases/entities/WorkflowEntity'; import { validateEntity } from '../GenericHelpers'; import { InternalHooksManager } from '../InternalHooksManager'; import { externalHooks } from '../Server'; +import { getLogger } from '../Logger'; import type { WorkflowRequest } from '../requests'; import { isBelowOnboardingThreshold } from '../WorkflowHelpers'; +import { EEWorkflowController } from './workflows.controller.ee'; const activeWorkflowRunner = ActiveWorkflowRunner.getInstance(); export const workflowsController = express.Router(); +/** + * Initialize Logger if needed + */ +workflowsController.use((req, res, next) => { + try { + LoggerProxy.getInstance(); + } catch (error) { + LoggerProxy.init(getLogger()); + } + next(); +}); + +workflowsController.use('/', EEWorkflowController); + const isTrigger = (nodeType: string) => ['trigger', 'webhook'].some((suffix) => nodeType.toLowerCase().includes(suffix)); diff --git a/packages/cli/src/workflows/workflows.services.ee.ts b/packages/cli/src/workflows/workflows.services.ee.ts new file mode 100644 index 0000000000..f55de55e52 --- /dev/null +++ b/packages/cli/src/workflows/workflows.services.ee.ts @@ -0,0 +1,73 @@ +import { DeleteResult, EntityManager, In, Not } from 'typeorm'; +import { Db } from '..'; +import { SharedWorkflow } from '../databases/entities/SharedWorkflow'; +import { User } from '../databases/entities/User'; +import { WorkflowEntity } from '../databases/entities/WorkflowEntity'; +import { RoleService } from '../role/role.service'; +import { UserService } from '../user/user.service'; +import { WorkflowsService } from './workflows.services'; + +export class EEWorkflowsService extends WorkflowsService { + static async isOwned( + user: User, + workflowId: string, + ): Promise<{ ownsWorkflow: boolean; workflow?: WorkflowEntity }> { + const sharing = await this.getSharing(user, workflowId, ['workflow', 'role'], { + allowGlobalOwner: false, + }); + + if (!sharing || sharing.role.name !== 'owner') return { ownsWorkflow: false }; + + const { workflow } = sharing; + + return { ownsWorkflow: true, workflow }; + } + + static async getSharings( + transaction: EntityManager, + workflowId: string, + ): Promise { + const workflow = await transaction.findOne(WorkflowEntity, workflowId, { + relations: ['shared'], + }); + return workflow?.shared ?? []; + } + + static async pruneSharings( + transaction: EntityManager, + workflowId: string, + userIds: string[], + ): Promise { + return transaction.delete(SharedWorkflow, { + workflow: { id: workflowId }, + user: { id: Not(In(userIds)) }, + }); + } + + static async share( + transaction: EntityManager, + workflow: WorkflowEntity, + shareWithIds: string[], + ): Promise { + const [users, role] = await Promise.all([ + UserService.getByIds(transaction, shareWithIds), + RoleService.trxGet(transaction, { scope: 'workflow', name: 'editor' }), + ]); + + const newSharedWorkflows = users.reduce((acc, user) => { + if (user.isPending) { + return acc; + } + acc.push( + Db.collections.SharedWorkflow.create({ + workflow, + user, + role, + }), + ); + return acc; + }, []); + + return transaction.save(newSharedWorkflows); + } +} diff --git a/packages/cli/src/workflows/workflows.services.ts b/packages/cli/src/workflows/workflows.services.ts new file mode 100644 index 0000000000..daf5fdc6cb --- /dev/null +++ b/packages/cli/src/workflows/workflows.services.ts @@ -0,0 +1,32 @@ +import { FindOneOptions, ObjectLiteral } from 'typeorm'; +import { Db } from '..'; +import { SharedWorkflow } from '../databases/entities/SharedWorkflow'; +import { User } from '../databases/entities/User'; + +export class WorkflowsService { + static async getSharing( + user: User, + workflowId: number | string, + relations: string[] = ['workflow'], + { allowGlobalOwner } = { allowGlobalOwner: true }, + ): Promise { + const options: FindOneOptions & { where: ObjectLiteral } = { + where: { + workflow: { id: workflowId }, + }, + }; + + // Omit user from where if the requesting user is the global + // owner. This allows the global owner to view and delete + // workflows they don't own. + if (!allowGlobalOwner || user.globalRole.name !== 'owner') { + options.where.user = { id: user.id }; + } + + if (relations?.length) { + options.relations = relations; + } + + return Db.collections.SharedWorkflow.findOne(options); + } +} diff --git a/packages/cli/test/integration/credentials.ee.test.ts b/packages/cli/test/integration/credentials.ee.test.ts index 44e8567014..83f8782adb 100644 --- a/packages/cli/test/integration/credentials.ee.test.ts +++ b/packages/cli/test/integration/credentials.ee.test.ts @@ -5,7 +5,7 @@ import { In } from 'typeorm'; import { Db, IUser } from '../../src'; import { RESPONSE_ERROR_MESSAGES } from '../../src/constants'; import type { CredentialWithSharings } from '../../src/credentials/credentials.types'; -import * as CredentialHelpers from '../../src/credentials/helpers'; +import * as UserManagementHelpers from '../../src/UserManagement/UserManagementHelper'; import type { Role } from '../../src/databases/entities/Role'; import { randomCredentialPayload } from './shared/random'; import * as testDb from './shared/testDb'; @@ -15,10 +15,7 @@ import * as utils from './shared/utils'; jest.mock('../../src/telemetry'); // mock whether credentialsSharing is enabled or not -const mockIsCredentialsSharingEnabled = jest.spyOn( - CredentialHelpers, - 'isCredentialsSharingEnabled', -); +const mockIsCredentialsSharingEnabled = jest.spyOn(UserManagementHelpers, 'isSharingEnabled'); mockIsCredentialsSharingEnabled.mockReturnValue(true); let app: express.Application; @@ -121,9 +118,9 @@ test('GET /credentials should return all creds for owner', async () => { const response = await authAgent(owner).get('/credentials'); expect(response.statusCode).toBe(200); - expect(response.body.data.length).toBe(2); // owner retrieved owner cred and member cred + expect(response.body.data).toHaveLength(2); // owner retrieved owner cred and member cred - const [ownerCredential, memberCredential] = response.body.data; + const [ownerCredential, memberCredential] = response.body.data as CredentialWithSharings[]; validateMainCredentialData(ownerCredential); expect(ownerCredential.data).toBeUndefined(); @@ -139,14 +136,20 @@ test('GET /credentials should return all creds for owner', async () => { }); expect(Array.isArray(ownerCredential.sharedWith)).toBe(true); - expect(ownerCredential.sharedWith.length).toBe(3); + expect(ownerCredential.sharedWith).toHaveLength(3); - ownerCredential.sharedWith.forEach((sharee: IUser, idx: number) => { + // Fix order issue (MySQL might return items in any order) + const ownerCredentialsSharedWithOrdered = [...ownerCredential.sharedWith!].sort( + (a: IUser, b: IUser) => (a.email < b.email ? -1 : 1), + ); + const orderedSharedWith = [...sharedWith].sort((a, b) => (a.email < b.email ? -1 : 1)); + + ownerCredentialsSharedWithOrdered.forEach((sharee: IUser, idx: number) => { expect(sharee).toMatchObject({ - id: sharedWith[idx].id, - email: sharedWith[idx].email, - firstName: sharedWith[idx].firstName, - lastName: sharedWith[idx].lastName, + id: orderedSharedWith[idx].id, + email: orderedSharedWith[idx].email, + firstName: orderedSharedWith[idx].firstName, + lastName: orderedSharedWith[idx].lastName, }); }); @@ -158,7 +161,7 @@ test('GET /credentials should return all creds for owner', async () => { }); expect(Array.isArray(memberCredential.sharedWith)).toBe(true); - expect(memberCredential.sharedWith.length).toBe(0); + expect(memberCredential.sharedWith).toHaveLength(0); }); test('GET /credentials should return only relevant creds for member', async () => { @@ -174,7 +177,7 @@ test('GET /credentials should return only relevant creds for member', async () = const response = await authAgent(member1).get('/credentials'); expect(response.statusCode).toBe(200); - expect(response.body.data.length).toBe(1); // member retrieved only member cred + expect(response.body.data).toHaveLength(1); // member retrieved only member cred const [member1Credential] = response.body.data; @@ -189,7 +192,7 @@ test('GET /credentials should return only relevant creds for member', async () = }); expect(Array.isArray(member1Credential.sharedWith)).toBe(true); - expect(member1Credential.sharedWith.length).toBe(1); + expect(member1Credential.sharedWith).toHaveLength(1); const [sharee] = member1Credential.sharedWith; @@ -223,7 +226,7 @@ test('GET /credentials/:id should retrieve owned cred for owner', async () => { firstName: ownerShell.firstName, lastName: ownerShell.lastName, }); - expect(firstCredential.sharedWith.length).toBe(0); + expect(firstCredential.sharedWith).toHaveLength(0); const secondResponse = await authOwnerAgent .get(`/credentials/${savedCredential.id}`) @@ -258,7 +261,7 @@ test('GET /credentials/:id should retrieve non-owned cred for owner', async () = firstName: member1.firstName, lastName: member1.lastName, }); - expect(response1.body.data.sharedWith.length).toBe(1); + expect(response1.body.data.sharedWith).toHaveLength(1); expect(response1.body.data.sharedWith[0]).toMatchObject({ id: member2.id, email: member2.email, @@ -274,7 +277,7 @@ test('GET /credentials/:id should retrieve non-owned cred for owner', async () = validateMainCredentialData(response2.body.data); expect(response2.body.data.data).toBeUndefined(); - expect(response2.body.data.sharedWith.length).toBe(1); + expect(response2.body.data.sharedWith).toHaveLength(1); }); test('GET /credentials/:id should retrieve owned cred for member', async () => { @@ -298,7 +301,7 @@ test('GET /credentials/:id should retrieve owned cred for member', async () => { firstName: member1.firstName, lastName: member1.lastName, }); - expect(firstCredential.sharedWith.length).toBe(2); + expect(firstCredential.sharedWith).toHaveLength(2); firstCredential.sharedWith.forEach((sharee: IUser, idx: number) => { expect([member2.id, member3.id]).toContain(sharee.id); }); @@ -312,7 +315,7 @@ test('GET /credentials/:id should retrieve owned cred for member', async () => { const { data: secondCredential } = secondResponse.body; validateMainCredentialData(secondCredential); expect(secondCredential.data).toBeDefined(); - expect(firstCredential.sharedWith.length).toBe(2); + expect(firstCredential.sharedWith).toHaveLength(2); }); test('GET /credentials/:id should not retrieve non-owned cred for member', async () => { @@ -478,7 +481,7 @@ test('PUT /credentials/:id/share should ignore pending sharee', async () => { where: { credentials: savedCredential }, }); - expect(sharedCredentials.length).toBe(1); + expect(sharedCredentials).toHaveLength(1); expect(sharedCredentials[0].userId).toBe(owner.id); }); @@ -496,7 +499,7 @@ test('PUT /credentials/:id/share should ignore non-existing sharee', async () => where: { credentials: savedCredential }, }); - expect(sharedCredentials.length).toBe(1); + expect(sharedCredentials).toHaveLength(1); expect(sharedCredentials[0].userId).toBe(owner.id); }); @@ -538,7 +541,7 @@ test('PUT /credentials/:id/share should unshare the credential', async () => { where: { credentials: savedCredential }, }); - expect(sharedCredentials.length).toBe(1); + expect(sharedCredentials).toHaveLength(1); expect(sharedCredentials[0].userId).toBe(owner.id); }); diff --git a/packages/cli/test/integration/credentials.test.ts b/packages/cli/test/integration/credentials.test.ts index aa1db81da2..6bb433a1e7 100644 --- a/packages/cli/test/integration/credentials.test.ts +++ b/packages/cli/test/integration/credentials.test.ts @@ -3,7 +3,7 @@ import { UserSettings } from 'n8n-core'; import { Db } from '../../src'; import { RESPONSE_ERROR_MESSAGES } from '../../src/constants'; -import * as CredentialHelpers from '../../src/credentials/helpers'; +import * as UserManagementHelpers from '../../src/UserManagement/UserManagementHelper'; import type { Role } from '../../src/databases/entities/Role'; import { randomCredentialPayload, randomName, randomString } from './shared/random'; import * as testDb from './shared/testDb'; @@ -17,10 +17,7 @@ import type { AuthAgent } from './shared/types'; jest.mock('../../src/telemetry'); // mock that credentialsSharing is not enabled -const mockIsCredentialsSharingEnabled = jest.spyOn( - CredentialHelpers, - 'isCredentialsSharingEnabled', -); +const mockIsCredentialsSharingEnabled = jest.spyOn(UserManagementHelpers, 'isSharingEnabled'); mockIsCredentialsSharingEnabled.mockReturnValue(false); let app: express.Application; diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index 7f902da19c..787687783c 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -96,7 +96,12 @@ export async function init() { try { const schema = config.getEnv('database.postgresdb.schema'); - await exec(`psql -d ${testDbName} -c "CREATE SCHEMA IF NOT EXISTS ${schema}";`); + const exportPasswordCli = pgOptions.password + ? `export PGPASSWORD=${pgOptions.password} && ` + : ''; + await exec( + `${exportPasswordCli} psql -h ${pgOptions.host} -U ${pgOptions.username} -d ${testDbName} -c "CREATE SCHEMA IF NOT EXISTS ${schema}";`, + ); } catch (error) { if (error instanceof Error && error.message.includes('command not found')) { console.error( @@ -647,6 +652,18 @@ export async function createWorkflowWithTrigger( return workflow; } +// ---------------------------------- +// workflow sharing +// ---------------------------------- + +export async function getWorkflowSharing(workflow: WorkflowEntity) { + return Db.collections.SharedWorkflow.find({ + where: { + workflow, + }, + }); +} + // ---------------------------------- // connection options // ---------------------------------- diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index 2cbfd948dd..ba51197ef6 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -41,7 +41,7 @@ import { authenticationMethods as authEndpoints } from '../../../src/UserManagem import { ownerNamespace as ownerEndpoints } from '../../../src/UserManagement/routes/owner'; import { passwordResetNamespace as passwordResetEndpoints } from '../../../src/UserManagement/routes/passwordReset'; import { nodesController } from '../../../src/api/nodes.api'; -import { workflowsController } from '../../../src/api/workflows.api'; +import { workflowsController } from '../../../src/workflows/workflows.controller'; import { AUTH_COOKIE_NAME, NODE_PACKAGE_PREFIX } from '../../../src/constants'; import { credentialsController } from '../../../src/credentials/credentials.controller'; import { InstalledPackages } from '../../../src/databases/entities/InstalledPackages'; diff --git a/packages/cli/test/integration/workflows.controller.ee.test.ts b/packages/cli/test/integration/workflows.controller.ee.test.ts new file mode 100644 index 0000000000..4794517c55 --- /dev/null +++ b/packages/cli/test/integration/workflows.controller.ee.test.ts @@ -0,0 +1,121 @@ +import express from 'express'; + +import * as utils from './shared/utils'; +import * as testDb from './shared/testDb'; +import { createWorkflow } from './shared/testDb'; +import * as UserManagementHelpers from '../../src/UserManagement/UserManagementHelper'; +import { v4 as uuid } from 'uuid'; + +import type { Role } from '../../src/databases/entities/Role'; +import config from '../../config'; +import type { AuthAgent } from './shared/types'; + +jest.mock('../../src/telemetry'); + +// mock whether sharing is enabled or not +jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(true); + +let app: express.Application; +let testDbName = ''; + +let globalOwnerRole: Role; +let globalMemberRole: Role; +let authAgent: AuthAgent; + +beforeAll(async () => { + app = await utils.initTestServer({ + endpointGroups: ['workflows'], + applyAuth: true, + }); + const initResult = await testDb.init(); + testDbName = initResult.testDbName; + + globalOwnerRole = await testDb.getGlobalOwnerRole(); + globalMemberRole = await testDb.getGlobalMemberRole(); + + authAgent = utils.createAuthAgent(app); + + utils.initTestLogger(); + utils.initTestTelemetry(); + + config.set('enterprise.workflowSharingEnabled', true); +}); + +beforeEach(async () => { + await testDb.truncate(['User', 'Workflow', 'SharedWorkflow'], testDbName); +}); + +afterAll(async () => { + await testDb.terminate(testDbName); +}); + +test('PUT /workflows/:id/share should save sharing with new users', async () => { + const owner = await testDb.createUser({ globalRole: globalOwnerRole }); + const member = await testDb.createUser({ globalRole: globalMemberRole }); + const workflow = await createWorkflow({}, owner); + + const response = await authAgent(owner) + .put(`/workflows/${workflow.id}/share`) + .send({ shareWithIds: [member.id] }); + + expect(response.statusCode).toBe(200); + + const sharedWorkflows = await testDb.getWorkflowSharing(workflow); + expect(sharedWorkflows).toHaveLength(2); +}); + +test('PUT /workflows/:id/share should not fail when sharing with invalid user-id', async () => { + const owner = await testDb.createUser({ globalRole: globalOwnerRole }); + const workflow = await createWorkflow({}, owner); + + const response = await authAgent(owner) + .put(`/workflows/${workflow.id}/share`) + .send({ shareWithIds: [uuid()] }); + + expect(response.statusCode).toBe(200); + + const sharedWorkflows = await testDb.getWorkflowSharing(workflow); + expect(sharedWorkflows).toHaveLength(1); +}); + +test('PUT /workflows/:id/share should allow sharing with multiple users', async () => { + const owner = await testDb.createUser({ globalRole: globalOwnerRole }); + const member = await testDb.createUser({ globalRole: globalMemberRole }); + const anotherMember = await testDb.createUser({ globalRole: globalMemberRole }); + const workflow = await createWorkflow({}, owner); + + const response = await authAgent(owner) + .put(`/workflows/${workflow.id}/share`) + .send({ shareWithIds: [member.id, anotherMember.id] }); + + expect(response.statusCode).toBe(200); + + const sharedWorkflows = await testDb.getWorkflowSharing(workflow); + expect(sharedWorkflows).toHaveLength(3); +}); + +test('PUT /workflows/:id/share should override sharing', async () => { + const owner = await testDb.createUser({ globalRole: globalOwnerRole }); + const member = await testDb.createUser({ globalRole: globalMemberRole }); + const anotherMember = await testDb.createUser({ globalRole: globalMemberRole }); + const workflow = await createWorkflow({}, owner); + + const authOwnerAgent = authAgent(owner); + + const response = await authOwnerAgent + .put(`/workflows/${workflow.id}/share`) + .send({ shareWithIds: [member.id, anotherMember.id] }); + + expect(response.statusCode).toBe(200); + + const sharedWorkflows = await testDb.getWorkflowSharing(workflow); + expect(sharedWorkflows).toHaveLength(3); + + const secondResponse = await authOwnerAgent + .put(`/workflows/${workflow.id}/share`) + .send({ shareWithIds: [member.id] }); + expect(secondResponse.statusCode).toBe(200); + + const secondSharedWorkflows = await testDb.getWorkflowSharing(workflow); + expect(secondSharedWorkflows).toHaveLength(2); +}); diff --git a/packages/cli/test/integration/workflows.api.test.ts b/packages/cli/test/integration/workflows.controller.test.ts similarity index 93% rename from packages/cli/test/integration/workflows.api.test.ts rename to packages/cli/test/integration/workflows.controller.test.ts index 6cb291a9dd..7b809fa949 100644 --- a/packages/cli/test/integration/workflows.api.test.ts +++ b/packages/cli/test/integration/workflows.controller.test.ts @@ -3,6 +3,7 @@ import express from 'express'; import * as utils from './shared/utils'; import * as testDb from './shared/testDb'; import { WorkflowEntity } from '../../src/databases/entities/WorkflowEntity'; +import * as UserManagementHelpers from '../../src/UserManagement/UserManagementHelper'; import type { Role } from '../../src/databases/entities/Role'; import type { IPinData } from 'n8n-workflow'; @@ -13,6 +14,9 @@ let app: express.Application; let testDbName = ''; let globalOwnerRole: Role; +// mock whether sharing is enabled or not +jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(false); + beforeAll(async () => { app = await utils.initTestServer({ endpointGroups: ['workflows'],