From 3e00b38e2fbd40d328b7b667d793efa6316ed0ef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Toma=C5=BE=20Jerman?= <tomaz.jerman121@gmail.com>
Date: Mon, 12 Dec 2022 15:29:15 +0100
Subject: [PATCH] Fix improper role resolution in workflow RBAC grant functions

---
 server/system/automation/rbac_handler.go   | 47 +++++++++++++++++-----
 server/system/automation/rbac_handler.yaml | 14 ++++---
 2 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/server/system/automation/rbac_handler.go b/server/system/automation/rbac_handler.go
index 330a54304..cb0bb338b 100644
--- a/server/system/automation/rbac_handler.go
+++ b/server/system/automation/rbac_handler.go
@@ -18,15 +18,11 @@ type (
 		FindByAny(ctx context.Context, identifier interface{}) (*types.User, error)
 	}
 
-	rbacRoleService interface {
-		Membership(ctx context.Context, userID uint64) (types.RoleMemberSet, error)
-	}
-
 	rbacHandler struct {
 		reg  rbacHandlerRegistry
 		rbac rbacService
 		user rbacUserService
-		role rbacRoleService
+		role roleService
 	}
 
 	auxResource struct{ res string }
@@ -38,7 +34,7 @@ func (r auxResource) RbacResource() string {
 	return r.res
 }
 
-func RbacHandler(reg rbacHandlerRegistry, rbac rbacService, user rbacUserService, role rbacRoleService) *rbacHandler {
+func RbacHandler(reg rbacHandlerRegistry, rbac rbacService, user rbacUserService, role roleService) *rbacHandler {
 	h := &rbacHandler{
 		reg:  reg,
 		rbac: rbac,
@@ -51,15 +47,48 @@ func RbacHandler(reg rbacHandlerRegistry, rbac rbacService, user rbacUserService
 }
 
 func (h rbacHandler) allow(ctx context.Context, args *rbacAllowArgs) (err error) {
-	return h.grant(ctx, rbac.AllowRule(args.roleID, args.Resource.RbacResource(), args.Operation))
+	r, err := lookupRole(ctx, h.role, rolesLookupArgs{
+		hasLookup:    args.hasRole,
+		Lookup:       args.Role,
+		lookupID:     args.roleID,
+		lookupHandle: args.roleHandle,
+		lookupRes:    args.roleRes,
+	})
+	if err != nil {
+		return
+	}
+
+	return h.grant(ctx, rbac.AllowRule(r.ID, args.Resource.RbacResource(), args.Operation))
 }
 
 func (h rbacHandler) deny(ctx context.Context, args *rbacDenyArgs) (err error) {
-	return h.grant(ctx, rbac.DenyRule(args.roleID, args.Resource.RbacResource(), args.Operation))
+	r, err := lookupRole(ctx, h.role, rolesLookupArgs{
+		hasLookup:    args.hasRole,
+		Lookup:       args.Role,
+		lookupID:     args.roleID,
+		lookupHandle: args.roleHandle,
+		lookupRes:    args.roleRes,
+	})
+	if err != nil {
+		return
+	}
+
+	return h.grant(ctx, rbac.DenyRule(r.ID, args.Resource.RbacResource(), args.Operation))
 }
 
 func (h rbacHandler) inherit(ctx context.Context, args *rbacInheritArgs) (err error) {
-	return h.grant(ctx, rbac.InheritRule(args.roleID, args.Resource.RbacResource(), args.Operation))
+	r, err := lookupRole(ctx, h.role, rolesLookupArgs{
+		hasLookup:    args.hasRole,
+		Lookup:       args.Role,
+		lookupID:     args.roleID,
+		lookupHandle: args.roleHandle,
+		lookupRes:    args.roleRes,
+	})
+	if err != nil {
+		return
+	}
+
+	return h.grant(ctx, rbac.InheritRule(r.ID, args.Resource.RbacResource(), args.Operation))
 }
 
 // verifies grant op (granter needs to be allowed to grant on the component
diff --git a/server/system/automation/rbac_handler.yaml b/server/system/automation/rbac_handler.yaml
index dbda5706c..ccc4bb7a0 100644
--- a/server/system/automation/rbac_handler.yaml
+++ b/server/system/automation/rbac_handler.yaml
@@ -3,6 +3,13 @@ imports:
   - github.com/cortezaproject/corteza/server/pkg/rbac
 
 params:
+  roleLookup: &roleLookup
+    required: true
+    types:
+      - { wf: ID,     }
+      - { wf: Handle, }
+      - { wf: Role,   suffix: res }
+
   manageParams: &manageParams
     resource:
       required: true
@@ -10,12 +17,7 @@ params:
         # Note that we can not accept string here because
         # we need whole resource with all the info loaded
         - { wf: RbacResource }
-    role:
-      required: true
-      types:
-        - { wf: ID }
-        - { wf: Handle }
-        - { wf: Role,   suffix: res }
+    role: *roleLookup
     operation:
       required: true
       types: