diff --git a/packages/cli/src/requests.d.ts b/packages/cli/src/requests.d.ts index 53ad893ec6..6c077f958f 100644 --- a/packages/cli/src/requests.d.ts +++ b/packages/cli/src/requests.d.ts @@ -56,7 +56,12 @@ export declare namespace WorkflowRequest { type Delete = Get; - type Update = AuthenticatedRequest<{ id: string }, {}, RequestBody>; + type Update = AuthenticatedRequest< + { id: string }, + {}, + RequestBody & { updatedAt: string }, + { forceSave?: string } + >; type NewName = AuthenticatedRequest<{}, {}, {}, { name?: string }>; diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index e6a2ea88ab..4fb03ead89 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -329,6 +329,7 @@ workflowsController.patch( `/:id`, ResponseHelper.send(async (req: WorkflowRequest.Update) => { const { id: workflowId } = req.params; + const { forceSave } = req.query; const updateData = new WorkflowEntity(); const { tags, ...rest } = req.body; @@ -355,6 +356,22 @@ workflowsController.patch( ); } + const lastKnownDate = new Date(req.body.updatedAt).getTime(); + const storedDate = new Date(shared.workflow.updatedAt).getTime(); + + if (!forceSave && lastKnownDate !== storedDate) { + LoggerProxy.info( + 'User was blocked from updating a workflow that was changed by another user', + { workflowId, userId: req.user.id }, + ); + + throw new ResponseHelper.ResponseError( + `Workflow ID ${workflowId} cannot be saved because it was changed by another user.`, + undefined, + 400, + ); + } + // check credentials for old format await WorkflowHelpers.replaceInvalidCredentials(updateData); diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index 39c81799a6..223050082c 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -706,10 +706,7 @@ export const emptyPackage = () => { // workflow // ---------------------------------- -export function makeWorkflow({ - withPinData, - withCredential, -}: { +export function makeWorkflow(options?: { withPinData: boolean; withCredential?: { id: string; name: string }; }) { @@ -724,9 +721,9 @@ export function makeWorkflow({ position: [740, 240], }; - if (withCredential) { + if (options?.withCredential) { node.credentials = { - spotifyApi: withCredential, + spotifyApi: options.withCredential, }; } @@ -735,7 +732,7 @@ export function makeWorkflow({ workflow.connections = {}; workflow.nodes = [node]; - if (withPinData) { + if (options?.withPinData) { workflow.pinData = MOCK_PINDATA; } diff --git a/packages/cli/test/integration/workflows.controller.ee.test.ts b/packages/cli/test/integration/workflows.controller.ee.test.ts index 30d304d89c..758a09164b 100644 --- a/packages/cli/test/integration/workflows.controller.ee.test.ts +++ b/packages/cli/test/integration/workflows.controller.ee.test.ts @@ -295,3 +295,153 @@ describe('POST /workflows', () => { expect(usedCredentials).toHaveLength(1); }); }); + +describe('PATCH /workflows/:id', () => { + it('should block owner update on interim update by member', async () => { + const owner = await testDb.createUser({ globalRole: globalOwnerRole }); + const member = await testDb.createUser({ globalRole: globalMemberRole }); + + // owner creates and shares workflow + + const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); + const { id, updatedAt: ownerLastKnownDate } = createResponse.body.data; + await authAgent(owner) + .put(`/workflows/${id}/share`) + .send({ shareWithIds: [member.id] }); + + // member accesses and updates workflow + + const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); + const { updatedAt: memberLastKnownDate } = memberGetResponse.body.data; + + await authAgent(member) + .patch(`/workflows/${id}`) + .send({ name: 'Update by member', updatedAt: memberLastKnownDate }); + + // owner blocked from updating workflow + + const updateAttemptResponse = await authAgent(owner) + .patch(`/workflows/${id}`) + .send({ name: 'Update attempt by owner', updatedAt: ownerLastKnownDate }); + + expect(updateAttemptResponse.status).toBe(400); + expect(updateAttemptResponse.body.message).toContain( + 'cannot be saved because it was changed by another user', + ); + }); + + it('should block member update on interim update by owner', async () => { + const owner = await testDb.createUser({ globalRole: globalOwnerRole }); + const member = await testDb.createUser({ globalRole: globalMemberRole }); + + // owner creates, updates and shares workflow + + const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); + const { id, updatedAt: ownerFirstUpdateDate } = createResponse.body.data; + + const updateResponse = await authAgent(owner) + .patch(`/workflows/${id}`) + .send({ name: 'Update by owner', updatedAt: ownerFirstUpdateDate }); + const { updatedAt: ownerSecondUpdateDate } = updateResponse.body.data; + + await authAgent(owner) + .put(`/workflows/${id}/share`) + .send({ shareWithIds: [member.id] }); + + // member accesses workflow + + const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); + const { updatedAt: memberLastKnownDate } = memberGetResponse.body.data; + + // owner re-updates workflow + + await authAgent(owner) + .patch(`/workflows/${id}`) + .send({ name: 'Owner update again', updatedAt: ownerSecondUpdateDate }); + + // member blocked from updating workflow + + const updateAttemptResponse = await authAgent(member) + .patch(`/workflows/${id}`) + .send({ name: 'Update attempt by member', updatedAt: memberLastKnownDate }); + + expect(updateAttemptResponse.status).toBe(400); + expect(updateAttemptResponse.body.message).toContain( + 'cannot be saved because it was changed by another user', + ); + }); + + it('should block owner activation on interim activation by member', async () => { + const owner = await testDb.createUser({ globalRole: globalOwnerRole }); + const member = await testDb.createUser({ globalRole: globalMemberRole }); + + // owner creates and shares workflow + + const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); + const { id, updatedAt: ownerLastKnownDate } = createResponse.body.data; + await authAgent(owner) + .put(`/workflows/${id}/share`) + .send({ shareWithIds: [member.id] }); + + // member accesses and activates workflow + + const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); + const { updatedAt: memberLastKnownDate } = memberGetResponse.body.data; + + await authAgent(member) + .patch(`/workflows/${id}`) + .send({ active: true, updatedAt: memberLastKnownDate }); + + // owner blocked from activating workflow + + const activationAttemptResponse = await authAgent(owner) + .patch(`/workflows/${id}`) + .send({ active: true, updatedAt: ownerLastKnownDate }); + + expect(activationAttemptResponse.status).toBe(400); + expect(activationAttemptResponse.body.message).toContain( + 'cannot be saved because it was changed by another user', + ); + }); + + it('should block member activation on interim activation by owner', async () => { + const owner = await testDb.createUser({ globalRole: globalOwnerRole }); + const member = await testDb.createUser({ globalRole: globalMemberRole }); + + // owner creates, updates and shares workflow + + const createResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); + const { id, updatedAt: ownerFirstUpdateDate } = createResponse.body.data; + + const updateResponse = await authAgent(owner) + .patch(`/workflows/${id}`) + .send({ name: 'Update by owner', updatedAt: ownerFirstUpdateDate }); + const { updatedAt: ownerSecondUpdateDate } = updateResponse.body.data; + + await authAgent(owner) + .put(`/workflows/${id}/share`) + .send({ shareWithIds: [member.id] }); + + // member accesses workflow + + const memberGetResponse = await authAgent(member).get(`/workflows/${id}`); + const { updatedAt: memberLastKnownDate } = memberGetResponse.body.data; + + // owner activates workflow + + await authAgent(owner) + .patch(`/workflows/${id}`) + .send({ active: true, updatedAt: ownerSecondUpdateDate }); + + // member blocked from activating workflow + + const updateAttemptResponse = await authAgent(member) + .patch(`/workflows/${id}`) + .send({ active: true, updatedAt: memberLastKnownDate }); + + expect(updateAttemptResponse.status).toBe(400); + expect(updateAttemptResponse.body.message).toContain( + 'cannot be saved because it was changed by another user', + ); + }); +}); diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 4613303b1c..7ea10e7e13 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -268,6 +268,7 @@ export interface IWorkflowData { settings?: IWorkflowSettings; tags?: string[]; pinData?: IPinData; + updatedAt?: string; } export interface IWorkflowDataUpdate { @@ -279,6 +280,7 @@ export interface IWorkflowDataUpdate { active?: boolean; tags?: ITag[] | string[]; // string[] when store or requested, ITag[] from API response pinData?: IPinData; + updatedAt?: string; } export interface IWorkflowToShare extends IWorkflowDataUpdate { diff --git a/packages/editor-ui/src/components/WorkflowCard.vue b/packages/editor-ui/src/components/WorkflowCard.vue index 4af6ba0336..0b76843a5a 100644 --- a/packages/editor-ui/src/components/WorkflowCard.vue +++ b/packages/editor-ui/src/components/WorkflowCard.vue @@ -10,7 +10,15 @@
- {{$locale.baseText('workflows.item.updated')}} | + + + {{ $locale.baseText('workflows.item.neverUpdated') }} | + + + {{ $locale.baseText('workflows.item.updated') }} + | + + {{$locale.baseText('workflows.item.created')}} {{ formattedCreatedAtDate }} { return state.workflow.id; }, + workflowUpdatedAt (state): string | number { + return state.workflow.updatedAt; + }, workflowSettings: (state): IWorkflowSettings => { if (state.workflow.settings === undefined) { diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index 8cdbde4c8a..7d42d6fc61 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -715,6 +715,7 @@ export default mixins( this.$store.commit('setActive', data.active || false); this.$store.commit('setWorkflowId', workflowId); + this.$store.commit('setWorkflowUpdatedAt', data.updatedAt); this.$store.commit('setWorkflowName', { newName: data.name, setStateDirty: false }); this.$store.commit('setWorkflowSettings', data.settings || {}); this.$store.commit('setWorkflowPinData', data.pinData || {});