fix(GWC-38): deny Galaxy browse when the ambient identity is absent under API-key auth

GatewayBrowseScopeProvider resolved `identityAccessor.Current?.EffectiveConstraints
?? ApiKeyConstraints.Empty`, so an absent ambient identity produced an empty
BrowseSubtrees list — which the shared GalaxyRepository library reads as "no
scoping". The control's fail state was therefore open: any future path reaching the
library's galaxy service without GatewayGrpcAuthorizationInterceptor having pushed
the identity would silently hand a browse-scoped key the full hierarchy, with
nothing failing and nothing logged. A constraint-narrowing control's absence is
invisible in normal operation, which is what made the honest "safe only because the
global interceptor runs first" comment insufficient on its own.

The provider now distinguishes the two states the old expression collapsed:

- authenticated key with no BrowseSubtrees constraint -> empty list, unscoped
  (unchanged, and now asserted in its own test rather than implied);
- no ambient identity with Authentication:Mode=ApiKey -> log a warning naming the
  broken ordering invariant and throw RpcException(PermissionDenied);
- no ambient identity with Authentication:Mode=Disabled -> unscoped, because
  nothing authenticates there so no key and no constraints exist to scope by.

The registration-order comment at GatewayApplication.cs now says the ordering is
still required but no longer silently trusted.

Tests: GatewayBrowseScopeProviderTests gains the PermissionDenied case and the
Disabled-mode case alongside the unconstrained-identity one.
GalaxyRepositoryHostWiringTests phase 1 now pushes an identity carrying no
BrowseSubtrees instead of pushing none — that, not an absent identity, is
production's unscoped case.

Docs: docs/Authorization.md's constraint section states the two-state distinction
and both fail behaviours under browse_subtrees.
This commit is contained in:
Joseph Doherty
2026-08-20 09:28:54 -04:00
parent f4919ba97c
commit b177636d7a
5 changed files with 169 additions and 22 deletions
@@ -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<string>? result = provider.ResolveBrowseSubtrees(new TestServerCallContext());
@@ -27,14 +31,16 @@ public sealed class GatewayBrowseScopeProviderTests
}
/// <summary>
/// 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.
/// </summary>
[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<string>? result = provider.ResolveBrowseSubtrees(new TestServerCallContext());
@@ -42,6 +48,55 @@ public sealed class GatewayBrowseScopeProviderTests
Assert.Empty(result);
}
/// <summary>
/// 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.
/// </summary>
[Fact]
public void ResolveBrowseSubtrees_WithNoCurrentIdentityAndApiKeyAuth_ThrowsPermissionDenied()
{
StubIdentityAccessor accessor = new(current: null);
GatewayBrowseScopeProvider provider = CreateProvider(accessor, AuthenticationMode.ApiKey);
RpcException exception = Assert.Throws<RpcException>(
() => provider.ResolveBrowseSubtrees(new TestServerCallContext()));
Assert.Equal(StatusCode.PermissionDenied, exception.StatusCode);
}
/// <summary>
/// Under <c>Authentication:Mode=Disabled</c> 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.
/// </summary>
[Fact]
public void ResolveBrowseSubtrees_WithNoCurrentIdentityAndAuthDisabled_ReturnsEmptyList()
{
StubIdentityAccessor accessor = new(current: null);
GatewayBrowseScopeProvider provider = CreateProvider(accessor, AuthenticationMode.Disabled);
IReadOnlyList<string>? 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<GatewayBrowseScopeProvider>.Instance);
private static ApiKeyIdentity CreateIdentity(IReadOnlyList<string> browseSubtrees) =>
new(
KeyId: "operator01",