From 75c07149d4dfbef958f06a271a498fd5ef297a2c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Apr 2026 20:40:07 -0400 Subject: [PATCH] =?UTF-8?q?Task=20#124=20=E2=80=94=20Phase=206.2=20multi-u?= =?UTF-8?q?ser=20authz=20interop=20matrix=20+=20close=20LdapGroups=20gap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Phase 6.2 evaluator was wired but received no input in production: RoleBasedIdentity (the IUserIdentity our LDAP path produces) implemented IRoleBearer but not ILdapGroupsBearer, so AuthorizationGate.BuildSessionState always returned null and the gate lax-mode-allowed every request. UserAuthResult also never carried the resolved LDAP groups, only the role-mapped strings. Closing the gap so the evaluator gets real data: - UserAuthResult adds Groups alongside Roles. LdapUserAuthenticator now surfaces the raw RDN values (ReadOnly / WriteOperate / ...) it already collected during the directory query. Roles stay separate per decision #150 (control-plane Admin role mapping vs data-plane NodeAcl key). - RoleBasedIdentity implements ILdapGroupsBearer so AuthorizationGate sees the groups via the same seam unit tests already use. ThreeUserInteropMatrixTests drives the closure end-to-end against the live GLAuth dev directory: - 5 distinct group memberships (readonly / writeop / writetune / writeconfig / alarmack) plus the multi-group admin user - Each is bound through the real LdapUserAuthenticator - Resolved groups feed an LdapBoundIdentity that goes through the strict-mode AuthorizationGate against a seeded TriePermissionEvaluator - 31 InlineData rows assert the role × operation matrix; failures pinpoint the exact (user, op) cell The remaining wire-level leg of #124 — a real OPC UA client driving UserName tokens through an encrypted endpoint policy — still needs a deployment knob and stays a manual cross-vendor smoke (#119 / #124 manual scope). The doc audit note in admin-ui-phase-6-status.md is updated to reflect what's now auto'd vs what stays manual. 33/33 new tests pass against live GLAuth; existing 270 non-LiveLdap tests in Server.Tests still pass; Core.Tests 205/205, Admin.Tests 109/109. The 7 integration-test failures observed during this run pre-exist this commit (NodeId-scheme regression from #134) and are tracked separately as #135. --- .../implementation/admin-ui-phase-6-status.md | 2 +- .../OpcUa/OtOpcUaServer.cs | 18 +- .../Security/IUserAuthenticator.cs | 11 +- .../Security/LdapUserAuthenticator.cs | 12 +- .../ThreeUserInteropMatrixTests.cs | 233 ++++++++++++++++++ 5 files changed, 260 insertions(+), 16 deletions(-) create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Server.Tests/ThreeUserInteropMatrixTests.cs diff --git a/docs/v2/implementation/admin-ui-phase-6-status.md b/docs/v2/implementation/admin-ui-phase-6-status.md index a1ed0f6..10f79ec 100644 --- a/docs/v2/implementation/admin-ui-phase-6-status.md +++ b/docs/v2/implementation/admin-ui-phase-6-status.md @@ -30,7 +30,7 @@ Audit pass that closes the Phase 6 Admin-UI tasks that were tracked as still-ope ## What's NOT in this audit -- `#124` — Phase 6.2 3-user interop matrix. Manual cross-client test; out of scope for code pass. +- `#124` — Phase 6.2 3-user interop matrix. Authz layer is now covered by `ThreeUserInteropMatrixTests` in `ZB.MOM.WW.OtOpcUa.Server.Tests` (drives the 5 GLAuth users + admin through `LdapUserAuthenticator` → `AuthorizationGate.IsAllowed` for the role × operation matrix). The wire-level OPC UA-client cross-vendor leg still needs a UserName-token endpoint policy + manual client drill — that part stays a manual deliverable. - `#119` — Phase 6.3 client interop matrix. Manual Ignition/Kepware/Aveva drills. - `#113` — OPC UA CTT conformance pass. Manual CTT run. - `#114` / `#115` — Redundancy cutover + deployment checklist. Manual. diff --git a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OtOpcUaServer.cs b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OtOpcUaServer.cs index 49dd62a..f2154eb 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OtOpcUaServer.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OtOpcUaServer.cs @@ -125,7 +125,7 @@ public sealed class OtOpcUaServer : StandardServer case AnonymousIdentityToken: args.Identity = _anonymousRoles.Count == 0 ? new UserIdentity() // anonymous, no roles — production default - : new RoleBasedIdentity("(anonymous)", "Anonymous", _anonymousRoles); + : new RoleBasedIdentity("(anonymous)", "Anonymous", _anonymousRoles, ldapGroups: []); return; case UserNameIdentityToken user: @@ -139,7 +139,7 @@ public sealed class OtOpcUaServer : StandardServer StatusCodes.BadUserAccessDenied, "Invalid username or password ({0})", result.Error ?? "no detail"); } - args.Identity = new RoleBasedIdentity(user.UserName, result.DisplayName, result.Roles); + args.Identity = new RoleBasedIdentity(user.UserName, result.DisplayName, result.Roles, result.Groups); return; } @@ -151,20 +151,24 @@ public sealed class OtOpcUaServer : StandardServer } /// - /// Tiny UserIdentity carrier that preserves the resolved roles so downstream node - /// managers can gate writes by role via session.Identity. Anonymous identity still - /// uses the stack's default. + /// Tiny UserIdentity carrier that preserves the resolved roles + LDAP groups so downstream + /// node managers can gate writes/reads via session.Identity. Implements both + /// (control-plane: WriteAuthzPolicy + Admin role mapping) and + /// (data-plane: evaluator). + /// Anonymous identity (no roles configured) still uses the stack's default UserIdentity. /// - private sealed class RoleBasedIdentity : UserIdentity, IRoleBearer + private sealed class RoleBasedIdentity : UserIdentity, IRoleBearer, ILdapGroupsBearer { public IReadOnlyList Roles { get; } + public IReadOnlyList LdapGroups { get; } public string? Display { get; } - public RoleBasedIdentity(string userName, string? displayName, IReadOnlyList roles) + public RoleBasedIdentity(string userName, string? displayName, IReadOnlyList roles, IReadOnlyList ldapGroups) : base(userName, "") { Display = displayName; Roles = roles; + LdapGroups = ldapGroups; } } diff --git a/src/ZB.MOM.WW.OtOpcUa.Server/Security/IUserAuthenticator.cs b/src/ZB.MOM.WW.OtOpcUa.Server/Security/IUserAuthenticator.cs index 9cfab54..40a6b50 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Server/Security/IUserAuthenticator.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Server/Security/IUserAuthenticator.cs @@ -10,7 +10,14 @@ public interface IUserAuthenticator Task AuthenticateAsync(string username, string password, CancellationToken ct = default); } -public sealed record UserAuthResult(bool Success, string? DisplayName, IReadOnlyList Roles, string? Error); +/// True iff the bind succeeded and roles/groups were resolved. +/// User display name from LDAP, or null on failure. +/// Mapped OPC UA role names (Admin / control-plane consumption — see decision #150). +/// Raw LDAP group names the user belongs to. Phase 6.2 data-path authorization +/// (NodeAcl evaluator) keys off this list directly, not Roles. Empty for anonymous / failed binds. +/// Human-readable failure reason, or null on success. +public sealed record UserAuthResult( + bool Success, string? DisplayName, IReadOnlyList Roles, IReadOnlyList Groups, string? Error); /// /// Always-reject authenticator used when no security config is provided. Lets the server @@ -19,5 +26,5 @@ public sealed record UserAuthResult(bool Success, string? DisplayName, IReadOnly public sealed class DenyAllUserAuthenticator : IUserAuthenticator { public Task AuthenticateAsync(string _, string __, CancellationToken ___) - => Task.FromResult(new UserAuthResult(false, null, [], "UserName token not supported")); + => Task.FromResult(new UserAuthResult(false, null, [], [], "UserName token not supported")); } diff --git a/src/ZB.MOM.WW.OtOpcUa.Server/Security/LdapUserAuthenticator.cs b/src/ZB.MOM.WW.OtOpcUa.Server/Security/LdapUserAuthenticator.cs index 744d89b..f43d181 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Server/Security/LdapUserAuthenticator.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Server/Security/LdapUserAuthenticator.cs @@ -15,12 +15,12 @@ public sealed class LdapUserAuthenticator(LdapOptions options, ILogger AuthenticateAsync(string username, string password, CancellationToken ct = default) { if (!options.Enabled) - return new UserAuthResult(false, null, [], "LDAP authentication disabled"); + return new UserAuthResult(false, null, [], [], "LDAP authentication disabled"); if (string.IsNullOrWhiteSpace(username) || string.IsNullOrWhiteSpace(password)) - return new UserAuthResult(false, null, [], "Credentials required"); + return new UserAuthResult(false, null, [], [], "Credentials required"); if (!options.UseTls && !options.AllowInsecureLdap) - return new UserAuthResult(false, null, [], + return new UserAuthResult(false, null, [], [], "Insecure LDAP is disabled. Set UseTls or AllowInsecureLdap for dev/test."); try @@ -84,17 +84,17 @@ public sealed class LdapUserAuthenticator(LdapOptions options, ILogger +/// Task #124 — Phase 6.2 multi-user interop matrix. Drives the live GLAuth dev directory +/// (5 distinct group memberships, plus a multi-group admin) end-to-end through: +/// LdapUserAuthenticator bind → resolved LDAP group list → +/// against a seeded +/// → expected allow/deny verdict. +/// +/// +/// +/// This is the closest a code pass can get to the manual "3-user interop matrix" Phase 6.2 +/// deliverable. The remaining wire-level layer (real OPC UA client, encrypted UserName +/// token through the endpoint policy) needs a security-profile knob that's tracked +/// separately and stays a manual cross-client smoke (#119 / #124 manual scope). +/// +/// +/// Closes the production gap surfaced while planning this test: RoleBasedIdentity +/// did not implement , so +/// lax-mode-allowed every request because it never received resolved LDAP groups. After +/// this PR carries Groups alongside Roles and +/// RoleBasedIdentity exposes them via the bearer interface. +/// +/// Skipped when GLAuth at localhost:3893 is unreachable so the suite stays +/// portable. +/// +[Trait("Category", "LiveLdap")] +public sealed class ThreeUserInteropMatrixTests +{ + private const string GlauthHost = "localhost"; + private const int GlauthPort = 3893; + private const string ClusterId = "c1"; + + private static bool GlauthReachable() + { + try + { + using var client = new TcpClient(); + var task = client.ConnectAsync(GlauthHost, GlauthPort); + return task.Wait(TimeSpan.FromSeconds(1)) && client.Connected; + } + catch { return false; } + } + + private static LdapOptions GlauthOptions() => new() + { + Enabled = true, + Server = GlauthHost, + Port = GlauthPort, + UseTls = false, + AllowInsecureLdap = true, + SearchBase = "dc=lmxopcua,dc=local", + ServiceAccountDn = "cn=serviceaccount,dc=lmxopcua,dc=local", + ServiceAccountPassword = "serviceaccount123", + DisplayNameAttribute = "cn", + GroupAttribute = "memberOf", + UserNameAttribute = "cn", + // Identity translation — GLAuth group RDN values are the same strings as the + // OPC UA roles we map to, so the GroupToRole table is straightforward. + GroupToRole = new(StringComparer.OrdinalIgnoreCase) + { + ["ReadOnly"] = "ReadOnly", + ["WriteOperate"] = WriteAuthzPolicy.RoleWriteOperate, + ["WriteTune"] = WriteAuthzPolicy.RoleWriteTune, + ["WriteConfigure"] = WriteAuthzPolicy.RoleWriteConfigure, + ["AlarmAck"] = "AlarmAck", + }, + }; + + private static LdapUserAuthenticator NewAuthenticator() => + new(GlauthOptions(), NullLogger.Instance); + + /// + /// Production-shaped ACL ruleset — one row per LDAP group, granted at Cluster scope so + /// it covers any node the matrix probes. Each group gets exactly the flags it needs; + /// the matrix asserts the flag-by-flag isolation the evaluator must preserve. + /// + private static NodeAcl[] AclMatrix() => + [ + Row("ReadOnly", NodePermissions.Browse | NodePermissions.Read | NodePermissions.Subscribe | NodePermissions.HistoryRead), + Row("WriteOperate", NodePermissions.Browse | NodePermissions.Read | NodePermissions.WriteOperate), + Row("WriteTune", NodePermissions.Browse | NodePermissions.Read | NodePermissions.WriteTune), + Row("WriteConfigure", NodePermissions.Browse | NodePermissions.Read | NodePermissions.WriteConfigure), + Row("AlarmAck", NodePermissions.Browse | NodePermissions.AlarmAcknowledge | NodePermissions.AlarmConfirm | NodePermissions.AlarmShelve), + ]; + + private static NodeAcl Row(string group, NodePermissions flags) => new() + { + NodeAclRowId = Guid.NewGuid(), + NodeAclId = Guid.NewGuid().ToString(), + GenerationId = 1, + ClusterId = ClusterId, + LdapGroup = group, + ScopeKind = NodeAclScopeKind.Cluster, + ScopeId = null, + PermissionFlags = flags, + }; + + private static NodeScope Scope() => new() + { + ClusterId = ClusterId, + NamespaceId = "ns", + UnsAreaId = "area", + UnsLineId = "line", + EquipmentId = "eq", + TagId = "tag1", + Kind = NodeHierarchyKind.Equipment, + }; + + private static AuthorizationGate MakeStrictGate() + { + var cache = new PermissionTrieCache(); + cache.Install(PermissionTrieBuilder.Build(ClusterId, 1, AclMatrix())); + return new AuthorizationGate(new TriePermissionEvaluator(cache), strictMode: true); + } + + private sealed class LdapBoundIdentity : UserIdentity, ILdapGroupsBearer + { + public LdapBoundIdentity(string userName, IReadOnlyList groups) + { + DisplayName = userName; + LdapGroups = groups; + } + public new string DisplayName { get; } + public IReadOnlyList LdapGroups { get; } + } + + /// + /// End-to-end: bind via LDAP, observe the resolved groups, drive every + /// in the relevant subset through the strict-mode gate, and + /// assert the expected verdict. One InlineData row per (user, operation) pair so failures + /// report the precise cell that broke. + /// + [Theory] + // readonly — read-side only + [InlineData("readonly", "readonly123", OpcUaOperation.Browse, true)] + [InlineData("readonly", "readonly123", OpcUaOperation.Read, true)] + [InlineData("readonly", "readonly123", OpcUaOperation.HistoryRead, true)] + [InlineData("readonly", "readonly123", OpcUaOperation.WriteOperate, false)] + [InlineData("readonly", "readonly123", OpcUaOperation.WriteTune, false)] + [InlineData("readonly", "readonly123", OpcUaOperation.WriteConfigure, false)] + [InlineData("readonly", "readonly123", OpcUaOperation.AlarmAcknowledge, false)] + // writeop — Operate writes only, no escalation to Tune/Configure/Alarm + [InlineData("writeop", "writeop123", OpcUaOperation.Read, true)] + [InlineData("writeop", "writeop123", OpcUaOperation.WriteOperate, true)] + [InlineData("writeop", "writeop123", OpcUaOperation.WriteTune, false)] + [InlineData("writeop", "writeop123", OpcUaOperation.WriteConfigure, false)] + [InlineData("writeop", "writeop123", OpcUaOperation.AlarmAcknowledge, false)] + // writetune — Tune writes only + [InlineData("writetune", "writetune123", OpcUaOperation.Read, true)] + [InlineData("writetune", "writetune123", OpcUaOperation.WriteOperate, false)] + [InlineData("writetune", "writetune123", OpcUaOperation.WriteTune, true)] + [InlineData("writetune", "writetune123", OpcUaOperation.WriteConfigure, false)] + // writeconfig — Configure writes only + [InlineData("writeconfig", "writeconfig123", OpcUaOperation.Read, true)] + [InlineData("writeconfig", "writeconfig123", OpcUaOperation.WriteOperate, false)] + [InlineData("writeconfig", "writeconfig123", OpcUaOperation.WriteTune, false)] + [InlineData("writeconfig", "writeconfig123", OpcUaOperation.WriteConfigure, true)] + // alarmack — alarm-only; deliberately has no Read grant. Verifies flag isolation. + [InlineData("alarmack", "alarmack123", OpcUaOperation.Browse, true)] + [InlineData("alarmack", "alarmack123", OpcUaOperation.Read, false)] + [InlineData("alarmack", "alarmack123", OpcUaOperation.WriteOperate, false)] + [InlineData("alarmack", "alarmack123", OpcUaOperation.AlarmAcknowledge, true)] + [InlineData("alarmack", "alarmack123", OpcUaOperation.AlarmConfirm, true)] + [InlineData("alarmack", "alarmack123", OpcUaOperation.AlarmShelve, true)] + // admin — member of every group; OR-ing across groups means everything is allowed. + [InlineData("admin", "admin123", OpcUaOperation.Read, true)] + [InlineData("admin", "admin123", OpcUaOperation.WriteOperate, true)] + [InlineData("admin", "admin123", OpcUaOperation.WriteTune, true)] + [InlineData("admin", "admin123", OpcUaOperation.WriteConfigure, true)] + [InlineData("admin", "admin123", OpcUaOperation.AlarmAcknowledge, true)] + public async Task Matrix(string username, string password, OpcUaOperation op, bool expectAllow) + { + if (!GlauthReachable()) Assert.Skip("GLAuth unreachable at localhost:3893 — start the dev directory to run this test."); + + var auth = await NewAuthenticator().AuthenticateAsync(username, password, TestContext.Current.CancellationToken); + auth.Success.ShouldBeTrue($"LDAP bind for {username} failed: {auth.Error}"); + auth.Groups.ShouldNotBeEmpty($"{username} resolved zero LDAP groups — the bind succeeded but the directory query returned nothing"); + + var identity = new LdapBoundIdentity(username, auth.Groups); + var gate = MakeStrictGate(); + + var allowed = gate.IsAllowed(identity, op, Scope()); + + allowed.ShouldBe(expectAllow, + $"user={username} op={op} groups=[{string.Join(",", auth.Groups)}] expected={expectAllow}"); + } + + [Fact] + public async Task Admin_Resolves_All_Five_Groups_From_LDAP() + { + // Sanity check separate from the matrix: the admin user must surface every group it + // belongs to via the new UserAuthResult.Groups channel — the matrix above relies on + // exactly this. If the directory query missed a group, the per-op allow rows for admin + // could pass for the wrong reason (e.g. through lax-mode fallback), so this test + // pins the resolution explicitly in strict mode. + if (!GlauthReachable()) Assert.Skip("GLAuth unreachable at localhost:3893."); + + var auth = await NewAuthenticator().AuthenticateAsync("admin", "admin123", TestContext.Current.CancellationToken); + + auth.Success.ShouldBeTrue(); + auth.Groups.ShouldContain("ReadOnly"); + auth.Groups.ShouldContain("WriteOperate"); + auth.Groups.ShouldContain("WriteTune"); + auth.Groups.ShouldContain("WriteConfigure"); + auth.Groups.ShouldContain("AlarmAck"); + } + + [Fact] + public async Task Failed_Bind_Returns_Empty_Groups_And_Empty_Roles() + { + // Failure path must not surface any group claims — the gate would be misled into + // resolving permissions for a user who never authenticated. + if (!GlauthReachable()) Assert.Skip("GLAuth unreachable at localhost:3893."); + + var auth = await NewAuthenticator().AuthenticateAsync("readonly", "wrong-password", TestContext.Current.CancellationToken); + + auth.Success.ShouldBeFalse(); + auth.Groups.ShouldBeEmpty(); + auth.Roles.ShouldBeEmpty(); + } +}