fix(auth): OtOpcUa 1.2 review fixes — startup insecure-transport guard + Ldaps in prod overlays, test fidelity, 0.1.1 pin

Directory.Packages.props

@@ -104,9 +104,9 @@
     <PackageVersion Include="ZB.MOM.WW.MxGateway.Client" Version="0.1.0" />
     <PackageVersion Include="ZB.MOM.WW.MxGateway.Contracts" Version="0.1.0" />
     <PackageVersion Include="ZB.MOM.WW.Configuration" Version="0.1.0" />
-    <PackageVersion Include="ZB.MOM.WW.Auth.Abstractions" Version="0.1.0" />
-    <PackageVersion Include="ZB.MOM.WW.Auth.Ldap" Version="0.1.0" />
-    <PackageVersion Include="ZB.MOM.WW.Auth.AspNetCore" Version="0.1.0" />
+    <PackageVersion Include="ZB.MOM.WW.Auth.Abstractions" Version="0.1.1" />
+    <PackageVersion Include="ZB.MOM.WW.Auth.Ldap" Version="0.1.1" />
+    <PackageVersion Include="ZB.MOM.WW.Auth.AspNetCore" Version="0.1.1" />
     <PackageVersion Include="ZB.MOM.WW.Audit" Version="0.1.0" />
   </ItemGroup>
 </Project>

src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs

@@ -1,5 +1,6 @@
 using ZB.MOM.WW.Configuration;
 using ZB.MOM.WW.OtOpcUa.Security.Ldap;
+using LdapTransport = ZB.MOM.WW.Auth.Abstractions.Ldap.LdapTransport;
 
 namespace ZB.MOM.WW.OtOpcUa.Host.Configuration;
 
@@ -10,13 +11,19 @@ namespace ZB.MOM.WW.OtOpcUa.Host.Configuration;
 ///     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"/>). The plaintext-transport-without-AllowInsecure
-///     guard is enforced at the auth boundary (<see cref="OtOpcUaLdapAuthService"/>) rather than here,
-///     to preserve the bespoke service's behaviour of booting and failing closed at login (not at
-///     startup) when a config selects insecure transport. Failure messages use <c>"Ldap:"</c> as a
+///     <see cref="LdapOptions.ServiceAccountDn"/>). Failure messages use <c>"Ldap:"</c> as a
 ///     human-readable field prefix — not the literal bound section path, which is
 ///     <c>Security:Ldap</c> (see <see cref="LdapOptions.SectionName"/>).
 /// </summary>
+/// <remarks>
+///     Insecure-transport guard (review fix): a real-LDAP config that selects plaintext transport
+///     (<see cref="LdapTransport.None"/>) without opting in via <see cref="LdapOptions.AllowInsecure"/>
+///     now FAILS startup validation, so an insecure-by-accident production overlay never boots.
+///     This mirrors the login-time fail-closed guard in <see cref="OtOpcUaLdapAuthService"/> and is
+///     gated on the same conditions (<see cref="LdapOptions.Enabled"/> AND not
+///     <see cref="LdapOptions.DevStubMode"/>): a disabled or dev-stub config is exempt, exactly as it
+///     is exempt from the real bind. The login-time guard remains as defence in depth.
+/// </remarks>
 public sealed class LdapOptionsValidator : OptionsValidatorBase<LdapOptions>
 {
     /// <inheritdoc />
@@ -32,5 +39,13 @@ public sealed class LdapOptionsValidator : OptionsValidatorBase<LdapOptions>
         builder.RequireThat(!string.IsNullOrWhiteSpace(options.SearchBase),
             "Ldap:SearchBase is required when LDAP login is enabled.");
         builder.Port(options.Port, "Ldap:Port");
+
+        // Fail closed at startup on a plaintext transport unless explicitly opted in — same
+        // condition the login-time guard in OtOpcUaLdapAuthService enforces, lifted to boot so an
+        // insecure-by-accident production overlay refuses to start rather than silently failing
+        // every bind at login.
+        builder.RequireThat(
+            !(options.Transport == LdapTransport.None && !options.AllowInsecure),
+            "LDAP transport is None (plaintext) but AllowInsecure is false — set Transport to Ldaps/StartTls or set AllowInsecure for dev.");
     }
 }

src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.admin-driver.json

@@ -11,7 +11,8 @@
   },
   "Security": {
     "Ldap": {
-      "DevStubMode": false
+      "DevStubMode": false,
+      "Transport": "Ldaps"
     }
   }
 }

src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.admin.json

@@ -10,7 +10,8 @@
   },
   "Security": {
     "Ldap": {
-      "DevStubMode": false
+      "DevStubMode": false,
+      "Transport": "Ldaps"
     }
   }
 }

src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.driver.json

@@ -10,7 +10,8 @@
   },
   "Security": {
     "Ldap": {
-      "DevStubMode": false
+      "DevStubMode": false,
+      "Transport": "Ldaps"
     }
   }
 }

src/Server/ZB.MOM.WW.OtOpcUa.Security/Endpoints/AuthEndpoints.cs

@@ -103,6 +103,12 @@ public static class AuthEndpoints
         {
             // A DB hiccup (or any mapper fault) must never block sign-in — fall back to the
             // pre-resolved baseline roles (empty on the real path, FleetAdmin under DevStub).
+            // This is intentionally FAIL-CLOSED on the real LDAP path: result.Roles is empty there
+            // (the library returns groups, never roles — the mapper is the sole role source), so a
+            // mapper fault signs the user in AUTHENTICATED but with ZERO role claims. They can prove
+            // identity but are denied every role-gated action until the mapper recovers — strictly
+            // safer than failing open with a stale/guessed role set. (See AuthEndpoints test
+            // Login_when_role_mapper_throws_signs_in_with_no_role_claims.)
             http.RequestServices.GetService<ILoggerFactory>()?
                 .CreateLogger("ZB.MOM.WW.OtOpcUa.Security.AuthEndpoints")
                 .LogWarning(ex, "Role-map lookup failed for {User}; using pre-resolved baseline roles", username);

tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsBindingTests.cs

@@ -1,7 +1,9 @@
 using Microsoft.Extensions.Configuration;
 using Shouldly;
 using Xunit;
+using ZB.MOM.WW.OtOpcUa.Host.Configuration;
 using ZB.MOM.WW.OtOpcUa.Security.Ldap;
+using LdapTransport = ZB.MOM.WW.Auth.Abstractions.Ldap.LdapTransport;
 
 namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests;
 
@@ -60,3 +62,64 @@ public sealed class LdapOptionsBindingTests
         options.DevStubMode.ShouldBeFalse();
     }
 }
+
+/// <summary>
+/// End-to-end guard for the shipped production overlays: binds each of the three prod overlay
+/// files' real <c>Security:Ldap</c> section (the same files the host loads at boot, copied into the
+/// test output via the Host project reference) and runs the <see cref="LdapOptionsValidator"/> the
+/// host wires via <c>AddValidatedOptions</c>. Proves each prod overlay declares a TLS transport and
+/// therefore PASSES startup validation — i.e. the host actually boots with these overlays after the
+/// insecure-transport guard was added. The <c>Development</c> overlay (DevStubMode) is verified to
+/// pass via the guard exemption.
+/// </summary>
+public sealed class ProdOverlayValidationTests
+{
+    private static readonly LdapOptionsValidator Sut = new();
+
+    private static LdapOptions BindOverlay(string fileName)
+    {
+        var path = Path.Combine(AppContext.BaseDirectory, fileName);
+        File.Exists(path).ShouldBeTrue($"overlay '{fileName}' should be copied to the test output");
+
+        var configuration = new ConfigurationBuilder()
+            .AddJsonFile(path, optional: false, reloadOnChange: false)
+            .Build();
+
+        return configuration.GetSection(LdapOptions.SectionName).Get<LdapOptions>() ?? new LdapOptions();
+    }
+
+    [Theory]
+    [InlineData("appsettings.admin.json")]
+    [InlineData("appsettings.driver.json")]
+    [InlineData("appsettings.admin-driver.json")]
+    public void Prod_overlay_declares_ldaps_transport(string fileName)
+    {
+        var options = BindOverlay(fileName);
+
+        options.DevStubMode.ShouldBeFalse();
+        options.Transport.ShouldBe(LdapTransport.Ldaps);
+    }
+
+    [Theory]
+    [InlineData("appsettings.admin.json")]
+    [InlineData("appsettings.driver.json")]
+    [InlineData("appsettings.admin-driver.json")]
+    public void Prod_overlay_passes_startup_validation(string fileName)
+    {
+        var options = BindOverlay(fileName);
+
+        // Match the host: these overlays only set Security:Ldap fields, so backfill the required
+        // Server/SearchBase/Port the way the base C# defaults do (LdapOptions defaults are valid),
+        // then validate exactly as AddValidatedOptions would at boot.
+        Sut.Validate(null, options).Succeeded.ShouldBeTrue();
+    }
+
+    [Fact]
+    public void Development_overlay_passes_startup_validation_via_devstub_exemption()
+    {
+        var options = BindOverlay("appsettings.Development.json");
+
+        options.DevStubMode.ShouldBeTrue();
+        Sut.Validate(null, options).Succeeded.ShouldBeTrue();
+    }
+}

tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsValidatorTests.cs

@@ -2,6 +2,7 @@ using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Host.Configuration;
 using ZB.MOM.WW.OtOpcUa.Security.Ldap;
+using LdapTransport = ZB.MOM.WW.Auth.Abstractions.Ldap.LdapTransport;
 
 namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests;
 
@@ -10,13 +11,18 @@ namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests;
 /// <c>ZB.MOM.WW.Configuration</c> <c>OptionsValidatorBase</c>/<c>ValidationBuilder</c>) gates on
 /// <see cref="LdapOptions.Enabled"/>, and that when enabled it requires <c>Server</c>,
 /// <c>SearchBase</c>, and a valid <c>Port</c>. Failure messages carry the real <c>"Ldap:"</c>
-/// section prefix so they read correctly when surfaced at host startup.
+/// section prefix so they read correctly when surfaced at host startup. Also verifies the
+/// insecure-transport startup guard: a real-LDAP config selecting plaintext transport without
+/// <see cref="LdapOptions.AllowInsecure"/> fails fast at boot.
 /// </summary>
 public sealed class LdapOptionsValidatorTests
 {
     private static readonly LdapOptionsValidator Sut = new();
 
-    /// <summary>Valid enabled options pass validation.</summary>
+    private const string InsecureTransportFailure =
+        "LDAP transport is None (plaintext) but AllowInsecure is false — set Transport to Ldaps/StartTls or set AllowInsecure for dev.";
+
+    /// <summary>Valid enabled options (a TLS transport) pass validation.</summary>
     [Fact]
     public void Valid_enabled_options_succeed()
     {
@@ -26,6 +32,102 @@ public sealed class LdapOptionsValidatorTests
             Server = "ldap",
             SearchBase = "dc=x",
             Port = 389,
+            Transport = LdapTransport.Ldaps,
+        };
+
+        Sut.Validate(null, options).Succeeded.ShouldBeTrue();
+    }
+
+    /// <summary>
+    /// Insecure-transport guard: an enabled real-LDAP config that selects plaintext
+    /// <see cref="LdapTransport.None"/> without <see cref="LdapOptions.AllowInsecure"/> fails
+    /// startup validation with the guard message.
+    /// </summary>
+    [Fact]
+    public void Enabled_with_plaintext_transport_and_not_allow_insecure_fails()
+    {
+        var options = new LdapOptions
+        {
+            Enabled = true,
+            Server = "ldap",
+            SearchBase = "dc=x",
+            Port = 389,
+            Transport = LdapTransport.None,
+            AllowInsecure = false,
+        };
+
+        var result = Sut.Validate(null, options);
+
+        result.Failed.ShouldBeTrue();
+        result.Failures.ShouldContain(InsecureTransportFailure);
+    }
+
+    /// <summary>A TLS transport (<see cref="LdapTransport.Ldaps"/>) satisfies the guard.</summary>
+    [Fact]
+    public void Enabled_with_ldaps_transport_passes_guard()
+    {
+        var options = new LdapOptions
+        {
+            Enabled = true,
+            Server = "ldap",
+            SearchBase = "dc=x",
+            Port = 636,
+            Transport = LdapTransport.Ldaps,
+        };
+
+        Sut.Validate(null, options).Succeeded.ShouldBeTrue();
+    }
+
+    /// <summary>
+    /// Explicit opt-in: plaintext transport with <see cref="LdapOptions.AllowInsecure"/> set is
+    /// permitted (dev/test escape hatch), so the guard does not trip.
+    /// </summary>
+    [Fact]
+    public void Enabled_plaintext_with_allow_insecure_passes_guard()
+    {
+        var options = new LdapOptions
+        {
+            Enabled = true,
+            Server = "ldap",
+            SearchBase = "dc=x",
+            Port = 389,
+            Transport = LdapTransport.None,
+            AllowInsecure = true,
+        };
+
+        Sut.Validate(null, options).Succeeded.ShouldBeTrue();
+    }
+
+    /// <summary>
+    /// DevStubMode is exempt from the insecure-transport guard: the dev stub bypasses the real
+    /// bind, so plaintext transport is irrelevant and must not block boot.
+    /// </summary>
+    [Fact]
+    public void DevStubMode_with_plaintext_transport_passes_guard()
+    {
+        var options = new LdapOptions
+        {
+            Enabled = true,
+            DevStubMode = true,
+            Transport = LdapTransport.None,
+            AllowInsecure = false,
+        };
+
+        Sut.Validate(null, options).Succeeded.ShouldBeTrue();
+    }
+
+    /// <summary>
+    /// A disabled config is exempt from the insecure-transport guard even with plaintext
+    /// transport — LDAP login never runs, so the guard must not trip.
+    /// </summary>
+    [Fact]
+    public void Disabled_with_plaintext_transport_passes_guard()
+    {
+        var options = new LdapOptions
+        {
+            Enabled = false,
+            Transport = LdapTransport.None,
+            AllowInsecure = false,
         };
 
         Sut.Validate(null, options).Succeeded.ShouldBeTrue();

tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/AuthEndpointsIntegrationTests.cs

@@ -59,6 +59,11 @@ public sealed class AuthEndpointsIntegrationTests : IAsyncLifetime
                             ["Security:Jwt:SigningKey"] = "test-signing-key-with-at-least-32-bytes-of-utf8-content",
                             ["Security:Jwt:Issuer"] = "otopcua-test",
                             ["Security:Jwt:Audience"] = "otopcua-test",
+                            // GroupToRole baseline bound onto LdapOptions: the production
+                            // OtOpcUaGroupRoleMapper resolves "ConfigViewer" from the LDAP group
+                            // "ReadOnly". This exercises the real mapper path — the stub no longer
+                            // pre-populates roles, so ConfigViewer can only come from the mapper.
+                            ["Security:Ldap:GroupToRole:ReadOnly"] = "ConfigViewer",
                         }).Build();
                     services.AddOtOpcUaAuth(configuration);
                     services.AddSingleton<ILdapAuthService, StubLdapAuthService>();
@@ -187,8 +192,9 @@ public sealed class AuthEndpointsIntegrationTests : IAsyncLifetime
     [Fact]
     public async Task Login_merges_db_role_grant_into_claims()
     {
-        // StubLdapAuthService returns Groups ["ReadOnly"], baseline Roles ["ConfigViewer"].
-        // A system-wide row maps "ReadOnly" → FleetAdmin, so the merged set is both.
+        // StubLdapAuthService returns Groups ["ReadOnly"] with empty Roles (the real production
+        // shape). The mapper resolves the appsettings baseline "ReadOnly" → ConfigViewer, then a
+        // system-wide DB row maps "ReadOnly" → FleetAdmin, so the merged set is both.
         _roleMappings.Rows.Add(new LdapGroupRoleMapping
         {
             Id = Guid.NewGuid(),
@@ -214,18 +220,24 @@ public sealed class AuthEndpointsIntegrationTests : IAsyncLifetime
         roles.ShouldContain("FleetAdmin");   // DB grant merged in
     }
 
-    /// <summary>When the DB role-map lookup throws, sign-in still succeeds with the appsettings
-    /// baseline roles — a DB hiccup must never block login.</summary>
+    /// <summary>Fail-closed (review I3): when the role mapper throws on the real production path
+    /// (the auth result carries no pre-resolved roles — roles come only from the mapper), sign-in
+    /// still SUCCEEDS but the user is granted ZERO role claims. They are authenticated (can prove
+    /// identity) yet authorized for nothing role-gated until the mapper recovers — the safe
+    /// fail-closed behaviour, not a fail-open with a stale role set.</summary>
     [Fact]
-    public async Task Login_when_db_role_map_throws_falls_back_to_baseline_roles()
+    public async Task Login_when_role_mapper_throws_signs_in_with_no_role_claims()
     {
+        // Simulate a mapper fault on the real path. The whole MapAsync throws (the appsettings
+        // baseline is computed inside the mapper, so it does NOT survive the throw): the login
+        // endpoint falls back to result.Roles, which is empty on the real LDAP path.
         _roleMappings.Throws = true;
 
         var client = NewClient();
         var loginResponse = await client.PostAsJsonAsync("/auth/login",
             new AuthEndpoints.LoginRequest("alice", "valid-password"), Ct);
 
-        // Login proceeds despite the simulated DB outage.
+        // Login proceeds despite the simulated DB outage — authenticated.
         loginResponse.StatusCode.ShouldBe(HttpStatusCode.NoContent);
 
         var tokenReq = new HttpRequestMessage(HttpMethod.Post, "/auth/token");
@@ -233,9 +245,10 @@ public sealed class AuthEndpointsIntegrationTests : IAsyncLifetime
         var tokenResp = await client.SendAsync(tokenReq, Ct);
         tokenResp.StatusCode.ShouldBe(HttpStatusCode.OK);
 
+        // No role claims at all — fail closed.
         var payload = await tokenResp.Content.ReadFromJsonAsync<JsonElement>(Ct);
         var roles = JwtRoleClaims(payload.GetProperty("token").GetString()!);
-        roles.ShouldContain("ConfigViewer"); // baseline still present
+        roles.ShouldBeEmpty();
     }
 
     /// <summary>Extracts the "Role" claim values from a JWT's payload segment.</summary>
@@ -330,7 +343,11 @@ public sealed class AuthEndpointsIntegrationTests : IAsyncLifetime
                     DisplayName: "Alice User",
                     Username: username,
                     Groups: ["ReadOnly"],
-                    Roles: ["ConfigViewer"],
+                    // Roles empty — the real production path returns groups, never roles. Role
+                    // resolution is the mapper's job (OtOpcUaGroupRoleMapper applies the
+                    // GroupToRole baseline). This proves roles flow through the mapper, not via
+                    // pre-population of the auth result.
+                    Roles: [],
                     Error: null));
             return Task.FromResult(new LdapAuthResult(
                 Success: false,