From dd939d3c972939c82ea978d0b790940f838d6c58 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 31 Jul 2026 13:20:14 -0400 Subject: [PATCH] feat(acl): anonymous becomes a normal grantable principal (#520 D3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Anonymous sessions used to short-circuit in HandleImpersonation — "let the SDK's default validation stand" — so they always connected and always carried no roles. Now admission and authority are separated: admission -> OpcUa:AllowAnonymous, the HOST's decision (it owns token policies) authority -> the authenticator, resolving the reserved principal "anonymous" through the SAME grant path a key's principal goes through So anonymous can be granted WRITE, not merely tolerated as read-only, and it arrives at the downstream gates as the same RoleCarryingUserIdentity a key produces — which is the real payoff: no gate carries an anonymous-shaped branch, so there is no bypass path to get wrong. AllowAnonymous defaults to TRUE on purpose. Defaulting it false would silently deny every client without a key the moment this deploys, including ScadaBridge's Data Connection Layer in another repo. Making keys mandatory is a rollout decision, not an upgrade side effect. When false, the Anonymous token policy is withheld so a client is refused at DISCOVERY rather than only at activation, and activation is rejected independently in case a client attempts a token type the endpoint never advertised. Two fail-closed choices, both falsified: - A failure to read anonymous's grants DENIES rather than admitting with no roles. Admitting is the pre-#520 behaviour, and defaulting to it exactly when the server cannot determine what anonymous may do stops being harmless once reads are gated. Making it admit reds Anonymous_denies_when_grants_cannot_be_read. - A key may not authenticate as the reserved "anonymous" principal, or a credentialled client silently inherits whatever anonymous was granted. AuthenticateAnonymousAsync is a DEFAULT interface implementation returning admit-with-no-roles, i.e. exactly the pre-#520 behaviour, so NullOpcUaUserAuthenticator and every test double keep working rather than being forced to reimplement it — important because driver-only nodes run the Null one and would otherwise black out. Note a default interface method is only reachable through the interface-typed reference, which is how OpcUaApplicationHost holds it. An existing test asserted the old fall-through and correctly went red; it is rewritten to pin the new contract rather than deleted, with the reason recorded. OpcUaServer 366, Host 42 green. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo --- CLAUDE.md | 6 + docs/security.md | 23 ++++ .../OpcUa/ApiKeyOpcUaUserAuthenticator.cs | 45 +++++++ .../OpcUa/ConfigDbApiKeyStore.cs | 20 ++- .../OpcUa/IApiKeyStore.cs | 9 ++ .../OpcUaApplicationHost.cs | 115 ++++++++++++++++-- .../Security/IOpcUaUserAuthenticator.cs | 37 ++++++ .../OpcUa/ApiKeyAuthenticationTests.cs | 89 ++++++++++++++ .../OpcUaApplicationHostImpersonationTests.cs | 35 +++++- .../OpcUaApplicationHostSecurityTests.cs | 14 +++ 10 files changed, 374 insertions(+), 19 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index bb9b51f9..11bb5bc3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -616,6 +616,12 @@ Two things to know before relying on any of this: honouring a narrower grant's flags while unable to honour its *scope* would turn "WriteOperate on Equipment X" into server-wide write authority. Read, Browse, HistoryRead and CreateMonitoredItems remain ungated for any admitted session, **including Anonymous**. +- **Anonymous is a normal grantable principal**, not a bypass. `OpcUa:AllowAnonymous` (default `true`, + preserving today's behaviour) controls *admission*; when admitted the session authenticates as the + reserved principal `anonymous` and gets roles from `NodeAcl` rows naming it, arriving at the gates as + the same `RoleCarryingUserIdentity` a key produces. Set it `false` to make keys mandatory. A key may + **not** use the `anonymous` principal, and a failure to read anonymous's grants **denies** rather + than admitting with no roles. - ⚠️ **A driver-only node has no key source yet.** `ConfigDbApiKeyStore` needs a ConfigDb, which Phase 4 removed from driver-only nodes; until the artifact-backed store lands they register `NullOpcUaUserAuthenticator` and deny every UserName token. Note `OtOpcUaServerHostedService` takes diff --git a/docs/security.md b/docs/security.md index a48a01cf..d1c0c1ee 100644 --- a/docs/security.md +++ b/docs/security.md @@ -193,6 +193,29 @@ of a credential was wrong. ConfigDb, so keys reach them inside the deployment artifact exactly as `NodeAcl` rows do. Clearing `Enabled` takes effect on a node only once a new artifact reaches it. +### Anonymous is a normal principal + +Anonymous sessions are **admitted by default** (`OpcUa:AllowAnonymous`, default `true` — flipping the +default would silently deny every client that has no key the moment this code deploys, including +ScadaBridge). Set it to `false` to make keys mandatory: the Anonymous token policy is then not +advertised, so a client is refused at *discovery* rather than only at activation, and anonymous +activation is rejected independently in case a client attempts a token type the endpoint never offered. + +When admitted, an anonymous session is **not a special case** — it authenticates as the reserved +principal `anonymous`, and `NodeAcl` rows naming it grant permissions exactly as they would a key's +principal. It can therefore be granted **write**, not merely tolerated as read-only. It arrives at the +downstream gates as the same `RoleCarryingUserIdentity` a key produces, so no gate carries an +anonymous-shaped branch and there is no bypass path to get wrong. + +Two consequences worth knowing: + +- `anonymous` is **reserved**: an API key may not use it as its principal, or a credentialled client + would silently inherit whatever anonymous was granted. Rejected at authentication as well as at + authoring. +- If the anonymous principal's grants **cannot be read**, the session is **denied**, not admitted with + no roles. Admitting is the pre-#520 behaviour, and defaulting to it precisely when the server cannot + determine what anonymous may do stops being harmless once reads are gated. + ⚠️ **Node ACLs are still not enforced per node.** Until the enforcement stages land, a key's grants are collapsed into the two existing server-wide role gates, and **only `Cluster`-scoped grants count** — a narrower grant confers nothing rather than silently widening to server-wide authority. Reads, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ApiKeyOpcUaUserAuthenticator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ApiKeyOpcUaUserAuthenticator.cs index 0415cc16..60fa56e0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ApiKeyOpcUaUserAuthenticator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ApiKeyOpcUaUserAuthenticator.cs @@ -1,5 +1,8 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +// Explicit: an unqualified Configuration.* inside this namespace binds to Host.Configuration, not to +// the ZB.MOM.WW.OtOpcUa.Configuration assembly. +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; using ZB.MOM.WW.OtOpcUa.Core.Authorization; using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security; @@ -120,6 +123,17 @@ public sealed class ApiKeyOpcUaUserAuthenticator : IOpcUaUserAuthenticator return OpcUaUserAuthResult.Deny(DenyMessage); } + if (string.Equals(lookup.Key.Principal, OpcUaPrincipals.Anonymous, StringComparison.OrdinalIgnoreCase)) + { + // Defence in depth against a key authored with the reserved principal (authoring blocks it). + // Allowing it would let a credentialled client silently inherit whatever anonymous was + // granted, and vice versa — two identities that are meant to be distinct sharing one grant. + _log.LogError( + "ApiKeyAuth: denied — key {ApiKeyId} uses the reserved principal '{Principal}'", + apiKeyId, OpcUaPrincipals.Anonymous); + return OpcUaUserAuthResult.Deny(DenyMessage); + } + var roles = ApiKeyInterimRoleMapper.Map(lookup.Grants, lookup.Key.Principal); _log.LogInformation( @@ -128,4 +142,35 @@ public sealed class ApiKeyOpcUaUserAuthenticator : IOpcUaUserAuthenticator return OpcUaUserAuthResult.Allow(lookup.Key.Principal, roles); } + + /// + /// + /// Resolves the anonymous principal's grants through the same mapper a key's principal + /// goes through, so anonymous is authorized by ordinary ACL rows rather than by a special case. + /// Whether anonymous may connect at all is the host's decision + /// (OpcUa:AllowAnonymous) and is settled before this is called. + /// + public async Task AuthenticateAnonymousAsync(CancellationToken ct) + { + IReadOnlyList grants; + try + { + grants = await _store.FindGrantsAsync(OpcUaPrincipals.Anonymous, ct).ConfigureAwait(false); + } + catch (Exception ex) + { + // Deny rather than admit-with-no-roles. Admitting would be the pre-#520 behaviour, and doing + // it precisely when we could not read what anonymous is allowed to do is the wrong default: + // once reads are gated (stage 6) a no-roles admission stops being harmless. + _log.LogError(ex, "ApiKeyAuth: key store threw resolving anonymous grants"); + return OpcUaUserAuthResult.Deny("Anonymous access is unavailable."); + } + + var roles = ApiKeyInterimRoleMapper.Map(grants, OpcUaPrincipals.Anonymous); + _log.LogInformation( + "ApiKeyAuth: anonymous admitted with roles [{Roles}]", + roles.Count == 0 ? "none" : string.Join(",", roles)); + + return OpcUaUserAuthResult.Allow(OpcUaPrincipals.Anonymous, roles); + } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ConfigDbApiKeyStore.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ConfigDbApiKeyStore.cs index f268f6f6..c1f83f14 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ConfigDbApiKeyStore.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ConfigDbApiKeyStore.cs @@ -43,15 +43,25 @@ public sealed class ConfigDbApiKeyStore : IApiKeyStore if (key is null) return null; + return new ApiKeyLookup(key, await LoadClusterGrantsAsync(db, key.Principal, ct).ConfigureAwait(false)); + } + + /// + public async Task> FindGrantsAsync(string principal, CancellationToken ct) + { + await using var db = await _factory.CreateDbContextAsync(ct).ConfigureAwait(false); + return await LoadClusterGrantsAsync(db, principal, ct).ConfigureAwait(false); + } + + private static async Task> LoadClusterGrantsAsync( + OtOpcUaConfigDbContext db, string principal, CancellationToken ct) + { // Only cluster-scoped rows can confer an interim role (see ApiKeyInterimRoleMapper), so only // those are fetched. Stage 5 widens this to the full grant set for per-node evaluation. - var grants = await db.NodeAcls + return await db.NodeAcls .AsNoTracking() - .Where(a => a.Principal == key.Principal - && a.ScopeKind == NodeAclScopeKind.Cluster) + .Where(a => a.Principal == principal && a.ScopeKind == NodeAclScopeKind.Cluster) .ToListAsync(ct) .ConfigureAwait(false); - - return new ApiKeyLookup(key, grants); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/IApiKeyStore.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/IApiKeyStore.cs index 67cfb2c1..6a01089b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/IApiKeyStore.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/IApiKeyStore.cs @@ -22,6 +22,15 @@ public interface IApiKeyStore /// The cancellation token. /// The key and its principal's grants, or null when no usable key matches. Task FindAsync(string apiKeyId, DateTime utcNow, CancellationToken ct); + + /// + /// Returns the ACL grants naming a principal, with no key involved. Used for the + /// anonymous principal, which has grants but obviously no credential. + /// + /// The principal to fetch grants for. + /// The cancellation token. + /// The matching ACL rows; empty when none. + Task> FindGrantsAsync(string principal, CancellationToken ct); } /// An API key resolved for authentication, with the ACL grants naming its principal. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs index 037feed2..a8172cac 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs @@ -51,6 +51,29 @@ public sealed class OpcUaApplicationHostOptions /// public string PkiStoreRoot { get; set; } = "pki"; + /// + /// Whether anonymous sessions may connect at all (#520). When the + /// Anonymous token policy is not advertised AND anonymous activation is rejected, so a client + /// must present an API key. + /// + /// + /// + /// Defaults to deliberately, which preserves today's behaviour. + /// Flipping the default would silently deny every client that has no key the moment this + /// code deploys — including ScadaBridge's Data Connection Layer, which consumes this address + /// space from another repo. Turning it off is the deployment-time decision that makes keys + /// mandatory, and it belongs in the rollout once keys have been issued. + /// + /// + /// Note this controls admission, not authority. An admitted anonymous session is a + /// normal principal (anonymous) whose grants come from NodeAcl rows naming it, + /// exactly like a key's principal — there is no anonymous bypass branch in the evaluator. + /// Until per-node enforcement lands, an admitted session of ANY kind can still read the whole + /// address space, because reads carry no authorization check yet. + /// + /// + public bool AllowAnonymous { get; set; } = true; + /// /// Transport-security profiles exposed by the server. The SDK publishes one endpoint /// descriptor per profile and clients choose at session open. Default = all three @@ -228,7 +251,7 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable } private void OnImpersonateUser(ISession session, ImpersonateEventArgs args) => - HandleImpersonation(_userAuthenticator, args, _logger); + HandleImpersonation(_userAuthenticator, args, _logger, _options.AllowAnonymous); /// /// Pure(-ish) impersonation handler: extracted so unit tests can drive it without booting @@ -256,11 +279,18 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable internal static void HandleImpersonation( IOpcUaUserAuthenticator authenticator, ImpersonateEventArgs args, - ILogger logger) + ILogger logger, + bool allowAnonymous = true) { + if (args.NewIdentity is AnonymousIdentityToken anonymousToken) + { + HandleAnonymousImpersonation(authenticator, args, logger, allowAnonymous, anonymousToken); + return; + } + if (args.NewIdentity is not UserNameIdentityToken token) { - // Anonymous + X509 tokens — let the SDK's default validation stand. + // X509 tokens — let the SDK's default validation stand. return; } @@ -309,6 +339,66 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable token.UserName, string.Join(",", result.Roles)); } + /// + /// Admits or rejects an anonymous session, and — when admitted — attaches the roles the + /// anonymous principal has been granted (#520). + /// + /// + /// Before #520 this path simply returned, leaving the SDK's default identity in place: anonymous + /// always connected and carried no roles. Two things change. Anonymous can now be refused + /// outright, and when admitted it is a normal principal whose grants come from ACL rows, so it + /// can be given read or write like any key. The identity attached is the same + /// the UserName path uses, so every downstream gate reads + /// it identically and there is no anonymous-shaped branch in any of them. + /// + private static void HandleAnonymousImpersonation( + IOpcUaUserAuthenticator authenticator, + ImpersonateEventArgs args, + ILogger logger, + bool allowAnonymous, + AnonymousIdentityToken token) + { + if (!allowAnonymous) + { + logger.LogInformation( + "OpcUaApplicationHost: anonymous session rejected (OpcUa:AllowAnonymous is false)"); + args.IdentityValidationError = new ServiceResult(StatusCodes.BadIdentityTokenRejected, + "Anonymous access is disabled on this server."); + return; + } + + OpcUaUserAuthResult result; + try + { + // Same synchronous bridge, and for the same irreducible reason, as the UserName path above. + result = authenticator.AuthenticateAnonymousAsync(CancellationToken.None) + .GetAwaiter().GetResult(); + } + catch (Exception ex) + { + // Fail closed. An anonymous session whose grants could not be resolved must not fall through + // to the SDK default identity, because that would silently ADMIT it — the pre-#520 + // behaviour — at exactly the moment we could not determine what it is allowed to do. + logger.LogError(ex, "OpcUaApplicationHost: anonymous authenticator threw"); + args.IdentityValidationError = new ServiceResult(StatusCodes.BadIdentityTokenRejected, + "Anonymous access is unavailable."); + return; + } + + if (!result.Success) + { + logger.LogInformation("OpcUaApplicationHost: anonymous session denied: {Error}", result.Error); + args.IdentityValidationError = new ServiceResult(StatusCodes.BadIdentityTokenRejected, + result.Error ?? "Anonymous access is denied."); + return; + } + + args.Identity = new RoleCarryingUserIdentity(token, result.Roles); + logger.LogInformation("OpcUaApplicationHost: anonymous session admitted as {Principal} ({Roles})", + OpcUaPrincipals.Anonymous, + result.Roles.Count == 0 ? "no roles" : string.Join(",", result.Roles)); + } + /// /// Guarantees the application instance certificate exists in {PkiStoreRoot}/own. /// The SDK auto-creates a self-signed certificate the first time this is called on a fresh @@ -350,7 +440,7 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable { serverConfig.SecurityPolicies.Add(policy); } - foreach (var token in BuildUserTokenPolicies()) + foreach (var token in BuildUserTokenPolicies(_options.AllowAnonymous)) { serverConfig.UserTokenPolicies.Add(token); } @@ -442,13 +532,20 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable /// UserName logins when none is registered. /// /// The Anonymous and UserName token policies. - internal static IEnumerable BuildUserTokenPolicies() + internal static IEnumerable BuildUserTokenPolicies(bool allowAnonymous = true) { - yield return new UserTokenPolicy(UserTokenType.Anonymous) + // Withholding the policy is what makes the refusal visible to a client at DISCOVERY rather than + // only at activation: it simply is not offered an anonymous option. HandleImpersonation still + // rejects anonymous activation independently, because a client may attempt a token type the + // endpoint never advertised. + if (allowAnonymous) { - PolicyId = "anonymous", - SecurityPolicyUri = SecurityPolicies.None, - }; + yield return new UserTokenPolicy(UserTokenType.Anonymous) + { + PolicyId = "anonymous", + SecurityPolicyUri = SecurityPolicies.None, + }; + } yield return new UserTokenPolicy(UserTokenType.UserName) { PolicyId = "username_basic256sha256", diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/IOpcUaUserAuthenticator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/IOpcUaUserAuthenticator.cs index 1f136333..837fdf23 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/IOpcUaUserAuthenticator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/IOpcUaUserAuthenticator.cs @@ -22,6 +22,43 @@ public interface IOpcUaUserAuthenticator /// The cancellation token. /// An OpcUaUserAuthResult indicating success or failure. Task AuthenticateUserNameAsync(string username, string password, CancellationToken ct); + + /// + /// Resolves what an anonymous session is allowed to do (#520). Anonymous is a normal + /// principal — — not a special case, so it can be granted + /// read or write through ordinary ACL rows. + /// + /// + /// + /// This is asked only AFTER the host has decided anonymous sessions are admissible at all + /// (OpcUaApplicationHostOptions.AllowAnonymous). Admission is the host's decision; + /// authority is this seam's. + /// + /// + /// The default implementation admits anonymous with no roles, which is exactly the + /// behaviour before #520: an anonymous session connects, and every role-gated operation + /// (write, alarm ack) denies. Implementors that can resolve grants override it; the rest + /// keep the safe status quo rather than being forced to reimplement it. + /// + /// + /// The cancellation token. + /// The roles to attach to the anonymous session, or a denial. + Task AuthenticateAnonymousAsync(CancellationToken ct) => + Task.FromResult(OpcUaUserAuthResult.Allow(OpcUaPrincipals.Anonymous, Array.Empty())); +} + +/// Reserved principal names for the OPC UA data plane (#520). +public static class OpcUaPrincipals +{ + /// + /// The principal an anonymous session authenticates as. NodeAcl rows naming it grant + /// anonymous clients permissions exactly as they would a key's principal. + /// + /// + /// Reserved: an API key must not be created with this principal, or a key would silently inherit + /// whatever anonymous was granted (and vice versa). Enforced at authoring time. + /// + public const string Anonymous = "anonymous"; } /// Outcome of a UserName authentication attempt. populates the session identity's role set. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/ApiKeyAuthenticationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/ApiKeyAuthenticationTests.cs index 767ebacc..54e709f0 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/ApiKeyAuthenticationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/ApiKeyAuthenticationTests.cs @@ -168,6 +168,85 @@ public sealed class ApiKeyAuthenticationTests (await auth.AuthenticateUserNameAsync("apikey", presented, default)).Success.ShouldBeFalse(); } + // ---------- anonymous as a normal principal (D3) ---------- + + [Fact] + public async Task Anonymous_resolves_through_the_same_grant_path_as_a_key() + { + var (_, store) = SetUp(); + store.PrincipalGrants.Add(new NodeAcl + { + NodeAclId = "ACL-anon", + ClusterId = "MAIN", + Principal = OpcUaPrincipals.Anonymous, + ScopeKind = NodeAclScopeKind.Cluster, + PermissionFlags = NodePermissions.WriteOperate, + }); + + var result = await Auth(store).AuthenticateAnonymousAsync(default); + + result.Success.ShouldBeTrue(); + result.DisplayName.ShouldBe(OpcUaPrincipals.Anonymous); + // The point of D3: anonymous can be granted WRITE, not merely tolerated as read-only. + result.Roles.ShouldContain(OpcUaDataPlaneRoles.WriteOperate); + } + + [Fact] + public async Task Anonymous_with_no_grants_is_admitted_with_no_roles() + { + // Today's behaviour preserved: it connects, and every role-gated operation denies. + var (_, store) = SetUp(); + + var result = await Auth(store).AuthenticateAnonymousAsync(default); + + result.Success.ShouldBeTrue(); + result.Roles.ShouldBeEmpty(); + } + + [Fact] + public async Task Anonymous_denies_when_grants_cannot_be_read() + { + // Deliberately NOT admit-with-no-roles. That is the pre-#520 behaviour, and defaulting to it + // exactly when we cannot read what anonymous may do stops being harmless once reads are gated. + var result = await Auth(new ThrowingApiKeyStore()).AuthenticateAnonymousAsync(default); + + result.Success.ShouldBeFalse(); + } + + [Fact] + public async Task A_key_may_not_authenticate_as_the_reserved_anonymous_principal() + { + var (presented, apiKeyId, hash) = ApiKeySecret.Generate(Pepper); + var key = new ApiKey + { + ApiKeyId = apiKeyId, + Name = "sneaky", + SecretHash = hash, + Principal = OpcUaPrincipals.Anonymous, + Enabled = true, + CreatedUtc = DateTime.UtcNow, + }; + + var result = await Auth(new FakeApiKeyStore(key, [])).AuthenticateUserNameAsync("apikey", presented, default); + + // Otherwise a credentialled client silently inherits whatever anonymous was granted. + result.Success.ShouldBeFalse(); + } + + [Fact] + public async Task The_default_interface_implementation_admits_anonymous_with_no_roles() + { + // NullOpcUaUserAuthenticator (driver-only nodes, pre-boot) must keep the pre-#520 behaviour + // rather than denying anonymous outright, or turning this on would black out those nodes. + // Interface-typed on purpose: a default interface implementation is only reachable through the + // interface, which is how OpcUaApplicationHost holds it. + IOpcUaUserAuthenticator sut = NullOpcUaUserAuthenticator.Instance; + var result = await sut.AuthenticateAnonymousAsync(default); + + result.Success.ShouldBeTrue(); + result.Roles.ShouldBeEmpty(); + } + // ---------- helpers ---------- private static NodeAcl Grant(NodeAclScopeKind scope, NodePermissions flags) => new() @@ -207,15 +286,25 @@ public sealed class ApiKeyAuthenticationTests { public string KeyId => key.ApiKeyId; + /// Grants returned for a bare principal lookup (the anonymous path). + public List PrincipalGrants { get; } = []; + public Task FindAsync(string apiKeyId, DateTime utcNow, CancellationToken ct) => Task.FromResult(string.Equals(apiKeyId, key.ApiKeyId, StringComparison.Ordinal) ? new ApiKeyLookup(key, grants) : null); + + public Task> FindGrantsAsync(string principal, CancellationToken ct) => + Task.FromResult>( + [.. PrincipalGrants.Where(g => string.Equals(g.Principal, principal, StringComparison.OrdinalIgnoreCase))]); } private sealed class ThrowingApiKeyStore : IApiKeyStore { public Task FindAsync(string apiKeyId, DateTime utcNow, CancellationToken ct) => throw new InvalidOperationException("store is down"); + + public Task> FindGrantsAsync(string principal, CancellationToken ct) => + throw new InvalidOperationException("store is down"); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OpcUaApplicationHostImpersonationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OpcUaApplicationHostImpersonationTests.cs index 81e97547..5a9275c2 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OpcUaApplicationHostImpersonationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OpcUaApplicationHostImpersonationTests.cs @@ -68,19 +68,44 @@ public sealed class OpcUaApplicationHostImpersonationTests args.IdentityValidationError.LocalizedText.Text.ShouldContain("Invalid credentials"); } - /// Verifies anonymous identity tokens are passed through to the SDK default handler. + /// + /// An admitted anonymous session now carries a role-bearing identity (#520). + /// + /// + /// This test previously asserted the opposite — that anonymous fell through untouched to the SDK + /// default identity. That was the pre-#520 behaviour and is exactly what changed: anonymous is a + /// normal principal whose grants come from ACL rows, so it must arrive at the downstream gates as + /// the same a key produces, or every gate would need an + /// anonymous-shaped branch. + /// [Fact] - public void HandleImpersonation_anonymous_token_falls_through_to_sdk_default() + public void HandleImpersonation_anonymous_token_is_admitted_with_a_role_carrying_identity() { var args = new ImpersonateEventArgs(new AnonymousIdentityToken(), AnonPolicy, new EndpointDescription()); var authenticator = new RecordingAuthenticator(OpcUaUserAuthResult.Allow("x", Array.Empty())); OpcUaApplicationHost.HandleImpersonation(authenticator, args, NullLogger.Instance); - // Handler leaves anonymous tokens untouched — no identity, no validation error. - args.Identity.ShouldBeNull(); args.IdentityValidationError.ShouldBeNull(); - authenticator.LastUsername.ShouldBeNull("anonymous tokens must not hit the authenticator"); + args.Identity.ShouldBeOfType(); + // Still no UserName authentication — the anonymous path must not consult the credential seam. + authenticator.LastUsername.ShouldBeNull("anonymous tokens must not hit the UserName authenticator"); + } + + /// + /// With anonymous disallowed the session is rejected outright rather than admitted with no roles. + /// + [Fact] + public void HandleImpersonation_anonymous_token_is_rejected_when_disallowed() + { + var args = new ImpersonateEventArgs(new AnonymousIdentityToken(), AnonPolicy, new EndpointDescription()); + var authenticator = new RecordingAuthenticator(OpcUaUserAuthResult.Allow("x", Array.Empty())); + + OpcUaApplicationHost.HandleImpersonation( + authenticator, args, NullLogger.Instance, allowAnonymous: false); + + args.Identity.ShouldBeNull(); + args.IdentityValidationError.Code.ShouldBe(StatusCodes.BadIdentityTokenRejected); } /// Verifies authenticator exceptions result in rejection with validation error. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OpcUaApplicationHostSecurityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OpcUaApplicationHostSecurityTests.cs index 8a8bc293..bcb13bee 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OpcUaApplicationHostSecurityTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OpcUaApplicationHostSecurityTests.cs @@ -88,6 +88,20 @@ public sealed class OpcUaApplicationHostSecurityTests : IDisposable userName.SecurityPolicyUri.ShouldBe(SecurityPolicies.Basic256Sha256); } + /// + /// With OpcUa:AllowAnonymous false the Anonymous policy is not advertised, so a client is + /// refused at discovery rather than only at activation — but UserName survives, which is what + /// makes the server key-only rather than unreachable (#520). + /// + [Fact] + public void BuildUserTokenPolicies_withholds_anonymous_when_disallowed() + { + var tokens = OpcUaApplicationHost.BuildUserTokenPolicies(allowAnonymous: false).ToList(); + + tokens.ShouldNotContain(t => t.TokenType == UserTokenType.Anonymous); + tokens.ShouldContain(t => t.TokenType == UserTokenType.UserName); + } + /// /// Verifies that StartAsync populates ServerConfiguration with all enabled security profiles. ///