diff --git a/engine/api/workflow_group.go b/engine/api/workflow_group.go index e1ca349453..052fe1d6db 100644 --- a/engine/api/workflow_group.go +++ b/engine/api/workflow_group.go @@ -106,12 +106,24 @@ func (api *API) putWorkflowGroupHandler() service.Handler { return sdk.WrapError(sdk.ErrNotFound, "no permission found for group %q on workflow", gp.Group.Name) } - g, err := group.LoadByName(ctx, api.mustDB(), gp.Group.Name, group.LoadOptions.WithOrganization) + g, err := group.LoadByName(ctx, api.mustDB(), gp.Group.Name, group.LoadOptions.WithOrganization, group.LoadOptions.WithMembers) if err != nil { return sdk.WrapError(err, "cannot load group with name %q", gp.Group.Name) } gp.Group = *g + if !isGroupAdmin(ctx, g) && gp.Permission > oldGp.Permission { + if isAdmin(ctx) { + trackSudo(ctx, w) + } else { + return sdk.WithStack(sdk.ErrInvalidGroupAdmin) + } + } + + if group.IsDefaultGroupID(g.ID) && gp.Permission > sdk.PermissionRead { + return sdk.NewErrorFrom(sdk.ErrDefaultGroupPermission, "only read permission is allowed to default group") + } + tx, err := api.mustDB().Begin() if err != nil { return sdk.WrapError(err, "cannot start transaction") @@ -164,12 +176,24 @@ func (api *API) postWorkflowGroupHandler() service.Handler { } } - g, err := group.LoadByName(ctx, api.mustDB(), gp.Group.Name, group.LoadOptions.WithOrganization) + g, err := group.LoadByName(ctx, api.mustDB(), gp.Group.Name, group.LoadOptions.WithOrganization, group.LoadOptions.WithMembers) if err != nil { return sdk.WrapError(err, "cannot load group with name %q", gp.Group.Name) } gp.Group = *g + if !isGroupAdmin(ctx, g) && gp.Permission > sdk.PermissionRead { + if isAdmin(ctx) { + trackSudo(ctx, w) + } else { + return sdk.WithStack(sdk.ErrInvalidGroupAdmin) + } + } + + if group.IsDefaultGroupID(g.ID) && gp.Permission > sdk.PermissionRead { + return sdk.NewErrorFrom(sdk.ErrDefaultGroupPermission, "only read permission is allowed to default group") + } + tx, err := api.mustDB().Begin() if err != nil { return sdk.WrapError(err, "cannot start transaction") diff --git a/engine/api/workflow_group_test.go b/engine/api/workflow_group_test.go index 704a3838ee..5190c1eb3c 100644 --- a/engine/api/workflow_group_test.go +++ b/engine/api/workflow_group_test.go @@ -7,7 +7,6 @@ import ( "net/http/httptest" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/ovh/cds/engine/api/group" @@ -19,328 +18,545 @@ import ( "github.com/ovh/cds/sdk" ) -func Test_postWorkflowGroupHandler(t *testing.T) { +func Test_postWorkflowGroupHandler_RequireGroupOnProject(t *testing.T) { api, db, router := newTestAPI(t) - u, pass := assets.InsertAdminUser(t, db) - key := sdk.RandomString(10) - proj := assets.InsertTestProject(t, db, api.Cache, key, key) + proj := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), sdk.RandomString(10)) - //First pipeline - pip := sdk.Pipeline{ - ProjectID: proj.ID, - ProjectKey: proj.Key, - Name: "pip1", - } - test.NoError(t, pipeline.InsertPipeline(api.mustDB(), &pip)) + g1 := proj.ProjectGroups[0].Group + g2 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) + g3 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) - w := sdk.Workflow{ - Name: sdk.RandomString(10), - WorkflowData: sdk.WorkflowData{ - Node: sdk.Node{ - Name: "root", - Type: sdk.NodeTypePipeline, - Context: &sdk.NodeContext{ - PipelineID: pip.ID, - }, - }, - }, - ProjectID: proj.ID, - ProjectKey: proj.Key, - } + u, jwt := assets.InsertAdminUser(t, db) - proj2, errP := project.Load(context.TODO(), api.mustDB(), proj.Key, - project.LoadOptions.WithPipelines, - project.LoadOptions.WithGroups, - ) - test.NoError(t, errP) + // Insert workflow that will inherit from project permission + w := assets.InsertTestWorkflow(t, db, api.Cache, proj, sdk.RandomString(10)) - require.NoError(t, workflow.Insert(context.TODO(), db, api.Cache, *proj2, &w)) + // Set g2 on project + require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ + GroupID: g2.ID, + ProjectID: proj.ID, + Role: sdk.PermissionReadWriteExecute, + })) - t.Logf("%+v\n", proj) + // User can add permission for g2 on workflow because it is on project + uri := router.GetRoute(http.MethodPost, api.postWorkflowGroupHandler, map[string]string{ + "key": proj.Key, + "permWorkflowName": w.Name, + }) + require.NotEmpty(t, uri) + req := assets.NewAuthentifiedRequest(t, u, jwt, http.MethodPost, uri, sdk.GroupPermission{ + Permission: sdk.PermissionReadWriteExecute, + Group: *g2, + }) + rec := httptest.NewRecorder() + router.Mux.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + var workflowResult sdk.Workflow + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &workflowResult)) + require.Len(t, workflowResult.Groups, 2) + require.NotNil(t, workflowResult.Groups.GetByGroupID(g1.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, workflowResult.Groups.GetByGroupID(g1.ID).Permission) + require.NotNil(t, workflowResult.Groups.GetByGroupID(g2.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, workflowResult.Groups.GetByGroupID(g2.ID).Permission) + + // User cannot add permission for g3 on workflow because it is not on project + uri = router.GetRoute(http.MethodPost, api.postWorkflowGroupHandler, map[string]string{ + "key": proj.Key, + "permWorkflowName": w.Name, + }) + require.NotEmpty(t, uri) + req = assets.NewAuthentifiedRequest(t, u, jwt, http.MethodPost, uri, sdk.GroupPermission{ + Permission: sdk.PermissionReadWriteExecute, + Group: *g3, + }) + rec = httptest.NewRecorder() + router.Mux.ServeHTTP(rec, req) + require.Equal(t, http.StatusBadRequest, rec.Code) + var sdkError sdk.Error + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &sdkError)) + require.Equal(t, "Cannot add this permission group on your workflow because this group is not already in the project's permissions", sdkError.Message) +} + +func Test_postWorkflowGroupHandler_ShouldBeRWXIfGroupIsRWXOnProject(t *testing.T) { + api, db, router := newTestAPI(t) + + proj := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), sdk.RandomString(10)) + + g1 := proj.ProjectGroups[0].Group + g2 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) + + u, jwt := assets.InsertAdminUser(t, db) - newGrp := assets.InsertTestGroup(t, db, sdk.RandomString(10)) + // Insert workflow that will inherit from project permission + w := assets.InsertTestWorkflow(t, db, api.Cache, proj, sdk.RandomString(10)) + + // Set g2 on project require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ - GroupID: newGrp.ID, + GroupID: g2.ID, ProjectID: proj.ID, - Role: sdk.PermissionRead, + Role: sdk.PermissionReadWriteExecute, })) - //Prepare request - vars := map[string]string{ + + // User cannot add permission