diff --git a/engine/api/api_helper.go b/engine/api/api_helper.go index 232f4199ba..dca1a18b19 100644 --- a/engine/api/api_helper.go +++ b/engine/api/api_helper.go @@ -21,10 +21,7 @@ func isGroupAdmin(ctx context.Context, g *sdk.Group) bool { if c == nil { return false } - member := g.IsMember(c.GetGroupIDs()) - admin := g.IsAdmin(*c.AuthentifiedUser) - log.Debug(ctx, "api.isGroupAdmin> member:%t admin:%t", member, admin) - return member && admin && c.Worker == nil + return group.IsConsumerGroupAdmin(g, c) } func isGroupMember(ctx context.Context, g *sdk.Group) bool { diff --git a/engine/api/event/publish_project.go b/engine/api/event/publish_project.go index a0a9c1c61a..8fdace9941 100644 --- a/engine/api/event/publish_project.go +++ b/engine/api/event/publish_project.go @@ -110,11 +110,11 @@ func PublishUpdateProjectPermission(ctx context.Context, p *sdk.Project, gp sdk. } // PublishDeleteProjectPermission publishes an event on deleting a group permission on the project -func PublishDeleteProjectPermission(ctx context.Context, p *sdk.Project, gp sdk.GroupPermission) { +func PublishDeleteProjectPermission(ctx context.Context, p *sdk.Project, gp sdk.GroupPermission, u sdk.Identifiable) { e := sdk.EventProjectPermissionDelete{ Permission: gp, } - PublishProjectEvent(ctx, e, p.Key, nil) + PublishProjectEvent(ctx, e, p.Key, u) } // PublishAddProjectKey publishes an event on adding a project key diff --git a/engine/api/group.go b/engine/api/group.go index 7b706e19a8..86899be9a6 100644 --- a/engine/api/group.go +++ b/engine/api/group.go @@ -213,7 +213,7 @@ func (api *API) deleteGroupHandler() service.Handler { // Send project permission changes for _, pg := range projPerms { - event.PublishDeleteProjectPermission(ctx, &pg.Project, sdk.GroupPermission{Group: *g}) + event.PublishDeleteProjectPermission(ctx, &pg.Project, sdk.GroupPermission{Group: *g}, getAPIConsumer(ctx)) } return service.WriteJSON(w, nil, http.StatusOK) diff --git a/engine/api/group/group.go b/engine/api/group/group.go index e626a8c8d2..23e9a98314 100644 --- a/engine/api/group/group.go +++ b/engine/api/group/group.go @@ -29,3 +29,11 @@ func CheckUserInDefaultGroup(ctx context.Context, db gorpmapper.SqlExecutorWithT return nil } + +// For given consumer check that it is group admin, member should be loaded +// on group and worker should be loaded on consumer if exists +func IsConsumerGroupAdmin(g *sdk.Group, c *sdk.AuthConsumer) bool { + member := g.IsMember(c.GetGroupIDs()) + admin := g.IsAdmin(*c.AuthentifiedUser) + return member && admin && c.Worker == nil +} diff --git a/engine/api/group/group_permission.go b/engine/api/group/group_permission.go new file mode 100644 index 0000000000..729ef1e20e --- /dev/null +++ b/engine/api/group/group_permission.go @@ -0,0 +1,69 @@ +package group + +import ( + "context" + + "github.com/go-gorp/gorp" + + "github.com/ovh/cds/sdk" +) + +func CheckGroupPermission(ctx context.Context, db gorp.SqlExecutor, projectGroups sdk.GroupPermissions, gp *sdk.GroupPermission, consumer *sdk.AuthConsumer) error { + if gp.Group.ID == 0 { + g, err := LoadByName(ctx, db, gp.Group.Name) + if err != nil { + return err + } + gp.Group = *g + } + if err := LoadOptions.WithMembers(ctx, db, &gp.Group); err != nil { + return err + } + + if IsDefaultGroupID(gp.Group.ID) && gp.Permission > sdk.PermissionRead { + return sdk.NewErrorFrom(sdk.ErrDefaultGroupPermission, "only read permission is allowed to default group") + } + + if err := CheckGroupPermissionOrganizationMatch(ctx, db, projectGroups, &gp.Group, gp.Permission); err != nil { + return err + } + + if !IsConsumerGroupAdmin(&gp.Group, consumer) && gp.Permission > sdk.PermissionRead { + return sdk.WithStack(sdk.ErrInvalidGroupAdmin) + } + + return nil +} + +func CheckProjectOrganizationMatch(ctx context.Context, db gorp.SqlExecutor, proj *sdk.Project, grp *sdk.Group, role int) error { + if err := LoadGroupsIntoProject(ctx, db, proj); err != nil { + return err + } + return CheckGroupPermissionOrganizationMatch(ctx, db, proj.ProjectGroups, grp, role) +} + +func CheckGroupPermissionOrganizationMatch(ctx context.Context, db gorp.SqlExecutor, projectGroups sdk.GroupPermissions, grp *sdk.Group, role int) error { + if role == sdk.PermissionRead { + return nil + } + + projectOrganization, err := projectGroups.ComputeOrganization() + if err != nil { + return sdk.NewError(sdk.ErrForbidden, err) + } + if projectOrganization == "" { + return nil + } + + if err := LoadOptions.WithOrganization(ctx, db, grp); err != nil { + return err + } + if grp.Organization != projectOrganization { + if grp.Organization == "" { + return sdk.NewErrorFrom(sdk.ErrForbidden, "given group without organization don't match project organization %q", projectOrganization) + } + return sdk.NewErrorFrom(sdk.ErrForbidden, "given group with organization %q don't match project organization %q", grp.Organization, projectOrganization) + } + + return nil +} diff --git a/engine/api/group/workflow_group.go b/engine/api/group/workflow_group.go index c98158e770..288e84a5a0 100644 --- a/engine/api/group/workflow_group.go +++ b/engine/api/group/workflow_group.go @@ -9,6 +9,7 @@ import ( "github.com/lib/pq" "github.com/ovh/cds/engine/api/database/gorpmapping" + "github.com/ovh/cds/engine/gorpmapper" "github.com/ovh/cds/sdk" ) @@ -27,7 +28,7 @@ func LoadRoleGroupInWorkflow(db gorp.SqlExecutor, workflowID, groupID int64) (in } // AddWorkflowGroup Add permission on the given workflow for the given group -func AddWorkflowGroup(ctx context.Context, db gorp.SqlExecutor, w *sdk.Workflow, gp sdk.GroupPermission) error { +func AddWorkflowGroup(ctx context.Context, db gorpmapper.SqlExecutorWithTx, w *sdk.Workflow, gp sdk.GroupPermission) error { link, err := LoadLinkGroupProjectForGroupIDAndProjectID(ctx, db, gp.Group.ID, w.ProjectID) if err != nil { if sdk.ErrorIs(err, sdk.ErrNotFound) { @@ -48,24 +49,20 @@ func AddWorkflowGroup(ctx context.Context, db gorp.SqlExecutor, w *sdk.Workflow, } // UpdateWorkflowGroup update group permission for the given group on the current workflow -func UpdateWorkflowGroup(ctx context.Context, db gorp.SqlExecutor, w *sdk.Workflow, gp sdk.GroupPermission) error { +func UpdateWorkflowGroup(ctx context.Context, db gorpmapper.SqlExecutorWithTx, w *sdk.Workflow, gp sdk.GroupPermission) error { link, err := LoadLinkGroupProjectForGroupIDAndProjectID(ctx, db, gp.Group.ID, w.ProjectID) if err != nil { + if sdk.ErrorIs(err, sdk.ErrNotFound) { + return sdk.WithStack(sdk.ErrGroupNotFoundInProject) + } return sdk.WrapError(err, "cannot load role for group %d in project %d", gp.Group.ID, w.ProjectID) } if link.Role == sdk.PermissionReadWriteExecute && gp.Permission < link.Role { return sdk.WithStack(sdk.ErrWorkflowPermInsufficient) } - query := ` - UPDATE workflow_perm - SET role = $1 - FROM project_group - WHERE project_group.id = workflow_perm.project_group_id - AND workflow_perm.workflow_id = $2 - AND project_group.group_id = $3 - ` - if _, err := db.Exec(query, gp.Permission, w.ID, gp.Group.ID); err != nil { + query := "UPDATE workflow_perm SET role = $3 WHERE project_group_id = $1 AND workflow_id = $2" + if _, err := db.Exec(query, link.ID, w.ID, gp.Permission); err != nil { return sdk.WithStack(err) } @@ -76,32 +73,40 @@ func UpdateWorkflowGroup(ctx context.Context, db gorp.SqlExecutor, w *sdk.Workfl } } - ok, err := checkAtLeastOneGroupWithWriteRoleOnWorkflow(db, w.ID) - if err != nil { + if err := checkAtLeastOneRWXRoleOnWorkflow(db, w.ID); err != nil { return err } - if !ok { - return sdk.WithStack(sdk.ErrLastGroupWithWriteRole) - } return nil } // UpsertAllWorkflowGroups upsert all groups in a workflow -func UpsertAllWorkflowGroups(db gorp.SqlExecutor, w *sdk.Workflow, gps []sdk.GroupPermission) error { +func UpsertAllWorkflowGroups(ctx context.Context, db gorpmapper.SqlExecutorWithTx, w *sdk.Workflow, gps []sdk.GroupPermission) error { + query := "DELETE FROM workflow_perm WHERE workflow_id = $1" + if _, err := db.Exec(query, w.ID); err != nil { + return sdk.WrapError(err, "unable to remove group permissions for workflow %d", w.ID) + } + for _, gp := range gps { + link, err := LoadLinkGroupProjectForGroupIDAndProjectID(ctx, db, gp.Group.ID, w.ProjectID) + if err != nil { + if sdk.ErrorIs(err, sdk.ErrNotFound) { + return sdk.WithStack(sdk.ErrGroupNotFoundInProject) + } + return sdk.WrapError(err, "cannot load role for group %d in project %d", gp.Group.ID, w.ProjectID) + } + if link.Role == sdk.PermissionReadWriteExecute && gp.Permission < link.Role { + return sdk.WithStack(sdk.ErrWorkflowPermInsufficient) + } + if err := UpsertWorkflowGroup(db, w.ProjectID, w.ID, gp); err != nil { return err } } - ok, err := checkAtLeastOneGroupWithWriteRoleOnWorkflow(db, w.ID) - if err != nil { + if err := checkAtLeastOneRWXRoleOnWorkflow(db, w.ID); err != nil { return err } - if !ok { - return sdk.WithStack(sdk.ErrLastGroupWithWriteRole) - } return nil } @@ -134,36 +139,25 @@ func DeleteWorkflowGroup(db gorp.SqlExecutor, w *sdk.Workflow, groupID int64, in return sdk.WithStack(err) } - ok, err := checkAtLeastOneGroupWithWriteRoleOnWorkflow(db, w.ID) - if err != nil { + if err := checkAtLeastOneRWXRoleOnWorkflow(db, w.ID); err != nil { return err } - if !ok { - return sdk.WithStack(sdk.ErrLastGroupWithWriteRole) - } + w.Groups = append(w.Groups[:index], w.Groups[index+1:]...) - 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) { +func checkAtLeastOneRWXRoleOnWorkflow(db gorp.SqlExecutor, wID int64) error { query := `select count(project_group_id) from workflow_perm where workflow_id = $1 and role = $2` - nb, err := db.SelectInt(query, wID, 7) + nb, err := db.SelectInt(query, wID, sdk.PermissionReadWriteExecute) if err != nil { - return false, sdk.WithStack(err) + return sdk.WithStack(err) + } + if nb == 0 { + return sdk.WithStack(sdk.ErrLastGroupWithWriteRole) } - return nb > 0, err + return nil } type LinkWorkflowGroupPermission struct { @@ -251,3 +245,22 @@ func LoadWorkflowGroups(db gorp.SqlExecutor, workflowID int64) ([]sdk.GroupPermi } return wgs, nil } + +func CheckWorkflowGroups(ctx context.Context, db gorp.SqlExecutor, proj *sdk.Project, w *sdk.Workflow, consumer *sdk.AuthConsumer) error { + if err := LoadGroupsIntoProject(ctx, db, proj); err != nil { + return err + } + for i := range w.Groups { + if err := CheckGroupPermission(ctx, db, proj.ProjectGroups, &w.Groups[i], consumer); err != nil { + return err + } + } + for _, n := range w.WorkflowData.Array() { + for i := range n.Groups { + if err := CheckGroupPermission(ctx, db, proj.ProjectGroups, &n.Groups[i], consumer); err != nil { + return err + } + } + } + return nil +} diff --git a/engine/api/group/workflow_group_test.go b/engine/api/group/workflow_group_test.go new file mode 100644 index 0000000000..5a43fc9f8b --- /dev/null +++ b/engine/api/group/workflow_group_test.go @@ -0,0 +1,231 @@ +package group_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/ovh/cds/engine/api/authentication" + "github.com/ovh/cds/engine/api/bootstrap" + "github.com/ovh/cds/engine/api/group" + "github.com/ovh/cds/engine/api/test" + "github.com/ovh/cds/engine/api/test/assets" + "github.com/ovh/cds/sdk" +) + +func TestCheckWorkflowGroups_UserShouldBeGroupAdminForRWAndRWX(t *testing.T) { + db, cache := test.SetupPG(t, bootstrap.InitiliazeDB) + + proj := assets.InsertTestProject(t, db, cache, sdk.RandomString(10), sdk.RandomString(10)) + + g1 := proj.ProjectGroups[0].Group + g2 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) + g3 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) + + u, _ := assets.InsertLambdaUser(t, db, &g1, g2) + assets.SetUserGroupAdmin(t, db, g1.ID, u.ID) + assets.SetUserGroupAdmin(t, db, g2.ID, u.ID) + + localConsumer, err := authentication.LoadConsumerByTypeAndUserID(context.TODO(), db, sdk.ConsumerLocal, u.ID, authentication.LoadConsumerOptions.WithAuthentifiedUser) + require.NoError(t, err) + + // Insert workflow that will inherit from project permission + w := assets.InsertTestWorkflow(t, db, cache, proj, sdk.RandomString(10)) + + // Set g2 and g3 on project + require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ + GroupID: g2.ID, + ProjectID: proj.ID, + Role: sdk.PermissionReadWriteExecute, + })) + require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ + GroupID: g3.ID, + ProjectID: proj.ID, + Role: sdk.PermissionRead, + })) + + // User cannot add RX permission for g3 on workflow because not admin of g3 + w.Groups = append(w.Groups, sdk.GroupPermission{ + Permission: sdk.PermissionReadExecute, + Group: *g3, + }) + err = group.CheckWorkflowGroups(context.TODO(), db, proj, w, localConsumer) + require.Error(t, err) + require.Equal(t, "User is not a group's admin", err.Error()) + + // User can add RX permission for g2 on workflow because admin of g2 + w.Groups = w.Groups[0 : len(w.Groups)-2] + w.Groups = append(w.Groups, sdk.GroupPermission{ + Permission: sdk.PermissionReadWriteExecute, + Group: *g2, + }) + require.NoError(t, group.CheckWorkflowGroups(context.TODO(), db, proj, w, localConsumer)) + + // User can add R permission for g3 on workflow + w.Groups = append(w.Groups, sdk.GroupPermission{ + Permission: sdk.PermissionRead, + Group: *g3, + }) + require.NoError(t, group.CheckWorkflowGroups(context.TODO(), db, proj, w, localConsumer)) +} + +func TestCheckWorkflowGroups_OnlyReadForDifferentOrganization(t *testing.T) { + db, cache := test.SetupPG(t, bootstrap.InitiliazeDB) + + proj := assets.InsertTestProject(t, db, cache, sdk.RandomString(10), sdk.RandomString(10)) + + g1 := &proj.ProjectGroups[0].Group + g2 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) + + // Set organization for groups + require.NoError(t, group.InsertOrganization(context.TODO(), db, &group.Organization{ + GroupID: g1.ID, + Organization: "one", + })) + require.NoError(t, group.InsertOrganization(context.TODO(), db, &group.Organization{ + GroupID: g2.ID, + Organization: "two", + })) + + u, _ := assets.InsertAdminUser(t, db) + + localConsumer, err := authentication.LoadConsumerByTypeAndUserID(context.TODO(), db, sdk.ConsumerLocal, u.ID, authentication.LoadConsumerOptions.WithAuthentifiedUser) + require.NoError(t, err) + + // Insert workflow that will inherit from project permission + w := assets.InsertTestWorkflow(t, db, cache, proj, sdk.RandomString(10)) + + // Set g2 on project + require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ + GroupID: g2.ID, + ProjectID: proj.ID, + Role: sdk.PermissionRead, + })) + + // Cannot add RX permission for g2 on workflow because organization is not the same as project's one + w.Groups = sdk.GroupPermissions{{ + Permission: sdk.PermissionReadExecute, + Group: *g2, + }} + err = group.CheckWorkflowGroups(context.TODO(), db, proj, w, localConsumer) + require.Error(t, err) + require.Equal(t, "forbidden (from: given group with organization \"two\" don't match project organization \"one\")", err.Error(), err) + + // Can add R permission for g2 on workflow + w.Groups = sdk.GroupPermissions{{ + Permission: sdk.PermissionRead, + Group: *g2, + }} + require.NoError(t, group.CheckWorkflowGroups(context.TODO(), db, proj, w, localConsumer)) +} + +func TestCheckWorkflowGroups_UserShouldBeGroupAdminForRWAndRWX_Node(t *testing.T) { + db, cache := test.SetupPG(t, bootstrap.InitiliazeDB) + + proj := assets.InsertTestProject(t, db, cache, sdk.RandomString(10), sdk.RandomString(10)) + + g1 := proj.ProjectGroups[0].Group + g2 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) + g3 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) + + u, _ := assets.InsertLambdaUser(t, db, &g1, g2) + assets.SetUserGroupAdmin(t, db, g1.ID, u.ID) + assets.SetUserGroupAdmin(t, db, g2.ID, u.ID) + + localConsumer, err := authentication.LoadConsumerByTypeAndUserID(context.TODO(), db, sdk.ConsumerLocal, u.ID, authentication.LoadConsumerOptions.WithAuthentifiedUser) + require.NoError(t, err) + + // Insert workflow that will inherit from project permission + w := assets.InsertTestWorkflow(t, db, cache, proj, sdk.RandomString(10)) + + // Set g2 and g3 on project + require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ + GroupID: g2.ID, + ProjectID: proj.ID, + Role: sdk.PermissionReadWriteExecute, + })) + require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ + GroupID: g3.ID, + ProjectID: proj.ID, + Role: sdk.PermissionRead, + })) + + // User cannot add RX permission for g3 on workflow node because not admin of g3 + w.Groups = nil + w.WorkflowData.Node.Groups = sdk.GroupPermissions{{ + Permission: sdk.PermissionReadExecute, + Group: *g3, + }} + err = group.CheckWorkflowGroups(context.TODO(), db, proj, w, localConsumer) + require.Error(t, err) + require.Equal(t, "User is not a group's admin", err.Error()) + + // User can add RX permission for g2 on workflow node because admin of g2 + w.Groups = nil + w.WorkflowData.Node.Groups = sdk.GroupPermissions{{ + Permission: sdk.PermissionReadExecute, + Group: *g2, + }} + require.NoError(t, group.CheckWorkflowGroups(context.TODO(), db, proj, w, localConsumer)) + + // User can add R permission for g3 on workflow node + w.Groups = nil + w.WorkflowData.Node.Groups = sdk.GroupPermissions{{ + Permission: sdk.PermissionRead, + Group: *g3, + }} + require.NoError(t, group.CheckWorkflowGroups(context.TODO(), db, proj, w, localConsumer)) +} + +func TestCheckWorkflowGroups_OnlyReadForDifferentOrganization_Node(t *testing.T) { + db, cache := test.SetupPG(t, bootstrap.InitiliazeDB) + + proj := assets.InsertTestProject(t, db, cache, sdk.RandomString(10), sdk.RandomString(10)) + + g1 := &proj.ProjectGroups[0].Group + g2 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) + + // Set organization for groups + require.NoError(t, group.InsertOrganization(context.TODO(), db, &group.Organization{ + GroupID: g1.ID, + Organization: "one", + })) + require.NoError(t, group.InsertOrganization(context.TODO(), db, &group.Organization{ + GroupID: g2.ID, + Organization: "two", + })) + + u, _ := assets.InsertAdminUser(t, db) + + localConsumer, err := authentication.LoadConsumerByTypeAndUserID(context.TODO(), db, sdk.ConsumerLocal, u.ID, authentication.LoadConsumerOptions.WithAuthentifiedUser) + require.NoError(t, err) + + // Insert workflow that will inherit from project permission + w := assets.InsertTestWorkflow(t, db, cache, proj, sdk.RandomString(10)) + + // Set g2 on project + require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ + GroupID: g2.ID, + ProjectID: proj.ID, + Role: sdk.PermissionRead, + })) + + // Cannot add RX permission for g2 on workflow node because organization is not the same as project's one + w.Groups = nil + w.WorkflowData.Node.Groups = sdk.GroupPermissions{{ + Permission: sdk.PermissionReadExecute, + Group: *g2, + }} + err = group.CheckWorkflowGroups(context.TODO(), db, proj, w, localConsumer) + require.Error(t, err) + require.Equal(t, "forbidden (from: given group with organization \"two\" don't match project organization \"one\")", err.Error(), err) + + // Can add R permission for g2 on workflow node + w.Groups = nil + w.WorkflowData.Node.Groups = sdk.GroupPermissions{{ + Permission: sdk.PermissionRead, + Group: *g2, + }} + require.NoError(t, group.CheckWorkflowGroups(context.TODO(), db, proj, w, localConsumer)) +} diff --git a/engine/api/project_group.go b/engine/api/project_group.go index 340eda1e35..ed114bc3b5 100644 --- a/engine/api/project_group.go +++ b/engine/api/project_group.go @@ -5,7 +5,6 @@ import ( "database/sql" "net/http" - "github.com/go-gorp/gorp" "github.com/gorilla/mux" "github.com/ovh/cds/engine/api/event" @@ -54,7 +53,7 @@ func (api *API) deleteGroupFromProjectHandler() service.Handler { event.PublishDeleteProjectPermission(ctx, proj, sdk.GroupPermission{ Group: *grp, Permission: link.Role, - }) + }, getAPIConsumer(ctx)) return service.WriteJSON(w, nil, http.StatusOK) } @@ -95,7 +94,7 @@ func (api *API) putGroupRoleOnProjectHandler() service.Handler { return sdk.NewErrorFrom(sdk.ErrDefaultGroupPermission, "only read permission is allowed to default group") } - if err := projectPermissionCheckOrganizationMatch(ctx, tx, proj, grp, data.Permission); err != nil { + if err := group.CheckProjectOrganizationMatch(ctx, tx, proj, grp, data.Permission); err != nil { return err } @@ -118,7 +117,8 @@ func (api *API) putGroupRoleOnProjectHandler() service.Handler { return err } - if !onlyProject { + shouldUpdateWorkflow := newLink.Role == sdk.PermissionReadWriteExecute || !onlyProject + if shouldUpdateWorkflow { wfList, err := workflow.LoadAllNames(tx, proj.ID) if err != nil { return sdk.WrapError(err, "cannot load all workflow names for project id %d key %s", proj.ID, proj.Key) @@ -131,8 +131,7 @@ func (api *API) putGroupRoleOnProjectHandler() service.Handler { } return sdk.WrapError(err, "cannot load role for workflow %s with id %d and group id %d", wf.Name, wf.ID, grp.ID) } - - if oldLink.Role != role { // If project role and workflow role aren't sync do not update + if newLink.Role == role { continue } @@ -188,7 +187,7 @@ func (api *API) postGroupInProjectHandler() service.Handler { return sdk.WrapError(err, "cannot find %s", data.Group.Name) } - if !isGroupAdmin(ctx, grp) { + if !isGroupAdmin(ctx, grp) && data.Permission > sdk.PermissionRead { if isAdmin(ctx) { trackSudo(ctx, w) } else { @@ -208,7 +207,7 @@ func (api *API) postGroupInProjectHandler() service.Handler { return sdk.NewErrorFrom(sdk.ErrGroupExists, "group already in the project %s", proj.Name) } - if err := projectPermissionCheckOrganizationMatch(ctx, tx, proj, grp, data.Permission); err != nil { + if err := group.CheckProjectOrganizationMatch(ctx, tx, proj, grp, data.Permission); err != nil { return err } @@ -221,7 +220,8 @@ func (api *API) postGroupInProjectHandler() service.Handler { return err } - if !onlyProject { + shouldUpdateWorkflow := newLink.Role == sdk.PermissionReadWriteExecute || !onlyProject + if shouldUpdateWorkflow { wfList, err := workflow.LoadAllNames(tx, proj.ID) if err != nil { return sdk.WrapError(err, "cannot load all workflow names for project id %d key %s", proj.ID, proj.Key) @@ -229,7 +229,7 @@ func (api *API) postGroupInProjectHandler() service.Handler { for _, wf := range wfList { if err := group.UpsertWorkflowGroup(tx, proj.ID, wf.ID, sdk.GroupPermission{ Group: *grp, - Permission: data.Permission, + Permission: newLink.Role, }); err != nil { return sdk.WrapError(err, "cannot upsert group %d in workflow %s with id %d", grp.ID, wf.Name, wf.ID) } @@ -246,27 +246,3 @@ func (api *API) postGroupInProjectHandler() service.Handler { return service.WriteJSON(w, newGroupPermission, http.StatusOK) } } - -func projectPermissionCheckOrganizationMatch(ctx context.Context, db gorp.SqlExecutor, proj *sdk.Project, grp *sdk.Group, role int) error { - if role > sdk.PermissionRead { - if len(proj.ProjectGroups) == 0 { - if err := group.LoadGroupsIntoProject(ctx, db, proj); err != nil { - return err - } - } - projectOrganization, err := proj.ProjectGroups.ComputeOrganization() - if err != nil { - return sdk.NewError(sdk.ErrForbidden, err) - } - if projectOrganization == "" { - return nil - } - if grp.Organization != projectOrganization { - if grp.Organization == "" { - return sdk.NewErrorFrom(sdk.ErrForbidden, "given group without organization don't match project organization %q", projectOrganization) - } - return sdk.NewErrorFrom(sdk.ErrForbidden, "given group with organization %q don't match project organization %q", grp.Organization, projectOrganization) - } - } - return nil -} diff --git a/engine/api/project_group_test.go b/engine/api/project_group_test.go index 752c89c3da..d0678f4f9a 100644 --- a/engine/api/project_group_test.go +++ b/engine/api/project_group_test.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/json" "net/http" "net/http/httptest" @@ -9,113 +10,305 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/ovh/cds/engine/api/group" "github.com/ovh/cds/engine/api/pipeline" "github.com/ovh/cds/engine/api/test" "github.com/ovh/cds/engine/api/test/assets" "github.com/ovh/cds/sdk" ) -func Test_putGroupRoleOnProjectHandler(t *testing.T) { - api, db, _ := newTestAPI(t) +func Test_postGroupInProjectHandler_UserShouldBeGroupAdminForRWAndRWX(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)) + g3 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) - // Create a lambda user 1 that is admin of g1 and g2 - u1, jwtLambda1 := assets.InsertLambdaUser(t, db, &g1, g2) - assets.SetUserGroupAdmin(t, db, g1.ID, u1.ID) - assets.SetUserGroupAdmin(t, db, g2.ID, u1.ID) - // Create a lambda user 2 that is member of g1 - _, jwtLambda2 := assets.InsertLambdaUser(t, db, &g1) + // Create a lambda user that is admin on g1 and g2 + u, jwtLambda := assets.InsertLambdaUser(t, db, &g1, g2, g3) + assets.SetUserGroupAdmin(t, db, g1.ID, u.ID) + assets.SetUserGroupAdmin(t, db, g2.ID, u.ID) - // User 1 can add g2 on project because admin of it - uri := api.Router.GetRoute(http.MethodPost, api.postGroupInProjectHandler, map[string]string{ + // User cannot add RX permission for g3 on project because not admin of g3 + uri := router.GetRoute(http.MethodPost, api.postGroupInProjectHandler, map[string]string{ "permProjectKey": proj.Key, }) require.NotEmpty(t, uri) - req := assets.NewJWTAuthentifiedRequest(t, jwtLambda1, http.MethodPost, uri, sdk.GroupPermission{Group: *g2, Permission: sdk.PermissionReadExecute}) + req := assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPost, uri, sdk.GroupPermission{ + Permission: sdk.PermissionReadExecute, + Group: *g3, + }) rec := httptest.NewRecorder() - api.Router.Mux.ServeHTTP(rec, req) - require.Equal(t, http.StatusOK, rec.Code) + router.Mux.ServeHTTP(rec, req) + require.Equal(t, http.StatusForbidden, rec.Code) + var sdkError sdk.Error + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &sdkError)) + require.Equal(t, "User is not a group's admin", sdkError.Message) - // User 2 cannot add more permission on group g2 because not admin of it - uri = api.Router.GetRoute(http.MethodPut, api.putGroupRoleOnProjectHandler, map[string]string{ + // User can add RX permission for g2 on project because admin of g2 + uri = router.GetRoute(http.MethodPost, api.postGroupInProjectHandler, map[string]string{ "permProjectKey": proj.Key, - "groupName": g2.Name, }) require.NotEmpty(t, uri) - req = assets.NewJWTAuthentifiedRequest(t, jwtLambda2, http.MethodPut, uri, sdk.GroupPermission{Group: *g2, Permission: sdk.PermissionReadWriteExecute}) + req = assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPost, uri, sdk.GroupPermission{ + Permission: sdk.PermissionReadExecute, + Group: *g2, + }) rec = httptest.NewRecorder() - api.Router.Mux.ServeHTTP(rec, req) - require.Equal(t, http.StatusForbidden, rec.Code) + router.Mux.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + require.NoError(t, group.LoadGroupsIntoProject(context.TODO(), db, proj)) + require.Len(t, proj.ProjectGroups, 2) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g1.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, proj.ProjectGroups.GetByGroupID(g1.ID).Permission) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g2.ID)) + require.Equal(t, sdk.PermissionReadExecute, proj.ProjectGroups.GetByGroupID(g2.ID).Permission) - // User 2 can downgrade permission on group g2 - uri = api.Router.GetRoute(http.MethodPut, api.putGroupRoleOnProjectHandler, map[string]string{ + // User can add R permission for g3 on project + uri = router.GetRoute(http.MethodPost, api.postGroupInProjectHandler, map[string]string{ "permProjectKey": proj.Key, - "groupName": g2.Name, }) require.NotEmpty(t, uri) - req = assets.NewJWTAuthentifiedRequest(t, jwtLambda2, http.MethodPut, uri, sdk.GroupPermission{Group: *g2, Permission: sdk.PermissionRead}) + req = assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPost, uri, sdk.GroupPermission{ + Permission: sdk.PermissionRead, + Group: *g3, + }) rec = httptest.NewRecorder() - api.Router.Mux.ServeHTTP(rec, req) + router.Mux.ServeHTTP(rec, req) require.Equal(t, http.StatusOK, rec.Code) + require.NoError(t, group.LoadGroupsIntoProject(context.TODO(), db, proj)) + require.Len(t, proj.ProjectGroups, 3) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g1.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, proj.ProjectGroups.GetByGroupID(g1.ID).Permission) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g2.ID)) + require.Equal(t, sdk.PermissionReadExecute, proj.ProjectGroups.GetByGroupID(g2.ID).Permission) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g3.ID)) + require.Equal(t, sdk.PermissionRead, proj.ProjectGroups.GetByGroupID(g3.ID).Permission) +} + +func Test_postGroupInProjectHandler_OnlyReadForDifferentOrganization(t *testing.T) { + api, db, router := newTestAPI(t) - // User 2 can remove permission on group g2 - uri = api.Router.GetRoute(http.MethodDelete, api.deleteGroupFromProjectHandler, map[string]string{ + 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)) + + // Set organization for groups + require.NoError(t, group.InsertOrganization(context.TODO(), db, &group.Organization{ + GroupID: g1.ID, + Organization: "one", + })) + require.NoError(t, group.InsertOrganization(context.TODO(), db, &group.Organization{ + GroupID: g2.ID, + Organization: "two", + })) + + _, jwt := assets.InsertAdminUser(t, db) + + // Cannot add RX permission for g2 on project because organization is not the same as project's one + uri := router.GetRoute(http.MethodPost, api.postGroupInProjectHandler, map[string]string{ + "permProjectKey": proj.Key, + }) + require.NotEmpty(t, uri) + req := assets.NewJWTAuthentifiedRequest(t, jwt, http.MethodPost, uri, sdk.GroupPermission{ + Permission: sdk.PermissionReadExecute, + Group: *g2, + }) + rec := httptest.NewRecorder() + router.Mux.ServeHTTP(rec, req) + require.Equal(t, http.StatusForbidden, rec.Code) + var sdkError sdk.Error + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &sdkError)) + require.Equal(t, "given group with organization \"two\" don't match project organization \"one\"", sdkError.From) + + // Can add R permission for g2 on project + uri = router.GetRoute(http.MethodPost, api.postGroupInProjectHandler, map[string]string{ "permProjectKey": proj.Key, - "groupName": g2.Name, }) require.NotEmpty(t, uri) - req = assets.NewJWTAuthentifiedRequest(t, jwtLambda2, http.MethodDelete, uri, nil) + req = assets.NewJWTAuthentifiedRequest(t, jwt, http.MethodPost, uri, sdk.GroupPermission{ + Permission: sdk.PermissionRead, + Group: *g2, + }) rec = httptest.NewRecorder() - api.Router.Mux.ServeHTTP(rec, req) + router.Mux.ServeHTTP(rec, req) require.Equal(t, http.StatusOK, rec.Code) + require.NoError(t, group.LoadGroupsIntoProject(context.TODO(), db, proj)) + require.Len(t, proj.ProjectGroups, 2) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g1.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, proj.ProjectGroups.GetByGroupID(g1.ID).Permission) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g2.ID)) + require.Equal(t, sdk.PermissionRead, proj.ProjectGroups.GetByGroupID(g2.ID).Permission) } -func Test_postGroupInProjectHandler(t *testing.T) { - api, db, _ := newTestAPI(t) +func Test_putGroupRoleOnProjectHandler_UserShouldBeGroupAdminForRWAndRWX(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)) g3 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) - g4 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) - // Create a lambda user that is admin on g1 and g2 - u, jwtLambda := assets.InsertLambdaUser(t, db, &g1, g2, g3) + // Create a lambda user that is admin of g1 and g2 + u, jwt := assets.InsertLambdaUser(t, db, &g1, g2) assets.SetUserGroupAdmin(t, db, g1.ID, u.ID) assets.SetUserGroupAdmin(t, db, g2.ID, u.ID) - // User is admin of g2 - uri := api.Router.GetRoute(http.MethodPost, api.postGroupInProjectHandler, map[string]string{ + // Set g2 and g3 on project + require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ + GroupID: g2.ID, + ProjectID: proj.ID, + Role: sdk.PermissionRead, + })) + require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ + GroupID: g3.ID, + ProjectID: proj.ID, + Role: sdk.PermissionReadExecute, + })) + + // User cannot set RWX permission for g3 on project because not admin of g3 + uri := router.GetRoute(http.MethodPut, api.putGroupRoleOnProjectHandler, map[string]string{ "permProjectKey": proj.Key, + "groupName": g3.Name, }) require.NotEmpty(t, uri) - req := assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPost, uri, sdk.GroupPermission{Group: *g2, Permission: sdk.PermissionRead}) + req := assets.NewAuthentifiedRequest(t, u, jwt, http.MethodPut, uri, sdk.GroupPermission{ + Permission: sdk.PermissionReadWriteExecute, + Group: *g3, + }) rec := httptest.NewRecorder() - api.Router.Mux.ServeHTTP(rec, req) + router.Mux.ServeHTTP(rec, req) + require.Equal(t, http.StatusForbidden, rec.Code) + var sdkError sdk.Error + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &sdkError)) + require.Equal(t, "User is not a group's admin", sdkError.Message) + + // User can set RX permission for g2 on project because admin of g2 + uri = router.GetRoute(http.MethodPut, api.putGroupRoleOnProjectHandler, map[string]string{ + "permProjectKey": proj.Key, + "groupName": g2.Name, + }) + require.NotEmpty(t, uri) + req = assets.NewAuthentifiedRequest(t, u, jwt, http.MethodPut, uri, sdk.GroupPermission{ + Permission: sdk.PermissionReadExecute, + Group: *g2, + }) + rec = httptest.NewRecorder() + router.Mux.ServeHTTP(rec, req) require.Equal(t, http.StatusOK, rec.Code) + require.NoError(t, group.LoadGroupsIntoProject(context.TODO(), db, proj)) + require.Len(t, proj.ProjectGroups, 3) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g1.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, proj.ProjectGroups.GetByGroupID(g1.ID).Permission) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g2.ID)) + require.Equal(t, sdk.PermissionReadExecute, proj.ProjectGroups.GetByGroupID(g2.ID).Permission) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g3.ID)) + require.Equal(t, sdk.PermissionReadExecute, proj.ProjectGroups.GetByGroupID(g3.ID).Permission) - // User is not admin of g3 - uri = api.Router.GetRoute(http.MethodPost, api.postGroupInProjectHandler, map[string]string{ + // User can downgrade permission to R for g3 on workflow + uri = router.GetRoute(http.MethodPut, api.putGroupRoleOnProjectHandler, map[string]string{ "permProjectKey": proj.Key, + "groupName": g3.Name, }) require.NotEmpty(t, uri) - req = assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPost, uri, sdk.GroupPermission{Group: *g3, Permission: sdk.PermissionRead}) + req = assets.NewAuthentifiedRequest(t, u, jwt, http.MethodPut, uri, sdk.GroupPermission{ + Permission: sdk.PermissionRead, + Group: *g3, + }) rec = httptest.NewRecorder() - api.Router.Mux.ServeHTTP(rec, req) + router.Mux.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + require.NoError(t, group.LoadGroupsIntoProject(context.TODO(), db, proj)) + require.Len(t, proj.ProjectGroups, 3) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g1.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, proj.ProjectGroups.GetByGroupID(g1.ID).Permission) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g2.ID)) + require.Equal(t, sdk.PermissionReadExecute, proj.ProjectGroups.GetByGroupID(g2.ID).Permission) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g3.ID)) + require.Equal(t, sdk.PermissionRead, proj.ProjectGroups.GetByGroupID(g3.ID).Permission) +} + +func Test_putGroupRoleOnProjectHandler_OnlyReadForDifferentOrganization(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)) + + // Set organization for groups + require.NoError(t, group.InsertOrganization(context.TODO(), db, &group.Organization{ + GroupID: g1.ID, + Organization: "one", + })) + require.NoError(t, group.InsertOrganization(context.TODO(), db, &group.Organization{ + GroupID: g2.ID, + Organization: "two", + })) + + _, jwt := assets.InsertAdminUser(t, db) + + // Set g2 on project + require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ + GroupID: g2.ID, + ProjectID: proj.ID, + Role: sdk.PermissionRead, + })) + + // Cannot set RX permission for g2 on project because organization is not the same as project's one + uri := router.GetRoute(http.MethodPut, api.putGroupRoleOnProjectHandler, map[string]string{ + "permProjectKey": proj.Key, + "groupName": g2.Name, + }) + require.NotEmpty(t, uri) + req := assets.NewJWTAuthentifiedRequest(t, jwt, http.MethodPut, uri, sdk.GroupPermission{ + Permission: sdk.PermissionReadExecute, + Group: *g2, + }) + rec := httptest.NewRecorder() + router.Mux.ServeHTTP(rec, req) require.Equal(t, http.StatusForbidden, rec.Code) + var sdkError sdk.Error + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &sdkError)) + require.Equal(t, "given group with organization \"two\" don't match project organization \"one\"", sdkError.From) +} + +func Test_deleteGroupFromProjectHandler(t *testing.T) { + api, db, _ := 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)) + + // Create a lambda user that is member of g1 + _, jwt := assets.InsertLambdaUser(t, db, &g1) + + // Add group g2 on project + require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ + GroupID: g2.ID, + ProjectID: proj.ID, + Role: sdk.PermissionRead, + })) - // User is not member of g4 - uri = api.Router.GetRoute(http.MethodPost, api.postGroupInProjectHandler, map[string]string{ + // User can remove permission on group g2 + uri := api.Router.GetRoute(http.MethodDelete, api.deleteGroupFromProjectHandler, map[string]string{ "permProjectKey": proj.Key, + "groupName": g2.Name, }) require.NotEmpty(t, uri) - req = assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPost, uri, sdk.GroupPermission{Group: *g4, Permission: sdk.PermissionRead}) - rec = httptest.NewRecorder() + req := assets.NewJWTAuthentifiedRequest(t, jwt, http.MethodDelete, uri, nil) + rec := httptest.NewRecorder() api.Router.Mux.ServeHTTP(rec, req) - require.Equal(t, http.StatusForbidden, rec.Code) + require.Equal(t, http.StatusOK, rec.Code) + require.NoError(t, group.LoadGroupsIntoProject(context.TODO(), db, proj)) + require.Len(t, proj.ProjectGroups, 1) + require.NotNil(t, proj.ProjectGroups.GetByGroupID(g1.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, proj.ProjectGroups.GetByGroupID(g1.ID).Permission) } // Test_ProjectPerms Useful to test permission on project diff --git a/engine/api/router_middleware_auth_permission_test.go b/engine/api/router_middleware_auth_permission_test.go index 5ca348ac3f..3ff0366069 100644 --- a/engine/api/router_middleware_auth_permission_test.go +++ b/engine/api/router_middleware_auth_permission_test.go @@ -708,10 +708,10 @@ func Test_checkWorkflowPermissionsByUser(t *testing.T) { } for groupName, permLevel := range tt.setup.WorkflowGroupPermissions { - g, err := group.LoadByName(context.TODO(), api.mustDB(), groupName+suffix, group.LoadOptions.WithMembers) + g, err := group.LoadByName(context.TODO(), db, groupName+suffix, group.LoadOptions.WithMembers) require.NoError(t, err) - require.NoError(t, group.AddWorkflowGroup(context.TODO(), api.mustDB(), wrkflw, sdk.GroupPermission{ + require.NoError(t, group.AddWorkflowGroup(context.TODO(), db, wrkflw, sdk.GroupPermission{ Group: *g, Permission: permLevel, })) diff --git a/engine/api/workflow.go b/engine/api/workflow.go index ddacfd11b7..454a76bd5e 100644 --- a/engine/api/workflow.go +++ b/engine/api/workflow.go @@ -495,22 +495,9 @@ func (api *API) postWorkflowHandler() service.Handler { } defer tx.Rollback() // nolint - for _, n := range data.WorkflowData.Array() { - for _, gp := range n.Groups { - if gp.Group.ID == 0 { - g, err := group.LoadByName(ctx, tx, gp.Group.Name) - if err != nil { - return err - } - gp.Group = *g - } - if err := group.LoadOptions.WithOrganization(ctx, tx, &gp.Group); err != nil { - return err - } - if err := projectPermissionCheckOrganizationMatch(ctx, tx, p, &gp.Group, gp.Permission); err != nil { - return err - } - } + // Check group permissions if given for workflow and nodes + if err := group.CheckWorkflowGroups(ctx, tx, p, &data, getAPIConsumer(ctx)); err != nil { + return err } if err := workflow.Insert(ctx, tx, api.Cache, *p, &data); err != nil { @@ -580,22 +567,12 @@ func (api *API) putWorkflowHandler() service.Handler { } defer tx.Rollback() // nolint - for _, n := range wf.WorkflowData.Array() { - for _, gp := range n.Groups { - if gp.Group.ID == 0 { - g, err := group.LoadByName(ctx, tx, gp.Group.Name) - if err != nil { - return err - } - gp.Group = *g - } - if err := group.LoadOptions.WithOrganization(ctx, tx, &gp.Group); err != nil { - return err - } - if err := projectPermissionCheckOrganizationMatch(ctx, tx, p, &gp.Group, gp.Permission); err != nil { - return err - } - } + // Update workflow do not handle workflow groups changes but only nodes groups + wf.Groups = nil + + // Check group permissions if given for workflow and nodes + if err := group.CheckWorkflowGroups(ctx, tx, p, &wf, getAPIConsumer(ctx)); err != nil { + return err } if err := workflow.Update(ctx, tx, api.Cache, *p, &wf, workflow.UpdateOptions{}); err != nil { diff --git a/engine/api/workflow/dao.go b/engine/api/workflow/dao.go index e0cbabce6a..6620fdd269 100644 --- a/engine/api/workflow/dao.go +++ b/engine/api/workflow/dao.go @@ -348,11 +348,14 @@ func Insert(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St } w.Groups[i].Group = *g } - if err := group.UpsertAllWorkflowGroups(db, w, w.Groups); err != nil { + if err := group.UpsertAllWorkflowGroups(ctx, db, w, w.Groups); err != nil { return sdk.WrapError(err, "Unable to update workflow") } } else { log.Debug(ctx, "inherit permissions from project") + if err := group.LoadGroupsIntoProject(ctx, db, &proj); err != nil { + return err + } for _, gp := range proj.ProjectGroups { if err := group.AddWorkflowGroup(ctx, db, w, gp); err != nil { return sdk.WrapError(err, "Cannot add group %s", gp.Group.Name) @@ -1251,7 +1254,7 @@ func checkPipeline(ctx context.Context, db gorp.SqlExecutor, proj sdk.Project, w // Push push a workflow from cds files func Push(ctx context.Context, db *gorp.DbMap, store cache.Store, proj *sdk.Project, data exportentities.WorkflowComponents, - opts *PushOption, u sdk.Identifiable, decryptFunc keys.DecryptFunc) ([]sdk.Message, *sdk.Workflow, *sdk.Workflow, *PushSecrets, error) { + opts *PushOption, consumer *sdk.AuthConsumer, decryptFunc keys.DecryptFunc) ([]sdk.Message, *sdk.Workflow, *sdk.Workflow, *PushSecrets, error) { ctx, end := telemetry.Span(ctx, "workflow.Push") defer end() if data.Workflow == nil { @@ -1299,7 +1302,7 @@ func Push(ctx context.Context, db *gorp.DbMap, store cache.Store, proj *sdk.Proj if opts != nil { fromRepo = opts.FromRepository } - appDB, appSecrets, msgList, err := application.ParseAndImport(ctx, tx, store, *proj, &app, application.ImportOptions{Force: true, FromRepository: fromRepo}, decryptFunc, u) + appDB, appSecrets, msgList, err := application.ParseAndImport(ctx, tx, store, *proj, &app, application.ImportOptions{Force: true, FromRepository: fromRepo}, decryptFunc, consumer) allMsg = append(allMsg, msgList...) if err != nil { return allMsg, nil, nil, nil, sdk.ErrorWithFallback(err, sdk.ErrWrongRequest, "unable to import application %s/%s", proj.Key, app.Name) @@ -1313,7 +1316,7 @@ func Push(ctx context.Context, db *gorp.DbMap, store cache.Store, proj *sdk.Proj if opts != nil { fromRepo = opts.FromRepository } - envDB, envsSecrets, msgList, err := environment.ParseAndImport(ctx, tx, *proj, env, environment.ImportOptions{Force: true, FromRepository: fromRepo}, decryptFunc, u) + envDB, envsSecrets, msgList, err := environment.ParseAndImport(ctx, tx, *proj, env, environment.ImportOptions{Force: true, FromRepository: fromRepo}, decryptFunc, consumer) allMsg = append(allMsg, msgList...) if err != nil { return allMsg, nil, nil, nil, sdk.ErrorWithFallback(err, sdk.ErrWrongRequest, "unable to import environment %s/%s", proj.Key, env.Name) @@ -1327,7 +1330,7 @@ func Push(ctx context.Context, db *gorp.DbMap, store cache.Store, proj *sdk.Proj if opts != nil { fromRepo = opts.FromRepository } - pipDB, msgList, err := pipeline.ParseAndImport(ctx, tx, store, *proj, &pip, u, pipeline.ImportOptions{Force: true, FromRepository: fromRepo}) + pipDB, msgList, err := pipeline.ParseAndImport(ctx, tx, store, *proj, &pip, consumer, pipeline.ImportOptions{Force: true, FromRepository: fromRepo}) allMsg = append(allMsg, msgList...) if err != nil { return allMsg, nil, nil, nil, sdk.ErrorWithFallback(err, sdk.ErrWrongRequest, "unable to import pipeline %s/%s", proj.Key, pip.Name) @@ -1354,7 +1357,7 @@ func Push(ctx context.Context, db *gorp.DbMap, store cache.Store, proj *sdk.Proj importOptions.HookUUID = opts.HookUUID } - wf, msgList, err := ParseAndImport(ctx, tx, store, *proj, oldWf, data.Workflow, u, importOptions) + wf, msgList, err := ParseAndImport(ctx, tx, store, *proj, oldWf, data.Workflow, consumer, importOptions) allMsg = append(allMsg, msgList...) if err != nil { return allMsg, nil, nil, nil, sdk.WrapError(err, "unable to import workflow %s", data.Workflow.GetName()) diff --git a/engine/api/workflow/dao_test.go b/engine/api/workflow/dao_test.go index 3eb725ae5d..5c2e1fcee2 100644 --- a/engine/api/workflow/dao_test.go +++ b/engine/api/workflow/dao_test.go @@ -20,6 +20,7 @@ import ( "gopkg.in/yaml.v2" "github.com/ovh/cds/engine/api/application" + "github.com/ovh/cds/engine/api/authentication" "github.com/ovh/cds/engine/api/bootstrap" "github.com/ovh/cds/engine/api/environment" "github.com/ovh/cds/engine/api/integration" @@ -1934,6 +1935,8 @@ func TestDeleteWorkflowWithDependencies(t *testing.T) { db, cache := test.SetupPG(t) u, _ := assets.InsertAdminUser(t, db) + localConsumer, err := authentication.LoadConsumerByTypeAndUserID(context.TODO(), db, sdk.ConsumerLocal, u.ID, authentication.LoadConsumerOptions.WithAuthentifiedUser) + require.NoError(t, err) bootstrap.InitiliazeDB(ctx, sdk.DefaultValues{}, func() *gorp.DbMap { return db.DbMap }) a, _ := assets.InsertService(t, db, "TestDeleteWorkflowWithDependenciesVCS", sdk.TypeVCS) @@ -2097,7 +2100,7 @@ workflow: eWf, err := exportentities.UnmarshalWorkflow([]byte(workflowS), exportentities.FormatYAML) require.NoError(t, err) - wf, _, err := workflow.ParseAndImport(ctx, db, cache, *proj, nil, eWf, u, workflow.ImportOptions{WorkflowName: "test-env", FromRepository: "from/my-repo"}) + wf, _, err := workflow.ParseAndImport(ctx, db, cache, *proj, nil, eWf, localConsumer, workflow.ImportOptions{WorkflowName: "test-env", FromRepository: "from/my-repo"}) require.NotNil(t, wf) require.NoError(t, err) @@ -2127,6 +2130,8 @@ func TestDeleteWorkflowWithDependencies2(t *testing.T) { db, cache := test.SetupPG(t) u, _ := assets.InsertAdminUser(t, db) + localConsumer, err := authentication.LoadConsumerByTypeAndUserID(context.TODO(), db, sdk.ConsumerLocal, u.ID, authentication.LoadConsumerOptions.WithAuthentifiedUser) + require.NoError(t, err) bootstrap.InitiliazeDB(ctx, sdk.DefaultValues{}, func() *gorp.DbMap { return db.DbMap }) a, _ := assets.InsertService(t, db, "TestDeleteWorkflowWithDependenciesVCS", sdk.TypeVCS) @@ -2290,7 +2295,7 @@ workflow: eWf, err := exportentities.UnmarshalWorkflow([]byte(workflowS), exportentities.FormatYAML) require.NoError(t, err) - wf, _, err := workflow.ParseAndImport(ctx, db, cache, *proj, nil, eWf, u, workflow.ImportOptions{WorkflowName: "test-env", FromRepository: "from/my-repo"}) + wf, _, err := workflow.ParseAndImport(ctx, db, cache, *proj, nil, eWf, localConsumer, workflow.ImportOptions{WorkflowName: "test-env", FromRepository: "from/my-repo"}) require.NotNil(t, wf) require.NoError(t, err) @@ -2306,7 +2311,7 @@ workflow: eWf2, err := exportentities.UnmarshalWorkflow([]byte(workflowS2), exportentities.FormatYAML) require.NoError(t, err) - wf2, _, err := workflow.ParseAndImport(ctx, db, cache, *proj, nil, eWf2, u, workflow.ImportOptions{WorkflowName: "test-env-2", FromRepository: "from/my-repo-2"}) + wf2, _, err := workflow.ParseAndImport(ctx, db, cache, *proj, nil, eWf2, localConsumer, workflow.ImportOptions{WorkflowName: "test-env-2", FromRepository: "from/my-repo-2"}) require.NotNil(t, wf2) require.NoError(t, err) @@ -2328,5 +2333,4 @@ workflow: env, err = environment.LoadEnvironmentByID(db.DbMap, env.ID) require.NoError(t, err) require.Empty(t, env.FromRepository) - } diff --git a/engine/api/workflow/workflow_importer.go b/engine/api/workflow/workflow_importer.go index 1e0fd3aa37..a25fa67a5c 100644 --- a/engine/api/workflow/workflow_importer.go +++ b/engine/api/workflow/workflow_importer.go @@ -18,7 +18,7 @@ import ( ) //Import is able to create a new workflow and all its components -func Import(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.Store, proj sdk.Project, oldW, w *sdk.Workflow, u sdk.Identifiable, opts ImportOptions, msgChan chan<- sdk.Message) error { +func Import(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.Store, proj sdk.Project, oldW, w *sdk.Workflow, consumer *sdk.AuthConsumer, opts ImportOptions, msgChan chan<- sdk.Message) error { ctx, end := telemetry.Span(ctx, "workflow.Import") defer end() @@ -50,7 +50,9 @@ func Import(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St }) } else { // The import is triggered by a user, we have to check the groups - // FIXME: call the same function than the handlers + if err := group.CheckWorkflowGroups(ctx, db, &proj, w, consumer); err != nil { + return err + } } // create the workflow if not exists @@ -64,6 +66,25 @@ func Import(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St return nil } + w.ID = oldW.ID + + // If not groups are given, do not change existing workflow groups + if len(w.Groups) > 0 { + for i := range w.Groups { + if w.Groups[i].Group.ID > 0 { + continue + } + g, err := group.LoadByName(ctx, db, w.Groups[i].Group.Name) + if err != nil { + return sdk.WrapError(err, "unable to load group %s", w.Groups[i].Group.Name) + } + w.Groups[i].Group = *g + } + if err := group.UpsertAllWorkflowGroups(ctx, db, w, w.Groups); err != nil { + return sdk.WrapError(err, "unable to update workflow") + } + } + if oldW.Icon != "" && w.Icon == "" { w.Icon = oldW.Icon } @@ -72,7 +93,7 @@ func Import(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St return sdk.NewErrorFrom(sdk.ErrAlreadyExist, "workflow exists") } - if opts.Force && oldW != nil && oldW.FromRepository != "" && w.FromRepository == "" { + if opts.Force && oldW.FromRepository != "" && w.FromRepository == "" { if err := detachResourceFromRepository(db, proj.ID, oldW, msgChan); err != nil { return err } @@ -100,7 +121,6 @@ func Import(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St } } } - w.ID = oldW.ID // HookRegistration after workflow.Update. It needs hooks to be created on DB // Hook registration must only be done on default branch in case of workflow as-code @@ -109,10 +129,6 @@ func Import(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St DisableHookManagement: w.DerivationBranch != "", } - if err := importWorkflowGroups(db, w); err != nil { - return err - } - if err := Update(ctx, db, store, proj, w, uptOptions); err != nil { return sdk.WrapError(err, "Unable to update workflow") } @@ -172,22 +188,3 @@ func detachResourceFromRepository(db gorp.SqlExecutor, projectID int64, oldW *sd return nil } - -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) - } - w.Groups[i].Group = *g - } - if err := group.UpsertAllWorkflowGroups(db, w, w.Groups); err != nil { - return sdk.WrapError(err, "unable to update workflow") - } - } - return nil -} diff --git a/engine/api/workflow/workflow_importer_test.go b/engine/api/workflow/workflow_importer_test.go index f5c8dee367..7b0acaecb4 100644 --- a/engine/api/workflow/workflow_importer_test.go +++ b/engine/api/workflow/workflow_importer_test.go @@ -9,7 +9,10 @@ import ( "net/http" "testing" + "github.com/stretchr/testify/require" + "github.com/ovh/cds/engine/api/application" + "github.com/ovh/cds/engine/api/authentication" "github.com/ovh/cds/engine/api/environment" "github.com/ovh/cds/engine/api/pipeline" "github.com/ovh/cds/engine/api/project" @@ -33,6 +36,9 @@ func TestImport(t *testing.T) { db, cache := test.SetupPG(t) u, _ := assets.InsertAdminUser(t, db) + localConsumer, err := authentication.LoadConsumerByTypeAndUserID(context.TODO(), db, sdk.ConsumerLocal, u.ID, authentication.LoadConsumerOptions.WithAuthentifiedUser) + require.NoError(t, err) + key := sdk.RandomString(10) proj := assets.InsertTestProject(t, db, cache, key, key) @@ -483,7 +489,7 @@ func TestImport(t *testing.T) { } } - if err := workflow.Import(context.TODO(), db, cache, *proj, wf, tt.args.w, u, workflow.ImportOptions{Force: tt.args.force}, nil); err != nil { + if err := workflow.Import(context.TODO(), db, cache, *proj, wf, tt.args.w, localConsumer, workflow.ImportOptions{Force: tt.args.force}, nil); err != nil { if !tt.wantErr { t.Errorf("Import() error = %v, wantErr %v", err, tt.wantErr) } else { diff --git a/engine/api/workflow/workflow_parser.go b/engine/api/workflow/workflow_parser.go index 94a9a13129..e040938b9b 100644 --- a/engine/api/workflow/workflow_parser.go +++ b/engine/api/workflow/workflow_parser.go @@ -39,25 +39,11 @@ func Parse(ctx context.Context, proj sdk.Project, oldW *sdk.Workflow, ew exporte w.ProjectID = proj.ID w.ProjectKey = proj.Key - // Get permission from old workflow if needed - if oldW != nil && len(w.Groups) == 0 { - w.Groups = make([]sdk.GroupPermission, 0, len(oldW.Groups)) - for _, g := range oldW.Groups { - perm := sdk.GroupPermission{Group: sdk.Group{Name: g.Group.Name}, Permission: g.Permission} - w.Groups = append(w.Groups, perm) - } - } else if len(w.Groups) == 0 { - w.Groups = make([]sdk.GroupPermission, 0, len(proj.ProjectGroups)) - for _, g := range proj.ProjectGroups { - perm := sdk.GroupPermission{Group: sdk.Group{Name: g.Group.Name}, Permission: g.Permission} - w.Groups = append(w.Groups, perm) - } - } return w, nil } // ParseAndImport parse an exportentities.workflow and insert or update the workflow in database -func ParseAndImport(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.Store, proj sdk.Project, oldW *sdk.Workflow, ew exportentities.Workflow, u sdk.Identifiable, opts ImportOptions) (*sdk.Workflow, []sdk.Message, error) { +func ParseAndImport(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.Store, proj sdk.Project, oldW *sdk.Workflow, ew exportentities.Workflow, consumer *sdk.AuthConsumer, opts ImportOptions) (*sdk.Workflow, []sdk.Message, error) { ctx, end := telemetry.Span(ctx, "workflow.ParseAndImport") defer end() @@ -204,7 +190,7 @@ func ParseAndImport(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store } }(&msgList) - globalError := Import(ctx, db, store, proj, oldW, w, u, opts, msgChan) + globalError := Import(ctx, db, store, proj, oldW, w, consumer, opts, msgChan) close(msgChan) done.Wait() diff --git a/engine/api/workflow/workflow_parser_test.go b/engine/api/workflow/workflow_parser_test.go index f83f5b6092..4f054d40f9 100644 --- a/engine/api/workflow/workflow_parser_test.go +++ b/engine/api/workflow/workflow_parser_test.go @@ -119,6 +119,8 @@ func TestParseAndImportFromRepository(t *testing.T) { db, cache := test.SetupPG(t) u, _ := assets.InsertAdminUser(t, db) + localConsumer, err := authentication.LoadConsumerByTypeAndUserID(context.TODO(), db, sdk.ConsumerLocal, u.ID, authentication.LoadConsumerOptions.WithAuthentifiedUser) + require.NoError(t, err) pkey := sdk.RandomString(10) proj := assets.InsertTestProject(t, db, cache, pkey, pkey) @@ -260,7 +262,7 @@ func TestParseAndImportFromRepository(t *testing.T) { }, } - _, _, err := workflow.ParseAndImport(context.TODO(), db, cache, *proj, nil, input, u, workflow.ImportOptions{Force: true, FromRepository: "foo/myrepo"}) + _, _, err = workflow.ParseAndImport(context.TODO(), db, cache, *proj, nil, input, localConsumer, workflow.ImportOptions{Force: true, FromRepository: "foo/myrepo"}) assert.NoError(t, err) w, errW := workflow.Load(context.TODO(), db, cache, *proj, input.Name, workflow.LoadOptions{}) diff --git a/engine/api/workflow_ascode_test.go b/engine/api/workflow_ascode_test.go index e3038d20fe..c4ec0c5022 100644 --- a/engine/api/workflow_ascode_test.go +++ b/engine/api/workflow_ascode_test.go @@ -20,6 +20,7 @@ import ( "gopkg.in/yaml.v2" "github.com/ovh/cds/engine/api/application" + "github.com/ovh/cds/engine/api/authentication" "github.com/ovh/cds/engine/api/event" "github.com/ovh/cds/engine/api/group" "github.com/ovh/cds/engine/api/pipeline" @@ -974,6 +975,9 @@ hooks: proj := assets.InsertTestProject(t, db, api.Cache, prjKey, prjKey) u, _ := assets.InsertLambdaUser(t, db, &proj.ProjectGroups[0].Group) + localConsumer, err := authentication.LoadConsumerByTypeAndUserID(context.TODO(), db, sdk.ConsumerLocal, u.ID, authentication.LoadConsumerOptions.WithAuthentifiedUser) + require.NoError(t, err) + vcsServer := sdk.ProjectVCSServerLink{ ProjectID: proj.ID, Name: "github", @@ -1203,7 +1207,7 @@ hooks: // Import Workflow ew, err := exportentities.UnmarshalWorkflow([]byte(wf), exportentities.FormatYAML) require.NoError(t, err) - workflowInserted, _, err := workflow.ParseAndImport(context.TODO(), db, api.Cache, *proj, nil, ew, u, workflow.ImportOptions{Force: true, FromRepository: "https://github.com/fsamin/go-repo.git"}) + workflowInserted, _, err := workflow.ParseAndImport(context.TODO(), db, api.Cache, *proj, nil, ew, localConsumer, workflow.ImportOptions{Force: true, FromRepository: "https://github.com/fsamin/go-repo.git"}) require.NoError(t, err) require.Equal(t, workflowInserted.FromRepository, "https://github.com/fsamin/go-repo.git") diff --git a/engine/api/workflow_group.go b/engine/api/workflow_group.go index 052fe1d6db..c92fdf0cf7 100644 --- a/engine/api/workflow_group.go +++ b/engine/api/workflow_group.go @@ -55,7 +55,7 @@ func (api *API) deleteWorkflowGroupHandler() service.Handler { defer tx.Rollback() // nolint if err := group.DeleteWorkflowGroup(tx, wf, oldGp.Group.ID, groupIndex); err != nil { - return sdk.WrapError(err, "cannot delete group") + return err } if err := tx.Commit(); err != nil { @@ -130,7 +130,7 @@ func (api *API) putWorkflowGroupHandler() service.Handler { } defer tx.Rollback() // nolint - if err := projectPermissionCheckOrganizationMatch(ctx, tx, proj, &gp.Group, gp.Permission); err != nil { + if err := group.CheckProjectOrganizationMatch(ctx, tx, proj, &gp.Group, gp.Permission); err != nil { return err } @@ -200,7 +200,7 @@ func (api *API) postWorkflowGroupHandler() service.Handler { } defer tx.Rollback() // nolint - if err := projectPermissionCheckOrganizationMatch(ctx, tx, proj, &gp.Group, gp.Permission); err != nil { + if err := group.CheckProjectOrganizationMatch(ctx, tx, proj, &gp.Group, gp.Permission); err != nil { return err } diff --git a/engine/api/workflow_group_test.go b/engine/api/workflow_group_test.go index 5190c1eb3c..5828e51357 100644 --- a/engine/api/workflow_group_test.go +++ b/engine/api/workflow_group_test.go @@ -12,7 +12,6 @@ import ( "github.com/ovh/cds/engine/api/group" "github.com/ovh/cds/engine/api/pipeline" "github.com/ovh/cds/engine/api/project" - "github.com/ovh/cds/engine/api/test" "github.com/ovh/cds/engine/api/test/assets" "github.com/ovh/cds/engine/api/workflow" "github.com/ovh/cds/sdk" @@ -379,7 +378,7 @@ func Test_putWorkflowGroupHandler_UserShouldBeGroupAdminForRWAndRWX(t *testing.T Group: *g3, })) - // User cannot set RX permission for g3 on workflow because not admin of g3 + // User cannot set RWX permission for g3 on workflow because not admin of g3 uri := router.GetRoute(http.MethodPut, api.putWorkflowGroupHandler, map[string]string{ "key": proj.Key, "permWorkflowName": w.Name, @@ -808,145 +807,93 @@ func Test_PermissionOnWorkflowInferiorOfProject(t *testing.T) { require.Equal(t, 403, w.Code) } -// Test_PermissionOnWorkflowWithRestrictionOnNode Useful to test when we add permission on a workflow node -func Test_PermissionOnWorkflowWithRestrictionOnNode(t *testing.T) { +// Test_postWorkflowRunHandler_WithRestrictionOnNode useful to test workflow run with permission on a node +func Test_postWorkflowRunHandler_WithRestrictionOnNode(t *testing.T) { api, db, router := newTestAPI(t) proj := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), sdk.RandomString(10)) - u, pass := assets.InsertLambdaUser(t, db, &proj.ProjectGroups[0].Group) - assets.SetUserGroupAdmin(t, db, proj.ProjectGroups[0].Group.ID, u.ID) - // Add a new group on project to let us update the previous group permission to READ (because we must have at least one RW permission on project) - newGr := assets.InsertTestGroup(t, db, sdk.RandomString(10)) + g1 := proj.ProjectGroups[0].Group + g2 := assets.InsertTestGroup(t, db, sdk.RandomString(10)) + + // Set g2 to RX on project require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{ - GroupID: newGr.ID, + GroupID: g2.ID, ProjectID: proj.ID, - Role: sdk.PermissionReadWriteExecute, + Role: sdk.PermissionReadExecute, })) - require.NoError(t, group.InsertLinkGroupUser(context.TODO(), db, &group.LinkGroupUser{ - GroupID: newGr.ID, - AuthentifiedUserID: u.ID, - Admin: true, - })) - oldLink, err := group.LoadLinkGroupProjectForGroupIDAndProjectID(context.TODO(), db, proj.ProjectGroups[0].Group.ID, proj.ID) - require.NoError(t, err) - newLink := *oldLink - newLink.Role = sdk.PermissionRead - require.NoError(t, group.UpdateLinkGroupProject(db, &newLink)) - - //First pipeline - pip := sdk.Pipeline{ - ProjectID: proj.ID, - ProjectKey: proj.Key, - Name: "pip1", - } - require.NoError(t, pipeline.InsertPipeline(api.mustDB(), &pip)) - - newWf := 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, - } - //Prepare request to create workflow - vars := map[string]string{ - "permProjectKey": proj.Key, - } - uri := router.GetRoute("POST", api.postWorkflowHandler, vars) - require.NotEmpty(t, uri) - req := assets.NewAuthentifiedRequest(t, u, pass, "POST", uri, &newWf) - //Do the request - w := httptest.NewRecorder() - router.Mux.ServeHTTP(w, req) - require.Equal(t, 201, w.Code) + // Create user 1 that is admin of g1 and g2 + u1, jwtLambda1 := assets.InsertLambdaUser(t, db, &g1) + assets.SetUserGroupAdmin(t, db, g1.ID, u1.ID) + // Create user 2 that is admin of g2 + u2, jwtLambda2 := assets.InsertLambdaUser(t, db, g2) + assets.SetUserGroupAdmin(t, db, g2.ID, u2.ID) - require.NoError(t, json.Unmarshal(w.Body.Bytes(), &newWf)) - require.NotEqual(t, 0, newWf.ID) + // Insert workflow that will inherit from project permission + w := assets.InsertTestWorkflow(t, db, api.Cache, proj, sdk.RandomString(10)) - // Update workflow group to change READ to RWX and get permission on project in READ and permission on workflow in RWX to test edition and run - vars = map[string]string{ + // Update workflow to add RX node permission only for g1 + w.WorkflowData.Node.Groups = []sdk.GroupPermission{{ + Group: g1, + Permission: sdk.PermissionReadExecute, + }} + uri := router.GetRoute(http.MethodPut, api.putWorkflowHandler, map[string]string{ "key": proj.Key, - "permWorkflowName": newWf.Name, - "groupName": proj.ProjectGroups[0].Group.Name, - } - uri = router.GetRoute("PUT", api.putWorkflowGroupHandler, vars) + "permWorkflowName": w.Name, + }) require.NotEmpty(t, uri) + req := assets.NewJWTAuthentifiedRequest(t, jwtLambda1, http.MethodPut, uri, &w) + 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.PermissionReadExecute, workflowResult.Groups.GetByGroupID(g2.ID).Permission) + require.Len(t, workflowResult.WorkflowData.Node.Groups, 1) + require.NotNil(t, workflowResult.WorkflowData.Node.Groups.GetByGroupID(g1.ID)) + require.Equal(t, sdk.PermissionReadExecute, workflowResult.WorkflowData.Node.Groups.GetByGroupID(g1.ID).Permission) - newGp := sdk.GroupPermission{ - Group: proj.ProjectGroups[0].Group, - Permission: sdk.PermissionReadWriteExecute, - } - req = assets.NewAuthentifiedRequest(t, u, pass, "PUT", uri, &newGp) - //Do the request - w = httptest.NewRecorder() - router.Mux.ServeHTTP(w, req) - require.Equal(t, 200, w.Code) - - require.NoError(t, group.DeleteLinkGroupUserForGroupIDAndUserID(db, proj.ProjectGroups[0].Group.ID, u.ID)) - - proj2, err := project.Load(context.TODO(), api.mustDB(), proj.Key, project.LoadOptions.WithPipelines, project.LoadOptions.WithGroups) - require.NoError(t, err) - wfLoaded, err := workflow.Load(context.Background(), db, api.Cache, *proj2, newWf.Name, workflow.LoadOptions{DeepPipeline: true}) - require.NoError(t, err) - require.Equal(t, 2, len(wfLoaded.Groups)) - - // Try to update workflow - vars = map[string]string{ + // User 1 should be able to run the workflow + uri = router.GetRoute(http.MethodPost, api.postWorkflowRunHandler, map[string]string{ "key": proj.Key, - "permWorkflowName": wfLoaded.Name, - } - uri = router.GetRoute("PUT", api.putWorkflowHandler, vars) - test.NotEmpty(t, uri) - - wfLoaded.HistoryLength = 300 - wfLoaded.WorkflowData.Node.Groups = []sdk.GroupPermission{ - { - Group: proj.ProjectGroups[0].Group, - Permission: sdk.PermissionReadExecute, + "permWorkflowName": w.Name, + }) + require.NotEmpty(t, uri) + req = assets.NewJWTAuthentifiedRequest(t, jwtLambda1, http.MethodPost, uri, &sdk.WorkflowRunPostHandlerOption{ + FromNodeIDs: []int64{w.WorkflowData.Node.ID}, + Manual: &sdk.WorkflowNodeRunManual{ + Username: u1.Username, + Fullname: u1.Fullname, + Email: u1.GetEmail(), }, - } - req = assets.NewAuthentifiedRequest(t, u, pass, "PUT", uri, &wfLoaded) - //Do the request - w = httptest.NewRecorder() - router.Mux.ServeHTTP(w, req) - require.Equal(t, 200, w.Code) - - wfLoaded, err = workflow.Load(context.Background(), db, api.Cache, *proj2, newWf.Name, workflow.LoadOptions{}) - require.NoError(t, err) - require.Equal(t, 2, len(wfLoaded.Groups)) - require.Equal(t, int64(300), wfLoaded.HistoryLength) + }) + rec = httptest.NewRecorder() + router.Mux.ServeHTTP(rec, req) + require.Equal(t, http.StatusAccepted, rec.Code) - // Try to run workflow - vars = map[string]string{ + // User 2 should not be able to run the workflow + uri = router.GetRoute(http.MethodPost, api.postWorkflowRunHandler, map[string]string{ "key": proj.Key, - "permWorkflowName": wfLoaded.Name, - } - uri = router.GetRoute("POST", api.postWorkflowRunHandler, vars) + "permWorkflowName": w.Name, + }) require.NotEmpty(t, uri) - - opts := sdk.WorkflowRunPostHandlerOption{ - FromNodeIDs: []int64{wfLoaded.WorkflowData.Node.ID}, + req = assets.NewJWTAuthentifiedRequest(t, jwtLambda2, http.MethodPost, uri, &sdk.WorkflowRunPostHandlerOption{ + FromNodeIDs: []int64{w.WorkflowData.Node.ID}, Manual: &sdk.WorkflowNodeRunManual{ - Username: u.Username, - Fullname: u.Fullname, - Email: u.GetEmail(), + Username: u2.Username, + Fullname: u2.Fullname, + Email: u2.GetEmail(), }, - } - req = assets.NewAuthentifiedRequest(t, u, pass, "POST", uri, &opts) - //Do the request - w = httptest.NewRecorder() - router.Mux.ServeHTTP(w, req) - require.Equal(t, 403, w.Code) + }) + rec = httptest.NewRecorder() + router.Mux.ServeHTTP(rec, req) + require.Equal(t, http.StatusForbidden, rec.Code) var wfError sdk.Error - require.NoError(t, json.Unmarshal(w.Body.Bytes(), &wfError)) + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &wfError)) require.Equal(t, "you don't have execution right", wfError.Message) } diff --git a/engine/api/workflow_import_test.go b/engine/api/workflow_import_test.go index 18f5b0a625..2d8f95328f 100644 --- a/engine/api/workflow_import_test.go +++ b/engine/api/workflow_import_test.go @@ -4,8 +4,8 @@ import ( "bytes" "context" "io" + "net/http" "net/http/httptest" - "sort" "strings" "testing" @@ -91,11 +91,12 @@ metadata: func Test_postWorkflowImportWithPermissionHandler(t *testing.T) { api, db, _ := newTestAPI(t) - u, pass := assets.InsertAdminUser(t, db) + proj := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), "a-"+sdk.RandomString(10)) + + g0 := proj.ProjectGroups[0].Group 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, @@ -107,6 +108,11 @@ func Test_postWorkflowImportWithPermissionHandler(t *testing.T) { Role: sdk.PermissionReadExecute, })) + u, jwtLambda := assets.InsertLambdaUser(t, db, &g0, g1, g2) + assets.SetUserGroupAdmin(t, db, g0.ID, u.ID) + assets.SetUserGroupAdmin(t, db, g1.ID, u.ID) + assets.SetUserGroupAdmin(t, db, g2.ID, u.ID) + pip := sdk.Pipeline{ ProjectID: proj.ID, ProjectKey: proj.Key, @@ -114,15 +120,13 @@ func Test_postWorkflowImportWithPermissionHandler(t *testing.T) { } require.NoError(t, pipeline.InsertPipeline(db, &pip)) - //Prepare request - vars := map[string]string{ + // Import workflow with permission + uri := api.Router.GetRoute(http.MethodPost, api.postWorkflowImportHandler, map[string]string{ "permProjectKey": proj.Key, - } - uri := api.Router.GetRoute("POST", api.postWorkflowImportHandler, vars) - test.NotEmpty(t, uri) - req := assets.NewAuthentifiedRequest(t, u, pass, "POST", uri, nil) - - body := `name: test2 + }) + require.NotEmpty(t, uri) + req := assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPost, uri, nil) + req.Body = io.NopCloser(strings.NewReader(`name: test2 version: v2.0 workflow: test: @@ -131,30 +135,22 @@ workflow: ` + g1.Name + `: 7 permissions: ` + g1.Name + `: 5 - ` + g2.Name + `: 7` - req.Body = io.NopCloser(strings.NewReader(body)) + ` + g2.Name + `: 7`)) req.Header.Set("Content-Type", "application/x-yaml") - - //Do the request rec := httptest.NewRecorder() api.Router.Mux.ServeHTTP(rec, req) - assert.Equal(t, 200, rec.Code) - - //Check result - t.Logf(">>%s", rec.Body.String()) + require.Equal(t, http.StatusOK, rec.Code) w, err := workflow.Load(context.TODO(), db, api.Cache, *proj, "test2", workflow.LoadOptions{}) - test.NoError(t, err) - - assert.NotNil(t, w) - - m, _ := dump.ToStringMap(w) - t.Logf("%+v", m) - assert.Equal(t, "test2", m["Workflow.Name"]) - assert.Equal(t, "test", m["Workflow.WorkflowData.Node.Name"]) - assert.Equal(t, "pip1", m["Workflow.WorkflowData.Node.Context.PipelineName"]) - assert.Equal(t, "7", m["Workflow.WorkflowData.Node.Groups.Groups0.Permission"]) - assert.Equal(t, g1.Name, m["Workflow.WorkflowData.Node.Groups.Groups0.Group.Name"]) + require.NoError(t, err) + require.Len(t, w.Groups, 2) + require.NotNil(t, w.Groups.GetByGroupID(g1.ID)) + require.Equal(t, sdk.PermissionReadExecute, w.Groups.GetByGroupID(g1.ID).Permission) + require.NotNil(t, w.Groups.GetByGroupID(g2.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, w.Groups.GetByGroupID(g2.ID).Permission) + require.Len(t, w.WorkflowData.Node.Groups, 1) + require.NotNil(t, w.WorkflowData.Node.Groups.GetByGroupID(g1.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, w.WorkflowData.Node.Groups.GetByGroupID(g1.ID).Permission) } func Test_postWorkflowImportHandlerWithExistingIcon(t *testing.T) { @@ -989,11 +985,12 @@ metadata: func Test_postWorkflowImportHandler_editPermissions(t *testing.T) { api, db, _ := newTestAPI(t) - u, pass := assets.InsertAdminUser(t, db) + proj := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), "a-"+sdk.RandomString(10)) + + g0 := proj.ProjectGroups[0].Group 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, @@ -1005,6 +1002,11 @@ func Test_postWorkflowImportHandler_editPermissions(t *testing.T) { Role: sdk.PermissionReadExecute, })) + u, jwtLambda := assets.InsertLambdaUser(t, db, &g0, g1, g2) + assets.SetUserGroupAdmin(t, db, g0.ID, u.ID) + assets.SetUserGroupAdmin(t, db, g1.ID, u.ID) + assets.SetUserGroupAdmin(t, db, g2.ID, u.ID) + pip := sdk.Pipeline{ ProjectID: proj.ID, ProjectKey: proj.Key, @@ -1012,109 +1014,89 @@ func Test_postWorkflowImportHandler_editPermissions(t *testing.T) { } require.NoError(t, pipeline.InsertPipeline(db, &pip)) - uri := api.Router.GetRoute("POST", api.postWorkflowImportHandler, map[string]string{ + uri := api.Router.GetRoute(http.MethodPost, api.postWorkflowImportHandler, map[string]string{ "permProjectKey": proj.Key, }) - req := assets.NewAuthentifiedRequest(t, u, pass, "POST", uri, nil) - - body := `name: test_1 + req := assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPost, uri, nil) + req.Body = io.NopCloser(strings.NewReader(`name: test_1 version: v2.0 workflow: pip1: - pipeline: pip1` - req.Body = io.NopCloser(strings.NewReader(body)) + pipeline: pip1`)) req.Header.Set("Content-Type", "application/x-yaml") rec := httptest.NewRecorder() api.Router.Mux.ServeHTTP(rec, req) - require.Equal(t, 200, rec.Code) + require.Equal(t, http.StatusOK, rec.Code) t.Logf(">>%s", rec.Body.String()) + // Workflow permissions should be inherited from project 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{ + require.NotNil(t, w.Groups.GetByGroupID(g0.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, w.Groups.GetByGroupID(g0.ID).Permission) + require.NotNil(t, w.Groups.GetByGroupID(g1.ID)) + require.Equal(t, sdk.PermissionReadExecute, w.Groups.GetByGroupID(g1.ID).Permission) + require.NotNil(t, w.Groups.GetByGroupID(g2.ID)) + require.Equal(t, sdk.PermissionReadExecute, w.Groups.GetByGroupID(g2.ID).Permission) + + // We want to change to permission for g2 and remove the permission for g1 + uri = api.Router.GetRoute(http.MethodPost, api.postWorkflowImportHandler, map[string]string{ "permProjectKey": proj.Key, }) - req = assets.NewAuthentifiedRequest(t, u, pass, "POST", uri, nil) + req = assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPost, uri, nil) q := req.URL.Query() q.Set("force", "true") req.URL.RawQuery = q.Encode() - - body = `name: test_1 + req.Body = io.NopCloser(strings.NewReader(`name: test_1 version: v2.0 workflow: pip1: pipeline: pip1 permissions: - ` + proj.ProjectGroups[0].Group.Name + `: 7 - ` + g2.Name + `: 4` - req.Body = io.NopCloser(strings.NewReader(body)) + ` + g0.Name + `: 7 + ` + g2.Name + `: 4`)) req.Header.Set("Content-Type", "application/x-yaml") - rec = httptest.NewRecorder() api.Router.Mux.ServeHTTP(rec, req) - require.Equal(t, 200, rec.Code) + require.Equal(t, http.StatusOK, 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) + require.NotNil(t, w.Groups.GetByGroupID(g0.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, w.Groups.GetByGroupID(g0.ID).Permission) + require.NotNil(t, w.Groups.GetByGroupID(g2.ID)) + require.Equal(t, sdk.PermissionRead, w.Groups.GetByGroupID(g2.ID).Permission) - // Import again the workflow without permissions should reset to previous workflow permissions - uri = api.Router.GetRoute("POST", api.postWorkflowImportHandler, map[string]string{ + // Import again the workflow without permissions should not not change existing permissions + uri = api.Router.GetRoute(http.MethodPost, api.postWorkflowImportHandler, map[string]string{ "permProjectKey": proj.Key, }) - req = assets.NewAuthentifiedRequest(t, u, pass, "POST", uri, nil) + req = assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPost, uri, nil) q = req.URL.Query() q.Set("force", "true") req.URL.RawQuery = q.Encode() - - body = `name: test_1 + req.Body = io.NopCloser(strings.NewReader(`name: test_1 version: v2.0 workflow: pip1: - pipeline: pip1` - req.Body = io.NopCloser(strings.NewReader(body)) + pipeline: pip1`)) req.Header.Set("Content-Type", "application/x-yaml") - rec = httptest.NewRecorder() api.Router.Mux.ServeHTTP(rec, req) - require.Equal(t, 200, rec.Code) + require.Equal(t, http.StatusOK, 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) + require.NotNil(t, w.Groups.GetByGroupID(g0.ID)) + require.Equal(t, sdk.PermissionReadWriteExecute, w.Groups.GetByGroupID(g0.ID).Permission) + require.NotNil(t, w.Groups.GetByGroupID(g2.ID)) + require.Equal(t, sdk.PermissionRead, w.Groups.GetByGroupID(g2.ID).Permission) } func Test_postWorkflowImportHandler_WithArtifactManager(t *testing.T) { diff --git a/engine/api/workflow_test.go b/engine/api/workflow_test.go index 778eb44234..f4220801ef 100644 --- a/engine/api/workflow_test.go +++ b/engine/api/workflow_test.go @@ -1527,14 +1527,19 @@ func Test_putWorkflowShouldNotCallHOOKSIfHookDoesNotChange(t *testing.T) { _, _ = assets.InsertService(t, db, t.Name()+"_HOOKS", sdk.TypeHooks) - u, pass := assets.InsertAdminUser(t, db) key := sdk.RandomString(10) proj := assets.InsertTestProject(t, db, api.Cache, key, key) + + g0 := proj.ProjectGroups[0].Group + + u, jwtLambda := assets.InsertLambdaUser(t, db, &g0) + assets.SetUserGroupAdmin(t, db, g0.ID, u.ID) + pip := sdk.Pipeline{ Name: "pipeline1", ProjectID: proj.ID, } - assert.NoError(t, pipeline.InsertPipeline(db, &pip)) + require.NoError(t, pipeline.InsertPipeline(db, &pip)) wf := sdk.Workflow{ ProjectID: proj.ID, @@ -1596,24 +1601,22 @@ func Test_putWorkflowShouldNotCallHOOKSIfHookDoesNotChange(t *testing.T) { ) // Insert the workflow - vars := map[string]string{ + uri := router.GetRoute(http.MethodPost, api.postWorkflowHandler, map[string]string{ "permProjectKey": proj.Key, - } - uri := router.GetRoute("POST", api.postWorkflowHandler, vars) - test.NotEmpty(t, uri) - req := assets.NewAuthentifiedRequest(t, u, pass, "POST", uri, &wf) + }) + require.NotEmpty(t, uri) + req := assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPost, uri, &wf) w := httptest.NewRecorder() router.Mux.ServeHTTP(w, req) - assert.Equal(t, 201, w.Code) + require.Equal(t, http.StatusCreated, w.Code) // Load the workflow - vars = map[string]string{ + uri = router.GetRoute(http.MethodGet, api.getWorkflowHandler, map[string]string{ "key": proj.Key, "permWorkflowName": wf.Name, - } - uri = router.GetRoute("GET", api.getWorkflowHandler, vars) - test.NotEmpty(t, uri) - req = assets.NewAuthentifiedRequest(t, u, pass, "GET", uri, nil) + }) + require.NotEmpty(t, uri) + req = assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodGet, uri, nil) w = httptest.NewRecorder() router.Mux.ServeHTTP(w, req) @@ -1622,13 +1625,15 @@ func Test_putWorkflowShouldNotCallHOOKSIfHookDoesNotChange(t *testing.T) { // Then call the PUT handler, it should not trigger /task/bulk on hooks service // Update the workflow - uri = router.GetRoute("PUT", api.putWorkflowHandler, vars) - test.NotEmpty(t, uri) - req = assets.NewAuthentifiedRequest(t, u, pass, "PUT", uri, &wf) + uri = router.GetRoute(http.MethodPut, api.putWorkflowHandler, map[string]string{ + "key": proj.Key, + "permWorkflowName": wf.Name, + }) + require.NotEmpty(t, uri) + req = assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPut, uri, &wf) w = httptest.NewRecorder() router.Mux.ServeHTTP(w, req) - assert.Equal(t, 200, w.Code) - + assert.Equal(t, http.StatusOK, w.Code) } func Test_putWorkflowWithDuplicateHooksShouldRaiseAnError(t *testing.T) { @@ -1643,13 +1648,12 @@ func Test_putWorkflowWithDuplicateHooksShouldRaiseAnError(t *testing.T) { Name: "pipeline1", ProjectID: proj.ID, } - assert.NoError(t, pipeline.InsertPipeline(db, &pip)) + require.NoError(t, pipeline.InsertPipeline(db, &pip)) wf := sdk.Workflow{ ProjectID: proj.ID, ProjectKey: proj.Key, Name: sdk.RandomString(10), - Groups: proj.ProjectGroups, WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "root", @@ -1705,23 +1709,21 @@ func Test_putWorkflowWithDuplicateHooksShouldRaiseAnError(t *testing.T) { ) // Insert the workflow - vars := map[string]string{ + uri := router.GetRoute("POST", api.postWorkflowHandler, map[string]string{ "permProjectKey": proj.Key, - } - uri := router.GetRoute("POST", api.postWorkflowHandler, vars) - test.NotEmpty(t, uri) + }) + require.NotEmpty(t, uri) req := assets.NewAuthentifiedRequest(t, u, pass, "POST", uri, &wf) w := httptest.NewRecorder() router.Mux.ServeHTTP(w, req) - assert.Equal(t, 201, w.Code) + require.Equal(t, 201, w.Code) // Load the workflow - vars = map[string]string{ + uri = router.GetRoute("GET", api.getWorkflowHandler, map[string]string{ "key": proj.Key, "permWorkflowName": wf.Name, - } - uri = router.GetRoute("GET", api.getWorkflowHandler, vars) - test.NotEmpty(t, uri) + }) + require.NotEmpty(t, uri) req = assets.NewAuthentifiedRequest(t, u, pass, "GET", uri, nil) w = httptest.NewRecorder() router.Mux.ServeHTTP(w, req) @@ -1730,7 +1732,6 @@ func Test_putWorkflowWithDuplicateHooksShouldRaiseAnError(t *testing.T) { require.NoError(t, json.Unmarshal(w.Body.Bytes(), &wf)) // Then add another hooks with similar properties. It should raise a 400 HTTP Error - wf.WorkflowData.Node.Hooks = append(wf.WorkflowData.Node.Hooks, sdk.NodeHook{ HookModelID: sdk.WebHookModel.ID, @@ -1743,13 +1744,15 @@ func Test_putWorkflowWithDuplicateHooksShouldRaiseAnError(t *testing.T) { ) // Update the workflow - uri = router.GetRoute("PUT", api.putWorkflowHandler, vars) - test.NotEmpty(t, uri) + uri = router.GetRoute("PUT", api.putWorkflowHandler, map[string]string{ + "key": proj.Key, + "permWorkflowName": wf.Name, + }) + require.NotEmpty(t, uri) req = assets.NewAuthentifiedRequest(t, u, pass, "PUT", uri, &wf) w = httptest.NewRecorder() router.Mux.ServeHTTP(w, req) - assert.Equal(t, 400, w.Code) - + require.Equal(t, 400, w.Code) } func Test_getWorkflowsHandler_FilterByRepo(t *testing.T) { @@ -2039,6 +2042,8 @@ func Test_getWorkfloDependencieswHandler(t *testing.T) { ) u, pass := assets.InsertAdminUser(t, db) + localConsumer, err := authentication.LoadConsumerByTypeAndUserID(context.TODO(), db, sdk.ConsumerLocal, u.ID, authentication.LoadConsumerOptions.WithAuthentifiedUser) + require.NoError(t, err) proj := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), sdk.RandomString(10)) vcsServer := sdk.ProjectVCSServerLink{ @@ -2110,7 +2115,7 @@ workflow: eWf, err := exportentities.UnmarshalWorkflow([]byte(workflowS), exportentities.FormatYAML) require.NoError(t, err) - wf, _, err := workflow.ParseAndImport(ctx, db, cache, *proj, nil, eWf, u, workflow.ImportOptions{WorkflowName: "test-env", FromRepository: "from/my-repo"}) + wf, _, err := workflow.ParseAndImport(ctx, db, cache, *proj, nil, eWf, localConsumer, workflow.ImportOptions{WorkflowName: "test-env", FromRepository: "from/my-repo"}) require.NotNil(t, wf) require.NoError(t, err) @@ -2149,7 +2154,7 @@ workflow: eWf2, err := exportentities.UnmarshalWorkflow([]byte(workflowS2), exportentities.FormatYAML) require.NoError(t, err) - wf2, _, err := workflow.ParseAndImport(ctx, db, cache, *proj, nil, eWf2, u, workflow.ImportOptions{WorkflowName: "test-env-2", FromRepository: "from/my-repo-2"}) + wf2, _, err := workflow.ParseAndImport(ctx, db, cache, *proj, nil, eWf2, localConsumer, workflow.ImportOptions{WorkflowName: "test-env-2", FromRepository: "from/my-repo-2"}) require.NotNil(t, wf2) require.NoError(t, err) @@ -2168,5 +2173,4 @@ workflow: require.Equal(t, app.Name, res2.UnlinkedAsCodeDependencies.Applications[0].Name) require.Equal(t, env.ID, res2.UnlinkedAsCodeDependencies.Environments[0].ID) require.Equal(t, env.Name, res2.UnlinkedAsCodeDependencies.Environments[0].Name) - } diff --git a/engine/api/workflowtemplate/instance.go b/engine/api/workflowtemplate/instance.go index fa43f8ba0c..9b39bc8d61 100644 --- a/engine/api/workflowtemplate/instance.go +++ b/engine/api/workflowtemplate/instance.go @@ -139,8 +139,9 @@ func CheckAndExecuteTemplate(ctx context.Context, db *gorp.DbMap, store cache.St return allMsgs, nil, sdk.WrapError(err, "error with loading group named %q", groupName) } + // Check if the template can be used by the current consumer var groupPermissionValid bool - if consumer.Admin() || consumer.Maintainer() { + if consumer.Maintainer() { groupPermissionValid = true } else if grp.ID == group.SharedInfraGroup.ID { groupPermissionValid = true @@ -157,6 +158,14 @@ func CheckAndExecuteTemplate(ctx context.Context, db *gorp.DbMap, store cache.St return allMsgs, nil, sdk.NewErrorFrom(sdk.ErrWrongRequest, "could not find given workflow template") } + // Check that the template is allowed for target project + if grp.ID != group.SharedInfraGroup.ID { + gp := p.ProjectGroups.GetByGroupID(grp.ID) + if gp == nil || gp.Permission < sdk.PermissionReadExecute { + return allMsgs, nil, sdk.NewErrorFrom(sdk.ErrForbidden, "could not use workflow template %q, missing RX permission for group %q on project %q", templateSlug, grp.Name, p.Key) + } + } + wt, err := LoadBySlugAndGroupID(ctx, tx, templateSlug, grp.ID) if err != nil { return allMsgs, nil, sdk.NewErrorFrom(err, "could not find a template with slug %s in group %s", templateSlug, grp.Name) diff --git a/engine/api/workflowtemplate/instance_test.go b/engine/api/workflowtemplate/instance_test.go index c401f00e49..820b08b333 100644 --- a/engine/api/workflowtemplate/instance_test.go +++ b/engine/api/workflowtemplate/instance_test.go @@ -175,7 +175,10 @@ func TestUpdateTemplateInstanceWithWorkflow(t *testing.T) { proj := assets.InsertTestProject(t, db, cache, sdk.RandomString(10), sdk.RandomString(10)) grp := proj.ProjectGroups[0].Group + usr, _ := assets.InsertLambdaUser(t, db, &grp) + assets.SetUserGroupAdmin(t, db, grp.ID, usr.ID) + consumer, err := authentication.LoadConsumerByTypeAndUserID(context.TODO(), db, sdk.ConsumerLocal, usr.ID, authentication.LoadConsumerOptions.WithAuthentifiedUser, authentication.LoadConsumerOptions.WithConsumerGroups, diff --git a/sdk/workflow_node.go b/sdk/workflow_node.go index 82db8ccb0d..5289dbda22 100644 --- a/sdk/workflow_node.go +++ b/sdk/workflow_node.go @@ -26,7 +26,7 @@ type Node struct { OutGoingHookContext *NodeOutGoingHook `json:"outgoing_hook" db:"-"` JoinContext []NodeJoin `json:"parents" db:"-"` Hooks []NodeHook `json:"hooks" db:"-"` - Groups []GroupPermission `json:"groups,omitempty" db:"-"` + Groups GroupPermissions `json:"groups,omitempty" db:"-"` } func (n Node) GetHook(UUID string) *NodeHook {