chore: pre filter groups before enforcing argoproj#4296 (argoproj#6651)

* chore: pre filter groups before enforcing Part of: argoproj#4296 Signed-off-by: Jan Jansen <[email protected]> * chore: prevent serialization if it is a mapclaims Signed-off-by: Jan Jansen <[email protected]> * add comments Signed-off-by: Jan Jansen <[email protected]>
dajudge · Jul 26, 2021 · af516e9 · af516e9
1 parent c3abe77
commit af516e9
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 9 deletions.
diff --git a/USERS.md b/USERS.md
@@ -46,6 +46,7 @@ Currently, the following organizations are **officially** using Argo CD:
 1. [Fave](https://myfave.com)
 1. [Future PLC](https://www.futureplc.com/)
 1. [Garner](https://www.garnercorp.com)
+1. [G DATA CyberDefense AG](https://www.gdata-software.com/)
 1. [Generali Deutschland AG](https://www.generali.de/)
 1. [Glovo](https://www.glovoapp.com)
 1. [GMETRI](https://gmetri.com/)

diff --git a/server/application/application_test.go b/server/application/application_test.go
@@ -3,6 +3,7 @@ package application
 import (
  "context"
  coreerrors "errors"
+ "fmt"
  "sync/atomic"
  "testing"
  "time"
@@ -73,6 +74,14 @@ func fakeAppList() *apiclient.AppList {
 
 // return an ApplicationServiceServer which returns fake data
 func newTestAppServer(objects ...runtime.Object) *Server {
+ f := func(enf *rbac.Enforcer) {
+ _ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
+ enf.SetDefaultRole("role:admin")
+ }
+ return newTestAppServerWithEnforcerConfigure(f, objects...)
+}
+
+func newTestAppServerWithEnforcerConfigure(f func(*rbac.Enforcer), objects ...runtime.Object) *Server {
  kubeclientset := fake.NewSimpleClientset(&v1.ConfigMap{
  ObjectMeta: metav1.ObjectMeta{
  Namespace: testNamespace,
@@ -139,12 +148,11 @@ func newTestAppServer(objects ...runtime.Object) *Server {
  objects = append(objects, defaultProj, myProj, projWithSyncWindows)
 
  fakeAppsClientset := apps.NewSimpleClientset(objects...)
- factory := appinformer.NewFilteredSharedInformerFactory(fakeAppsClientset, 0, "", func(options *metav1.ListOptions) {})
+ factory := appinformer.NewSharedInformerFactoryWithOptions(fakeAppsClientset, 0, appinformer.WithNamespace(""), appinformer.WithTweakListOptions(func(options *metav1.ListOptions) {}))
  fakeProjLister := factory.Argoproj().V1alpha1().AppProjects().Lister().AppProjects(testNamespace)
 
  enforcer := rbac.NewEnforcer(kubeclientset, testNamespace, common.ArgoCDRBACConfigMapName, nil)
- _ = enforcer.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
- enforcer.SetDefaultRole("role:admin")
+ f(enforcer)
  enforcer.SetClaimsEnforcerFunc(rbacpolicy.NewRBACPolicyEnforcer(enforcer, fakeProjLister).EnforceClaims)
 
  settingsMgr := settings.NewSettingsManager(ctx, kubeclientset, testNamespace)
@@ -280,6 +288,49 @@ func TestListApps(t *testing.T) {
  assert.Equal(t, []string{"abc", "bcd", "def"}, names)
 }
 
+func TestCoupleAppsListApps(t *testing.T) {
+ var objects []runtime.Object
+ ctx := context.Background()
+
+ var groups []string
+ for i := 0; i < 50; i++ {
+ groups = append(groups, fmt.Sprintf("group-%d", i))
+ }
+ // nolint:staticcheck
+ ctx = context.WithValue(ctx, "claims", &jwt.MapClaims{"groups": groups})
+ for projectId := 0; projectId < 100; projectId++ {
+ projectName := fmt.Sprintf("proj-%d", projectId)
+ for appId := 0; appId < 100; appId++ {
+ objects = append(objects, newTestApp(func(app *appsv1.Application) {
+ app.Name = fmt.Sprintf("app-%d-%d", projectId, appId)
+ app.Spec.Project = projectName
+ }))
+ }
+ }
+
+ f := func(enf *rbac.Enforcer) {
+ policy := `
+p, role:test, applications, *, proj-10/*, allow
+g, group-45, role:test
+p, role:test2, applications, *, proj-15/*, allow
+g, group-47, role:test2
+p, role:test3, applications, *, proj-20/*, allow
+g, group-49, role:test3
+`
+ _ = enf.SetUserPolicy(policy)
+ }
+ appServer := newTestAppServerWithEnforcerConfigure(f, objects...)
+
+ res, err := appServer.List(ctx, &application.ApplicationQuery{})
+
+ assert.NoError(t, err)
+ var names []string
+ for i := range res.Items {
+ names = append(names, res.Items[i].Name)
+ }
+ assert.Equal(t, 300, len(names))
+}
+
 func TestCreateApp(t *testing.T) {
  testApp := newTestApp()
  appServer := newTestAppServer()

diff --git a/server/rbacpolicy/rbacpolicy.go b/server/rbacpolicy/rbacpolicy.go
@@ -113,10 +113,13 @@ func (p *RBACPolicyEnforcer) EnforceClaims(claims jwt.Claims, rvals ...interface
  runtimePolicy = proj.ProjectPoliciesString()
  }
 
+ // NOTE: This calls prevent multiple creation of the wrapped enforcer
+ enforcer := p.enf.CreateEnforcerWithRuntimePolicy(runtimePolicy)
+
  // Check the subject. This is typically the 'admin' case.
- // NOTE: the call to EnforceRuntimePolicy will also consider the default role
+ // NOTE: the call to EnforceWithCustomEnforcer will also consider the default role
  vals := append([]interface{}{subject}, rvals[1:]...)
- if p.enf.EnforceRuntimePolicy(runtimePolicy, vals...) {
+ if p.enf.EnforceWithCustomEnforcer(enforcer, vals...) {
  return true
  }
 
@@ -126,10 +129,19 @@ func (p *RBACPolicyEnforcer) EnforceClaims(claims jwt.Claims, rvals ...interface
  }
  // Finally check if any of the user's groups grant them permissions
  groups := jwtutil.GetScopeValues(mapClaims, scopes)
- for _, group := range groups {
- vals := append([]interface{}{group}, rvals[1:]...)
- if p.enf.EnforceRuntimePolicy(runtimePolicy, vals...) {
- return true
+
+ // Get groups to reduce the amount to checking groups
+ groupingPolicies := enforcer.GetGroupingPolicy()
+ for gidx := range groups {
+ for gpidx := range groupingPolicies {
+ // Prefilter user groups by groups defined in the model
+ if groupingPolicies[gpidx][0] == groups[gidx] {
+ vals := append([]interface{}{groups[gidx]}, rvals[1:]...)
+ if p.enf.EnforceWithCustomEnforcer(enforcer, vals...) {
+ return true
+ }
+ break
+ }
  }
  }
  logCtx := log.WithField("claims", claims).WithField("rval", rvals)

diff --git a/util/jwt/jwt.go b/util/jwt/jwt.go
@@ -12,6 +12,9 @@ import (
 
 // MapClaims converts a jwt.Claims to a MapClaims
 func MapClaims(claims jwtgo.Claims) (jwtgo.MapClaims, error) {
+ if mapClaims, ok := claims.(*jwtgo.MapClaims); ok {
+ return *mapClaims, nil
+ }
  claimsBytes, err := json.Marshal(claims)
  if err != nil {
  return nil, err

diff --git a/util/rbac/rbac.go b/util/rbac/rbac.go
@@ -150,6 +150,14 @@ func (e *Enforcer) EnforceErr(rvals ...interface{}) error {
 // 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)
+ 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 == "" {
@@ -161,6 +169,11 @@ func (e *Enforcer) EnforceRuntimePolicy(policy string, rvals ...interface{}) boo
  enf = e.Enforcer
  }
  }
+ return enf
+}
+
+// EnforceWithCustomEnforcer wraps enforce with an custom enforcer
+func (e *Enforcer) EnforceWithCustomEnforcer(enf *casbin.Enforcer, rvals ...interface{}) bool {
  return enforce(enf, e.defaultRole, e.claimsEnforcerFunc, rvals...)
 }