Skip to content

Revert "chore: org viewers cannot become project admins (#2251)" #2262

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 15, 2025
Merged

Revert "chore: org viewers cannot become project admins (#2251)" #2262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/controlplane/internal/service/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ func (s *service) authorizeResource(ctx context.Context, op *authz.Policy, resou
// find the resource membership that matches the resource type and ID
// for example admin in project1, then apply RBAC enforcement
m := entities.CurrentMembership(ctx)

var matchingResources []*entities.ResourceMembership
// First, collect all memberships that match the requested resource type and ID
for _, rm := range m.Resources {
Expand Down
17 changes: 1 addition & 16 deletions app/controlplane/pkg/biz/membership.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,22 +340,7 @@ func (uc *MembershipUseCase) ListAllMembershipsForUser(ctx context.Context, user
return nil, fmt.Errorf("failed to list group memberships for user: %w", err)
}

// remove incompatible/illegal combinations (org viewer and project admin)
combined := make([]*Membership, 0)
combined = append(combined, userMemberships...)
for _, um := range userMemberships {
if um.ResourceType == authz.ResourceTypeOrganization && um.Role == authz.RoleViewer {
for _, gm := range groupMemberships {
// if user is org viewer and project admin through a group, skip it.
if gm.Role == authz.RoleProjectAdmin {
continue
}
combined = append(combined, gm)
}
}
}

return combined, nil
return append(userMemberships, groupMemberships...), nil
}

// SetProjectOwner sets the project owner (admin role). It skips the operation if an owner exists already
Expand Down
5 changes: 0 additions & 5 deletions app/controlplane/pkg/biz/project.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,6 @@ func (uc *ProjectUseCase) addUserToProject(ctx context.Context, orgID uuid.UUID,
return uc.handleNonExistingUser(ctx, orgID, projectID, opts)
}

// Org viewers cannot be added as project admin, since they cannot perform updates on resources
if opts.Role == authz.RoleProjectAdmin && userMembership.Role == authz.RoleViewer {
return nil, NewErrValidationStr("users with org role Org Viewer cannot be Project Admins")
}

userUUID := uuid.MustParse(userMembership.User.ID)

// Check if the user is already a member of the project
Expand Down
14 changes: 7 additions & 7 deletions app/controlplane/pkg/biz/project_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (s *projectMembersIntegrationTestSuite) TestListMembers() {
// Add users to organization
_, err = s.Membership.Create(ctx, s.org.ID, user2.ID)
require.NoError(s.T(), err)
_, err = s.Membership.Create(ctx, s.org.ID, user3.ID, biz.WithMembershipRole(authz.RoleOrgMember))
_, err = s.Membership.Create(ctx, s.org.ID, user3.ID)
require.NoError(s.T(), err)

// Add users to the project
Expand Down Expand Up @@ -201,7 +201,7 @@ func (s *projectMembersIntegrationTestSuite) TestAddMemberToProject() {
// Add users to organization
_, err = s.Membership.Create(ctx, s.org.ID, user2.ID)
require.NoError(s.T(), err)
_, err = s.Membership.Create(ctx, s.org.ID, user3.ID, biz.WithMembershipRole(authz.RoleOrgMember))
_, err = s.Membership.Create(ctx, s.org.ID, user3.ID)
require.NoError(s.T(), err)

projectID := s.project.ID
Expand Down Expand Up @@ -421,7 +421,7 @@ func (s *projectMembersIntegrationTestSuite) TestRemoveMemberFromProject() {
// Add users to organization
_, err = s.Membership.Create(ctx, s.org.ID, user2.ID)
require.NoError(s.T(), err)
_, err = s.Membership.Create(ctx, s.org.ID, user3.ID, biz.WithMembershipRole(authz.RoleOrgMember))
_, err = s.Membership.Create(ctx, s.org.ID, user3.ID)
require.NoError(s.T(), err)
_, err = s.Membership.Create(ctx, s.org.ID, user4.ID)
require.NoError(s.T(), err)
Expand Down Expand Up @@ -649,7 +649,7 @@ func (s *projectAdminPermissionsTestSuite) TestAdminPermissions() {
require.NoError(s.T(), err)

// Add the user to the organization
_, err = s.Membership.Create(ctx, s.org.ID, user2.ID, biz.WithCurrentMembership(), biz.WithMembershipRole(authz.RoleOrgMember))
_, err = s.Membership.Create(ctx, s.org.ID, user2.ID, biz.WithCurrentMembership())
require.NoError(s.T(), err)

// Grant project admin role to the user
Expand Down Expand Up @@ -770,15 +770,15 @@ func (s *projectPermissionsTestSuite) SetupTest() {
assert.NoError(err)

// Add project admin user to organization as a regular member
_, err = s.Membership.Create(ctx, s.org.ID, s.projectAdminUser.ID, biz.WithCurrentMembership(), biz.WithMembershipRole(authz.RoleOrgMember))
_, err = s.Membership.Create(ctx, s.org.ID, s.projectAdminUser.ID, biz.WithCurrentMembership())
assert.NoError(err)

// Create a regular user
s.regularUser, err = s.User.UpsertByEmail(ctx, fmt.Sprintf("regular-user-%s@example.com", uuid.New().String()), nil)
assert.NoError(err)

// Add regular user to organization as a regular member
_, err = s.Membership.Create(ctx, s.org.ID, s.regularUser.ID, biz.WithMembershipRole(authz.RoleOrgMember))
_, err = s.Membership.Create(ctx, s.org.ID, s.regularUser.ID)
assert.NoError(err)

// Create a project for tests
Expand Down Expand Up @@ -1340,7 +1340,7 @@ func (s *projectMembersIntegrationTestSuite) TestUpdateUserRoleInProject() {
// Add users to organization
_, err = s.Membership.Create(ctx, s.org.ID, user1.ID)
require.NoError(s.T(), err)
_, err = s.Membership.Create(ctx, s.org.ID, user2.ID, biz.WithMembershipRole(authz.RoleOrgMember))
_, err = s.Membership.Create(ctx, s.org.ID, user2.ID)
require.NoError(s.T(), err)

projectID := s.project.ID
Expand Down
Loading