fix(mqtt-transport): warn when a configured password is dropped for want of a username

Driver.Mqtt.Transport-002 (Low, correctness). Credentials were attached only
when Username was non-empty, so a config carrying a Password and no Username
connected anonymously with the secret silently discarded — the deployment reads
as authenticated and is not, which is the direction an operator cannot see from
the config.

Warned rather than sent. MQTT 3.1.1 may not carry a password without a
username, so the password-only CONNECT is not available on every protocol
version this factory serves, and synthesizing one under V500 only would change
the wire behaviour of deployments that connect happily today. The configured
values stay authoritative; the drop just stops being silent. The secret is
never formatted into the message.

Tests: the warning fires and does NOT contain the password, plus the falsifying
control that an ordinary anonymous config (neither credential set) stays silent
— without it the fix could warn on every connect and still pass.
This commit is contained in:
Joseph Doherty
2026-08-10 07:58:19 -04:00
parent 94b8633e68
commit 5d4dd17d7f
2 changed files with 49 additions and 0 deletions
@@ -98,6 +98,18 @@ public static class MqttClientOptionsFactory
{
builder = builder.WithCredentials(options.Username, options.Password ?? string.Empty);
}
else if (!string.IsNullOrEmpty(options.Password))
{
// A password with no username is dropped, and the drop is the direction an operator cannot
// see: the config reads as authenticated and the CONNECT goes out anonymous. MQTT 3.1.1 may
// not carry a password without a username, so synthesizing one is not available on every
// protocol version this factory serves — and inventing a password-only CONNECT under 5.0
// only would change the wire behaviour of deployments that connect happily today. Say so
// once, here, and leave the configured values authoritative. Never format the secret.
logger?.LogWarning(
"MQTT: a password is configured with no username, so it is NOT being sent — this connection "
+ "will be made anonymously. Set the username, or clear the password to make that intentional.");
}
builder = builder.WithTlsOptions(tls => ConfigureTls(tls, options, logger));
@@ -366,6 +366,43 @@ public sealed class MqttConnectionTests
built.Credentials.GetUserName(built).ShouldBe("svc");
}
/// <summary>
/// <b>Driver.Mqtt.Transport-002.</b> A configured password with no username is dropped — MQTT
/// 3.1.1 may not carry one without the other, and this factory does not synthesize the
/// password-only CONNECT 5.0 would permit. What must not happen is the drop being silent: the
/// deployment then believes it authenticates and does not, which is the one direction an
/// operator cannot see from the config. The secret itself stays out of the log.
/// </summary>
[Fact]
public void BuildClientOptions_PasswordWithoutUsername_IsDroppedLoudlyAndWithoutLeakingIt()
{
const string secret = "s3cr3t-that-must-not-be-logged";
var log = new CapturingLogger();
var built = MqttClientOptionsFactory.BuildClientOptions(
new MqttDriverOptions { Host = Host, Password = secret }, clientIdSuffix: null, logger: log);
built.Credentials.ShouldBeNull();
log.CountContaining("password").ShouldBe(1);
log.CountContaining(secret).ShouldBe(0);
}
/// <summary>
/// The falsifying control for the test above: an ordinary anonymous deployment configures
/// neither credential and must stay silent, or every such driver warns on every connect.
/// </summary>
[Fact]
public void BuildClientOptions_NoCredentialsAtAll_SaysNothing()
{
var log = new CapturingLogger();
MqttClientOptionsFactory.BuildClientOptions(
new MqttDriverOptions { Host = Host }, clientIdSuffix: null, logger: log)
.Credentials.ShouldBeNull();
log.CountContaining("password").ShouldBe(0);
}
/// <summary>The ctor must not touch the network — the universal-browser throwaway-instance pattern.</summary>
[Fact]
public void Ctor_IsConnectionFree()