diff --git a/docs/Authorization.md b/docs/Authorization.md index b1fb269..12c393d 100644 --- a/docs/Authorization.md +++ b/docs/Authorization.md @@ -180,6 +180,21 @@ Supported constraints are: | `read_historized_only` | Read/subscription commands must target objects with historized attributes. | | `dashboard_tags` | Dashboard event-visibility tags. **Not a data-access constraint** — see below. | +`browse_subtrees` is applied by `GatewayBrowseScopeProvider`, the host seam the +shared `ZB.MOM.WW.GalaxyRepository` package calls through `IGalaxyBrowseScopeProvider`; +it reads the constraint off the ambient per-call identity that +`GatewayGrpcAuthorizationInterceptor` pushed earlier in the same call. An empty +`browse_subtrees` list is the library's "no scoping" signal, so the provider has to +distinguish two states that look alike (GWC-38): an authenticated key with no browse +constraint is legitimately unscoped and returns the empty list, whereas **no ambient +identity at all** means the interceptor did not run before the Galaxy service and the +request's scope cannot be established. In that second case, with +`MxGateway:Authentication:Mode = ApiKey`, the provider logs a warning naming the broken +ordering invariant and fails the call with `PermissionDenied` rather than serving the full +hierarchy — a narrowing control's silent absence is invisible in normal operation, so it is +made loud instead. Under `Mode = Disabled` an absent identity is the expected steady state +(nothing authenticates, so no key and no constraints exist) and browse stays unscoped. + `dashboard_tags` is the one member of the blob that constrains nothing on the gRPC data path. No read, write, browse, or subscribe check consults it, and `HasReadConstraints` / `HasWriteConstraints` deliberately ignore it: adding a tag diff --git a/src/ZB.MOM.WW.MxGateway.Server/GatewayApplication.cs b/src/ZB.MOM.WW.MxGateway.Server/GatewayApplication.cs index d5415c8..e075983 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/GatewayApplication.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/GatewayApplication.cs @@ -150,6 +150,12 @@ public static class GatewayApplication builder.Services.Configure(o => o.AddSecretsAuthorization()); // Register the gateway's browse-scope provider before AddZbGalaxyRepository so the // library's TryAddSingleton default (NullGalaxyBrowseScopeProvider) does not win. + // The provider depends on GatewayGrpcAuthorizationInterceptor (registered globally above) + // having pushed the ambient identity before the library's galaxy service runs. That + // ordering is still required, but it is no longer trusted silently: with + // Authentication:Mode=ApiKey the provider denies with PermissionDenied and logs the broken + // invariant when the identity is absent, instead of resolving to the full hierarchy + // (GWC-38). Under Mode=Disabled identity-absent is expected and stays unscoped. builder.Services.AddSingleton(); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/GatewayBrowseScopeProvider.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/GatewayBrowseScopeProvider.cs index a7426e0..94186bd 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/GatewayBrowseScopeProvider.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/GatewayBrowseScopeProvider.cs @@ -1,26 +1,72 @@ using Grpc.Core; +using Microsoft.Extensions.Options; using ZB.MOM.WW.GalaxyRepository.Grpc; +using ZB.MOM.WW.MxGateway.Server.Configuration; using ZB.MOM.WW.MxGateway.Server.Security.Authentication; namespace ZB.MOM.WW.MxGateway.Server.Security.Authorization; /// Scopes Galaxy browse results to the calling API key's BrowseSubtrees constraint. -public sealed class GatewayBrowseScopeProvider(IGatewayRequestIdentityAccessor identityAccessor) +/// +/// Distinguishes two states the pre-GWC-38 code collapsed into one: "authenticated, no +/// BrowseSubtrees constraint" (legitimately unscoped — the common case) and "no ambient identity +/// at all" (the interceptor-ordering invariant below is broken). The second used to resolve to the +/// full hierarchy silently, which made a scoping bypass invisible; it now denies loudly. +/// +/// Accessor for the ambient per-call API-key identity. +/// Gateway options supplying MxGateway:Authentication:Mode. +/// Logger used to name the broken invariant when identity is absent. +public sealed class GatewayBrowseScopeProvider( + IGatewayRequestIdentityAccessor identityAccessor, + IOptions options, + ILogger logger) : IGalaxyBrowseScopeProvider { /// Resolves the Galaxy browse subtrees the calling API key is scoped to, from the ambient request identity. /// The gRPC server call context for the in-flight browse request. /// The caller's configured browse subtrees, or /empty when the caller is unscoped (full hierarchy). + /// + /// when API-key authentication is enabled but no + /// ambient identity is present — the invariant described below has been broken and the + /// request's scope cannot be established. + /// public IReadOnlyList? ResolveBrowseSubtrees(ServerCallContext context) { // Invariant: the caller identity is the ambient one pushed by // GatewayGrpcAuthorizationInterceptor earlier in this same call. That interceptor is // registered globally and therefore runs before the library's galaxy service, so - // Current is populated for any authenticated galaxy RPC. If it is somehow absent we - // return empty BrowseSubtrees, which the library treats as "no scoping" (full - // hierarchy) — safe only because the global interceptor has already authenticated and - // authorized the request first. Do not reorder the interceptor without revisiting this. - ApiKeyConstraints constraints = identityAccessor.Current?.EffectiveConstraints ?? ApiKeyConstraints.Empty; - return constraints.BrowseSubtrees; + // Current is populated for any authenticated galaxy RPC. Do not reorder the interceptor + // without revisiting this. + ApiKeyIdentity? identity = identityAccessor.Current; + if (identity is not null) + { + return identity.EffectiveConstraints.BrowseSubtrees; + } + + // Mode=Disabled is the one configuration in which identity-absent is expected rather than + // broken: nothing authenticates, so no key and no constraints exist to scope by, and the + // deployment has already accepted an unauthenticated gRPC surface. Empty BrowseSubtrees is + // what the library reads as "no scoping". + if (options.Value.Authentication.Mode == AuthenticationMode.Disabled) + { + return ApiKeyConstraints.Empty.BrowseSubtrees; + } + + // Authentication is on, so an identity MUST have been pushed before the galaxy service ran. + // Failing open here would hand a browse-scoped key the full hierarchy with nothing logged + // and nothing failing — a constraint-narrowing control's absence is invisible in normal + // operation. Deny instead, and name the invariant so the misconfiguration is diagnosable + // from one log line rather than from a scoping audit nobody thought to run. + logger.LogWarning( + "Galaxy browse scope could not be resolved for {Method}: no ambient API-key identity " + + "while Authentication:Mode is {Mode}. GatewayGrpcAuthorizationInterceptor must run " + + "before the Galaxy service on every call; the request is denied rather than served " + + "unscoped.", + context.Method, + options.Value.Authentication.Mode); + + throw new RpcException(new Status( + StatusCode.PermissionDenied, + "Browse scope could not be established for this request.")); } } diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryHostWiringTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryHostWiringTests.cs index 2be85ae..0178369 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryHostWiringTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryHostWiringTests.cs @@ -1,5 +1,8 @@ +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using ZB.MOM.WW.GalaxyRepository; using ZB.MOM.WW.GalaxyRepository.Grpc; +using ZB.MOM.WW.MxGateway.Server.Configuration; using ZB.MOM.WW.MxGateway.Server.Security.Authentication; using ZB.MOM.WW.MxGateway.Server.Security.Authorization; using ZB.MOM.WW.MxGateway.Tests.TestSupport; @@ -23,8 +26,11 @@ public sealed class GalaxyRepositoryHostWiringTests { /// /// Verifies the full production authz-filtering chain end-to-end. - /// Phase 1: no identity pushed → BrowseChildren returns both children - /// (unconstrained). Phase 2: identity with BrowseSubtrees = ["NonExistent"] + /// Phase 1: an identity carrying no BrowseSubtrees pushed → BrowseChildren + /// returns both children (unconstrained). That, not an absent identity, is production's + /// unscoped case since GWC-38 — with Authentication:Mode=ApiKey an absent identity + /// now denies, which the isolated provider test pins. + /// Phase 2: identity with BrowseSubtrees = ["NonExistent"] /// pushed via → the real /// threads the constraint into the lib /// service → BrowseChildren returns empty children. @@ -35,7 +41,16 @@ public sealed class GalaxyRepositoryHostWiringTests { // Arrange: construct the REAL host seam — not stubs. GatewayRequestIdentityAccessor identityAccessor = new(); - GatewayBrowseScopeProvider scopeProvider = new(identityAccessor); + GatewayBrowseScopeProvider scopeProvider = new( + identityAccessor, + Options.Create(new GatewayOptions + { + Authentication = new Server.Configuration.AuthenticationOptions + { + Mode = AuthenticationMode.ApiKey, + }, + }), + NullLogger.Instance); GalaxyObject[] objects = CreateFilterObjects(); GalaxyHierarchyCacheEntry entry = GalaxyHierarchyCacheEntry.Empty with @@ -55,13 +70,23 @@ public sealed class GalaxyRepositoryHostWiringTests new GalaxyDeployNotifier(), scopeProvider); - // Phase 1: no identity on the accessor → browse is unconstrained. - // Pump_001 and Valve_001 are both children of Line3 (GobjectId = 2). - BrowseChildrenReply unconstrained = await service.BrowseChildren( - new BrowseChildrenRequest { ParentGobjectId = 2 }, - new TestServerCallContext()); + // Phase 1: an authenticated identity with no BrowseSubtrees constraint → browse is + // unconstrained. Pump_001 and Valve_001 are both children of Line3 (GobjectId = 2). + ApiKeyIdentity unscopedIdentity = new( + KeyId: "test-key", + KeyPrefix: "mxgw_test", + DisplayName: "unscoped", + Scopes: new HashSet(StringComparer.Ordinal) { GatewayScopes.MetadataRead }, + Constraints: ApiKeyConstraints.Empty); - Assert.Equal(2, unconstrained.Children.Count); + using (identityAccessor.Push(unscopedIdentity)) + { + BrowseChildrenReply unconstrained = await service.BrowseChildren( + new BrowseChildrenRequest { ParentGobjectId = 2 }, + new TestServerCallContext()); + + Assert.Equal(2, unconstrained.Children.Count); + } // Phase 2: push a constraint whose BrowseSubtrees do not match any seeded object. // GatewayBrowseScopeProvider.ResolveBrowseSubtrees must return ["NonExistent"], diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/GatewayBrowseScopeProviderTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/GatewayBrowseScopeProviderTests.cs index 9972670..1e0eba2 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/GatewayBrowseScopeProviderTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/GatewayBrowseScopeProviderTests.cs @@ -1,3 +1,7 @@ +using Grpc.Core; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using ZB.MOM.WW.MxGateway.Server.Configuration; using ZB.MOM.WW.MxGateway.Server.Security.Authentication; using ZB.MOM.WW.MxGateway.Server.Security.Authorization; using ZB.MOM.WW.MxGateway.Tests.TestSupport; @@ -19,7 +23,7 @@ public sealed class GatewayBrowseScopeProviderTests public void ResolveBrowseSubtrees_WithCurrentIdentity_ReturnsConstraintBrowseSubtrees() { StubIdentityAccessor accessor = new(CreateIdentity(["AreaA"])); - GatewayBrowseScopeProvider provider = new(accessor); + GatewayBrowseScopeProvider provider = CreateProvider(accessor, AuthenticationMode.ApiKey); IReadOnlyList? result = provider.ResolveBrowseSubtrees(new TestServerCallContext()); @@ -27,14 +31,16 @@ public sealed class GatewayBrowseScopeProviderTests } /// - /// With no ambient identity, the provider falls back to the empty browse-subtree - /// list, which the library treats as "no scoping" rather than denying everything. + /// An authenticated key with no BrowseSubtrees constraint is legitimately unscoped: the + /// empty list is what the library reads as "no scoping". This is the state GWC-38 required + /// to stay distinguishable from an absent identity, so it is asserted alongside its + /// denying twin below rather than left implicit. /// [Fact] - public void ResolveBrowseSubtrees_WithNoCurrentIdentity_ReturnsEmptyList() + public void ResolveBrowseSubtrees_WithUnconstrainedIdentity_ReturnsEmptyList() { - StubIdentityAccessor accessor = new(current: null); - GatewayBrowseScopeProvider provider = new(accessor); + StubIdentityAccessor accessor = new(CreateIdentity([])); + GatewayBrowseScopeProvider provider = CreateProvider(accessor, AuthenticationMode.ApiKey); IReadOnlyList? result = provider.ResolveBrowseSubtrees(new TestServerCallContext()); @@ -42,6 +48,55 @@ public sealed class GatewayBrowseScopeProviderTests Assert.Empty(result); } + /// + /// GWC-38: with API-key authentication enabled, an absent ambient identity means the + /// interceptor-ordering invariant is broken — the request's browse scope cannot be + /// established. The provider must DENY rather than resolve to the full hierarchy, because + /// a silently-unscoped browse is an invisible authorization bypass. The status code is + /// pinned: PermissionDenied is what the gRPC surface returns for an authorization failure + /// the caller cannot fix by re-authenticating. + /// + [Fact] + public void ResolveBrowseSubtrees_WithNoCurrentIdentityAndApiKeyAuth_ThrowsPermissionDenied() + { + StubIdentityAccessor accessor = new(current: null); + GatewayBrowseScopeProvider provider = CreateProvider(accessor, AuthenticationMode.ApiKey); + + RpcException exception = Assert.Throws( + () => provider.ResolveBrowseSubtrees(new TestServerCallContext())); + + Assert.Equal(StatusCode.PermissionDenied, exception.StatusCode); + } + + /// + /// Under Authentication:Mode=Disabled nothing authenticates, so an absent identity + /// is the expected steady state rather than a broken invariant: there is no key and no + /// constraint to scope by. The provider keeps returning unscoped, so disabling + /// authentication does not turn every Galaxy browse into a PermissionDenied. + /// + [Fact] + public void ResolveBrowseSubtrees_WithNoCurrentIdentityAndAuthDisabled_ReturnsEmptyList() + { + StubIdentityAccessor accessor = new(current: null); + GatewayBrowseScopeProvider provider = CreateProvider(accessor, AuthenticationMode.Disabled); + + IReadOnlyList? result = provider.ResolveBrowseSubtrees(new TestServerCallContext()); + + Assert.NotNull(result); + Assert.Empty(result); + } + + private static GatewayBrowseScopeProvider CreateProvider( + StubIdentityAccessor accessor, + AuthenticationMode mode) => + new( + accessor, + Options.Create(new GatewayOptions + { + Authentication = new Server.Configuration.AuthenticationOptions { Mode = mode }, + }), + NullLogger.Instance); + private static ApiKeyIdentity CreateIdentity(IReadOnlyList browseSubtrees) => new( KeyId: "operator01",