Skip to content

Commit 5de81ca

Browse files
Copilothsluoyz
andcommitted
Add useAdapter parameter to bypass adapter calls in self* methods
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
1 parent c87a0af commit 5de81ca

File tree

3 files changed

+272
-18
lines changed

3 files changed

+272
-18
lines changed

src/internalEnforcer.ts

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ export class InternalEnforcer extends CoreEnforcer {
2424
/**
2525
* addPolicyInternal adds a rule to the current policy.
2626
*/
27-
protected async addPolicyInternal(sec: string, ptype: string, rule: string[], useWatcher: boolean): Promise<boolean> {
27+
protected async addPolicyInternal(sec: string, ptype: string, rule: string[], useWatcher: boolean, useAdapter = true): Promise<boolean> {
2828
if (this.model.hasPolicy(sec, ptype, rule)) {
2929
return false;
3030
}
3131

32-
if (this.adapter && this.autoSave) {
32+
if (useAdapter && this.adapter && this.autoSave) {
3333
try {
3434
await this.adapter.addPolicy(sec, ptype, rule);
3535
} catch (e) {
@@ -60,14 +60,20 @@ export class InternalEnforcer extends CoreEnforcer {
6060

6161
// addPolicies adds rules to the current policy.
6262
// removePolicies removes rules from the current policy.
63-
protected async addPoliciesInternal(sec: string, ptype: string, rules: string[][], useWatcher: boolean): Promise<boolean> {
63+
protected async addPoliciesInternal(
64+
sec: string,
65+
ptype: string,
66+
rules: string[][],
67+
useWatcher: boolean,
68+
useAdapter = true
69+
): Promise<boolean> {
6470
for (const rule of rules) {
6571
if (this.model.hasPolicy(sec, ptype, rule)) {
6672
return false;
6773
}
6874
}
6975

70-
if (this.autoSave) {
76+
if (useAdapter && this.autoSave) {
7177
if ('addPolicies' in this.adapter) {
7278
try {
7379
await this.adapter.addPolicies(sec, ptype, rules);
@@ -107,13 +113,14 @@ export class InternalEnforcer extends CoreEnforcer {
107113
ptype: string,
108114
oldRule: string[],
109115
newRule: string[],
110-
useWatcher: boolean
116+
useWatcher: boolean,
117+
useAdapter = true
111118
): Promise<boolean> {
112119
if (!this.model.hasPolicy(sec, ptype, oldRule)) {
113120
return false;
114121
}
115122

116-
if (this.autoSave) {
123+
if (useAdapter && this.autoSave) {
117124
if ('updatePolicy' in this.adapter) {
118125
try {
119126
await this.adapter.updatePolicy(sec, ptype, oldRule, newRule);
@@ -149,12 +156,18 @@ export class InternalEnforcer extends CoreEnforcer {
149156
/**
150157
* removePolicyInternal removes a rule from the current policy.
151158
*/
152-
protected async removePolicyInternal(sec: string, ptype: string, rule: string[], useWatcher: boolean): Promise<boolean> {
159+
protected async removePolicyInternal(
160+
sec: string,
161+
ptype: string,
162+
rule: string[],
163+
useWatcher: boolean,
164+
useAdapter = true
165+
): Promise<boolean> {
153166
if (!this.model.hasPolicy(sec, ptype, rule)) {
154167
return false;
155168
}
156169

157-
if (this.adapter && this.autoSave) {
170+
if (useAdapter && this.adapter && this.autoSave) {
158171
try {
159172
await this.adapter.removePolicy(sec, ptype, rule);
160173
} catch (e) {
@@ -183,14 +196,20 @@ export class InternalEnforcer extends CoreEnforcer {
183196
}
184197

185198
// removePolicies removes rules from the current policy.
186-
protected async removePoliciesInternal(sec: string, ptype: string, rules: string[][], useWatcher: boolean): Promise<boolean> {
199+
protected async removePoliciesInternal(
200+
sec: string,
201+
ptype: string,
202+
rules: string[][],
203+
useWatcher: boolean,
204+
useAdapter = true
205+
): Promise<boolean> {
187206
for (const rule of rules) {
188207
if (!this.model.hasPolicy(sec, ptype, rule)) {
189208
return false;
190209
}
191210
}
192211

193-
if (this.autoSave) {
212+
if (useAdapter && this.autoSave) {
194213
if ('removePolicies' in this.adapter) {
195214
try {
196215
await this.adapter.removePolicies(sec, ptype, rules);
@@ -230,9 +249,10 @@ export class InternalEnforcer extends CoreEnforcer {
230249
ptype: string,
231250
fieldIndex: number,
232251
fieldValues: string[],
233-
useWatcher: boolean
252+
useWatcher: boolean,
253+
useAdapter = true
234254
): Promise<boolean> {
235-
if (this.adapter && this.autoSave) {
255+
if (useAdapter && this.adapter && this.autoSave) {
236256
try {
237257
await this.adapter.removeFilteredPolicy(sec, ptype, fieldIndex, ...fieldValues);
238258
} catch (e) {

src/managementEnforcer.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -541,26 +541,26 @@ export class ManagementEnforcer extends InternalEnforcer {
541541
}
542542

543543
public async selfAddPolicy(sec: string, ptype: string, rule: string[]): Promise<boolean> {
544-
return this.addPolicyInternal(sec, ptype, rule, false);
544+
return this.addPolicyInternal(sec, ptype, rule, false, false);
545545
}
546546

547547
public async selfRemovePolicy(sec: string, ptype: string, rule: string[]): Promise<boolean> {
548-
return this.removePolicyInternal(sec, ptype, rule, false);
548+
return this.removePolicyInternal(sec, ptype, rule, false, false);
549549
}
550550

551551
public async selfRemoveFilteredPolicy(sec: string, ptype: string, fieldIndex: number, fieldValues: string[]): Promise<boolean> {
552-
return this.removeFilteredPolicyInternal(sec, ptype, fieldIndex, fieldValues, false);
552+
return this.removeFilteredPolicyInternal(sec, ptype, fieldIndex, fieldValues, false, false);
553553
}
554554

555555
public async selfUpdatePolicy(sec: string, ptype: string, oldRule: string[], newRule: string[]): Promise<boolean> {
556-
return this.updatePolicyInternal(sec, ptype, oldRule, newRule, false);
556+
return this.updatePolicyInternal(sec, ptype, oldRule, newRule, false, false);
557557
}
558558

559559
public async selfAddPolicies(sec: string, ptype: string, rule: string[][]): Promise<boolean> {
560-
return this.addPoliciesInternal(sec, ptype, rule, false);
560+
return this.addPoliciesInternal(sec, ptype, rule, false, false);
561561
}
562562

563563
public async selfRemovePolicies(sec: string, ptype: string, rule: string[][]): Promise<boolean> {
564-
return this.removePoliciesInternal(sec, ptype, rule, false);
564+
return this.removePoliciesInternal(sec, ptype, rule, false, false);
565565
}
566566
}

test/selfMethods.test.ts

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
// Copyright 2024 The Casbin Authors. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import { newEnforcer, Enforcer, Model } from '../src';
16+
import { Adapter } from '../src';
17+
18+
// Mock adapter that tracks method calls
19+
class MockAdapter implements Adapter {
20+
public addPolicyCalled = false;
21+
public removePolicyCalled = false;
22+
public addPoliciesCalled = false;
23+
public removePoliciesCalled = false;
24+
25+
async loadPolicy(model: Model): Promise<void> {
26+
// No-op
27+
}
28+
29+
async savePolicy(model: Model): Promise<boolean> {
30+
throw new Error('not implemented');
31+
}
32+
33+
async addPolicy(sec: string, ptype: string, rule: string[]): Promise<void> {
34+
this.addPolicyCalled = true;
35+
}
36+
37+
async removePolicy(sec: string, ptype: string, rule: string[]): Promise<void> {
38+
this.removePolicyCalled = true;
39+
}
40+
41+
async removeFilteredPolicy(sec: string, ptype: string, fieldIndex: number, ...fieldValues: string[]): Promise<void> {
42+
throw new Error('not implemented');
43+
}
44+
}
45+
46+
// Mock adapter that also implements BatchAdapter
47+
class MockBatchAdapter extends MockAdapter {
48+
async addPolicies(sec: string, ptype: string, rules: string[][]): Promise<void> {
49+
this.addPoliciesCalled = true;
50+
}
51+
52+
async removePolicies(sec: string, ptype: string, rules: string[][]): Promise<void> {
53+
this.removePoliciesCalled = true;
54+
}
55+
}
56+
57+
describe('Self* methods should not call adapter', () => {
58+
let e: Enforcer;
59+
let adapter: MockBatchAdapter;
60+
61+
beforeEach(async () => {
62+
adapter = new MockBatchAdapter();
63+
e = await newEnforcer('examples/rbac_model.conf', adapter);
64+
65+
// Load initial policy manually
66+
await e.addPolicy('alice', 'data1', 'read');
67+
await e.addPolicy('bob', 'data2', 'write');
68+
69+
// Reset flags after setup
70+
adapter.addPolicyCalled = false;
71+
adapter.removePolicyCalled = false;
72+
adapter.addPoliciesCalled = false;
73+
adapter.removePoliciesCalled = false;
74+
});
75+
76+
test('selfAddPolicy should not call adapter', async () => {
77+
const result = await e.selfAddPolicy('p', 'p', ['charlie', 'data3', 'read']);
78+
79+
expect(result).toBe(true);
80+
expect(adapter.addPolicyCalled).toBe(false);
81+
82+
// Verify the policy was added to the model
83+
const hasPolicy = await e.hasPolicy('charlie', 'data3', 'read');
84+
expect(hasPolicy).toBe(true);
85+
});
86+
87+
test('selfRemovePolicy should not call adapter', async () => {
88+
const result = await e.selfRemovePolicy('p', 'p', ['alice', 'data1', 'read']);
89+
90+
expect(result).toBe(true);
91+
expect(adapter.removePolicyCalled).toBe(false);
92+
93+
// Verify the policy was removed from the model
94+
const hasPolicy = await e.hasPolicy('alice', 'data1', 'read');
95+
expect(hasPolicy).toBe(false);
96+
});
97+
98+
test('selfAddPolicies should not call adapter', async () => {
99+
const rules = [
100+
['charlie', 'data3', 'read'],
101+
['david', 'data4', 'write'],
102+
];
103+
const result = await e.selfAddPolicies('p', 'p', rules);
104+
105+
expect(result).toBe(true);
106+
expect(adapter.addPoliciesCalled).toBe(false);
107+
108+
// Verify the policies were added to the model
109+
expect(await e.hasPolicy('charlie', 'data3', 'read')).toBe(true);
110+
expect(await e.hasPolicy('david', 'data4', 'write')).toBe(true);
111+
});
112+
113+
test('selfRemovePolicies should not call adapter', async () => {
114+
const rules = [
115+
['alice', 'data1', 'read'],
116+
['bob', 'data2', 'write'],
117+
];
118+
const result = await e.selfRemovePolicies('p', 'p', rules);
119+
120+
expect(result).toBe(true);
121+
expect(adapter.removePoliciesCalled).toBe(false);
122+
123+
// Verify the policies were removed from the model
124+
expect(await e.hasPolicy('alice', 'data1', 'read')).toBe(false);
125+
expect(await e.hasPolicy('bob', 'data2', 'write')).toBe(false);
126+
});
127+
128+
test('selfUpdatePolicy should not call adapter', async () => {
129+
const result = await e.selfUpdatePolicy('p', 'p', ['alice', 'data1', 'read'], ['alice', 'data1', 'write']);
130+
131+
expect(result).toBe(true);
132+
// Note: updatePolicy is not in our basic MockAdapter, but we verify it doesn't call addPolicy or removePolicy
133+
expect(adapter.addPolicyCalled).toBe(false);
134+
expect(adapter.removePolicyCalled).toBe(false);
135+
136+
// Verify the policy was updated in the model
137+
expect(await e.hasPolicy('alice', 'data1', 'read')).toBe(false);
138+
expect(await e.hasPolicy('alice', 'data1', 'write')).toBe(true);
139+
});
140+
141+
test('selfRemoveFilteredPolicy should not call adapter', async () => {
142+
await e.addPolicy('alice', 'data2', 'read');
143+
adapter.addPolicyCalled = false;
144+
145+
const result = await e.selfRemoveFilteredPolicy('p', 'p', 0, ['alice']);
146+
147+
expect(result).toBe(true);
148+
expect(adapter.removePolicyCalled).toBe(false);
149+
150+
// Verify the filtered policies were removed from the model
151+
expect(await e.hasPolicy('alice', 'data1', 'read')).toBe(false);
152+
expect(await e.hasPolicy('alice', 'data2', 'read')).toBe(false);
153+
});
154+
155+
test('regular addPolicy should call adapter with autoSave enabled', async () => {
156+
e.enableAutoSave(true);
157+
const result = await e.addPolicy('charlie', 'data3', 'read');
158+
159+
expect(result).toBe(true);
160+
expect(adapter.addPolicyCalled).toBe(true);
161+
});
162+
163+
test('regular removePolicy should call adapter with autoSave enabled', async () => {
164+
e.enableAutoSave(true);
165+
const result = await e.removePolicy('alice', 'data1', 'read');
166+
167+
expect(result).toBe(true);
168+
expect(adapter.removePolicyCalled).toBe(true);
169+
});
170+
171+
test('self* methods work correctly even when autoSave is disabled', async () => {
172+
e.enableAutoSave(false);
173+
174+
const addResult = await e.selfAddPolicy('p', 'p', ['charlie', 'data3', 'read']);
175+
expect(addResult).toBe(true);
176+
expect(adapter.addPolicyCalled).toBe(false);
177+
expect(await e.hasPolicy('charlie', 'data3', 'read')).toBe(true);
178+
179+
const removeResult = await e.selfRemovePolicy('p', 'p', ['charlie', 'data3', 'read']);
180+
expect(removeResult).toBe(true);
181+
expect(adapter.removePolicyCalled).toBe(false);
182+
expect(await e.hasPolicy('charlie', 'data3', 'read')).toBe(false);
183+
});
184+
});
185+
186+
describe('Self* methods with RBAC', () => {
187+
let e: Enforcer;
188+
let adapter: MockBatchAdapter;
189+
190+
beforeEach(async () => {
191+
adapter = new MockBatchAdapter();
192+
e = await newEnforcer('examples/rbac_model.conf', adapter);
193+
194+
// Load initial policy manually
195+
await e.addGroupingPolicy('alice', 'admin');
196+
await e.addPolicy('admin', 'data1', 'read');
197+
198+
// Reset flags after setup
199+
adapter.addPolicyCalled = false;
200+
adapter.removePolicyCalled = false;
201+
adapter.addPoliciesCalled = false;
202+
adapter.removePoliciesCalled = false;
203+
});
204+
205+
test('selfAddPolicy with roles should not call adapter', async () => {
206+
const result = await e.selfAddPolicy('g', 'g', ['bob', 'admin']);
207+
208+
expect(result).toBe(true);
209+
expect(adapter.addPolicyCalled).toBe(false);
210+
211+
// Verify the role was added
212+
const hasRole = await e.hasGroupingPolicy('bob', 'admin');
213+
expect(hasRole).toBe(true);
214+
215+
// Verify role links work (bob should inherit admin permissions)
216+
const enforce = await e.enforce('bob', 'data1', 'read');
217+
expect(enforce).toBe(true);
218+
});
219+
220+
test('selfRemovePolicy with roles should not call adapter', async () => {
221+
const result = await e.selfRemovePolicy('g', 'g', ['alice', 'admin']);
222+
223+
expect(result).toBe(true);
224+
expect(adapter.removePolicyCalled).toBe(false);
225+
226+
// Verify the role was removed
227+
const hasRole = await e.hasGroupingPolicy('alice', 'admin');
228+
expect(hasRole).toBe(false);
229+
230+
// Verify role links were updated (alice should no longer have admin permissions)
231+
const enforce = await e.enforce('alice', 'data1', 'read');
232+
expect(enforce).toBe(false);
233+
});
234+
});

0 commit comments

Comments
 (0)