yyqdbngt opened a new pull request, #4703:
URL: https://github.com/apache/rocketmq-dashboard/pull/4703
<!-- Base branch: `rocketmq-studio`, the RocketMQ Studio trunk. -->
### Brief Description
`MybatisPlusAclRepository.createAndUpdatePlainAccessConfig` guarded the
per-resource permission
entries (`topicPerms` / `groupPerms`, added in #2563) but bound the two
*default* permissions straight
into `rmq_acl_rule`:
```java
if (config.getDefaultTopicPerm() != null) {
rules.add(plainRule(config.getAccessKey(), "*", "Cluster",
config.getDefaultTopicPerm(), "DEFAULT_TOPIC"));
}
```
So `POST /api/acl/plain-access-config` with
`{"accessKey":"svc-x","secretKey":"...","defaultTopicPerm":""}`
(or `" "`) answered `200` and persisted a durable `DEFAULT_TOPIC` /
`DEFAULT_GROUP` rule whose
`actions` column is empty — an account-level default that grants nothing but
is stored as configured
(and exported as an empty `actions` entry), while the equivalent mistake in
`topicPerms`/
`groupPerms` is rejected with `400 topicPerms[0] must use non-blank
resource=permission format`.
Surrounding whitespace around a real value was stored verbatim too (`" DENY
"`), unlike the
per-resource entries, which are trimmed by `splitPerm`.
Both defaults now go through the same guard as their siblings:
- `MybatisPlusAclRepository.java:218-219` normalizes `defaultTopicPerm` /
`defaultGroupPerm` before
anything is read or written, so the request fails before the account or
its rules are touched;
- the new `normalizeDefaultPermission` (line 432) rejects a blank value with
`400 defaultTopicPerm must be a non-blank permission` and trims a real
one, mirroring
`validatePermissionEntries`;
- an absent default (`null`, i.e. the field omitted) keeps meaning "no
default permission" and still
writes no rule, and the write response now echoes the trimmed value, as it
already does for the
white remote address.
### How Did You Test This Change?
New tests in `MybatisPlusAclRepositoryTest`:
`upsertShouldRejectBlankDefaultTopicPermissionBeforeMutatingAccount`,
`upsertShouldRejectBlankDefaultGroupPermissionBeforeMutatingAccount` (each
asserts the `400` message
and `verifyNoInteractions(userMapper, ruleMapper)`) and
`upsertShouldTrimDefaultPermissionsBeforePersisting` (asserts the two
persisted rules carry
`DENY` / `PUB`).
Before the fix (red):
```
$ cd server && mvn -B -ntp test
-Dtest='MybatisPlusAclRepositoryTest#upsertShouldRejectBlankDefaultTopicPermissionBeforeMutatingAccount+upsertShouldRejectBlankDefaultGroupPermissionBeforeMutatingAccount+upsertShouldTrimDefaultPermissionsBeforePersisting'
[ERROR] Tests run: 3, Failures: 3, Errors: 0, Skipped: 0, Time elapsed:
2.079 s <<< FAILURE! -- in
org.apache.rocketmq.studio.instance.acl.MybatisPlusAclRepositoryTest
[ERROR]
MybatisPlusAclRepositoryTest.upsertShouldRejectBlankDefaultGroupPermissionBeforeMutatingAccount
-- Time elapsed: 1.925 s <<< FAILURE!
java.lang.AssertionError:
Expecting code to raise a throwable.
at
org.apache.rocketmq.studio.instance.acl.MybatisPlusAclRepositoryTest.upsertShouldRejectBlankDefaultGroupPermissionBeforeMutatingAccount(MybatisPlusAclRepositoryTest.java:106)
[ERROR]
MybatisPlusAclRepositoryTest.upsertShouldRejectBlankDefaultTopicPermissionBeforeMutatingAccount
-- Time elapsed: 0.009 s <<< FAILURE!
java.lang.AssertionError:
Expecting code to raise a throwable.
at
org.apache.rocketmq.studio.instance.acl.MybatisPlusAclRepositoryTest.upsertShouldRejectBlankDefaultTopicPermissionBeforeMutatingAccount(MybatisPlusAclRepositoryTest.java:90)
[ERROR]
MybatisPlusAclRepositoryTest.upsertShouldTrimDefaultPermissionsBeforePersisting
-- Time elapsed: 0.082 s <<< FAILURE!
org.opentest4j.AssertionFailedError:
Expecting actual:
at
org.apache.rocketmq.studio.instance.acl.MybatisPlusAclRepositoryTest.upsertShouldTrimDefaultPermissionsBeforePersisting(MybatisPlusAclRepositoryTest.java:133)
[ERROR] Tests run: 3, Failures: 3, Errors: 0, Skipped: 0
[INFO] BUILD FAILURE
```
After the fix (green) — the three ACL classes that exercise this path, so
the neighbouring
plain-access, rule and user tests are covered too:
```
$ cd server && mvn -B -ntp test
-Dtest='MybatisPlusAclRepositoryTest,AclServiceTest,AclControllerTest'
[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: 26, Failures: 0, Errors: 0, Skipped: 0 -- in
org.apache.rocketmq.studio.instance.acl.MybatisPlusAclRepositoryTest
[INFO] Tests run: 131, 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 payload javadoc in `UpsertPlainAccessConfigDTO` already describes the
defaults as optional, and a blank value was never a valid permission)
--
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]
