Skip to content

Commit b2a65de

Browse files
Copilotnomeguy
andcommitted
Fix subject priority for programmatically added policies
Call sortPoliciesBySubjectHierarchy() after adding/removing/updating policies to ensure correct ordering when using subject priority effect Co-authored-by: nomeguy <85475922+nomeguy@users.noreply.github.com>
1 parent bb471ad commit b2a65de

File tree

2 files changed

+30
-2
lines changed

2 files changed

+30
-2
lines changed

src/internalEnforcer.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ export class InternalEnforcer extends CoreEnforcer {
5454

5555
if (sec === 'g' && ok) {
5656
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype, [rule]);
57+
// Sort policies by subject hierarchy after adding grouping policy
58+
this.model.sortPoliciesBySubjectHierarchy();
59+
} else if (sec === 'p' && ok) {
60+
// Sort policies by subject hierarchy after adding policy
61+
this.model.sortPoliciesBySubjectHierarchy();
5762
}
5863
return ok;
5964
}
@@ -95,6 +100,11 @@ export class InternalEnforcer extends CoreEnforcer {
95100
const [ok, effects] = await this.model.addPolicies(sec, ptype, rules);
96101
if (sec === 'g' && ok && effects?.length) {
97102
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype, effects);
103+
// Sort policies by subject hierarchy after adding grouping policies
104+
this.model.sortPoliciesBySubjectHierarchy();
105+
} else if (sec === 'p' && ok) {
106+
// Sort policies by subject hierarchy after adding policies
107+
this.model.sortPoliciesBySubjectHierarchy();
98108
}
99109
return ok;
100110
}
@@ -141,6 +151,11 @@ export class InternalEnforcer extends CoreEnforcer {
141151
if (sec === 'g' && ok) {
142152
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype, [oldRule]);
143153
await this.buildIncrementalRoleLinks(PolicyOp.PolicyAdd, ptype, [newRule]);
154+
// Sort policies by subject hierarchy after updating grouping policy
155+
this.model.sortPoliciesBySubjectHierarchy();
156+
} else if (sec === 'p' && ok) {
157+
// Sort policies by subject hierarchy after updating policy
158+
this.model.sortPoliciesBySubjectHierarchy();
144159
}
145160

146161
return ok;
@@ -178,6 +193,11 @@ export class InternalEnforcer extends CoreEnforcer {
178193
const ok = await this.model.removePolicy(sec, ptype, rule);
179194
if (sec === 'g' && ok) {
180195
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype, [rule]);
196+
// Sort policies by subject hierarchy after removing grouping policy
197+
this.model.sortPoliciesBySubjectHierarchy();
198+
} else if (sec === 'p' && ok) {
199+
// Sort policies by subject hierarchy after removing policy
200+
this.model.sortPoliciesBySubjectHierarchy();
181201
}
182202
return ok;
183203
}
@@ -218,6 +238,11 @@ export class InternalEnforcer extends CoreEnforcer {
218238
const [ok, effects] = this.model.removePolicies(sec, ptype, rules);
219239
if (sec === 'g' && ok && effects?.length) {
220240
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype, effects);
241+
// Sort policies by subject hierarchy after removing grouping policies
242+
this.model.sortPoliciesBySubjectHierarchy();
243+
} else if (sec === 'p' && ok) {
244+
// Sort policies by subject hierarchy after removing policies
245+
this.model.sortPoliciesBySubjectHierarchy();
221246
}
222247
return ok;
223248
}
@@ -256,6 +281,11 @@ export class InternalEnforcer extends CoreEnforcer {
256281
const [ok, effects] = this.model.removeFilteredPolicy(sec, ptype, fieldIndex, ...fieldValues);
257282
if (sec === 'g' && ok && effects?.length) {
258283
await this.buildIncrementalRoleLinks(PolicyOp.PolicyRemove, ptype, effects);
284+
// Sort policies by subject hierarchy after removing filtered grouping policies
285+
this.model.sortPoliciesBySubjectHierarchy();
286+
} else if (sec === 'p' && ok) {
287+
// Sort policies by subject hierarchy after removing filtered policies
288+
this.model.sortPoliciesBySubjectHierarchy();
259289
}
260290
return ok;
261291
}

test/enforcer.test.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -761,7 +761,6 @@ test('TestSubjectPriorityWithDomain', async () => {
761761

762762
test('TestSubjectPriority simpler with CSV', async () => {
763763
const e = await newEnforcer('examples/subject_priority_model.conf', 'examples/subject_priority_policy_simple.csv');
764-
fs.writeFileSync('/tmp/csv_policies.txt', JSON.stringify(e.getPolicy(), null, 2));
765764
// user should be allowed to read data1 because the direct allow policy takes priority over the inherited deny policy from the group role
766765
testEnforceEx(e, 'user', 'data1', 'read', [true, ['user', 'data1', 'read', 'allow']]);
767766
});
@@ -771,7 +770,6 @@ test('TestSubjectPriority simpler with addPolicy', async () => {
771770
await e.addPolicy('group', 'data1', 'read', 'deny');
772771
await e.addPolicy('user', 'data1', 'read', 'allow');
773772
await e.addGroupingPolicy('user', 'group');
774-
fs.writeFileSync('/tmp/addpolicy_policies.txt', JSON.stringify(e.getPolicy(), null, 2));
775773
// user should be allowed to read data1 because the direct allow policy takes priority over the inherited deny policy from the group role
776774
testEnforceEx(e, 'user', 'data1', 'read', [true, ['user', 'data1', 'read', 'allow']]);
777775
});

0 commit comments

Comments
 (0)