remove_grouping_policy: If there are duplicate group rules, remove_grouping_policy will be invalid

[cs1137195420]

I accidentally created two identical group rules in mysql, and then when I called the remove_grouping_policy function, I found that it failed to delete. There were still two identical group rules in mysql. So, should the remove_grouping_policy function remove all the same grouping rules?

[cs1137195420]

Here is my code:

    DATABAEE = peewee.MySQLDatabase('casbin', user='root', passwd='root')
    adapter = casbin_peewee_adapter.Adapter(database=DATABAEE)

    e = casbin.Enforcer('model.conf', adapter)

    uid = 'bob'
    role = 'admin'
    result = e.remove_grouping_policy(uid, role)
    print(result)

    sub = "bob"  # the user that wants to access a resource.
    obj = "data1"  # the resource that is going to be accessed.
    act = "write"  # the operation that the user performs on the resource.

    if e.enforce(sub, obj, act):
        print('pass')
    else:
        print('not pass')

mysql

model.conf

[request_definition]
r = sub, obj, act

[policy_definition]
p = sub, obj, act

[role_definition]
g = _, _

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

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

[casbin-bot]

@ffyuanda @Zxilly @techoner @elfisworking

[abichinger]

@cs1137195420 I tried to remove duplicate rules with remove_grouping_policy and it worked for me.
Example:

from casbin_peewee_adapter import CasbinRule, Adapter
import casbin
import peewee
import os

db_name = 'db.sqlite3'

if os.path.exists(db_name):
    os.remove(db_name)

db = peewee.SqliteDatabase(db_name)
adapter = Adapter(database=db)
db.create_tables([CasbinRule])

e = casbin.Enforcer('model.conf', adapter)

#add twice
e.add_grouping_policy('bob', 'admin')
e.add_grouping_policy('bob', 'admin')

assert CasbinRule.select().count() == 1

#add manually
rule = CasbinRule.create(ptype='g', v0='bob', v1='admin')
rule.save()

assert CasbinRule.select().count() == 2

e.remove_grouping_policy('bob', 'admin')

assert CasbinRule.select().count() == 0

print("OK")

output:

OK

Can you try this example with peewee.MySQLDatabase?

[cs1137195420]

@cs1137195420 I tried to remove duplicate rules with remove_grouping_policy and it worked for me. Example:

from casbin_peewee_adapter import CasbinRule, Adapter
import casbin
import peewee
import os

db_name = 'db.sqlite3'

if os.path.exists(db_name):
    os.remove(db_name)

db = peewee.SqliteDatabase(db_name)
adapter = Adapter(database=db)
db.create_tables([CasbinRule])

e = casbin.Enforcer('model.conf', adapter)

#add twice
e.add_grouping_policy('bob', 'admin')
e.add_grouping_policy('bob', 'admin')

assert CasbinRule.select().count() == 1

#add manually
rule = CasbinRule.create(ptype='g', v0='bob', v1='admin')
rule.save()

assert CasbinRule.select().count() == 2

e.remove_grouping_policy('bob', 'admin')

assert CasbinRule.select().count() == 0

print("OK")

output:

OK

Can you try this example with peewee.MySQLDatabase?

I tried it with peewee.MySQLDatabase, and my program also printed OK. However, when I called the add_grouping_policy, CasbinRule.create and remove_grouping_policy functions respectively, the error reappeared.
The error appeared in this way:

1. First run the following code

db = peewee.MySQLDatabase('casbin', user='root', passwd='root')
adapter = Adapter(database=db)
db.create_tables([CasbinRule])
e = casbin.Enforcer('model.conf', adapter)

# add twice
e.add_grouping_policy('bob', 'admin')
e.add_grouping_policy('bob', 'admin')
assert CasbinRule.select().count() == 1

# add manually
rule = CasbinRule.create(ptype='g', v0='bob', v1='admin')
rule.save()
assert CasbinRule.select().count() == 2

print("OK")

output:

OK

2. Next run the following code

db = peewee.MySQLDatabase('casbin', user='root', passwd='root')
adapter = Adapter(database=db)
db.create_tables([CasbinRule])
e = casbin.Enforcer('model.conf', adapter)

e.remove_grouping_policy('bob', 'admin')
assert CasbinRule.select().count() == 0

print("OK")

output:

Traceback (most recent call last):
  File "D:/project_hyb/casbin/pycasbin/main.py", line 22, in <module>
    assert CasbinRule.select().count() == 0
AssertionError

Can you try again in this way?

[abichinger]

This time I was able to reproduce your error.

Example:

from casbin_peewee_adapter import CasbinRule, Adapter
import casbin
import peewee
import os

db_name = 'db.sqlite3'

if os.path.exists(db_name):
    os.remove(db_name)

db = peewee.SqliteDatabase(db_name)
adapter = Adapter(database=db)
db.create_tables([CasbinRule])

e = casbin.Enforcer('model.conf', adapter)

#add twice
e.add_grouping_policy('bob', 'admin')
e.add_grouping_policy('bob', 'admin')

assert CasbinRule.select().count() == 1

#add manually
rule = CasbinRule.create(ptype='g', v0='bob', v1='admin')
rule.save()

assert CasbinRule.select().count() == 2

e2 = casbin.Enforcer('model.conf', adapter)
e2.remove_grouping_policy('bob', 'admin')

assert CasbinRule.select().count() == 0, f'{CasbinRule.select().count()} == 0'

print("OK")

Output:

Traceback (most recent call last):
  File "main.py", line 32, in <module>
    assert CasbinRule.select().count() == 0, f'{CasbinRule.select().count()} == 0'
AssertionError: 2 == 0

The problem is, that the peewee adapter loads the rule twice into the enforcer instance e2.
This happens during a call to Enforcer.load_policy.

-> e2.model.model['g']['g'].policy
[['bob', 'admin'], ['bob', 'admin']]

When e2.remove_grouping_policy is called line 211 in policy.py returns False, because there is still one rule left, after removing one.

In my opinion, the best solution would be to prevent duplicate entries in the adapter, using a unique constraint.

[cs1137195420]

@abichinger In case there are duplicate entries in the adapter, should remove_grouping_policy delete all of them?

[hsluoyz]

@cs1137195420 @abichinger plz see how Golang Casbin works

[abichinger]

@abichinger In case there are duplicate entries in the adapter, should remove_grouping_policy delete all of them?

I would say yes, but that's something the adapter has to do.

@hsluoyz The golang version has a similar issue. (link is above)

I think we could fix this issue, if we use Policy.add_policy inside persist.load_policy_line, instead of directly modifying the policy. This way, the duplicate rule should only be added once and remove_grouping_policy should work as expected

@cs1137195420 Would you like to create a PR for this?

[cs1137195420]

@abichinger In case there are duplicate entries in the adapter, should remove_grouping_policy delete all of them?

I would say yes, but that's something the adapter has to do.

@hsluoyz The golang version has a similar issue. (link is above)

I think we could fix this issue, if we use Policy.add_policy inside persist.load_policy_line, instead of directly modifying the policy. This way, the duplicate rule should only be added once and remove_grouping_policy should work as expected

@cs1137195420 Would you like to create a PR for this?

Sure, I would like to.

[github-actions]

🎉 This issue has been resolved in version 1.16.5 🎉

The release is available on:

• v1.16.5
• GitHub release

Your semantic-release bot 📦🚀