Euphonium-1 opened a new pull request, #4673:
URL: https://github.com/apache/rocketmq-dashboard/pull/4673
## What
Both resources audit every mutation, and neither audited the one read that
hands the caller a plaintext secret:
| Resource | Audited | Not audited |
| --- | --- | --- |
| Cloud credential | `CREATE_` / `UPDATE_` / `DELETE_CLOUD_CREDENTIAL` |
`reveal` |
| ACL user | `CREATE_` / `UPDATE_` / `DELETE_ACL_USER`, `CREATE_` /
`DELETE_ACL_RULE`, `UPSERT_PLAIN_ACCESS_CONFIG` | `getUserCredentials` |
`GET /api/cloud-credentials/{id}/credentials` and `GET
/api/acl/users/{id}/credentials` are the complete set of secret-returning
endpoints in the server module (a repository-wide search for mappings
containing `credential|secret|reveal|password` returns only these two plus the
password-change endpoints).
## Why it matters
Both endpoints are admin-only - `AuthInterceptor.isAdminOnlyGetPath` covers
the `/api/acl/users/` and `/api/cloud-credentials/` prefixes - so this is not a
privilege gap. It is the loss of the only durable record that a disclosure
happened: `OperationAuditService.record` stamps
`AuthenticatedUserContext.currentUsernameOrSystem()` onto the row, and with no
row a credential read was indistinguishable from no activity at all.
## The change
- `CloudCredentialService.reveal` records `REVEAL_CLOUD_CREDENTIAL`, reusing
the existing `credentialAuditDetail` helper, so the detail names the credential
and vendor but never the secret.
- `AclService.getUserCredentials` records `REVEAL_ACL_USER_CREDENTIALS`,
with the detail carrying only `instanceId`.
`getUserCredentials` is restructured from two early returns into a single
exit so the audit call covers both the Apache repository path and the Tencent
vendor path - the Tencent branch returned directly and would otherwise have
missed it. `recordAudit` keeps swallowing sink failures with a warning, so an
audit outage still cannot fail an operation that already succeeded.
## Tests
- `revealShouldAuditTheSecretReadWithoutRecordingTheSecretTest` - asserts
the audit call and that the detail contains the credential name but not the
secret.
- `getUserCredentialsShouldAuditTheRevealedSecretWithoutRecordingIt` -
Apache path; asserts neither the secret key nor the access key reaches the
audit detail.
- `getUserCredentialsShouldAuditTheSecretRevealedThroughTheTencentPath` -
the vendor path, which the restructure was needed for.
## ⚠️ Verification disclosure
**I could not build or run this in the environment where it was written** -
no Maven or wrapper, no populated `~/.m2`, and only JDK 17 against a Java 21
target. Signatures used by the new tests
(`TencentAclService.getUserCredentials(String, String) -> AclUserVO`,
`BusinessException(int, String)`, the 7-arg `OperationAuditService.record`,
Lombok `@Data` getters on `AclUserVO`) were each confirmed by reading the
declarations, and the tests follow the conventions of the files they sit in -
but nothing has been compiled or executed, so please let CI confirm before
review. Edits were checked by inspection against
`server/style/rmq_checkstyle.xml`.
--
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]
