yyqdbngt opened a new pull request, #4720:
URL: https://github.com/apache/rocketmq-dashboard/pull/4720
<!-- Base branch: `rocketmq-studio`, the RocketMQ Studio trunk. -->
### Related issue
No open issue covers this; found while reading the plain-access read-back
path for the campaign.
### Brief Description
`MybatisPlusAclRepository.toPlainAccessConfig` (line 366) renders every
`Topic`/`Group` rule of an
account as `resource + "=" + actions` without checking that the rule still
has an action list:
```java
String actions = joinNormalizedCsv(rule.getActions());
if ("Topic".equals(rule.getResourceType())) {
topicPerms.add(rule.getResource() + "=" + actions);
```
`joinNormalizedCsv` returns `null` for a null or blank action list, so such
a rule is reported as the
literal entry `"orders=null"`. That leaks into `GET
/api/acl/cluster-config?clusterId=...` (the
`accounts[].topicPerms` / `accounts[].groupPerms` the ACL page renders) and
it round-trips badly:
`validatePermissionEntries` only rejects a *blank* right-hand side, so
copying that read-back value
back through `POST /api/acl/plain-access-config` persists `null` as the
actual permission
(`plainRule(..., "null", ...)`).
A rule can lose its action list in normal use: `AclService.updateRule` ->
`replaceRule` clears the
`actions` column explicitly (the `clearColumn` path), and `POST
/api/acl/rules/create` accepts a
payload whose `actions` is omitted (only `principal` and `resource` are
required). The same loop
already treats an absent action list as "not configured" for `DEFAULT_TOPIC`
/ `DEFAULT_GROUP`, so
the per-resource branches now do the same and skip an entry that carries no
permission.
Disclosure: `MybatisPlusAclRepository.java` is also touched by #4669 (LIKE
escaping in `ruleQuery`)
and #4703 (default-permission validation in
`createAndUpdatePlainAccessConfig`); this is a different
defect in a different method (`toPlainAccessConfig`), and the two hunks do
not overlap.
### How Did You Test This Change?
New tests in `MybatisPlusAclRepositoryTest`:
`examineShouldSkipTopicPermissionsWhoseActionListIsEmpty` and
`examineShouldSkipGroupPermissionsWhoseActionListIsBlank`. Each seeds one
rule without a usable
action list plus one granted rule and asserts the fabricated entry is gone.
Before the fix (red):
```
$ cd server && mvn -B -ntp test
-Dtest='MybatisPlusAclRepositoryTest#examineShouldSkipTopicPermissionsWhoseActionListIsEmpty+examineShouldSkipGroupPermissionsWhoseActionListIsBlank'
[ERROR] Tests run: 2, Failures: 2, Errors: 0, Skipped: 0, Time elapsed:
2.614 s <<< FAILURE! -- in
org.apache.rocketmq.studio.instance.acl.MybatisPlusAclRepositoryTest
org.apache.rocketmq.studio.instance.acl.MybatisPlusAclRepositoryTest.examineShouldSkipGroupPermissionsWhoseActionListIsBlank
-- Time elapsed: 2.520 s <<< FAILURE!
org.opentest4j.AssertionFailedError:
Expecting actual:
["cg-order=null", "cg-payment=SUB"]
to contain exactly (and in same order):
["cg-payment=SUB"]
but some elements were not expected:
["cg-order=null"]
at
org.apache.rocketmq.studio.instance.acl.MybatisPlusAclRepositoryTest.examineShouldSkipGroupPermissionsWhoseActionListIsBlank(MybatisPlusAclRepositoryTest.java:534)
org.apache.rocketmq.studio.instance.acl.MybatisPlusAclRepositoryTest.examineShouldSkipTopicPermissionsWhoseActionListIsEmpty
-- Time elapsed: 0.025 s <<< FAILURE!
org.opentest4j.AssertionFailedError:
Expecting actual:
["orders=null", "payments=SUB"]
to contain exactly (and in same order):
["payments=SUB"]
but some elements were not expected:
["orders=null"]
at
org.apache.rocketmq.studio.instance.acl.MybatisPlusAclRepositoryTest.examineShouldSkipTopicPermissionsWhoseActionListIsEmpty(MybatisPlusAclRepositoryTest.java:520)
[ERROR] Tests run: 2, Failures: 2, Errors: 0, Skipped: 0
[INFO] BUILD FAILURE
```
After the fix (green) — every ACL test class, so the neighbouring rule, user
and plain-access tests
are covered too:
```
$ cd server && mvn -B -ntp test
-Dtest='MybatisPlusAclRepositoryTest,AclServiceTest,AclControllerTest,ApacheAclReadServiceTest'
[INFO] Tests run: 27, Failures: 0, Errors: 0, Skipped: 0 -- in
org.apache.rocketmq.studio.instance.acl.AclControllerTest
[INFO] Tests run: 78, Failures: 0, Errors: 0, Skipped: 0 -- in
org.apache.rocketmq.studio.instance.acl.AclServiceTest
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0 -- in
org.apache.rocketmq.studio.instance.acl.ApacheAclReadServiceTest
[INFO] Tests run: 25, Failures: 0, Errors: 0, Skipped: 0 -- in
org.apache.rocketmq.studio.instance.acl.MybatisPlusAclRepositoryTest
[INFO] Tests run: 133, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
`mvn test` runs checkstyle in the `validate` phase: `You have 0 Checkstyle
violations.`
Note on the full suite: on a clean `rocketmq-studio` checkout `mvn -B -ntp
test` already reports
`Tests run: 3051, Failures: 6, Errors: 25, Skipped: 4`. The 11 red classes
are the MySQL 8 backed
Spring integration tests (`AuthServiceBootstrapIntegrationTest`,
`AuthServiceConcurrencyIntegrationTest`,
`AuthServiceSessionOverviewIntegrationTest`,
`HealthProbeIntegrationTest`, `QueryHistoryServiceIntegrationTest`,
`NativeAlertEvaluationTransactionTest`,
`NotificationOutboxMapperIntegrationTest`,
`RmqAlertStateMapperIntegrationTest`, `StudioApplicationTest`) plus the
external-CLI ones
(`CliAgentProviderTest`, `ClaudeCodeAgentProviderTest`). None of them are
touched by this change.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix:`)
- [x] Tests added or updated for non-trivial changes, test methods named
`...Test`
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
(no UI text in this change)
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [x] New source files carry the ASF license header (no new files)
- [x] Documentation touched where behaviour changed (no doc change needed:
the endpoint already documents the accounts view as stored
`resource=permission` entries)
--
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]
