fix(cdn,api): check workflow access with id, use workflow name from r…

…un (#5753)
ovh · Mar 9, 2021 · 28fcaed · 28fcaed
1 parent 7dd451e
commit 28fcaed
Show file tree

Hide file tree

Showing 11 changed files with 67 additions and 33 deletions.
diff --git a/engine/Makefile b/engine/Makefile
@@ -119,7 +119,7 @@ test-db-stop-docker:
 
 test-db-start-docker:
 	$(TEST_DB_START_DOCKER)
-	@sleep 5
+	@sleep 10
 	$(MAKE) test-api-db-create
 	$(MAKE) test-cdn-db-create
 

diff --git a/engine/api/api_routes.go b/engine/api/api_routes.go
@@ -281,7 +281,7 @@ func (api *API) InitRouter() {
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/nodes/{nodeRunID}/job/{runJobID}/service/{serviceName}/log", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowNodeRunJobServiceLogHandler))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/nodes/{nodeRunID}/job/{runJobID}/links", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowNodeRunJobStepLinksHandler))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/nodes/{nodeRunID}/job/{runJobID}/step/{stepOrder}/link", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowNodeRunJobStepLinkHandler))
-	r.Handle("/project/{key}/workflows/{workflowName}/type/{type}/access", ScopeNone(), r.GET(api.getWorkflowAccessHandler))
+	r.Handle("/project/{key}/workflows/{workflowID}/type/{type}/access", ScopeNone(), r.GET(api.getWorkflowAccessHandler))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/nodes/{nodeRunID}/job/{runJobID}/step/{stepOrder}/log", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowNodeRunJobStepLogHandler))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/node/{nodeID}/triggers/condition", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowTriggerConditionHandler))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/hook/triggers/condition", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowTriggerHookConditionHandler))

diff --git a/engine/api/workflow/dao.go b/engine/api/workflow/dao.go
@@ -79,6 +79,21 @@ func Exists(db gorp.SqlExecutor, key string, name string) (bool, error) {
 	return count > 0, nil
 }
 
+// ExistsID checks if a workflow exists for given ID and project
+func ExistsID(db gorp.SqlExecutor, key string, id int64) (bool, error) {
+	query := `
+		select count(1)
+		from workflow
+		join project on project.id = workflow.project_id
+		where project.projectkey = $1
+		and workflow.id = $2`
+	count, err := db.SelectInt(query, key, id)
+	if err != nil {
+		return false, sdk.WithStack(err)
+	}
+	return count > 0, nil
+}
+
 func LoadByRepo(ctx context.Context, db gorp.SqlExecutor, proj sdk.Project, repo string, opts LoadOptions) (*sdk.Workflow, error) {
 	ctx, end := telemetry.Span(ctx, "workflow.Load")
 	defer end()

diff --git a/engine/api/workflow_run_log.go b/engine/api/workflow_run_log.go
@@ -154,12 +154,19 @@ func (api *API) getWorkflowNodeRunJobStepLinksHandler() service.Handler {
 			return sdk.NewErrorFrom(sdk.ErrNotFound, "cannot find run job for id %d", runJobID)
 		}
 
+		jobRun, err := workflow.LoadRunByID(api.mustDB(), nodeRun.WorkflowRunID, workflow.LoadRunOptions{
+			DisableDetailledNodeRun: true,
+		})
+		if err != nil {
+			return err
+		}
+
 		refs := make([]sdk.CDNLogAPIRef, 0)
 		apiRef := sdk.CDNLogAPIRef{
 			ProjectKey:     projectKey,
-			WorkflowName:   workflowName,
-			WorkflowID:     nodeRun.WorkflowID,
-			RunID:          nodeRun.WorkflowRunID,
+			WorkflowName:   jobRun.Workflow.Name,
+			WorkflowID:     jobRun.WorkflowID,
+			RunID:          jobRun.ID,
 			NodeRunName:    nodeRun.WorkflowNodeName,
 			NodeRunID:      nodeRun.ID,
 			NodeRunJobName: runJob.Job.Action.Name,
@@ -243,11 +250,18 @@ func (api *API) getWorkflowNodeRunJobLogLinkHandler(ctx context.Context, w http.
 		return sdk.NewErrorFrom(sdk.ErrNotFound, "cannot find run job for id %d", runJobID)
 	}
 
+	jobRun, err := workflow.LoadRunByID(api.mustDB(), nodeRun.WorkflowRunID, workflow.LoadRunOptions{
+		DisableDetailledNodeRun: true,
+	})
+	if err != nil {
+		return err
+	}
+
 	apiRef := sdk.CDNLogAPIRef{
 		ProjectKey:     projectKey,
-		WorkflowName:   workflowName,
-		WorkflowID:     nodeRun.WorkflowID,
-		RunID:          nodeRun.WorkflowRunID,
+		WorkflowName:   jobRun.Workflow.Name,
+		WorkflowID:     jobRun.WorkflowID,
+		RunID:          jobRun.ID,
 		NodeRunName:    nodeRun.WorkflowNodeName,
 		NodeRunID:      nodeRun.ID,
 		NodeRunJobName: runJob.Job.Action.Name,
@@ -334,9 +348,12 @@ func (api *API) getWorkflowAccessHandler() service.Handler {
 			return sdk.WrapError(sdk.ErrForbidden, "missing session id header")
 		}
 
-		workflowName := vars["workflowName"]
+		workflowID, err := requestVarInt(r, "workflowID")
+		if err != nil {
+			return err
+		}
 
-		exists, err := workflow.Exists(api.mustDB(), projectKey, workflowName)
+		exists, err := workflow.ExistsID(api.mustDB(), projectKey, workflowID)
 		if err != nil {
 			return err
 		}
@@ -359,13 +376,14 @@ func (api *API) getWorkflowAccessHandler() service.Handler {
 
 		maintainerOrAdmin := consumer.Maintainer() || consumer.Admin()
 
-		perms, err := permission.LoadWorkflowMaxLevelPermission(ctx, api.mustDB(), projectKey, []string{workflowName}, consumer.GetGroupIDs())
+		perms, err := permission.LoadWorkflowMaxLevelPermissionByWorkflowIDs(ctx, api.mustDB(), []int64{workflowID}, consumer.GetGroupIDs())
 		if err != nil {
 			return sdk.NewErrorWithStack(err, sdk.ErrUnauthorized)
 		}
-		maxLevelPermission := perms.Level(workflowName)
+		workflowIDString := strconv.FormatInt(workflowID, 10)
+		maxLevelPermission := perms.Level(workflowIDString)
 		if maxLevelPermission < sdk.PermissionRead && !maintainerOrAdmin {
-			return sdk.WrapError(sdk.ErrUnauthorized, "not authorized for workflow %s/%s", projectKey, workflowName)
+			return sdk.WrapError(sdk.ErrUnauthorized, "not authorized for workflow %s/%s", projectKey, workflowIDString)
 		}
 
 		return service.WriteJSON(w, nil, http.StatusOK)

diff --git a/engine/cdn/auth.go b/engine/cdn/auth.go
@@ -89,16 +89,17 @@ func (s *Service) itemAccessCheck(ctx context.Context, item sdk.CDNItem) error {
 		return nil
 	}
 
-	var projectKey, workflowName string
+	var projectKey string
+	var workflowID int64
 	switch item.Type {
 	case sdk.CDNTypeItemStepLog, sdk.CDNTypeItemServiceLog:
 		logRef, _ := item.GetCDNLogApiRef()
 		projectKey = logRef.ProjectKey
-		workflowName = logRef.WorkflowName
+		workflowID = logRef.WorkflowID
 	case sdk.CDNTypeItemRunResult:
 		artRef, _ := item.GetCDNRunResultApiRef()
 		projectKey = artRef.ProjectKey
-		workflowName = artRef.WorkflowName
+		workflowID = artRef.WorkflowID
 	case sdk.CDNTypeItemWorkerCache:
 		artRef, _ := item.GetCDNWorkerCacheApiRef()
 		projectKey = artRef.ProjectKey
@@ -108,7 +109,7 @@ func (s *Service) itemAccessCheck(ctx context.Context, item sdk.CDNItem) error {
 
 	switch item.Type {
 	case sdk.CDNTypeItemStepLog, sdk.CDNTypeItemServiceLog, sdk.CDNTypeItemRunResult:
-		if err := s.Client.WorkflowAccess(ctx, projectKey, workflowName, sessionID, item.Type); err != nil {
+		if err := s.Client.WorkflowAccess(ctx, projectKey, workflowID, sessionID, item.Type); err != nil {
 			return sdk.NewErrorWithStack(err, sdk.ErrNotFound)
 		}
 	case sdk.CDNTypeItemWorkerCache:

diff --git a/engine/cdn/auth_test.go b/engine/cdn/auth_test.go
@@ -45,7 +45,7 @@ func Test_itemAccessMiddleware(t *testing.T) {
 	sessionID := sdk.UUID()
 	mockClient.EXPECT().
 		WorkflowAccess(gomock.Any(), gomock.Any(), gomock.Any(), sessionID, gomock.Any()).
-		DoAndReturn(func(ctx context.Context, projectKey, workflowName, sessionID string, itemItem sdk.CDNItemType) error {
+		DoAndReturn(func(ctx context.Context, projectKey string, workflowName int64, sessionID string, itemItem sdk.CDNItemType) error {
 			return nil
 		}).
 		Times(1)

diff --git a/engine/cdn/item_handler_test.go b/engine/cdn/item_handler_test.go
@@ -111,7 +111,7 @@ func TestGetItemLogsDownloadHandler(t *testing.T) {
 	s, db := newTestService(t)
 	s.Client = cdsclient.New(cdsclient.Config{Host: "http://lolcat.api", InsecureSkipVerifyTLS: false})
 	gock.InterceptClient(s.Client.(cdsclient.Raw).HTTPClient())
-	gock.New("http://lolcat.api").Get("/project/" + projectKey + "/workflows/MyWorkflow/type/step-log/access").Reply(http.StatusOK).JSON(nil)
+	gock.New("http://lolcat.api").Get("/project/" + projectKey + "/workflows/1/type/step-log/access").Reply(http.StatusOK).JSON(nil)
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	t.Cleanup(cancel)
@@ -213,7 +213,7 @@ func TestGetItemArtifactDownloadHandler(t *testing.T) {
 	s.Client = cdsclient.New(cdsclient.Config{Host: "http://lolcat.api", InsecureSkipVerifyTLS: false})
 	gock.InterceptClient(s.Client.(cdsclient.Raw).HTTPClient())
 	t.Cleanup(gock.OffAll)
-	gock.New("http://lolcat.api").Get("/project/" + projectKey + "/workflows/WfName/type/run-result/access").Reply(http.StatusOK).JSON(nil)
+	gock.New("http://lolcat.api").Get("/project/" + projectKey + "/workflows/1/type/run-result/access").Reply(http.StatusOK).JSON(nil)
 
 	gock.New("http://lolcat.api").Post("/project/" + projectKey + "/workflows/WfName/runs/0/results/check").Reply(http.StatusNoContent)
 	gock.New("http://lolcat.api").Post("/project/" + projectKey + "/workflows/WfName/runs/0/results").Reply(http.StatusNoContent)

diff --git a/engine/cdn/item_logs_handler_test.go b/engine/cdn/item_logs_handler_test.go
@@ -202,7 +202,7 @@ func TestGetItemLogsLinesHandler(t *testing.T) {
 	s.Client = cdsclient.New(cdsclient.Config{Host: "http://lolcat.api", InsecureSkipVerifyTLS: false})
 	gock.InterceptClient(s.Client.(cdsclient.Raw).HTTPClient())
 	t.Cleanup(gock.Off)
-	gock.New("http://lolcat.api").Get("/project/" + projectKey + "/workflows/MyWorkflow/type/step-log/access").Reply(http.StatusOK).JSON(nil)
+	gock.New("http://lolcat.api").Get("/project/" + projectKey + "/workflows/1/type/step-log/access").Reply(http.StatusOK).JSON(nil)
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	t.Cleanup(cancel)
@@ -296,7 +296,7 @@ func TestGetItemLogsStreamHandler(t *testing.T) {
 	s.Client = cdsclient.New(cdsclient.Config{Host: "http://lolcat.api", InsecureSkipVerifyTLS: false})
 	gock.InterceptClient(s.Client.(cdsclient.Raw).HTTPClient())
 	t.Cleanup(gock.Off)
-	gock.New("http://lolcat.api").Get("/project/" + projectKey + "/workflows/MyWorkflow/type/step-log/access").Reply(http.StatusOK).JSON(nil)
+	gock.New("http://lolcat.api").Get("/project/" + projectKey + "/workflows/1/type/step-log/access").Reply(http.StatusOK).JSON(nil)
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	t.Cleanup(cancel)

diff --git a/sdk/cdsclient/client_workflow.go b/sdk/cdsclient/client_workflow.go
@@ -249,8 +249,8 @@ func (c *client) WorkflowNodeRunJobServiceLink(ctx context.Context, projectKey s
 	return &a, nil
 }
 
-func (c *client) WorkflowAccess(ctx context.Context, projectKey, workflowName, sessionID string, itemType sdk.CDNItemType) error {
-	url := fmt.Sprintf("/project/%s/workflows/%s/type/%s/access", projectKey, workflowName, itemType)
+func (c *client) WorkflowAccess(ctx context.Context, projectKey string, workflowID int64, sessionID string, itemType sdk.CDNItemType) error {
+	url := fmt.Sprintf("/project/%s/workflows/%d/type/%s/access", projectKey, workflowID, itemType)
 	if _, err := c.GetJSON(ctx, url, nil, SetHeader(sdk.CDSSessionID, sessionID)); err != nil {
 		return err
 	}

diff --git a/sdk/cdsclient/interface.go b/sdk/cdsclient/interface.go
@@ -351,7 +351,7 @@ type WorkflowClient interface {
 	WorkflowNodeRunJobStepLog(ctx context.Context, projectKey string, workflowName string, nodeRunID, job int64, step int64) (*sdk.BuildState, error)
 	WorkflowNodeRunJobServiceLink(ctx context.Context, projectKey string, workflowName string, nodeRunID, job int64, serviceName string) (*sdk.CDNLogLink, error)
 	WorkflowNodeRunJobServiceLog(ctx context.Context, projectKey string, workflowName string, nodeRunID, job int64, serviceName string) (*sdk.ServiceLog, error)
-	WorkflowAccess(ctx context.Context, projectKey, workflowName, sessionID string, itemType sdk.CDNItemType) error
+	WorkflowAccess(ctx context.Context, projectKey string, workflowID int64, sessionID string, itemType sdk.CDNItemType) error
 	WorkflowLogDownload(ctx context.Context, link sdk.CDNLogLink) ([]byte, error)
 	WorkflowNodeRunRelease(projectKey string, workflowName string, runNumber int64, nodeRunID int64, release sdk.WorkflowNodeRunRelease) error
 	WorkflowAllHooksList() ([]sdk.NodeHook, error)

diff --git a/sdk/cdsclient/mock_cdsclient/interface_mock.go b/sdk/cdsclient/mock_cdsclient/interface_mock.go