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()