From 15752f8c2d11475e1c688e0b2037a6006ac21be4 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 3 Jun 2026 13:06:41 -0400 Subject: [PATCH] fix(security): make auth cookie name configurable, override per env The auth cookie name was hardcoded to ZB.MOM.WW.ScadaBridge.Auth. Because browser cookies are scoped by host+path but NOT by port, two ScadaBridge clusters on the same host (the local docker stack on localhost:9000 and docker-env2 on localhost:9100) shared one cookie jar: signing into one overwrote the other's cookie, and since the clusters use different JWT signing keys + separate Data Protection key rings, the overwritten side could no longer validate its cookie and the session died. Add SecurityOptions.CookieName (default = canonical ZB.MOM.WW.ScadaBridge.Auth, blank falls back to the default) applied via the SecurityOptions-bound cookie PostConfigure. Override it to ...Auth.env2 in both docker-env2 Central nodes so the two local clusters no longer collide; the primary cluster keeps the default so its live sessions and production are unaffected. Adds 3 Security.Tests cases. --- .../central-node-a/appsettings.Central.json | 3 +- .../central-node-b/appsettings.Central.json | 3 +- .../SecurityOptions.cs | 14 ++++ .../ServiceCollectionExtensions.cs | 33 ++++++---- .../SecurityTests.cs | 66 +++++++++++++++++++ 5 files changed, 105 insertions(+), 14 deletions(-) diff --git a/docker-env2/central-node-a/appsettings.Central.json b/docker-env2/central-node-a/appsettings.Central.json index a082186f..a5383bd0 100644 --- a/docker-env2/central-node-a/appsettings.Central.json +++ b/docker-env2/central-node-a/appsettings.Central.json @@ -34,7 +34,8 @@ "JwtSigningKey": "scadabridge-env2-dev-jwt-signing-key-must-be-at-least-32-characters-long", "JwtExpiryMinutes": 15, "IdleTimeoutMinutes": 30, - "RequireHttpsCookie": false + "RequireHttpsCookie": false, + "CookieName": "ZB.MOM.WW.ScadaBridge.Auth.env2" }, "Communication": { "DeploymentTimeout": "00:02:00", diff --git a/docker-env2/central-node-b/appsettings.Central.json b/docker-env2/central-node-b/appsettings.Central.json index 44968208..37076f67 100644 --- a/docker-env2/central-node-b/appsettings.Central.json +++ b/docker-env2/central-node-b/appsettings.Central.json @@ -34,7 +34,8 @@ "JwtSigningKey": "scadabridge-env2-dev-jwt-signing-key-must-be-at-least-32-characters-long", "JwtExpiryMinutes": 15, "IdleTimeoutMinutes": 30, - "RequireHttpsCookie": false + "RequireHttpsCookie": false, + "CookieName": "ZB.MOM.WW.ScadaBridge.Auth.env2" }, "Communication": { "DeploymentTimeout": "00:02:00", diff --git a/src/ZB.MOM.WW.ScadaBridge.Security/SecurityOptions.cs b/src/ZB.MOM.WW.ScadaBridge.Security/SecurityOptions.cs index 34c19d22..05dc9fd4 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Security/SecurityOptions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Security/SecurityOptions.cs @@ -44,4 +44,18 @@ public class SecurityOptions /// any HTTPS request but is usable over plain HTTP. /// public bool RequireHttpsCookie { get; set; } = true; + + /// The canonical default authentication-cookie name (). + public const string DefaultCookieName = "ZB.MOM.WW.ScadaBridge.Auth"; + + /// + /// Authentication cookie name. Defaults to . Override it + /// (ScadaBridge:Security:CookieName) to give a distinct name to a deployment that + /// shares a hostname with another ScadaBridge environment — browser cookies are scoped by + /// host+path but NOT by port, so two clusters on the same host (e.g. two local Docker + /// stacks on localhost:9000 and localhost:9100) would otherwise clobber each + /// other's session under a shared cookie name. A blank/whitespace value falls back to + /// . Changing this invalidates existing sessions on next deploy. + /// + public string CookieName { get; set; } = DefaultCookieName; } diff --git a/src/ZB.MOM.WW.ScadaBridge.Security/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.Security/ServiceCollectionExtensions.cs index 4106c1ac..bd7fbab7 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Security/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Security/ServiceCollectionExtensions.cs @@ -77,23 +77,23 @@ public static class ServiceCollectionExtensions // now enforces Server + SearchBase + ServiceAccountDn + transport at startup. The // JWT signing key continues to fail-fast at JwtTokenService construction. - // Register ASP.NET Core authentication with cookie scheme. The non- - // SecurityOptions-coupled settings (paths, cookie name) are set here; the - // hardened cookie defaults that depend on SecurityOptions (idle timeout, - // HTTPS policy) are applied via the SecurityOptions-bound PostConfigure - // below through ZbCookieDefaults.Apply. + // Register ASP.NET Core authentication with cookie scheme. Only the static + // SecurityOptions-independent settings (login/logout paths) are set here; the + // cookie NAME and the hardened defaults that depend on SecurityOptions (idle + // timeout, HTTPS policy) are applied via the SecurityOptions-bound PostConfigure + // below (cookie name through SecurityOptions.CookieName, the rest through + // ZbCookieDefaults.Apply). services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme) .AddCookie(options => { options.LoginPath = "/login"; options.LogoutPath = "/auth/logout"; - // The cookie NAME is app-owned and not set by ZbCookieDefaults.Apply - // (so co-hosted ZB apps do not clobber each other's session). Keep - // ScadaBridge's existing name so live sessions survive this change. - options.Cookie.Name = "ZB.MOM.WW.ScadaBridge.Auth"; - // HttpOnly / SameSite / SecurePolicy / SlidingExpiration / - // ExpireTimeSpan are all set by ZbCookieDefaults.Apply in the - // SecurityOptions-bound PostConfigure below. + // Cookie.Name is app-owned (not set by ZbCookieDefaults.Apply, so co-hosted + // ZB apps do not clobber each other) AND now config-driven — it is set from + // SecurityOptions.CookieName in the PostConfigure below so two ScadaBridge + // environments sharing a hostname can be given distinct names. HttpOnly / + // SameSite / SecurePolicy / SlidingExpiration / ExpireTimeSpan are likewise + // applied there via ZbCookieDefaults.Apply. }); // CentralUI-005: configure the cookie session as a sliding window so the @@ -124,6 +124,15 @@ public static class ServiceCollectionExtensions requireHttps: securityOptions.Value.RequireHttpsCookie, idleTimeout: TimeSpan.FromMinutes(securityOptions.Value.IdleTimeoutMinutes)); + // App-owned, config-driven cookie name (ScadaBridge:Security:CookieName). + // ZbCookieDefaults.Apply intentionally leaves the name untouched. A + // blank/whitespace value falls back to the canonical default so a + // misconfiguration cannot produce an unnamed cookie. + var cookieName = securityOptions.Value.CookieName; + cookieOptions.Cookie.Name = string.IsNullOrWhiteSpace(cookieName) + ? SecurityOptions.DefaultCookieName + : cookieName; + // Security-021: when the operator opts out of HTTPS-only cookies, // log a Warning so an HTTP-only deployment is at least audible in // the startup log. The cookie carries the embedded JWT bearer diff --git a/tests/ZB.MOM.WW.ScadaBridge.Security.Tests/SecurityTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Security.Tests/SecurityTests.cs index d4e305a7..72037388 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Security.Tests/SecurityTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Security.Tests/SecurityTests.cs @@ -515,6 +515,72 @@ public class SecurityReviewRegressionTests Assert.True(cookieOptions.SlidingExpiration); } + // --- Configurable cookie name: two ScadaBridge environments sharing a hostname + // (browser cookies are scoped by host+path, NOT port) must be able to use + // distinct cookie names so signing into one does not clobber the other's session. --- + + [Fact] + public void AddSecurity_AuthCookie_DefaultsToCanonicalName() + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddDataProtection(); + services.AddSecurity(); + + using var provider = services.BuildServiceProvider(); + var cookieOptions = provider + .GetRequiredService>() + .Get(Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme); + + // Unconfigured deployments (incl. production and the primary docker cluster) keep the + // canonical name, so existing sessions survive and only an explicit override diverges. + Assert.Equal(SecurityOptions.DefaultCookieName, cookieOptions.Cookie.Name); + Assert.Equal("ZB.MOM.WW.ScadaBridge.Auth", cookieOptions.Cookie.Name); + } + + [Fact] + public void AddSecurity_AuthCookie_CookieNameIsConfigurable() + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddDataProtection(); + services.AddSecurity(); + // The per-environment override the docker-env2 cluster uses to avoid clobbering the + // primary cluster's cookie on localhost. + services.Configure(o => o.CookieName = "ZB.MOM.WW.ScadaBridge.Auth.env2"); + + using var provider = services.BuildServiceProvider(); + var cookieOptions = provider + .GetRequiredService>() + .Get(Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme); + + Assert.Equal("ZB.MOM.WW.ScadaBridge.Auth.env2", cookieOptions.Cookie.Name); + // The name override must be purely additive — it must not reset the hardened defaults. + Assert.True(cookieOptions.Cookie.HttpOnly); + Assert.Equal(Microsoft.AspNetCore.Http.SameSiteMode.Strict, cookieOptions.Cookie.SameSite); + Assert.True(cookieOptions.SlidingExpiration); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + public void AddSecurity_AuthCookie_BlankCookieName_FallsBackToDefault(string blank) + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddDataProtection(); + services.AddSecurity(); + services.Configure(o => o.CookieName = blank); + + using var provider = services.BuildServiceProvider(); + var cookieOptions = provider + .GetRequiredService>() + .Get(Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme); + + // A blank value must never produce an unnamed cookie. + Assert.Equal(SecurityOptions.DefaultCookieName, cookieOptions.Cookie.Name); + } + // --- Security-001: transport security (now owned by the shared LdapOptions) --- [Fact]