Skip to content

Commit e0b387c

Browse files
adelowoadelowo
committed
fix reviews
1 parent e7f1a33 commit e0b387c

3 files changed

Lines changed: 168 additions & 47 deletions

File tree

‎internal/ent/hooks/orgmembers.go‎

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -155,35 +155,64 @@ func HookBlockOwnerRoleChange() ent.Hook {
155155
return next.Mutate(ctx, m)
156156
}
157157

158-
oldRole, err := getUserMembershipRole(ctx, m)
158+
oldRoles, err := getUserMembershipRoles(ctx, m)
159159
if err != nil {
160160
return nil, err
161161
}
162162

163-
if newRole == enums.RoleOwner || oldRole == enums.RoleOwner {
163+
if newRole == enums.RoleOwner {
164164
return nil, ErrOrgOwnerCannotBeUpdated
165165
}
166166

167+
for _, oldRole := range oldRoles {
168+
if oldRole == enums.RoleOwner {
169+
return nil, ErrOrgOwnerCannotBeUpdated
170+
}
171+
}
172+
167173
return next.Mutate(ctx, m)
168174
})
169175
}, ent.OpUpdate|ent.OpUpdateOne)
170176
}
171177

172-
func getUserMembershipRole(ctx context.Context, m *generated.OrgMembershipMutation) (enums.Role, error) {
173-
id, ok := m.ID()
174-
if !ok {
175-
return "", fmt.Errorf("%w: %s", ErrInvalidInput, "id is required")
178+
func getUserMembershipRoles(ctx context.Context, m *generated.OrgMembershipMutation) ([]enums.Role, error) {
179+
var ids []string
180+
var err error
181+
182+
switch {
183+
case m.Op().Is(ent.OpUpdateOne):
184+
if id, exists := m.ID(); exists {
185+
ids = []string{id}
186+
}
187+
188+
case m.Op().Is(ent.OpUpdate):
189+
ids, err = m.IDs(ctx)
190+
if err != nil {
191+
return nil, err
192+
}
193+
}
194+
195+
if len(ids) == 0 {
196+
ids, err = m.IDs(ctx)
197+
}
198+
if err != nil {
199+
return nil, err
176200
}
177201

178-
member, err := m.Client().OrgMembership.Query().
179-
Where(orgmembership.ID(id)).
202+
members, err := m.Client().OrgMembership.Query().
203+
Where(orgmembership.IDIn(ids...)).
180204
Select(orgmembership.FieldRole).
181-
Only(ctx)
205+
All(ctx)
182206
if err != nil {
183-
return "", err
207+
return nil, err
208+
}
209+
210+
roles := make([]enums.Role, len(members))
211+
for i, member := range members {
212+
roles[i] = member.Role
184213
}
185214

186-
return member.Role, nil
215+
return roles, nil
187216
}
188217

189218
// HookOrgMembersDelete is a hook that runs during the delete operation of an org membership

‎internal/ent/privacy/rule/membership.go‎

Lines changed: 65 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package rule
33
import (
44
"context"
55

6+
"entgo.io/ent"
7+
68
"github.com/theopenlane/iam/auth"
79
"github.com/theopenlane/iam/fgax"
810

@@ -50,57 +52,84 @@ func AllowOrgMemberRoleUpdate() privacy.OrgMembershipMutationRuleFunc {
5052
return privacy.Skip
5153
}
5254

53-
id, ok := m.ID()
54-
if !ok {
55-
return privacy.Skip
56-
}
55+
var ids []string
56+
var err error
5757

58-
member, err := m.Client().OrgMembership.Query().
59-
Where(orgmembership.ID(id)).
60-
Select(orgmembership.FieldOrganizationID, orgmembership.FieldRole).
61-
Only(ctx)
62-
if err != nil {
63-
return privacy.Skipf("unable to get org membership: %v", err)
64-
}
58+
switch {
59+
case m.Op().Is(ent.OpUpdateOne):
6560

66-
if newRole == enums.RoleOwner || member.Role == enums.RoleOwner {
67-
return privacy.Skip
68-
}
61+
if id, ok := m.ID(); ok {
62+
ids = []string{id}
63+
}
6964

70-
caller, ok := auth.CallerFromContext(ctx)
71-
if !ok || caller == nil {
72-
return auth.ErrNoAuthUser
65+
case m.Op().Is(ent.OpUpdate):
66+
67+
ids, err = m.IDs(ctx)
68+
if err != nil {
69+
return privacy.Skipf("unable to get org membership ids: %v", err)
70+
}
7371
}
7472

75-
check := fgax.AccessCheck{
76-
SubjectID: caller.SubjectID,
77-
SubjectType: caller.SubjectType(),
78-
ObjectID: member.OrganizationID,
79-
Relation: InviteRelationForRole(member.Role),
80-
Context: utils.NewOrganizationContextKey(caller.SubjectEmail),
73+
if len(ids) == 0 {
74+
ids, err = m.IDs(ctx)
8175
}
8276

83-
access, err := m.Authz.CheckOrgAccess(ctx, check)
77+
members, err := m.Client().OrgMembership.Query().
78+
Where(orgmembership.IDIn(ids...)).
79+
Select(orgmembership.FieldOrganizationID, orgmembership.FieldRole).
80+
All(ctx)
8481
if err != nil {
85-
logx.FromContext(ctx).Error().Err(err).Interface("tuple", check).Msg("unable to check role assignment access")
86-
return privacy.Skipf("unable to check access: %v", err)
82+
return privacy.Skipf("unable to get org membership: %v", err)
8783
}
8884

89-
if !access {
90-
return generated.ErrPermissionDenied
85+
if len(members) == 0 {
86+
return privacy.Allow
9187
}
9288

93-
newRoleAccess := check
94-
newRoleAccess.Relation = InviteRelationForRole(newRole)
89+
if newRole == enums.RoleOwner {
90+
return privacy.Skip
91+
}
9592

96-
access, err = m.Authz.CheckOrgAccess(ctx, newRoleAccess)
97-
if err != nil {
98-
logx.FromContext(ctx).Error().Err(err).Interface("tuple", newRoleAccess).Msg("unable to check role assignment access")
99-
return privacy.Skipf("unable to check access: %v", err)
93+
caller, ok := auth.CallerFromContext(ctx)
94+
if !ok || caller == nil {
95+
return auth.ErrNoAuthUser
10096
}
10197

102-
if !access {
103-
return generated.ErrPermissionDenied
98+
for _, member := range members {
99+
if member.Role == enums.RoleOwner {
100+
return privacy.Skip
101+
}
102+
103+
check := fgax.AccessCheck{
104+
SubjectID: caller.SubjectID,
105+
SubjectType: caller.SubjectType(),
106+
ObjectID: member.OrganizationID,
107+
Relation: InviteRelationForRole(member.Role),
108+
Context: utils.NewOrganizationContextKey(caller.SubjectEmail),
109+
}
110+
111+
access, err := m.Authz.CheckOrgAccess(ctx, check)
112+
if err != nil {
113+
logx.FromContext(ctx).Error().Err(err).Interface("tuple", check).Msg("unable to check role assignment access")
114+
return privacy.Skipf("unable to check access: %v", err)
115+
}
116+
117+
if !access {
118+
return generated.ErrPermissionDenied
119+
}
120+
121+
newRoleAccess := check
122+
newRoleAccess.Relation = InviteRelationForRole(newRole)
123+
124+
access, err = m.Authz.CheckOrgAccess(ctx, newRoleAccess)
125+
if err != nil {
126+
logx.FromContext(ctx).Error().Err(err).Interface("tuple", newRoleAccess).Msg("unable to check role assignment access")
127+
return privacy.Skipf("unable to check access: %v", err)
128+
}
129+
130+
if !access {
131+
return generated.ErrPermissionDenied
132+
}
104133
}
105134

106135
return privacy.Allow

‎internal/graphapi/orgmembers_test.go‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,69 @@ func TestMutationUpdateOrgMemberRole(t *testing.T) {
510510
cleanupOrganizationDataWithContext(org.owner.UserCtx, t)
511511
}
512512

513+
func TestMutationBulkUpdateOrgMemberRole(t *testing.T) {
514+
t.Parallel()
515+
516+
org := suite.seedFreshOrgUsers(t)
517+
allowCtx := privacy.DecisionContext(context.Background(), privacy.Allow)
518+
519+
user1 := suite.userBuilder(context.Background(), t)
520+
user2 := suite.userBuilder(context.Background(), t)
521+
522+
suite.addUserToOrganization(org.owner.UserCtx, t, &user1, enums.RoleMember, org.owner.OrganizationID)
523+
suite.addUserToOrganization(org.owner.UserCtx, t, &user2, enums.RoleMember, org.owner.OrganizationID)
524+
525+
currentMembers, err := suite.client.db.OrgMembership.Query().
526+
Where(
527+
orgmembership.OrganizationID(org.owner.OrganizationID),
528+
orgmembership.UserIDIn(user1.ID, user2.ID),
529+
).
530+
All(allowCtx)
531+
assert.NilError(t, err)
532+
assert.Check(t, is.Len(currentMembers, 2))
533+
534+
ids := make([]string, len(currentMembers))
535+
for i, member := range currentMembers {
536+
ids[i] = member.ID
537+
}
538+
539+
adminRole := enums.RoleAdmin
540+
541+
input := testclient.UpdateOrgMembershipInput{
542+
Role: &adminRole,
543+
}
544+
545+
for _, id := range ids {
546+
_, err = suite.client.api.UpdateUserRoleInOrg(org.admin.UserCtx, id, input)
547+
assert.NilError(t, err)
548+
}
549+
550+
updatedMembers, err := suite.client.db.OrgMembership.Query().
551+
Where(orgmembership.IDIn(ids...)).
552+
All(allowCtx)
553+
assert.NilError(t, err)
554+
555+
for _, member := range updatedMembers {
556+
assert.Check(t, is.Equal(enums.RoleAdmin, member.Role))
557+
}
558+
559+
ownerMember, err := suite.client.db.OrgMembership.Query().
560+
Where(
561+
orgmembership.OrganizationID(org.owner.OrganizationID),
562+
orgmembership.UserID(org.owner.ID),
563+
).
564+
Only(allowCtx)
565+
assert.NilError(t, err)
566+
567+
memberRole := enums.RoleMember
568+
input.Role = &memberRole
569+
570+
_, err = suite.client.api.UpdateUserRoleInOrg(org.admin.UserCtx, ownerMember.ID, input)
571+
assert.ErrorContains(t, err, hooks.ErrOrgOwnerCannotBeUpdated.Error())
572+
573+
cleanupOrganizationDataWithContext(org.owner.UserCtx, t)
574+
}
575+
513576
func TestMutationDeleteOrgMembers(t *testing.T) {
514577
t.Parallel()
515578

0 commit comments

Comments
 (0)