From 184072cb67e82fbd755c79b5a5a7dcb09676eb85 Mon Sep 17 00:00:00 2001 From: Coenen Benjamin Date: Wed, 5 Dec 2018 14:17:21 +0100 Subject: [PATCH] fix(api): return error when a workflow contains default payload on non root node (#3688) * fix(api): return error when a workflow contains default payload on non root node Signed-off-by: Benjamin Coenen * fix(api): return error when a workflow contains default payload on non root node Signed-off-by: Benjamin Coenen * fix(api): return error when a workflow contains default payload on non root node Signed-off-by: Benjamin Coenen --- engine/api/workflow/dao_node.go | 4 ++ engine/api/workflow_test.go | 57 +++++++++++++++++++ sdk/error.go | 3 + sdk/exportentities/workflow.go | 3 + sdk/exportentities/workflow_test.go | 25 ++++++++ ui/src/app/app.component.ts | 7 ++- .../trigger/workflow.trigger.component.ts | 36 ++++++------ 7 files changed, 116 insertions(+), 19 deletions(-) diff --git a/engine/api/workflow/dao_node.go b/engine/api/workflow/dao_node.go index 718f348f6e..c5627dec26 100644 --- a/engine/api/workflow/dao_node.go +++ b/engine/api/workflow/dao_node.go @@ -92,6 +92,10 @@ func insertNode(db gorp.SqlExecutor, store cache.Store, w *sdk.Workflow, n *sdk. n.Context.DefaultPipelineParameters = defaultPipParams } + if n.Name != w.WorkflowData.Node.Name && n.Context.DefaultPayload != nil { + return sdk.ErrInvalidNodeDefaultPayload + } + if n.ID == 0 { //Insert new node dbwn := Node(*n) diff --git a/engine/api/workflow_test.go b/engine/api/workflow_test.go index 1a179e8117..ee4a596c7f 100644 --- a/engine/api/workflow_test.go +++ b/engine/api/workflow_test.go @@ -376,6 +376,63 @@ func Test_putWorkflowHandler(t *testing.T) { assert.NotEmpty(t, payload["git.branch"], "git.branch should not be empty") } +func Test_postWorkflowHandlerWithError(t *testing.T) { + // Init database + api, db, router, end := newTestAPI(t) + defer end() + + // Init user + u, pass := assets.InsertAdminUser(api.mustDB()) + // Init project + key := sdk.RandomString(10) + proj := assets.InsertTestProject(t, db, api.Cache, key, key, u) + + // Init pipeline + pip := sdk.Pipeline{ + Name: "pipeline1", + ProjectID: proj.ID, + Type: sdk.BuildPipeline, + } + test.NoError(t, pipeline.InsertPipeline(api.mustDB(), api.Cache, proj, &pip, nil)) + + //Prepare request + vars := map[string]string{ + "permProjectKey": proj.Key, + } + uri := router.GetRoute("POST", api.postWorkflowHandler, vars) + test.NotEmpty(t, uri) + + var workflow = &sdk.Workflow{ + Name: "Name", + Description: "Description", + WorkflowData: &sdk.WorkflowData{ + Node: sdk.Node{ + Type: sdk.NodeTypePipeline, + Context: &sdk.NodeContext{ + PipelineID: pip.ID, + }, + Triggers: []sdk.NodeTrigger{{ + ChildNode: sdk.Node{ + Type: sdk.NodeTypePipeline, + Context: &sdk.NodeContext{ + PipelineID: pip.ID, + DefaultPayload: map[string]interface{}{ + "test": "content", + }, + }, + }, + }}, + }, + }, + } + + req := assets.NewAuthentifiedRequest(t, u, pass, "POST", uri, &workflow) + + //Do the request + w := httptest.NewRecorder() + router.Mux.ServeHTTP(w, req) + assert.Equal(t, 400, w.Code) +} func Test_postWorkflowRollbackHandler(t *testing.T) { // Init database diff --git a/sdk/error.go b/sdk/error.go index 10d9b36bf9..8dd41da189 100644 --- a/sdk/error.go +++ b/sdk/error.go @@ -167,6 +167,7 @@ var ( ErrInvalidGroupAdmin = Error{ID: 150, Status: http.StatusForbidden} ErrWorkflowNotGenerated = Error{ID: 151, Status: http.StatusForbidden} ErrAlreadyLatestTemplate = Error{ID: 152, Status: http.StatusForbidden} + ErrInvalidNodeDefaultPayload = Error{ID: 153, Status: http.StatusBadRequest} ) var errorsAmericanEnglish = map[int]string{ @@ -316,6 +317,7 @@ var errorsAmericanEnglish = map[int]string{ ErrInvalidData.ID: "Cannot validate given data", ErrInvalidGroupAdmin.ID: "User is not a group's admin", ErrWorkflowNotGenerated.ID: "Workflow was not generated by a template", + ErrInvalidNodeDefaultPayload.ID: "Workflow node which isn't a root node cannot have a default payload", } var errorsFrench = map[int]string{ @@ -465,6 +467,7 @@ var errorsFrench = map[int]string{ ErrInvalidData.ID: "Impossible de valider les données", ErrInvalidGroupAdmin.ID: "L'utilisateur n'est pas administrateur du groupe", ErrWorkflowNotGenerated.ID: "Le workflow n'a pas été généré par un template", + ErrInvalidNodeDefaultPayload.ID: "Le workflow est incorrect. Un payload par défaut ne peut pas être sur un pipeline autre que le premier du workflow", } var errorsLanguages = []map[int]string{ diff --git a/sdk/exportentities/workflow.go b/sdk/exportentities/workflow.go index 650b75ab5a..44ba184d5e 100644 --- a/sdk/exportentities/workflow.go +++ b/sdk/exportentities/workflow.go @@ -529,6 +529,9 @@ func (e *NodeEntry) getNode(name string, w *sdk.Workflow) (*sdk.Node, error) { } if len(e.Payload) > 0 { + if len(e.DependsOn) > 0 { + return nil, fmt.Errorf("Default payload cannot be set on another node than the first one") + } node.Context.DefaultPayload = e.Payload } diff --git a/sdk/exportentities/workflow_test.go b/sdk/exportentities/workflow_test.go index b9cd9ddd74..aed0f80108 100644 --- a/sdk/exportentities/workflow_test.go +++ b/sdk/exportentities/workflow_test.go @@ -314,6 +314,26 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, }, // root(pipeline-root) -> child(pipeline-child) + { + name: "Complexe workflow without joins with a default payload on a non root node should raise an error", + fields: fields{ + Workflow: map[string]NodeEntry{ + "root": NodeEntry{ + PipelineName: "pipeline-root", + }, + "child": NodeEntry{ + PipelineName: "pipeline-child", + DependsOn: []string{"root"}, + Payload: map[string]interface{}{ + "test": "content", + }, + OneAtATime: &True, + }, + }, + }, + wantErr: true, + }, + // root(pipeline-root) -> child(pipeline-child) { name: "Complexe workflow unordered without joins should not raise an error", fields: fields{ @@ -679,6 +699,11 @@ func TestWorkflow_GetWorkflow(t *testing.T) { return } + if tt.wantErr { + assert.Error(t, err) + return + } + got.HookModels = nil got.OutGoingHookModels = nil got.Applications = nil diff --git a/ui/src/app/app.component.ts b/ui/src/app/app.component.ts index 84cdcf149d..9403dca3ae 100644 --- a/ui/src/app/app.component.ts +++ b/ui/src/app/app.component.ts @@ -140,14 +140,17 @@ export class AppComponent implements OnInit { } let authKey: string; let authValue: string; + let user = this._authStore.getUser(); // ADD user AUTH let sessionToken = this._authStore.getSessionToken(); if (sessionToken) { authKey = this._authStore.localStorageSessionKey; authValue = sessionToken; - } else { + } else if (user) { authKey = 'Authorization'; - authValue = 'Basic ' + this._authStore.getUser().token; + authValue = 'Basic ' + user.token; + } else { + return; } if (window['SharedWorker']) { diff --git a/ui/src/app/shared/workflow/modal/trigger/workflow.trigger.component.ts b/ui/src/app/shared/workflow/modal/trigger/workflow.trigger.component.ts index 372901c383..38a814477b 100644 --- a/ui/src/app/shared/workflow/modal/trigger/workflow.trigger.component.ts +++ b/ui/src/app/shared/workflow/modal/trigger/workflow.trigger.component.ts @@ -1,15 +1,12 @@ -import {Component, EventEmitter, Input, Output, ViewChild} from '@angular/core'; -import {cloneDeep} from 'lodash'; -import {ModalTemplate, SuiModalService, TemplateModalConfig} from 'ng2-semantic-ui'; -import {ActiveModal} from 'ng2-semantic-ui/dist'; -import {PipelineStatus} from '../../../../model/pipeline.model'; -import {Project} from '../../../../model/project.model'; -import { - WNode, WNodeTrigger, - Workflow, WorkflowNodeCondition, WorkflowNodeConditions -} from '../../../../model/workflow.model'; -import {WorkflowNodeOutGoingHookFormComponent} from '../../node/outgoinghook-form/outgoinghook.form.component'; -import {WorkflowNodeAddWizardComponent} from '../../node/wizard/node.wizard.component'; +import { Component, EventEmitter, Input, Output, ViewChild } from '@angular/core'; +import { cloneDeep } from 'lodash'; +import { ModalTemplate, SuiModalService, TemplateModalConfig } from 'ng2-semantic-ui'; +import { ActiveModal } from 'ng2-semantic-ui/dist'; +import { PipelineStatus } from '../../../../model/pipeline.model'; +import { Project } from '../../../../model/project.model'; +import { WNode, WNodeTrigger, Workflow, WorkflowNodeCondition, WorkflowNodeConditions } from '../../../../model/workflow.model'; +import { WorkflowNodeOutGoingHookFormComponent } from '../../node/outgoinghook-form/outgoinghook.form.component'; +import { WorkflowNodeAddWizardComponent } from '../../node/wizard/node.wizard.component'; @Component({ selector: 'app-workflow-trigger', @@ -75,20 +72,25 @@ export class WorkflowTriggerComponent { let clonedWorkflow = cloneDeep(this.workflow); if (this.source && !this.isParent) { - let n = Workflow.getNodeByID(this.source.id, clonedWorkflow); - if (!n.triggers) { - n.triggers = new Array(); + let sourceNode = Workflow.getNodeByID(this.source.id, clonedWorkflow); + if (!sourceNode.triggers) { + sourceNode.triggers = new Array(); } let newTrigger = new WNodeTrigger(); - newTrigger.parent_node_name = n.ref; + let previousDefaultPayload = this.destNode.context.default_payload; + this.destNode.context.default_payload = null; + newTrigger.parent_node_name = sourceNode.ref; newTrigger.child_node = this.destNode; - n.triggers.push(newTrigger); + sourceNode.context.default_payload = previousDefaultPayload; + sourceNode.triggers.push(newTrigger); this.triggerEvent.emit(clonedWorkflow); } else if (this.isParent) { this.destNode.triggers = new Array(); let newTrigger = new WNodeTrigger(); newTrigger.child_node = clonedWorkflow.workflow_data.node; this.destNode.triggers.push(newTrigger); + this.destNode.context.default_payload = newTrigger.child_node.context.default_payload; + newTrigger.child_node.context.default_payload = null; this.destNode.hooks = cloneDeep(clonedWorkflow.workflow_data.node.hooks); clonedWorkflow.workflow_data.node.hooks = []; clonedWorkflow.workflow_data.node = this.destNode;