fix(rbac): org creation authz changes (#2361)

Signed-off-by: Miguel Martinez <[email protected]>

Author: migmartri

app/controlplane/internal/service/organization.go

@@ -98,12 +98,28 @@ func (s *OrganizationService) Update(ctx context.Context, req *pb.OrganizationSe
 }
 
 func (s *OrganizationService) Delete(ctx context.Context, req *pb.OrganizationServiceDeleteRequest) (*pb.OrganizationServiceDeleteResponse, error) {
-	currentUser, err := requireCurrentUser(ctx)
-	if err != nil {
+	if _, err := requireCurrentUser(ctx); err != nil {
 		return nil, err
 	}
 
-	if err := s.orgUC.DeleteByUser(ctx, req.Name, currentUser.ID); err != nil {
+	// Find the organization to get its UUID for authorization
+	org, err := s.orgUC.FindByName(ctx, req.Name)
+	if err != nil {
+		return nil, handleUseCaseErr(err, s.log)
+	}
+
+	orgUUID, err := uuid.Parse(org.ID)
+	if err != nil {
+		return nil, handleUseCaseErr(biz.NewErrInvalidUUID(err), s.log)
+	}
+
+	// Check if user has permission to delete this specific organization
+	// Force RBAC to ensure only owners can delete, even if they have admin privileges elsewhere
+	if err := s.authorizeResource(ctx, authz.PolicyOrganizationDelete, authz.ResourceTypeOrganization, orgUUID, withForceRBAC()); err != nil {
+		return nil, errors.Forbidden("forbidden", "only organization owners can delete the organization")
+	}
+
+	if err := s.orgUC.Delete(ctx, orgUUID.String()); err != nil {
 		return nil, handleUseCaseErr(err, s.log)
 	}
 

app/controlplane/internal/service/service.go

@@ -164,14 +164,32 @@ func WithGroupUseCase(groupUseCase *biz.GroupUseCase) NewOpt {
 	}
 }
 
+type authorizeResourceOpts struct {
+	forceRBAC bool
+}
+
+type AuthorizeResourceOpt func(*authorizeResourceOpts)
+
+// withForceRBAC forces RBAC checks even for admin roles that would normally skip RBAC
+func withForceRBAC() AuthorizeResourceOpt {
+	return func(opts *authorizeResourceOpts) {
+		opts.forceRBAC = true
+	}
+}
+
 // authorizeResource is a helper that checks if the user has a particular `op` permission policy on a particular resource
 // For example: `s.authorizeResource(ctx, authz.PolicyAttachedIntegrationDetach, authz.ResourceTypeProject, projectUUID);`
 // checks if the user has a role in the project that allows to detach integrations on it.
 // This method is available to every service that embeds `service`
 // It goes through all the memberships of the user, direct memberships and indirect memberships (Groups)
 // and checks if the user has any role that allows the operation on the resourceType and resourceID.
-func (s *service) authorizeResource(ctx context.Context, op *authz.Policy, resourceType authz.ResourceType, resourceID uuid.UUID) error {
-	if !rbacEnabled(ctx) {
+func (s *service) authorizeResource(ctx context.Context, op *authz.Policy, resourceType authz.ResourceType, resourceID uuid.UUID, opts ...AuthorizeResourceOpt) error {
+	options := &authorizeResourceOpts{}
+	for _, opt := range opts {
+		opt(options)
+	}
+
+	if !options.forceRBAC && !rbacEnabled(ctx) {
 		return nil
 	}
 
@@ -188,24 +206,28 @@ func (s *service) authorizeResource(ctx context.Context, op *authz.Policy, resou
 		return errors.Forbidden("forbidden", fmt.Errorf("operation not allowed: This auth token is valid only with the project %q", *token.ProjectName).Error())
 	}
 
+	var defaultMessage = fmt.Sprintf("you do not have permissions to access the %q with id %q", resourceType, resourceID.String())
 	// 2 - We are a user
 	// 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
+	var foundRoles []string
 	// First, collect all memberships that match the requested resource type and ID
 	for _, rm := range m.Resources {
 		if rm.ResourceType == resourceType && rm.ResourceID == resourceID {
 			matchingResources = append(matchingResources, rm)
+			foundRoles = append(foundRoles, string(rm.Role))
 		}
 	}
 
-	var defaultMessage = fmt.Sprintf("you do not have permissions to access to the %s associated with this resource", resourceType)
 	// If no matching resources were found, return forbidden error
 	if len(matchingResources) == 0 {
 		return errors.Forbidden("forbidden", defaultMessage)
 	}
 
+	defaultMessage = fmt.Sprintf("%s, roles=%v", defaultMessage, foundRoles)
+
 	// Try to enforce the policy with each matching role
 	// If any role passes, authorize the request
 	for _, rm := range matchingResources {

app/controlplane/pkg/authz/authz.go

@@ -167,7 +167,9 @@ var (
 	// Projects
 	PolicyProjectCreate = &Policy{ResourceProject, ActionCreate}
 
+	// Organization
 	PolicyOrganizationCreate = &Policy{Organization, ActionCreate}
+	PolicyOrganizationDelete = &Policy{Organization, ActionDelete}
 	// User Membership
 	PolicyOrganizationRead            = &Policy{Organization, ActionRead}
 	PolicyOrganizationListMemberships = &Policy{OrganizationMemberships, ActionList}
@@ -200,6 +202,18 @@ var RolesMap = map[Role][]*Policy{
 	RoleInstanceAdmin: {
 		PolicyOrganizationCreate,
 	},
+	RoleOwner: {
+		PolicyOrganizationDelete,
+	},
+	// RoleAdmin is an org-scoped role that provides super admin privileges (it's the higher role)
+	RoleAdmin: {
+		// We do a manual check in the artifact upload endpoint
+		// so we need the actual policy in place skipping it is not enough
+		PolicyArtifactUpload,
+		// We manually check this policy to be able to know if the user can invite users to the system
+		PolicyOrganizationInvitationsCreate,
+		// + all the policies from the viewer role inherited automatically
+	},
 	// RoleViewer is an org-scoped role that provides read-only access to all resources
 	RoleViewer: {
 		// Referrer
@@ -234,16 +248,6 @@ var RolesMap = map[Role][]*Policy{
 		// List organization memberships
 		PolicyOrganizationListMemberships,
 	},
-	// RoleAdmin is an org-scoped role that provides super admin privileges (it's the higher role)
-	RoleAdmin: {
-		// We do a manual check in the artifact upload endpoint
-		// so we need the actual policy in place skipping it is not enough
-		PolicyArtifactUpload,
-		// We manually check this policy to be able to know if the user can invite users to the system
-		PolicyOrganizationInvitationsCreate,
-		// + all the policies from the viewer role inherited automatically
-	},
-
 	// RoleOrgMember is an org-scoped role that enables RBAC in the underlying resources. Users with this role at
 	// the organization level will need specific project roles to access their contents
 	RoleOrgContributor: {
@@ -403,6 +407,9 @@ var ServerOperationsMap = map[string][]*Policy{
 	// since all the permissions here are in the context of an organization
 	// Create new organization
 	"/controlplane.v1.OrganizationService/Create": {},
+	// Delete an organization makes checks at the service level since the
+	// user can explicitly set the org they want to delete and might not be the current one
+	"/controlplane.v1.OrganizationService/Delete": {},
 
 	// List global memberships
 	"/controlplane.v1.OrganizationService/ListMemberships": {PolicyOrganizationListMemberships},

app/controlplane/pkg/biz/organization.go

@@ -290,43 +290,6 @@ func (uc *OrganizationUseCase) Delete(ctx context.Context, id string) error {
 	return uc.orgRepo.Delete(ctx, orgUUID)
 }
 
-// DeleteByUser deletes an organization initiated by a user with owner validation
-// Only organization owners can delete an organization
-func (uc *OrganizationUseCase) DeleteByUser(ctx context.Context, orgName, userID string) error {
-	// Find organization by name
-	org, err := uc.orgRepo.FindByName(ctx, orgName)
-	if err != nil {
-		return err
-	} else if org == nil {
-		return NewErrNotFound("organization")
-	}
-
-	orgUUID, err := uuid.Parse(org.ID)
-	if err != nil {
-		return NewErrInvalidUUID(err)
-	}
-
-	userUUID, err := uuid.Parse(userID)
-	if err != nil {
-		return NewErrInvalidUUID(err)
-	}
-
-	// Check if user is an owner of the organization
-	m, err := uc.membershipRepo.FindByOrgAndUser(ctx, orgUUID, userUUID)
-	if err != nil {
-		return fmt.Errorf("failed to find owners: %w", err)
-	}
-
-	if m == nil || m.Role != authz.RoleOwner {
-		return NewErrValidationStr("only organization owners can delete the organization")
-	}
-
-	uc.logger.Infow("msg", "User deleting organization", "user_id", userID, "organization_id", org.ID)
-
-	// Use the existing Delete method to handle the actual deletion
-	return uc.Delete(ctx, org.ID)
-}
-
 // AutoOnboardOrganizations creates the organizations specified in the onboarding config and assigns the user to them
 // with the specified role if they are not already a member.
 func (uc *OrganizationUseCase) AutoOnboardOrganizations(ctx context.Context, userID string) error {