rbac: checking user defined role referential integrity

Signed-off-by: Mike Cutsail <[email protected]>

cmd/argocd/commands/admin/settings_rbac.go

@@ -430,6 +430,9 @@ func checkPolicy(subject, action, resource, subResource, builtinPolicy, userPoli
 			log.Fatalf("could not set user policy: %v", err)
 			return false
 		}
+		if err := enf.CheckUserDefinedRoleReferentialIntegrity(); err != nil {
+			log.Fatal(err.Error())
+		}
 	}
 
 	// User could have used a mutation of the resource name (i.e. 'cert' for

util/rbac/rbac.go

@@ -51,6 +51,8 @@ type CasbinEnforcer interface {
 	EnableEnforce(bool)
 	AddFunction(name string, function govaluate.ExpressionFunction)
 	GetGroupingPolicy() ([][]string, error)
+	GetAllRoles() ([]string, error)
+	GetAllSubjects() ([]string, error)
 }
 
 // Enforcer is a wrapper around an Casbin enforcer that:
@@ -90,16 +92,16 @@ func (e *Enforcer) invalidateCache(actions ...func()) {
 	e.enforcerCache.Flush()
 }
 
-func (e *Enforcer) getCabinEnforcer(project string, policy string) CasbinEnforcer {
-	res, err := e.tryGetCabinEnforcer(project, policy)
+func (e *Enforcer) getCasbinEnforcer(project string, policy string) CasbinEnforcer {
+	res, err := e.tryGetCasbinEnforcer(project, policy)
 	if err != nil {
 		panic(err)
 	}
 	return res
 }
 
-// tryGetCabinEnforcer returns the cached enforcer for the given optional project and project policy.
-func (e *Enforcer) tryGetCabinEnforcer(project string, policy string) (CasbinEnforcer, error) {
+// tryGetCasbinEnforcer returns the cached enforcer for the given optional project and project policy.
+func (e *Enforcer) tryGetCasbinEnforcer(project string, policy string) (CasbinEnforcer, error) {
 	e.lock.Lock()
 	defer e.lock.Unlock()
 	var cached *cachedEnforcer
@@ -188,10 +190,43 @@ func (e *Enforcer) EnableEnforce(s bool) {
 
 // LoadPolicy executes casbin.Enforcer functionality and will invalidate cache if required.
 func (e *Enforcer) LoadPolicy() error {
-	_, err := e.tryGetCabinEnforcer("", "")
+	_, err := e.tryGetCasbinEnforcer("", "")
 	return err
 }
 
+// CheckUserDefinedRoleReferentialIntegrity iterates over roles and policies to
+func (e *Enforcer) CheckUserDefinedRoleReferentialIntegrity() error {
+	casbinEnforcer, err := e.tryGetCasbinEnforcer("", "")
+	if err != nil {
+		return err
+	}
+	allRoles, err := casbinEnforcer.GetAllRoles()
+	if err != nil {
+		return err
+	}
+	allSubjects, err := casbinEnforcer.GetAllSubjects()
+	if err != nil {
+		return err
+	}
+	notFound := make([]string, 0)
+	for _, role := range allRoles {
+		found := false
+		for _, subj := range allSubjects {
+			if role == subj {
+				found = true
+				break
+			}
+		}
+		if !found {
+			notFound = append(notFound, role)
+		}
+	}
+	if len(notFound) > 0 {
+		return fmt.Errorf("user defined roles not found in policies: %s", strings.Join(notFound, ","))
+	}
+	return nil
+}
+
 // Glob match func
 func globMatchFunc(args ...interface{}) (interface{}, error) {
 	if len(args) < 2 {
@@ -237,7 +272,7 @@ func (e *Enforcer) SetClaimsEnforcerFunc(claimsEnforcer ClaimsEnforcerFunc) {
 // Enforce is a wrapper around casbin.Enforce to additionally enforce a default role and a custom
 // claims function
 func (e *Enforcer) Enforce(rvals ...interface{}) bool {
-	return enforce(e.getCabinEnforcer("", ""), e.defaultRole, e.claimsEnforcerFunc, rvals...)
+	return enforce(e.getCasbinEnforcer("", ""), e.defaultRole, e.claimsEnforcerFunc, rvals...)
 }
 
 // EnforceErr is a convenience helper to wrap a failed enforcement with a detailed error about the request
@@ -281,7 +316,7 @@ func (e *Enforcer) EnforceRuntimePolicy(project string, policy string, rvals ...
 // user-defined policy. This allows any explicit denies of the built-in, and user-defined policies
 // to override the run-time policy. Runs normal enforcement if run-time policy is empty.
 func (e *Enforcer) CreateEnforcerWithRuntimePolicy(project string, policy string) CasbinEnforcer {
-	return e.getCabinEnforcer(project, policy)
+	return e.getCasbinEnforcer(project, policy)
 }
 
 // EnforceWithCustomEnforcer wraps enforce with an custom enforcer
@@ -331,7 +366,14 @@ func (e *Enforcer) SetUserPolicy(policy string) error {
 	e.invalidateCache(func() {
 		e.adapter.userDefinedPolicy = policy
 	})
-	return e.LoadPolicy()
+	err := e.LoadPolicy()
+	if err != nil {
+		return err
+	}
+	if err := e.CheckUserDefinedRoleReferentialIntegrity(); err != nil {
+		log.Warning(err.Error())
+	}
+	return nil
 }
 
 // newInformers returns an informer which watches updates on the rbac configmap

util/rbac/rbac_test.go

@@ -272,6 +272,28 @@ func TestNoPolicy(t *testing.T) {
 	assert.False(t, enf.Enforce("admin", "applications", "delete", "foo/bar"))
 }
 
+func TestCheckUserDefinedPolicyReferentialIntegrity(t *testing.T) {
+	kubeclientset := fake.NewSimpleClientset(fakeConfigMap())
+	enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
+
+	policy := `
+p, role:depA, *, get, foo/obj, allow
+p, role:depB, *, get, foo/obj, deny
+g, depA, role:depA
+g, depB, role:depB
+`
+	assert.NoError(t, enf.SetUserPolicy(policy))
+	assert.NoError(t, enf.CheckUserDefinedRoleReferentialIntegrity())
+
+	policy = `
+p, role:depA, *, get, foo/obj, allow
+p, role:depB, *, get, foo/obj, deny
+g, depC, role:depC
+`
+	assert.NoError(t, enf.SetUserPolicy(policy))
+	assert.Error(t, enf.CheckUserDefinedRoleReferentialIntegrity())
+}
+
 // TestClaimsEnforcerFunc tests
 func TestClaimsEnforcerFunc(t *testing.T) {
 	kubeclientset := fake.NewSimpleClientset()