feat(admin): rc admin account and user two-factor commands

[sineld]

Summary

Adds CLI coverage for the self-service account and two-factor surface introduced in rustfs/rustfs#6596.

rc admin account info <alias>
rc admin account passwd <alias> [secret sources]
rc admin account mfa status|enroll|activate|disable|recovery-codes <alias> [...]

rc admin user passwd <alias> <access-key> [secret sources]
rc admin user mfa status|reset <alias> <access-key>

account acts on the identity the alias authenticates as; user acts on another identity. The split mirrors the server, where /account/* never takes a target and the user endpoints always do, so a command cannot act on the wrong account.

Merge order

rustfs/rustfs#6596 has merged, so the endpoints this PR calls now exist on
main. This PR and rustfs/console#208 are independent of each other and can land
in either order.

Design notes for reviewers

Two-factor authentication does not gate rc, on purpose. rc signs every request with the alias access key, and that path is not gated. Gating it would break every script the moment a human enabled the second factor on their own account, and would add no protection — whoever holds the secret key already has full access without presenting a code. The second factor guards session minting (AssumeRole), which rc does not use. Documented in docs/reference/rc/admin.md so operators don't read the absence as an oversight.

No secret on the command line. Passwords come from --*-from-env, --*-file, or a prompt with echo off — never an argument, where they would land in shell history and ps output. Verification codes additionally accept --code because a TOTP code is valid for at most 90 seconds; --code and --code-from-env are mutually exclusive.

Nothing blocks on input it cannot receive. In --json mode, or when stdin is not a terminal, a command that would need to prompt exits with USAGE_ERROR naming the flag to pass instead.

No QR encoder here. The server renders the code and rc prints the Unicode block art it returns, so the console and the CLI show the same symbol from one implementation. Skipped below 33 columns, where a wrapped symbol cannot be scanned; --no-qr prints only the setup key and URI. --json omits the QR entirely.

Recovery codes. --output-file writes with mode 0600 and refuses to overwrite an existing file, which may hold the only copy of a previous set. SecretValue zeroes every password and code on drop and cannot be printed into a log or a panic message.

user mfa reset is break-glass for a user who lost both their authenticator and their recovery codes. It names the target and requires --yes when scripted.

Testing

cargo fmt --all --check                                  # clean
cargo clippy --workspace --all-targets -- -D warnings    # clean
cargo test --workspace                                   # 60 suites, 2376 tests, 0 failed

New: crates/cli/tests/admin_account.rs gives each new command at least two
exit-code scenarios, per AGENTS.md. Two of them pin specific failures found in
review rather than only an exit status — that a 40-byte secret reaches the server
whole, and that an occupied recovery-code output path is refused before any
request leaves. Plus unit tests for the parameterized file reader, the shared
confirmation prompt and the private-file writer.

Contracts

• schemas/output_v3.json — unchanged. The new commands reuse the existing envelopes, so this is not a breaking output change.
• crates/cli/src/exit_code.rs — unchanged.
• crates/core/src/config.rs — unchanged; no schema_version bump needed.
• docs/reference/rc/admin.md — updated.

docs/reference/rc/ is a protected path, so this carries the BREAKING marker
that AGENTS.md § Breaking Change Process step 3 requires for any change to the
CLI behaviour contract. To be clear about what that marker does and does not mean
here: no released behaviour changes. The reference edits document commands
this PR introduces, and clarify how --*-file reads a secret — a flag that also
arrives in this PR. Steps 1 and 2 of that process are satisfied without work:
there is no schema_version to bump and no new output schema, and the command
reference sections are exactly what was updated. The SSE-C key reader shares code
with the new secret reader and its behaviour, including its error strings, is
deliberately left byte-identical; tests/sse_customer.rs holds it there.

If you would rather this marker not ride along on an additive PR, the check in
ci.yml fires on any diff under docs/reference/rc/, including a pure addition —
happy to follow whatever you prefer instead.

Additional Notes

rc admin account passwd prints a reminder to update the alias with rc alias set afterwards, since rotating the secret invalidates the stored one. Automatically rewriting the alias felt like the wrong call — the command would then mutate local config as a side effect of a remote operation — but I'll change it if you'd prefer that.

[mertefecerit]

+1

[trueWD]

+1

[overtrue]

Took this for a proper spin — and since rustfs/rustfs#6596 is merged now, I checked the wire contract against the actual server handlers instead of guessing. The design writeup matches the code, and the account/user split is clean. That said, I found two things I'd consider blockers, plus a handful of smaller ones.

Blockers

1. --*-file silently truncates secrets to 33 bytes.
SecretSource::load goes through read_protected_key_file, and read_exact_key does reader.take(33) (secret_input.rs:131) — that bound is SSE-C-specific. Feed --password-file a 40-char AWS-style secret and the server receives the first 33 bytes with no error anywhere; the operator has now set a password they don't actually know. The same reuse also drags in the symlink rejection (Kubernetes secret mounts are symlinks) and error strings that tell someone changing a password about an "SSE-C key file". I think the file reader needs to be parameterized (length bound + error wording), not reused as-is.

2. Recovery codes can be lost for good.
emit_recovery_codes writes to --output-file after the server has already activated/rotated, and write_recovery_codes uses create_new. So if the path exists — say, left over from a previous attempt, which is exactly the case the create_new comment worries about — the command exits nonzero and the one-time set is dropped unprinted. For recovery-codes the previous set was just invalidated too, so the user ends up with no working codes at all. Printing the codes as a fallback when the write fails (loudly), or checking the path before making the API call, would close this.

Should fix

• account.rs:351 — when mutable.password is false, the explanation printed is info.mfa.enrollment_blocked_reason. That's the MFA field: for the env-root case it's null so the user gets a bare refusal (the docs promise a message naming the env var), and when it is set, an MFA restriction gets attributed to the password.
• print_enrollment prints "Scan this QR code with your authenticator app:" unconditionally and discards print_qr's return value — with --no-qr, a narrow terminal, or an empty payload, the user is told to scan something that was never printed. The bool exists for exactly this.
• Server-controlled strings go to the TTY through plain println without sanitize_text — qr_utf8 especially (print_qr writes it raw, line by line), but also otpauth_uri, timestamps, blocked reasons, and the recovery codes themselves. Everywhere else in the tree escapes server text before printing; a hostile endpoint could repaint the setup-key line with a substituted secret.
• execute_passwd does an unconditional account_info() round-trip just to fill the JSON access_key, and on failure falls back to the alias name — so --json can report "access_key": "prod" where prod is the alias, not an access key. I'd rather the field be absent than wrong; and the call could live inside the JSON branch so the human path doesn't pay for it.
• mfa disable resolves the code (possibly prompting) before validating the password flags, so conflicting --password-* flags burn a live TOTP code before the usage error appears. execute_passwd already sequences this the right way.
• The AGENTS.md checklist asks for 2 exit-code test scenarios per new command, but everything here is clap-parse or helper-level — nothing asserts an ExitCode, and the from_i32(...).unwrap_or(GeneralError) mappings are untested. tests/admin_service_account.rs is the existing pattern.

Take or leave

• request_account_bytes/json/empty re-derive what request_bounded_json + request_no_response already do, and the three user endpoints hand-build ?accessKey= into the path instead of passing the helpers' query param. Worth folding in so the sign/bound path doesn't fork and query encoding can't drift from SigV4 canonicalization.
• The local fail/usage_failure route through formatter.error (i.e. from_message), so the --json error envelope carries code: null while commands on formatter.fail* carry the real code. Formatter::fail already exists and does both halves.
• confirm_mfa_reset is the third copy of the confirm prompt (idp.rs::confirm_delete, replicate.rs), and write_recovery_codes re-does config.rs::write_private_file. Fine to punt, but a shared helper is overdue at three copies.
• Two small zeroization gaps at odds with the writeup: the File arm does String::from_utf8(bytes.to_vec()), leaving two unzeroized copies of the password (std::str::from_utf8(&bytes) borrows instead), and the enroll JSON branch clones secret_base32/otpauth_uri where it could move them.

Checked and fine

In case anyone else wonders: both password endpoints return a sessions_revoked JSON body, so request_account_json rejecting an empty body is safe; the server DTOs for this family are snake_case, matching yours; and the server always serializes algorithm and qr_utf8, so the non-defaulted serde fields are OK against this server.

Things I genuinely liked: create_new + 0600 on the codes file (modulo the ordering issue above), the const assert pinning the QR floor to the symbol geometry, and the "2FA doesn't gate rc" writeup — that last one is exactly the doc a future operator will need.

[sineld]

Thank you — checking the wire contract against the merged handlers rather than the writeup caught two things I would not have found from the CLI side alone. Everything is addressed except one item I left, called out at the end.

Pushed as two commits: d0432c6 for the review, 95d0c52 for the shared helpers.

Blockers

1. --*-file truncation. Confirmed, and worse than the exit code suggests: no error anywhere, so the operator ends up holding a password that is the first 33 bytes of what they meant to set.

read_protected_key_file/read_exact_key are now read_protected_file(path, &ProtectedFileSpec), with the read bound, the error wording and the hardening rules coming from the caller. Two specs: SSE_C_KEY_FILE and ACCOUNT_SECRET_FILE.

The SSE-C path is byte-identical to before, deliberately. It still reads 33 rather than rejecting at 32, because load_customer_key's own "exactly 32 bytes" check is what should speak for a wrong-length key — that is the string tests/sse_customer.rs pins, and rewording another command's error is not this PR's business. That is what the reject_oversize flag is for: off there, on for the account path, where nothing downstream verifies the length and a silent prefix is the failure. One Failed to inspect {subject} message keeps its missing article for the same reason, with a comment saying why.

The account spec accepts a symlink and a group-readable mode. You are right that a projected Kubernetes secret is a symlink into ..data/; it is also 0644 by default, so allowing the symlink without relaxing the mode check would still have rejected the ordinary deployment. There is a unix test for exactly that shape. The strict rules stay on the SSE-C key, which is long-lived encryption material an operator places directly.

Bound is 4096 bytes, and a longer file is reported rather than truncated.

2. Recovery codes lost. Fixed at both ends, because either alone leaves a hole.

The output path is now checked before the request — via symlink_metadata, not exists, since a dangling symlink reports absent and still makes the create_new open fail with AlreadyExists. And if the write fails anyway, the codes are printed instead of dropped, with a non-zero exit saying the file was not written. Under --json they go to stdout and the error to stderr, so a script gets both.

Both are covered: mfa_activate_refuses_an_occupied_output_path_before_the_server_rotates asserts nothing reaches an unreachable host, and recovery_codes_prints_the_set_when_the_file_cannot_be_written uses a mistyped directory, which passes the up-front check and fails at the open — the window the fallback exists for.

The pre-flight reports Conflict, matching what write_recovery_codes returns for the same condition later, rather than UsageError.

Should fix

• account.rs:351. Fixed. There is no per-field reason on the wire, so the hint is derived from credentials_source and identity_type instead. Worth noting the field was not just mislabelled: in the env-root case it is null, so the most likely reader of that line got a bare refusal.
• print_enrollment. Fixed; the bool now picks between "scan the code above" and "add this by hand".
• sanitize_text. Fixed for the QR (line by line), the setup key, the URI, the parameters, the timestamps, the blocked reasons, the status string, the session key, the policies and groups, and the recovery codes. One deliberate exception: JSON output is not sanitized. serde_json escapes control characters correctly and a consumer needs the value the server sent — escaping is a terminal concern, so it belongs on the human path only.
• execute_passwd round-trip. Fixed. The call moved inside the JSON branch and the field is now Option, omitted rather than wrong.
• mfa disable ordering. Fixed; the password flags are validated before the code is resolved.
• Exit-code tests. Added crates/cli/tests/admin_account.rs, 16 tests, two per new command. Two of them pin the blockers above rather than only an exit status: that a 40-byte secret arrives whole, and that an occupied path sends nothing.

Take or leave — taken

• Request helpers. request_bounded_bytes extracted from request_bounded_json; the account family sits on it, and the three user endpoints pass accessKey through the query mechanism instead of building it into the path.
• fail/usage_failure. Now Formatter::fail, so the --json envelope carries the real code. Also applied to the formatter.error sites this PR added in user.rs; the pre-existing ones in that file are left alone.
• Zeroization. The File arm borrows via std::str::from_utf8(&bytes), and the enroll JSON branch moves.
• confirm_mfa_reset and write_recovery_codes. Done in 95d0c52. crate::confirm holds the prompt, with each caller supplying its three strings; crate::private_file holds the writer. The classification stays with the callers because it genuinely differs — the config.rs test pinning an export refusal as a plain I/O failure still passes, while an occupied recovery-code path stays a Conflict.

Not done

Nothing. I had intended to punt the third-copy item since you marked it fine to, but it turned out to be small once the strings were the only difference, so it is in the second commit.

cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings and cargo test --workspace (2376 tests) all pass.

One thing I did not verify: I have no way to test the projected-Kubernetes-secret path end to end, only the file shape it produces. If you know of a mount layout that differs from symlink-plus-0644, say so and I will widen it.

[sineld]

Heads-up on the one red check, since it needs a nudge rather than a fix.

Protected Files Check failed because this PR touches docs/reference/rc/, and the job wants BREAKING in the PR description. I have added it, with a paragraph in the Contracts section saying plainly what it does and does not mean: the reference edits document commands this PR introduces, and no released behaviour changes. Steps 1 and 2 of the AGENTS.md process need no work here — there is no schema_version to bump and no new output schema, and the command reference is exactly what was updated.

The check will not pick that up on its own. ci.yml triggers on pull_request with no types:, so the default is opened, synchronize, reopened — editing the description is not in that set, and I confirmed no new run was created. A re-run would not help either, since it replays the original event payload with the old body (and gh reports the job as not re-runnable for me anyway). Only a push re-evaluates it.

So: happy to push a trivial commit to re-trigger, but I did not want to put a no-op in the history of your repo unasked. If there is another round of changes coming, that push will clear it by itself. Everything else is green — Format Check, Clippy, Documentation, MSRV, advisories, and the Linux/macOS/Windows test jobs.

Worth saying too: the check fires on any diff under docs/reference/rc/, including a pure addition, so every new-command PR will hit this. If you would rather it only fired on modified or deleted lines, that is a small change to the job and I am glad to send it separately.