feat(acl): variable-depth scope model + the scopePaths producer (#520 stage 3)
Makes the /raw device tree addressable by an ACL rule, and builds the
scope-path producer PermissionTrieBuilder has accepted since it was written
but which had no caller anywhere.
The plan for this stage said "add NodeHierarchyKind.Raw, whose path is the
RawPath segments Folder/Driver/Device/TagGroup/Tag". That shape is wrong.
RawFolder.ParentRawFolderId and TagGroup.ParentTagGroupId are both
self-referencing, so folders and tag groups each nest without bound and a real
RawPath is folder…/Driver/Device/group…/Tag. NodeScope was fixed-arity and
PermissionTrie.CollectMatches was five hardcoded steps, so the scope model
could not address a real raw tree at all. NodeScope is now {ClusterId, Kind,
Segments} and the walk is a segment loop. The trie STRUCTURE was already
variable-depth (NodeAclPath.Segments is a list, Children is a recursive
dictionary), so only the walker and the scope record had to change.
Four decisions worth keeping:
- The namespace level became the realm. Level 1 gated on NodeScope.NamespaceId,
but v3 has no Namespace entity — the snapshot was retired when the address
space went dual-namespace, and the identifier appears nowhere in production
outside NodeScope. So the level could only ever fail to match, which silently
made every sub-cluster grant unreachable while cluster grants kept working.
It is now a realm token derived from the scope's Kind, not carried on it, so
the walk and the producer cannot disagree about its spelling.
- Provenance comes from the granting row's own ScopeKind, recorded on TrieGrant
at build time. Depth-derived provenance could not name a raw level at all,
since the depth of "Device" varies with folder nesting.
- The trie comparer moved from OrdinalIgnoreCase to Ordinal. Raw sibling-name
uniqueness is ordinal, so Speed and speed can both exist under one device and
a grant on either would have matched both.
- The null-scopePaths "deterministic test mode" is deleted. It planted
unresolvable rows at a direct child of the trie root, a position the walker
cannot reach, so the grant looked placed and could never match — and every
test using that mode proved nothing about production placement. scopePaths is
now required, and a row that does not resolve is dropped with a diagnostic.
A scope resolves to a LIST of paths: a raw tag projected into N equipments is
reachable at N+1 NodeIds and a write through any of them lands on the same
driver reference, so a Tag-scoped grant covering only the raw node would be
bypassable through a UNS one. The lookup is keyed on (ScopeKind, ScopeId), not
ScopeId alone — ScopeId has no FK and no type discriminator, so an id collision
across entity types would hand a grant the wrong node's paths.
Also closed here, all found by review rather than asked for:
- NodeAcl rows had no deploy-time validation of any kind and were not reachable
from DraftValidator at all (DraftSnapshot carried no NodeAcls). They are now
gated for dangling ids, cross-cluster scopes, blank/contradictory ScopeId, the
retired FolderSegment kind, and undefined ordinals.
- ConfigDbApiKeyStore conferred roles from Cluster-scoped grants authored in ANY
cluster, because it filtered on scope kind with no ClusterId predicate while
ArtifactApiKeyStore read an already-cluster-filtered snapshot. On a fused
admin+driver node that was a cross-cluster over-grant. Its doc comment claimed
the two stores matched.
- The two AdminUI ACL banners still said enforcement was LDAP-group -> role
mapping, which stages 1/2 made false in both directions.
- The /apikeys page was never classified in PageAuthorizationGuardTests, so that
test has been red since the key-minting commit merged — I did not run
AdminUI.Tests before merging stage 4.
Falsified rather than assumed: 8 mutations against the trie tests, 5 against the
deploy gate (including one proving a cross-cluster row is not silently
mis-diagnosed as unresolved), a staged red->green for the over-grant regression,
and the realm gate, the comparer and the malformed-path guard each broken
individually to confirm only their own tests go red.
Node ACLs are still not ENFORCED — nothing constructs a NodeScope or calls the
evaluator yet. That is stages 5 and 6; this stage makes the model able to
express what those will enforce.
Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
This commit is contained in:
@@ -78,9 +78,9 @@ string; only what produces that string changes.
|
||||
|
||||
| # | #520's blocker | Resolution under this design |
|
||||
|---|---|---|
|
||||
| 1 | Raw-realm nodes have no representable `NodeScope` (`NodeHierarchyKind` has one member, `Equipment`) | **Add `NodeHierarchyKind.Raw`**, whose path is the RawPath segments `Folder/Driver/Device/TagGroup/Tag`. `NodeAclScopeKind` already carries `Namespace` and `FolderSegment` — `FolderSegment` becomes matchable for the first time, at raw-folder level. This is the one genuine **design** item, not wiring. |
|
||||
| 1 | Raw-realm nodes have no representable `NodeScope` (`NodeHierarchyKind` has one member, `Equipment`) | **Add `NodeHierarchyKind.Raw`.** ⚠️ **This cell was wrong about the shape** — it described the path as the five-tuple `Folder/Driver/Device/TagGroup/Tag`, but folders and tag groups both self-nest, so the real path is `folder… / Driver / Device / group… / Tag` with unbounded depth on two axes. The fix was therefore not an enum member but a variable-depth `NodeScope` + segment-loop walk. `FolderSegment` was **not** repurposed as the raw-folder level; it is retired, and `RawFolder`/`RawDriver`/`RawDevice`/`RawTagGroup` were appended instead. Correctly identified as the one genuine **design** item. |
|
||||
| 2 | Session carries roles, not LDAP groups | **Dissolved by D1** — the principal comes from the key, not from a directory. |
|
||||
| 3 | `PermissionTrieBuilder`'s `scopePaths` has no producer | Built node-side from the artifact's UNS hierarchy + raw tree. Note every existing evaluator unit test runs in the no-`scopePaths` "deterministic test mode", so **the production hierarchy path is currently untested** — it needs its own coverage, not just a producer. |
|
||||
| 3 | `PermissionTrieBuilder`'s `scopePaths` has no producer | **Done in stage 3** — `AclScopePathResolver`, built node-side from the artifact's raw + UNS topology (no DB read, so a driver-only node can build it). The coverage note was right in substance but imprecise: six tests *did* pass a `scopePaths` dictionary, but every one hand-wrote it in the test body with exactly the segments the walker expected, so nothing pinned a producer — and there was none to pin. That gap is closed by an artifact → parse → build → walk test. |
|
||||
| 4 | No session lifecycle / `AuthGenerationId` | Stamp the generation from the artifact `RevisionHash` at session activation; a new deploy invalidates cached decisions. `UserAuthorizationState` already has the freshness predicates. |
|
||||
|
||||
Plus, from the same audit: the node never parses ACLs out of the artifact, and `OtOpcUaNodeManager`
|
||||
@@ -99,11 +99,18 @@ mismatch).
|
||||
## Smaller inconsistencies to settle en route
|
||||
|
||||
- `NodeAclScopeKind.FolderSegment` is offered in the authoring dropdown but the trie walker has no
|
||||
level to match it at — fixed by blocker #1's `Raw` kind.
|
||||
level to match it at — **settled in stage 3, but not the way this line assumed.** It is a retired v2
|
||||
concept (it described the `SystemPlatform`/Galaxy hierarchy v3 deleted), so rather than being given a
|
||||
level it is now explicitly unresolvable, with `RawFolder` as its v3 replacement. It survives in the
|
||||
enum only because the artifact stores the ordinal, so removing it would renumber every later member
|
||||
in every deployed artifact. The dropdown labels it retired, and the deploy gate names it.
|
||||
⚠️ The old claim that the walker could not match it was slightly overstated: `Build` routed it through
|
||||
`Descend` like any other kind, so given a path it *would* have matched and been reported under
|
||||
whatever level it happened to land on. It was unlabellable, not unplaceable.
|
||||
- `NodePermissions.AlarmRead` has no `OpcUaOperation` mapping — grantable but unreachable.
|
||||
- `NodeAcl.ScopeId` has no FK and no existence check, so scope ids dangle silently.
|
||||
- `ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj` does not reference `Core` at all — the evaluator is not even
|
||||
on the node manager's compile-time reference graph.
|
||||
- ~~`ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj` does not reference `Core` at all~~ — **stale**; stage 1 added
|
||||
the reference. The line read as an open item long after it was closed.
|
||||
- `NodePermissions.HistoryUpdate` is deliberately in no bundle until HistoryUpdate exists. Leave it.
|
||||
|
||||
## Stages
|
||||
@@ -115,8 +122,55 @@ Each stage is one commit, builds clean, and leaves the tree shippable.
|
||||
2. ✅ **Identity cutover.** `ApiKeyOpcUaUserAuthenticator` replaces `LdapOpcUaUserAuthenticator` on the
|
||||
OPC UA data plane; Anonymous resolves to the `anonymous` principal or is rejected per setting.
|
||||
AdminUI LDAP untouched.
|
||||
3. **Raw scope.** *(not started — sequenced after stage 4 at the owner's request)* `NodeHierarchyKind.Raw` + RawPath scope paths; `scopePaths` producer built from the
|
||||
artifact; coverage for the production hierarchy path that today has none.
|
||||
3. **Raw scope.** ✅ **Done.** `NodeHierarchyKind.Raw`, a variable-depth scope model, the `scopePaths`
|
||||
producer (`AclScopePathResolver`) fed from the artifact, a deploy-time gate for ACL scopes, and the
|
||||
first coverage of the production hierarchy path.
|
||||
|
||||
**It was substantially larger than this line originally implied, because the sketch had the shape of
|
||||
the raw tree wrong.** The plan said the Raw path is the five-tuple
|
||||
`Folder/Driver/Device/TagGroup/Tag`. It is not: `RawFolder.ParentRawFolderId` and
|
||||
`TagGroup.ParentTagGroupId` are both self-referencing, so folders and tag groups each nest without
|
||||
bound and a real RawPath is `folder… / Driver / Device / group… / Tag`. `NodeScope` was fixed-arity
|
||||
(five nullable level fields) and `PermissionTrie.CollectMatches` was five hardcoded steps, so the
|
||||
scope model could not address a real raw tree at all. `NodeScope` is now `{ClusterId, Kind,
|
||||
IReadOnlyList<string> Segments}` and the walk is a segment loop. Good news found en route: the trie
|
||||
*structure* was already variable-depth (`NodeAclPath.Segments` is a list; `PermissionTrieNode.Children`
|
||||
is a recursive dictionary), so only the walker and the scope record had to change.
|
||||
|
||||
Four decisions worth keeping:
|
||||
|
||||
- **The namespace level became the realm.** Level 1 of the walk gated on `NodeScope.NamespaceId`, but
|
||||
v3 **has no `Namespace` entity** — the snapshot was retired when the address space went
|
||||
dual-namespace, and the identifier appeared nowhere in production outside `NodeScope`. So level 1
|
||||
could only ever fail to match, which silently made *every* sub-cluster grant unreachable while
|
||||
cluster grants kept working. It is now the realm token (`raw`/`uns`), derived from the scope's
|
||||
`Kind` by `NodeAclRealms` rather than carried on it, so the walk and the path producer cannot
|
||||
disagree about its spelling. `NodeAclScopeKind.Namespace` becomes meaningful again as "a whole
|
||||
realm".
|
||||
- **Provenance comes from the granting row's `ScopeKind`**, recorded on `TrieGrant` at build time,
|
||||
not from the depth the grant sits at. Depth-derived provenance could not name a raw level at all
|
||||
(the depth of "Device" varies with folder nesting) and misreported any path whose length disagreed
|
||||
with its kind.
|
||||
- **The trie comparer moved from `OrdinalIgnoreCase` to `Ordinal`.** Raw sibling-name uniqueness is
|
||||
ordinal, so `Speed` and `speed` can both exist under one device and a grant on either would have
|
||||
matched both — a silent widening. Case-sensitive is also the fail-closed direction.
|
||||
- **The null-`scopePaths` "deterministic test mode" is deleted.** It planted unresolvable rows at a
|
||||
direct child of the trie root, a position the walker cannot reach — so the grant looked placed and
|
||||
could never match, and every test using that mode proved nothing about production placement. An
|
||||
unresolvable row is now dropped with a diagnostic, and `scopePaths` is required.
|
||||
|
||||
A scope resolves to a **list** of paths, not one: a raw tag projected into N equipments is reachable
|
||||
at N+1 NodeIds and a write through any of them lands on the same driver reference, so a Tag-scoped
|
||||
grant that covered only the raw node would be bypassable through a UNS one. The lookup is keyed on
|
||||
`(ScopeKind, ScopeId)` rather than `ScopeId` alone, because `ScopeId` has no FK and no type
|
||||
discriminator — ids are prefixed only by convention, and an id collision across entity types would
|
||||
hand a grant the wrong node's paths.
|
||||
|
||||
Also closed here: `NodeAcl` rows had **no deploy-time validation of any kind** and were not even
|
||||
reachable from `DraftValidator` (`DraftSnapshot` carried no `NodeAcls` collection). They are now
|
||||
gated for dangling ids, cross-cluster scopes, blank/contradictory `ScopeId`, the retired
|
||||
`FolderSegment` kind, and undefined ordinals. That gate is what substitutes for the ACL scope picker
|
||||
until stage 7 — `ScopeId` is still a free-text box.
|
||||
4. **Artifact + lifecycle.** ✅ **Done.** `ConfigComposer` snapshots `ApiKeys` alongside the `NodeAcls`
|
||||
it already carried; `DeploymentArtifact.ParseDataPlaneAuthority` decodes both node-side (grants
|
||||
scoped to the node's own cluster, keys not — a key is a fleet-wide credential and the ACL rows are
|
||||
|
||||
Reference in New Issue
Block a user