From 8cbcb646fe6fb41080991ec66aa45a8d63434090 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Dec 2025 14:00:50 +0000 Subject: [PATCH 1/5] Initial plan From bd992ed5c7d152511bcfbe9b29e530e570178a9d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Dec 2025 14:10:03 +0000 Subject: [PATCH 2/5] Add getAllDomains and getDomainsForUser methods with tests Co-authored-by: mserico <140243407+mserico@users.noreply.github.com> --- src/enforcer.ts | 25 +++++++++++++++++++++++++ src/managementEnforcer.ts | 25 +++++++++++++++++++++++++ test/managementAPI.test.ts | 14 ++++++++++++++ test/rbacwDomainAPI.test.ts | 18 ++++++++++++++++++ 4 files changed, 82 insertions(+) diff --git a/src/enforcer.ts b/src/enforcer.ts index 9ca947ce..244fa311 100644 --- a/src/enforcer.ts +++ b/src/enforcer.ts @@ -464,6 +464,31 @@ export class Enforcer extends ManagementEnforcer { return res.filter((n) => !inherits.some((m) => n === m)); } + + /** + * getDomainsForUser gets all domains that a user has. + * For example: + * g, alice, admin, domain1 + * g, alice, member, domain2 + * + * getDomainsForUser("alice") will get: ["domain1", "domain2"]. + * + * @param user the user. + * @return the domains that the user has. + */ + public async getDomainsForUser(user: string): Promise { + const groupingPolicies = await this.getFilteredGroupingPolicy(0, user); + const domains = new Set(); + + for (const policy of groupingPolicies) { + // In a policy like ["alice", "admin", "domain1"], domain is at index 2 + if (policy.length > 2 && policy[2]) { + domains.add(policy[2]); + } + } + + return Array.from(domains); + } } export async function newEnforcerWithClass(enforcer: new () => T, ...params: any[]): Promise { diff --git a/src/managementEnforcer.ts b/src/managementEnforcer.ts index 1393c0d1..9706e515 100644 --- a/src/managementEnforcer.ts +++ b/src/managementEnforcer.ts @@ -118,6 +118,31 @@ export class ManagementEnforcer extends InternalEnforcer { return this.model.getValuesForFieldInPolicy('g', ptype, 1); } + /** + * getAllDomains gets the list of domains that show up in the current policy. + * + * @return all the domains in "g" policy rules. It actually collects + * the 2-index elements of "g" policy rules. So make sure your + * domain is the 2-index element, like (sub, role, domain). + * Duplicates are removed. + */ + public async getAllDomains(): Promise { + return this.getAllNamedDomains('g'); + } + + /** + * getAllNamedDomains gets the list of domains that show up in the current named policy. + * + * @param ptype the policy type, can be "g", "g2", "g3", .. + * @return all the domains in policy rules of the ptype type. It actually + * collects the 2-index elements of the policy rules. So make + * sure your domain is the 2-index element, like (sub, role, domain). + * Duplicates are removed. + */ + public async getAllNamedDomains(ptype: string): Promise { + return this.model.getValuesForFieldInPolicy('g', ptype, 2).filter((domain) => domain !== undefined && domain !== ''); + } + /** * getPolicy gets all the authorization rules in the policy. * diff --git a/test/managementAPI.test.ts b/test/managementAPI.test.ts index 2afc92e6..1afaff4d 100644 --- a/test/managementAPI.test.ts +++ b/test/managementAPI.test.ts @@ -75,6 +75,20 @@ test('getAllNamedRoles', async () => { testArrayEquals(allNamedRoles, []); }); +test('getAllDomains', async () => { + // The basic RBAC model doesn't have domains, so this should return empty + const allDomains = await e.getAllDomains(); + testArrayEquals(allDomains, []); +}); + +test('getAllNamedDomains', async () => { + // The basic RBAC model doesn't have domains, so this should return empty + let allNamedDomains = await e.getAllNamedDomains('g'); + testArrayEquals(allNamedDomains, []); + allNamedDomains = await e.getAllNamedDomains('g1'); + testArrayEquals(allNamedDomains, []); +}); + test('getPolicy', async () => { const policy = await e.getPolicy(); testArray2DEquals(policy, [ diff --git a/test/rbacwDomainAPI.test.ts b/test/rbacwDomainAPI.test.ts index c8b8855e..57f6209c 100644 --- a/test/rbacwDomainAPI.test.ts +++ b/test/rbacwDomainAPI.test.ts @@ -29,3 +29,21 @@ test('test getUsersForRoleInDomain', async () => { expect(await e.getUsersForRoleInDomain('superadmin', 'domain1')).toEqual([]); expect(await e.getUsersForRoleInDomain('superadmin', 'domain2')).toEqual([]); }); + +test('test getAllDomains', async () => { + const e = await newEnforcer('examples/rbac_with_domains_model.conf', 'examples/rbac_with_domains_policy.csv'); + const domains = await e.getAllDomains(); + expect(domains.sort()).toEqual(['domain1', 'domain2']); +}); + +test('test getDomainsForUser', async () => { + const e = await newEnforcer('examples/rbac_with_domains_model.conf', 'examples/rbac_with_domains_policy.csv'); + expect(await e.getDomainsForUser('alice')).toEqual(['domain1']); + expect(await e.getDomainsForUser('bob')).toEqual(['domain2']); + expect(await e.getDomainsForUser('nonexistent')).toEqual([]); + + // Add alice to another domain and verify she appears in both + await e.addGroupingPolicy('alice', 'admin', 'domain2'); + const aliceDomains = await e.getDomainsForUser('alice'); + expect(aliceDomains.sort()).toEqual(['domain1', 'domain2']); +}); From 37a8c5f1d25272c37f46777e5f82fa6552d82b2c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Dec 2025 14:12:23 +0000 Subject: [PATCH 3/5] Address code review: improve domain filtering to handle edge cases Co-authored-by: mserico <140243407+mserico@users.noreply.github.com> --- src/enforcer.ts | 2 +- test/rbacwDomainAPI.test.ts | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/enforcer.ts b/src/enforcer.ts index 244fa311..00208a3f 100644 --- a/src/enforcer.ts +++ b/src/enforcer.ts @@ -482,7 +482,7 @@ export class Enforcer extends ManagementEnforcer { for (const policy of groupingPolicies) { // In a policy like ["alice", "admin", "domain1"], domain is at index 2 - if (policy.length > 2 && policy[2]) { + if (policy.length > 2 && policy[2] && policy[2].trim() !== '') { domains.add(policy[2]); } } diff --git a/test/rbacwDomainAPI.test.ts b/test/rbacwDomainAPI.test.ts index 57f6209c..a29f9c90 100644 --- a/test/rbacwDomainAPI.test.ts +++ b/test/rbacwDomainAPI.test.ts @@ -46,4 +46,12 @@ test('test getDomainsForUser', async () => { await e.addGroupingPolicy('alice', 'admin', 'domain2'); const aliceDomains = await e.getDomainsForUser('alice'); expect(aliceDomains.sort()).toEqual(['domain1', 'domain2']); + + // Test that empty string domains are filtered out + await e.addGroupingPolicy('charlie', 'member', ''); + expect(await e.getDomainsForUser('charlie')).toEqual([]); + + // Test that whitespace-only domains are filtered out + await e.addGroupingPolicy('dave', 'member', ' '); + expect(await e.getDomainsForUser('dave')).toEqual([]); }); From 93c23fa2ede926a828ee8ff33d48523697cf4827 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Dec 2025 15:45:16 +0000 Subject: [PATCH 4/5] 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> --- examples/rbac_with_domains_policy2.csv | 9 ++++++ src/enforcer.ts | 35 +++++++++++------------ src/managementEnforcer.ts | 25 ----------------- src/rbac/defaultRoleManager.ts | 39 ++++++++++++++++++++++++++ src/rbac/roleManager.ts | 4 +++ test/managementAPI.test.ts | 14 --------- test/rbacwDomainAPI.test.ts | 39 ++++++++++++-------------- 7 files changed, 87 insertions(+), 78 deletions(-) create mode 100644 examples/rbac_with_domains_policy2.csv diff --git a/examples/rbac_with_domains_policy2.csv b/examples/rbac_with_domains_policy2.csv new file mode 100644 index 00000000..baa06f06 --- /dev/null +++ b/examples/rbac_with_domains_policy2.csv @@ -0,0 +1,9 @@ +p, admin, domain1, data1, read +p, admin, domain1, data1, write +p, admin, domain2, data2, read +p, admin, domain2, data2, write +p, user, domain3, data2, read +g, alice, admin, domain1 +g, alice, admin, domain2 +g, bob, admin, domain2 +g, bob, user, domain3 diff --git a/src/enforcer.ts b/src/enforcer.ts index 00208a3f..28608dcf 100644 --- a/src/enforcer.ts +++ b/src/enforcer.ts @@ -466,28 +466,27 @@ export class Enforcer extends ManagementEnforcer { } /** - * getDomainsForUser gets all domains that a user has. - * For example: - * g, alice, admin, domain1 - * g, alice, member, domain2 - * - * getDomainsForUser("alice") will get: ["domain1", "domain2"]. - * - * @param user the user. - * @return the domains that the user has. + * getDomainsForUser gets all domains. */ public async getDomainsForUser(user: string): Promise { - const groupingPolicies = await this.getFilteredGroupingPolicy(0, user); - const domains = new Set(); - - for (const policy of groupingPolicies) { - // In a policy like ["alice", "admin", "domain1"], domain is at index 2 - if (policy.length > 2 && policy[2] && policy[2].trim() !== '') { - domains.add(policy[2]); - } + const domains: string[] = []; + for (const rm of this.rmMap.values()) { + const domain = await rm.getDomains(user); + domains.push(...domain); } + return domains; + } - return Array.from(domains); + /** + * getAllDomains gets all domains. + */ + public async getAllDomains(): Promise { + const domains: string[] = []; + for (const rm of this.rmMap.values()) { + const domain = await rm.getAllDomains(); + domains.push(...domain); + } + return arrayRemoveDuplicates(domains); } } diff --git a/src/managementEnforcer.ts b/src/managementEnforcer.ts index 9706e515..1393c0d1 100644 --- a/src/managementEnforcer.ts +++ b/src/managementEnforcer.ts @@ -118,31 +118,6 @@ export class ManagementEnforcer extends InternalEnforcer { return this.model.getValuesForFieldInPolicy('g', ptype, 1); } - /** - * getAllDomains gets the list of domains that show up in the current policy. - * - * @return all the domains in "g" policy rules. It actually collects - * the 2-index elements of "g" policy rules. So make sure your - * domain is the 2-index element, like (sub, role, domain). - * Duplicates are removed. - */ - public async getAllDomains(): Promise { - return this.getAllNamedDomains('g'); - } - - /** - * getAllNamedDomains gets the list of domains that show up in the current named policy. - * - * @param ptype the policy type, can be "g", "g2", "g3", .. - * @return all the domains in policy rules of the ptype type. It actually - * collects the 2-index elements of the policy rules. So make - * sure your domain is the 2-index element, like (sub, role, domain). - * Duplicates are removed. - */ - public async getAllNamedDomains(ptype: string): Promise { - return this.model.getValuesForFieldInPolicy('g', ptype, 2).filter((domain) => domain !== undefined && domain !== ''); - } - /** * getPolicy gets all the authorization rules in the policy. * diff --git a/src/rbac/defaultRoleManager.ts b/src/rbac/defaultRoleManager.ts index 9d8f39b6..2fd863e9 100644 --- a/src/rbac/defaultRoleManager.ts +++ b/src/rbac/defaultRoleManager.ts @@ -352,4 +352,43 @@ export class DefaultRoleManager implements RoleManager { }); } } + + /** + * getDomains gets domains that a user has. + */ + public async getDomains(name: string): Promise { + const domains: string[] = []; + this.allDomains.forEach((roles, domain) => { + // Skip the default domain if there are other domains + if (domain === DEFAULT_DOMAIN && this.allDomains.size > 1) { + return; + } + const role = roles.get(name); + if (role && (role.getRoles().length > 0 || this.hasUserOrRole(roles, name))) { + domains.push(domain); + } + }); + return domains; + } + + /** + * getAllDomains gets all domains. + */ + public async getAllDomains(): Promise { + const domains = Array.from(this.allDomains.keys()); + // Filter out the default domain if there are other domains + if (domains.length > 1) { + return domains.filter((d) => d !== DEFAULT_DOMAIN); + } + return domains; + } + + private hasUserOrRole(roles: Roles, name: string): boolean { + for (const role of roles.values()) { + if (role.hasDirectRole(name)) { + return true; + } + } + return false; + } } diff --git a/src/rbac/roleManager.ts b/src/rbac/roleManager.ts index 54d99c38..4c3afb7e 100644 --- a/src/rbac/roleManager.ts +++ b/src/rbac/roleManager.ts @@ -36,4 +36,8 @@ export interface RoleManager { getUsers(name: string, ...domain: string[]): Promise; // PrintRoles prints all the roles to log. printRoles(): Promise; + // GetDomains gets domains that a user has + getDomains(name: string): Promise; + // GetAllDomains gets all domains + getAllDomains(): Promise; } diff --git a/test/managementAPI.test.ts b/test/managementAPI.test.ts index 1afaff4d..2afc92e6 100644 --- a/test/managementAPI.test.ts +++ b/test/managementAPI.test.ts @@ -75,20 +75,6 @@ test('getAllNamedRoles', async () => { testArrayEquals(allNamedRoles, []); }); -test('getAllDomains', async () => { - // The basic RBAC model doesn't have domains, so this should return empty - const allDomains = await e.getAllDomains(); - testArrayEquals(allDomains, []); -}); - -test('getAllNamedDomains', async () => { - // The basic RBAC model doesn't have domains, so this should return empty - let allNamedDomains = await e.getAllNamedDomains('g'); - testArrayEquals(allNamedDomains, []); - allNamedDomains = await e.getAllNamedDomains('g1'); - testArrayEquals(allNamedDomains, []); -}); - test('getPolicy', async () => { const policy = await e.getPolicy(); testArray2DEquals(policy, [ diff --git a/test/rbacwDomainAPI.test.ts b/test/rbacwDomainAPI.test.ts index a29f9c90..ea29bd81 100644 --- a/test/rbacwDomainAPI.test.ts +++ b/test/rbacwDomainAPI.test.ts @@ -30,28 +30,25 @@ test('test getUsersForRoleInDomain', async () => { expect(await e.getUsersForRoleInDomain('superadmin', 'domain2')).toEqual([]); }); -test('test getAllDomains', async () => { - const e = await newEnforcer('examples/rbac_with_domains_model.conf', 'examples/rbac_with_domains_policy.csv'); - const domains = await e.getAllDomains(); - expect(domains.sort()).toEqual(['domain1', 'domain2']); +test('test getDomainsForUser', async () => { + const e = await newEnforcer('examples/rbac_with_domains_model.conf', 'examples/rbac_with_domains_policy2.csv'); + + let myRes = await e.getDomainsForUser('alice'); + myRes.sort(); + expect(myRes).toEqual(['domain1', 'domain2']); + + myRes = await e.getDomainsForUser('bob'); + myRes.sort(); + expect(myRes).toEqual(['domain2', 'domain3']); + + myRes = await e.getDomainsForUser('user'); + expect(myRes).toEqual(['domain3']); }); -test('test getDomainsForUser', async () => { +test('test getAllDomains', async () => { const e = await newEnforcer('examples/rbac_with_domains_model.conf', 'examples/rbac_with_domains_policy.csv'); - expect(await e.getDomainsForUser('alice')).toEqual(['domain1']); - expect(await e.getDomainsForUser('bob')).toEqual(['domain2']); - expect(await e.getDomainsForUser('nonexistent')).toEqual([]); - - // Add alice to another domain and verify she appears in both - await e.addGroupingPolicy('alice', 'admin', 'domain2'); - const aliceDomains = await e.getDomainsForUser('alice'); - expect(aliceDomains.sort()).toEqual(['domain1', 'domain2']); - - // Test that empty string domains are filtered out - await e.addGroupingPolicy('charlie', 'member', ''); - expect(await e.getDomainsForUser('charlie')).toEqual([]); - - // Test that whitespace-only domains are filtered out - await e.addGroupingPolicy('dave', 'member', ' '); - expect(await e.getDomainsForUser('dave')).toEqual([]); + + const myRes = await e.getAllDomains(); + myRes.sort(); + expect(myRes).toEqual(['domain1', 'domain2']); }); From 2ba42d120f847afc008849fb41cb3ad23b75bbfc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Dec 2025 15:47:55 +0000 Subject: [PATCH 5/5] Fix comments and improve logic to match Go Casbin exactly - Update getDomainsForUser comment to be more descriptive - Improve getDomains logic to check both roles and users - Rename helper method for clarity (hasUserForRole) - All tests pass Co-authored-by: mserico <140243407+mserico@users.noreply.github.com> --- src/enforcer.ts | 2 +- src/rbac/defaultRoleManager.ts | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/enforcer.ts b/src/enforcer.ts index 28608dcf..f784c138 100644 --- a/src/enforcer.ts +++ b/src/enforcer.ts @@ -466,7 +466,7 @@ export class Enforcer extends ManagementEnforcer { } /** - * getDomainsForUser gets all domains. + * getDomainsForUser gets all domains that a user has. */ public async getDomainsForUser(user: string): Promise { const domains: string[] = []; diff --git a/src/rbac/defaultRoleManager.ts b/src/rbac/defaultRoleManager.ts index 2fd863e9..5271f83a 100644 --- a/src/rbac/defaultRoleManager.ts +++ b/src/rbac/defaultRoleManager.ts @@ -364,8 +364,13 @@ export class DefaultRoleManager implements RoleManager { return; } const role = roles.get(name); - if (role && (role.getRoles().length > 0 || this.hasUserOrRole(roles, name))) { - domains.push(domain); + if (role) { + // Check if role has any roles it inherits OR if any other role inherits from it + const hasRoles = role.getRoles().length > 0; + const hasUsers = this.hasUserForRole(roles, name); + if (hasRoles || hasUsers) { + domains.push(domain); + } } }); return domains; @@ -383,7 +388,7 @@ export class DefaultRoleManager implements RoleManager { return domains; } - private hasUserOrRole(roles: Roles, name: string): boolean { + private hasUserForRole(roles: Roles, name: string): boolean { for (const role of roles.values()) { if (role.hasDirectRole(name)) { return true;