dohertj2/lmxopcua
1
0
Fork 0
Code Issues 32 Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(acl): delete the LDAP data-plane authenticator and its tests (#520)

Browse Source 
Stage 2b. LdapOpcUaUserAuthenticator was left registered nowhere by stage 2a;
this removes it, its three test files, and the config surface that existed only
to serve it.

Deleted: LdapOpcUaUserAuthenticator, LdapOpcUaUserAuthenticatorTests,
LdapAuthResilienceTests, LdapGlauthOutageLiveTests (19 tests).

Also removed four Security:Ldap keys — AuthTimeoutMs,
MaxConcurrentAuthentications, OutageFailureThreshold, OutageCooldownSeconds —
plus their validator rules and binding/validator test coverage. Verified first
that the ONLY consumers were the deleted class and the validator rules asserting
them; none appear in any checked-in appsettings. Leaving them would have created
exactly the dead-config surface #521/#524/#525 were filed to clear up.

Worth stating why that resilience machinery does not need relocating rather than
just deleting. It existed because the OPC UA SDK raises impersonation
SYNCHRONOUSLY under a SessionManager-wide event lock, so a hung LDAP bind there
serialized every session activation server-wide — Anonymous and X509 included.
Key verification is one indexed lookup plus one HMAC with no network I/O, so the
hazard the wall-clock bound, concurrency cap and outage circuit guarded against
no longer exists on that path. LDAP now runs only on AdminUI login, an ordinary
async request path where a slow directory affects the one request making it.

ConnectionTimeoutMs is RETAINED: it is projected into the shared library and is
not authenticator-specific.

Docs corrected rather than left to rot:
- CLAUDE.md's "LDAP Authentication" section described the deleted file as the
  data-plane authenticator. Rewritten, including a warning not to "restore" it on
  seeing the LDAP wiring that AdminUI legitimately still needs.
- infra/README §5b (the GLAuth outage live gate) marked RETIRED with the reason,
  rather than silently dropped — the harness GLAuth is still used by
  OTOPCUA_HARNESS_USE_LDAP=1 and is unaffected.
- docs/security.md's five-key resilience table cut to the one surviving key, and
  a new API-key identity section added.
Historical archreview/ records are left as-is; they are dated accounts.

Host.IntegrationTests 211 tests (was 230, i.e. exactly the 19 removed) with the
same 3 pre-existing failures verified against clean master. Host 37, Security 89.

Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
This commit is contained in:
Joseph Doherty
2026-07-31 13:08:50 -04:00
parent 0329fa4d4b
commit 65fda7237f
13 changed files with 124 additions and 1152 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs
+5 -10
View File
@@ -48,17 +48,12 @@ public sealed class LdapOptionsValidator : OptionsValidatorBase<LdapOptions>
!(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.");
// S2 resilience knobs — a non-positive timeout / concurrency bound would silently disable the
// wall-clock/concurrency protection the OPC UA data-plane authenticator relies on, so fail fast.
// A non-positive connect/search timeout would leave the shared library unbounded against a hung
// directory, so fail fast. The other four S2 knobs validated here (AuthTimeoutMs,
// MaxConcurrentAuthentications, OutageFailureThreshold, OutageCooldownSeconds) went with
// LdapOpcUaUserAuthenticator in #520 — they guarded an LDAP bind running inline on OPC UA
// session activation, and no remaining LDAP consumer runs on that path.
builder.RequireThat(options.ConnectionTimeoutMs > 0,
"Ldap:ConnectionTimeoutMs must be > 0.");
builder.RequireThat(options.AuthTimeoutMs > 0,
"Ldap:AuthTimeoutMs must be > 0.");
builder.RequireThat(options.MaxConcurrentAuthentications > 0,
"Ldap:MaxConcurrentAuthentications must be > 0.");
// An open circuit with no cooldown would re-probe LDAP on every call — pointless. Only enforce
// a positive cooldown when the circuit is actually enabled (OutageFailureThreshold > 0; 0 disables).
builder.RequireThat(options.OutageFailureThreshold <= 0 || options.OutageCooldownSeconds > 0,
"Ldap:OutageCooldownSeconds must be > 0 when Ldap:OutageFailureThreshold > 0 (0 disables the circuit).");
}
}
src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/LdapOpcUaUserAuthenticator.cs
-260
View File
@@ -1,260 +0,0 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using ZB.MOM.WW.Auth.Abstractions.Roles;
using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security;
using ZB.MOM.WW.OtOpcUa.Security.Ldap;
namespace ZB.MOM.WW.OtOpcUa.Host.OpcUa;
/// <summary>
/// Production <see cref="IOpcUaUserAuthenticator"/> adapter that bridges OPC UA UserName
/// tokens to the same <see cref="ILdapAuthService"/> the Admin UI cookie/JWT flows use, so a
/// single LDAP source-of-truth governs both control-plane (Admin) and data-plane (OPC UA)
/// session identities. Roles are resolved through the shared
/// <see cref="IGroupRoleMapper{TRole}"/> seam from the LDAP groups returned by the directory —
/// the same seam the login endpoint uses — and the resolved set is attached to the OPC UA
/// session identity for the downstream data-plane ACL evaluator.
/// </summary>
/// <remarks>
/// <para>
/// This authenticator is registered as a singleton, but <see cref="IGroupRoleMapper{TRole}"/>
/// (and its DbContext-backed mapping service) is scoped. A per-call DI scope is opened to
/// resolve the mapper so the singleton never captures a scoped dependency.
/// </para>
/// <para>
/// <b>Resilience (arch-review 03/S2).</b> The OPC UA SDK raises the impersonation callback that
/// reaches here <em>synchronously</em>, inside a <c>SessionManager</c>-wide event lock (verified
/// against <c>OPCFoundation.NetStandard.Opc.Ua.Server</c> 1.5.378 —
/// <c>SessionManager.ActivateSessionAsync</c> holds <c>m_eventLock</c> while invoking the
/// <c>void</c> impersonation delegate). A slow / hung LDAP bind inside that callback therefore does
/// not merely stall one SDK thread — it serializes <em>every</em> session activation server-wide
/// (Anonymous and X509 included). The callback contract is irreducibly synchronous (there is no
/// async impersonation callback), so the fix lives entirely at this boundary: a hard whole-flow
/// wall-clock bound (<see cref="LdapOptions.AuthTimeoutMs"/>) and a bounded in-flight concurrency cap
/// (<see cref="LdapOptions.MaxConcurrentAuthentications"/>), both fail-closed. All new paths deny.
/// </para>
/// </remarks>
public sealed class LdapOpcUaUserAuthenticator : IOpcUaUserAuthenticator
{
private readonly ILdapAuthService _ldap;
private readonly IServiceScopeFactory _scopeFactory;
private readonly ILogger<LdapOpcUaUserAuthenticator> _logger;
private readonly LdapOptions _options;
/// <summary>Bounds in-flight authentication flows. A boundary timeout abandons its core task, which
/// keeps holding its slot until it actually completes — so an outage cannot accumulate unbounded
/// orphan binds.</summary>
private readonly SemaphoreSlim _gate;
/// <summary>Guards the outage-circuit state (<see cref="_consecutiveSystemFailures"/> +
/// <see cref="_circuitOpenUntilTicks"/>).</summary>
private readonly object _circuitLock = new();
/// <summary>Count of CONSECUTIVE system-side failures; reset by any success / user-side deny.</summary>
private int _consecutiveSystemFailures;
/// <summary><see cref="Environment.TickCount64"/> the circuit stays open until; <c>0</c> = closed.</summary>
private long _circuitOpenUntilTicks;
/// <summary>Constructs the authenticator from its dependencies and the bound LDAP options.</summary>
/// <param name="ldap">The app LDAP auth service (shared with the Admin UI login flow).</param>
/// <param name="scopeFactory">Opens a per-call DI scope to resolve the scoped role mapper.</param>
/// <param name="options">The bound <see cref="LdapOptions"/> (timeout + concurrency knobs).</param>
/// <param name="logger">The logger.</param>
public LdapOpcUaUserAuthenticator(
ILdapAuthService ldap,
IServiceScopeFactory scopeFactory,
IOptions<LdapOptions> options,
ILogger<LdapOpcUaUserAuthenticator> logger)
{
_ldap = ldap;
_scopeFactory = scopeFactory;
_options = options.Value;
_logger = logger;
_gate = new SemaphoreSlim(Math.Max(1, _options.MaxConcurrentAuthentications));
}
/// <inheritdoc />
public async Task<OpcUaUserAuthResult> AuthenticateUserNameAsync(string username, string password, CancellationToken ct)
{
var authTimeout = TimeSpan.FromMilliseconds(Math.Max(1, _options.AuthTimeoutMs));
// Directory-outage circuit: after a run of consecutive system-side failures, fail fast WITHOUT
// touching LDAP for the cooldown window — so a sustained outage cannot serialize every session
// activation behind a per-call stall (the SDK raises this callback under a server-wide lock).
if (IsCircuitOpen())
{
_logger.LogDebug(
"LDAP outage circuit open — fast-denying OPC UA user {User} without a directory call", username);
return OpcUaUserAuthResult.Deny("Authentication backend unavailable");
}
// Bounded in-flight concurrency: acquire a slot within our own wall-clock budget. All slots
// busy is LOCAL backpressure, not directory evidence — deny "busy" (fail-closed) and do NOT
// feed the outage circuit.
if (!await _gate.WaitAsync(authTimeout, ct).ConfigureAwait(false))
{
_logger.LogWarning(
"LDAP authentication backend busy (concurrency cap {Cap}) — denying OPC UA user {User}",
_options.MaxConcurrentAuthentications, username);
return OpcUaUserAuthResult.Deny("Authentication backend busy");
}
// Release the slot when the CORE task completes — deliberately NOT in a finally here. A boundary
// timeout below abandons the core task (it runs to completion in the background; the underlying
// library is documented never-throw, so there are no unobserved exceptions), and it must keep
// counting against the cap until it finishes. The point of the boundary is to release the SDK
// thread (and m_eventLock), not to cancel the bind.
var core = AuthenticateCoreAsync(username, password, ct);
_ = core.ContinueWith(
static (_, state) => ((SemaphoreSlim)state!).Release(),
_gate,
CancellationToken.None,
TaskContinuationOptions.None,
TaskScheduler.Default);
try
{
var outcome = await core.WaitAsync(authTimeout, ct).ConfigureAwait(false);
RecordOutcome(outcome.SystemFailure, username);
return outcome.Result;
}
catch (TimeoutException)
{
// A boundary timeout is directory evidence — it feeds the circuit.
RecordOutcome(systemFailure: true, username);
_logger.LogWarning(
"LDAP authentication timed out after {Timeout} for OPC UA user {User} — denying (fail-closed)",
authTimeout, username);
return OpcUaUserAuthResult.Deny("Authentication timed out");
}
}
/// <summary>Whether the outage circuit is currently open (fast-deny window active). Always false when
/// the circuit is disabled (<see cref="LdapOptions.OutageFailureThreshold"/> ≤ 0).</summary>
private bool IsCircuitOpen()
{
if (_options.OutageFailureThreshold <= 0) return false;
lock (_circuitLock)
{
return _circuitOpenUntilTicks > Environment.TickCount64;
}
}
/// <summary>
/// Updates the outage-circuit state from one authentication outcome. A system-side failure advances
/// the consecutive-failure streak and opens the circuit once it reaches
/// <see cref="LdapOptions.OutageFailureThreshold"/>; any non-system outcome (success or user-side
/// deny) resets the streak and closes the circuit. Half-open behaviour is implicit: once the cooldown
/// elapses <see cref="IsCircuitOpen"/> lets the next call through with the streak still at threshold,
/// so a single further system failure re-opens immediately.
/// </summary>
/// <param name="systemFailure">Whether this outcome was a system-side (directory) failure.</param>
/// <param name="username">The login name, for diagnostics.</param>
private void RecordOutcome(bool systemFailure, string username)
{
if (_options.OutageFailureThreshold <= 0) return; // circuit disabled
lock (_circuitLock)
{
if (systemFailure)
{
_consecutiveSystemFailures++;
if (_consecutiveSystemFailures >= _options.OutageFailureThreshold)
{
var wasOpen = _circuitOpenUntilTicks > Environment.TickCount64;
_circuitOpenUntilTicks =
Environment.TickCount64 + (long)Math.Max(1, _options.OutageCooldownSeconds) * 1000L;
if (!wasOpen)
{
_logger.LogWarning(
"LDAP outage circuit OPENED after {Failures} consecutive system-side failures — " +
"fast-denying for {Cooldown}s (last user {User})",
_consecutiveSystemFailures, _options.OutageCooldownSeconds, username);
}
}
}
else
{
if (_consecutiveSystemFailures >= _options.OutageFailureThreshold)
{
_logger.LogInformation(
"LDAP outage circuit CLOSED — directory recovered (probe succeeded for {User})", username);
}
_consecutiveSystemFailures = 0;
_circuitOpenUntilTicks = 0;
}
}
}
/// <summary>
/// The unbounded core authenticate flow (bind + role-map) that the outer
/// <see cref="AuthenticateUserNameAsync"/> wraps in a wall-clock bound + concurrency cap. Fail-closed:
/// any backend throw becomes a denial rather than escaping into the SDK. The returned
/// <see cref="CoreOutcome.SystemFailure"/> flags directory-side faults (a system-side
/// <see cref="LdapAuthResult.IsSystemFailure"/> deny, or an unexpected throw) so the caller can feed
/// the outage circuit; a user-credential deny is NOT a system failure.
/// </summary>
/// <param name="username">The username to authenticate.</param>
/// <param name="password">The cleartext password.</param>
/// <param name="ct">Cancellation token.</param>
private async Task<CoreOutcome> AuthenticateCoreAsync(string username, string password, CancellationToken ct)
{
try
{
var result = await _ldap.AuthenticateAsync(username, password, ct).ConfigureAwait(false);
if (!result.Success)
{
return new CoreOutcome(
OpcUaUserAuthResult.Deny(result.Error ?? "Invalid credentials"), result.IsSystemFailure);
}
var roles = await ResolveRolesAsync(result.Groups, result.Roles, username, ct).ConfigureAwait(false);
return new CoreOutcome(OpcUaUserAuthResult.Allow(result.DisplayName ?? username, roles), SystemFailure: false);
}
catch (Exception ex) when (ex is not OperationCanceledException)
{
_logger.LogWarning(ex, "LDAP authentication threw for OPC UA user {User}", username);
return new CoreOutcome(OpcUaUserAuthResult.Deny("Authentication backend error"), SystemFailure: true);
}
}
/// <summary>The result of the core authenticate flow plus whether it was a system-side (directory)
/// failure — the signal that drives the outage circuit.</summary>
/// <param name="Result">The auth result to return to the SDK.</param>
/// <param name="SystemFailure">Whether the outcome was a directory-side fault.</param>
private readonly record struct CoreOutcome(OpcUaUserAuthResult Result, bool SystemFailure);
/// <summary>
/// Resolves the user's roles from their LDAP groups via the scoped
/// <see cref="IGroupRoleMapper{TRole}"/>, unioned with any pre-resolved roles (the DevStub
/// FleetAdmin grant). A mapper fault (e.g. a DB outage) must not deny an otherwise-authenticated
/// session: it falls back to the pre-resolved roles, matching the login endpoint's behaviour.
/// </summary>
/// <param name="groups">The LDAP groups returned by the directory.</param>
/// <param name="preResolved">Pre-resolved roles (empty on the real path; FleetAdmin under DevStub).</param>
/// <param name="username">The login name, for diagnostics.</param>
/// <param name="ct">Cancellation token.</param>
private async Task<IReadOnlyList<string>> ResolveRolesAsync(
IReadOnlyList<string> groups, IReadOnlyList<string> preResolved, string username, CancellationToken ct)
{
try
{
await using var scope = _scopeFactory.CreateAsyncScope();
var mapper = scope.ServiceProvider.GetRequiredService<IGroupRoleMapper<string>>();
var mapping = await mapper.MapAsync(groups, ct).ConfigureAwait(false);
var roles = new HashSet<string>(preResolved, StringComparer.OrdinalIgnoreCase);
foreach (var role in mapping.Roles)
roles.Add(role);
return [.. roles];
}
catch (Exception ex) when (ex is not OperationCanceledException)
{
_logger.LogWarning(ex,
"Role-map lookup failed for OPC UA user {User}; using pre-resolved baseline roles", username);
return preResolved;
}
}
}
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs
+3 -1
View File
@@ -246,7 +246,9 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable
/// (<c>GetAwaiter().GetResult()</c> under <see cref="CancellationToken.None"/>) is irreducible: the
/// callback must complete inline. Because the SDK gives no timeout, the hard wall-clock bound + bounded
/// concurrency + directory-outage circuit that keep a slow/hung directory from wedging the SDK live
/// inside the authenticator (<c>LdapOpcUaUserAuthenticator</c>, arch-review 03/S2) — NOT here.
/// inside the authenticator — NOT here. (#520 replaced the LDAP authenticator with
/// <c>ApiKeyOpcUaUserAuthenticator</c>, whose verification does no network I/O at all, so the
/// bound/concurrency/circuit machinery that used to live there is gone rather than relocated.)
/// </remarks>
/// <param name="authenticator">The user authenticator to validate credentials.</param>
/// <param name="args">The impersonation event arguments to process.</param>
src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapOptions.cs
+9 -35
View File
@@ -88,44 +88,18 @@ public sealed class LdapOptions
/// per-operation search inside a single bind flow. Projected into the library's
/// <see cref="LibLdapOptions.ConnectionTimeoutMs"/> via <see cref="ToLibraryOptions"/>. The
/// default (10000) matches the library default, so behaviour is unchanged unless configured.
/// This is the FIRST line of defence against a directory outage; the whole-flow
/// <see cref="AuthTimeoutMs"/> is the backstop (03/S2).
/// </summary>
/// <remarks>
/// This is now the ONLY directory-outage bound. Its former backstops — <c>AuthTimeoutMs</c>,
/// <c>MaxConcurrentAuthentications</c>, <c>OutageFailureThreshold</c> and
/// <c>OutageCooldownSeconds</c> — were removed with <c>LdapOpcUaUserAuthenticator</c> in #520
/// (see the note on this type). They existed because an LDAP bind ran inline on OPC UA session
/// activation, under a SessionManager-wide lock; nothing on the remaining AdminUI login path has
/// that property, so they had no consumer left. Setting any of them in configuration is now
/// silently ignored rather than an error, which is how options binding treats unknown keys.
/// </remarks>
public int ConnectionTimeoutMs { get; set; } = 10_000;
/// <summary>
/// Hard whole-flow wall-clock bound (in milliseconds) the OPC UA data-plane authenticator
/// (<c>LdapOpcUaUserAuthenticator</c>) applies across the ENTIRE authenticate flow — bind plus
/// role-map — so the OPC UA SDK's synchronous impersonation callback (raised under a
/// <c>SessionManager</c>-wide event lock) can never be wedged by a slow / hung directory.
/// Deliberately &gt; <see cref="ConnectionTimeoutMs"/> (default 15000) so a healthy-but-slow
/// full flow is not cut off while an outage is bounded by the library timeout first (03/S2).
/// </summary>
public int AuthTimeoutMs { get; set; } = 15_000;
/// <summary>
/// Maximum number of in-flight OPC UA authentication flows the data-plane authenticator admits
/// at once. A caller that cannot acquire a slot within its budget is denied "backend busy"
/// (local backpressure, not counted against the outage circuit). Bounds accumulation of
/// orphaned / timed-out bind tasks during an outage (03/S2). Default 8.
/// </summary>
public int MaxConcurrentAuthentications { get; set; } = 8;
/// <summary>
/// Number of CONSECUTIVE system-side failures (boundary timeout, directory-unreachable result,
/// or unexpected throw) after which the data-plane authenticator's outage circuit opens and
/// denies instantly for <see cref="OutageCooldownSeconds"/> without touching LDAP. Any success
/// or user-side (credential) deny resets the counter. <c>0</c> disables the circuit (03/S2).
/// Default 3.
/// </summary>
public int OutageFailureThreshold { get; set; } = 3;
/// <summary>
/// How long (in seconds) the outage circuit stays open before a half-open probe is allowed
/// through. Only meaningful when <see cref="OutageFailureThreshold"/> &gt; 0. Default 15 (03/S2).
/// </summary>
public int OutageCooldownSeconds { get; set; } = 15;
/// <summary>
/// Projects the wire fields onto the shared <c>ZB.MOM.WW.Auth.Ldap</c>
/// <see cref="LibLdapOptions"/> the directory client consumes. App-only concerns
src/Server/ZB.MOM.WW.OtOpcUa.Security/ServiceCollectionExtensions.cs
+2 -2
View File
@@ -57,7 +57,7 @@ public static class ServiceCollectionExtensions
// Singleton — OtOpcUaLdapAuthService is stateless (the shared-library directory client it
// wraps opens/disposes an LdapConnection per call) and must be consumable by the Singleton
// LdapOpcUaUserAuthenticator on driver-role nodes. This is the app's ILdapAuthService: it
// AdminUI login on admin-role nodes. This is the app's ILdapAuthService: it
// adds the Enabled master switch + DevStubMode bypass on top of the shared ZB.MOM.WW.Auth.Ldap
// service. TryAdd so a fused admin+driver node (which also registers it in Program.cs for the
// driver path) ends up with exactly one descriptor regardless of registration order.
@@ -65,7 +65,7 @@ public static class ServiceCollectionExtensions
// Shared ZB.MOM.WW.Auth group→role mapper seam. Wraps the existing RoleMapper.Map +
// RoleMapper.Merge logic; consumed by AuthEndpoints.cs (/auth/login) and by
// LdapOpcUaUserAuthenticator (OPC UA UserName session auth). Scoped to match
// AdminUI login (the OPC UA data plane uses API keys since #520). Scoped to match
// ILdapGroupRoleMappingService (DbContext-backed, registered Scoped) — a singleton here
// would capture the scoped DB service.
services.TryAddScoped<IGroupRoleMapper<string>, OtOpcUaGroupRoleMapper>();