php-casbin icon indicating copy to clipboard operation
php-casbin copied to clipboard

RBAC with domain and domain pattern returns invalid results

Open Nalweakette opened this issue 1 year ago • 3 comments

When using model with keymatch, the role manager doesn't create links like it is supposed to do, giving invalid enforce result. RoleManager used to work when it was a "clone" of the node.js version, but broke when it was reworked on this commit https://github.com/php-casbin/php-casbin/commit/a4091540c57d3abbe6cf84671e1436b76ce1ccb2

Also note that now buildRoleLinks() must be called to have some "good" results

How to reproduce:

====================================================================== model.conf

[request_definition] r = sub, dom, obj, act

[policy_definition] p = sub, dom, obj, act

[role_definition] g = _, _, _

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

[matchers] m = g(r.sub, p.sub, r.dom) && keyMatch(r.dom, p.dom) && r.obj == p.obj && r.act == p.act

====================================================================== policies.csv

p,perm1,,data1,read p,perm2,,data1,write p,perm3,,data2,read p,perm4,,data2,write

g,adminrole,perm1,* g,adminrole,perm2,* g,adminrole,perm3,* g,adminrole,perm4,* g,readerrole,perm1,* g,readerrole,perm3,*

g,admingroup, adminrole, * g,readergroup, readerrole, *

g,usergroup4, readergroup, domain4 g,usergroup4, perm4, domain4

g,alice,admingroup,domain1 g,alice,readergroup,domain2 g,alice,readergroup,domain4

======================================================================

result tests in https://casbin.org/editor/

alice, domain1, data1, read => true alice, domain1, data1, write => true alice, domain1, data2, read => true alice, domain1, data2, write => true bob, domain1, data2, write => false alice, domain2, data1, read => true alice, domain2, data1, write => false alice, domain2, data2, read => true alice, domain2, data2, write => false alice, domain3, data1, read => false alice, domain3, data1, write => false alice, domain3, data2, read => false alice, domain3, data2, write => false alice, domain4, data1, write => false alice, domain4, data1, read => true alice, domain4, data2, read => true alice, domain4, data2, write => true

======================================================================

end to end to in EnforcerTest.php

public function testRbacWithDomain()
{
    $e = new Enforcer(
        $this->modelAndPolicyPath . '/rbac_with_domain_pattern_model_and_keymatch_model.conf',
        $this->modelAndPolicyPath . '/rbac_with_domain_pattern_model_and_keymatch_policy.csv'
    );

    $e->addNamedDomainMatchingFunc('g', 'keyMatch', function (string $key1, string $key2) {
        return BuiltinOperations::keyMatch($key1, $key2);
    });

   // NOTE : without $e->buildRoleLinks(); everything is false;
   // WITH $e->buildRoleLinks(); everything is true
   $e->buildRoleLinks();

    $this->assertTrue($e->enforce('alice', 'domain1', 'data1', 'read'));
    $this->assertTrue($e->enforce('alice', 'domain1', 'data1', 'write'));
    $this->assertTrue($e->enforce('alice', 'domain1', 'data2', 'read'));
    $this->assertTrue($e->enforce('alice', 'domain1', 'data2', 'write'));

    $this->assertTrue($e->enforce('alice',  'domain2', 'data1', 'read'));
    $this->assertFalse($e->enforce('alice', 'domain2', 'data1', 'write'));
    $this->assertTrue($e->enforce('alice',  'domain2', 'data2', 'read'));
    $this->assertFalse($e->enforce('alice', 'domain2', 'data2', 'write'));

    $this->assertFalse($e->enforce('alice', 'domain3', 'data1', 'read'));
    $this->assertFalse($e->enforce('alice', 'domain3', 'data1', 'write'));
    $this->assertFalse($e->enforce('alice', 'domain3', 'data2', 'read'));
    $this->assertFalse($e->enforce('alice', 'domain3', 'data2', 'write'));

    $this->assertFalse($e->enforce('alice', 'domain4', 'data1', 'write'));
    $this->assertTrue($e->enforce('alice', 'domain4', 'data1', 'read'));
    $this->assertTrue($e->enforce('alice', 'domain4', 'data2', 'read'));
    $this->assertTrue($e->enforce('alice', 'domain4', 'data2', 'write'));
}

======================================================================

Results are all true, or all false. They doesn't match online editor

Nalweakette avatar Nov 14 '23 15:11 Nalweakette

+1 to me I'm fairy interested by That @leeqvip or @basakest it's a really nice catch to fix what's your opinion ?

woprrr avatar Nov 14 '23 15:11 woprrr

@leeqvip can we revoke this PR? https://github.com/php-casbin/php-casbin/pull/120

hsluoyz avatar Nov 29 '23 13:11 hsluoyz

@Nalweakette It has been fixed in v3.21.4, but this is not the final solution. It should be pointed out that for blank domain names in the policies, use * instead.

p,perm1,*,data1,read
p,perm2,*,data1,write
p,perm3,*,data2,read
p,perm4,*,data2,write
...

leeqvip avatar Nov 30 '23 16:11 leeqvip

Closed as resolved

hsluoyz avatar May 29 '24 02:05 hsluoyz