[ISSUE #11446] fix(tools): stop trimming the ACL password in mqadmin create/updateUser

[iloveeyjafjalla]

Which Issue(s) This PR Fixes

Fixes #11446

Problem

CreateUserSubCommand and UpdateUserSubCommand trimmed the ACL password before calling the admin API:

String password = StringUtils.trim(commandLine.getOptionValue('p'));

The broker stores the secret verbatim (AuthMigrator copies accessConfig.getSecretKey(); User.of(accessKey, secretKey, ...) in the authentication manager builds the credential from it), so a passphrase with leading or trailing whitespace is legal and is what a client must present. The CLI stored the trimmed value instead: the real secret is then rejected as unauthenticated, nothing in the CLI output or the broker log points at the difference, and the admin API stores the same secret verbatim, so the CLI and the API disagree about the same user. sendMessage carried the same shape for the message body until #11414.

Change

• Drop the trim on the password in both commands (the username, user type, user status and address keep theirs).
• Give both commands the createAdminExt(RPCHook) seam CopyUsersSubCommand already exposes, so the argument that reaches the admin API can be captured in a test.

Verification

New tests capture the password the mocked DefaultMQAdminExt receives. With the trim in place:

CreateUserSubCommandTest.testCreateKeepsThePasswordVerbatim:65 expected:<[  s3cret  ]> but was:<[s3cret]>
UpdateUserSubCommandTest.testUpdateKeepsThePasswordVerbatim:65 expected:<[ p@ss word ]> but was:<[p@ss word]>
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0   (on JDK 8)

After the change, in the tools module with JDK 8 (mvn -B -ntp -pl tools test -Dtest='CreateUserSubCommandTest,UpdateUserSubCommandTest,CopyUsersSubCommandTest' -DfailIfNoTests=false -Djacoco.skip=true):

Tests run: 6, Failures: 0, Errors: 0, Skipped: 0
checkstyle:3.2.0:check (validate): passed
spotbugs:4.8.6.8:check (check): BugInstance size is 0

The unit test is the verification for this one; it exercises the command class, not a live broker. The change was prepared with AI assistance and reviewed, tested and verified by me.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Fixes a bug where mqadmin createUser and updateUser silently trimmed the ACL password before storing it, causing authentication failures when the actual secret contained leading/trailing whitespace. The broker stores the secret verbatim, so the CLI must not alter it.

Findings

• [Info] CreateUserSubCommand.java:78 / UpdateUserSubCommand.java:83 — The createAdminExt(RPCHook) seam is package-private, which is appropriate for same-package test access. This is consistent with the existing pattern in CopyUsersSubCommand. ✓

• [Info] CreateUserSubCommand.java:80 — The comment clearly explains the rationale for removing the trim. Good documentation of the intent.

• [Info] Tests — Both test classes use ArgumentCaptor to verify the exact password value reaching the admin API, which directly validates the fix. The test data (" s3cret " and " p@ss word ") covers both leading/trailing spaces and internal spaces.

Observations

1. The test only exercises the -b (direct broker) code path. The -c (cluster) path shares the same password handling (trim removal happens before the branch), so coverage is adequate for this fix.

2. Other fields (username, userType, userStatus, address) retain their StringUtils.trim() calls, which is correct — those are identifiers, not secrets.

3. Minor backward note: Users who previously created accounts with space-padded passwords via the CLI (where spaces were silently stripped) will need to re-create those users with the correct password. This is the intended behavior change and is documented in the PR body.

Verdict

Clean, focused fix with good test coverage. The createAdminExt seam improves testability without changing production behavior beyond the intended bug fix.

---

Automated review by github-manager-bot

[codecov-commenter]

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.75%. Comparing base (5d68880) to head (3487d58).

Files with missing lines	Patch %	Lines
...ketmq/tools/command/auth/CreateUserSubCommand.java	66.66%	1 Missing ⚠️
...ketmq/tools/command/auth/UpdateUserSubCommand.java	66.66%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #11447      +/-   ##
=============================================
- Coverage      49.76%   49.75%   -0.02%     
+ Complexity     14377    14369       -8     
=============================================
  Files           1390     1390              
  Lines         103262   103264       +2     
  Branches       13504    13504              
=============================================
- Hits           51392    51380      -12     
+ Misses         45681    45679       -2     
- Partials        6189     6205      +16     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.