Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Make Casbin matcher configurable on runtime(globMatch(default) or RegexMatch) #7165

Merged
merged 8 commits into from
Oct 9, 2021
2 changes: 1 addition & 1 deletion assets/model.conf
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ g = _, _
e = some(where (p.eft == allow)) && !some(where (p.eft == deny))

[matchers]
m = g(r.sub, p.sub) && globMatch(r.res, p.res) && globMatch(r.act, p.act) && globMatch(r.obj, p.obj)
m = g(r.sub, p.sub) && globOrRegexMatch(r.res, p.res) && globOrRegexMatch(r.act, p.act) && globOrRegexMatch(r.obj, p.obj)
61 changes: 43 additions & 18 deletions util/rbac/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/casbin/casbin"
"github.com/casbin/casbin/model"
"github.com/casbin/casbin/util"
jwt "github.com/dgrijalva/jwt-go/v4"
log "github.com/sirupsen/logrus"
"google.golang.org/grpc/codes"
Expand All @@ -31,8 +32,10 @@ const (
ConfigMapPolicyCSVKey = "policy.csv"
ConfigMapPolicyDefaultKey = "policy.default"
ConfigMapScopesKey = "scopes"

defaultRBACSyncPeriod = 10 * time.Minute
ConfigMapMatchModeKey = "match.mode"
GlobMatchMode = "glob"
RegexMatchMode = "regex"
defaultRBACSyncPeriod = 10 * time.Minute
)

// Enforcer is a wrapper around an Casbin enforcer that:
Expand All @@ -50,6 +53,7 @@ type Enforcer struct {
claimsEnforcerFunc ClaimsEnforcerFunc
model model.Model
defaultRole string
matchMode string
}

// ClaimsEnforcerFunc is func template to enforce a JWT claims. The subject is replaced
Expand All @@ -60,22 +64,8 @@ func newEnforcerSafe(params ...interface{}) (e *casbin.Enforcer, err error) {
if err != nil {
return nil, err
}
enfs.AddFunction("globMatch", func(args ...interface{}) (interface{}, error) {
if len(args) < 2 {
return false, nil
}
val, ok := args[0].(string)
if !ok {
return false, nil
}

pattern, ok := args[1].(string)
if !ok {
return false, nil
}

return glob.Match(pattern, val), nil
})
// Default glob match mode
enfs.AddFunction("globOrRegexMatch", globMatchFunc)
return enfs, nil
}

Expand All @@ -98,6 +88,40 @@ func NewEnforcer(clientset kubernetes.Interface, namespace, configmap string, cl
}
}

// Glob match func
func globMatchFunc(args ...interface{}) (interface{}, error) {
if len(args) < 2 {
return false, nil
}
val, ok := args[0].(string)
if !ok {
return false, nil
}

pattern, ok := args[1].(string)
if !ok {
return false, nil
}

return glob.Match(pattern, val), nil
}

// 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) {
if mode == RegexMatchMode {
return util.RegexMatchFunc(args...)
} else {
return globMatchFunc(args...)
}
})
}

// SetDefaultRole sets a default role to use during enforcement. Will fall back to this role if
// normal enforcement fails
func (e *Enforcer) SetDefaultRole(roleName string) {
Expand Down Expand Up @@ -281,6 +305,7 @@ func (e *Enforcer) runInformer(ctx context.Context, onUpdated func(cm *apiv1.Con
// syncUpdate updates the enforcer
func (e *Enforcer) syncUpdate(cm *apiv1.ConfigMap, onUpdated func(cm *apiv1.ConfigMap) error) error {
e.SetDefaultRole(cm.Data[ConfigMapPolicyDefaultKey])
e.SetMatchMode(cm.Data[ConfigMapMatchModeKey])
policyCSV, ok := cm.Data[ConfigMapPolicyCSVKey]
if !ok {
policyCSV = ""
Expand Down
63 changes: 63 additions & 0 deletions util/rbac/rbac_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,3 +336,66 @@ func TestEnforceErrorMessage(t *testing.T) {
assert.Equal(t, "rpc error: code = PermissionDenied desc = permission denied: project, sub: proj:default:admin", err.Error())

}

func TestDefaultGlobMatchMode(t *testing.T) {
kubeclientset := fake.NewSimpleClientset()
enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
err := enf.syncUpdate(fakeConfigMap(), noOpUpdate)
assert.Nil(t, err)
policy := `
p, alice, clusters, get, "https://github.com/*/*.git", allow
`
_ = enf.SetUserPolicy(policy)

assert.True(t, enf.Enforce("alice", "clusters", "get", "https://github.com/argoproj/argo-cd.git"))
assert.False(t, enf.Enforce("alice", "repositories", "get", "https://github.com/argoproj/argo-cd.git"))

}

func TestGlobMatchMode(t *testing.T) {
cm := fakeConfigMap()
cm.Data[ConfigMapMatchModeKey] = GlobMatchMode
kubeclientset := fake.NewSimpleClientset()
enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
err := enf.syncUpdate(cm, noOpUpdate)
assert.Nil(t, err)
policy := `
p, alice, clusters, get, "https://github.com/*/*.git", allow
`
_ = enf.SetUserPolicy(policy)

assert.True(t, enf.Enforce("alice", "clusters", "get", "https://github.com/argoproj/argo-cd.git"))
assert.False(t, enf.Enforce("alice", "repositories", "get", "https://github.com/argoproj/argo-cd.git"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this test, the policy is for cluster, p, alice, clusters, get, "https://github.com/*/*.git", allow, wonder if you want to verify for repositories? Should it be testing for a cluster which does not match with the expression?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point is to test policy only for clusters, other case should fail on this scenario. So what's your suggestion, only verify test cases for clusters policy?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is to test GlobMatch, I am thinking of focusing the test data on which fails the pattern https://github.com/*/*.git. If the test data is using repository, then it failed not due to it failed the GlobMatch.


}

func TestRegexMatchMode(t *testing.T) {
cm := fakeConfigMap()
cm.Data[ConfigMapMatchModeKey] = RegexMatchMode
kubeclientset := fake.NewSimpleClientset()
enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
err := enf.syncUpdate(cm, noOpUpdate)
assert.Nil(t, err)
policy := `
p, alice, clusters, get, "https://github.com/argo[a-z]{4}/argo-[a-z]+.git", allow
`
_ = enf.SetUserPolicy(policy)

assert.True(t, enf.Enforce("alice", "clusters", "get", "https://github.com/argoproj/argo-cd.git"))
assert.False(t, enf.Enforce("alice", "repositories", "get", "https://github.com/argoproj/1argo-cd.git"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the same question as above.


}

func TestGlobMatchFunc(t *testing.T) {
ok, _ := globMatchFunc("arg1")
assert.False(t, ok.(bool))

ok, _ = globMatchFunc(time.Now(), "arg2")
assert.False(t, ok.(bool))

ok, _ = globMatchFunc("arg1", time.Now())
assert.False(t, ok.(bool))

ok, _ = globMatchFunc("arg/123", "arg/*")
assert.True(t, ok.(bool))
}