diff --git a/Directory.Packages.props b/Directory.Packages.props index 6d958cd8..0d9471b1 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -104,9 +104,9 @@ - - - + + + \ No newline at end of file diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs index e95377c8..9b483a75 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs +++ b/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 DevStubMode bypasses the real bind — all checks are /// skipped. ServiceAccountDn/Password are /// intentionally not required — an empty pair selects the direct-bind path (see -/// ). The plaintext-transport-without-AllowInsecure -/// guard is enforced at the auth boundary () 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 "Ldap:" as a +/// ). Failure messages use "Ldap:" as a /// human-readable field prefix — not the literal bound section path, which is /// Security:Ldap (see ). /// +/// +/// Insecure-transport guard (review fix): a real-LDAP config that selects plaintext transport +/// () without opting in via +/// now FAILS startup validation, so an insecure-by-accident production overlay never boots. +/// This mirrors the login-time fail-closed guard in and is +/// gated on the same conditions ( AND not +/// ): 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. +/// public sealed class LdapOptionsValidator : OptionsValidatorBase { /// @@ -32,5 +39,13 @@ public sealed class LdapOptionsValidator : OptionsValidatorBase 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."); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.admin-driver.json b/src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.admin-driver.json index 3a61aed8..c4cc21cb 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.admin-driver.json +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.admin-driver.json @@ -11,7 +11,8 @@ }, "Security": { "Ldap": { - "DevStubMode": false + "DevStubMode": false, + "Transport": "Ldaps" } } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.admin.json b/src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.admin.json index bc78f171..2c86e16f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.admin.json +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.admin.json @@ -10,7 +10,8 @@ }, "Security": { "Ldap": { - "DevStubMode": false + "DevStubMode": false, + "Transport": "Ldaps" } } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.driver.json b/src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.driver.json index 33022922..1db8bd01 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.driver.json +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.driver.json @@ -10,7 +10,8 @@ }, "Security": { "Ldap": { - "DevStubMode": false + "DevStubMode": false, + "Transport": "Ldaps" } } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Endpoints/AuthEndpoints.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Endpoints/AuthEndpoints.cs index 43de033e..36e67cc5 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Endpoints/AuthEndpoints.cs +++ b/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()? .CreateLogger("ZB.MOM.WW.OtOpcUa.Security.AuthEndpoints") .LogWarning(ex, "Role-map lookup failed for {User}; using pre-resolved baseline roles", username); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsBindingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsBindingTests.cs index 8ff24c04..c18b8d1e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsBindingTests.cs +++ b/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(); } } + +/// +/// End-to-end guard for the shipped production overlays: binds each of the three prod overlay +/// files' real Security:Ldap section (the same files the host loads at boot, copied into the +/// test output via the Host project reference) and runs the the +/// host wires via AddValidatedOptions. 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 Development overlay (DevStubMode) is verified to +/// pass via the guard exemption. +/// +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() ?? 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(); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsValidatorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsValidatorTests.cs index 1de27607..058f6ff1 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsValidatorTests.cs +++ b/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; /// ZB.MOM.WW.Configuration OptionsValidatorBase/ValidationBuilder) gates on /// , and that when enabled it requires Server, /// SearchBase, and a valid Port. Failure messages carry the real "Ldap:" -/// 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 +/// fails fast at boot. /// public sealed class LdapOptionsValidatorTests { private static readonly LdapOptionsValidator Sut = new(); - /// Valid enabled options pass validation. + private const string InsecureTransportFailure = + "LDAP transport is None (plaintext) but AllowInsecure is false — set Transport to Ldaps/StartTls or set AllowInsecure for dev."; + + /// Valid enabled options (a TLS transport) pass validation. [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(); + } + + /// + /// Insecure-transport guard: an enabled real-LDAP config that selects plaintext + /// without fails + /// startup validation with the guard message. + /// + [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); + } + + /// A TLS transport () satisfies the guard. + [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(); + } + + /// + /// Explicit opt-in: plaintext transport with set is + /// permitted (dev/test escape hatch), so the guard does not trip. + /// + [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(); + } + + /// + /// 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. + /// + [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(); + } + + /// + /// A disabled config is exempt from the insecure-transport guard even with plaintext + /// transport — LDAP login never runs, so the guard must not trip. + /// + [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(); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/AuthEndpointsIntegrationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/AuthEndpointsIntegrationTests.cs index 3e84cf4d..5fc22fd7 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/AuthEndpointsIntegrationTests.cs +++ b/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(); @@ -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 } - /// When the DB role-map lookup throws, sign-in still succeeds with the appsettings - /// baseline roles — a DB hiccup must never block login. + /// 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. [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(Ct); var roles = JwtRoleClaims(payload.GetProperty("token").GetString()!); - roles.ShouldContain("ConfigViewer"); // baseline still present + roles.ShouldBeEmpty(); } /// Extracts the "Role" claim values from a JWT's payload segment. @@ -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,