From 5d4dd17d7f2513a35c6e676a5359ac69782b53f5 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 07:58:19 -0400 Subject: [PATCH] fix(mqtt-transport): warn when a configured password is dropped for want of a username MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../MqttClientOptionsFactory.cs | 12 ++++++ .../MqttConnectionTests.cs | 37 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs index 0f7e4333..17dde107 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs @@ -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)); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttConnectionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttConnectionTests.cs index f6a82601..315ca370 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttConnectionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttConnectionTests.cs @@ -366,6 +366,43 @@ public sealed class MqttConnectionTests built.Credentials.GetUserName(built).ShouldBe("svc"); } + /// + /// Driver.Mqtt.Transport-002. 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. + /// + [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); + } + + /// + /// 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. + /// + [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); + } + /// The ctor must not touch the network — the universal-browser throwaway-instance pattern. [Fact] public void Ctor_IsConnectionFree()