fix(auth): move AddZbLdapAuth to Host composition root so component-lib AddSecurity() drops IConfiguration param (satisfy OptionsTests arch rule; fix pre-existing ac34dac red); behaviour-preserving

This commit is contained in:
Joseph Doherty
2026-06-02 03:50:16 -04:00
parent 7e25efa790
commit 55099b19f6
3 changed files with 55 additions and 34 deletions
+11 -1
View File
@@ -1,4 +1,5 @@
using ZB.MOM.WW.Auth.ApiKeys.DependencyInjection; using ZB.MOM.WW.Auth.ApiKeys.DependencyInjection;
using ZB.MOM.WW.Auth.AspNetCore;
using ZB.MOM.WW.Health; using ZB.MOM.WW.Health;
using ZB.MOM.WW.Health.Akka; using ZB.MOM.WW.Health.Akka;
using ZB.MOM.WW.Health.EntityFrameworkCore; using ZB.MOM.WW.Health.EntityFrameworkCore;
@@ -104,7 +105,16 @@ try
builder.Services.AddSiteCallAudit(); builder.Services.AddSiteCallAudit();
builder.Services.AddTemplateEngine(); builder.Services.AddTemplateEngine();
builder.Services.AddDeploymentManager(); builder.Services.AddDeploymentManager();
builder.Services.AddSecurity(builder.Configuration); // Host is the composition root and owns config-coupled wiring: register the
// shared LDAP auth (binds LdapOptions + IValidateOptions<LdapOptions> with
// ValidateOnStart + ILdapAuthService singleton) here, then AddSecurity() for the
// config-free remainder. AddSecurity is a component library and takes no
// IConfiguration (Options pattern only). Behaviour-preserving: identical
// registrations to the previous AddSecurity(builder.Configuration) call.
builder.Services.AddZbLdapAuth(
builder.Configuration,
ZB.MOM.WW.ScadaBridge.Security.ServiceCollectionExtensions.LdapSectionPath);
builder.Services.AddSecurity();
builder.Services.AddCentralUI(); builder.Services.AddCentralUI();
builder.Services.AddInboundAPI(); builder.Services.AddInboundAPI();
@@ -1,5 +1,4 @@
using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Authentication.Cookies;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options; using Microsoft.Extensions.Options;
@@ -15,29 +14,37 @@ public static class ServiceCollectionExtensions
/// under the existing <c>ScadaBridge:Security</c> section as a <c>Ldap</c> sub-section /// under the existing <c>ScadaBridge:Security</c> section as a <c>Ldap</c> sub-section
/// (Task 1.4 config rename) so the non-LDAP <see cref="SecurityOptions"/> fields stay /// (Task 1.4 config rename) so the non-LDAP <see cref="SecurityOptions"/> fields stay
/// where they are while the LDAP connection settings bind to the shared library. /// where they are while the LDAP connection settings bind to the shared library.
/// The Host composition root references this constant to register the shared LDAP
/// auth (<c>AddZbLdapAuth</c>) before calling <see cref="AddSecurity"/>.
/// </summary> /// </summary>
public const string LdapSectionPath = "ScadaBridge:Security:Ldap"; public const string LdapSectionPath = "ScadaBridge:Security:Ldap";
/// <summary> /// <summary>
/// Registers LDAP authentication (shared <c>ZB.MOM.WW.Auth.Ldap</c>), JWT token service, /// Registers the JWT token service, role mapper, cookie authentication, and
/// role mapper, cookie authentication, and authorization policies. /// authorization policies. This is a component library and therefore takes no
/// <c>IConfiguration</c> (Options pattern only).
/// </summary> /// </summary>
/// <remarks>
/// LDAP authentication (shared <c>ZB.MOM.WW.Auth.Ldap</c> via <c>AddZbLdapAuth</c>) is
/// registered by the Host composition root — which calls <c>AddZbLdapAuth</c> with the
/// nested <see cref="LdapSectionPath"/> <em>before</em> this method — because that
/// registration is config-coupled (it binds <c>LdapOptions</c> from
/// <c>IConfiguration</c>) and component libraries must not accept <c>IConfiguration</c>.
/// </remarks>
/// <param name="services">The service collection to register into.</param> /// <param name="services">The service collection to register into.</param>
/// <param name="configuration"> public static IServiceCollection AddSecurity(this IServiceCollection services)
/// Application configuration, read for the nested <see cref="LdapSectionPath"/> LDAP
/// options bound + validated by <c>AddZbLdapAuth</c>.
/// </param>
public static IServiceCollection AddSecurity(this IServiceCollection services, IConfiguration configuration)
{ {
// Task 1.2 cutover: replace ScadaBridge's bespoke LdapAuthService with the shared // Task 1.2 cutover: ScadaBridge's bespoke LdapAuthService was replaced by the
// ZB.MOM.WW.Auth.Ldap implementation (ScadaBridge was the donor for its hardened // shared ZB.MOM.WW.Auth.Ldap implementation (ScadaBridge was the donor for its
// bind-then-search / escaping / fail-closed semantics, so this is a behaviour- // hardened bind-then-search / escaping / fail-closed semantics, so this was a
// equivalent re-point). AddZbLdapAuth binds LdapOptions from the nested Ldap // behaviour-equivalent re-point). That registration — AddZbLdapAuth, which binds
// sub-section, registers IValidateOptions<LdapOptions> with ValidateOnStart (so a // LdapOptions from the nested Ldap sub-section, registers IValidateOptions<LdapOptions>
// misconfigured directory fails fast at boot — superseding the old // with ValidateOnStart (so a misconfigured directory fails fast at boot —
// SecurityOptionsValidator LDAP checks), and registers ILdapAuthService as a // superseding the old SecurityOptionsValidator LDAP checks), and registers
// stateless singleton. // ILdapAuthService as a stateless singleton — now lives at the Host composition
services.AddZbLdapAuth(configuration, LdapSectionPath); // root (which calls AddZbLdapAuth(configuration, LdapSectionPath) immediately
// before AddSecurity()), because it is config-coupled and this is a component
// library.
services.AddScoped<JwtTokenService>(); services.AddScoped<JwtTokenService>();
services.AddScoped<RoleMapper>(); services.AddScoped<RoleMapper>();
@@ -6,6 +6,7 @@ using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options; using Microsoft.Extensions.Options;
using ZB.MOM.WW.Auth.Abstractions.Ldap; using ZB.MOM.WW.Auth.Abstractions.Ldap;
using ZB.MOM.WW.Auth.AspNetCore;
using ZB.MOM.WW.Auth.Ldap; using ZB.MOM.WW.Auth.Ldap;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Security; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Security;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
@@ -441,7 +442,7 @@ public class SecurityReviewRegressionTests
var services = new ServiceCollection(); var services = new ServiceCollection();
services.AddLogging(); services.AddLogging();
services.AddDataProtection(); services.AddDataProtection();
services.AddSecurity(EmptyConfig()); services.AddSecurity();
using var provider = services.BuildServiceProvider(); using var provider = services.BuildServiceProvider();
var cookieOptions = provider var cookieOptions = provider
@@ -454,16 +455,6 @@ public class SecurityReviewRegressionTests
Assert.True(cookieOptions.Cookie.HttpOnly); Assert.True(cookieOptions.Cookie.HttpOnly);
} }
/// <summary>
/// Task 1.2: an empty <see cref="Microsoft.Extensions.Configuration.IConfiguration"/>
/// for <c>AddSecurity(config)</c>. The cookie PostConfigure under test reads only the
/// non-LDAP <see cref="SecurityOptions"/> fields (idle timeout / HTTPS-cookie policy),
/// and the library's <c>LdapOptions</c> ValidateOnStart only fires at host start (not on
/// <c>BuildServiceProvider</c>), so no LDAP config is needed to resolve the cookie wiring.
/// </summary>
private static Microsoft.Extensions.Configuration.IConfiguration EmptyConfig() =>
new Microsoft.Extensions.Configuration.ConfigurationBuilder().Build();
// --- CentralUI-005: cookie auth must use a sliding session window --- // --- CentralUI-005: cookie auth must use a sliding session window ---
// Documented policy (CLAUDE.md Security & Auth): sliding refresh with a // Documented policy (CLAUDE.md Security & Auth): sliding refresh with a
// 30-minute idle timeout. The cookie middleware must enable SlidingExpiration // 30-minute idle timeout. The cookie middleware must enable SlidingExpiration
@@ -475,7 +466,7 @@ public class SecurityReviewRegressionTests
var services = new ServiceCollection(); var services = new ServiceCollection();
services.AddLogging(); services.AddLogging();
services.AddDataProtection(); services.AddDataProtection();
services.AddSecurity(EmptyConfig()); services.AddSecurity();
using var provider = services.BuildServiceProvider(); using var provider = services.BuildServiceProvider();
var cookieOptions = provider var cookieOptions = provider
@@ -491,7 +482,7 @@ public class SecurityReviewRegressionTests
var services = new ServiceCollection(); var services = new ServiceCollection();
services.AddLogging(); services.AddLogging();
services.AddDataProtection(); services.AddDataProtection();
services.AddSecurity(EmptyConfig()); services.AddSecurity();
// The idle timeout drives the cookie's expiry window. // The idle timeout drives the cookie's expiry window.
services.Configure<SecurityOptions>(o => o.IdleTimeoutMinutes = 30); services.Configure<SecurityOptions>(o => o.IdleTimeoutMinutes = 30);
@@ -509,7 +500,7 @@ public class SecurityReviewRegressionTests
var services = new ServiceCollection(); var services = new ServiceCollection();
services.AddLogging(); services.AddLogging();
services.AddDataProtection(); services.AddDataProtection();
services.AddSecurity(EmptyConfig()); services.AddSecurity();
services.Configure<SecurityOptions>(o => o.IdleTimeoutMinutes = 45); services.Configure<SecurityOptions>(o => o.IdleTimeoutMinutes = 45);
using var provider = services.BuildServiceProvider(); using var provider = services.BuildServiceProvider();
@@ -1179,14 +1170,27 @@ public class SecurityOptionsValidatorTests
Assert.Contains(nameof(LdapOptions.ServiceAccountDn), result.FailureMessage); Assert.Contains(nameof(LdapOptions.ServiceAccountDn), result.FailureMessage);
} }
/// <summary>
/// Verifies the security composition the Host performs at its composition root —
/// <c>AddZbLdapAuth(configuration, LdapSectionPath)</c> followed by <c>AddSecurity()</c> —
/// wires the shared <see cref="LdapOptionsValidator"/> as an
/// <see cref="IValidateOptions{TOptions}"/> for <c>LdapOptions</c> (which is what makes
/// <c>ValidateOnStart()</c> fire). The LDAP registration moved to the Host because it is
/// config-coupled; <c>AddSecurity()</c> is a component library and no longer takes
/// <c>IConfiguration</c>.
/// </summary>
[Fact] [Fact]
public void AddSecurity_RegistersLdapOptionsValidator_WithValidateOnStart() public void AddSecurity_RegistersLdapOptionsValidator_WithValidateOnStart()
{ {
var services = new ServiceCollection(); var services = new ServiceCollection();
services.AddLogging(); services.AddLogging();
services.AddDataProtection(); services.AddDataProtection();
services.AddSecurity( // Mirror the Host composition order: shared LDAP auth (config-coupled) first,
new Microsoft.Extensions.Configuration.ConfigurationBuilder().Build()); // then the config-free AddSecurity().
services.AddZbLdapAuth(
new Microsoft.Extensions.Configuration.ConfigurationBuilder().Build(),
ZB.MOM.WW.ScadaBridge.Security.ServiceCollectionExtensions.LdapSectionPath);
services.AddSecurity();
using var provider = services.BuildServiceProvider(); using var provider = services.BuildServiceProvider();