Skip to content

Fix subject priority sorting for programmatically added policies #515

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.

Sign up for GitHub

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

Closed
Copilot wants to merge 3 commits into from
Closed

Fix subject priority sorting for programmatically added policies #515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a3a3840
Initial plan
Copilot Nov 11, 2025
bb471ad
Initial plan
Copilot Nov 11, 2025
b2a65de
Fix subject priority for programmatically added policies
Copilot Nov 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions examples/subject_priority_policy_simple.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
p, group, data1, read, deny
p, user, data1, read, allow
g, user, group
30 changes: 30 additions & 0 deletions src/internalEnforcer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ export class InternalEnforcer extends CoreEnforcer {

if (sec === 'g' && ok) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype, [rule]);
// Sort policies by subject hierarchy after adding grouping policy
this.model.sortPoliciesBySubjectHierarchy();
} else if (sec === 'p' && ok) {
// Sort policies by subject hierarchy after adding policy
this.model.sortPoliciesBySubjectHierarchy();
}
return ok;
}
Expand Down Expand Up @@ -95,6 +100,11 @@ export class InternalEnforcer extends CoreEnforcer {
const [ok, effects] = await this.model.addPolicies(sec, ptype, rules);
if (sec === 'g' && ok && effects?.length) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype, effects);
// Sort policies by subject hierarchy after adding grouping policies
this.model.sortPoliciesBySubjectHierarchy();
} else if (sec === 'p' && ok) {
// Sort policies by subject hierarchy after adding policies
this.model.sortPoliciesBySubjectHierarchy();
}
return ok;
}
Expand Down Expand Up @@ -141,6 +151,11 @@ export class InternalEnforcer extends CoreEnforcer {
if (sec === 'g' && ok) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype, [oldRule]);
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype, [newRule]);
// Sort policies by subject hierarchy after updating grouping policy
this.model.sortPoliciesBySubjectHierarchy();
} else if (sec === 'p' && ok) {
// Sort policies by subject hierarchy after updating policy
this.model.sortPoliciesBySubjectHierarchy();
}

return ok;
Expand Down Expand Up @@ -178,6 +193,11 @@ export class InternalEnforcer extends CoreEnforcer {
const ok = await this.model.removePolicy(sec, ptype, rule);
if (sec === 'g' && ok) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype, [rule]);
// Sort policies by subject hierarchy after removing grouping policy
this.model.sortPoliciesBySubjectHierarchy();
} else if (sec === 'p' && ok) {
// Sort policies by subject hierarchy after removing policy
this.model.sortPoliciesBySubjectHierarchy();
}
return ok;
}
Expand Down Expand Up @@ -218,6 +238,11 @@ export class InternalEnforcer extends CoreEnforcer {
const [ok, effects] = this.model.removePolicies(sec, ptype, rules);
if (sec === 'g' && ok && effects?.length) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype, effects);
// Sort policies by subject hierarchy after removing grouping policies
this.model.sortPoliciesBySubjectHierarchy();
} else if (sec === 'p' && ok) {
// Sort policies by subject hierarchy after removing policies
this.model.sortPoliciesBySubjectHierarchy();
}
return ok;
}
Expand Down Expand Up @@ -256,6 +281,11 @@ export class InternalEnforcer extends CoreEnforcer {
const [ok, effects] = this.model.removeFilteredPolicy(sec, ptype, fieldIndex, ...fieldValues);
if (sec === 'g' && ok && effects?.length) {
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype, effects);
// Sort policies by subject hierarchy after removing filtered grouping policies
this.model.sortPoliciesBySubjectHierarchy();
} else if (sec === 'p' && ok) {
// Sort policies by subject hierarchy after removing filtered policies
this.model.sortPoliciesBySubjectHierarchy();
}
return ok;
}
Expand Down
32 changes: 32 additions & 0 deletions test/enforcer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,38 @@ test('TestSubjectPriorityWithDomain', async () => {
testEnforceEx(e, 'bob', 'data2', 'write', [true, ['bob', 'data2', 'domain2', 'write', 'allow']], 'domain2');
});

test('TestSubjectPriority simpler with CSV', async () => {
const e = await newEnforcer('examples/subject_priority_model.conf', 'examples/subject_priority_policy_simple.csv');
// user should be allowed to read data1 because the direct allow policy takes priority over the inherited deny policy from the group role
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');
// user should be allowed to read data1 because the direct allow policy takes priority over the inherited deny policy from the group role
testEnforceEx(e, 'user', 'data1', 'read', [true, ['user', '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');
testEnforceEx(e, 'jane', 'data1', 'read', [true, ['jane', 'data1', 'read', 'allow']]);
testEnforceEx(e, 'alice', 'data1', 'read', [true, ['alice', 'data1', 'read', 'allow']]);
});

test('TestEnforcerWithScopeFileSystem', async () => {
const e = await newEnforcer();
const defaultFileSystem = {
Expand Down
Loading
Loading