Skip to content

Commit b289f6b

Browse files
richardltrichardlt
authored
fix(api): check if workflow exists found before permission (#5084)
1 parent fc05887 commit b289f6b

File tree

10 files changed

+25
-19
lines changed

10 files changed

+25
-19
lines changed

‎cli/cdsctl/workflow_delete.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ var workflowDeleteCmd = cli.Command{
1919

2020
func workflowDeleteRun(v cli.Values) error {
2121
err := client.WorkflowDelete(v.GetString(_ProjectKey), v.GetString(_WorkflowName))
22-
if err != nil && v.GetBool("force") && sdk.ErrorIs(err, sdk.ErrWorkflowNotFound) {
22+
if err != nil && v.GetBool("force") && sdk.ErrorIs(err, sdk.ErrNotFound) {
2323
fmt.Println(err.Error())
2424
os.Exit(0)
2525
}

‎engine/api/pipeline_test.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,9 @@ import (
1010
"testing"
1111
"time"
1212

13-
"github.com/ovh/cds/engine/api/application"
14-
1513
"github.com/stretchr/testify/assert"
1614

15+
"github.com/ovh/cds/engine/api/application"
1716
"github.com/ovh/cds/engine/api/pipeline"
1817
"github.com/ovh/cds/engine/api/repositoriesmanager"
1918
"github.com/ovh/cds/engine/api/services"
@@ -104,6 +103,7 @@ func TestUpdateAsCodePipelineHandler(t *testing.T) {
104103
"secret": "bar",
105104
},
106105
}))
106+
wkf := assets.InsertTestWorkflow(t, db, api.Cache, proj, sdk.RandomString(10))
107107

108108
pip := sdk.Pipeline{
109109
Name: sdk.RandomString(10),
@@ -154,7 +154,7 @@ func TestUpdateAsCodePipelineHandler(t *testing.T) {
154154
// Get operation
155155
uriGET := api.Router.GetRoute("GET", api.getWorkflowAsCodeHandler, map[string]string{
156156
"key": proj.Key,
157-
"permWorkflowName": pip.Name,
157+
"permWorkflowName": wkf.Name,
158158
"uuid": myOpe.UUID,
159159
})
160160
reqGET, err := http.NewRequest("GET", uriGET, nil)
@@ -176,5 +176,4 @@ func TestUpdateAsCodePipelineHandler(t *testing.T) {
176176
assert.Equal(t, "myURL", myOpeGet.Setup.Push.PRLink)
177177
break
178178
}
179-
180179
}

‎engine/api/router_middleware_auth_permission.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,13 +153,21 @@ func (api *API) checkWorkflowPermissions(ctx context.Context, workflowName strin
153153
projectKey, has = routeVars["key"]
154154
}
155155
if !has {
156-
return sdk.WrapError(sdk.ErrForbidden, "not authorized for workflow %s, missing project key value", workflowName)
156+
return sdk.WithStack(sdk.ErrNotFound)
157157
}
158158

159159
if workflowName == "" {
160160
return sdk.WrapError(sdk.ErrWrongRequest, "invalid given workflow name")
161161
}
162162

163+
exists, err := workflow.Exists(api.mustDB(), projectKey, workflowName)
164+
if err != nil {
165+
return err
166+
}
167+
if !exists {
168+
return sdk.WithStack(sdk.ErrNotFound)
169+
}
170+
163171
perms, err := permission.LoadWorkflowMaxLevelPermission(ctx, api.mustDB(), projectKey, []string{workflowName}, getAPIConsumer(ctx).GetGroupIDs())
164172
if err != nil {
165173
return sdk.NewError(sdk.ErrForbidden, err)

‎engine/api/templates.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ func (api *API) getTemplateInstanceHandler() service.Handler {
690690

691691
wf, err := workflow.Load(ctx, api.mustDB(), api.Cache, *proj, workflowName, workflow.LoadOptions{})
692692
if err != nil {
693-
if sdk.ErrorIs(err, sdk.ErrWorkflowNotFound) {
693+
if sdk.ErrorIs(err, sdk.ErrNotFound) {
694694
return sdk.NewErrorFrom(sdk.ErrNotFound, "cannot load workflow %s", workflowName)
695695
}
696696
return sdk.WithStack(err)

‎engine/api/workflow.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ func (api *API) deleteWorkflowHandler() service.Handler {
577577
return sdk.WrapError(errW, "Cannot check Workflow %s", key)
578578
}
579579
if !b {
580-
return sdk.WithStack(sdk.ErrWorkflowNotFound)
580+
return sdk.WithStack(sdk.ErrNotFound)
581581
}
582582

583583
tx, errT := api.mustDB().Begin()
@@ -713,7 +713,7 @@ func (api *API) getWorkflowNotificationsConditionsHandler() service.Handler {
713713

714714
wr, errr := workflow.LoadLastRun(api.mustDB(), key, name, workflow.LoadRunOptions{})
715715
if errr != nil {
716-
if !sdk.ErrorIs(errr, sdk.ErrWorkflowNotFound) {
716+
if !sdk.ErrorIs(errr, sdk.ErrNotFound) {
717717
return sdk.WrapError(errr, "getWorkflowTriggerConditionHandler> Unable to load last run workflow")
718718
}
719719
}

‎engine/api/workflow/dao.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ func LoadAll(db gorp.SqlExecutor, projectKey string) (sdk.Workflows, error) {
263263

264264
if _, err := db.Select(&dbRes, query, projectKey); err != nil {
265265
if err == sql.ErrNoRows {
266-
return nil, sdk.ErrWorkflowNotFound
266+
return nil, sdk.WithStack(sdk.ErrNotFound)
267267
}
268268
return nil, sdk.WrapError(err, "Unable to load workflows project %s", projectKey)
269269
}
@@ -511,7 +511,7 @@ func load(ctx context.Context, db gorp.SqlExecutor, proj sdk.Project, opts LoadO
511511
next()
512512
if err != nil {
513513
if err == sql.ErrNoRows {
514-
return nil, sdk.ErrWorkflowNotFound
514+
return nil, sdk.WithStack(sdk.ErrNotFound)
515515
}
516516
return nil, sdk.WrapError(err, "Unable to load workflow")
517517
}

‎engine/api/workflow/dao_run.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -439,13 +439,13 @@ func loadRun(db gorp.SqlExecutor, loadOpts LoadRunOptions, query string, args ..
439439
runDB := &Run{}
440440
if err := db.SelectOne(runDB, query, args...); err != nil {
441441
if err == sql.ErrNoRows {
442-
return nil, sdk.ErrWorkflowNotFound
442+
return nil, sdk.WithStack(sdk.ErrNotFound)
443443
}
444444
return nil, sdk.WrapError(err, "Unable to load workflow run. query:%s args:%v", query, args)
445445
}
446446
wr := sdk.WorkflowRun(*runDB)
447447
if !loadOpts.WithDeleted && wr.ToDelete {
448-
return nil, sdk.WithStack(sdk.ErrWorkflowNotFound)
448+
return nil, sdk.WithStack(sdk.ErrNotFound)
449449
}
450450

451451
tags, errT := loadTagsByRunID(db, wr.ID)

‎engine/api/workflow_run_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2522,14 +2522,16 @@ func Test_deleteWorkflowRunHandler(t *testing.T) {
25222522
func Test_postWorkflowRunHandlerBadResyncOptions(t *testing.T) {
25232523
api, db, router, end := newTestAPI(t)
25242524
defer end()
2525-
u, pass := assets.InsertAdminUser(t, api.mustDB())
2525+
25262526
key := sdk.RandomString(10)
25272527
proj := assets.InsertTestProject(t, db, api.Cache, key, key)
2528+
w := assets.InsertTestWorkflow(t, db, api.Cache, proj, sdk.RandomString(10))
2529+
u, pass := assets.InsertLambdaUser(t, api.mustDB(), &proj.ProjectGroups[0].Group)
25282530

25292531
//Prepare request
25302532
vars := map[string]string{
25312533
"key": proj.Key,
2532-
"permWorkflowName": "foo",
2534+
"permWorkflowName": w.Name,
25332535
}
25342536
uri := router.GetRoute("POST", api.postWorkflowRunHandler, vars)
25352537
test.NotEmpty(t, uri)

‎engine/api/workflow_trigger.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func (api *API) getWorkflowTriggerConditionHandler() service.Handler {
3939

4040
wr, err := workflow.LoadLastRun(api.mustDB(), key, name, workflow.LoadRunOptions{})
4141
if err != nil {
42-
if !sdk.ErrorIs(err, sdk.ErrWorkflowNotFound) {
42+
if !sdk.ErrorIs(err, sdk.ErrNotFound) {
4343
return sdk.WrapError(err, "unable to load last run workflow")
4444
}
4545
}

‎sdk/error.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ var (
105105
ErrJobAlreadyBooked = Error{ID: 89, Status: http.StatusConflict}
106106
ErrPipelineBuildNotFound = Error{ID: 90, Status: http.StatusNotFound}
107107
ErrAlreadyTaken = Error{ID: 91, Status: http.StatusGone}
108-
ErrWorkflowNotFound = Error{ID: 92, Status: http.StatusNotFound}
109108
ErrWorkflowNodeNotFound = Error{ID: 93, Status: http.StatusNotFound}
110109
ErrWorkflowInvalidRoot = Error{ID: 94, Status: http.StatusBadRequest}
111110
ErrWorkflowNodeRef = Error{ID: 95, Status: http.StatusBadRequest}
@@ -296,7 +295,6 @@ var errorsAmericanEnglish = map[int]string{
296295
ErrJobAlreadyBooked.ID: "Job already booked",
297296
ErrPipelineBuildNotFound.ID: "Pipeline build not found",
298297
ErrAlreadyTaken.ID: "This job is already taken by another worker",
299-
ErrWorkflowNotFound.ID: "Workflow not found",
300298
ErrWorkflowNodeNotFound.ID: "Workflow node not found",
301299
ErrWorkflowInvalidRoot.ID: "Invalid workflow root",
302300
ErrWorkflowNodeRef.ID: "Invalid workflow node reference",
@@ -480,7 +478,6 @@ var errorsFrench = map[int]string{
480478
ErrJobAlreadyBooked.ID: "Le job est déjà réservé",
481479
ErrPipelineBuildNotFound.ID: "Le pipeline build n'a pu être trouvé",
482480
ErrAlreadyTaken.ID: "Ce job est déjà en cours de traitement par un autre worker",
483-
ErrWorkflowNotFound.ID: "Workflow introuvable",
484481
ErrWorkflowNodeNotFound.ID: "Noeud de Workflow introuvable",
485482
ErrWorkflowInvalidRoot.ID: "Racine de Workflow invalide",
486483
ErrWorkflowNodeRef.ID: "Référence de noeud de workflow invalide",

0 commit comments

Comments
 (0)