From 3de688f8d61278432a43e00a90680b81324f3a19 Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Fri, 22 May 2026 06:27:11 -0400
Subject: [PATCH] fix(admin): resolve High code-review findings (Admin-003,
 Admin-004, Admin-005)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Admin-003 — SignalR hubs were anonymously reachable: an unauthenticated
client could open /hubs/fleet, /hubs/alerts and /hubs/script-log and
stream fleet state, alert detail text and server script-log contents.
Added [Authorize] to FleetStatusHub, AlertHub and ScriptLogHub, and
chained .RequireAuthorization() onto all three MapHub() calls as a
belt-and-braces backstop.

Admin-004 — appsettings.json committed live-looking secrets (the `sa`
ConfigDb password and the LDAP ServiceAccountPassword) in plaintext.
Replaced both with empty placeholders sourced from user-secrets (dev) or
the ConnectionStrings__ConfigDb / Authentication__Ldap__ServiceAccountPassword
environment variables (prod); added a UserSecretsId to the Admin csproj
and a fail-fast guard in Program.cs when ConfigDb is empty/missing.

Admin-005 — Login.razor performed SignInAsync from an interactive Blazor
circuit, where the original HTTP response has long completed so the auth
cookie was not emitted. Rewrote it as a static-rendered plain HTML form
(data-enhance="false") posting to a new AuthEndpoints.MapAuthEndpoints()
minimal-API handler (/auth/login, /auth/logout) that does the LDAP bind,
grant resolution, cookie SignInAsync and redirect while the endpoint
still owns the response. Includes an open-redirect guard on returnUrl.

Added xUnit + Shouldly regression tests: AuthEndpointsTests (login cookie
issuance, failed-bind redirect, open-redirect rejection, logout, anonymous
hub negotiate rejection) and AppSettingsSecretHygieneTests (no committed
secrets). All 26 auth-related tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 code-reviews/Admin/findings.md                |  14 +-
 .../Components/Pages/Login.razor              | 114 +++--------
 .../ZB.MOM.WW.OtOpcUa.Admin/Hubs/AlertHub.cs  |   6 +
 .../Hubs/FleetStatusHub.cs                    |   7 +
 .../Hubs/ScriptLogHub.cs                      |   6 +
 src/Server/ZB.MOM.WW.OtOpcUa.Admin/Program.cs |  36 ++--
 .../Security/AuthEndpoints.cs                 | 101 ++++++++++
 .../ZB.MOM.WW.OtOpcUa.Admin.csproj            |   3 +
 .../ZB.MOM.WW.OtOpcUa.Admin/appsettings.json  |   5 +-
 .../AppSettingsSecretHygieneTests.cs          |  79 ++++++++
 .../AuthEndpointsTests.cs                     | 184 ++++++++++++++++++
 11 files changed, 447 insertions(+), 108 deletions(-)
 create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.Admin/Security/AuthEndpoints.cs
 create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Admin.Tests/AppSettingsSecretHygieneTests.cs
 create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Admin.Tests/AuthEndpointsTests.cs

diff --git a/code-reviews/Admin/findings.md b/code-reviews/Admin/findings.md
index 57a8645..1833584 100644
--- a/code-reviews/Admin/findings.md
+++ b/code-reviews/Admin/findings.md
@@ -7,7 +7,7 @@
 | Review date | 2026-05-22 |
 | Commit reviewed | `76d35d1` |
 | Status | Reviewed |
-| Open findings | 10 |
+| Open findings | 7 |
 
 ## Checklist coverage
 
@@ -63,13 +63,13 @@
 | Severity | High |
 | Category | Security |
 | Location | `Program.cs:137-139`, `Hubs/FleetStatusHub.cs:11`, `Hubs/AlertHub.cs:10`, `Hubs/ScriptLogHub.cs:30` |
-| Status | Open |
+| Status | Resolved |
 
 **Description:** All three SignalR hubs (`/hubs/fleet`, `/hubs/alerts`, `/hubs/script-log`) are mapped with no `[Authorize]` attribute and no `.RequireAuthorization()` on the `MapHub` call. Any unauthenticated client can open a hub connection: `FleetStatusHub.SubscribeFleet()` streams every node generation/role/resilience state, `AlertHub` pushes all fleet alerts (including failure detail text), and `ScriptLogHub.TailLogAsync` streams the contents of the server `scripts-*.log` files. This is an unauthenticated information-disclosure channel that bypasses the (already broken — see Admin-001) page auth entirely.
 
 **Recommendation:** Add `[Authorize]` to each `Hub` class, or chain `.RequireAuthorization()` onto each `MapHub(...)` call in `Program.cs`. The hub `SubscribeCluster`/`TailLogAsync` methods should additionally validate that the caller claims permit the requested cluster/script scope.
 
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-22 — `[Authorize]` added to `FleetStatusHub`, `AlertHub` and `ScriptLogHub`, and `.RequireAuthorization()` chained onto all three `MapHub(...)` calls in `Program.cs` as a belt-and-braces backstop, so an anonymous client can no longer open any hub connection. Covered by `AuthEndpointsTests.Anonymous_hub_negotiate_is_rejected`.
 
 ### Admin-004
 
@@ -78,13 +78,13 @@
 | Severity | High |
 | Category | Security |
 | Location | `appsettings.json:3,13-14` |
-| Status | Open |
+| Status | Resolved |
 
 **Description:** The checked-in `appsettings.json` contains live-looking secrets in plaintext: the `ConfigDb` connection string with `User Id=sa;Password=OtOpcUaDev_2026!` and the LDAP `ServiceAccountPassword: "serviceaccount123"`. It also sets `Encrypt=False` and `AllowInsecureLdap: true`, so the SQL and LDAP credentials travel unencrypted on the wire. Committing the `sa` account password and a service-account password to source control is a credential-exposure risk; `sa` additionally grants full server control, conflicting with the `ClusterService` doc comment that production should connect with a least-privilege grant.
 
 **Recommendation:** Move all secrets out of the committed file — use user-secrets for dev and environment variables / a secret store for production; leave only non-secret placeholders in `appsettings.json`. Use a least-privilege SQL login rather than `sa`. Enable TLS for both SQL (`Encrypt=True`) and LDAP (`UseTls=true`, `AllowInsecureLdap=false`) for any non-loopback deployment, and document the dev-only exception.
 
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-22 — the `sa` connection string and the LDAP `ServiceAccountPassword` were replaced with empty placeholders in `appsettings.json`; a `_secrets` note documents that they are supplied via user-secrets (dev) or the `ConnectionStrings__ConfigDb` / `Authentication__Ldap__ServiceAccountPassword` environment variables (prod), and that the connection string must use `Encrypt=True` and a least-privilege SQL login. A `UserSecretsId` was added to the Admin csproj, and `Program.cs` now fails fast with a clear message when `ConfigDb` is empty/missing. Covered by `AppSettingsSecretHygieneTests`.
 
 ### Admin-005
 
@@ -93,13 +93,13 @@
 | Severity | High |
 | Category | Correctness & logic bugs |
 | Location | `Components/Pages/Login.razor:15,107-110` |
-| Status | Open |
+| Status | Resolved |
 
 **Description:** `Login.razor` is an interactive component (the project default render mode is interactive server; the page declares no `@rendermode` but uses `EditForm`/`InputText` interactive binding and runs `SignInAsync` from an event handler). It calls `HttpContext.SignInAsync(...)` followed by `ctx.Response.Redirect("/")` from within a SignalR circuit callback. Writing auth cookies and HTTP redirect headers requires a live, unstarted HTTP response; in an interactive circuit the original HTTP response has long completed, so the cookie is typically not emitted and the redirect is ineffective (or throws "response has already started"). `admin-ui.md` section "Operator authentication" explicitly specifies the login as a static server-rendered HTML form POSTing to a `/auth/login` minimal-API endpoint with `data-enhance="false"` — that endpoint is not implemented and is not mapped in `Program.cs`.
 
 **Recommendation:** Implement the login as designed: a static-rendered form (`@rendermode` none, `data-enhance="false"`) posting to a `MapPost("/auth/login", ...)` minimal-API handler that does the LDAP bind, grant resolution, `SignInAsync` and redirect while the HTTP response is still owned by the endpoint. Do not perform `SignInAsync` from an interactive circuit.
 
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-22 — `Login.razor` rewritten as a static-rendered plain HTML `<form method="post" action="/auth/login" data-enhance="false">` (no `@rendermode`, no `EditForm`/`SignInAsync` in a circuit); the LDAP bind, grant resolution, cookie `SignInAsync` and redirect now run in a new `AuthEndpoints.MapAuthEndpoints()` minimal-API handler (`/auth/login`, `/auth/logout`) while the endpoint still owns the HTTP response. The handler is `AllowAnonymous`, carries an open-redirect guard on `returnUrl`, and surfaces bind errors back to the login page via a query-string. Covered by `AuthEndpointsTests` (valid login issues the cookie, invalid login redirects with error, open-redirect rejected, logout clears the cookie).
 
 ### Admin-006
 
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Login.razor b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Login.razor
index cec8f2a..b0e6b30 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Login.razor
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Login.razor
@@ -2,38 +2,40 @@
 @* The login page must stay anonymously reachable — otherwise the fallback
    authorization policy (Admin-001) would lock operators out of the only way in. *@
 @attribute [Microsoft.AspNetCore.Authorization.AllowAnonymous]
-@using System.Security.Claims
-@using Microsoft.AspNetCore.Authentication
-@using Microsoft.AspNetCore.Authentication.Cookies
-@using ZB.MOM.WW.OtOpcUa.Admin.Security
-@inject IHttpContextAccessor Http
-@inject ILdapAuthService LdapAuth
-@inject IAdminRoleGrantResolver GrantResolver
-@inject NavigationManager Nav
+
+@* Admin-005: this page is static server-rendered (no @rendermode). It is a plain HTML
+   form that POSTs to the /auth/login minimal-API endpoint with data-enhance="false", so
+   the LDAP bind, cookie SignInAsync and redirect all run while the endpoint still owns
+   an unstarted HTTP response. SignInAsync must NOT be called from an interactive Blazor
+   circuit — by then the original HTTP response has long completed. *@
 
 <div class="login-wrap rise" style="animation-delay:.02s">
     <section class="panel">
         <div class="panel-head">OtOpcUa Admin &mdash; sign in</div>
         <div style="padding:1.1rem 1.1rem 1.25rem">
-            <EditForm Model="_input" OnValidSubmit="SignInAsync" FormName="login">
+            <form method="post" action="/auth/login" data-enhance="false">
+                @if (ReturnUrl is not null)
+                {
+                    <input type="hidden" name="returnUrl" value="@ReturnUrl"/>
+                }
                 <div class="mb-3">
-                    <label class="form-label">Username</label>
-                    <InputText @bind-Value="_input.Username" class="form-control form-control-sm" autocomplete="username"/>
+                    <label class="form-label" for="username">Username</label>
+                    <input id="username" name="username" type="text"
+                           class="form-control form-control-sm" autocomplete="username"/>
                 </div>
                 <div class="mb-3">
-                    <label class="form-label">Password</label>
-                    <InputText type="password" @bind-Value="_input.Password" class="form-control form-control-sm" autocomplete="current-password"/>
+                    <label class="form-label" for="password">Password</label>
+                    <input id="password" name="password" type="password"
+                           class="form-control form-control-sm" autocomplete="current-password"/>
                 </div>
 
-                @if (_error is not null)
+                @if (!string.IsNullOrWhiteSpace(Error))
                 {
-                    <div class="panel notice" style="margin-bottom:.85rem">@_error</div>
+                    <div class="panel notice" style="margin-bottom:.85rem">@Error</div>
                 }
 
-                <button class="btn btn-primary w-100" type="submit" disabled="@_busy">
-                    @(_busy ? "Signing in…" : "Sign in")
-                </button>
-            </EditForm>
+                <button class="btn btn-primary w-100" type="submit">Sign in</button>
+            </form>
 
             <div style="margin-top:1rem;padding-top:.85rem;border-top:1px solid var(--rule);
                         font-size:.78rem;color:var(--ink-faint)">
@@ -45,73 +47,11 @@
 </div>
 
 @code {
-    private sealed class Input
-    {
-        public string Username { get; set; } = string.Empty;
-        public string Password { get; set; } = string.Empty;
-    }
+    /// <summary>Error message surfaced by the /auth/login endpoint after a failed bind.</summary>
+    [SupplyParameterFromQuery]
+    private string? Error { get; set; }
 
-    // Static-SSR form post: the model must be [SupplyParameterFromForm] or the
-    // submitted username/password never bind back onto _input. The property
-    // cannot carry an initializer (BL0008) — seed it in OnInitialized instead.
-    [SupplyParameterFromForm]
-    private Input _input { get; set; } = default!;
-
-    private string? _error;
-    private bool _busy;
-
-    protected override void OnInitialized() => _input ??= new();
-
-    private async Task SignInAsync()
-    {
-        _error = null;
-        _busy = true;
-        try
-        {
-            if (string.IsNullOrWhiteSpace(_input.Username) || string.IsNullOrWhiteSpace(_input.Password))
-            {
-                _error = "Username and password are required";
-                return;
-            }
-
-            var result = await LdapAuth.AuthenticateAsync(_input.Username, _input.Password, CancellationToken.None);
-            if (!result.Success)
-            {
-                _error = result.Error ?? "Sign-in failed";
-                return;
-            }
-
-            // Resolve grants from the static bootstrap dictionary + DB-backed role grants.
-            // result.Roles (static-only) is intentionally not consulted here.
-            var grants = await GrantResolver.ResolveAsync(result.Groups, CancellationToken.None);
-            if (grants.IsEmpty)
-            {
-                _error = "Sign-in succeeded but no Admin roles mapped for your LDAP groups. Contact your administrator.";
-                return;
-            }
-
-            var ctx = Http.HttpContext
-                ?? throw new InvalidOperationException("HttpContext unavailable at sign-in");
-
-            var claims = new List<Claim>
-            {
-                new(ClaimTypes.Name, result.DisplayName ?? result.Username ?? _input.Username),
-                new(ClaimTypes.NameIdentifier, _input.Username),
-            };
-            foreach (var role in grants.FleetRoles)
-                claims.Add(new Claim(ClaimTypes.Role, role));
-            foreach (var clusterGrant in grants.ClusterRoles)
-                claims.Add(new Claim(ClusterRoleClaims.ClaimType,
-                    ClusterRoleClaims.Encode(clusterGrant.ClusterId, clusterGrant.Role)));
-            foreach (var group in result.Groups)
-                claims.Add(new Claim("ldap_group", group));
-
-            var identity = new ClaimsIdentity(claims, CookieAuthenticationDefaults.AuthenticationScheme);
-            await ctx.SignInAsync(CookieAuthenticationDefaults.AuthenticationScheme,
-                new ClaimsPrincipal(identity));
-
-            ctx.Response.Redirect("/");
-        }
-        finally { _busy = false; }
-    }
+    /// <summary>Original protected URL the operator was bounced from; round-tripped to the endpoint.</summary>
+    [SupplyParameterFromQuery]
+    private string? ReturnUrl { get; set; }
 }
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Hubs/AlertHub.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Hubs/AlertHub.cs
index a108115..c90a0bf 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Hubs/AlertHub.cs
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Hubs/AlertHub.cs
@@ -1,3 +1,4 @@
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.SignalR;
 
 namespace ZB.MOM.WW.OtOpcUa.Admin.Hubs;
@@ -7,6 +8,11 @@ namespace ZB.MOM.WW.OtOpcUa.Admin.Hubs;
 ///     anomalies) to subscribed admin clients. Alerts don't auto-clear — the operator acks them
 ///     from the UI via <see cref="AcknowledgeAsync"/>.
 /// </summary>
+/// <remarks>
+///     <see cref="AuthorizeAttribute"/> gates the hub so failure-detail alert text is not pushed
+///     to anonymous connections (Admin-003).
+/// </remarks>
+[Authorize]
 public sealed class AlertHub : Hub
 {
     public const string AllAlertsGroup = "__alerts__";
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Hubs/FleetStatusHub.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Hubs/FleetStatusHub.cs
index 5af7e66..72bc96c 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Hubs/FleetStatusHub.cs
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Hubs/FleetStatusHub.cs
@@ -1,3 +1,4 @@
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.SignalR;
 
 namespace ZB.MOM.WW.OtOpcUa.Admin.Hubs;
@@ -8,6 +9,12 @@ namespace ZB.MOM.WW.OtOpcUa.Admin.Hubs;
 ///     scope notifications; the server sends <c>NodeStateChanged</c> messages whenever the poller
 ///     observes a delta.
 /// </summary>
+/// <remarks>
+///     <see cref="AuthorizeAttribute"/> gates the hub: an unauthenticated client cannot open the
+///     connection, so the fleet generation/role/resilience stream is not an anonymous
+///     information-disclosure channel (Admin-003).
+/// </remarks>
+[Authorize]
 public sealed class FleetStatusHub : Hub
 {
     public Task SubscribeCluster(string clusterId)
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Hubs/ScriptLogHub.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Hubs/ScriptLogHub.cs
index ef79a34..28e837c 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Hubs/ScriptLogHub.cs
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Hubs/ScriptLogHub.cs
@@ -1,5 +1,6 @@
 using System.Runtime.CompilerServices;
 using System.Text.RegularExpressions;
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.SignalR;
 
 namespace ZB.MOM.WW.OtOpcUa.Admin.Hubs;
@@ -26,7 +27,12 @@ namespace ZB.MOM.WW.OtOpcUa.Admin.Hubs;
 ///         first, then new lines are emitted as they are appended. The stream is cancelled when
 ///         the client disconnects.
 ///     </para>
+///     <para>
+///         <see cref="AuthorizeAttribute"/> gates the hub: only an authenticated operator can
+///         open the connection and tail the server <c>scripts-*.log</c> contents (Admin-003).
+///     </para>
 /// </remarks>
+[Authorize]
 public sealed class ScriptLogHub(IConfiguration configuration, ILogger<ScriptLogHub> logger) : Hub
 {
     /// <summary>Number of existing lines to replay from the end of the file before live-tailing.</summary>
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Program.cs
index 7c98a6e..5634d96 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Program.cs
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Program.cs
@@ -1,4 +1,3 @@
-using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authentication.Cookies;
 using Microsoft.EntityFrameworkCore;
 using OpenTelemetry.Metrics;
@@ -42,9 +41,20 @@ builder.Services.AddAuthorizationBuilder()
 
 builder.Services.AddCascadingAuthenticationState();
 
-builder.Services.AddDbContext<OtOpcUaConfigDbContext>(opt =>
-    opt.UseSqlServer(builder.Configuration.GetConnectionString("ConfigDb")
-        ?? throw new InvalidOperationException("ConnectionStrings:ConfigDb not configured")));
+// Admin-004: the connection string is a secret — it is NOT committed to appsettings.json.
+// Supply it via user-secrets (dev) or the ConnectionStrings__ConfigDb environment variable
+// (prod). An empty/missing value is treated as "not configured" so a misconfigured deploy
+// fails fast with a clear message rather than a downstream SqlClient parse error.
+builder.Services.AddDbContext<OtOpcUaConfigDbContext>((sp, opt) =>
+{
+    var connectionString = sp.GetRequiredService<IConfiguration>().GetConnectionString("ConfigDb");
+    if (string.IsNullOrWhiteSpace(connectionString))
+        throw new InvalidOperationException(
+            "ConnectionStrings:ConfigDb is not configured. Set it via user-secrets " +
+            "(dotnet user-secrets set \"ConnectionStrings:ConfigDb\" \"...\") or the " +
+            "ConnectionStrings__ConfigDb environment variable.");
+    opt.UseSqlServer(connectionString);
+});
 
 builder.Services.AddScoped<ClusterService>();
 builder.Services.AddScoped<GenerationService>();
@@ -136,15 +146,17 @@ app.UseAuthentication();
 app.UseAuthorization();
 app.UseAntiforgery();
 
-app.MapPost("/auth/logout", async (HttpContext ctx) =>
-{
-    await ctx.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme);
-    ctx.Response.Redirect("/");
-});
+// Admin-005: login + logout are minimal-API endpoints. The login does the LDAP bind,
+// grant resolution, cookie SignInAsync and redirect while the HTTP response is still
+// owned by the endpoint — a static-rendered Login.razor form posts here.
+app.MapAuthEndpoints();
 
-app.MapHub<FleetStatusHub>("/hubs/fleet");
-app.MapHub<AlertHub>("/hubs/alerts");
-app.MapHub<ScriptLogHub>("/hubs/script-log");
+// Admin-003: every SignalR hub requires an authenticated caller. The [Authorize] attribute
+// on each Hub class is the primary gate; .RequireAuthorization() on the endpoint is the
+// belt-and-braces backstop so a hub added without the attribute still cannot ship anonymous.
+app.MapHub<FleetStatusHub>("/hubs/fleet").RequireAuthorization();
+app.MapHub<AlertHub>("/hubs/alerts").RequireAuthorization();
+app.MapHub<ScriptLogHub>("/hubs/script-log").RequireAuthorization();
 
 if (metricsEnabled)
 {
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Security/AuthEndpoints.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Security/AuthEndpoints.cs
new file mode 100644
index 0000000..5aae189
--- /dev/null
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Security/AuthEndpoints.cs
@@ -0,0 +1,101 @@
+using System.Security.Claims;
+using Microsoft.AspNetCore.Authentication;
+using Microsoft.AspNetCore.Authentication.Cookies;
+using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Mvc;
+
+namespace ZB.MOM.WW.OtOpcUa.Admin.Security;
+
+/// <summary>
+///     Minimal-API authentication endpoints. Admin-005: the login is a static-rendered HTML
+///     form (<c>Login.razor</c>, <c>data-enhance="false"</c>) POSTing here, so the LDAP bind,
+///     grant resolution, <see cref="AuthenticationHttpContextExtensions.SignInAsync(HttpContext,
+///     string?, ClaimsPrincipal)"/> cookie write and the redirect all happen while the endpoint
+///     still owns an unstarted HTTP response. Performing <c>SignInAsync</c> from an interactive
+///     Blazor circuit (the previous implementation) could not emit the auth cookie because the
+///     original HTTP response had already completed.
+/// </summary>
+public static class AuthEndpoints
+{
+    /// <summary>Maps <c>POST /auth/login</c> and <c>POST /auth/logout</c>.</summary>
+    public static IEndpointRouteBuilder MapAuthEndpoints(this IEndpointRouteBuilder endpoints)
+    {
+        // Anonymous: the login POST is the only way in, so the fallback authorization policy
+        // (Admin-001) must not gate it. DisableAntiforgery — the static form posts with
+        // data-enhance="false" and renders no token; the cookie scheme + LDAP bind are the
+        // gate here. (Admin-006 covers emitting a token for a hardened build.)
+        endpoints.MapPost("/auth/login", (Delegate)LoginAsync)
+            .AllowAnonymous()
+            .DisableAntiforgery();
+
+        endpoints.MapPost("/auth/logout", (Delegate)LogoutAsync)
+            .DisableAntiforgery();
+
+        return endpoints;
+    }
+
+    private static async Task<IResult> LoginAsync(
+        HttpContext ctx,
+        [FromForm] string? username,
+        [FromForm] string? password,
+        [FromForm] string? returnUrl,
+        ILdapAuthService ldapAuth,
+        IAdminRoleGrantResolver grantResolver,
+        CancellationToken ct)
+    {
+        if (string.IsNullOrWhiteSpace(username) || string.IsNullOrWhiteSpace(password))
+            return RedirectToLogin("Username and password are required", returnUrl);
+
+        var result = await ldapAuth.AuthenticateAsync(username, password, ct);
+        if (!result.Success)
+            return RedirectToLogin(result.Error ?? "Sign-in failed", returnUrl);
+
+        // Grants come from the static bootstrap dictionary + DB-backed role grants;
+        // result.Roles (static-only) is intentionally not consulted here.
+        var grants = await grantResolver.ResolveAsync(result.Groups, ct);
+        if (grants.IsEmpty)
+            return RedirectToLogin(
+                "Sign-in succeeded but no Admin roles mapped for your LDAP groups. Contact your administrator.",
+                returnUrl);
+
+        var claims = new List<Claim>
+        {
+            new(ClaimTypes.Name, result.DisplayName ?? result.Username ?? username),
+            new(ClaimTypes.NameIdentifier, username),
+        };
+        foreach (var role in grants.FleetRoles)
+            claims.Add(new Claim(ClaimTypes.Role, role));
+        foreach (var clusterGrant in grants.ClusterRoles)
+            claims.Add(new Claim(ClusterRoleClaims.ClaimType,
+                ClusterRoleClaims.Encode(clusterGrant.ClusterId, clusterGrant.Role)));
+        foreach (var group in result.Groups)
+            claims.Add(new Claim("ldap_group", group));
+
+        var identity = new ClaimsIdentity(claims, CookieAuthenticationDefaults.AuthenticationScheme);
+        await ctx.SignInAsync(CookieAuthenticationDefaults.AuthenticationScheme,
+            new ClaimsPrincipal(identity));
+
+        return Results.Redirect(SafeReturnUrl(returnUrl));
+    }
+
+    private static async Task<IResult> LogoutAsync(HttpContext ctx)
+    {
+        await ctx.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme);
+        return Results.Redirect("/login");
+    }
+
+    private static IResult RedirectToLogin(string error, string? returnUrl)
+    {
+        var target = $"/login?error={Uri.EscapeDataString(error)}";
+        if (!string.IsNullOrWhiteSpace(returnUrl) && IsLocalUrl(returnUrl))
+            target += $"&returnUrl={Uri.EscapeDataString(returnUrl)}";
+        return Results.Redirect(target);
+    }
+
+    /// <summary>Open-redirect guard — only same-site relative paths are honoured.</summary>
+    private static string SafeReturnUrl(string? returnUrl) =>
+        !string.IsNullOrWhiteSpace(returnUrl) && IsLocalUrl(returnUrl) ? returnUrl : "/";
+
+    private static bool IsLocalUrl(string url) =>
+        url.StartsWith('/') && !url.StartsWith("//") && !url.StartsWith("/\\");
+}
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/ZB.MOM.WW.OtOpcUa.Admin.csproj b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/ZB.MOM.WW.OtOpcUa.Admin.csproj
index 8f29e90..ff14093 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/ZB.MOM.WW.OtOpcUa.Admin.csproj
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/ZB.MOM.WW.OtOpcUa.Admin.csproj
@@ -9,6 +9,9 @@
         <NoWarn>$(NoWarn);CS1591</NoWarn>
         <RootNamespace>ZB.MOM.WW.OtOpcUa.Admin</RootNamespace>
         <AssemblyName>OtOpcUa.Admin</AssemblyName>
+        <!-- Admin-004: dev secrets (ConfigDb connection string, LDAP service-account password)
+             live in user-secrets, not in the committed appsettings.json. -->
+        <UserSecretsId>zb-mom-ww-otopcua-admin</UserSecretsId>
     </PropertyGroup>
 
     <ItemGroup>
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/appsettings.json b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/appsettings.json
index 77e7cde..c1b1e22 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/appsettings.json
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/appsettings.json
@@ -1,6 +1,7 @@
 {
+  "_secrets": "Admin-004: no secrets are committed here. Supply the ConfigDb connection string and the LDAP service-account password via user-secrets (dev) or environment variables / a secret store (prod). Env-var keys: ConnectionStrings__ConfigDb and Authentication__Ldap__ServiceAccountPassword. The connection string defaults to Encrypt=True (TLS); use a least-privilege SQL login, not 'sa'.",
   "ConnectionStrings": {
-    "ConfigDb": "Server=10.100.0.35,14330;Database=OtOpcUaConfig;User Id=sa;Password=OtOpcUaDev_2026!;TrustServerCertificate=True;Encrypt=False;"
+    "ConfigDb": ""
   },
   "Authentication": {
     "Ldap": {
@@ -11,7 +12,7 @@
       "AllowInsecureLdap": true,
       "SearchBase": "dc=lmxopcua,dc=local",
       "ServiceAccountDn": "cn=serviceaccount,dc=lmxopcua,dc=local",
-      "ServiceAccountPassword": "serviceaccount123",
+      "ServiceAccountPassword": "",
       "DisplayNameAttribute": "cn",
       "GroupAttribute": "memberOf",
       "GroupToRole": {
diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Admin.Tests/AppSettingsSecretHygieneTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Admin.Tests/AppSettingsSecretHygieneTests.cs
new file mode 100644
index 0000000..9d56422
--- /dev/null
+++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Admin.Tests/AppSettingsSecretHygieneTests.cs
@@ -0,0 +1,79 @@
+using System.Text.Json;
+using Shouldly;
+using Xunit;
+
+namespace ZB.MOM.WW.OtOpcUa.Admin.Tests;
+
+/// <summary>
+///     Regression coverage for Admin-004 — the committed <c>appsettings.json</c> must carry no
+///     plaintext secrets. The <c>ConfigDb</c> connection string and the LDAP
+///     <c>ServiceAccountPassword</c> are supplied at runtime via user-secrets (dev) or
+///     environment variables (prod); the checked-in file holds only empty placeholders.
+/// </summary>
+[Trait("Category", "Unit")]
+public sealed class AppSettingsSecretHygieneTests
+{
+    private static JsonDocument LoadAdminAppSettings()
+    {
+        // Walk up from the test assembly to the repo root (the dir holding the .slnx) and
+        // read the SOURCE appsettings.json — not a bin/ copy — so the test asserts on what
+        // is actually committed.
+        var dir = AppContext.BaseDirectory;
+        while (dir is not null && !File.Exists(Path.Combine(dir, "ZB.MOM.WW.OtOpcUa.slnx")))
+            dir = Path.GetDirectoryName(dir);
+
+        dir.ShouldNotBeNull("could not locate the repo root (ZB.MOM.WW.OtOpcUa.slnx)");
+        var path = Path.Combine(dir, "src", "Server", "ZB.MOM.WW.OtOpcUa.Admin", "appsettings.json");
+        File.Exists(path).ShouldBeTrue($"Admin appsettings.json not found at {path}");
+
+        return JsonDocument.Parse(File.ReadAllText(path));
+    }
+
+    [Fact]
+    public void ConfigDb_connection_string_is_an_empty_placeholder()
+    {
+        using var doc = LoadAdminAppSettings();
+
+        var connectionString = doc.RootElement
+            .GetProperty("ConnectionStrings")
+            .GetProperty("ConfigDb")
+            .GetString();
+
+        connectionString.ShouldBeNullOrEmpty(
+            "the ConfigDb connection string must not be committed — supply it via user-secrets " +
+            "or the ConnectionStrings__ConfigDb environment variable (Admin-004)");
+    }
+
+    [Fact]
+    public void Ldap_service_account_password_is_an_empty_placeholder()
+    {
+        using var doc = LoadAdminAppSettings();
+
+        var password = doc.RootElement
+            .GetProperty("Authentication")
+            .GetProperty("Ldap")
+            .GetProperty("ServiceAccountPassword")
+            .GetString();
+
+        password.ShouldBeNullOrEmpty(
+            "the LDAP ServiceAccountPassword must not be committed (Admin-004)");
+    }
+
+    [Fact]
+    public void No_known_dev_secret_literals_appear_anywhere_in_appsettings()
+    {
+        var dir = AppContext.BaseDirectory;
+        while (dir is not null && !File.Exists(Path.Combine(dir, "ZB.MOM.WW.OtOpcUa.slnx")))
+            dir = Path.GetDirectoryName(dir);
+        dir.ShouldNotBeNull();
+
+        var raw = File.ReadAllText(Path.Combine(
+            dir, "src", "Server", "ZB.MOM.WW.OtOpcUa.Admin", "appsettings.json"));
+
+        // The exact secret literals the review (Admin-004) flagged must be gone entirely —
+        // not relocated to another key, not present as a comment.
+        raw.ShouldNotContain("OtOpcUaDev_2026!");
+        raw.ShouldNotContain("serviceaccount123");
+        raw.ShouldNotContain("User Id=sa");
+    }
+}
diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Admin.Tests/AuthEndpointsTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Admin.Tests/AuthEndpointsTests.cs
new file mode 100644
index 0000000..7c324f4
--- /dev/null
+++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Admin.Tests/AuthEndpointsTests.cs
@@ -0,0 +1,184 @@
+using System.Net;
+using Microsoft.AspNetCore.Mvc.Testing;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Admin.Security;
+using ZB.MOM.WW.OtOpcUa.Admin.Services;
+
+namespace ZB.MOM.WW.OtOpcUa.Admin.Tests;
+
+/// <summary>
+///     Regression coverage for Admin-003 / Admin-005.
+///
+///     Admin-005 — the login is a static-rendered form posting to the <c>/auth/login</c>
+///     minimal-API endpoint, which performs the LDAP bind, cookie <c>SignInAsync</c> and
+///     redirect while it still owns the HTTP response (no interactive Blazor circuit).
+///
+///     Admin-003 — the three SignalR hubs reject anonymous connections.
+///
+///     These are HTTP-pipeline tests with a stubbed <see cref="ILdapAuthService"/>, so they
+///     run without LDAP or the central SQL Server.
+/// </summary>
+public sealed class AuthEndpointsTests : IClassFixture<AuthEndpointsTests.StubbedAuthAppFactory>
+{
+    private readonly StubbedAuthAppFactory _factory;
+
+    public AuthEndpointsTests(StubbedAuthAppFactory factory) => _factory = factory;
+
+    /// <summary>
+    ///     Admin app host with the LDAP service stubbed (a fixed-credential pass/fail) and the
+    ///     background poller removed so the host starts clean without DB or directory access.
+    /// </summary>
+    public sealed class StubbedAuthAppFactory : WebApplicationFactory<Program>
+    {
+        protected override IHost CreateHost(IHostBuilder builder)
+        {
+            builder.ConfigureServices(services =>
+            {
+                var poller = services.SingleOrDefault(d =>
+                    d.ImplementationType?.Name == "FleetStatusPoller");
+                if (poller is not null) services.Remove(poller);
+
+                var ldap = services.SingleOrDefault(d => d.ServiceType == typeof(ILdapAuthService));
+                if (ldap is not null) services.Remove(ldap);
+                services.AddScoped<ILdapAuthService, StubLdapAuthService>();
+
+                var resolver = services.SingleOrDefault(d => d.ServiceType == typeof(IAdminRoleGrantResolver));
+                if (resolver is not null) services.Remove(resolver);
+                services.AddScoped<IAdminRoleGrantResolver, StubRoleGrantResolver>();
+            });
+            return base.CreateHost(builder);
+        }
+
+        public HttpClient CreateNonRedirectingClient() =>
+            CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false });
+    }
+
+    /// <summary>Stub LDAP: <c>good</c>/<c>pw</c> binds; anything else fails.</summary>
+    private sealed class StubLdapAuthService : ILdapAuthService
+    {
+        public Task<LdapAuthResult> AuthenticateAsync(string username, string password, CancellationToken ct = default) =>
+            Task.FromResult(username == "good" && password == "pw"
+                ? new LdapAuthResult(true, "Good Operator", "good", ["FleetAdmins"], ["FleetAdmin"], null)
+                : new LdapAuthResult(false, null, username, [], [], "Invalid username or password"));
+    }
+
+    /// <summary>Stub resolver: any non-empty group set yields a FleetAdmin grant.</summary>
+    private sealed class StubRoleGrantResolver : IAdminRoleGrantResolver
+    {
+        public Task<AdminRoleGrants> ResolveAsync(IReadOnlyList<string> ldapGroups, CancellationToken cancellationToken) =>
+            Task.FromResult(ldapGroups.Count == 0
+                ? AdminRoleGrants.Empty
+                : new AdminRoleGrants([AdminRoles.FleetAdmin], []));
+    }
+
+    private static FormUrlEncodedContent Form(params (string Key, string Value)[] fields) =>
+        new(fields.Select(f => new KeyValuePair<string, string>(f.Key, f.Value)));
+
+    // ── Admin-005: /auth/login endpoint ─────────────────────────────────────────
+
+    [Fact]
+    public async Task Valid_login_issues_the_auth_cookie_and_redirects_home()
+    {
+        using var client = _factory.CreateNonRedirectingClient();
+
+        var response = await client.PostAsync("/auth/login",
+            Form(("username", "good"), ("password", "pw")));
+
+        // The endpoint owns the response, so the Set-Cookie header is actually emitted.
+        response.StatusCode.ShouldBeOneOf(HttpStatusCode.Redirect, HttpStatusCode.Found);
+        response.Headers.Location!.OriginalString.ShouldBe("/");
+        response.Headers.TryGetValues("Set-Cookie", out var cookies).ShouldBeTrue(
+            "a successful /auth/login must emit the auth cookie");
+        string.Join(';', cookies!).ShouldContain("OtOpcUa.Admin");
+    }
+
+    [Fact]
+    public async Task Invalid_login_redirects_back_to_login_with_an_error()
+    {
+        using var client = _factory.CreateNonRedirectingClient();
+
+        var response = await client.PostAsync("/auth/login",
+            Form(("username", "bad"), ("password", "wrong")));
+
+        response.StatusCode.ShouldBeOneOf(HttpStatusCode.Redirect, HttpStatusCode.Found);
+        response.Headers.Location!.OriginalString.ShouldContain("/login");
+        response.Headers.Location!.OriginalString.ShouldContain("error");
+        response.Headers.TryGetValues("Set-Cookie", out var cookies);
+        (cookies is null || !string.Join(';', cookies).Contains("OtOpcUa.Admin")).ShouldBeTrue(
+            "a failed bind must not issue the auth cookie");
+    }
+
+    [Fact]
+    public async Task Login_with_missing_credentials_redirects_back_to_login()
+    {
+        using var client = _factory.CreateNonRedirectingClient();
+
+        var response = await client.PostAsync("/auth/login", Form(("username", ""), ("password", "")));
+
+        response.StatusCode.ShouldBeOneOf(HttpStatusCode.Redirect, HttpStatusCode.Found);
+        response.Headers.Location!.OriginalString.ShouldContain("/login");
+    }
+
+    [Fact]
+    public async Task Login_redirect_target_is_open_redirect_safe()
+    {
+        using var client = _factory.CreateNonRedirectingClient();
+
+        // A returnUrl pointing off-site must be ignored — the post lands at the site root.
+        var response = await client.PostAsync("/auth/login",
+            Form(("username", "good"), ("password", "pw"), ("returnUrl", "https://evil.example.com/")));
+
+        response.StatusCode.ShouldBeOneOf(HttpStatusCode.Redirect, HttpStatusCode.Found);
+        response.Headers.Location!.OriginalString.ShouldBe("/");
+    }
+
+    [Fact]
+    public async Task Login_honours_a_local_return_url()
+    {
+        using var client = _factory.CreateNonRedirectingClient();
+
+        var response = await client.PostAsync("/auth/login",
+            Form(("username", "good"), ("password", "pw"), ("returnUrl", "/fleet")));
+
+        response.StatusCode.ShouldBeOneOf(HttpStatusCode.Redirect, HttpStatusCode.Found);
+        response.Headers.Location!.OriginalString.ShouldBe("/fleet");
+    }
+
+    [Fact]
+    public async Task Logout_endpoint_clears_the_cookie_and_redirects_to_login()
+    {
+        using var client = _factory.CreateNonRedirectingClient();
+
+        var response = await client.PostAsync("/auth/logout",
+            new FormUrlEncodedContent(Array.Empty<KeyValuePair<string, string>>()));
+
+        // No antiforgery 400 — the endpoint opts out (Admin-006 note in AuthEndpoints).
+        response.StatusCode.ShouldBeOneOf(HttpStatusCode.Redirect, HttpStatusCode.Found);
+        response.Headers.Location!.OriginalString.ShouldContain("/login");
+    }
+
+    // ── Admin-003: SignalR hubs reject anonymous connections ────────────────────
+
+    [Theory]
+    [InlineData("/hubs/fleet")]
+    [InlineData("/hubs/alerts")]
+    [InlineData("/hubs/script-log")]
+    public async Task Anonymous_hub_negotiate_is_rejected(string hubPath)
+    {
+        using var client = _factory.CreateNonRedirectingClient();
+
+        // The SignalR negotiate handshake is a POST to <hub>/negotiate. An [Authorize]'d hub
+        // must refuse it for an unauthenticated caller (302 to login or 401).
+        var response = await client.PostAsync($"{hubPath}/negotiate",
+            new FormUrlEncodedContent(Array.Empty<KeyValuePair<string, string>>()));
+
+        response.StatusCode.ShouldNotBe(HttpStatusCode.OK,
+            $"anonymous negotiate of {hubPath} must not succeed — the hub is [Authorize]-gated");
+        response.StatusCode.ShouldBeOneOf(
+            HttpStatusCode.Unauthorized, HttpStatusCode.Forbidden,
+            HttpStatusCode.Redirect, HttpStatusCode.Found);
+    }
+}