228ff8b428
Caught by the Phase 0 live gate, not by the suite. Grpc.AspNetCore registers the interceptor BY TYPE, and InterceptorRegistration.GetFactory() throws "Multiple constructors accepting all given argument types have been found" when more than one public constructor is applicable. The interceptor had two: the DI one and a prefix-set overload added for later phases. The failure mode is nasty. The throw happens inside the interceptor pipeline on every call, so nothing fails at startup — the site node boots, joins, reports healthy. Every gated call then dies with Unknown / "Exception was thrown by handler", which reads as a handler bug rather than an auth bug. And it fails OPEN in the sense that matters least and closed in the sense that matters most: no call is ever authorized, but no call is ever correctly REFUSED either, so the rig showed identical errors for a correct key, a wrong key and no key at all. Live evidence, site-a: three PullAuditEvents calls, three identical InvalidOperationExceptions in the node log. Fix: the prefix-set constructor is internal (Host.Tests already has InternalsVisibleTo). Later phases extend DefaultGatedPrefixes rather than adding a second public registration shape. Why the tests missed it, and what changed: ControlPlaneAuthEndToEndTests registered the interceptor with AddSingleton alongside AddGrpc, so DI handed back the instance and Grpc.AspNetCore's activation path — the thing that throws — never ran. The harness now registers exactly as Program.cs does, by type and not in DI. Plus a direct reflection assertion that the type has exactly one public constructor, since that is the real invariant and it is cheap to pin.
235 lines
11 KiB
C#
235 lines
11 KiB
C#
using Grpc.Core;
|
|
using Microsoft.Extensions.Logging.Abstractions;
|
|
using Microsoft.Extensions.Options;
|
|
using ZB.MOM.WW.ScadaBridge.Communication;
|
|
using ZB.MOM.WW.ScadaBridge.Communication.Grpc;
|
|
|
|
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
|
|
|
/// <summary>
|
|
/// The site↔central gRPC control plane's inbound gate (ClusterClient→gRPC migration, T0.3).
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// <para>
|
|
/// <c>SiteStreamService</c> shipped unauthenticated: plaintext h2c with no interceptor, so
|
|
/// anything that could reach a site node's gRPC port could open a live data stream or pull audit
|
|
/// rows back with <c>PullAuditEvents</c>/<c>PullSiteCalls</c>. These tests pin the gate that
|
|
/// closes it, and — just as importantly — pin that it does NOT gate LocalDb sync, which has its
|
|
/// own interceptor and its own key.
|
|
/// </para>
|
|
/// <para>
|
|
/// Sibling of <see cref="LocalDbSyncAuthInterceptorTests"/>; the two interceptors share a shape
|
|
/// deliberately, so the cases mirror each other.
|
|
/// </para>
|
|
/// </remarks>
|
|
public class ControlPlaneAuthInterceptorTests
|
|
{
|
|
// Real method paths: package `sitestream`, service `SiteStreamService` (sitestream.proto).
|
|
private const string SubscribeMethod = "/sitestream.SiteStreamService/SubscribeInstance";
|
|
private const string PullAuditMethod = "/sitestream.SiteStreamService/PullAuditEvents";
|
|
private const string LocalDbSyncMethod = "/localdb_sync.v1.LocalDbSync/Sync";
|
|
|
|
private static ControlPlaneAuthInterceptor CreateInterceptor(string? psk)
|
|
=> new(
|
|
Options.Create(new CommunicationOptions { GrpcPsk = psk ?? "" }),
|
|
NullLogger<ControlPlaneAuthInterceptor>.Instance);
|
|
|
|
private static ServerCallContext CreateContext(string method, string? authorizationHeader)
|
|
{
|
|
var headers = new Metadata();
|
|
if (authorizationHeader is not null)
|
|
headers.Add("authorization", authorizationHeader);
|
|
|
|
return new FakeServerCallContext(method, headers);
|
|
}
|
|
|
|
/// <summary>
|
|
/// Minimal <see cref="ServerCallContext"/> carrying just a method name and request headers —
|
|
/// the only two things the interceptor reads. Hand-rolled for the same reason the LocalDb
|
|
/// sibling hand-rolls one: <c>Grpc.Core.Testing.TestServerCallContext</c> lives in the
|
|
/// retired native package and does not exist on the grpc-dotnet stack.
|
|
/// </summary>
|
|
private sealed class FakeServerCallContext(string method, Metadata requestHeaders)
|
|
: ServerCallContext
|
|
{
|
|
protected override string MethodCore => method;
|
|
protected override string HostCore => "localhost";
|
|
protected override string PeerCore => "ipv4:127.0.0.1:12345";
|
|
protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1);
|
|
protected override Metadata RequestHeadersCore => requestHeaders;
|
|
protected override CancellationToken CancellationTokenCore => CancellationToken.None;
|
|
protected override Metadata ResponseTrailersCore { get; } = [];
|
|
protected override Status StatusCore { get; set; }
|
|
protected override WriteOptions? WriteOptionsCore { get; set; }
|
|
protected override AuthContext AuthContextCore { get; } =
|
|
new(null, new Dictionary<string, List<AuthProperty>>());
|
|
|
|
protected override ContextPropagationToken CreatePropagationTokenCore(
|
|
ContextPropagationOptions? options)
|
|
=> throw new NotSupportedException();
|
|
|
|
protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders)
|
|
=> Task.CompletedTask;
|
|
}
|
|
|
|
/// <summary>Invokes the interceptor's unary path with a trivial continuation.</summary>
|
|
private static Task<string> Invoke(
|
|
ControlPlaneAuthInterceptor interceptor, ServerCallContext context)
|
|
=> interceptor.UnaryServerHandler<string, string>(
|
|
"request", context, (_, _) => Task.FromResult("ok"));
|
|
|
|
[Fact]
|
|
public async Task LocalDbSyncMethod_PassesThrough_BecauseItHasItsOwnGateAndItsOwnKey()
|
|
{
|
|
// Both interceptors sit on the same site AddGrpc pipeline and see every call. If this
|
|
// one also gated sync, a site would need its central-facing key to equal its pair-replication
|
|
// key — collapsing two distinct trust relationships into one secret.
|
|
var interceptor = CreateInterceptor("the-site-key");
|
|
var context = CreateContext(LocalDbSyncMethod, authorizationHeader: null);
|
|
|
|
Assert.Equal("ok", await Invoke(interceptor, context));
|
|
}
|
|
|
|
[Fact]
|
|
public async Task GatedMethod_WithNoKeyConfigured_IsDenied_EvenWithABearerToken()
|
|
{
|
|
// Fail-closed, and this is the case that differs in consequence from LocalDb's: an
|
|
// unset key here does not disable an optional feature, it closes the site's entire
|
|
// central-facing surface. Loud refusal beats silent unauthenticated service.
|
|
var interceptor = CreateInterceptor(psk: null);
|
|
var context = CreateContext(SubscribeMethod, "Bearer anything-at-all");
|
|
|
|
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
|
|
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task GatedMethod_WithNoBearerToken_IsDenied()
|
|
{
|
|
var interceptor = CreateInterceptor("the-site-key");
|
|
var context = CreateContext(SubscribeMethod, authorizationHeader: null);
|
|
|
|
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
|
|
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task GatedMethod_WithWrongBearerToken_IsDenied()
|
|
{
|
|
var interceptor = CreateInterceptor("the-site-key");
|
|
var context = CreateContext(SubscribeMethod, "Bearer some-other-sites-key");
|
|
|
|
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
|
|
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task GatedMethod_WithCorrectBearerToken_PassesThrough()
|
|
{
|
|
var interceptor = CreateInterceptor("the-site-key");
|
|
var context = CreateContext(SubscribeMethod, "Bearer the-site-key");
|
|
|
|
Assert.Equal("ok", await Invoke(interceptor, context));
|
|
}
|
|
|
|
[Fact]
|
|
public async Task GatedMethod_WithCorrectKey_ButNoBearerScheme_IsDenied()
|
|
{
|
|
// A raw key with no "Bearer " prefix is not what ControlPlaneCredentials sends;
|
|
// accepting it would widen the accepted credential shape for nothing.
|
|
var interceptor = CreateInterceptor("the-site-key");
|
|
var context = CreateContext(SubscribeMethod, "the-site-key");
|
|
|
|
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
|
|
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task GatedMethod_TokenComparison_IsNotAPrefixMatch()
|
|
{
|
|
// A StartsWith comparison would accept a truncated key and make the secret recoverable
|
|
// one character at a time. FixedTimeEquals also rejects on length.
|
|
var interceptor = CreateInterceptor("the-site-key");
|
|
var context = CreateContext(SubscribeMethod, "Bearer the-site-ke");
|
|
|
|
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
|
|
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task ServerStreaming_IsGated_BecauseThatIsHowSubscriptionsActuallyRun()
|
|
{
|
|
// SubscribeInstance/SubscribeSite are server-streaming. Gating only the unary path
|
|
// would leave the live data feed wide open while every unary test still passed.
|
|
var interceptor = CreateInterceptor("the-site-key");
|
|
var context = CreateContext(SubscribeMethod, "Bearer the-wrong-key");
|
|
|
|
var ex = await Assert.ThrowsAsync<RpcException>(() =>
|
|
interceptor.ServerStreamingServerHandler<string, string>(
|
|
"request",
|
|
responseStream: null!,
|
|
context,
|
|
(_, _, _) => Task.CompletedTask));
|
|
|
|
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task PullRpcs_AreGated_BecauseTheyReturnAuditRows()
|
|
{
|
|
// The strongest reason this gate exists: PullAuditEvents/PullSiteCalls hand back audit
|
|
// content to anyone who asks. Unary, so it would be easy to miss in a streaming-focused
|
|
// reading of the service.
|
|
var interceptor = CreateInterceptor("the-site-key");
|
|
var context = CreateContext(PullAuditMethod, authorizationHeader: null);
|
|
|
|
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
|
|
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task GatedPrefixes_AreConstructorProvided_SoLaterPhasesAddServicesNotInterceptors()
|
|
{
|
|
// Phases 1A/1B add CentralControlService and SiteCommandService to this same gate.
|
|
var interceptor = new ControlPlaneAuthInterceptor(
|
|
Options.Create(new CommunicationOptions { GrpcPsk = "k" }),
|
|
NullLogger<ControlPlaneAuthInterceptor>.Instance,
|
|
new[] { "/scadabridge.sitecommand.v1.SiteCommandService/" });
|
|
|
|
var gated = CreateContext("/scadabridge.sitecommand.v1.SiteCommandService/ExecuteQuery", null);
|
|
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, gated));
|
|
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
|
|
|
// ...and the default set is no longer implied once an explicit set is supplied.
|
|
var notGated = CreateContext(SubscribeMethod, authorizationHeader: null);
|
|
Assert.Equal("ok", await Invoke(interceptor, notGated));
|
|
}
|
|
|
|
[Fact]
|
|
public void TheInterceptorHasExactlyOnePublicConstructor()
|
|
{
|
|
// Grpc.AspNetCore registers this interceptor BY TYPE, and
|
|
// InterceptorRegistration.GetFactory() throws "Multiple constructors accepting all given
|
|
// argument types have been found" the moment a second public constructor is applicable.
|
|
// The throw lands inside the pipeline on every call, so the symptom is not a startup
|
|
// failure but a gate that authorizes nothing and fails everything with
|
|
// Unknown / "Exception was thrown by handler" — a shape that looks like a handler bug,
|
|
// not an auth bug. This shipped once and was caught only on the docker rig; the
|
|
// prefix-set constructor is internal now to keep it from recurring.
|
|
var publicCtors = typeof(ControlPlaneAuthInterceptor).GetConstructors();
|
|
|
|
Assert.Single(publicCtors);
|
|
}
|
|
|
|
[Fact]
|
|
public void DefaultGatedPrefixes_MatchTheRealSiteStreamServicePath()
|
|
{
|
|
// A typo here disables the whole gate silently: every call would simply pass through.
|
|
// Pin it against a path taken from the generated service, not from the proto text.
|
|
var method = SiteStreamService.Descriptor.FullName;
|
|
Assert.Contains(
|
|
ControlPlaneAuthInterceptor.DefaultGatedPrefixes,
|
|
p => p == $"/{method}/");
|
|
}
|
|
}
|