breken-ai opened a new pull request, #1765:
URL: https://github.com/apache/casbin/pull/1765
## Description
With a domain matching function (for example
`e.AddNamedDomainMatchingFunc("g", "KeyMatch", util.KeyMatch)`), removing one
grouping rule can revoke a role that another rule still grants. `Enforce` then
denies requests the policy allows, and keeps doing so until `LoadPolicy()` runs.
Using `examples/rbac_with_domain_pattern_*`:
```go
// policy already has: g, alice, admin, * and g, bob, admin, domain2
e.AddGroupingPolicy("alice", "admin", "domain1")
e.RemoveGroupingPolicy("alice", "admin", "domain1")
e.Enforce("alice", "domain1", "data1", "read") // false, but "g, alice,
admin, *" still grants it
e.AddGroupingPolicy("bob", "admin", "*")
e.RemoveGroupingPolicy("bob", "admin", "*")
e.Enforce("bob", "domain2", "data2", "read") // false, but "g, bob, admin,
domain2" is still there
e.LoadPolicy() // both are true again
```
Any code path that calls `DeleteLink` does this: `RemoveGroupingPolicy`,
`DeleteRoleForUserInDomain`, `UpdateGroupingPolicy` and
`RemoveFilteredGroupingPolicy`, as well as watcher updates on other nodes.
**Cause:** `DomainManager.getRoleManager` copies the links of every matching
pattern domain into a concrete domain's role manager, and `AddLink` adds a
pattern link to every domain that already exists. After that copy, a domain's
role manager can't tell its own links from the ones it inherited.
`DomainManager.DeleteLink` deleted the link from the domain itself and from
every domain the deleted pattern matches, without checking whether another link
still granted it there.
**Fix:** `DomainManager` now records each link with the domain it was added
in. `DeleteLink` removes that record first, then deletes the link from a domain
only when no remaining record grants it there: neither the domain itself nor a
pattern that matches it. `rebuild()` (run by `AddDomainMatchingFunc`) replays
these records. Before, it replayed the per-domain role managers, and those also
hold the copied pattern links, so each copied link would be recorded as the
domain's own. `Clear` resets the records, and `DeleteDomain` drops the records
for that domain.
`ConditionalDomainManager` has its own `AddLink`/`DeleteLink`/`rebuild`, so
this change doesn't touch it.
## Testing
- `TestRemoveGroupingPolicyWithDomainPattern`
(`rbac_api_with_domains_test.go`) runs the scenario above through the enforcer
and checks that `LoadPolicy()` gives the same answers.
- `TestDomainPatternDeleteLinkKeepsLinksStillGranted`
(`rbac/default-role-manager/role_manager_test.go`) covers both directions at
the role manager level, and checks that a link is gone once no rule grants it
any more.
- `TestDomainPatternRebuildUsesAddedLinks` guards the `rebuild()` change. A
`"*"` link copied into `domain1` must not become a `domain1` link of its own
when the matching function is set again.
Results:
- On `master` @ `524f3f2`: the first two tests fail (`alice, domain1, data1,
read: false, supposed to be true`; `bob, domain2, data2, read: false, supposed
to be true`).
- With the fix: all three pass.
- The full `go test ./...` suite passes, `go test -race ./rbac/... .`
passes, and `gofmt` and `golangci-lint run ./...` (v2, repo config) report 0
issues.
---
I found this bug with AI assistance (Claude, per the [ASF generative tooling
guidance](https://www.apache.org/legal/generative-tooling.html)) and used it to
write the fix and tests; I reviewed the change and ran the tests above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
