feat: Support pattern function in 3rd args of g

[dovics]

This commit not only supports pattern function in 3rd args of g, but also can be used in policy, such as

p, data1_admin, *, data1, read
p, data1_admin, *, data1, write

g, alice, data1_admin, domain1

[dovics]

Looks like there are still some problems, I will fix it immediately

[hsluoyz]

@dovics I think p, data1_admin, *, data1, read is not needed to support. We actually have keyMatch() and regexMatch() to support such requirements. Supporting * natively will only make things ambigious.

[dovics]

@dovics I think p, data1_admin, *, data1, read is not needed to support. We actually have keyMatch() and regexMatch() to support such requirements. Supporting * natively will only make things ambigious.

It may be that I am not expressing clearly enough, what I mean here is that we can use pattern in policy.domain，Otherwise the user needs to write

p, data1_admin, domain1, data1, read
p, data1_admin, domain1, data1, write
p, data1_admin, domain2, data1,read
p, data1_admin, domain2, data1, write

p, alice, data_admin1, domian1

This is just an incidental feature, If we don't need it, we only need to make r.dom == p.dom

The * can be replaced by any pattern string，now it is supported by globMatch()

[coveralls]

Coverage increased (+0.1%) to 75.081% when pulling d35a057 on dovics:support-pattern-function-in-3rd-args-of-g into b8d7ab2 on casbin:master.

[nodece]

@dovics Can we only use the AddMatchingFunc to support pattern?

[hsluoyz]

@dovics OK. I understood it now. But how to know which is dom? What about p, *, data1_admin, data1, read? Or you only recognize it from model: p = sub, dom, obj, act's dom?

[dovics]

@dovics OK. I understood it now. But how to know which is dom? What about p, *, data1_admin, data1, read? Or you only recognize it from model: p = sub, dom, obj, act's dom?

@hsluoyz the matcher is

g(r.sub, p.sub, r.dom) && globMatch(r.dom, p.dom) && r.obj == p.obj && r.act == p.act 

The data in memory is like this

domain1: data1_admin -> domian1: alice

The key to being able to write like this is globMatch(r.dom, p.dom).If there is no this pr, we can write this too. But this is not pointed out in the doc

[dovics]

@dovics Can we only use the AddMatchingFunc to support pattern?

@nodece I merged the two functions in the new commit

[GopherJ]

I'm not sure if we will need:

p, data1_admin, *, data1, read

because normally different domain has different data, in my opinion it's not so necessary.

The importance is to make:

g, alice, admin, *

AND

g, /book/:id, book_admin, *

works.

Currently if we use pattern the domain and user/obj are mixed, normally we have 3 cases:

1. pure domain pattern

g, alice, admin, *
g, bob, guest, domain1

2. user/obj pattern (already supported)

g, /book/:id, book_admin

3. domain + user/obj pattern

g, /book/:id, book_admin, *

To solve 2, we let /book/3, /book/4...inherit /book/:id hence book_admin.

To solve 1, I believe we can still do everything in CreateRole function, because when we run CreateRole("domain1::bob") we already know that there is a new domain called domain1, so we can iterate over all roles and check if any domain matches domain1, then certainly we will find *::alice and it has role *::admin, in this case we will be able to just replace * by the new domain `domain1 and add the following link.

domain1::admin
      |
domain1::alice

is this right?

About 3, I don't have an idea yet

[dovics]

@GopherJ I think you are right.

Let me first state my implementation. If call HasLink(alice, admin, domain1) We will get the following relationship.

domain1::admin
       |
    *::admin
       |
    *::alice
       |
domain1:alice

but I just need one matchFunc, and It is easy to support case 3, I added a new function to support it.

domain1::book_admin
       |
  *::book_admin
       |
  *::/book/:id
       |
domain::/book/1

Do you really think we have to deal with it in createRole()?

In addition, I removed the part about p, data1_admin, *, data1, read in the unit test and examples

[nodece]

Do you really think we have to deal with it in createRole()?

@dovics Because our previous design can't meet the current feature, so you don't have to follow the current design.

[GopherJ]

@nodece @dovics

Since I'm afraid of having bad bench on pattern(because we always need to iterate over all). I'm thinking if we can change all_roles' structure.

Here some of my thoughts

The idea is to stop using domain::user/role and make every domain has its own graph. if a domain is a pattern domain (eg. *). we clone its graph's reference to matched domain.

The advantage here is:

we don't need to iterate over all users/roles anymore, we iterate over all domains

we do first domain matching, user/role matching is inside this domain and it'll be easy to handle

do you think it may work?

[nodece]

I'm worried about that, too. Your idea looks good.

[dovics]

@GopherJ I think It's a good idea, but I still have a little question

if a domain is a pattern domain (eg. *). we clone its graph's reference to matched domain.

if we create the matched domain first, and then create pattern domain, we couldn't clone the pattern domain's graph to matched domain, so we still need create the pattern domain first, and then merge the two graph to matched domain.

so I think the general step of add_link should be

1. Find the domain that not use matching func
2. Add the link,
3. Find the mathced domain or pattern domain, Merge the pattern domain's graph and the matched domain's graph, and store it to the matched domain

If I'm missing something, please point it out

[nodece]

I think matching function should be invoked when getting roles/links/users. when add link, we
don't use matching function.

[GopherJ]

@dovics I have been exploring my solution for a while but I didn't succeed to find a simple way to merge two graphs. simple and efficient.

My requirement is:

1. being able to keep every graph seperate in their own domain.
2. while calling create_role, create a link if a domain matches other domains, like: domain1 -> ["*"] so that if we call has_link, get_users, get_roles we will know directly if we need to merge graph or not.
3. while calling has_link, get_users, get_roles, if the domain matches another domain, then generate a temporary graph of domain1, merge *'s graph into domain1's graph, then do what we have been doing.

I don't want to call any AddRole directly on original graph, because I'd like to have a solution without side effects, that means if one day I call:

DeleteLink("alice", "admin", "*")

I don't need to do any cleanup.

[dovics]

@GopherJ I don't see a good enough way either, but I think we could use the worst way to add link one by one to the new graph, which is still more efficient than this implementation. I'm going to implement the current idea what you say now, and then take longer to think of a good enough solution

[dovics]

@GopherJ I'm basically done what I understand, but I don’t know if it is far from your expectations, please give me some suggestions.

[github-actions]

🎉 This PR is included in version 2.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[hsluoyz]

@dovics @nodece please update this exciting change to our docs: https://casbin.org/docs/en/rbac#use-pattern-matching-in-rbac

[nodece]

@hsluoyz I have a PR for the feature here: casbin/casbin-website#102

[hsluoyz]

@nodece awesome!