Skip to content

feat: Support multiple policy definitions (p, p2)#518

Merged
hsluoyz merged 9 commits intomasterfrom
copilot/fix-casbin-editor-policy-sets
Dec 8, 2025
Merged

feat: Support multiple policy definitions (p, p2)#518
hsluoyz merged 9 commits intomasterfrom
copilot/fix-casbin-editor-policy-sets

Conversation

Copy link
Contributor

Copilot AI commented Dec 8, 2025

Node-casbin failed to enforce policies when models contained multiple policy definitions (p, p2, p3, etc.). The enforcer only evaluated policies from the default type, causing p2/p3 references to fail.

Changes

Core enforcement logic (src/coreEnforcer.ts):

  • Added policy type reference check (matching Go Casbin): Only evaluates policies if matcher contains policy type tokens (e.g., p_, p2_)
  • Matches Go's implementation: strings.Contains(expString, pType+"_")
  • Uses single pType per enforce call via EnforceContext (does not iterate all policy types)

Go Casbin Compatibility:
Implementation now exactly matches Go Casbin's behavior:

  • Evaluates only the specific pType from EnforceContext (default 'p')
  • Separate matchers for different policy types (m for p, m2 for p2)
  • Uses EnforceContext to select which r/p/e/m combination to use
  • Verified against Go Casbin v2.134.0 implementation

Example Files Added (from Go Casbin):

  • multiple_policy_definitions_model.conf and .csv
  • rbac_with_multiple_policy_model.conf and .csv

How It Works (Same as Go):

// Model with separate matchers for different policy types
[policy_definition]
p = sub, obj, act
p2= sub_rule, obj, act, eft

[matchers]
m = g(r.sub, p.sub) && r.obj == p.obj && r.act == p.act
m2 = eval(p2.sub_rule) && r2.obj == p2.obj && r2.act == p2.act

// Use default context (r, p, e, m)
await enforcer.enforce('alice', 'data2', 'read');

// Use EnforceContext to select r2, p2, e, m2
const ctx = newEnforceContext('2');
ctx.eType = 'e';
await enforcer.enforce(ctx, { Age: 30 }, '/data1', 'read');

Tests: Added test matching Go's TestMultiplePolicyDefinitions and RBAC with multiple policy test. All 183 tests pass.

Original prompt

This section details on the original issue you should resolve

<issue_title>[feature] Policy definition contains 2 different policy sets (p and p2) doesn't work in Casbin editor</issue_title>
<issue_description>Want to prioritize this issue? Try:

issuehunt-to-marktext


Describe the bug
The case from the doc with to types of policies (p and p2) doesn't work on https://editor.casbin.org.

Link to the case: https://editor.casbin.org/#2YLC95Q57

To Reproduce
Establish following Model:

[request_definition]
r = sub, obj, act

[policy_definition]
p = sub, obj, act
p2 = sub, act

[policy_effect]
e = some(where (p.eft == allow))

[matchers]
m = r.sub == p.sub && r.obj == p.obj && r.act == p.act

and Policy:

p, alice, data1, read
p2, bob, write-all-objects

Requests:

alice, data1, read
bob, data1, write-all-objects

Expected behavior
all requests should return true, but 2nd request returns false

Screenshots
image
</issue_description>

Comments on the Issue (you are @copilot in this section)

@hsluoyz @taksedo the Casbin editor is based on Node.js version Casbin, which currently doesn't support `p2` yet. This Go Casbin supports it. @hsluoyz need to support `p2` like Golang Casbin

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@hsluoyz hsluoyz marked this pull request as ready for review December 8, 2025 10:15
Copilot AI and others added 5 commits December 8, 2025 10:19
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Modified coreEnforcer to iterate over all policy types when evaluating matchers, enabling support for models with multiple policy definitions like Go Casbin.

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Added tests for p/p2/p3 scenarios, enforceEx, backward compatibility, and the exact GitHub issue example.

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
- Add effectDone flag to properly terminate evaluation across policy types
- Use ptype for eft parameter naming (correct behavior for multi-policy)
- Add test for explicit eft column with multiple policy types

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Use Array.from() and Array.some() for better readability and performance.

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix issue with policy sets in Casbin editor Support multiple policy definitions (p, p2, p3) in enforcement Dec 8, 2025
Copilot AI requested a review from hsluoyz December 8, 2025 10:35
@hsluoyz
Copy link
Member

hsluoyz commented Dec 8, 2025

@copilot the newly added code and test case should refer to the implementation Golang Casbin's: https://github.com/casbin/casbin (aka use the same implementation)

Copy link
Contributor Author

Copilot AI commented Dec 8, 2025

@copilot the newly added code and test case should refer to the implementation Golang Casbin's: https://github.com/casbin/casbin (aka use the same implementation)

I've analyzed the Go Casbin implementation. There's a key difference in approach:

Go Casbin: Uses separate matchers for each policy type (m for p, m2 for p2). You select which to use via EnforceContext. A matcher like m = r.sub == p.sub && ... only evaluates 'p' policies.

My implementation: Allows a single matcher to reference multiple policy types (e.g., m = r.sub == p.sub && ... || r.sub == p2.sub && ...). This evaluates all policy types in one enforce call.

The original issue shows a matcher that ONLY references 'p', yet expects 'p2' policies to match. With Go's approach, this wouldn't work either - you'd need either:

  1. Separate matchers (m, m2) + multiple enforce calls
  2. A matcher referencing both (p and p2)

My implementation enables option 2 by default. Should I instead:

  • Match Go's behavior (only evaluate pType from EnforceContext)?
  • Keep current behavior (evaluate all policy types)?
  • Add the pType check (strings.Contains(expString, pType+"_")) like Go does?

Please clarify the expected behavior.

@hsluoyz
Copy link
Member

hsluoyz commented Dec 8, 2025

@copilot should exactly use the same behavior as Go Casbin, including code and test cases and naming patterns

Copilot AI and others added 2 commits December 8, 2025 11:09
Check if matcher contains policy type tokens (e.g., p_, p2_) before evaluating, consistent with Go Casbin implementation.

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Document the implementation details, comparison with Go Casbin, and usage guide.

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Copilot AI changed the title Support multiple policy definitions (p, p2, p3) in enforcement Support multiple policy definitions (p, p2, p3) in enforcement - aligned with Go Casbin Dec 8, 2025
- Reverted to single pType evaluation per enforce call
- Added pType reference check like Go: expString.includes(pType+"_")
- Added Go's example files: multiple_policy_definitions and rbac_with_multiple_policy
- Added test matching Go's TestMultiplePolicyDefinitions
- Removed custom implementation that iterated all policy types

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 8, 2025

@copilot should exactly use the same behavior as Go Casbin, including code and test cases and naming patterns

Done. Implementation now exactly matches Go Casbin:

Changes (commit b5327b0):

  • ✅ Added pType reference check: expString.includes(\${enforceContext.pType}_`)`
  • ✅ Single pType evaluation per enforce call (not iterating all types)
  • ✅ Added Go's example files: multiple_policy_definitions and rbac_with_multiple_policy
  • ✅ Added test matching Go's TestMultiplePolicyDefinitions
  • ✅ Test naming and structure matches Go

Behavior:
Uses EnforceContext to select which r/p/e/m combination, exactly like Go. Tests pass (183 total).

Copilot AI changed the title Support multiple policy definitions (p, p2, p3) in enforcement - aligned with Go Casbin Support multiple policy definitions (p, p2) - Go Casbin compatible Dec 8, 2025
@hsluoyz hsluoyz changed the title Support multiple policy definitions (p, p2) - Go Casbin compatible feat: Support multiple policy definitions (p, p2) Dec 8, 2025
@hsluoyz hsluoyz merged commit 287dc05 into master Dec 8, 2025
11 of 13 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 8, 2025
# [5.42.0](v5.41.0...v5.42.0) (2025-12-08)

### Features

* configure CI workflow to trigger on master branch ([#521](#521)) ([02ec045](02ec045))
* fix duplicate CI runs on pull requests ([#513](#513)) ([67ab8ee](67ab8ee))
* fix escapeAssertion to preserve string literals ([#514](#514)) ([7801732](7801732))
* Support multiple policy definitions (p, p2) ([#518](#518)) ([1b1f1bb](1b1f1bb))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feature] Policy definition contains 2 different policy sets (p and p2) doesn't work in Casbin editor

3 participants