Skip to content

Commit 2a39893

Browse files
authored
feat(authz): prevent non-owners from managing owner memberships (#2776)
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
1 parent ba8c998 commit 2a39893

6 files changed

Lines changed: 140 additions & 33 deletions

File tree

app/controlplane/cmd/wire_gen.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/controlplane/internal/service/organization.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"context"
2020

2121
pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1"
22+
"github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext"
2223
"github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext/entities"
2324
"github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz"
2425
"github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz"
@@ -196,7 +197,8 @@ func (s *OrganizationService) DeleteMembership(ctx context.Context, req *pb.Orga
196197
return nil, err
197198
}
198199

199-
if err := s.membershipUC.DeleteOther(ctx, currentOrg.ID, currentUser.ID, req.MembershipId); err != nil {
200+
callerRole := authz.Role(usercontext.CurrentAuthzSubject(ctx))
201+
if err := s.membershipUC.DeleteOther(ctx, currentOrg.ID, currentUser.ID, req.MembershipId, callerRole); err != nil {
200202
return nil, handleUseCaseErr(err, s.log)
201203
}
202204

@@ -214,7 +216,8 @@ func (s *OrganizationService) UpdateMembership(ctx context.Context, req *pb.Orga
214216
return nil, err
215217
}
216218

217-
m, err := s.membershipUC.UpdateRole(ctx, currentOrg.ID, currentUser.ID, req.MembershipId, biz.PbRoleToBiz(req.Role))
219+
callerRole := authz.Role(usercontext.CurrentAuthzSubject(ctx))
220+
m, err := s.membershipUC.UpdateRole(ctx, currentOrg.ID, currentUser.ID, req.MembershipId, biz.PbRoleToBiz(req.Role), callerRole)
218221
if err != nil {
219222
return nil, handleUseCaseErr(err, s.log)
220223
}

app/controlplane/pkg/authz/authz.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2024-2025 The Chainloop Authors.
2+
// Copyright 2024-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -36,11 +36,12 @@ func (r Role) IsAdmin() bool {
3636
const (
3737
// Actions
3838

39-
ActionRead = "read"
40-
ActionList = "list"
41-
ActionCreate = "create"
42-
ActionUpdate = "update"
43-
ActionDelete = "delete"
39+
ActionRead = "read"
40+
ActionList = "list"
41+
ActionCreate = "create"
42+
ActionUpdate = "update"
43+
ActionDelete = "delete"
44+
ActionManageOwners = "manage_owners"
4445

4546
// Resources
4647

@@ -175,6 +176,9 @@ var (
175176
PolicyProjectRemoveMemberships = &Policy{ResourceProjectMembership, ActionDelete}
176177
// Organization Invitations
177178
PolicyOrganizationInvitationsCreate = &Policy{ResourceOrganizationInvitations, ActionCreate}
179+
180+
// Manage owners (promote to/demote from owner, remove owners)
181+
PolicyOrganizationManageOwners = &Policy{OrganizationMemberships, ActionManageOwners}
178182
)
179183

180184
// RolesMap The default list of policies for each role
@@ -193,6 +197,7 @@ var RolesMap = map[Role][]*Policy{
193197
},
194198
RoleOwner: {
195199
PolicyOrganizationDelete,
200+
PolicyOrganizationManageOwners,
196201
},
197202
// RoleAdmin is an org-scoped role that provides super admin privileges (it's the higher role)
198203
RoleAdmin: {

app/controlplane/pkg/biz/membership.go

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2024-2025 The Chainloop Authors.
2+
// Copyright 2024-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -90,15 +90,17 @@ type MembershipUseCase struct {
9090
// Use Cases
9191
orgUseCase *OrganizationUseCase
9292
auditor *AuditorUseCase
93+
authzUC *AuthzUseCase
9394
}
9495

95-
func NewMembershipUseCase(repo MembershipRepo, orgUC *OrganizationUseCase, auditor *AuditorUseCase, userRepo UserRepo, logger log.Logger) *MembershipUseCase {
96-
return &MembershipUseCase{repo: repo, orgUseCase: orgUC, logger: log.NewHelper(logger), userRepo: userRepo, auditor: auditor}
96+
func NewMembershipUseCase(repo MembershipRepo, orgUC *OrganizationUseCase, auditor *AuditorUseCase, userRepo UserRepo, authzUC *AuthzUseCase, logger log.Logger) *MembershipUseCase {
97+
return &MembershipUseCase{repo: repo, orgUseCase: orgUC, logger: log.NewHelper(logger), userRepo: userRepo, auditor: auditor, authzUC: authzUC}
9798
}
9899

99100
// DeleteOther just deletes a membership from the database
100-
// but ensures that the user is not deleting itself from the org
101-
func (uc *MembershipUseCase) DeleteOther(ctx context.Context, orgID, userID, membershipID string) error {
101+
// but ensures that the user is not deleting itself from the org.
102+
// callerRole is the authorization subject of the caller (e.g. their org role).
103+
func (uc *MembershipUseCase) DeleteOther(ctx context.Context, orgID, userID, membershipID string, callerRole authz.Role) error {
102104
membershipUUID, err := uuid.Parse(membershipID)
103105
if err != nil {
104106
return NewErrInvalidUUID(err)
@@ -120,6 +122,13 @@ func (uc *MembershipUseCase) DeleteOther(ctx context.Context, orgID, userID, mem
120122
return NewErrValidationStr("cannot delete yourself from the org")
121123
}
122124

125+
// Only owners can remove other owners
126+
if m.Role == authz.RoleOwner {
127+
if err := uc.enforceManageOwners(ctx, callerRole); err != nil {
128+
return err
129+
}
130+
}
131+
123132
uc.logger.Infow("msg", "Deleting membership", "org_id", orgID, "membership_id", m.ID.String())
124133

125134
// Delete the main membership - this will also remove the user from all groups in the org
@@ -131,7 +140,7 @@ func (uc *MembershipUseCase) DeleteOther(ctx context.Context, orgID, userID, mem
131140
return nil
132141
}
133142

134-
func (uc *MembershipUseCase) UpdateRole(ctx context.Context, orgID, userID, membershipID string, role authz.Role) (*Membership, error) {
143+
func (uc *MembershipUseCase) UpdateRole(ctx context.Context, orgID, userID, membershipID string, role authz.Role, callerRole authz.Role) (*Membership, error) {
135144
// If it has ben overrode by the user, validate it
136145
if role == "" {
137146
return nil, NewErrValidationStr("role is required")
@@ -160,6 +169,13 @@ func (uc *MembershipUseCase) UpdateRole(ctx context.Context, orgID, userID, memb
160169
return nil, NewErrValidationStr("cannot update yourself")
161170
}
162171

172+
// Only owners can modify owner memberships or promote users to owner
173+
if m.Role == authz.RoleOwner || role == authz.RoleOwner {
174+
if err := uc.enforceManageOwners(ctx, callerRole); err != nil {
175+
return nil, err
176+
}
177+
}
178+
163179
userUUID, err := uuid.Parse(m.User.ID)
164180
if err != nil {
165181
return nil, NewErrInvalidUUID(err)
@@ -187,6 +203,20 @@ func (uc *MembershipUseCase) UpdateRole(ctx context.Context, orgID, userID, memb
187203
return updatedMembership, nil
188204
}
189205

206+
// enforceManageOwners checks that the caller has the manage_owners policy.
207+
func (uc *MembershipUseCase) enforceManageOwners(ctx context.Context, callerRole authz.Role) error {
208+
ok, err := uc.authzUC.Enforce(ctx, string(callerRole), authz.PolicyOrganizationManageOwners)
209+
if err != nil {
210+
return fmt.Errorf("failed to enforce manage owners policy: %w", err)
211+
}
212+
213+
if !ok {
214+
return NewErrUnauthorizedStr("only organization owners can manage owner memberships")
215+
}
216+
217+
return nil
218+
}
219+
190220
type membershipCreateOpts struct {
191221
current bool
192222
role authz.Role

app/controlplane/pkg/biz/membership_integration_test.go

Lines changed: 77 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2024-2025 The Chainloop Authors.
2+
// Copyright 2024-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -229,12 +229,12 @@ func (s *membershipIntegrationTestSuite) TestDeleteOther() {
229229
s.NoError(err)
230230

231231
s.T().Run("I can not delete my own membership", func(t *testing.T) {
232-
err := s.Membership.DeleteOther(ctx, sharedOrg.ID, user2.ID, mUser2SharedOrg.ID.String())
232+
err := s.Membership.DeleteOther(ctx, sharedOrg.ID, user2.ID, mUser2SharedOrg.ID.String(), authz.RoleOwner)
233233
s.ErrorContains(err, "cannot delete yourself from the org")
234234
})
235235

236236
s.T().Run("I can't find the membership", func(t *testing.T) {
237-
err := s.Membership.DeleteOther(ctx, otherOrg.ID, user2.ID, mUser2SharedOrg.ID.String())
237+
err := s.Membership.DeleteOther(ctx, otherOrg.ID, user2.ID, mUser2SharedOrg.ID.String(), authz.RoleOwner)
238238
s.True(biz.IsNotFound(err))
239239
})
240240

@@ -244,7 +244,7 @@ func (s *membershipIntegrationTestSuite) TestDeleteOther() {
244244
s.Len(memberships, 1)
245245
s.Equal(1, count)
246246

247-
err = s.Membership.DeleteOther(ctx, sharedOrg.ID, user.ID, mUser2SharedOrg.ID.String())
247+
err = s.Membership.DeleteOther(ctx, sharedOrg.ID, user.ID, mUser2SharedOrg.ID.String(), authz.RoleOwner)
248248
s.NoError(err)
249249

250250
memberships, count, err = s.Membership.ByOrg(ctx, sharedOrg.ID, &biz.ListByOrgOpts{}, nil)
@@ -270,17 +270,17 @@ func (s *membershipIntegrationTestSuite) TestUpdateRole() {
270270

271271
// empty role
272272
s.T().Run("a role is required", func(t *testing.T) {
273-
_, err := s.Membership.UpdateRole(ctx, sharedOrg.ID, user.ID, mUser2SharedOrg.ID.String(), "")
273+
_, err := s.Membership.UpdateRole(ctx, sharedOrg.ID, user.ID, mUser2SharedOrg.ID.String(), "", authz.RoleOwner)
274274
s.ErrorContains(err, "role is required")
275275
})
276276

277277
s.T().Run("I can not update my own membership", func(t *testing.T) {
278-
_, err := s.Membership.UpdateRole(ctx, sharedOrg.ID, user2.ID, mUser2SharedOrg.ID.String(), authz.RoleAdmin)
278+
_, err := s.Membership.UpdateRole(ctx, sharedOrg.ID, user2.ID, mUser2SharedOrg.ID.String(), authz.RoleAdmin, authz.RoleOwner)
279279
s.ErrorContains(err, "cannot update yourself")
280280
})
281281

282282
s.T().Run("I can't find the membership in another org", func(t *testing.T) {
283-
_, err := s.Membership.UpdateRole(ctx, otherOrg.ID, user.ID, mUser2SharedOrg.ID.String(), authz.RoleAdmin)
283+
_, err := s.Membership.UpdateRole(ctx, otherOrg.ID, user.ID, mUser2SharedOrg.ID.String(), authz.RoleAdmin, authz.RoleOwner)
284284
s.True(biz.IsNotFound(err))
285285
})
286286

@@ -291,12 +291,81 @@ func (s *membershipIntegrationTestSuite) TestUpdateRole() {
291291
s.Equal(1, count)
292292
s.Equal(authz.RoleViewer, memberships[0].Role)
293293

294-
got, err := s.Membership.UpdateRole(ctx, sharedOrg.ID, user.ID, mUser2SharedOrg.ID.String(), authz.RoleAdmin)
294+
got, err := s.Membership.UpdateRole(ctx, sharedOrg.ID, user.ID, mUser2SharedOrg.ID.String(), authz.RoleAdmin, authz.RoleOwner)
295295
s.NoError(err)
296296
s.Equal(authz.RoleAdmin, got.Role)
297297
})
298298
}
299299

300+
func (s *membershipIntegrationTestSuite) TestOwnerGuards() {
301+
ctx := context.Background()
302+
303+
owner, err := s.User.UpsertByEmail(ctx, "owner@test.com", nil)
304+
s.NoError(err)
305+
admin, err := s.User.UpsertByEmail(ctx, "admin@test.com", nil)
306+
s.NoError(err)
307+
target, err := s.User.UpsertByEmail(ctx, "target@test.com", nil)
308+
s.NoError(err)
309+
310+
org, err := s.Organization.CreateWithRandomName(ctx)
311+
s.NoError(err)
312+
313+
// owner is org owner, admin is org admin, target starts as viewer
314+
_, err = s.Membership.Create(ctx, org.ID, owner.ID, biz.WithMembershipRole(authz.RoleOwner))
315+
s.NoError(err)
316+
_, err = s.Membership.Create(ctx, org.ID, admin.ID, biz.WithMembershipRole(authz.RoleAdmin))
317+
s.NoError(err)
318+
mTarget, err := s.Membership.Create(ctx, org.ID, target.ID, biz.WithMembershipRole(authz.RoleViewer))
319+
s.NoError(err)
320+
321+
s.Run("admin cannot promote user to owner", func() {
322+
_, err := s.Membership.UpdateRole(ctx, org.ID, admin.ID, mTarget.ID.String(), authz.RoleOwner, authz.RoleAdmin)
323+
s.Error(err)
324+
s.True(biz.IsErrUnauthorized(err))
325+
s.Contains(err.Error(), "only organization owners can manage owner memberships")
326+
})
327+
328+
s.Run("owner can promote user to owner", func() {
329+
got, err := s.Membership.UpdateRole(ctx, org.ID, owner.ID, mTarget.ID.String(), authz.RoleOwner, authz.RoleOwner)
330+
s.NoError(err)
331+
s.Equal(authz.RoleOwner, got.Role)
332+
})
333+
334+
s.Run("admin cannot demote an owner", func() {
335+
_, err := s.Membership.UpdateRole(ctx, org.ID, admin.ID, mTarget.ID.String(), authz.RoleAdmin, authz.RoleAdmin)
336+
s.Error(err)
337+
s.True(biz.IsErrUnauthorized(err))
338+
s.Contains(err.Error(), "only organization owners can manage owner memberships")
339+
})
340+
341+
s.Run("owner can demote another owner", func() {
342+
got, err := s.Membership.UpdateRole(ctx, org.ID, owner.ID, mTarget.ID.String(), authz.RoleAdmin, authz.RoleOwner)
343+
s.NoError(err)
344+
s.Equal(authz.RoleAdmin, got.Role)
345+
})
346+
347+
s.Run("admin cannot delete an owner membership", func() {
348+
// First promote target back to owner
349+
_, err := s.Membership.UpdateRole(ctx, org.ID, owner.ID, mTarget.ID.String(), authz.RoleOwner, authz.RoleOwner)
350+
s.NoError(err)
351+
352+
err = s.Membership.DeleteOther(ctx, org.ID, admin.ID, mTarget.ID.String(), authz.RoleAdmin)
353+
s.Error(err)
354+
s.True(biz.IsErrUnauthorized(err))
355+
s.Contains(err.Error(), "only organization owners can manage owner memberships")
356+
})
357+
358+
s.Run("owner can delete another owner membership", func() {
359+
err := s.Membership.DeleteOther(ctx, org.ID, owner.ID, mTarget.ID.String(), authz.RoleOwner)
360+
s.NoError(err)
361+
362+
memberships, count, err := s.Membership.ByOrg(ctx, org.ID, &biz.ListByOrgOpts{}, nil)
363+
s.NoError(err)
364+
s.Equal(2, count) // only owner and admin remain
365+
s.Len(memberships, 2)
366+
})
367+
}
368+
300369
func (s *membershipIntegrationTestSuite) TestCreateMembership() {
301370
assert := assert.New(s.T())
302371
ctx := context.Background()

app/controlplane/pkg/biz/testhelpers/wire_gen.go

Lines changed: 10 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)