fix(api): update permission for workflow when exists in upsert (#5256)

engine/api/api_routes.go

@@ -258,12 +258,12 @@ func (api *API) InitRouter() {
 	// Workflows run
 	r.Handle("/project/{permProjectKey}/runs", Scope(sdk.AuthConsumerScopeProject), r.GET(api.getWorkflowAllRunsHandler, EnableTracing()))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/artifact/{artifactId}", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getDownloadArtifactHandler))
-	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowRunsHandler, EnableTracing()), r.POSTEXECUTE(api.postWorkflowRunHandler /*, AllowServices(true)*/, EnableTracing()))
-	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/branch/{branch}", Scope(sdk.AuthConsumerScopeRun), r.DELETE(api.deleteWorkflowRunsBranchHandler /*, NeedService()*/))
+	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowRunsHandler, EnableTracing()), r.POSTEXECUTE(api.postWorkflowRunHandler, EnableTracing()))
+	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/branch/{branch}", Scope(sdk.AuthConsumerScopeRun), r.DELETE(api.deleteWorkflowRunsBranchHandler))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/latest", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getLatestWorkflowRunHandler))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/tags", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowRunTagsHandler))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/num", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowRunNumHandler), r.POST(api.postWorkflowRunNumHandler))
-	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/{number}", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowRunHandler /*, AllowServices(true)*/, EnableTracing()), r.DELETE(api.deleteWorkflowRunHandler))
+	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/{number}", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowRunHandler, EnableTracing()), r.DELETE(api.deleteWorkflowRunHandler))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/{number}/stop", Scope(sdk.AuthConsumerScopeRun), r.POSTEXECUTE(api.stopWorkflowRunHandler, EnableTracing(), MaintenanceAware()))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/{number}/vcs/resync", Scope(sdk.AuthConsumerScopeRun), r.POSTEXECUTE(api.postResyncVCSWorkflowRunHandler))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/{number}/artifacts", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowRunArtifactsHandler))
@@ -278,8 +278,8 @@ func (api *API) InitRouter() {
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/hook/triggers/condition", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowTriggerHookConditionHandler))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/triggers/condition", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowTriggerConditionHandler))
 	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/{number}/nodes/{nodeRunID}/release", Scope(sdk.AuthConsumerScopeRun), r.POST(api.releaseApplicationWorkflowHandler, MaintenanceAware()))
-	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/{number}/hooks/{hookRunID}/callback", Scope(sdk.AuthConsumerScopeRun), r.POST(api.postWorkflowJobHookCallbackHandler, MaintenanceAware() /*, AllowServices(true)*/))
-	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/{number}/hooks/{hookRunID}/details", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowJobHookDetailsHandler /*, NeedService()*/))
+	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/{number}/hooks/{hookRunID}/callback", Scope(sdk.AuthConsumerScopeRun), r.POST(api.postWorkflowJobHookCallbackHandler, MaintenanceAware()))
+	r.Handle("/project/{key}/workflows/{permWorkflowName}/runs/{number}/hooks/{hookRunID}/details", Scope(sdk.AuthConsumerScopeRun), r.GET(api.getWorkflowJobHookDetailsHandler))
 
 	// Environment
 	r.Handle("/project/{permProjectKey}/environment", Scope(sdk.AuthConsumerScopeProject), r.GET(api.getEnvironmentsHandler), r.POST(api.addEnvironmentHandler))

engine/api/group/workflow_group.go

@@ -83,6 +83,7 @@ func UpdateWorkflowGroup(ctx context.Context, db gorp.SqlExecutor, w *sdk.Workfl
 	if !ok {
 		return sdk.WithStack(sdk.ErrLastGroupWithWriteRole)
 	}
+
 	return nil
 }
 
@@ -96,11 +97,12 @@ func UpsertAllWorkflowGroups(db gorp.SqlExecutor, w *sdk.Workflow, gps []sdk.Gro
 
 	ok, err := checkAtLeastOneGroupWithWriteRoleOnWorkflow(db, w.ID)
 	if err != nil {
-		return sdk.WrapError(err, "U")
+		return err
 	}
 	if !ok {
-		return sdk.WrapError(sdk.ErrLastGroupWithWriteRole, "U")
+		return sdk.WithStack(sdk.ErrLastGroupWithWriteRole)
 	}
+
 	return nil
 }
 
@@ -111,7 +113,7 @@ func UpsertWorkflowGroup(db gorp.SqlExecutor, projectID, workflowID int64, gp sd
 				(SELECT id FROM project_group WHERE project_group.project_id = $1 AND project_group.group_id = $2),
 				$3,
 				$4
-			) ON CONFLICT DO NOTHING`
+			) ON CONFLICT (project_group_id, workflow_id) DO UPDATE SET role = $4`
 	if _, err := db.Exec(query, projectID, gp.Group.ID, workflowID, gp.Permission); err != nil {
 		if strings.Contains(err.Error(), `null value in column "project_group_id"`) {
 			return sdk.WrapError(sdk.ErrNotFound, "cannot add this group on workflow because there isn't in the project groups : %v", err)
@@ -123,9 +125,11 @@ func UpsertWorkflowGroup(db gorp.SqlExecutor, projectID, workflowID int64, gp sd
 
 // DeleteWorkflowGroup remove group permission on the given workflow
 func DeleteWorkflowGroup(db gorp.SqlExecutor, w *sdk.Workflow, groupID int64, index int) error {
-	query := `DELETE FROM workflow_perm
+	query := `
+    DELETE FROM workflow_perm
 		USING project_group
-	WHERE workflow_perm.project_group_id = project_group.id AND workflow_perm.workflow_id = $1 AND project_group.group_id = $2`
+    WHERE workflow_perm.project_group_id = project_group.id AND workflow_perm.workflow_id = $1 AND project_group.group_id = $2
+  `
 	if _, err := db.Exec(query, w.ID, groupID); err != nil {
 		return sdk.WithStack(err)
 	}
@@ -141,6 +145,18 @@ func DeleteWorkflowGroup(db gorp.SqlExecutor, w *sdk.Workflow, groupID int64, in
 	return nil
 }
 
+// DeleteAllWorkflowGroups removes all group permission for the given workflow.
+func DeleteAllWorkflowGroups(db gorp.SqlExecutor, workflowID int64) error {
+	query := `
+    DELETE FROM workflow_perm
+    WHERE workflow_id = $1
+  `
+	if _, err := db.Exec(query, workflowID); err != nil {
+		return sdk.WrapError(err, "unable to remove group permissions for workflow %d", workflowID)
+	}
+	return nil
+}
+
 func checkAtLeastOneGroupWithWriteRoleOnWorkflow(db gorp.SqlExecutor, wID int64) (bool, error) {
 	query := `select count(project_group_id) from workflow_perm where workflow_id = $1 and role = $2`
 	nb, err := db.SelectInt(query, wID, 7)

engine/api/project_group.go

@@ -136,7 +136,7 @@ func (api *API) putGroupRoleOnProjectHandler() service.Handler {
 		}
 
 		if err := tx.Commit(); err != nil {
-			return sdk.WrapError(err, "updateGroupRoleHandler: Cannot start transaction")
+			return sdk.WrapError(err, "cannot start transaction")
 		}
 
 		newGroupPermission := sdk.GroupPermission{Permission: newLink.Role, Group: *grp}

engine/api/workflow/workflow_importer.go

@@ -92,15 +92,18 @@ func Import(ctx context.Context, db gorp.SqlExecutor, store cache.Store, proj sd
 
 func importWorkflowGroups(db gorp.SqlExecutor, w *sdk.Workflow) error {
 	if len(w.Groups) > 0 {
+		if err := group.DeleteAllWorkflowGroups(db, w.ID); err != nil {
+			return err
+		}
 		for i := range w.Groups {
 			g, err := group.LoadByName(context.Background(), db, w.Groups[i].Group.Name)
 			if err != nil {
-				return sdk.WrapError(err, "Unable to load group %s", w.Groups[i].Group.Name)
+				return sdk.WrapError(err, "unable to load group %s", w.Groups[i].Group.Name)
 			}
 			w.Groups[i].Group = *g
 		}
 		if err := group.UpsertAllWorkflowGroups(db, w, w.Groups); err != nil {
-			return sdk.WrapError(err, "Unable to update workflow")
+			return sdk.WrapError(err, "unable to update workflow")
 		}
 	}
 	return nil

engine/api/workflow_group.go

@@ -44,14 +44,13 @@ func (api *API) deleteWorkflowGroupHandler() service.Handler {
 				break
 			}
 		}
-
 		if oldGp.Permission == 0 {
 			return sdk.WithStack(sdk.ErrNotFound)
 		}
 
-		tx, errT := api.mustDB().Begin()
-		if errT != nil {
-			return sdk.WrapError(errT, "cannot start transaction")
+		tx, err := api.mustDB().Begin()
+		if err != nil {
+			return sdk.WrapError(err, "cannot start transaction")
 		}
 		defer tx.Rollback() // nolint
 

engine/api/workflow_import_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"io/ioutil"
 	"net/http/httptest"
+	"sort"
 	"strings"
 	"testing"
 
@@ -914,3 +915,136 @@ metadata:
 	t.Logf("%+v", wUpdated.WorkflowData)
 	assert.Equal(t, 1, len(wUpdated.WorkflowData.Joins))
 }
+
+func Test_postWorkflowImportHandler_editPermissions(t *testing.T) {
+	api, db, _ := newTestAPI(t)
+
+	u, pass := assets.InsertAdminUser(t, db)
+	g1 := assets.InsertTestGroup(t, db, "b-"+sdk.RandomString(10))
+	g2 := assets.InsertTestGroup(t, db, "c-"+sdk.RandomString(10))
+
+	proj := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), "a-"+sdk.RandomString(10))
+	require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{
+		GroupID:   g1.ID,
+		ProjectID: proj.ID,
+		Role:      sdk.PermissionReadExecute,
+	}))
+	require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{
+		GroupID:   g2.ID,
+		ProjectID: proj.ID,
+		Role:      sdk.PermissionReadExecute,
+	}))
+
+	pip := sdk.Pipeline{
+		ProjectID:  proj.ID,
+		ProjectKey: proj.Key,
+		Name:       "pip1",
+	}
+	require.NoError(t, pipeline.InsertPipeline(db, &pip))
+
+	uri := api.Router.GetRoute("POST", api.postWorkflowImportHandler, map[string]string{
+		"permProjectKey": proj.Key,
+	})
+	req := assets.NewAuthentifiedRequest(t, u, pass, "POST", uri, nil)
+
+	body := `name: test_1
+version: v2.0
+workflow:
+  pip1:
+    pipeline: pip1`
+	req.Body = ioutil.NopCloser(strings.NewReader(body))
+	req.Header.Set("Content-Type", "application/x-yaml")
+
+	rec := httptest.NewRecorder()
+	api.Router.Mux.ServeHTTP(rec, req)
+	require.Equal(t, 200, rec.Code)
+	t.Logf(">>%s", rec.Body.String())
+
+	w, err := workflow.Load(context.TODO(), db, api.Cache, *proj, "test_1", workflow.LoadOptions{})
+	require.NoError(t, err)
+
+	// Workflow permissions should be inherited from project
+	require.Len(t, w.Groups, 3)
+	sort.Slice(w.Groups, func(i, j int) bool {
+		return w.Groups[i].Group.Name < w.Groups[j].Group.Name
+	})
+	assert.Equal(t, proj.ProjectGroups[0].Group.Name, w.Groups[0].Group.Name)
+	assert.Equal(t, sdk.PermissionReadWriteExecute, w.Groups[0].Permission)
+	assert.Equal(t, g1.Name, w.Groups[1].Group.Name)
+	assert.Equal(t, sdk.PermissionReadExecute, w.Groups[1].Permission)
+	assert.Equal(t, g2.Name, w.Groups[2].Group.Name)
+	assert.Equal(t, sdk.PermissionReadExecute, w.Groups[2].Permission)
+
+	// We want to change to permisison for g2 and remove the permission for g1
+	uri = api.Router.GetRoute("POST", api.postWorkflowImportHandler, map[string]string{
+		"permProjectKey": proj.Key,
+	})
+	req = assets.NewAuthentifiedRequest(t, u, pass, "POST", uri, nil)
+	q := req.URL.Query()
+	q.Set("force", "true")
+	req.URL.RawQuery = q.Encode()
+
+	body = `name: test_1
+version: v2.0
+workflow:
+  pip1:
+    pipeline: pip1
+permissions:
+  ` + proj.ProjectGroups[0].Group.Name + `: 7
+  ` + g2.Name + `: 4`
+	req.Body = ioutil.NopCloser(strings.NewReader(body))
+	req.Header.Set("Content-Type", "application/x-yaml")
+
+	rec = httptest.NewRecorder()
+	api.Router.Mux.ServeHTTP(rec, req)
+	require.Equal(t, 200, rec.Code)
+	t.Logf(">>%s", rec.Body.String())
+
+	w, err = workflow.Load(context.TODO(), db, api.Cache, *proj, "test_1", workflow.LoadOptions{})
+	require.NoError(t, err)
+
+	require.Len(t, w.Groups, 2)
+	sort.Slice(w.Groups, func(i, j int) bool {
+		return w.Groups[i].Group.Name < w.Groups[j].Group.Name
+	})
+	assert.Equal(t, proj.ProjectGroups[0].Group.Name, w.Groups[0].Group.Name)
+	assert.Equal(t, sdk.PermissionReadWriteExecute, w.Groups[0].Permission)
+	assert.Equal(t, g2.Name, w.Groups[1].Group.Name)
+	assert.Equal(t, sdk.PermissionRead, w.Groups[1].Permission)
+
+	// Import again the workflow without permissions should reset to project permissions
+	uri = api.Router.GetRoute("POST", api.postWorkflowImportHandler, map[string]string{
+		"permProjectKey": proj.Key,
+	})
+	req = assets.NewAuthentifiedRequest(t, u, pass, "POST", uri, nil)
+	q = req.URL.Query()
+	q.Set("force", "true")
+	req.URL.RawQuery = q.Encode()
+
+	body = `name: test_1
+version: v2.0
+workflow:
+  pip1:
+    pipeline: pip1`
+	req.Body = ioutil.NopCloser(strings.NewReader(body))
+	req.Header.Set("Content-Type", "application/x-yaml")
+
+	rec = httptest.NewRecorder()
+	api.Router.Mux.ServeHTTP(rec, req)
+	require.Equal(t, 200, rec.Code)
+	t.Logf(">>%s", rec.Body.String())
+
+	w, err = workflow.Load(context.TODO(), db, api.Cache, *proj, "test_1", workflow.LoadOptions{})
+	require.NoError(t, err)
+
+	require.Len(t, w.Groups, 3)
+	sort.Slice(w.Groups, func(i, j int) bool {
+		return w.Groups[i].Group.Name < w.Groups[j].Group.Name
+	})
+	assert.Equal(t, proj.ProjectGroups[0].Group.Name, w.Groups[0].Group.Name)
+	assert.Equal(t, sdk.PermissionReadWriteExecute, w.Groups[0].Permission)
+	assert.Equal(t, g1.Name, w.Groups[1].Group.Name)
+	assert.Equal(t, sdk.PermissionReadExecute, w.Groups[1].Permission)
+	assert.Equal(t, g2.Name, w.Groups[2].Group.Name)
+	assert.Equal(t, sdk.PermissionReadExecute, w.Groups[2].Permission)
+}

sdk/cdsclient/client.go

@@ -1,10 +1,11 @@
+// For more information about how to user cdsclient package have a look at https://ovh.github.io/cds/development/sdk/golang/.
+
 package cdsclient
 
 import (
 	"context"
 	"crypto/tls"
 	"encoding/base64"
-	"io"
 	"net"
 	"net/http"
 	"os"
@@ -97,16 +98,6 @@ func NewWorker(endpoint string, name string, c *http.Client) WorkerInterface {
 	return cli
 }
 
-// NewClientFromConfig returns a client from the config file
-func NewClientFromConfig(r io.Reader) (Interface, error) {
-	return nil, nil
-}
-
-// NewClientFromEnv returns a client from the environment variables
-func NewClientFromEnv() (Interface, error) {
-	return nil, nil
-}
-
 // NewProviderClient returns an implementation for ProviderClient interface
 func NewProviderClient(cfg ProviderConfig) ProviderClient {
 	conf := Config{