diff --git a/internal/ent/generated/groupmembership/groupmembership.go b/internal/ent/generated/groupmembership/groupmembership.go index a8ab8ee2..e52138b8 100644 --- a/internal/ent/generated/groupmembership/groupmembership.go +++ b/internal/ent/generated/groupmembership/groupmembership.go @@ -121,7 +121,7 @@ func ValidColumn(column string) bool { // // import _ "github.com/theopenlane/core/internal/ent/generated/runtime" var ( - Hooks [4]ent.Hook + Hooks [5]ent.Hook Interceptors [1]ent.Interceptor Policy ent.Policy // DefaultCreatedAt holds the default value on creation for the "created_at" field. diff --git a/internal/ent/generated/orgmembership/orgmembership.go b/internal/ent/generated/orgmembership/orgmembership.go index 58fddae1..7953aff1 100644 --- a/internal/ent/generated/orgmembership/orgmembership.go +++ b/internal/ent/generated/orgmembership/orgmembership.go @@ -101,7 +101,7 @@ func ValidColumn(column string) bool { // // import _ "github.com/theopenlane/core/internal/ent/generated/runtime" var ( - Hooks [6]ent.Hook + Hooks [7]ent.Hook Interceptors [3]ent.Interceptor Policy ent.Policy // DefaultCreatedAt holds the default value on creation for the "created_at" field. diff --git a/internal/ent/generated/programmembership/programmembership.go b/internal/ent/generated/programmembership/programmembership.go index b7bc86fd..2a3e1999 100644 --- a/internal/ent/generated/programmembership/programmembership.go +++ b/internal/ent/generated/programmembership/programmembership.go @@ -108,7 +108,7 @@ func ValidColumn(column string) bool { // // import _ "github.com/theopenlane/core/internal/ent/generated/runtime" var ( - Hooks [4]ent.Hook + Hooks [5]ent.Hook Interceptors [1]ent.Interceptor Policy ent.Policy // DefaultCreatedAt holds the default value on creation for the "created_at" field. diff --git a/internal/ent/generated/runtime/runtime.go b/internal/ent/generated/runtime/runtime.go index 7a59d286..daaa593c 100644 --- a/internal/ent/generated/runtime/runtime.go +++ b/internal/ent/generated/runtime/runtime.go @@ -1329,6 +1329,8 @@ func init() { groupmembership.Hooks[2] = groupmembershipMixinHooks2[0] groupmembership.Hooks[3] = groupmembershipHooks[0] + + groupmembership.Hooks[4] = groupmembershipHooks[1] groupmembershipMixinInters2 := groupmembershipMixin[2].Interceptors() groupmembership.Interceptors[0] = groupmembershipMixinInters2[0] groupmembershipMixinFields0 := groupmembershipMixin[0].Fields() @@ -1997,6 +1999,8 @@ func init() { orgmembership.Hooks[4] = orgmembershipHooks[1] orgmembership.Hooks[5] = orgmembershipHooks[2] + + orgmembership.Hooks[6] = orgmembershipHooks[3] orgmembershipMixinInters2 := orgmembershipMixin[2].Interceptors() orgmembershipInters := schema.OrgMembership{}.Interceptors() orgmembership.Interceptors[0] = orgmembershipMixinInters2[0] @@ -2775,6 +2779,8 @@ func init() { programmembership.Hooks[2] = programmembershipMixinHooks2[0] programmembership.Hooks[3] = programmembershipHooks[0] + + programmembership.Hooks[4] = programmembershipHooks[1] programmembershipMixinInters2 := programmembershipMixin[2].Interceptors() programmembership.Interceptors[0] = programmembershipMixinInters2[0] programmembershipMixinFields0 := programmembershipMixin[0].Fields() diff --git a/internal/ent/hooks/errors.go b/internal/ent/hooks/errors.go index 2000c520..ee7e4a03 100644 --- a/internal/ent/hooks/errors.go +++ b/internal/ent/hooks/errors.go @@ -14,6 +14,8 @@ var ( ErrPersonalOrgsNoChildren = errors.New("personal organizations are not allowed to have child organizations") // ErrPersonalOrgsNoMembers is returned when personal org attempts to add members ErrPersonalOrgsNoMembers = errors.New("personal organizations are not allowed to have members other than the owner") + // ErrOrgOwnerCannotBeDeleted is returned when an org owner is attempted to be deleted + ErrOrgOwnerCannotBeDeleted = errors.New("organization owner cannot be deleted, it must be transferred to a new owner first") // ErrPersonalOrgsNoUser is returned when personal org has no user associated, so no permissions can be added ErrPersonalOrgsNoUser = errors.New("personal organizations missing user association") // ErrUserNotInOrg is returned when a user is not a member of an organization when trying to add them to a group diff --git a/internal/ent/hooks/memberhelpers.go b/internal/ent/hooks/memberhelpers.go new file mode 100644 index 00000000..eac0c4ea --- /dev/null +++ b/internal/ent/hooks/memberhelpers.go @@ -0,0 +1,162 @@ +package hooks + +import ( + "context" + "strings" + + "entgo.io/ent" + "entgo.io/ent/dialect/sql" + "github.com/99designs/gqlgen/graphql" + "github.com/rs/zerolog/log" + "github.com/theopenlane/iam/auth" + + "github.com/theopenlane/core/internal/ent/generated" + "github.com/theopenlane/core/internal/ent/generated/privacy" +) + +// MutationMember is an interface that can be implemented by a member mutation to get IDs +type MutationMember interface { + UserIDs() []string + UserID() (string, bool) + ID() (string, bool) + IDs(ctx context.Context) ([]string, error) +} + +// HookMembershipSelf is a hook that runs on membership mutations +// to prevent users from updating their own membership +func HookMembershipSelf(table string) ent.Hook { + return func(next ent.Mutator) ent.Mutator { + return ent.MutateFunc(func(ctx context.Context, m ent.Mutation) (ent.Value, error) { + // bypass privacy check if the context allows it + if _, allow := privacy.DecisionFromContext(ctx); allow { + return next.Mutate(ctx, m) + } + + mutationMember, ok := m.(MutationMember) + if !ok { + return next.Mutate(ctx, m) + } + + // check if group member is the authenticated user + userID, err := auth.GetUserIDFromContext(ctx) + if err != nil { + return nil, err + } + + if m.Op().Is(ent.OpCreate) { + // Only run this hook on membership mutations + // you can create a group/program/etc with yourself as a member + if !checkMutation(ctx) { + return next.Mutate(ctx, m) + } + + if err := createMembershipCheck(mutationMember, userID); err != nil { + return nil, err + } + + return next.Mutate(ctx, m) + } + + // if its not a create, check for updates with the user instead. This includes + // update and delete operations + if err := updateMembershipCheck(ctx, mutationMember, table, userID); err != nil { + return nil, err + } + + return next.Mutate(ctx, m) + }) + } +} + +// createMembershipCheck is a helper function to check if a user is trying to add themselves to a membership +func createMembershipCheck(m MutationMember, actorID string) error { + userIds := m.UserIDs() + if len(userIds) == 0 { + userID, ok := m.UserID() + if !ok { + return nil + } + + userIds = append(userIds, userID) + } + + for _, userID := range userIds { + if userID == actorID { + log.Error().Msg("user cannot add themselves to a membership") + + return generated.ErrPermissionDenied + } + } + + return nil +} + +// updateMembershipCheck is a helper function to check if a user is trying to update themselves in a membership +func updateMembershipCheck(ctx context.Context, m MutationMember, table string, actorID string) error { + memberIDs := getMutationMemberIDs(ctx, m) + if len(memberIDs) == 0 { + return nil + } + + query := "SELECT user_id FROM " + table + " WHERE id in ($1)" + + var rows sql.Rows + if err := generated.FromContext(ctx).Driver().Query(ctx, query, []any{strings.Join(memberIDs, ",")}, &rows); err != nil { + log.Error().Err(err).Msg("failed to get user ID from membership") + + return err + } + + defer rows.Close() + + if rows.Next() { + var userID string + + if err := rows.Scan(&userID); err != nil { + log.Error().Err(err).Msg("failed to scan user ID from membership") + + return err + } + + if userID == actorID { + log.Error().Msg("user cannot update their own membership") + + return generated.ErrPermissionDenied + } + } + + return nil +} + +// getMutationMemberIDs is a helper function to get the member IDs from a mutation +// this can be used for group, program, and org membership mutations because +// they all implement the MutationMember interface +func getMutationMemberIDs(ctx context.Context, m MutationMember) []string { + id, ok := m.ID() + if ok { + return []string{id} + } + + ids, err := m.IDs(ctx) + if err == nil && len(ids) > 0 { + return ids + } + + return ids +} + +func checkMutation(ctx context.Context) bool { + rootFieldCtx := graphql.GetRootFieldContext(ctx) + + // skip if not a graphql mutation + if rootFieldCtx == nil { + return false + } + + // Check if the mutation is a membership mutation + if strings.Contains(rootFieldCtx.Object, "Membership") { + return true + } + + return false +} diff --git a/internal/ent/hooks/orgmembers.go b/internal/ent/hooks/orgmembers.go index e895693c..cbc8a646 100644 --- a/internal/ent/hooks/orgmembers.go +++ b/internal/ent/hooks/orgmembers.go @@ -115,6 +115,10 @@ func HookOrgMembersDelete() ent.Hook { return nil, err } + if orgMembership.Role == enums.RoleOwner { + return nil, ErrOrgOwnerCannotBeDeleted + } + // execute the delete operation retValue, err := next.Mutate(ctx, m) if err != nil { diff --git a/internal/ent/schema/groupmembership.go b/internal/ent/schema/groupmembership.go index 0f936fa1..c4f332aa 100644 --- a/internal/ent/schema/groupmembership.go +++ b/internal/ent/schema/groupmembership.go @@ -89,6 +89,7 @@ func (GroupMembership) Mixin() []ent.Mixin { func (GroupMembership) Hooks() []ent.Hook { return []ent.Hook{ hooks.HookGroupMembers(), + hooks.HookMembershipSelf("group_memberships"), } } diff --git a/internal/ent/schema/orgmembership.go b/internal/ent/schema/orgmembership.go index d02fcf7a..ed217015 100644 --- a/internal/ent/schema/orgmembership.go +++ b/internal/ent/schema/orgmembership.go @@ -102,6 +102,7 @@ func (OrgMembership) Hooks() []ent.Hook { hooks.HookUpdateManagedGroups(), hooks.HookOrgMembers(), hooks.HookOrgMembersDelete(), + hooks.HookMembershipSelf("org_memberships"), } } diff --git a/internal/ent/schema/programmembership.go b/internal/ent/schema/programmembership.go index 2c816e56..c700d89a 100644 --- a/internal/ent/schema/programmembership.go +++ b/internal/ent/schema/programmembership.go @@ -88,6 +88,7 @@ func (ProgramMembership) Mixin() []ent.Mixin { func (ProgramMembership) Hooks() []ent.Hook { return []ent.Hook{ hooks.HookProgramMembers(), + hooks.HookMembershipSelf("program_memberships"), } } diff --git a/internal/graphapi/groupextended.resolvers.go b/internal/graphapi/groupextended.resolvers.go index 5ed183fa..bc1bb63c 100644 --- a/internal/graphapi/groupextended.resolvers.go +++ b/internal/graphapi/groupextended.resolvers.go @@ -133,10 +133,7 @@ func (r *mutationResolver) CreateGroupByClone(ctx context.Context, groupInput ge return nil, rout.NewMissingRequiredFieldError("owner_id") } - // TODO (sfunk): we should generate this code in the future - // if we have a group to inherit permissions from, get the group with permissions - // and append the permissions to the group input - if inheritGroupPermissions != nil && *inheritGroupPermissions != "" { + if inheritGroupPermissions != nil { groupWithPermissions, err := getGroupByIDWithPermissionsEdges(ctx, inheritGroupPermissions) if err != nil { return nil, parseRequestError(err, action{action: ActionCreate, object: "group"}) @@ -224,8 +221,7 @@ func (r *mutationResolver) CreateGroupByClone(ctx context.Context, groupInput ge return nil, parseRequestError(err, action{action: ActionCreate, object: "group"}) } - // if we have a group to clone members from, get the members and add them to the new group - if cloneGroupMembers != nil && *cloneGroupMembers != "" { + if cloneGroupMembers != nil { existingMembers, err := res.Members(ctx) if err != nil { return nil, parseRequestError(err, action{action: ActionCreate, object: "group"}) @@ -236,7 +232,6 @@ func (r *mutationResolver) CreateGroupByClone(ctx context.Context, groupInput ge } } - // retrieve the final result with all members finalResult, err := withTransactionalMutation(ctx).Group. Query(). WithMembers(). @@ -292,6 +287,10 @@ func (r *updateGroupInputResolver) AddGroupMembers(ctx context.Context, obj *gen // RemoveGroupMembers is the resolver for the removeGroupMembers field. func (r *updateGroupInputResolver) RemoveGroupMembers(ctx context.Context, obj *generated.UpdateGroupInput, data []string) error { + if len(data) == 0 { + return nil + } + opCtx := graphql.GetOperationContext(ctx) groupID, ok := opCtx.Variables["updateGroupId"] if !ok { @@ -302,14 +301,13 @@ func (r *updateGroupInputResolver) RemoveGroupMembers(ctx context.Context, obj * c := withTransactionalMutation(ctx) - _, err := c.GroupMembership.Delete(). - Where( - groupmembership.GroupID(groupID.(string)), - groupmembership.IDIn(data...), - ). - Exec(ctx) - if err != nil { - return err + for _, id := range data { + if err := c.GroupMembership.DeleteOneID(id). + Where(groupmembership.GroupID(groupID.(string))). + Exec(ctx); err != nil { + + return err + } } return nil @@ -354,11 +352,10 @@ func (r *updateGroupInputResolver) UpdateGroupSettings(ctx context.Context, obj // InheritGroupPermissions is the resolver for the inheritGroupPermissions field. func (r *updateGroupInputResolver) InheritGroupPermissions(ctx context.Context, obj *generated.UpdateGroupInput, data *string) error { // if data is nil, we don't need to do anything - if data == nil || *data == "" { + if data == nil { return nil } - // TODO (sfunk): we should generate this code in the future based off the group permissions mixin groupWithPermissions, err := getGroupByIDWithPermissionsEdges(ctx, data) if err != nil { return parseRequestError(err, action{action: ActionCreate, object: "group"}) diff --git a/internal/graphapi/groupmembers_test.go b/internal/graphapi/groupmembers_test.go index a8147ea9..f237f60f 100644 --- a/internal/graphapi/groupmembers_test.go +++ b/internal/graphapi/groupmembers_test.go @@ -154,6 +154,15 @@ func (suite *GraphTestSuite) TestMutationCreateGroupMembers() { client: suite.client.apiWithPAT, ctx: context.Background(), }, + { + name: "cannot add self to group", + groupID: group1.ID, + userID: adminUser.UserInfo.ID, + role: enums.RoleAdmin, + client: suite.client.api, + ctx: adminUser.UserCtx, + errMsg: notAuthorizedErrorMsg, + }, { name: "add member, no access", groupID: group1.ID, @@ -246,46 +255,75 @@ func (suite *GraphTestSuite) TestMutationCreateGroupMembers() { func (suite *GraphTestSuite) TestMutationUpdateGroupMembers() { t := suite.T() - gm := (&GroupMemberBuilder{client: suite.client}).MustNew(testUser1.UserCtx, t) + gm := (&GroupMemberBuilder{client: suite.client, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t) + + // get all group members so we know the id of the test user group member + groupMembers, err := suite.client.api.GetGroupMembersByGroupID(testUser1.UserCtx, &openlaneclient.GroupMembershipWhereInput{ + GroupID: &testUser1.GroupID, + }) + + require.NoError(t, err) + + testUser1GroupMember := "" + for _, gm := range groupMembers.GroupMemberships.Edges { + if gm.Node.UserID == testUser1.UserInfo.ID { + testUser1GroupMember = gm.Node.ID + break + } + } testCases := []struct { - name string - role enums.Role - client *openlaneclient.OpenlaneClient - ctx context.Context - errMsg string + name string + groupMemberID string + role enums.Role + client *openlaneclient.OpenlaneClient + ctx context.Context + errMsg string }{ { - name: "happy path, update to admin from member", - role: enums.RoleAdmin, - client: suite.client.api, - ctx: testUser1.UserCtx, + name: "happy path, update to admin from member", + groupMemberID: gm.ID, + role: enums.RoleAdmin, + client: suite.client.api, + ctx: testUser1.UserCtx, }, { - name: "happy path, update to member from admin using api token", - role: enums.RoleMember, - client: suite.client.apiWithToken, - ctx: context.Background(), + name: "update self from admin to member, not allowed", + groupMemberID: testUser1GroupMember, + role: enums.RoleMember, + client: suite.client.api, + ctx: testUser1.UserCtx, + errMsg: notAuthorizedErrorMsg, }, { - name: "happy path, update to admin from member using personal access token", - role: enums.RoleAdmin, - client: suite.client.apiWithPAT, - ctx: context.Background(), + name: "happy path, update to member from admin using api token", + groupMemberID: gm.ID, + role: enums.RoleMember, + client: suite.client.apiWithToken, + ctx: context.Background(), }, { - name: "invalid role", - role: enums.RoleInvalid, - client: suite.client.api, - ctx: testUser1.UserCtx, - errMsg: "not a valid GroupMembershipRole", + name: "happy path, update to admin from member using personal access token", + groupMemberID: gm.ID, + role: enums.RoleAdmin, + client: suite.client.apiWithPAT, + ctx: context.Background(), }, { - name: "no access", - role: enums.RoleMember, - client: suite.client.api, - ctx: viewOnlyUser.UserCtx, - errMsg: notAuthorizedErrorMsg, + name: "invalid role", + groupMemberID: gm.ID, + role: enums.RoleInvalid, + client: suite.client.api, + ctx: testUser1.UserCtx, + errMsg: "not a valid GroupMembershipRole", + }, + { + name: "no access", + groupMemberID: gm.ID, + role: enums.RoleMember, + client: suite.client.api, + ctx: viewOnlyUser.UserCtx, + errMsg: notAuthorizedErrorMsg, }, } @@ -296,7 +334,7 @@ func (suite *GraphTestSuite) TestMutationUpdateGroupMembers() { Role: &role, } - resp, err := tc.client.UpdateUserRoleInGroup(tc.ctx, gm.ID, input) + resp, err := tc.client.UpdateUserRoleInGroup(tc.ctx, tc.groupMemberID, input) if tc.errMsg != "" { require.Error(t, err) @@ -316,9 +354,24 @@ func (suite *GraphTestSuite) TestMutationUpdateGroupMembers() { func (suite *GraphTestSuite) TestMutationDeleteGroupMembers() { t := suite.T() - gm1 := (&GroupMemberBuilder{client: suite.client}).MustNew(testUser1.UserCtx, t) - gm2 := (&GroupMemberBuilder{client: suite.client}).MustNew(testUser1.UserCtx, t) - gm3 := (&GroupMemberBuilder{client: suite.client}).MustNew(testUser1.UserCtx, t) + gm1 := (&GroupMemberBuilder{client: suite.client, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t) + gm2 := (&GroupMemberBuilder{client: suite.client, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t) + gm3 := (&GroupMemberBuilder{client: suite.client, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t) + + // get all group members so we know the id of the test user group member + groupMembers, err := suite.client.api.GetGroupMembersByGroupID(testUser1.UserCtx, &openlaneclient.GroupMembershipWhereInput{ + GroupID: &testUser1.GroupID, + }) + + require.NoError(t, err) + + testUser1GroupMember := "" + for _, gm := range groupMembers.GroupMemberships.Edges { + if gm.Node.UserID == testUser1.UserInfo.ID { + testUser1GroupMember = gm.Node.ID + break + } + } testCases := []struct { name string @@ -334,6 +387,13 @@ func (suite *GraphTestSuite) TestMutationDeleteGroupMembers() { ctx: viewOnlyUser.UserCtx, expectedErr: notAuthorizedErrorMsg, }, + { + name: "not allowed to delete self", + idToDelete: testUser1GroupMember, + client: suite.client.api, + ctx: testUser1.UserCtx, + expectedErr: notAuthorizedErrorMsg, + }, { name: "not allowed to delete, not found", idToDelete: gm1.ID, diff --git a/internal/graphapi/orgmembers_test.go b/internal/graphapi/orgmembers_test.go index 533ec0ab..773400f7 100644 --- a/internal/graphapi/orgmembers_test.go +++ b/internal/graphapi/orgmembers_test.go @@ -156,6 +156,14 @@ func (suite *GraphTestSuite) TestMutationCreateOrgMembers() { ctx: userCtx, errMsg: "already exists", }, + { + name: "cannot add self to organization", + orgID: org1.ID, + userID: testUser2.ID, + role: enums.RoleMember, + ctx: testUser2.UserCtx, + errMsg: notAuthorizedErrorMsg, + }, { name: "add user to personal org not allowed", orgID: testUser1.PersonalOrgID, @@ -230,27 +238,52 @@ func (suite *GraphTestSuite) TestMutationUpdateOrgMembers() { om := (&OrgMemberBuilder{client: suite.client, OrgID: testUser1.OrganizationID}).MustNew(testUser1.UserCtx, t) + orgMembers, err := suite.client.api.GetOrgMembersByOrgID(testUser1.UserCtx, &openlaneclient.OrgMembershipWhereInput{ + OrganizationID: &testUser1.OrganizationID, + }) + require.NoError(t, err) + + testUser1OrgMember := "" + + for _, edge := range orgMembers.OrgMemberships.Edges { + if edge.Node.UserID == testUser1.ID { + testUser1OrgMember = edge.Node.ID + break + } + } + testCases := []struct { - name string - role enums.Role - errMsg string + name string + orgMemberID string + role enums.Role + errMsg string }{ { - name: "happy path, update to admin from member", - role: enums.RoleAdmin, + name: "happy path, update to admin from member", + orgMemberID: om.ID, + role: enums.RoleAdmin, }, { - name: "happy path, update to member from admin", - role: enums.RoleMember, + name: "happy path, update to member from admin", + orgMemberID: om.ID, + role: enums.RoleMember, }, { - name: "update to same role", - role: enums.RoleMember, + name: "update to same role", + orgMemberID: om.ID, + role: enums.RoleMember, }, { - name: "invalid role", - role: enums.RoleInvalid, - errMsg: "not a valid OrgMembershipRole", + name: "update self from admin to member, not allowed", + orgMemberID: testUser1OrgMember, + role: enums.RoleMember, + errMsg: notAuthorizedErrorMsg, + }, + { + name: "invalid role", + orgMemberID: om.ID, + role: enums.RoleInvalid, + errMsg: "not a valid OrgMembershipRole", }, } @@ -260,7 +293,7 @@ func (suite *GraphTestSuite) TestMutationUpdateOrgMembers() { Role: &tc.role, } - resp, err := suite.client.api.UpdateUserRoleInOrg(testUser1.UserCtx, om.ID, input) + resp, err := suite.client.api.UpdateUserRoleInOrg(testUser1.UserCtx, tc.orgMemberID, input) if tc.errMsg != "" { require.Error(t, err) @@ -303,6 +336,28 @@ func (suite *GraphTestSuite) TestMutationDeleteOrgMembers() { }) require.NoError(t, err) + + // cant remove self from org and owners cannot be removed + orgMembers, err := suite.client.api.GetOrgMembersByOrgID(testUser1.UserCtx, &openlaneclient.OrgMembershipWhereInput{ + OrganizationID: &testUser1.OrganizationID, + }) + require.NoError(t, err) + + for _, edge := range orgMembers.OrgMemberships.Edges { + // cannot delete self + if edge.Node.UserID == adminUser.ID { + _, err := suite.client.api.RemoveUserFromOrg(adminUser.UserCtx, edge.Node.ID) + require.Error(t, err) + + } + + // organization owner cannot be deleted + if edge.Node.UserID == testUser1.ID { + _, err = suite.client.api.RemoveUserFromOrg(adminUser.UserCtx, edge.Node.ID) + require.Error(t, err) + break + } + } } func (suite *GraphTestSuite) assertDefaultOrgUpdate(ctx context.Context, userID, orgID string, isEqual bool) { diff --git a/internal/graphapi/program_test.go b/internal/graphapi/program_test.go index c8986e56..6fc41750 100644 --- a/internal/graphapi/program_test.go +++ b/internal/graphapi/program_test.go @@ -410,6 +410,19 @@ func (suite *GraphTestSuite) TestMutationUpdateProgram() { program := (&ProgramBuilder{client: suite.client}).MustNew(testUser1.UserCtx, t) + programMembers, err := suite.client.api.GetProgramMembersByProgramID(testUser1.UserCtx, &openlaneclient.ProgramMembershipWhereInput{ + ProgramID: &program.ID, + }) + + require.NoError(t, err) + + testUserProgramMemberID := "" + for _, pm := range programMembers.ProgramMemberships.Edges { + if pm.Node.UserID == testUser1.ID { + testUserProgramMemberID = pm.Node.ID + } + } + // create program user to remove programUser := suite.userBuilder(context.Background()) (&OrgMemberBuilder{client: suite.client, UserID: programUser.ID, OrgID: testUser1.OrganizationID}).MustNew(testUser1.UserCtx, t) @@ -478,7 +491,7 @@ func (suite *GraphTestSuite) TestMutationUpdateProgram() { ctx: testUser1.UserCtx, }, { - name: "happy path, update multiple fields", + name: "happy path, update multiple fields using pat", request: openlaneclient.UpdateProgramInput{ Status: &enums.ProgramStatusReadyForAuditor, EndDate: lo.ToPtr(time.Now().AddDate(0, 0, 30)), @@ -489,6 +502,40 @@ func (suite *GraphTestSuite) TestMutationUpdateProgram() { client: suite.client.apiWithPAT, ctx: context.Background(), }, + { + name: "remove program member, cannot remove self", + request: openlaneclient.UpdateProgramInput{ + RemoveProgramMembers: []string{testUserProgramMemberID}, + }, + client: suite.client.api, + ctx: testUser1.UserCtx, + expectedErr: notAuthorizedErrorMsg, + }, + { + name: "add program member, cannot add self", + request: openlaneclient.UpdateProgramInput{ + AddProgramMembers: []*openlaneclient.CreateProgramMembershipInput{ + { + UserID: adminUser.ID, + }, + }, + }, + client: suite.client.api, + ctx: adminUser.UserCtx, + expectedErr: notAuthorizedErrorMsg, + }, + { + name: "add program member, can add another user", + request: openlaneclient.UpdateProgramInput{ + AddProgramMembers: []*openlaneclient.CreateProgramMembershipInput{ + { + UserID: adminUser.ID, + }, + }, + }, + client: suite.client.api, + ctx: testUser1.UserCtx, + }, { name: "happy path, remove program member", request: openlaneclient.UpdateProgramInput{ @@ -646,11 +693,20 @@ func (suite *GraphTestSuite) TestMutationUpdateProgram() { assert.Equal(t, program.ID, res.Program.ID) } - // member was removed, ensure there is only one member left + if len(tc.request.AddProgramMembers) > 0 { + require.Len(t, resp.UpdateProgram.Program.Members, 3) + + // it should have the owner and the admin user and the other user added in the test setup + require.Equal(t, testUser1.ID, resp.UpdateProgram.Program.Members[0].User.ID) + require.Equal(t, programUser.ID, resp.UpdateProgram.Program.Members[1].User.ID) + require.Equal(t, adminUser.ID, resp.UpdateProgram.Program.Members[2].User.ID) + } + + // member was removed, ensure there are two members left if len(tc.request.RemoveProgramMembers) > 0 { - require.Len(t, resp.UpdateProgram.Program.Members, 1) + require.Len(t, resp.UpdateProgram.Program.Members, 2) - // it should only be the owner left + // it should have the owner and the admin user require.Equal(t, testUser1.ID, resp.UpdateProgram.Program.Members[0].User.ID) } }) diff --git a/internal/graphapi/programextended.resolvers.go b/internal/graphapi/programextended.resolvers.go index fc4964d5..7d1f7adc 100644 --- a/internal/graphapi/programextended.resolvers.go +++ b/internal/graphapi/programextended.resolvers.go @@ -211,6 +211,10 @@ func (r *updateProgramInputResolver) AddProgramMembers(ctx context.Context, obj // RemoveProgramMembers is the resolver for the removeProgramMembers field. func (r *updateProgramInputResolver) RemoveProgramMembers(ctx context.Context, obj *generated.UpdateProgramInput, data []string) error { + if len(data) == 0 { + return nil + } + opCtx := graphql.GetOperationContext(ctx) programID, ok := opCtx.Variables["updateProgramId"] if !ok { @@ -221,14 +225,13 @@ func (r *updateProgramInputResolver) RemoveProgramMembers(ctx context.Context, o c := withTransactionalMutation(ctx) - _, err := c.ProgramMembership.Delete(). - Where( - programmembership.ProgramID(programID.(string)), - programmembership.IDIn(data...), - ). - Exec(ctx) - if err != nil { - return err + for _, id := range data { + if err := c.ProgramMembership.DeleteOneID(id). + Where(programmembership.ProgramID(programID.(string))). + Exec(ctx); err != nil { + + return err + } } return nil diff --git a/internal/graphapi/programmembers_test.go b/internal/graphapi/programmembers_test.go new file mode 100644 index 00000000..2ac82b84 --- /dev/null +++ b/internal/graphapi/programmembers_test.go @@ -0,0 +1,229 @@ +package graphapi_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/theopenlane/core/pkg/enums" + "github.com/theopenlane/core/pkg/openlaneclient" +) + +func (suite *GraphTestSuite) TestMutationCreateProgramMembers() { + t := suite.T() + + program := (&ProgramBuilder{client: suite.client}).MustNew(testUser1.UserCtx, t) + + orgMember1 := (&OrgMemberBuilder{client: suite.client, OrgID: testUser1.OrganizationID}).MustNew(testUser1.UserCtx, t) + orgMember2 := (&OrgMemberBuilder{client: suite.client, OrgID: testUser1.OrganizationID}).MustNew(testUser1.UserCtx, t) + orgMember3 := (&OrgMemberBuilder{client: suite.client, OrgID: testUser1.OrganizationID}).MustNew(testUser1.UserCtx, t) + + testCases := []struct { + name string + programID string + userID string + role enums.Role + client *openlaneclient.OpenlaneClient + ctx context.Context + errMsg string + }{ + { + name: "happy path, add admin", + programID: program.ID, + userID: orgMember1.UserID, + role: enums.RoleAdmin, + client: suite.client.api, + ctx: testUser1.UserCtx, + }, + { + name: "happy path, add member using personal access token", + programID: program.ID, + userID: orgMember3.UserID, + role: enums.RoleMember, + client: suite.client.apiWithPAT, + ctx: context.Background(), + }, + { + name: "cannot add self to program", + programID: program.ID, + userID: adminUser.UserInfo.ID, + role: enums.RoleAdmin, + client: suite.client.api, + ctx: adminUser.UserCtx, + errMsg: notAuthorizedErrorMsg, + }, + { + name: "add member, no access", + programID: program.ID, + userID: orgMember2.UserID, + role: enums.RoleMember, + client: suite.client.api, + ctx: viewOnlyUser.UserCtx, + errMsg: notAuthorizedErrorMsg, + }, + { + name: "owner relation not valid for programs", + programID: program.ID, + userID: orgMember2.UserID, + role: enums.RoleOwner, + client: suite.client.api, + ctx: testUser1.UserCtx, + errMsg: "OWNER is not a valid ProgramMembershipRole", + }, + { + name: "duplicate user, different role", + programID: program.ID, + userID: orgMember1.UserID, + role: enums.RoleMember, + client: suite.client.api, + ctx: testUser1.UserCtx, + errMsg: "already exists", + }, + { + name: "invalid user", + programID: program.ID, + userID: "not-a-valid-user-id", + role: enums.RoleMember, + client: suite.client.api, + ctx: testUser1.UserCtx, + errMsg: "user not in organization", + }, + { + name: "invalid program", + programID: "not-a-valid-program-id", + userID: orgMember1.UserID, + role: enums.RoleMember, + client: suite.client.api, + ctx: testUser1.UserCtx, + errMsg: notAuthorizedErrorMsg, + }, + { + name: "invalid role", + programID: program.ID, + userID: orgMember1.UserID, + role: enums.RoleInvalid, + client: suite.client.api, + ctx: testUser1.UserCtx, + errMsg: "not a valid ProgramMembershipRole", + }, + } + + for _, tc := range testCases { + t.Run("Create "+tc.name, func(t *testing.T) { + role := tc.role + input := openlaneclient.CreateProgramMembershipInput{ + ProgramID: tc.programID, + UserID: tc.userID, + Role: &role, + } + + resp, err := tc.client.AddUserToProgramWithRole(tc.ctx, input) + + if tc.errMsg != "" { + require.Error(t, err) + assert.ErrorContains(t, err, tc.errMsg) + + return + } + + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.CreateProgramMembership) + assert.Equal(t, tc.userID, resp.CreateProgramMembership.ProgramMembership.UserID) + assert.Equal(t, tc.programID, resp.CreateProgramMembership.ProgramMembership.ProgramID) + assert.Equal(t, tc.role, resp.CreateProgramMembership.ProgramMembership.Role) + }) + } +} + +func (suite *GraphTestSuite) TestMutationUpdateProgramMembers() { + t := suite.T() + + pm := (&ProgramMemberBuilder{client: suite.client}).MustNew(testUser1.UserCtx, t) + + // get all program members so we know the id of the test user program member + programMembers, err := suite.client.api.GetProgramMembersByProgramID(testUser1.UserCtx, &openlaneclient.ProgramMembershipWhereInput{ + ProgramID: &pm.ProgramID, + }) + require.NoError(t, err) + + testUser1ProgramMember := "" + for _, pm := range programMembers.ProgramMemberships.Edges { + if pm.Node.UserID == testUser1.UserInfo.ID { + testUser1ProgramMember = pm.Node.ID + break + } + } + + testCases := []struct { + name string + programMemberID string + role enums.Role + client *openlaneclient.OpenlaneClient + ctx context.Context + errMsg string + }{ + { + name: "happy path, update to admin from member", + programMemberID: pm.ID, + role: enums.RoleAdmin, + client: suite.client.api, + ctx: testUser1.UserCtx, + }, + { + name: "update self from admin to member, not allowed", + programMemberID: testUser1ProgramMember, + role: enums.RoleMember, + client: suite.client.api, + ctx: testUser1.UserCtx, + errMsg: notAuthorizedErrorMsg, + }, + { + name: "happy path, update to admin from member using personal access token", + programMemberID: pm.ID, + role: enums.RoleAdmin, + client: suite.client.apiWithPAT, + ctx: context.Background(), + }, + { + name: "invalid role", + programMemberID: pm.ID, + role: enums.RoleInvalid, + client: suite.client.api, + ctx: testUser1.UserCtx, + errMsg: "not a valid ProgramMembershipRole", + }, + { + name: "no access", + programMemberID: pm.ID, + role: enums.RoleMember, + client: suite.client.api, + ctx: viewOnlyUser.UserCtx, + errMsg: notFoundErrorMsg, + }, + } + + for _, tc := range testCases { + t.Run("Update "+tc.name, func(t *testing.T) { + role := tc.role + input := openlaneclient.UpdateProgramMembershipInput{ + Role: &role, + } + + resp, err := tc.client.UpdateUserRoleInProgram(tc.ctx, tc.programMemberID, input) + + if tc.errMsg != "" { + require.Error(t, err) + assert.ErrorContains(t, err, tc.errMsg) + + return + } + + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.UpdateProgramMembership) + assert.Equal(t, tc.role, resp.UpdateProgramMembership.ProgramMembership.Role) + }) + } +}