fix(CLI-45): standardize the CLI credential env var and fail fast on empty passwords
All five client CLIs now share one credential contract for `authenticate-user`: flags `--password` / `--password-env` (Go: `-password` / `-password-env`) with default env `MXGATEWAY_VERIFY_PASSWORD`, resolution flag-then-env, and a resolved credential that is missing *or empty* is a usage error naming the flag and the variable. The value is never echoed and never reaches the wire. Go and Java previously sent an empty credential when the variable was unset, turning a misconfigured environment into a real MXAccess authentication attempt. Go now returns the guard error before dialing; Java throws a picocli ParameterException instead of falling back to "". Python's `--password-env` gained the canonical default and its UsageError names the resolved variable. Rust treats an empty flag or env value as missing, with the resolution extracted into a testable `resolve_verify_user_password`. .NET adopts the canonical flags and keeps `--verify-user-password`, `--verify-user-password-env`, and MXGATEWAY_VERIFY_USER_PASSWORD as deprecated aliases for one release. Docs same commit: CrossLanguageSmokeMatrix.md gains the credential contract and the per-CLI subcommand-coverage table (the documented-not-fixed half of the finding); all five READMEs name the canonical variable and the fail-fast rule, and the .NET README carries the deprecation note. Tracking flipped to Done in both remediation registers with a change-log row. No .proto changed; no generated code regenerated.
This commit is contained in:
@@ -216,7 +216,12 @@ the wire — the client never logs them and never embeds them in an `Error`'s
|
||||
`Display`/`Debug`; the only error text that can surface (from `tonic::Status`
|
||||
messages and reply diagnostics) is scrubbed by the credential-redaction seam.
|
||||
The CLI mirrors these as `authenticate-user` (password via `--password` or the
|
||||
`--password-env` env var, never echoed) and `write-secured`.
|
||||
variable named by `--password-env`, default `MXGATEWAY_VERIFY_PASSWORD`, never
|
||||
echoed) and `write-secured`. The credential is required: a missing or empty
|
||||
resolved value is a usage error naming the flag and the variable, so the CLI
|
||||
fails before dialing instead of authenticating with an empty password.
|
||||
`MXGATEWAY_VERIFY_PASSWORD` is the canonical variable across all five client
|
||||
CLIs — see [Cross-Language Smoke Matrix](../../docs/CrossLanguageSmokeMatrix.md).
|
||||
|
||||
The remaining single-item command helpers round out MXAccess parity:
|
||||
`unregister`, `suspend` / `activate` (each returns the operation's
|
||||
|
||||
@@ -752,17 +752,7 @@ async fn dispatch(command: Command) -> Result<(), Error> {
|
||||
password_env,
|
||||
json,
|
||||
} => {
|
||||
// Resolve the credential from --password or the named env var.
|
||||
// The password is passed straight to the typed helper and is never
|
||||
// echoed to stdout/stderr or embedded in an error message.
|
||||
let verify_user_password = password
|
||||
.or_else(|| env::var(&password_env).ok())
|
||||
.ok_or_else(|| Error::InvalidArgument {
|
||||
name: "password".to_owned(),
|
||||
detail: format!(
|
||||
"supply --password or set the environment variable `{password_env}`"
|
||||
),
|
||||
})?;
|
||||
let verify_user_password = resolve_verify_user_password(password, &password_env)?;
|
||||
let session = session_for(connection, session_id).await?;
|
||||
let user_id = session
|
||||
.authenticate_user(server_handle, &verify_user, &verify_user_password)
|
||||
@@ -1736,6 +1726,31 @@ fn print_ok(operation: &str, use_json: bool) {
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolves the `authenticate-user` credential from `--password`, falling back to
|
||||
/// the environment variable named by `--password-env` (default
|
||||
/// `MXGATEWAY_VERIFY_PASSWORD`).
|
||||
///
|
||||
/// An empty value from either source counts as missing (CLI-45): the CLI fails
|
||||
/// fast with a usage error rather than sending a fabricated empty credential to
|
||||
/// the wire. The error names the flag and the variable only — never the value,
|
||||
/// which is never echoed to stdout/stderr or embedded in an error message.
|
||||
fn resolve_verify_user_password(
|
||||
password: Option<String>,
|
||||
password_env: &str,
|
||||
) -> Result<String, Error> {
|
||||
password
|
||||
.filter(|value| !value.is_empty())
|
||||
.or_else(|| {
|
||||
env::var(password_env)
|
||||
.ok()
|
||||
.filter(|value| !value.is_empty())
|
||||
})
|
||||
.ok_or_else(|| Error::InvalidArgument {
|
||||
name: "password".to_owned(),
|
||||
detail: format!("supply --password or set the environment variable `{password_env}`"),
|
||||
})
|
||||
}
|
||||
|
||||
fn print_bulk_results(
|
||||
operation: &str,
|
||||
results: &[zb_mom_ww_mxgateway_client::generated::mxaccess_gateway::v1::SubscribeResult],
|
||||
@@ -2617,6 +2632,66 @@ mod tests {
|
||||
assert!(parsed.is_ok(), "parse failed: {parsed:?}");
|
||||
}
|
||||
|
||||
/// CLI-45: `--password-env` must default to the canonical
|
||||
/// `MXGATEWAY_VERIFY_PASSWORD` shared by every official client CLI.
|
||||
#[test]
|
||||
fn authenticate_user_password_env_defaults_to_canonical_name() {
|
||||
let parsed = Cli::try_parse_from([
|
||||
"mxgw",
|
||||
"authenticate-user",
|
||||
"--session-id",
|
||||
"session-1",
|
||||
"--server-handle",
|
||||
"7",
|
||||
"--verify-user",
|
||||
"verifier",
|
||||
])
|
||||
.expect("parse");
|
||||
match parsed.command {
|
||||
Command::AuthenticateUser { password_env, .. } => {
|
||||
assert_eq!(password_env, "MXGATEWAY_VERIFY_PASSWORD");
|
||||
}
|
||||
other => panic!("expected authenticate-user, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
/// CLI-45: a credential that resolves to an empty string — whether from the
|
||||
/// flag or from the named environment variable — is treated as missing, so
|
||||
/// the CLI never sends a fabricated empty password to the wire. The usage
|
||||
/// error names the flag and the variable, never a value.
|
||||
#[test]
|
||||
fn resolve_verify_user_password_rejects_missing_and_empty_values() {
|
||||
const ABSENT: &str = "MXGW_CLI45_ABSENT_PASSWORD_VAR";
|
||||
const EMPTY: &str = "MXGW_CLI45_EMPTY_PASSWORD_VAR";
|
||||
const PRESENT: &str = "MXGW_CLI45_PRESENT_PASSWORD_VAR";
|
||||
std::env::remove_var(ABSENT);
|
||||
std::env::set_var(EMPTY, "");
|
||||
std::env::set_var(PRESENT, "env-sourced-credential");
|
||||
|
||||
for (password, env_name) in [
|
||||
(None, ABSENT),
|
||||
(Some(String::new()), ABSENT),
|
||||
(None, EMPTY),
|
||||
(Some(String::new()), EMPTY),
|
||||
] {
|
||||
let error = super::resolve_verify_user_password(password, env_name)
|
||||
.expect_err("empty or missing credential must be a usage error");
|
||||
let rendered = error.to_string();
|
||||
assert!(rendered.contains("--password"), "{rendered}");
|
||||
assert!(rendered.contains(env_name), "{rendered}");
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
super::resolve_verify_user_password(None, PRESENT).expect("env credential"),
|
||||
"env-sourced-credential"
|
||||
);
|
||||
assert_eq!(
|
||||
super::resolve_verify_user_password(Some("flag-credential".to_owned()), EMPTY)
|
||||
.expect("flag credential"),
|
||||
"flag-credential"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_write_secured_command() {
|
||||
let parsed = Cli::try_parse_from([
|
||||
|
||||
Reference in New Issue
Block a user