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

The power of multiple matchers #299

Open
weberr13 opened this issue Oct 3, 2019 · 14 comments
Open

The power of multiple matchers #299

weberr13 opened this issue Oct 3, 2019 · 14 comments
Assignees
Labels

Comments

@weberr13
Copy link

@weberr13 weberr13 commented Oct 3, 2019

In the docs there is a note where the decision not to have multiple matchers is left open for discussion. Here is the use case I have where I use multiple matchers, and some code that I use currently to do it:

I have a role with a pattern "foo/bar/" that grants an access level of "read/write" to a user.
I have another role with a pattern "foo/bar/secret/
" that restricts access to "read".

I give a user both the above roles and I expect the following behavior:

Enforce(user, "foo/bar/baz", "write") -> true
Enforce(user, "foo/bar/secret/baz", "write") -> false

In order to do this I created a "negative" matcher. When I find a role where access is granted I do a check for a role that matches the "negative" of the above role:

<REDACTED>

This "works" of course, but it does illustrate that these "negative" matchers are necessary in order to support this sort of RBAC scenario.

@hsluoyz
Copy link
Contributor

@hsluoyz hsluoyz commented Oct 3, 2019

Hi @weberr13 ,

  1. I don't know why you call GetRolesForUser() and enforce each role. Casbin RBAC will automatically check all direct and implicit roles for a user. It's more convenient and efficient.

  2. Let's say your two matchers are m1 and m2. You can do: m = m1 && ! m2. It's the same with your logic.

@hsluoyz hsluoyz self-assigned this Oct 3, 2019
@weberr13
Copy link
Author

@weberr13 weberr13 commented Oct 3, 2019

As for your first, I can try that. For the second though, this is a situation with 2 different roles. Isn't the matcher run through the roles individually? How can m2 match on a potentially different "p.action" without doing it this way?

@hsluoyz
Copy link
Contributor

@hsluoyz hsluoyz commented Oct 3, 2019

I think you are right. We will figure out how to support multiple matchers. The question here is how to use effect to merge the results from multiple matchers. Do you have any suggestions?

@weberr13
Copy link
Author

@weberr13 weberr13 commented Oct 3, 2019

My suggestions are possibly tailored to my use case but in the most generic way I want a function like this:

func EnforceWithMultipleMatch(positiveMatchers []string, negativeMatchers []string, ...interface{}) (bool, error)

where the result would be some(positiveMattchers) AND NOT any(negativeMachers)

@hsluoyz
Copy link
Contributor

@hsluoyz hsluoyz commented Oct 3, 2019

A more general solution would be:

[policy_effect]
e = some(where (m.p.eft == allow)) && !some(where (m2.p.eft == allow))

[matchers]
m = <1st matcher>
m2 = <2nd matcher>

Effect can be customized to add m3, m4, etc. So all can be done in model language. Can you make a PR for it?

@weberr13
Copy link
Author

@weberr13 weberr13 commented Oct 3, 2019

This is exactly what I envisioned. As for a MR, I am bound by a legal department but I will quickly submit a request to contribute to them and once they do I can craft something.

@weberr13
Copy link
Author

@weberr13 weberr13 commented Oct 3, 2019

I checked with legal and F5 has a number of patents around RBAC and it would likely take months of legal wrangling in order to verify that my contributions were allowed. Hopefully my use case example is inspiring enough that you or someone else can act on the design above.

@hsluoyz
Copy link
Contributor

@hsluoyz hsluoyz commented Oct 4, 2019

OK. @nodece can you help implement this?

@nodece
Copy link
Member

@nodece nodece commented Oct 4, 2019

Hi @weberr13, Maybe you don't need multiple matchers or see: https://casbin.org/docs/en/function#how-to-add-a-customized-function and https://github.com/casbin/casbin/blob/master/model_test.go#L435

My provide a test to help you.

Enforce(user, "foo/bar/baz", "write") -> true
Enforce(user, "foo/bar/secret/baz", "write") -> false

func TestNewEnforcer299(t *testing.T) {
	m := model.NewModel()
	m.AddDef("r", "r", "sub, obj, act")
	m.AddDef("p", "p", "sub, obj, act")
	m.AddDef("e", "e", "some(where (p.eft == allow))")
	m.AddDef("m", "m", "r.sub == p.sub && r.obj == p.obj && regexMatch(r.act, p.act)")

	e, _ := NewEnforcer(m)

	e.AddPolicy("user", "foo/bar/baz", "(read)|(write)")
	e.AddPolicy("user", "foo/bar/secret/baz", "(write)")

	ok, _ := e.Enforce("user", "foo/bar/baz", "read")
	if ok != true {
		t.Error("supposed to be true")
	}

	ok, _ = e.Enforce("user", "foo/bar/baz", "write")
	if ok != true {
		t.Error("supposed to be true")
	}

	ok, _ = e.Enforce("user", "foo/bar/secret/baz", "read")
	if ok != false {
		t.Error("supposed to be false")
	}
}

@weberr13
Copy link
Author

@weberr13 weberr13 commented Oct 9, 2019

This example doesn't give the functionality I want. I want:

Policy A : R/W to /foo/bar/*
Policy B : R only to /foo/bar/baz/*

notice that the paths overlap. The only way to do that sort of thing is with a second custom enforce step with a "negative" matcher that can falsify a match based on a second match.

@nodece
Copy link
Member

@nodece nodece commented Oct 10, 2019

Customized function can expect what you envisioned.

Please test it at https://casbin.org/en/editor.

Add a "negativeMatcher" function, it returns true if r.sub equals "alice".

Model

[request_definition]
r = sub, obj, act

[policy_definition]
p = sub, obj, act

[policy_effect]
e = some(where (p.eft == allow))

[matchers]
m = r.sub == p.sub && r.obj == p.obj && r.act == p.act && negativeMatcher(r.sub)

Policy

p, alice, data1, read
p, bob, data2, write

Request

alice, data1, read
bob, data2, write

Custom Function

var fns = {
	 negativeMatcher: (sub) => {
        return sub === "alice" ? true: false
    }
}

@weberr13
Copy link
Author

@weberr13 weberr13 commented Oct 11, 2019

Yeah, I have a negative matcher hacked in using EnforceWithMatcher but the original issue is that I would like both matchers in my config file. Currently the config file only supports one matcher entry. In summary I need some(positiveMatch(all permissions)) && !any(negativeMatch(all permissions) and thus 2 matchers in the config file.

@GopherJ
Copy link
Contributor

@GopherJ GopherJ commented Apr 19, 2020

@weberr13 not sure if casbin/casbin-rs#123 solves this problem but it's a good idea to have multiple matchers. I had a first implementation there.

@visur
Copy link

@visur visur commented Jun 3, 2020

I think, @weberr13, you want something that I solved by matcher function overwrite:
Model:

m := model.NewModel()
m.AddDef("r", "r", "usr, url, mtd")
m.AddDef("p", "p", "scp, per, url, mtd")
m.AddDef("g", "g", "_, _, _")
m.AddDef("e", "e", "some(where (p.eft == allow))")
m.AddDef("m", "m", "g(p.per, r.usr, p.scp) && keyMatch(r.url, p.url) && regexMatch(r.mtd, p.mtd)")

Policy:

// scope, permission, route, method
persist.LoadPolicyLine("p, networks, r, /m/networks, POST", model)
persist.LoadPolicyLine("p, networks, w, /m/networks/scan, POST", model)
persist.LoadPolicyLine("p, networks, e, /m/networks/scan-now, POST", model)
persist.LoadPolicyLine("p, networks, e, /m/networks/cancel-scan, POST", model)
persist.LoadPolicyLine("p, user_management, r, /m/users/*, GET", model)
persist.LoadPolicyLine("p, user_management, w, /m/users/edit, POST", model)

// users_permissions_regex, user_id, user_scope
persist.LoadPolicyLine("g, [rwe]+, 1, networks", model)
persist.LoadPolicyLine("g, [rwe]+, 1, query_advanced", model)
persist.LoadPolicyLine("g, [rwe]+, 1, query_catalog", model)
persist.LoadPolicyLine("g, [rwe]+, 1, scripts", model)
persist.LoadPolicyLine("g, [r]+, 1, settings", model)
persist.LoadPolicyLine("g, [r]+, 1, user_management", model)

I used matching function overwrite:

rm := enforcer.GetRoleManager().(*defaultrolemanager.RoleManager)
rm.AddMatchingFunc("RegexMatch", util.RegexMatch)

Test:

ok, _ := enforcer.Enforce("1", "/m/networks/scan", "POST") // true
assert.True(t, ok)

ok, _ = enforcer.Enforce("1", "/m/users/5", "GET") // true
assert.True(t, ok)

ok, _ = enforcer.Enforce("1", "/m/users/5", "POST") // false
assert.False(t, ok)

ok, _ = enforcer.Enforce("1", "/m/users/edit", "POST") // false
assert.False(t, ok)

Sorry that it not in one test function, because it was copied from different places.

My enhancement suggestion is to add possibility to place custom match functions to model "g" as:

g = (fn1, fn2, _)

and to add functions as:

rm.AddMatchingFunc("fn1", util.RegexMatch)
rm.AddMatchingFunc("fn2", util.KeyMatch)

It will be more flexible than to use one general match function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants