Skip to content

Commit

Permalink
Fix Role RBAC
Browse files Browse the repository at this point in the history
So bloody simple when you think about it!  When reading roles, only
return the ones you already have all the permissions for at the same or
greated scope.  Basically reuses the existing functions like magic!
  • Loading branch information
spjmurray committed Jul 4, 2024
1 parent 1f8753d commit 72dddd8
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 27 deletions.
4 changes: 2 additions & 2 deletions charts/identity/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ description: A Helm chart for deploying Unikorn's IdP

type: application

version: v0.2.20
appVersion: v0.2.20
version: v0.2.21
appVersion: v0.2.21

icon: https://raw.githubusercontent.com/unikorn-cloud/assets/main/images/logos/dark-on-light/icon.png

Expand Down
5 changes: 0 additions & 5 deletions charts/identity/crds/identity.unikorn-cloud.org_roles.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,6 @@ spec:
spec:
description: RoleSpec defines the role's requested state.
properties:
protected:
description: |-
Protected means the role will only be shown to users if they already
have it in the scope of the organization.
type: boolean
scopes:
description: Scopes are a list of uniquely named scopes for the role.
properties:
Expand Down
3 changes: 0 additions & 3 deletions pkg/apis/unikorn/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,6 @@ type Role struct {

// RoleSpec defines the role's requested state.
type RoleSpec struct {
// Protected means the role will only be shown to users if they already
// have it in the scope of the organization.
Protected bool `json:"protected,omitempty"`
// Scopes are a list of uniquely named scopes for the role.
Scopes RoleScopes `json:"scopes,omitempty"`
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func (h *Handler) GetApiV1OrganizationsOrganizationIDRoles(w http.ResponseWriter
return
}

result, err := roles.New(h.client, h.namespace).List(r.Context())
result, err := roles.New(h.client, h.namespace).List(r.Context(), organizationID)
if err != nil {
errors.HandleError(w, r, err)
return
Expand Down
20 changes: 9 additions & 11 deletions pkg/handler/roles/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/unikorn-cloud/core/pkg/server/conversion"
unikornv1 "github.com/unikorn-cloud/identity/pkg/apis/unikorn/v1alpha1"
"github.com/unikorn-cloud/identity/pkg/openapi"
"github.com/unikorn-cloud/identity/pkg/rbac"

"sigs.k8s.io/controller-runtime/pkg/client"
)
Expand All @@ -41,9 +42,9 @@ func New(client client.Client, namespace string) *Client {
}
}

func convert(in *unikornv1.Role) openapi.RoleRead {
func convert(in unikornv1.Role) openapi.RoleRead {
out := openapi.RoleRead{
Metadata: conversion.ResourceReadMetadata(in, coreapi.ResourceProvisioningStatusProvisioned),
Metadata: conversion.ResourceReadMetadata(&in, coreapi.ResourceProvisioningStatusProvisioned),
}

return out
Expand All @@ -52,14 +53,7 @@ func convert(in *unikornv1.Role) openapi.RoleRead {
func convertList(in unikornv1.RoleList) openapi.Roles {
var out openapi.Roles

for i := range in.Items {
resource := &in.Items[i]

// We need to only display these if we have them in scope.
if resource.Spec.Protected {
continue
}

for _, resource := range in.Items {
out = append(out, convert(resource))
}

Expand All @@ -70,12 +64,16 @@ func convertList(in unikornv1.RoleList) openapi.Roles {
return out
}

func (c *Client) List(ctx context.Context) (openapi.Roles, error) {
func (c *Client) List(ctx context.Context, organizationID string) (openapi.Roles, error) {
var result unikornv1.RoleList

if err := c.client.List(ctx, &result, &client.ListOptions{Namespace: c.namespace}); err != nil {
return nil, err
}

result.Items = slices.DeleteFunc(result.Items, func(role unikornv1.Role) bool {
return rbac.AllowRole(ctx, &role, organizationID) != nil
})

return convertList(result), nil
}
42 changes: 37 additions & 5 deletions pkg/rbac/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@ import (
"slices"

"github.com/unikorn-cloud/core/pkg/server/errors"
unikornv1 "github.com/unikorn-cloud/identity/pkg/apis/unikorn/v1alpha1"
"github.com/unikorn-cloud/identity/pkg/openapi"
)

// allowEndpoints iterates through all endpoints and tries to match the required name and
// operationAllowedByEndpoints iterates through all endpoints and tries to match the required name and
// operation.
func allowEndpoints(endpoints openapi.AclEndpoints, endpoint string, operation openapi.AclOperation) error {
func operationAllowedByEndpoints(endpoints openapi.AclEndpoints, endpoint string, operation openapi.AclOperation) error {
for _, e := range endpoints {
if e.Name != endpoint {
continue
Expand All @@ -50,7 +51,7 @@ func AllowGlobalScope(ctx context.Context, endpoint string, operation openapi.Ac
return errors.HTTPForbidden("operation is not allowed by rbac (no global endpoints)")
}

return allowEndpoints(*acl.Global, endpoint, operation)
return operationAllowedByEndpoints(*acl.Global, endpoint, operation)
}

// AllowOrganizationScope tries to allow the requested operation at the global scope, then
Expand All @@ -66,7 +67,7 @@ func AllowOrganizationScope(ctx context.Context, endpoint string, operation open
return errors.HTTPForbidden("operation is not allowed by rbac (no matching organization endpoints)")
}

return allowEndpoints(acl.Organization.Endpoints, endpoint, operation)
return operationAllowedByEndpoints(acl.Organization.Endpoints, endpoint, operation)
}

// AllowProjectScope tries to allow the requested operation at the global scope, then
Expand All @@ -87,10 +88,41 @@ func AllowProjectScope(ctx context.Context, endpoint string, operation openapi.A
continue
}

if err := allowEndpoints(project.Endpoints, endpoint, operation); err == nil {
if err := operationAllowedByEndpoints(project.Endpoints, endpoint, operation); err == nil {
return nil
}
}

return errors.HTTPForbidden("operation is not allowed by rbac (no matching project endpoints)")
}

// AllowRole determines whether your ACL contains the same or higher privileges than
// the role, which is then used to determine role visibility and limit privilege
// escalation.
func AllowRole(ctx context.Context, role *unikornv1.Role, organizationID string) error {
for _, endpoint := range role.Spec.Scopes.Global {
for _, operation := range endpoint.Operations {
if err := AllowGlobalScope(ctx, endpoint.Name, convertOperation(operation)); err != nil {
return err
}
}
}

for _, endpoint := range role.Spec.Scopes.Organization {
for _, operation := range endpoint.Operations {
if err := AllowOrganizationScope(ctx, endpoint.Name, convertOperation(operation), organizationID); err != nil {
return err
}
}
}

for _, endpoint := range role.Spec.Scopes.Project {
for _, operation := range endpoint.Operations {
if err := AllowOrganizationScope(ctx, endpoint.Name, convertOperation(operation), organizationID); err != nil {
return err
}
}
}

return nil
}

0 comments on commit 72dddd8

Please sign in to comment.