Apply suggestions from code review

Co-authored-by: John Schaeffer <[email protected]> Signed-off-by: Bailin He <[email protected]>
infratographer · Apr 4, 2024 · 88ef2b0 · 88ef2b0
1 parent 852c142
commit 88ef2b0
Show file tree

Hide file tree

Showing 9 changed files with 254 additions and 268 deletions.
diff --git 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
@@ -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,39 +181,59 @@ 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
     RelationshipAction *ConditionRelationshipAction
   }
 ```
 
-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
@@ -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"},
 				},
 			},
 		},