diff --git a/packages/cli/src/requests.d.ts b/packages/cli/src/requests.d.ts index 53ad893ec6ab3..6c077f958f448 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 e6a2ea88abb26..4fb03ead89a0d 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 39c81799a68f3..223050082c6b7 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 30d304d89c377..118ebf8a6ad83 100644 --- a/packages/cli/test/integration/workflows.controller.ee.test.ts +++ b/packages/cli/test/integration/workflows.controller.ee.test.ts @@ -294,4 +294,78 @@ describe('POST /workflows', () => { const usedCredentials = await testDb.getCredentialUsageInWorkflow(response.body.data.id); expect(usedCredentials).toHaveLength(1); }); + + it('PATCH /workflows/:id should be blocked on interim change - owner blocked', 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('PATCH /workflows/:id should be blocked on interim change - member blocked', 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', + ); + }); }); diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 1a10c0f4c8db0..0d634795204ff 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -241,6 +241,7 @@ export interface IWorkflowData { settings?: IWorkflowSettings; tags?: string[]; pinData?: IPinData; + updatedAt?: string; } export interface IWorkflowDataUpdate { @@ -252,6 +253,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/mixins/workflowHelpers.ts b/packages/editor-ui/src/components/mixins/workflowHelpers.ts index 74210ae7419cd..733a9cf5fe121 100644 --- a/packages/editor-ui/src/components/mixins/workflowHelpers.ts +++ b/packages/editor-ui/src/components/mixins/workflowHelpers.ts @@ -400,6 +400,7 @@ export const workflowHelpers = mixins( active: this.$store.getters.isActive, settings: this.$store.getters.workflowSettings, tags: this.$store.getters.workflowTags, + updatedAt: this.$store.getters.workflowUpdatedAt, }; const workflowId = this.$store.getters.workflowId; @@ -678,6 +679,8 @@ export const workflowHelpers = mixins( } else { this.$store.commit('setWorkflowInactive', workflowId); } + + this.$store.commit('setWorkflowUpdatedAt', data.updatedAt); }, async saveCurrentWorkflow({name, tags}: {name?: string, tags?: string[]} = {}, redirect = true): Promise { @@ -714,6 +717,7 @@ export const workflowHelpers = mixins( this.$store.commit('setStateDirty', false); this.$store.commit('removeActiveAction', 'workflowSaving'); + this.$store.commit('setWorkflowUpdatedAt', workflowData.updatedAt); this.$externalHooks().run('workflow.afterUpdate', { workflowData }); return true; diff --git a/packages/editor-ui/src/store.ts b/packages/editor-ui/src/store.ts index b18211592dec0..6ec17c700d07a 100644 --- a/packages/editor-ui/src/store.ts +++ b/packages/editor-ui/src/store.ts @@ -464,6 +464,11 @@ export const store = new Vuex.Store({ state.workflow.id = id; }, + // updatedAt + setWorkflowUpdatedAt (state, updatedAt: string) { + state.workflow.updatedAt = updatedAt; + }, + // Name setWorkflowName(state, data) { if (data.setStateDirty === true) { @@ -1009,6 +1014,9 @@ export const store = new Vuex.Store({ workflowId: (state): string => { 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 9ba2eaf4dc3b3..4adaf36697148 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -735,6 +735,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 || {});