fix: bind OtOpcUa LdapOptions from real Security:Ldap section; gate validator on DevStubMode

2026-06-01 22:46:09 -04:00
parent 88e773af36
commit d3ab2bfbaf
7 changed files with 94 additions and 10 deletions
@@ -7,7 +7,8 @@ namespace ZB.MOM.WW.OtOpcUa.Host.Configuration;
 ///     Fail-fast startup validator for <see cref="LdapOptions"/>, built on the shared
 ///     <c>ZB.MOM.WW.Configuration</c> <see cref="OptionsValidatorBase{TOptions}"/>. When LDAP login
 ///     is enabled, <c>Server</c> and <c>SearchBase</c> must be set and <c>Port</c> must be a valid
-///     TCP port; when disabled, all checks are skipped. <c>ServiceAccountDn</c>/<c>Password</c> are
+///     TCP port; when disabled — or when <c>DevStubMode</c> bypasses the real bind — all checks are
+///     skipped. <c>ServiceAccountDn</c>/<c>Password</c> are
 ///     intentionally not required — an empty pair selects the direct-bind path (see
 ///     <see cref="LdapOptions.ServiceAccountDn"/>). Failure messages carry the real <c>"Ldap:"</c>
 ///     section prefix matching the bound configuration section.
@@ -17,7 +18,10 @@ public sealed class LdapOptionsValidator : OptionsValidatorBase<LdapOptions>
    /// <inheritdoc />
    protected override void Validate(ValidationBuilder builder, LdapOptions options)
    {
-        if (!options.Enabled) return;
+        // Skip the real-LDAP field checks when LDAP login is disabled, or when the dev stub is
+        // active — DevStubMode bypasses the real bind entirely, so Server/SearchBase/Port are
+        // irrelevant and would otherwise force dev configs to carry meaningless placeholders.
+        if (!options.Enabled || options.DevStubMode) return;

        builder.RequireThat(!string.IsNullOrWhiteSpace(options.Server),
            "Ldap:Server is required when LDAP login is enabled.");
@@ -25,8 +25,9 @@ public sealed class OpcUaApplicationHostOptionsValidator : OptionsValidatorBase<
        builder.Required(o.PublicHostname, "OpcUa:PublicHostname");
        builder.Required(o.PkiStoreRoot, "OpcUa:PkiStoreRoot");
        builder.Port(o.OpcUaPort, "OpcUa:OpcUaPort");
-        // EnabledSecurityProfiles is typed IList<T>, which does not implement IReadOnlyCollection<T>;
-        // ToList() bridges to the shared MinCount primitive while preserving the count (and message).
+        // EnabledSecurityProfiles is declared as IList<T> — that interface does not derive from
+        // IReadOnlyCollection<T>, so it can't bind to MinCount's IReadOnlyCollection<T> parameter
+        // directly. ToList() bridges to the shared primitive while preserving the count (and message).
        builder.MinCount(o.EnabledSecurityProfiles?.ToList(), 1, "OpcUa:EnabledSecurityProfiles");
    }
 }
@@ -59,11 +59,9 @@ public sealed class OtOpcUaServerHostedService : IHostedService, IAsyncDisposabl
    /// <param name="cancellationToken">Cancellation token.</param>
    public async Task StartAsync(CancellationToken cancellationToken)
    {
-        var options = _options;
-
        _server = new OtOpcUaSdkServer();
        _appHost = new OpcUaApplicationHost(
-            options,
+            _options,
            _loggerFactory.CreateLogger<OpcUaApplicationHost>(),
            _userAuthenticator);

@@ -99,7 +99,7 @@ if (hasDriver)
        new RoslynScriptedAlarmEvaluator(sp.GetRequiredService<ILoggerFactory>().CreateLogger<RoslynScriptedAlarmEvaluator>()));
    builder.Services.AddSingleton<IScriptedAlarmEvaluator>(sp => sp.GetRequiredService<RoslynScriptedAlarmEvaluator>());

-    builder.Services.AddValidatedOptions<LdapOptions, LdapOptionsValidator>(builder.Configuration, "Ldap");
+    builder.Services.AddValidatedOptions<LdapOptions, LdapOptionsValidator>(builder.Configuration, LdapOptions.SectionName);
    builder.Services.AddSingleton<ILdapAuthService, LdapAuthService>();
    builder.Services.AddSingleton<IOpcUaUserAuthenticator, LdapOpcUaUserAuthenticator>();

@@ -2,12 +2,12 @@ namespace ZB.MOM.WW.OtOpcUa.Security.Ldap;

 /// <summary>
 ///     LDAP + role-mapping configuration for the Admin UI. Bound from <c>appsettings.json</c>
-///     <c>Authentication:Ldap</c> section. Defaults point at the local GLAuth dev instance (see
+///     <c>Security:Ldap</c> section. Defaults point at the local GLAuth dev instance (see
 ///     <c>C:\publish\glauth\auth.md</c>).
 /// </summary>
 public sealed class LdapOptions
 {
-    public const string SectionName = "Authentication:Ldap";
+    public const string SectionName = "Security:Ldap";

    /// <summary>Gets or sets a value indicating whether LDAP authentication is enabled.</summary>
    public bool Enabled { get; set; } = true;
@@ -0,0 +1,62 @@
+using Microsoft.Extensions.Configuration;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Security.Ldap;
+
+namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests;
+
+/// <summary>
+/// Regression guard for the LDAP config-section fix. The real config (admin/driver/Development
+/// overlays) lives under <c>Security:Ldap</c>, and <see cref="LdapOptions.SectionName"/> must point
+/// there so the configured <c>DevStubMode</c> actually binds. Previously the binders used the
+/// nonexistent <c>"Ldap"</c>/<c>"Authentication:Ldap"</c> sections, so the dev stub never activated.
+/// </summary>
+public sealed class LdapOptionsBindingTests
+{
+    /// <summary><see cref="LdapOptions.SectionName"/> resolves to the real overlay section.</summary>
+    [Fact]
+    public void SectionName_is_Security_Ldap()
+    {
+        LdapOptions.SectionName.ShouldBe("Security:Ldap");
+    }
+
+    /// <summary>
+    /// Binding from <see cref="LdapOptions.SectionName"/> reads the configured <c>DevStubMode</c>
+    /// from the real <c>Security:Ldap</c> section — proving the dev stub now takes effect.
+    /// </summary>
+    [Fact]
+    public void Binding_from_SectionName_reads_Security_Ldap_DevStubMode()
+    {
+        var configuration = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string?>
+            {
+                ["Security:Ldap:DevStubMode"] = "true",
+            })
+            .Build();
+
+        var options = configuration.GetSection(LdapOptions.SectionName).Get<LdapOptions>();
+
+        options.ShouldNotBeNull();
+        options.DevStubMode.ShouldBeTrue();
+    }
+
+    /// <summary>
+    /// Negative control: binding from the old (nonexistent) <c>"Ldap"</c> section against the same
+    /// <c>Security:Ldap</c> config does NOT pick up <c>DevStubMode</c> — it falls back to the C#
+    /// default (false). This is the pre-fix behaviour the change corrects.
+    /// </summary>
+    [Fact]
+    public void Binding_from_old_Ldap_section_does_not_read_DevStubMode()
+    {
+        var configuration = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string?>
+            {
+                ["Security:Ldap:DevStubMode"] = "true",
+            })
+            .Build();
+
+        var options = configuration.GetSection("Ldap").Get<LdapOptions>() ?? new LdapOptions();
+
+        options.DevStubMode.ShouldBeFalse();
+    }
+}
@@ -46,6 +46,25 @@ public sealed class LdapOptionsValidatorTests
        Sut.Validate(null, options).Succeeded.ShouldBeTrue();
    }

+    /// <summary>
+    /// When the dev stub is active the real LDAP fields are irrelevant (the bind is bypassed), so
+    /// the gate skips the Server/SearchBase/Port checks even though LDAP is nominally enabled.
+    /// </summary>
+    [Fact]
+    public void DevStubMode_options_succeed_even_when_server_blank()
+    {
+        var options = new LdapOptions
+        {
+            Enabled = true,
+            DevStubMode = true,
+            Server = string.Empty,
+            SearchBase = string.Empty,
+            Port = 0,
+        };
+
+        Sut.Validate(null, options).Succeeded.ShouldBeTrue();
+    }
+
    /// <summary>Enabled with a blank server reports the required-server failure.</summary>
    [Fact]
    public void Enabled_with_blank_server_fails()