From 0de13900c564ce28a2e41b9d326fd3c4e819cba4 Mon Sep 17 00:00:00 2001 From: Bailin He <15058035+bailinhe@users.noreply.github.com> Date: Fri, 22 Mar 2024 15:41:59 -0400 Subject: [PATCH] Apply suggestions from code review Co-authored-by: John Schaeffer Signed-off-by: Bailin He <15058035+bailinhe@users.noreply.github.com> --- cmd/schema_mermaid.go | 9 +- docs/rbac.md | 52 ++++++-- internal/iapl/default.go | 24 ++-- internal/iapl/policy.go | 219 ++++++++++++++----------------- internal/iapl/policy_test.go | 78 +++++------ internal/iapl/rbac.go | 26 ++-- internal/query/relations.go | 2 +- internal/query/relations_test.go | 4 +- policy.example.yaml | 108 +++++++-------- 9 files changed, 254 insertions(+), 268 deletions(-) diff --git a/cmd/schema_mermaid.go b/cmd/schema_mermaid.go index 36c74e1cb..0faccdab7 100644 --- a/cmd/schema_mermaid.go +++ b/cmd/schema_mermaid.go @@ -13,7 +13,7 @@ import ( var ( mermaidTemplate = `erDiagram -{{- if ne .RBAC.RoleResource nil}} +{{- if ne .RBAC nil}} {{ .RBAC.RoleBindingResource }} }o--o{ {{ .RBAC.RoleResource }} : role {{- range $subj := .RBAC.RoleBindingSubjects }} {{ $.RBAC.RoleBindingResource }} }o--o{ {{ $subj.Name }} : subject @@ -33,9 +33,6 @@ var ( {{- end }} } {{- range $rel := $resource.Relationships }} - {{- range $targetName := $rel.TargetTypeNames }} - {{ $resource.Name }} }o--o{ {{ $targetName }} : {{ $rel.Relation }} - {{- end }} {{- range $target := $rel.TargetTypes }} {{ $resource.Name }} }o--o{ {{ $target.Name -}} : {{ $rel.Relation -}} @@ -54,10 +51,6 @@ var ( {{- end }} {{- end }} } - {{- range $typ := $union.ResourceTypeNames }} - {{ $union.Name }} ||--|| {{ $typ }} : alias - {{- end }} - {{- range $typ := $union.ResourceTypes }} {{ $union.Name }} ||--|| {{ $typ.Name -}} : alias {{- end}} diff --git a/docs/rbac.md b/docs/rbac.md index 924c13516..4918a59d0 100644 --- a/docs/rbac.md +++ b/docs/rbac.md @@ -88,8 +88,6 @@ in IAPL policy terms: - the RoleBindingResource would be "role_binding", - the RoleRelationshipSubject would be `[user, client]`. - the RoleBindingSubjects would be `[{name: user}, {name: group, subjectrelation: member}]`. -- the RolebindingPermissionsPrefix would be "rolebinding" -- the GrantRelationship would be "grant" ### Roles @@ -183,10 +181,18 @@ use cases. ### Ownerships -To accommodate inheritance of the grant relationships, a new type of `Condition` -is introduced to the IAPL: +To accommodate inheritance of the grant relationships, +a new property `RoleBindingV2` is added to the resource type definitions +and a new type of `Condition` is introduced to the IAPL: ```diff + type ResourceType struct { + Name string + IDPrefix string ++ RoleBindingV2 *ResourceRoleBindingV2 + Relationships []Relationship + } + type Condition struct { RoleBinding *ConditionRoleBinding + RoleBindingV2 *ConditionRoleBindingV2 @@ -194,28 +200,40 @@ is introduced to the IAPL: } ``` -a property of `InheritGrants []string` is defined in `ConditionRoleBindingV2` +A property of `InheritPermissionsFrom []string` is defined in `ResourceRoleBindingV2` that allows the IAPL to generate a permission line in the SpiceDB schema that -allows grants to be inherited from its owner or parent. +allows grants and roles to be inherited from its owner or parent. + +When `RoleBindingV2` is defined in a given `Condition`, the IAPL will look for +the `resourcetype.RoleBindingV2.InheritPermissionsFrom` property in the resource +type that the condition's action belongs to. For example, consider the following `ActionBinding`: ```yaml # ... +resourcetypes: + - name: doc + idprefix: doc + rolebindingv2: + inheritpermissionsfrom: + - owner + - name: tenant + idprefix: tenant + rolebindingv2: + inheritpermissionsfrom: + - parent + actionbindings: - actionname: read_doc typename: doc conditions: - rolebindingv2: - inheritgrants: - - owner + rolebindingv2: {} - actionname: read_doc typename: tenant conditions: - rolebindingv2: - inheritgrants: - - parent + rolebindingv2: {} # ... ``` @@ -385,3 +403,13 @@ flowchart TD permok-->ok{{ok ✅}} memberok-->ok ``` + +## Glossary + +- **Subject**: The entities that permissions can be granted to, such as users, clients, or group members +- **Role**: An entity that contains a set of permissions +- **RoleBinding**: An entity that creates a relationship between a role and some subjects, + meaning that these subjects are "in possession" of the permissions defined in the role +- **Grant**: The relationship between a role-binding and a resource, effectively creating a + three way relationship between a role, a resource, and the subjects +- **Inheritance**: The ability to propagate permissions and roles from a parent resource to its children diff --git a/internal/iapl/default.go b/internal/iapl/default.go index e9454d3f6..f9069b507 100644 --- a/internal/iapl/default.go +++ b/internal/iapl/default.go @@ -1,5 +1,7 @@ package iapl +import "go.infratographer.com/permissions-api/internal/types" + // DefaultPolicyDocument returns the default policy document for permissions-api. func DefaultPolicyDocument() PolicyDocument { return PolicyDocument{ @@ -10,8 +12,8 @@ func DefaultPolicyDocument() PolicyDocument { Relationships: []Relationship{ { Relation: "subject", - TargetTypeNames: []string{ - "subject", + TargetTypes: []types.TargetType{ + {Name: "subject"}, }, }, }, @@ -30,8 +32,8 @@ func DefaultPolicyDocument() PolicyDocument { Relationships: []Relationship{ { Relation: "parent", - TargetTypeNames: []string{ - "tenant", + TargetTypes: []types.TargetType{ + {Name: "tenant"}, }, }, }, @@ -42,8 +44,8 @@ func DefaultPolicyDocument() PolicyDocument { Relationships: []Relationship{ { Relation: "owner", - TargetTypeNames: []string{ - "resourceowner", + TargetTypes: []types.TargetType{ + {Name: "resourceowner"}, }, }, }, @@ -52,15 +54,15 @@ func DefaultPolicyDocument() PolicyDocument { Unions: []Union{ { Name: "subject", - ResourceTypeNames: []string{ - "user", - "client", + ResourceTypes: []types.TargetType{ + {Name: "user"}, + {Name: "client"}, }, }, { Name: "resourceowner", - ResourceTypeNames: []string{ - "tenant", + ResourceTypes: []types.TargetType{ + {Name: "tenant"}, }, }, }, diff --git a/internal/iapl/policy.go b/internal/iapl/policy.go index bb2144625..c54f6559e 100644 --- a/internal/iapl/policy.go +++ b/internal/iapl/policy.go @@ -28,16 +28,14 @@ type ResourceType struct { // Relationship represents a named relation between two resources. type Relationship struct { - Relation string - TargetTypeNames []string - TargetTypes []types.TargetType + Relation string + TargetTypes []types.TargetType } // Union represents a named union of multiple concrete resource types. type Union struct { - Name string - ResourceTypeNames []string - ResourceTypes []types.TargetType + Name string + ResourceTypes []types.TargetType } // Action represents an action that can be taken in an authorization policy. @@ -66,9 +64,7 @@ type ConditionRoleBinding struct{} // ConditionRoleBindingV2 represents a condition where a role binding is necessary to perform an action. // Using this condition type in the policy will instruct the policy engine to // create all the necessary relationships in the schema to support RBAC V2 -type ConditionRoleBindingV2 struct { - InheritGrantsFrom []string -} +type ConditionRoleBindingV2 struct{} // ConditionRelationshipAction represents a condition where another action must be allowed on a resource // along a relation to perform an action. @@ -97,7 +93,7 @@ type policy struct { // NewPolicy creates a policy from the given policy document. func NewPolicy(p PolicyDocument) Policy { - rt := make(map[string]ResourceType, len(p.ResourceTypes)) + rt := make(map[string]ResourceType) for _, r := range p.ResourceTypes { rt[r.Name] = r } @@ -158,12 +154,6 @@ func (v *policy) validateUnions() error { return fmt.Errorf("%s: %w", union.Name, ErrorTypeExists) } - for _, rtName := range union.ResourceTypeNames { - if _, ok := v.rt[rtName]; !ok { - return fmt.Errorf("%s: resourceTypeNames: %s: %w", union.Name, rtName, ErrorUnknownType) - } - } - for _, rt := range union.ResourceTypes { if _, ok := v.rt[rt.Name]; !ok { return fmt.Errorf("%s: resourceTypes: %s: %w", union.Name, rt.Name, ErrorUnknownType) @@ -175,23 +165,13 @@ func (v *policy) validateUnions() error { } func (v *policy) validateResourceTypes() error { - findRelationship := func(rels []Relationship, name string) bool { - for _, rel := range rels { - if rel.Relation == name { - return true - } - } - - return false - } - for _, resourceType := range v.rt { for _, rel := range resourceType.Relationships { // two kinds of relationships target types to be validated // 1. simple target type names - for _, name := range rel.TargetTypeNames { - if _, ok := v.rt[name]; !ok { - return fmt.Errorf("%s: relationships: %s: %w", resourceType.Name, name, ErrorUnknownType) + for _, tt := range rel.TargetTypes { + if _, ok := v.rt[tt.Name]; !ok { + return fmt.Errorf("%s: relationships: %s: %w", resourceType.Name, tt.Name, ErrorUnknownType) } } @@ -201,7 +181,7 @@ func (v *policy) validateResourceTypes() error { return fmt.Errorf("%s: relationships: %s: %w", resourceType.Name, tt.Name, ErrorUnknownType) } - if tt.SubjectRelation != "" && !findRelationship(v.rt[tt.Name].Relationships, tt.SubjectRelation) { + if tt.SubjectRelation != "" && !v.findRelationship(v.rt[tt.Name].Relationships, tt.SubjectRelation) { return fmt.Errorf("%s: subject-relation: %s: %w", resourceType.Name, tt.SubjectRelation, ErrorUnknownRelation) } } @@ -230,9 +210,13 @@ func (v *policy) validateConditionRelationshipAction(rt ResourceType, c Conditio return fmt.Errorf("%s: %w", c.Relation, ErrorUnknownRelation) } - for _, tn := range rel.TargetTypeNames { - if _, ok := v.rb[tn][c.ActionName]; !ok { - return fmt.Errorf("%s: %s: %s: %w", c.Relation, tn, c.ActionName, ErrorUnknownAction) + if c.ActionName == "" { + return nil + } + + for _, tt := range rel.TargetTypes { + if _, ok := v.rb[tt.Name][c.ActionName]; !ok { + return fmt.Errorf("%s: %s: %s: %w", c.Relation, tt.Name, c.ActionName, ErrorUnknownAction) } } @@ -287,6 +271,41 @@ func (v *policy) validateActionBindings() error { return nil } +// validateMemberRoleRelationship ensures that there's a valid `member_role` +// relationship exists in a given resource type +func (v *policy) validateMemberRoleRelationship(res ResourceType) error { + relationshipExists := false + targetOk := false + + for _, rel := range res.Relationships { + if rel.Relation != RoleOwnerMemberRoleRelation { + continue + } + + relationshipExists = true + + for i := 0; i < len(rel.TargetTypes) && !targetOk; i++ { + if rel.TargetTypes[i].Name == v.p.RBAC.RoleResource.Name { + targetOk = true + } + } + + if !targetOk { + break + } + } + + if !relationshipExists || !targetOk { + return fmt.Errorf( + "%w: role owner %s must have %s relation to %s", + ErrorMissingRelationship, res.Name, RoleOwnerMemberRoleRelation, + v.p.RBAC.RoleResource, + ) + } + + return nil +} + // validateRoles validates V2 role resource types to ensure that: // 1. role resource type has a valid owner relationship // 2. role-owner resource types have a valid member-role relationship points @@ -305,43 +324,8 @@ func (v *policy) validateRoles() error { } // role owner must have `member_role` relationship to `role` resource - ensureMemberRole := func() bool { - relationshipExists := false - targetOk := false - - for _, rel := range owner.Relationships { - if rel.Relation != RoleOwnerMemberRoleRelation { - continue - } - - relationshipExists = true - - for i := 0; i < len(rel.TargetTypes) && !targetOk; i++ { - if rel.TargetTypes[i].Name == v.p.RBAC.RoleResource { - targetOk = true - } - } - - for i := 0; i < len(rel.TargetTypeNames) && !targetOk; i++ { - if rel.TargetTypeNames[i] == v.p.RBAC.RoleResource { - targetOk = true - } - } - - if !targetOk { - return false - } - } - - return relationshipExists && targetOk - } - - if ok := ensureMemberRole(); !ok { - return fmt.Errorf( - "%w: role owner %s must have %s relation to %s", - ErrorMissingRelationship, roleOwnerName, RoleOwnerMemberRoleRelation, - v.p.RBAC.RoleResource, - ) + if err := v.validateMemberRoleRelationship(owner); err != nil { + return err } } @@ -351,16 +335,6 @@ func (v *policy) validateRoles() error { func (v *policy) expandActionBindings() { for _, bn := range v.p.ActionBindings { if u, ok := v.un[bn.TypeName]; ok { - for _, typeName := range u.ResourceTypeNames { - binding := ActionBinding{ - TypeName: typeName, - ActionName: bn.ActionName, - Conditions: bn.Conditions, - ConditionSets: bn.ConditionSets, - } - v.bn = append(v.bn, binding) - } - for _, resourceType := range u.ResourceTypes { binding := ActionBinding{ TypeName: resourceType.Name, @@ -391,9 +365,9 @@ func (v *policy) expandActionBindings() { // representing all the actions, as well as relationships and permissions for // the management of the roles themselves. func (v *policy) createV2RoleResourceType() { - role, ok := v.rt[v.p.RBAC.RoleResource] - if !ok { - panic("v2 role is specified but role resource type is not defined") + role := ResourceType{ + Name: v.p.RBAC.RoleResource.Name, + IDPrefix: v.p.RBAC.RoleResource.IDPrefix, } // 1. create a relationship for role owners @@ -406,8 +380,8 @@ func (v *policy) createV2RoleResourceType() { roleOwners.TargetTypes[i] = types.TargetType{Name: owner} } - // 2. create a list of relationships for all permissions - permsRel := make([]Relationship, 0, len(v.ac)) + // 2. create a list of relationships for all permissions and ownerships + roleRel := make([]Relationship, 0, len(v.ac)+1) for _, action := range v.ac { targettypes := make([]types.TargetType, len(v.p.RBAC.RoleSubjectTypes)) @@ -416,7 +390,7 @@ func (v *policy) createV2RoleResourceType() { targettypes[j] = types.TargetType{Name: subject, SubjectIdentifier: "*"} } - permsRel = append(permsRel, + roleRel = append(roleRel, Relationship{ Relation: action.Name + PermissionRelationSuffix, TargetTypes: targettypes, @@ -425,9 +399,9 @@ func (v *policy) createV2RoleResourceType() { } // 3. create a role resource type containing all the relationships shown above - permsRel = append(permsRel, roleOwners) + roleRel = append(roleRel, roleOwners) - role.Relationships = permsRel + role.Relationships = roleRel v.rt[role.Name] = role } @@ -436,15 +410,15 @@ func (v *policy) createV2RoleResourceType() { // The role-binding resources will be used to create a 3-way relationship // between a resource, a subject and a role func (v *policy) createRoleBindingResourceType() { - rolebinding, ok := v.rt[v.p.RBAC.RoleBindingResource] - if !ok { - panic("v2 role-binding is specified but role-binding resource type is not defined") + rolebinding := ResourceType{ + Name: v.p.RBAC.RoleBindingResource.Name, + IDPrefix: v.p.RBAC.RoleBindingResource.IDPrefix, } role := Relationship{ Relation: RolebindingRoleRelation, TargetTypes: []types.TargetType{ - {Name: v.p.RBAC.RoleResource}, + {Name: v.p.RBAC.RoleResource.Name}, }, } @@ -455,13 +429,13 @@ func (v *policy) createRoleBindingResourceType() { } // 3. create a list of action-bindings representing permissions for all the - // actions + // actions in the policy actionbindings := make([]ActionBinding, 0, len(v.ac)) for actionName := range v.ac { ab := ActionBinding{ ActionName: actionName, - TypeName: v.p.RBAC.RoleBindingResource, + TypeName: v.p.RBAC.RoleBindingResource.Name, ConditionSets: []types.ConditionSet{ { Conditions: []types.Condition{ @@ -488,7 +462,7 @@ func (v *policy) createRoleBindingResourceType() { // 4. create role-binding resource type rolebinding.Relationships = []Relationship{role, subjects} - v.rt[v.p.RBAC.RoleBindingResource] = rolebinding + v.rt[v.p.RBAC.RoleBindingResource.Name] = rolebinding } // expandRBACV2Relationships adds RBAC V2 relationships to all the resource @@ -509,7 +483,7 @@ func (v *policy) expandRBACV2Relationships() { memberRoleRelation := Relationship{ Relation: RoleOwnerMemberRoleRelation, TargetTypes: []types.TargetType{ - {Name: v.p.RBAC.RoleResource}, + {Name: v.p.RBAC.RoleResource.Name}, }, } @@ -525,7 +499,7 @@ func (v *policy) expandRBACV2Relationships() { // i.e. avail_role = from[0]->avail_role + from[1]->avail_role ... if resourceType.RoleBindingV2 != nil { - for _, from := range resourceType.RoleBindingV2.AvailableRolesFrom { + for _, from := range resourceType.RoleBindingV2.InheritPermissionsFrom { availableRoles = append(availableRoles, Condition{ RelationshipAction: &ConditionRelationshipAction{ Relation: from, @@ -553,30 +527,16 @@ func (v *policy) expandRBACV2Relationships() { func (v *policy) expandResourceTypes() { for name, resourceType := range v.rt { for i, rel := range resourceType.Relationships { - var typeNames []string - - targettypes := rel.TargetTypes - - for _, typeName := range rel.TargetTypeNames { - if u, ok := v.un[typeName]; ok { - // two kinds of relationships target types to be expanded - if len(u.ResourceTypes) > 0 { - // 1. target types with optional subject relation or subject identifier - targettypes = append(targettypes, u.ResourceTypes...) - } else { - // 2. simple target type names - typeNames = append(typeNames, u.ResourceTypeNames...) - } + targettypes := []types.TargetType{} + + for _, tt := range rel.TargetTypes { + if u, ok := v.un[tt.Name]; ok { + targettypes = append(targettypes, u.ResourceTypes...) } else { - typeNames = append(typeNames, typeName) + targettypes = append(targettypes, tt) } } - for _, tn := range typeNames { - targettypes = append(targettypes, types.TargetType{Name: tn}) - } - - resourceType.Relationships[i].TargetTypeNames = typeNames resourceType.Relationships[i].TargetTypes = targettypes } @@ -633,6 +593,9 @@ func (v *policy) Schema() []types.ResourceType { Name: actionName, } + // rbac V2 actions + res := v.rt[b.TypeName] + for _, c := range b.Conditions { var conditions []types.Condition @@ -653,12 +616,12 @@ func (v *policy) Schema() []types.ResourceType { } typeMap[b.TypeName].Relationships = append(typeMap[b.TypeName].Relationships, actionRel) - case c.RoleBindingV2 != nil: - conditions = v.RBAC().CreateRoleBindingConditionsForAction(actionName, c.RoleBindingV2.InheritGrantsFrom...) + case c.RoleBindingV2 != nil && res.RoleBindingV2 != nil: + conditions = v.RBAC().CreateRoleBindingConditionsForAction(actionName, res.RoleBindingV2.InheritPermissionsFrom...) // add role-binding v2 conditions to the resource, if not exists if _, ok := rbv2Actions[b.TypeName]; !ok { - rbv2Actions[b.TypeName] = v.RBAC().CreateRoleBindingActionsForResource(c.RoleBindingV2.InheritGrantsFrom...) + rbv2Actions[b.TypeName] = v.RBAC().CreateRoleBindingActionsForResource(res.RoleBindingV2.InheritPermissionsFrom...) } default: conditions = []types.Condition{ @@ -680,9 +643,9 @@ func (v *policy) Schema() []types.ResourceType { typeMap[resType].Actions = append(typeMap[resType].Actions, actions...) } - out := make([]types.ResourceType, len(v.p.ResourceTypes)) - for i, rt := range v.p.ResourceTypes { - out[i] = *typeMap[rt.Name] + out := make([]types.ResourceType, 0, len(typeMap)) + for _, rt := range typeMap { + out = append(out, *rt) } return out @@ -692,3 +655,13 @@ func (v *policy) Schema() []types.ResourceType { func (v *policy) RBAC() *RBAC { return v.p.RBAC } + +func (v *policy) findRelationship(rels []Relationship, name string) bool { + for _, rel := range rels { + if rel.Relation == name { + return true + } + } + + return false +} diff --git a/internal/iapl/policy_test.go b/internal/iapl/policy_test.go index 21061e69a..a309b4c77 100644 --- a/internal/iapl/policy_test.go +++ b/internal/iapl/policy_test.go @@ -25,8 +25,8 @@ func TestPolicy(t *testing.T) { Unions: []Union{ { Name: "foo", - ResourceTypeNames: []string{ - "foo", + ResourceTypes: []types.TargetType{ + {Name: "foo"}, }, }, }, @@ -46,8 +46,8 @@ func TestPolicy(t *testing.T) { Unions: []Union{ { Name: "bar", - ResourceTypeNames: []string{ - "baz", + ResourceTypes: []types.TargetType{ + {Name: "baz"}, }, }, }, @@ -67,8 +67,8 @@ func TestPolicy(t *testing.T) { Unions: []Union{ { Name: "bar", - ResourceTypeNames: []string{ - "baz", + ResourceTypes: []types.TargetType{ + {Name: "baz"}, }, }, }, @@ -86,8 +86,8 @@ func TestPolicy(t *testing.T) { Relationships: []Relationship{ { Relation: "bar", - TargetTypeNames: []string{ - "baz", + TargetTypes: []types.TargetType{ + {Name: "baz"}, }, }, }, @@ -107,8 +107,8 @@ func TestPolicy(t *testing.T) { Relationships: []Relationship{ { Relation: "bar", - TargetTypeNames: []string{ - "foo", + TargetTypes: []types.TargetType{ + {Name: "foo"}, }, }, }, @@ -139,8 +139,8 @@ func TestPolicy(t *testing.T) { Relationships: []Relationship{ { Relation: "bar", - TargetTypeNames: []string{ - "foo", + TargetTypes: []types.TargetType{ + {Name: "foo"}, }, }, }, @@ -211,8 +211,8 @@ func TestPolicy(t *testing.T) { Relationships: []Relationship{ { Relation: "bar", - TargetTypeNames: []string{ - "foo", + TargetTypes: []types.TargetType{ + {Name: "foo"}, }, }, }, @@ -224,9 +224,9 @@ func TestPolicy(t *testing.T) { Unions: []Union{ { Name: "buzz", - ResourceTypeNames: []string{ - "foo", - "baz", + ResourceTypes: []types.TargetType{ + {Name: "foo"}, + {Name: "baz"}, }, }, }, @@ -263,8 +263,8 @@ func TestPolicy(t *testing.T) { Relationships: []Relationship{ { Relation: "bar", - TargetTypeNames: []string{ - "foo", + TargetTypes: []types.TargetType{ + {Name: "foo"}, }, }, }, @@ -274,8 +274,8 @@ func TestPolicy(t *testing.T) { Relationships: []Relationship{ { Relation: "bar", - TargetTypeNames: []string{ - "foo", + TargetTypes: []types.TargetType{ + {Name: "foo"}, }, }, }, @@ -284,9 +284,9 @@ func TestPolicy(t *testing.T) { Unions: []Union{ { Name: "buzz", - ResourceTypeNames: []string{ - "foo", - "baz", + ResourceTypes: []types.TargetType{ + {Name: "foo"}, + {Name: "baz"}, }, }, }, @@ -323,8 +323,8 @@ func TestPolicy(t *testing.T) { Relationships: []Relationship{ { Relation: "bar", - TargetTypeNames: []string{ - "foo", + TargetTypes: []types.TargetType{ + {Name: "foo"}, }, }, }, @@ -334,8 +334,8 @@ func TestPolicy(t *testing.T) { Relationships: []Relationship{ { Relation: "bar", - TargetTypeNames: []string{ - "foo", + TargetTypes: []types.TargetType{ + {Name: "foo"}, }, }, }, @@ -344,9 +344,9 @@ func TestPolicy(t *testing.T) { Unions: []Union{ { Name: "buzz", - ResourceTypeNames: []string{ - "foo", - "baz", + ResourceTypes: []types.TargetType{ + {Name: "foo"}, + {Name: "baz"}, }, }, }, @@ -420,24 +420,16 @@ func TestPolicy(t *testing.T) { Name: "RBAC_OK", Input: PolicyDocument{ RBAC: &RBAC{ - RoleResource: "rolev2", + RoleResource: rbacResourceDefinition{"rolev2", "permrv2"}, + RoleBindingResource: rbacResourceDefinition{"role_binding", "permrbn"}, RoleSubjectTypes: []string{"user"}, RoleOwners: []string{"tenant"}, - RoleBindingResource: "role_binding", RoleBindingSubjects: []types.TargetType{{Name: "user"}}, }, ResourceTypes: []ResourceType{ { Name: "tenant", }, - { - Name: "rolev2", - IDPrefix: "permrv2", - }, - { - Name: "role_binding", - IDPrefix: "permrbn", - }, { Name: "user", IDPrefix: "idntusr", @@ -466,10 +458,10 @@ func TestPolicy(t *testing.T) { func defaultRBAC() RBAC { return RBAC{ - RoleResource: "rolev2", + RoleResource: rbacResourceDefinition{"rolev2", "permrv2"}, + RoleBindingResource: rbacResourceDefinition{"role_binding", "permrbn"}, RoleSubjectTypes: []string{"user", "client"}, RoleOwners: []string{"tenant"}, - RoleBindingResource: "role_binding", RoleBindingSubjects: []types.TargetType{{Name: "user"}, {Name: "client"}, {Name: "group", SubjectRelation: "member"}}, } } diff --git a/internal/iapl/rbac.go b/internal/iapl/rbac.go index c988fc8e7..56aa98a41 100644 --- a/internal/iapl/rbac.go +++ b/internal/iapl/rbac.go @@ -45,14 +45,15 @@ const ( // ResourceRoleBindingV2 describes the relationships that will be created // for a resource to support role-binding V2 type ResourceRoleBindingV2 struct { - // AvailableRolesFrom is the list of resource types that can provide roles to this resource + // InheritPermissionsFrom is the list of resource types that can provide roles + // and grants to this resource // Note that not all roles are available to all resources. This relationship is used to // determine which roles are available to a resource. // Before creating a role binding for a resource, one should check whether or // not the role is available for the resource. // // Also see the RoleOwners field in the RBAC struct - AvailableRolesFrom []string + InheritPermissionsFrom []string } /* @@ -70,8 +71,8 @@ For example, consider the following spicedb schema: definition organization { relation parent: organization - relation member: user | client | organization#member - relation member_role: role | organization#member_role + relation member: user | client + relation member_role: role relation grant: rolebinding permissions rolebinding_list: grant->rolebinding_list @@ -87,7 +88,7 @@ For example, consider the following spicedb schema: relation rolebinding_delete_rel: user:* | client:* } - definition role_binding { + definition rolebinding { relation role: role relation subject: user | group#member permission view_organization = subject & role->view_organization @@ -98,16 +99,16 @@ For example, consider the following spicedb schema: ``` in IAPL policy terms: -- the RoleResource would be "role" -- the RoleBindingResource would be "role_binding", +- the RoleResource would be "{name: role, idprefix: someprefix}" +- the RoleBindingResource would be "{name: rolebinding, idprefix: someprefix}", - the RoleRelationshipSubject would be `[user, client]`. - the RoleBindingSubjects would be `[{name: user}, {name: group, subjectrelation: member}]`. */ type RBAC struct { // RoleResource is the name of the resource type that represents a role. - RoleResource string + RoleResource rbacResourceDefinition // RoleBindingResource is the name of the resource type that represents a role binding. - RoleBindingResource string + RoleBindingResource rbacResourceDefinition // RoleSubjectTypes is a list of subject types that the relationships in a // role resource will contain, see the example above. RoleSubjectTypes []string @@ -127,6 +128,13 @@ type RBAC struct { roleownersset map[string]struct{} } +// rbacResourceDefinition is a struct to define a resource type for a role +// and role-bindings +type rbacResourceDefinition struct { + Name string + IDPrefix string +} + // CreateRoleBindingConditionsForAction creates the conditions that is used for role binding v2, // for a given action name. e.g. for a doc_read action, it will create the following conditions: // doc_read = grant->doc_read + from[0]->doc_read + ... from[n]->doc_read diff --git a/internal/query/relations.go b/internal/query/relations.go index 7f845d81e..f7e317b8b 100644 --- a/internal/query/relations.go +++ b/internal/query/relations.go @@ -858,7 +858,7 @@ func (e *engine) ListRoles(ctx context.Context, resource types.Resource) ([]type return nil, err } - if res.Type == e.rbac.RoleResource { + if res.Type == e.rbac.RoleResource.Name { continue } diff --git a/internal/query/relations_test.go b/internal/query/relations_test.go index 2235c6123..cb322d789 100644 --- a/internal/query/relations_test.go +++ b/internal/query/relations_test.go @@ -75,8 +75,8 @@ func testPolicy() iapl.Policy { Relationships: []iapl.Relationship{ { Relation: "parent", - TargetTypeNames: []string{ - "tenant", + TargetTypes: []types.TargetType{ + {Name: "tenant"}, }, }, }, diff --git a/policy.example.yaml b/policy.example.yaml index 2cf56876b..5c52a1f75 100644 --- a/policy.example.yaml +++ b/policy.example.yaml @@ -1,20 +1,20 @@ rbac: - roleresource: rolev2 + roleresource: + name: rolev2 + idprefix: permrv2 + rolebindingresource: + name: rolebinding + idprefix: permrbn rolesubjecttypes: - user - client roleowners: - tenant - rolebindingresource: role_binding rolebindingsubjects: - name: user - name: client - name: group subjectrelation: member - # rolebindingpermissionsprefix generates permissions to manage role bindings, - # e.g. rolebinding_create, rolebinding_get, rolebinding_delete, etc. - rolebindingpermissionsprefix: rolebinding - grantrelationship: grant unions: - name: group_member @@ -40,9 +40,9 @@ unions: - name: tenant subjectrelation: parent - name: subject - resourcetypenames: - - user - - client + resourcetypes: + - name: user + - name: client - name: group_parent resourcetypes: - name: group @@ -62,12 +62,8 @@ resourcetypes: idprefix: permrol relationships: - relation: subject - targettypenames: - - subject - - name: rolev2 - idprefix: permrv2 - - name: role_binding - idprefix: permrbn + targettypes: + - name: subject - name: user idprefix: idntusr @@ -78,18 +74,18 @@ resourcetypes: idprefix: idntgrp rolebindingv2: &rolesFromParent - availablerolesfrom: + inheritpermissionsfrom: - parent relationships: - relation: parent - targettypenames: - - group_parent + targettypes: + - name: group_parent - relation: member - targettypenames: - - group_member + targettypes: + - name: group_member - relation: grant targettypes: - - name: role_binding + - name: rolebinding - name: tenant idprefix: tnntten @@ -97,27 +93,27 @@ resourcetypes: *rolesFromParent relationships: - relation: parent - targettypenames: - - tenant_parent + targettypes: + - name: tenant_parent - relation: member - targettypenames: - - tenant_member + targettypes: + - name: tenant_member - relation: grant targettypes: - - name: role_binding + - name: rolebinding - name: loadbalancer idprefix: loadbal rolebindingv2: - availablerolesfrom: + inheritpermissionsfrom: - owner relationships: - relation: owner - targettypenames: - - resourceowner_relationship + targettypes: + - name: resourceowner_relationship - relation: grant targettypes: - - name: role_binding + - name: rolebinding actions: - name: role_create @@ -155,123 +151,117 @@ actionbindings: - actionname: role_create typename: resourceowner conditions: - - &rbv2parent - rolebindingv2: - inheritgrantsfrom: - - parent + - rolebindingv2: {} - rolebinding: {} - actionname: role_create typename: group conditions: - - *rbv2parent + - rolebindingv2: {} - actionname: role_get typename: resourceowner conditions: - - *rbv2parent + - rolebindingv2: {} - rolebinding: {} - actionname: role_get typename: group conditions: - - *rbv2parent + - rolebindingv2: {} - actionname: role_list typename: resourceowner conditions: - - *rbv2parent + - rolebindingv2: {} - rolebinding: {} - actionname: role_list typename: group conditions: - - *rbv2parent + - rolebindingv2: {} - actionname: role_update typename: resourceowner conditions: - - *rbv2parent + - rolebindingv2: {} - rolebinding: {} - actionname: role_update typename: group conditions: - - *rbv2parent + - rolebindingv2: {} - actionname: role_delete typename: resourceowner conditions: - - *rbv2parent + - rolebindingv2: {} - rolebinding: {} - actionname: role_delete typename: group conditions: - - *rbv2parent + - rolebindingv2: {} # loadbalancer management - permissions on loadbalancer - actionname: loadbalancer_get typename: loadbalancer conditions: - rolebinding: {} - - &rbv2owner - rolebindingv2: - inheritgrantsfrom: - - owner + - rolebindingv2: {} - actionname: loadbalancer_update typename: loadbalancer conditions: - rolebinding: {} - - *rbv2owner + - rolebindingv2: {} - actionname: loadbalancer_delete typename: loadbalancer conditions: - rolebinding: {} - - *rbv2owner + - rolebindingv2: {} # loadbalancer management - permissions on owner - actionname: loadbalancer_create typename: resourceowner conditions: - - *rbv2parent + - rolebindingv2: {} - rolebinding: {} - actionname: loadbalancer_create typename: group conditions: - - *rbv2parent + - rolebindingv2: {} - actionname: loadbalancer_get typename: resourceowner conditions: - - *rbv2parent + - rolebindingv2: {} - rolebinding: {} - actionname: loadbalancer_get typename: group conditions: - - *rbv2parent + - rolebindingv2: {} - actionname: loadbalancer_list typename: resourceowner conditions: - - *rbv2parent + - rolebindingv2: {} - rolebinding: {} - actionname: loadbalancer_list typename: group conditions: - - *rbv2parent + - rolebindingv2: {} - actionname: loadbalancer_update typename: resourceowner conditions: - - *rbv2parent + - rolebindingv2: {} - rolebinding: {} - actionname: loadbalancer_update typename: group conditions: - - *rbv2parent + - rolebindingv2: {} - actionname: loadbalancer_delete typename: resourceowner conditions: - - *rbv2parent + - rolebindingv2: {} - rolebinding: {} - actionname: loadbalancer_delete typename: group conditions: - - *rbv2parent + - rolebindingv2: {}