The #733 miss budget (MissBurst / MissRefillPerSecond) was process-wide, because
the impersonation seam did not carry the client's address, so one noisy client
could drain it for everybody while the known-id set was unavailable.
- IOpcUaUserAuthenticator gains AuthenticateUserNameAsync(username, password,
IPEndPoint? remoteEndpoint, ct) (default: ignore it and delegate); the server
calls it. IApiKeyStore gains the matching FindAsync(..., IPAddress?, ...).
- The SDK (1.5.378) exposes no client address publicly. SecureChannelRemoteEndpoint
reads it from the TCP listener's private channel map by reflection (through the
protected TransportListeners, exposed by OtOpcUaSdkServer), keyed by the
session's secure channel id. Fail-soft: null => process-wide accounting only.
- BoundedApiKeyStore charges a miss to the client's own bucket (keyed by IP
address) and then to the process-wide one, which stays as the backstop. A
refusal by either takes from neither; a hit refunds both. New options
Security:ApiKeys:MissBurstPerEndpoint (10) / MissRefillPerSecondPerEndpoint (2),
0 disables; telemetry outcome endpoint_miss_budget. At most 4096 addresses are
tracked; idle buckets are pruned and an address beyond the cap is charged
process-wide only.
Tests: BoundedApiKeyStorePerEndpointTests (noisy client vs quiet one, process-wide
backstop, refund + refill, off switch, authenticator forwards the address) and
over the wire ApiKeyMissBudgetPerEndpointTests (activation hands the authenticator
the loopback endpoint; real authenticator + bounded store refuse a client's 3rd
and 4th unknown key without a store read). Each new behaviour test fails with its
fix mutated out. Host.Tests 224/0, OpcUaServer.Tests 1019/1 (the one failure is the
wall-clock NodeManagerLockTimeBenchmarkTests under suite load; passes alone).
Docs: security.md.
Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
Clears CommentChecker findings in src/ (3,280 -> 209, the remainder being false
positives like PLC-5/IEEE-754/M0.0) by adding missing summaries/params/returns,
normalising inheritdoc usage, and removing Gitea/review tracking IDs while keeping
the rationale they annotated. Comment-only: comment-stripped sources are identical
to the parent commit.
`Security:Acl:Enabled` (default false) makes scoped `NodeAcl` grants decide
data-plane authorization at the three seams that have one, and gives HistoryRead
a check it never had. Read, Browse and Subscribe stay ungated — stage 6.
The ACL decision REPLACES the server-wide role check rather than chaining with
it. Chained, a grant would have to confer the role as well as pass the scope
check, and only Cluster-scoped grants confer a role — so every narrow grant
would have stayed inert and the flag would have looked inert too. The
load-bearing test is an ALLOW: a session carrying no roles at all writing
successfully on the strength of its grant. A deny-only test passes against a
build where the ACL is never consulted, because the role gate denies too.
Found en route: the principal never reached the session. The authenticator
passed it as OpcUaUserAuthResult.DisplayName, but RoleCarryingUserIdentity
derives DisplayName from the presented token — for a key-bearing client that is
the static username `apikey`, identical fleet-wide. So the trie's matching key
was dropped at the identity boundary, and the audit ClientUserId on a rejected
write recorded `apikey` for every consuming system. Principal and ApiKeyId are
now carried explicitly.
Beyond the stage as sketched:
- Revocation reaches an OPEN session. Every decision re-checks the
authenticating key against the current snapshot; without it a key revoked by a
deployment kept its authority until the client chose to reconnect, which for a
machine-to-machine consumer can be indefinitely.
- UserAuthorizationState is built per CALL, not per session. The per-session
shape exists so an LDAP session can re-resolve membership that changed out of
band; nothing can change out of band under API keys, so the generation stamp is
the whole invalidation signal and a per-call record picks it up for free.
IsStale/NeedsRefresh therefore cannot fire here and are documented as vestigial.
- The trie is built inside DataPlaneAuthorityState.Apply, under the same
generation as the snapshot, rather than by a second wiring at the publish
actor. An uninstalled trie denies everything — indistinguishable from working
enforcement. Deleting the install turns 13 of the 22 new tests red.
- The deploy log reports what the BUILD dropped instead of re-deriving it. The
old count agreed with the builder on one of its four drop reasons; a rule with
a blank ScopeId, a retired scope kind, or a path not starting at a realm
vanished with the log reading zero.
Both IApiKeyStore implementations widened to the full grant set. The narrowing
was redundant — ApiKeyInterimRoleMapper applies the same filter itself — and
became a trap once scope was enforced. Pinned by asserting the roles are
unchanged, with the two grants carrying different flags so it cannot pass by
accident.
Falsified by mutation: enforcement never engaging, the write seam ignoring the
ACL, the realm dropped from the scope, the revocation check removed, and the
trie never installed each turn the intended tests red and no others.
Full solution suite green.
Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
Pulled forward from stage 7 because without it the feature is unusable and
un-live-gateable: there was no way to create a key short of hand-inserting a row
and computing an HMAC by hand.
The minting logic is an IApiKeyAdminService in Core.Authorization, NOT logic in
the Razor page — this repo has no bUnit, so anything only a page can reach is
untestable, and minting credentials is not where that belongs. The page is a thin
shell over it.
Moved to Core.Authorization so both ends can reach them: ApiKeyOptions (was in
Host) and OpcUaPrincipals (was in OpcUaServer.Security). The OPC UA server
resolves principals; the AdminUI must refuse to mint a key claiming a reserved
one. OpcUaServer already gained a Core reference in stage 1b, so no new edge —
AdminUI gains one.
ApiKeyOptions is now bound UNCONDITIONALLY rather than in the driver branch: a
driver node needs the pepper to VERIFY keys and an admin node needs it to MINT
them, and getting that wrong on either side is a silent authentication failure
rather than a startup error.
Validation rules, all covered: the reserved 'anonymous' principal is refused
case-insensitively (so "ANONYMOUS" cannot slip past into a distinct principal
that the evaluator would then match case-INsensitively against anonymous's
grants); blank name/principal; a principal longer than the column, rejected
rather than truncated, because silent truncation yields a key that authenticates
and is granted nothing with no visible cause; a past expiry. Generation runs
BEFORE the DbContext is opened so an unconfigured pepper surfaces as a clear
error instead of a stored row whose hash nothing can verify.
The page shows the plaintext ONCE and says so; it is held in component state and
written nowhere else, because the server keeps only a hash. Revocation is flagged
in the UI as taking effect only after the next deployment — driver-only nodes
have no ConfigDb, so keys ride the artifact.
FleetAdmin-only (Administrator), not ConfigEditor: this mints credentials.
Core.Tests 68 green.
Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
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
Adds <summary>/<param>/<returns>/<inheritdoc> where missing and removes
project bookkeeping IDs (task/tracking refs) from shipped code comments,
so the docs read cleanly and CommentChecker is quiet except for known
false positives (PLC/protocol terms, event/IEqualityComparer inheritdoc).
Doc/comment-only; no logic changed; solution builds clean.
Adds <summary>, <param>, <typeparam>, and <inheritdoc/> tags to public
members surfaced by commentchecker — resolves 5,847 of 5,869 issues
(99.6%) across three /fixdocs passes.
Adds IOpcUaUserAuthenticator seam in OpcUaServer.Security with a deny-all
NullOpcUaUserAuthenticator default. OpcUaApplicationHost subscribes to
SessionManager.ImpersonateUser after _application.Start so UserName tokens
flow through the authenticator and either attach a UserIdentity to the
session (Allow) or set IdentityValidationError = BadIdentityTokenRejected
(Deny / authenticator exception). Anonymous + X509 tokens fall through to
SDK defaults.
LdapOpcUaUserAuthenticator (Host project) bridges to the same
ILdapAuthService that AddOtOpcUaAuth uses for Admin cookies / JWT, so a
single LDAP source-of-truth governs both Admin control plane and OPC UA
data plane. Program.cs registers LdapOptions + LdapAuthService +
IOpcUaUserAuthenticator on driver-role hosts; admin-only nodes are
unchanged.
OtOpcUaServerHostedService threads the resolved authenticator into
OpcUaApplicationHost so the seam respects Host DI.
10 new tests: 6 in OpcUaServer.Tests cover the pure HandleImpersonation
static method (success / denial / anonymous fallthrough / authenticator-
throw / null-username / Null authenticator); 4 in Host.IntegrationTests
cover the LdapOpcUaUserAuthenticator adapter (LDAP allow → Allow with
roles, LDAP deny → Deny, exception → backend-error denial, display-name
fallback). OpcUaServer suite is 40 / 40 green.
Closes#104. Unblocks Task 60 (dual-endpoint + ServiceLevel tests) once
#81 residual lands.
Joseph Doherty