iloveeyjafjalla opened a new pull request, #6861:
URL: https://github.com/apache/rocketmq-dashboard/pull/6861
Fixes #6860
## The defect
`pushAclToBroker` read each account's rules with
`findRulePage(principalOf(user.getUsername())…)`, i.e. under a `user:<name>`
key that no write path stores, through a filter that matches principals as
substrings. Every account whose rules exist therefore arrived at the broker
with an empty plan, `upsertAccount` returned before `updateAcl`, and the push
reported success with `policyCount=0`.
Evidence that the prefixed key exists nowhere else in the product: `"user:"`
occurs only in `AclService` (the lookup and its private helper), the console's
rule form stores `u.username` (`web/src/pages/instance/acl.tsx:1598-1601`), the
plain-access upsert stores the access key, and the AI tool stores its input.
`User:<name>` is the broker-side ACL 2.0 subject the push *creates*, not the
store's key.
## The fix
- `AclRepository.findRulesByPrincipal(principal, limit)` — an equality
lookup (`eq("principal", …)`), the same shape `MybatisPlusAclRepository`
already uses for the plain-access identity paths (`:345`, `:398`), returning
the newest rows up to the cap.
- `pushAclToBroker` reads the account's rules through it; `principalOf` and
the comment claiming the store uses `user:` are gone.
- The three per-account test stubs that fed the `user:`-prefixed key were
encoding the wrong assumption and now stub the key the store actually writes.
The substring `findRulePage` stays as it is for the console's search box, so
an account's push no longer depends on search semantics — and cannot over-grant
to a principal that merely contains its name.
## Tests
Red before the fix:
```
[ERROR]
AclServiceTest.pushAclToBrokerPushesTheRulesStoredUnderTheAccountNameTest:1242
expected: 1
but was: 0
```
The test stores one account (`svc-a`) with one rule whose principal is
`svc-a`, pushes, and asserts `policyCount == 1` plus
`verify(admin).updateAcl(...)`; the legacy prefixed lookup is stubbed leniently
so the same test documents what the store answers for it (nothing).
Green after, plus the repository-level pin that the lookup is not a `LIKE`:
```
[INFO] Tests run: 84, Failures: 0, Errors: 0 -- AclServiceTest
[INFO] Tests run: 28, Failures: 0, Errors: 0 -- MybatisPlusAclRepositoryTest
[INFO] Tests run: 27, Failures: 0, Errors: 0 -- AclControllerTest
[INFO] You have 0 Checkstyle violations.
```
## Note
This is a separate seam of the same method as #6851 (which scopes the push
to the instance's cluster): different defect, different lines, and the two do
not overlap.
--
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]
