Skip to content

Commit 93c23fa

Browse files
Copilotmserico
andcommitted
Refactor to match Go Casbin implementation pattern
- Add getDomains and getAllDomains methods to RoleManager interface - Implement methods in DefaultRoleManager following Go pattern - Update Enforcer to call RoleManager methods (not ManagementEnforcer) - Add rbac_with_domains_policy2.csv test file matching Go Casbin - Update tests to match Go Casbin test patterns and naming - Filter out default domain when other domains exist Co-authored-by: mserico <140243407+mserico@users.noreply.github.com>
1 parent 37a8c5f commit 93c23fa

File tree

7 files changed

+87
-78
lines changed

7 files changed

+87
-78
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
p, admin, domain1, data1, read
2+
p, admin, domain1, data1, write
3+
p, admin, domain2, data2, read
4+
p, admin, domain2, data2, write
5+
p, user, domain3, data2, read
6+
g, alice, admin, domain1
7+
g, alice, admin, domain2
8+
g, bob, admin, domain2
9+
g, bob, user, domain3

src/enforcer.ts

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -466,28 +466,27 @@ export class Enforcer extends ManagementEnforcer {
466466
}
467467

468468
/**
469-
* getDomainsForUser gets all domains that a user has.
470-
* For example:
471-
* g, alice, admin, domain1
472-
* g, alice, member, domain2
473-
*
474-
* getDomainsForUser("alice") will get: ["domain1", "domain2"].
475-
*
476-
* @param user the user.
477-
* @return the domains that the user has.
469+
* getDomainsForUser gets all domains.
478470
*/
479471
public async getDomainsForUser(user: string): Promise<string[]> {
480-
const groupingPolicies = await this.getFilteredGroupingPolicy(0, user);
481-
const domains = new Set<string>();
482-
483-
for (const policy of groupingPolicies) {
484-
// In a policy like ["alice", "admin", "domain1"], domain is at index 2
485-
if (policy.length > 2 && policy[2] && policy[2].trim() !== '') {
486-
domains.add(policy[2]);
487-
}
472+
const domains: string[] = [];
473+
for (const rm of this.rmMap.values()) {
474+
const domain = await rm.getDomains(user);
475+
domains.push(...domain);
488476
}
477+
return domains;
478+
}
489479

490-
return Array.from(domains);
480+
/**
481+
* getAllDomains gets all domains.
482+
*/
483+
public async getAllDomains(): Promise<string[]> {
484+
const domains: string[] = [];
485+
for (const rm of this.rmMap.values()) {
486+
const domain = await rm.getAllDomains();
487+
domains.push(...domain);
488+
}
489+
return arrayRemoveDuplicates(domains);
491490
}
492491
}
493492

src/managementEnforcer.ts

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -118,31 +118,6 @@ export class ManagementEnforcer extends InternalEnforcer {
118118
return this.model.getValuesForFieldInPolicy('g', ptype, 1);
119119
}
120120

121-
/**
122-
* getAllDomains gets the list of domains that show up in the current policy.
123-
*
124-
* @return all the domains in "g" policy rules. It actually collects
125-
* the 2-index elements of "g" policy rules. So make sure your
126-
* domain is the 2-index element, like (sub, role, domain).
127-
* Duplicates are removed.
128-
*/
129-
public async getAllDomains(): Promise<string[]> {
130-
return this.getAllNamedDomains('g');
131-
}
132-
133-
/**
134-
* getAllNamedDomains gets the list of domains that show up in the current named policy.
135-
*
136-
* @param ptype the policy type, can be "g", "g2", "g3", ..
137-
* @return all the domains in policy rules of the ptype type. It actually
138-
* collects the 2-index elements of the policy rules. So make
139-
* sure your domain is the 2-index element, like (sub, role, domain).
140-
* Duplicates are removed.
141-
*/
142-
public async getAllNamedDomains(ptype: string): Promise<string[]> {
143-
return this.model.getValuesForFieldInPolicy('g', ptype, 2).filter((domain) => domain !== undefined && domain !== '');
144-
}
145-
146121
/**
147122
* getPolicy gets all the authorization rules in the policy.
148123
*

src/rbac/defaultRoleManager.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,4 +352,43 @@ export class DefaultRoleManager implements RoleManager {
352352
});
353353
}
354354
}
355+
356+
/**
357+
* getDomains gets domains that a user has.
358+
*/
359+
public async getDomains(name: string): Promise<string[]> {
360+
const domains: string[] = [];
361+
this.allDomains.forEach((roles, domain) => {
362+
// Skip the default domain if there are other domains
363+
if (domain === DEFAULT_DOMAIN && this.allDomains.size > 1) {
364+
return;
365+
}
366+
const role = roles.get(name);
367+
if (role && (role.getRoles().length > 0 || this.hasUserOrRole(roles, name))) {
368+
domains.push(domain);
369+
}
370+
});
371+
return domains;
372+
}
373+
374+
/**
375+
* getAllDomains gets all domains.
376+
*/
377+
public async getAllDomains(): Promise<string[]> {
378+
const domains = Array.from(this.allDomains.keys());
379+
// Filter out the default domain if there are other domains
380+
if (domains.length > 1) {
381+
return domains.filter((d) => d !== DEFAULT_DOMAIN);
382+
}
383+
return domains;
384+
}
385+
386+
private hasUserOrRole(roles: Roles, name: string): boolean {
387+
for (const role of roles.values()) {
388+
if (role.hasDirectRole(name)) {
389+
return true;
390+
}
391+
}
392+
return false;
393+
}
355394
}

src/rbac/roleManager.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,8 @@ export interface RoleManager {
3636
getUsers(name: string, ...domain: string[]): Promise<string[]>;
3737
// PrintRoles prints all the roles to log.
3838
printRoles(): Promise<void>;
39+
// GetDomains gets domains that a user has
40+
getDomains(name: string): Promise<string[]>;
41+
// GetAllDomains gets all domains
42+
getAllDomains(): Promise<string[]>;
3943
}

test/managementAPI.test.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -75,20 +75,6 @@ test('getAllNamedRoles', async () => {
7575
testArrayEquals(allNamedRoles, []);
7676
});
7777

78-
test('getAllDomains', async () => {
79-
// The basic RBAC model doesn't have domains, so this should return empty
80-
const allDomains = await e.getAllDomains();
81-
testArrayEquals(allDomains, []);
82-
});
83-
84-
test('getAllNamedDomains', async () => {
85-
// The basic RBAC model doesn't have domains, so this should return empty
86-
let allNamedDomains = await e.getAllNamedDomains('g');
87-
testArrayEquals(allNamedDomains, []);
88-
allNamedDomains = await e.getAllNamedDomains('g1');
89-
testArrayEquals(allNamedDomains, []);
90-
});
91-
9278
test('getPolicy', async () => {
9379
const policy = await e.getPolicy();
9480
testArray2DEquals(policy, [

test/rbacwDomainAPI.test.ts

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,28 +30,25 @@ test('test getUsersForRoleInDomain', async () => {
3030
expect(await e.getUsersForRoleInDomain('superadmin', 'domain2')).toEqual([]);
3131
});
3232

33-
test('test getAllDomains', async () => {
34-
const e = await newEnforcer('examples/rbac_with_domains_model.conf', 'examples/rbac_with_domains_policy.csv');
35-
const domains = await e.getAllDomains();
36-
expect(domains.sort()).toEqual(['domain1', 'domain2']);
33+
test('test getDomainsForUser', async () => {
34+
const e = await newEnforcer('examples/rbac_with_domains_model.conf', 'examples/rbac_with_domains_policy2.csv');
35+
36+
let myRes = await e.getDomainsForUser('alice');
37+
myRes.sort();
38+
expect(myRes).toEqual(['domain1', 'domain2']);
39+
40+
myRes = await e.getDomainsForUser('bob');
41+
myRes.sort();
42+
expect(myRes).toEqual(['domain2', 'domain3']);
43+
44+
myRes = await e.getDomainsForUser('user');
45+
expect(myRes).toEqual(['domain3']);
3746
});
3847

39-
test('test getDomainsForUser', async () => {
48+
test('test getAllDomains', async () => {
4049
const e = await newEnforcer('examples/rbac_with_domains_model.conf', 'examples/rbac_with_domains_policy.csv');
41-
expect(await e.getDomainsForUser('alice')).toEqual(['domain1']);
42-
expect(await e.getDomainsForUser('bob')).toEqual(['domain2']);
43-
expect(await e.getDomainsForUser('nonexistent')).toEqual([]);
44-
45-
// Add alice to another domain and verify she appears in both
46-
await e.addGroupingPolicy('alice', 'admin', 'domain2');
47-
const aliceDomains = await e.getDomainsForUser('alice');
48-
expect(aliceDomains.sort()).toEqual(['domain1', 'domain2']);
49-
50-
// Test that empty string domains are filtered out
51-
await e.addGroupingPolicy('charlie', 'member', '');
52-
expect(await e.getDomainsForUser('charlie')).toEqual([]);
53-
54-
// Test that whitespace-only domains are filtered out
55-
await e.addGroupingPolicy('dave', 'member', ' ');
56-
expect(await e.getDomainsForUser('dave')).toEqual([]);
50+
51+
const myRes = await e.getAllDomains();
52+
myRes.sort();
53+
expect(myRes).toEqual(['domain1', 'domain2']);
5754
});

0 commit comments

Comments
 (0)