diff --git a/server/cluster/cluster_test.go b/server/cluster/cluster_test.go index 42b64307a5722..37f726968b3c2 100644 --- a/server/cluster/cluster_test.go +++ b/server/cluster/cluster_test.go @@ -39,7 +39,7 @@ func newServerInMemoryCache() *servercache.Cache { func newNoopEnforcer() *rbac.Enforcer { enf := rbac.NewEnforcer(fake.NewSimpleClientset(test.NewFakeConfigMap()), test.FakeArgoCDNamespace, common.ArgoCDConfigMapName, nil) - enf.Enforcer.EnableEnforce(false) + enf.EnableEnforce(false) return enf } diff --git a/server/rbacpolicy/rbacpolicy.go b/server/rbacpolicy/rbacpolicy.go index 7fff88962238b..e38a2183afae1 100644 --- a/server/rbacpolicy/rbacpolicy.go +++ b/server/rbacpolicy/rbacpolicy.go @@ -105,16 +105,18 @@ func (p *RBACPolicyEnforcer) EnforceClaims(claims jwt.Claims, rvals ...interface // Check if the request is for an application resource. We have special enforcement which takes // into consideration the project's token and group bindings var runtimePolicy string + var projName string proj := p.getProjectFromRequest(rvals...) if proj != nil { if IsProjectSubject(subject) { return p.enforceProjectToken(subject, proj, rvals...) } runtimePolicy = proj.ProjectPoliciesString() + projName = proj.Name } // NOTE: This calls prevent multiple creation of the wrapped enforcer - enforcer := p.enf.CreateEnforcerWithRuntimePolicy(runtimePolicy) + enforcer := p.enf.CreateEnforcerWithRuntimePolicy(projName, runtimePolicy) // Check the subject. This is typically the 'admin' case. // NOTE: the call to EnforceWithCustomEnforcer will also consider the default role @@ -191,6 +193,5 @@ func (p *RBACPolicyEnforcer) enforceProjectToken(subject string, proj *v1alpha1. } vals := append([]interface{}{subject}, rvals[1:]...) - return p.enf.EnforceRuntimePolicy(proj.ProjectPoliciesString(), vals...) - + return p.enf.EnforceRuntimePolicy(proj.Name, proj.ProjectPoliciesString(), vals...) } diff --git a/server/rbacpolicy/rbacpolicy_test.go b/server/rbacpolicy/rbacpolicy_test.go index 99c26dca964c3..657966369eee7 100644 --- a/server/rbacpolicy/rbacpolicy_test.go +++ b/server/rbacpolicy/rbacpolicy_test.go @@ -107,6 +107,33 @@ p, cam, applications, %s/argoproj.io/Rollout/resume, my-proj/*, allow assert.False(t, enf.Enforce(claims, "applications", ActionAction+"/argoproj.io/Rollout/resume", "my-proj/my-app")) } +func TestInvalidatedCache(t *testing.T) { + kubeclientset := fake.NewSimpleClientset(test.NewFakeConfigMap()) + projLister := test.NewFakeProjLister(newFakeProj()) + enf := rbac.NewEnforcer(kubeclientset, test.FakeArgoCDNamespace, common.ArgoCDConfigMapName, nil) + enf.EnableLog(true) + _ = enf.SetBuiltinPolicy(`p, alice, applications, create, my-proj/*, allow`) + _ = enf.SetUserPolicy(`p, bob, applications, create, my-proj/*, allow`) + rbacEnf := NewRBACPolicyEnforcer(enf, projLister) + enf.SetClaimsEnforcerFunc(rbacEnf.EnforceClaims) + + claims := jwt.MapClaims{"sub": "alice"} + assert.True(t, enf.Enforce(claims, "applications", "create", "my-proj/my-app")) + claims = jwt.MapClaims{"sub": "bob"} + assert.True(t, enf.Enforce(claims, "applications", "create", "my-proj/my-app")) + + _ = enf.SetBuiltinPolicy(`p, alice, applications, create, my-proj2/*, allow`) + _ = enf.SetUserPolicy(`p, bob, applications, create, my-proj2/*, allow`) + claims = jwt.MapClaims{"sub": "alice"} + assert.True(t, enf.Enforce(claims, "applications", "create", "my-proj2/my-app")) + claims = jwt.MapClaims{"sub": "bob"} + assert.True(t, enf.Enforce(claims, "applications", "create", "my-proj2/my-app")) + claims = jwt.MapClaims{"sub": "alice"} + assert.False(t, enf.Enforce(claims, "applications", "create", "my-proj/my-app")) + claims = jwt.MapClaims{"sub": "bob"} + assert.False(t, enf.Enforce(claims, "applications", "create", "my-proj/my-app")) +} + func TestGetScopes_DefaultScopes(t *testing.T) { rbacEnforcer := NewRBACPolicyEnforcer(nil, nil) diff --git a/util/rbac/rbac.go b/util/rbac/rbac.go index f9e09596299c5..908f7c5ad8f6e 100644 --- a/util/rbac/rbac.go +++ b/util/rbac/rbac.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "strings" + "sync" "time" "github.com/argoproj/argo-cd/v2/util/assets" @@ -16,6 +17,7 @@ import ( "github.com/casbin/casbin/model" "github.com/casbin/casbin/util" jwt "github.com/dgrijalva/jwt-go/v4" + gocache "github.com/patrickmn/go-cache" log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -35,9 +37,20 @@ const ( ConfigMapMatchModeKey = "policy.matchMode" GlobMatchMode = "glob" RegexMatchMode = "regex" - defaultRBACSyncPeriod = 10 * time.Minute + + defaultRBACSyncPeriod = 10 * time.Minute ) +// CasbinEnforcer represents methods that must be implemented by a Casbin enforces +type CasbinEnforcer interface { + EnableLog(bool) + Enforce(rvals ...interface{}) bool + LoadPolicy() error + EnableEnforce(bool) + AddFunction(name string, function func(args ...interface{}) (interface{}, error)) + GetGroupingPolicy() [][]string +} + // Enforcer is a wrapper around an Casbin enforcer that: // * is backed by a kubernetes config map // * has a predefined RBAC model @@ -45,8 +58,11 @@ const ( // * supports a user-defined policy // * supports a custom JWT claims enforce function type Enforcer struct { - *casbin.Enforcer + lock sync.Mutex + enforcerCache *gocache.Cache adapter *argocdAdapter + enableLog bool + enabled bool clientset kubernetes.Interface namespace string configmap string @@ -56,11 +72,81 @@ type Enforcer struct { matchMode string } +// cachedEnforcer holds the Casbin enforcer instances and optional custom project policy +type cachedEnforcer struct { + enforcer CasbinEnforcer + policy string +} + +func (e *Enforcer) invalidateCache(actions ...func()) { + e.lock.Lock() + defer e.lock.Unlock() + + for _, action := range actions { + action() + } + e.enforcerCache.Flush() +} + +func (e *Enforcer) getCabinEnforcer(project string, policy string) CasbinEnforcer { + res, err := e.tryGetCabinEnforcer(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) { + e.lock.Lock() + defer e.lock.Unlock() + var cached *cachedEnforcer + val, ok := e.enforcerCache.Get(project) + if ok { + if c, ok := val.(*cachedEnforcer); ok && c.policy == policy { + cached = c + } + } + if cached != nil { + return cached.enforcer, nil + } + var err error + var enforcer CasbinEnforcer + if policy != "" { + if enforcer, err = newEnforcerSafe(e.model, newAdapter(e.adapter.builtinPolicy, e.adapter.userDefinedPolicy, policy)); err != nil { + // fallback to default policy if project policy is invalid + log.Errorf("Failed to load project '%s' policy", project) + enforcer, err = newEnforcerSafe(e.model, e.adapter) + } + } else { + enforcer, err = newEnforcerSafe(e.model, e.adapter) + } + if err != nil { + return nil, err + } + + matchFunc := globMatchFunc + if e.matchMode == RegexMatchMode { + matchFunc = util.RegexMatchFunc + } + enforcer.AddFunction("globOrRegexMatch", matchFunc) + enforcer.EnableLog(e.enableLog) + enforcer.EnableEnforce(e.enabled) + e.enforcerCache.SetDefault(project, &cachedEnforcer{enforcer: enforcer, policy: policy}) + return enforcer, nil +} + // ClaimsEnforcerFunc is func template to enforce a JWT claims. The subject is replaced type ClaimsEnforcerFunc func(claims jwt.Claims, rvals ...interface{}) bool -func newEnforcerSafe(params ...interface{}) (e *casbin.Enforcer, err error) { - enfs, err := casbin.NewEnforcerSafe(params...) +func newEnforcerSafe(params ...interface{}) (e CasbinEnforcer, err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("%v", r) + e = nil + } + }() + enfs := casbin.NewCachedEnforcer(params...) if err != nil { return nil, err } @@ -72,22 +158,38 @@ func newEnforcerSafe(params ...interface{}) (e *casbin.Enforcer, err error) { func NewEnforcer(clientset kubernetes.Interface, namespace, configmap string, claimsEnforcer ClaimsEnforcerFunc) *Enforcer { adapter := newAdapter("", "", "") builtInModel := newBuiltInModel() - enf, err := newEnforcerSafe(builtInModel, adapter) - if err != nil { - panic(err) - } - enf.EnableLog(false) return &Enforcer{ - Enforcer: enf, + enforcerCache: gocache.New(time.Hour, time.Hour), adapter: adapter, clientset: clientset, namespace: namespace, configmap: configmap, model: builtInModel, claimsEnforcerFunc: claimsEnforcer, + enabled: true, } } +// EnableLog executes casbin.Enforcer functionality. +func (e *Enforcer) EnableLog(s bool) { + e.invalidateCache(func() { + e.enableLog = s + }) +} + +// EnableEnforce executes casbin.Enforcer functionality and will invalidate cache if required. +func (e *Enforcer) EnableEnforce(s bool) { + e.invalidateCache(func() { + e.enabled = s + }) +} + +// LoadPolicy executes casbin.Enforcer functionality and will invalidate cache if required. +func (e *Enforcer) LoadPolicy() error { + _, err := e.tryGetCabinEnforcer("", "") + return err +} + // Glob match func func globMatchFunc(args ...interface{}) (interface{}, error) { if len(args) < 2 { @@ -108,16 +210,11 @@ func globMatchFunc(args ...interface{}) (interface{}, error) { // SetMatchMode set match mode on runtime, glob match or regex match func (e *Enforcer) SetMatchMode(mode string) { - if mode == RegexMatchMode { - e.matchMode = RegexMatchMode - } else { - e.matchMode = GlobMatchMode - } - e.Enforcer.AddFunction("globOrRegexMatch", func(args ...interface{}) (interface{}, error) { + e.invalidateCache(func() { if mode == RegexMatchMode { - return util.RegexMatchFunc(args...) + e.matchMode = RegexMatchMode } else { - return globMatchFunc(args...) + e.matchMode = GlobMatchMode } }) } @@ -138,7 +235,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.Enforcer, e.defaultRole, e.claimsEnforcerFunc, rvals...) + return enforce(e.getCabinEnforcer("", ""), e.defaultRole, e.claimsEnforcerFunc, rvals...) } // EnforceErr is a convenience helper to wrap a failed enforcement with a detailed error about the request @@ -173,36 +270,25 @@ func (e *Enforcer) EnforceErr(rvals ...interface{}) error { // EnforceRuntimePolicy enforces a policy defined at run-time which augments the built-in and // 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) EnforceRuntimePolicy(policy string, rvals ...interface{}) bool { - enf := e.CreateEnforcerWithRuntimePolicy(policy) +func (e *Enforcer) EnforceRuntimePolicy(project string, policy string, rvals ...interface{}) bool { + enf := e.CreateEnforcerWithRuntimePolicy(project, policy) return e.EnforceWithCustomEnforcer(enf, rvals...) } // CreateEnforcerWithRuntimePolicy creates an enforcer with a policy defined at run-time which augments the built-in and // 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(policy string) *casbin.Enforcer { - var enf *casbin.Enforcer - var err error - if policy == "" { - enf = e.Enforcer - } else { - enf, err = newEnforcerSafe(newBuiltInModel(), newAdapter(e.adapter.builtinPolicy, e.adapter.userDefinedPolicy, policy)) - if err != nil { - log.Warnf("invalid runtime policy: %s", policy) - enf = e.Enforcer - } - } - return enf +func (e *Enforcer) CreateEnforcerWithRuntimePolicy(project string, policy string) CasbinEnforcer { + return e.getCabinEnforcer(project, policy) } // EnforceWithCustomEnforcer wraps enforce with an custom enforcer -func (e *Enforcer) EnforceWithCustomEnforcer(enf *casbin.Enforcer, rvals ...interface{}) bool { +func (e *Enforcer) EnforceWithCustomEnforcer(enf CasbinEnforcer, rvals ...interface{}) bool { return enforce(enf, e.defaultRole, e.claimsEnforcerFunc, rvals...) } // enforce is a helper to additionally check a default role and invoke a custom claims enforcement function -func enforce(enf *casbin.Enforcer, defaultRole string, claimsEnforcerFunc ClaimsEnforcerFunc, rvals ...interface{}) bool { +func enforce(enf CasbinEnforcer, defaultRole string, claimsEnforcerFunc ClaimsEnforcerFunc, rvals ...interface{}) bool { // check the default role if defaultRole != "" && len(rvals) >= 2 { if enf.Enforce(append([]interface{}{defaultRole}, rvals[1:]...)...) { @@ -231,13 +317,17 @@ func enforce(enf *casbin.Enforcer, defaultRole string, claimsEnforcerFunc Claims // SetBuiltinPolicy sets a built-in policy, which augments any user defined policies func (e *Enforcer) SetBuiltinPolicy(policy string) error { - e.adapter.builtinPolicy = policy + e.invalidateCache(func() { + e.adapter.builtinPolicy = policy + }) return e.LoadPolicy() } // SetUserPolicy sets a user policy, augmenting the built-in policy func (e *Enforcer) SetUserPolicy(policy string) error { - e.adapter.userDefinedPolicy = policy + e.invalidateCache(func() { + e.adapter.userDefinedPolicy = policy + }) return e.LoadPolicy() } diff --git a/util/rbac/rbac_test.go b/util/rbac/rbac_test.go index 68844ee007752..8e8f01d3b2b4c 100644 --- a/util/rbac/rbac_test.go +++ b/util/rbac/rbac_test.go @@ -236,9 +236,9 @@ func TestDefaultRoleWithRuntimePolicy(t *testing.T) { err := enf.syncUpdate(fakeConfigMap(), noOpUpdate) assert.Nil(t, err) runtimePolicy := assets.BuiltinPolicyCSV - assert.False(t, enf.EnforceRuntimePolicy(runtimePolicy, "bob", "applications", "get", "foo/bar")) + assert.False(t, enf.EnforceRuntimePolicy("", runtimePolicy, "bob", "applications", "get", "foo/bar")) enf.SetDefaultRole("role:readonly") - assert.True(t, enf.EnforceRuntimePolicy(runtimePolicy, "bob", "applications", "get", "foo/bar")) + assert.True(t, enf.EnforceRuntimePolicy("", runtimePolicy, "bob", "applications", "get", "foo/bar")) } // TestClaimsEnforcerFuncWithRuntimePolicy tests the ability for claims enforcer function to still @@ -252,11 +252,11 @@ func TestClaimsEnforcerFuncWithRuntimePolicy(t *testing.T) { claims := jwt.StandardClaims{ Subject: "foo", } - assert.False(t, enf.EnforceRuntimePolicy(runtimePolicy, claims, "applications", "get", "foo/bar")) + assert.False(t, enf.EnforceRuntimePolicy("", runtimePolicy, claims, "applications", "get", "foo/bar")) enf.SetClaimsEnforcerFunc(func(claims jwt.Claims, rvals ...interface{}) bool { return true }) - assert.True(t, enf.EnforceRuntimePolicy(runtimePolicy, claims, "applications", "get", "foo/bar")) + assert.True(t, enf.EnforceRuntimePolicy("", runtimePolicy, claims, "applications", "get", "foo/bar")) } // TestInvalidRuntimePolicy tests when an invalid policy is supplied, it falls back to normal enforcement @@ -267,11 +267,11 @@ func TestInvalidRuntimePolicy(t *testing.T) { err := enf.syncUpdate(fakeConfigMap(), noOpUpdate) assert.Nil(t, err) _ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV) - assert.True(t, enf.EnforceRuntimePolicy("", "admin", "applications", "update", "foo/bar")) - assert.False(t, enf.EnforceRuntimePolicy("", "role:readonly", "applications", "update", "foo/bar")) + assert.True(t, enf.EnforceRuntimePolicy("", "", "admin", "applications", "update", "foo/bar")) + assert.False(t, enf.EnforceRuntimePolicy("", "", "role:readonly", "applications", "update", "foo/bar")) badPolicy := "this, is, not, a, good, policy" - assert.True(t, enf.EnforceRuntimePolicy(badPolicy, "admin", "applications", "update", "foo/bar")) - assert.False(t, enf.EnforceRuntimePolicy(badPolicy, "role:readonly", "applications", "update", "foo/bar")) + assert.True(t, enf.EnforceRuntimePolicy("", badPolicy, "admin", "applications", "update", "foo/bar")) + assert.False(t, enf.EnforceRuntimePolicy("", badPolicy, "role:readonly", "applications", "update", "foo/bar")) } func TestValidatePolicy(t *testing.T) {