This is an automated email from the ASF dual-hosted git repository.
hsluoyz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/casbin-node-casbin.git
The following commit(s) were added to refs/heads/master by this push:
new 0a8629a fix: re-sort policies by subject hierarchy on policy change
0a8629a is described below
commit 0a8629a80f2de7af00cb51c21927e426f9d3f667
Author: Yang Luo <[email protected]>
AuthorDate: Thu Aug 6 23:02:16 2026 +0800
fix: re-sort policies by subject hierarchy on policy change
---
examples/subject_priority_policy_simple.csv | 4 ++
src/internalEnforcer.ts | 40 ++++++++-----
src/model/model.ts | 16 ++++--
test/enforcer.test.ts | 87 ++++++++++++++++++++++-------
4 files changed, 106 insertions(+), 41 deletions(-)
diff --git a/examples/subject_priority_policy_simple.csv
b/examples/subject_priority_policy_simple.csv
new file mode 100644
index 0000000..04e3993
--- /dev/null
+++ b/examples/subject_priority_policy_simple.csv
@@ -0,0 +1,4 @@
+p, group, data1, read, deny
+p, user, data1, read, allow
+
+g, user, group
diff --git a/src/internalEnforcer.ts b/src/internalEnforcer.ts
index bb898fb..9f7792b 100644
--- a/src/internalEnforcer.ts
+++ b/src/internalEnforcer.ts
@@ -21,6 +21,18 @@ import { PolicyOp } from './model';
* InternalEnforcer = CoreEnforcer + Internal API.
*/
export class InternalEnforcer extends CoreEnforcer {
+ /**
+ * The subjectPriority effect resolves conflicts by the order of the "p"
rules, so that order has
+ * to be rebuilt whenever a "p" or "g" rule changes, not only when the
policy is loaded from an
+ * adapter. It is a no-op for models that do not use the effect.
+ */
+ private resortPoliciesBySubjectHierarchy(ok: boolean): boolean {
+ if (ok) {
+ this.model.sortPoliciesBySubjectHierarchy();
+ }
+ return ok;
+ }
+
/**
* addPolicyInternal adds a rule to the current policy.
*/
@@ -56,7 +68,7 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype, [rule]);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
// addPolicies adds rules to the current policy.
@@ -98,7 +110,7 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok && effects?.length) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype, effects);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
/**
@@ -145,7 +157,7 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok && effects?.length) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype, effects);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
/**
@@ -192,7 +204,7 @@ export class InternalEnforcer extends CoreEnforcer {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype,
[newRule]);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
/**
@@ -229,7 +241,7 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype,
[rule]);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
// removePolicies removes rules from the current policy.
@@ -270,7 +282,7 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok && effects?.length) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype,
effects);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
/**
@@ -309,7 +321,7 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok && effects?.length) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype,
effects);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
/**
@@ -336,7 +348,7 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype, [rule]);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
protected async addPoliciesWithoutNotify(sec: string, ptype: string, rules:
string[][]): Promise<boolean> {
@@ -350,7 +362,7 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok && effects?.length) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype, effects);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
protected async addPoliciesWithoutNotifyEx(sec: string, ptype: string,
rules: string[][]): Promise<boolean> {
@@ -363,7 +375,7 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok && effects?.length) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype, effects);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
protected async updatePolicyWithoutNotify(sec: string, ptype: string,
oldRule: string[], newRule: string[]): Promise<boolean> {
@@ -376,7 +388,7 @@ export class InternalEnforcer extends CoreEnforcer {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype,
[oldRule]);
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype,
[newRule]);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
protected async removePolicyWithoutNotify(sec: string, ptype: string, rule:
string[]): Promise<boolean> {
@@ -388,7 +400,7 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype,
[rule]);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
protected async removePoliciesWithoutNotify(sec: string, ptype: string,
rules: string[][]): Promise<boolean> {
@@ -402,7 +414,7 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok && effects?.length) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype,
effects);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
protected async removeFilteredPolicyWithoutNotify(
@@ -415,7 +427,7 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok && effects?.length) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype,
effects);
}
- return ok;
+ return this.resortPoliciesBySubjectHierarchy(ok);
}
protected async updatePoliciesWithoutNotify(sec: string, ptype: string,
oldRules: string[][], newRules: string[][]): Promise<boolean> {
diff --git a/src/model/model.ts b/src/model/model.ts
index 0ec9050..49a7cd1 100644
--- a/src/model/model.ts
+++ b/src/model/model.ts
@@ -512,19 +512,23 @@ export class Model {
return;
}
+ const groupPolicies = this.model.get('g')?.get('g')?.policy;
+ if (!groupPolicies) {
+ return;
+ }
+
this.model.get('p')?.forEach((assertion, ptype) => {
const domainIndex = this.getFieldIndex(ptype, FieldIndex.Domain);
const subIndex = this.getFieldIndex(ptype, FieldIndex.Subject);
- // eslint-disable-next-line
- const subjectHierarchyMap =
this.getSubjectHierarchyMap(this.model.get('g')!.get('g')!.policy);
+ const subjectHierarchyMap = this.getSubjectHierarchyMap(groupPolicies);
assertion.policy.sort((policyA, policyB) => {
const domainA = domainIndex === -1 ? defaultDomain :
policyA[domainIndex];
const domainB = domainIndex === -1 ? defaultDomain :
policyB[domainIndex];
- // eslint-disable-next-line
- const priorityA =
subjectHierarchyMap.get(this.getNameWithDomain(domainA, policyA[subIndex]))!;
- // eslint-disable-next-line
- const priorityB =
subjectHierarchyMap.get(this.getNameWithDomain(domainB, policyB[subIndex]))!;
+ // A subject that takes part in no "g" rule has no depth, so it sorts
as a root would.
+ // Without the fallback the subtraction below yields NaN and scrambles
the whole array.
+ const priorityA =
subjectHierarchyMap.get(this.getNameWithDomain(domainA, policyA[subIndex])) ??
0;
+ const priorityB =
subjectHierarchyMap.get(this.getNameWithDomain(domainB, policyB[subIndex])) ??
0;
return priorityB - priorityA;
});
});
diff --git a/test/enforcer.test.ts b/test/enforcer.test.ts
index fe7750c..78d38a4 100644
--- a/test/enforcer.test.ts
+++ b/test/enforcer.test.ts
@@ -674,8 +674,8 @@ test('TestEnforceEx', async () => {
await e.addPermissionForUser('alice', 'data1', 'invalid');
- testEnforceEx(e, 'alice', 'data1', 'read', [false, []]);
- testEnforceEx(e, 'alice', 'data1', 'invalid', [true, ['alice', 'data1',
'invalid']]);
+ await testEnforceEx(e, 'alice', 'data1', 'read', [false, []]);
+ await testEnforceEx(e, 'alice', 'data1', 'invalid', [true, ['alice',
'data1', 'invalid']]);
});
test('TestSyncEnforceEx', async () => {
@@ -718,45 +718,90 @@ test('TestKeyGet2', async () => {
test('TestEnforceExWithRBACDenyModel', async () => {
const e = await newEnforcer('examples/rbac_with_deny_model.conf',
'examples/rbac_with_deny_policy.csv');
- testEnforceEx(e, 'alice', 'data1', 'read', [true, ['alice', 'data1', 'read',
'allow']]);
- testEnforceEx(e, 'bob', 'data2', 'write', [true, ['bob', 'data2', 'write',
'allow']]);
- testEnforceEx(e, 'alice', 'data2', 'write', [false, ['alice', 'data2',
'write', 'deny']]);
- testEnforceEx(e, 'data2_admin', 'data2', 'read', [true, ['data2_admin',
'data2', 'read', 'allow']]);
- testEnforceEx(e, 'data2_admin', 'data2', 'write', [true, ['data2_admin',
'data2', 'write', 'allow']]);
- testEnforceEx(e, 'data2_admin', 'data1', 'read', [false, []]);
- testEnforceEx(e, 'data2_admin', 'data1', 'write', [false, []]);
+ await testEnforceEx(e, 'alice', 'data1', 'read', [true, ['alice', 'data1',
'read', 'allow']]);
+ await testEnforceEx(e, 'bob', 'data2', 'write', [true, ['bob', 'data2',
'write', 'allow']]);
+ await testEnforceEx(e, 'alice', 'data2', 'write', [false, ['alice', 'data2',
'write', 'deny']]);
+ await testEnforceEx(e, 'data2_admin', 'data2', 'read', [true,
['data2_admin', 'data2', 'read', 'allow']]);
+ await testEnforceEx(e, 'data2_admin', 'data2', 'write', [true,
['data2_admin', 'data2', 'write', 'allow']]);
+ await testEnforceEx(e, 'data2_admin', 'data1', 'read', [false, []]);
+ await testEnforceEx(e, 'data2_admin', 'data1', 'write', [false, []]);
});
test('TestEnforceExWithGlobModel', async () => {
const e = await newEnforcer('examples/glob_model.conf',
'examples/glob_policy.csv');
- testEnforceEx(e, 'u1', '/foo/1', 'read', [true, ['u1', '/foo/*', 'read']]);
- testEnforceEx(e, 'u2', '/foo1', 'read', [true, ['u2', '/foo*', 'read']]);
- testEnforceEx(e, 'u3', '/foo/2', 'read', [false, []]);
+ await testEnforceEx(e, 'u1', '/foo/1', 'read', [true, ['u1', '/foo/*',
'read']]);
+ await testEnforceEx(e, 'u2', '/foo1', 'read', [true, ['u2', '/foo*',
'read']]);
+ await testEnforceEx(e, 'u3', '/foo/2', 'read', [false, []]);
});
test('TestEnforceExWithKeyMatchModel', async () => {
const e = await newEnforcer('examples/keymatch_model.conf',
'examples/keymatch_policy.csv');
- testEnforceEx(e, 'alice', '/alice_data/1', 'GET', [true, ['alice',
'/alice_data/*', 'GET']]);
- testEnforceEx(e, 'bob', '/alice_data/1', 'POST', [false, []]);
+ await testEnforceEx(e, 'alice', '/alice_data/1', 'GET', [true, ['alice',
'/alice_data/*', 'GET']]);
+ await testEnforceEx(e, 'bob', '/alice_data/1', 'POST', [false, []]);
});
test('TestEnforceExWithPriorityModel', async () => {
const e = await newEnforcer('examples/priority_model.conf',
'examples/priority_policy.csv');
- testEnforceEx(e, 'alice', 'data1', 'read', [true, ['alice', 'data1', 'read',
'allow']]);
- testEnforceEx(e, 'bob', 'data2', 'read', [true, ['data2_allow_group',
'data2', 'read', 'allow']]);
- testEnforceEx(e, 'alice', 'data2', 'read', [false, []]);
+ await testEnforceEx(e, 'alice', 'data1', 'read', [true, ['alice', 'data1',
'read', 'allow']]);
+ await testEnforceEx(e, 'bob', 'data2', 'read', [true, ['data2_allow_group',
'data2', 'read', 'allow']]);
+ await testEnforceEx(e, 'alice', 'data2', 'read', [false, []]);
});
test('TestSubjectPriority', async () => {
const e = await newEnforcer('examples/subject_priority_model.conf',
'examples/subject_priority_policy.csv');
- testEnforceEx(e, 'jane', 'data1', 'read', [true, ['jane', 'data1', 'read',
'allow']]);
- testEnforceEx(e, 'alice', 'data1', 'read', [true, ['alice', 'data1', 'read',
'allow']]);
+ await testEnforceEx(e, 'jane', 'data1', 'read', [true, ['jane', 'data1',
'read', 'allow']]);
+ await testEnforceEx(e, 'alice', 'data1', 'read', [true, ['alice', 'data1',
'read', 'allow']]);
+});
+
+test('TestSubjectPriority with CSV converted to addPolicy/addGroupingPolicy',
async () => {
+ const e = await newEnforcer('examples/subject_priority_model.conf');
+
+ await e.addPolicy('root', 'data1', 'read', 'deny');
+ await e.addPolicy('admin', 'data1', 'read', 'deny');
+ await e.addPolicy('editor', 'data1', 'read', 'deny');
+ await e.addPolicy('subscriber', 'data1', 'read', 'deny');
+ await e.addPolicy('jane', 'data1', 'read', 'allow');
+ await e.addPolicy('alice', 'data1', 'read', 'allow');
+
+ await e.addGroupingPolicy('admin', 'root');
+ await e.addGroupingPolicy('editor', 'admin');
+ await e.addGroupingPolicy('subscriber', 'admin');
+ await e.addGroupingPolicy('jane', 'editor');
+ await e.addGroupingPolicy('alice', 'subscriber');
+
+ await testEnforceEx(e, 'jane', 'data1', 'read', [true, ['jane', 'data1',
'read', 'allow']]);
+ await testEnforceEx(e, 'alice', 'data1', 'read', [true, ['alice', 'data1',
'read', 'allow']]);
+});
+
+test('TestSubjectPriority simpler with CSV', async () => {
+ const e = await newEnforcer('examples/subject_priority_model.conf',
'examples/subject_priority_policy_simple.csv');
+
+ await testEnforceEx(e, 'user', 'data1', 'read', [true, ['user', 'data1',
'read', 'allow']]);
+});
+
+test('TestSubjectPriority simpler with addPolicy', async () => {
+ const e = await newEnforcer('examples/subject_priority_model.conf');
+
+ await e.addPolicy('group', 'data1', 'read', 'deny');
+ await e.addPolicy('user', 'data1', 'read', 'allow');
+ await e.addGroupingPolicy('user', 'group');
+
+ await testEnforceEx(e, 'user', 'data1', 'read', [true, ['user', 'data1',
'read', 'allow']]);
+});
+
+test('TestSubjectPriority is kept after removing a grouping policy', async ()
=> {
+ const e = await newEnforcer('examples/subject_priority_model.conf',
'examples/subject_priority_policy.csv');
+
+ // jane loses her role, so only the "jane" rule itself is left to match her.
+ await e.removeGroupingPolicy('jane', 'editor');
+ await testEnforceEx(e, 'jane', 'data1', 'read', [true, ['jane', 'data1',
'read', 'allow']]);
+ await testEnforceEx(e, 'alice', 'data1', 'read', [true, ['alice', 'data1',
'read', 'allow']]);
});
test('TestSubjectPriorityWithDomain', async () => {
const e = await
newEnforcer('examples/subject_priority_model_with_domain.conf',
'examples/subject_priority_policy_with_domain.csv');
- testEnforceEx(e, 'alice', 'data1', 'write', [true, ['alice', 'data1',
'domain1', 'write', 'allow']], 'domain1');
- testEnforceEx(e, 'bob', 'data2', 'write', [true, ['bob', 'data2', 'domain2',
'write', 'allow']], 'domain2');
+ await testEnforceEx(e, 'alice', 'data1', 'write', [true, ['alice', 'data1',
'domain1', 'write', 'allow']], 'domain1');
+ await testEnforceEx(e, 'bob', 'data2', 'write', [true, ['bob', 'data2',
'domain2', 'write', 'allow']], 'domain2');
});
test('TestEnforcerWithScopeFileSystem', async () => {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
