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

[Question] Is it expected that keyMatch3 handles * pattern? #1367

Closed
mih-kopylov opened this issue Feb 24, 2024 · 3 comments
Closed

[Question] Is it expected that keyMatch3 handles * pattern? #1367

mih-kopylov opened this issue Feb 24, 2024 · 3 comments

Comments

@mih-kopylov
Copy link

mih-kopylov commented Feb 24, 2024

Hello!

I'm looking at doc https://casbin.org/docs/function/

It explicitly says that

  • keyMatch handles a URL path or a * pattern like /alice_data/*
  • keyMatch3 handles a URL path or a {} pattern like /alice_data/{resource}

At the same time the

func TestKeyMatch3(t *testing.T) {
// keyMatch3() is similar with KeyMatch2(), except using "/proxy/{id}" instead of "/proxy/:id".
testKeyMatch3(t, "/foo", "/foo", true)
testKeyMatch3(t, "/foo", "/foo*", true)
testKeyMatch3(t, "/foo", "/foo/*", false)
testKeyMatch3(t, "/foo/bar", "/foo", false)
testKeyMatch3(t, "/foo/bar", "/foo*", false)
testKeyMatch3(t, "/foo/bar", "/foo/*", true)
testKeyMatch3(t, "/foobar", "/foo", false)
testKeyMatch3(t, "/foobar", "/foo*", false)
testKeyMatch3(t, "/foobar", "/foo/*", false)
testKeyMatch3(t, "/", "/{resource}", false)
testKeyMatch3(t, "/resource1", "/{resource}", true)
testKeyMatch3(t, "/myid", "/{id}/using/{resId}", false)
testKeyMatch3(t, "/myid/using/myresid", "/{id}/using/{resId}", true)
testKeyMatch3(t, "/proxy/myid", "/proxy/{id}/*", false)
testKeyMatch3(t, "/proxy/myid/", "/proxy/{id}/*", true)
testKeyMatch3(t, "/proxy/myid/res", "/proxy/{id}/*", true)
testKeyMatch3(t, "/proxy/myid/res/res2", "/proxy/{id}/*", true)
testKeyMatch3(t, "/proxy/myid/res/res2/res3", "/proxy/{id}/*", true)
testKeyMatch3(t, "/proxy/", "/proxy/{id}/*", false)
testKeyMatch3(t, "/myid/using/myresid", "/{id/using/{resId}", false)
}
tests demonstrate different behaviour:

  • testKeyMatch3(t, "/foo/bar", "/foo/*", true)
  • testKeyMatch3(t, "/proxy/myid/res", "/proxy/{id}/*", true)

According to the tests, the keyMatch3 function handles both {} pattern AND * pattern, while it's not documented.

If that's an expected behaviour, could you please document that explicitly.
Otherwise the tests should be fixed.

Moving forward, there's keyMatch5 function as well which handles a URL path, a {} or * pattern like /alice_data/{id}/*, according to the documentation.
And the tests

func TestKeyMatch5Func(t *testing.T) {
testKeyMatch5Func(t, false, "keyMatch5: expected 2 arguments, but got 1", "/foo")
testKeyMatch5Func(t, false, "keyMatch5: expected 2 arguments, but got 3", "/foo/create/123", "/foo/*", "/foo/update/123")
testKeyMatch5Func(t, false, "keyMatch5: argument must be a string", "/parent/123", true)
testKeyMatch5Func(t, true, "", "/parent/child?status=1&type=2", "/parent/child")
testKeyMatch5Func(t, false, "", "/parent?status=1&type=2", "/parent/child")
testKeyMatch5Func(t, true, "", "/parent/child/?status=1&type=2", "/parent/child/")
testKeyMatch5Func(t, false, "", "/parent/child/?status=1&type=2", "/parent/child")
testKeyMatch5Func(t, false, "", "/parent/child?status=1&type=2", "/parent/child/")
testKeyMatch5Func(t, true, "", "/foo", "/foo")
testKeyMatch5Func(t, true, "", "/foo", "/foo*")
testKeyMatch5Func(t, false, "", "/foo", "/foo/*")
testKeyMatch5Func(t, false, "", "/foo/bar", "/foo")
testKeyMatch5Func(t, false, "", "/foo/bar", "/foo*")
testKeyMatch5Func(t, true, "", "/foo/bar", "/foo/*")
testKeyMatch5Func(t, false, "", "/foobar", "/foo")
testKeyMatch5Func(t, false, "", "/foobar", "/foo*")
testKeyMatch5Func(t, false, "", "/foobar", "/foo/*")
testKeyMatch5Func(t, false, "", "/", "/{resource}")
testKeyMatch5Func(t, true, "", "/resource1", "/{resource}")
testKeyMatch5Func(t, false, "", "/myid", "/{id}/using/{resId}")
testKeyMatch5Func(t, true, "", "/myid/using/myresid", "/{id}/using/{resId}")
testKeyMatch5Func(t, false, "", "/proxy/myid", "/proxy/{id}/*")
testKeyMatch5Func(t, true, "", "/proxy/myid/", "/proxy/{id}/*")
testKeyMatch5Func(t, true, "", "/proxy/myid/res", "/proxy/{id}/*")
testKeyMatch5Func(t, true, "", "/proxy/myid/res/res2", "/proxy/{id}/*")
testKeyMatch5Func(t, true, "", "/proxy/myid/res/res2/res3", "/proxy/{id}/*")
testKeyMatch5Func(t, false, "", "/proxy/", "/proxy/{id}/*")
testKeyMatch5Func(t, false, "", "/proxy/myid?status=1&type=2", "/proxy/{id}/*")
testKeyMatch5Func(t, true, "", "/proxy/myid/", "/proxy/{id}/*")
testKeyMatch5Func(t, true, "", "/proxy/myid/res?status=1&type=2", "/proxy/{id}/*")
testKeyMatch5Func(t, true, "", "/proxy/myid/res/res2?status=1&type=2", "/proxy/{id}/*")
testKeyMatch5Func(t, true, "", "/proxy/myid/res/res2/res3?status=1&type=2", "/proxy/{id}/*")
testKeyMatch5Func(t, false, "", "/proxy/", "/proxy/{id}/*")
}
prove it.

It looks like keyMatch3 and keyMatch5 behave equally, while the documentation says they shouldn't.

@casbin-bot
Copy link
Member

@tangyang9464 @JalinWang

@mih-kopylov
Copy link
Author

It's also not quite clear about the keyMatch3 and keyMatch4 difference.

According to the doc, the latter handles double {} parameters

a URL path or a {} pattern like /alice_data/{id}/book/{id}

while the former handles only single {} parameters

a URL path or a {} pattern like /alice_data/{resource}

But in the following example both keyMatch3 and keyMatch4 work equally

[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 && keyMatch3(r.obj, p.obj) && (r.act == p.act)
p, alice, /api/{param1}/data/{param2}, GET
alice, /api/12/data/23, GET

@hsluoyz
Copy link
Member

hsluoyz commented Mar 31, 2024

@mih-kopylov

keyMatch3() and keyMatch4() both support * as well

keyMatch4() tests if the same "id" occurrences have the same value

func TestKeyMatch4(t *testing.T) {
testKeyMatch4(t, "/parent/123/child/123", "/parent/{id}/child/{id}", true)
testKeyMatch4(t, "/parent/123/child/456", "/parent/{id}/child/{id}", false)
testKeyMatch4(t, "/parent/123/child/123", "/parent/{id}/child/{another_id}", true)
testKeyMatch4(t, "/parent/123/child/456", "/parent/{id}/child/{another_id}", true)
testKeyMatch4(t, "/parent/123/child/123/book/123", "/parent/{id}/child/{id}/book/{id}", true)
testKeyMatch4(t, "/parent/123/child/123/book/456", "/parent/{id}/child/{id}/book/{id}", false)
testKeyMatch4(t, "/parent/123/child/456/book/123", "/parent/{id}/child/{id}/book/{id}", false)
testKeyMatch4(t, "/parent/123/child/456/book/", "/parent/{id}/child/{id}/book/{id}", false)
testKeyMatch4(t, "/parent/123/child/456", "/parent/{id}/child/{id}/book/{id}", false)
testKeyMatch4(t, "/parent/123/child/123", "/parent/{i/d}/child/{i/d}", false)
}

@hsluoyz hsluoyz closed this as completed Mar 31, 2024
@casbin casbin deleted a comment from mih-kopylov May 4, 2024
@casbin casbin deleted a comment from mih-kopylov May 4, 2024
@casbin casbin locked as resolved and limited conversation to collaborators May 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

3 participants