feat(localdb): fail-closed auth on the sync endpoint + replication health signal
Tasks 8 and 9 of the LocalDb Phase 1 adoption plan.
Task 8 - LocalDbSyncAuthInterceptor. The replication library's LocalDbSyncService
verifies nothing; inbound auth is explicitly the host's job. Without this,
anything able to reach a site node's gRPC port could stream arbitrary rows into
the consolidated site database - including OperationTracking, which central
reconciles from.
Scoped strictly to /localdb_sync.v1.LocalDbSync/; SiteStream shares the same
AddGrpc pipeline and passes through untouched. Fail-closed: with no
LocalDb:Replication:ApiKey configured NO sync stream is accepted, authenticated
or not. That is deliberate - "no key" is the default every site node ships with,
so treating it as "no auth required" would expose the endpoint on precisely the
most common configuration. Comparison is FixedTimeEquals over UTF-8 bytes.
All four server handler shapes are gated, not just unary: the sync RPC is a
bidirectional stream, so gating only the unary path would leave the real endpoint
open while every unary test still passed. There is a test for that.
Deviation from the plan: it specified Grpc.Core.Testing for the fake
ServerCallContext. That type ships in the retired native Grpc.Core package and
does not exist on the grpc-dotnet stack this solution uses; a minimal
FakeServerCallContext in the test file was the better trade than adding a dead
dependency.
Task 9 - ISyncStatus onto the site health report as LocalDbReplicationConnected
and LocalDbOplogBacklog, via a delegate-seam hosted service following the
AddSiteEventLogHealthMetricsBridge precedent (HealthMonitoring takes no reference
on the replication library). Both are additive init properties, so the
Akka-remoted SiteHealthReport constructor signature is untouched.
Both fields are nullable and the distinction is load-bearing:
- null = the reporter has not run / replication is not wired ("no data");
- false/0 = a real reading. On a node with no peer that IS the healthy
default-OFF state, not an outage.
OplogBacklog is passed through nullable end-to-end because ISyncStatus returns
null when the poll fails - flattening it to 0 would report a replication pair
that cannot read its own oplog as perfectly healthy. The collector stores both
values as one tuple and CollectReport reads it once, so a torn read cannot pair a
fresh Connected with a stale backlog.
Verified: build 0 warnings; Host 307/307 (8 interceptor + 3 health tests new),
HealthMonitoring 97/97, Commons 684/684.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
This commit is contained in:
@@ -0,0 +1,171 @@
|
||||
using Grpc.Core;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Microsoft.Extensions.Options;
|
||||
using ZB.MOM.WW.LocalDb.Replication;
|
||||
using ZB.MOM.WW.ScadaBridge.Host;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// LocalDb Phase 1 (Task 8) — the passive sync endpoint's inbound gate.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The replication library's <c>LocalDbSyncService</c> verifies nothing; inbound auth is
|
||||
/// explicitly the host's job. Without this interceptor, anything able to reach a site
|
||||
/// node's gRPC port could stream arbitrary rows straight into the consolidated site
|
||||
/// database — including into <c>OperationTracking</c>, which central reconciles from.
|
||||
/// </remarks>
|
||||
public class LocalDbSyncAuthInterceptorTests
|
||||
{
|
||||
private const string SyncMethod = "/localdb_sync.v1.LocalDbSync/Sync";
|
||||
private const string SiteStreamMethod = "/scadabridge.SiteStream/Connect";
|
||||
|
||||
private static LocalDbSyncAuthInterceptor CreateInterceptor(string? apiKey)
|
||||
=> new(
|
||||
Options.Create(new ReplicationOptions { ApiKey = apiKey }),
|
||||
NullLogger<LocalDbSyncAuthInterceptor>.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.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Hand-rolled rather than using <c>Grpc.Core.Testing.TestServerCallContext</c>: that
|
||||
/// type ships in the retired native <c>Grpc.Core</c> package and does not exist on the
|
||||
/// grpc-dotnet stack this solution runs on. Pulling in the dead package to get one
|
||||
/// test helper would be a worse trade than these few overrides.
|
||||
/// </remarks>
|
||||
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(
|
||||
LocalDbSyncAuthInterceptor interceptor, ServerCallContext context)
|
||||
=> interceptor.UnaryServerHandler<string, string>(
|
||||
"request", context, (_, _) => Task.FromResult("ok"));
|
||||
|
||||
[Fact]
|
||||
public async Task NonSyncMethod_PassesThrough_EvenWithNoKeyConfigured()
|
||||
{
|
||||
// The interceptor is registered on the shared site AddGrpc pipeline, so it sees
|
||||
// every call. It must be scoped strictly to the sync service — gating SiteStream
|
||||
// would break site↔central communication outright.
|
||||
var interceptor = CreateInterceptor(apiKey: null);
|
||||
var context = CreateContext(SiteStreamMethod, authorizationHeader: null);
|
||||
|
||||
Assert.Equal("ok", await Invoke(interceptor, context));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SyncMethod_WithNoKeyConfigured_IsDenied_EvenWithABearerToken()
|
||||
{
|
||||
// Fail-closed. "No key configured" is the DEFAULT every site node ships with, so
|
||||
// treating it as "no auth required" would silently expose the endpoint on exactly
|
||||
// the configuration that is most common. Presenting a token must not help.
|
||||
var interceptor = CreateInterceptor(apiKey: null);
|
||||
var context = CreateContext(SyncMethod, "Bearer anything-at-all");
|
||||
|
||||
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
|
||||
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SyncMethod_WithNoBearerToken_IsDenied()
|
||||
{
|
||||
var interceptor = CreateInterceptor("the-shared-key");
|
||||
var context = CreateContext(SyncMethod, authorizationHeader: null);
|
||||
|
||||
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
|
||||
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SyncMethod_WithWrongBearerToken_IsDenied()
|
||||
{
|
||||
var interceptor = CreateInterceptor("the-shared-key");
|
||||
var context = CreateContext(SyncMethod, "Bearer the-wrong-key");
|
||||
|
||||
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
|
||||
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SyncMethod_WithCorrectBearerToken_PassesThrough()
|
||||
{
|
||||
var interceptor = CreateInterceptor("the-shared-key");
|
||||
var context = CreateContext(SyncMethod, "Bearer the-shared-key");
|
||||
|
||||
Assert.Equal("ok", await Invoke(interceptor, context));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SyncMethod_WithCorrectKey_ButNoBearerScheme_IsDenied()
|
||||
{
|
||||
// A raw key with no "Bearer " prefix is not what SyncBackgroundService sends, and
|
||||
// accepting it would widen the accepted credential shape for no reason.
|
||||
var interceptor = CreateInterceptor("the-shared-key");
|
||||
var context = CreateContext(SyncMethod, "the-shared-key");
|
||||
|
||||
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
|
||||
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SyncMethod_TokenComparison_IsNotAPrefixMatch()
|
||||
{
|
||||
// A prefix/StartsWith comparison would accept a truncated key and make the secret
|
||||
// recoverable one character at a time.
|
||||
var interceptor = CreateInterceptor("the-shared-key");
|
||||
var context = CreateContext(SyncMethod, "Bearer the-shared-ke");
|
||||
|
||||
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
|
||||
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DuplexStreaming_IsGated_BecauseThatIsHowSyncActuallyRuns()
|
||||
{
|
||||
// The sync RPC is a bidirectional stream. Gating only the unary path would leave
|
||||
// the real endpoint wide open while every unary test still passed.
|
||||
var interceptor = CreateInterceptor("the-shared-key");
|
||||
var context = CreateContext(SyncMethod, "Bearer the-wrong-key");
|
||||
|
||||
var ex = await Assert.ThrowsAsync<RpcException>(() =>
|
||||
interceptor.DuplexStreamingServerHandler<string, string>(
|
||||
requestStream: null!,
|
||||
responseStream: null!,
|
||||
context,
|
||||
(_, _, _) => Task.CompletedTask));
|
||||
|
||||
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
|
||||
}
|
||||
}
|
||||
@@ -1,6 +1,9 @@
|
||||
using Microsoft.AspNetCore.Builder;
|
||||
using Microsoft.Extensions.Configuration;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Hosting;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using ZB.MOM.WW.ScadaBridge.HealthMonitoring;
|
||||
using ZB.MOM.WW.LocalDb;
|
||||
using ZB.MOM.WW.LocalDb.Replication;
|
||||
using ZB.MOM.WW.ScadaBridge.Host;
|
||||
@@ -150,6 +153,49 @@ public class SiteLocalDbWiringTests : IDisposable
|
||||
Assert.Equal(0L, status.OplogBacklog);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Site_Replication_HealthBridge_IsRegisteredAsAHostedService()
|
||||
{
|
||||
// Task 9. Registered via a factory lambda, so ImplementationType is null and the
|
||||
// only honest assertion is on the resolved instance.
|
||||
var hosted = _host.Services.GetServices<IHostedService>();
|
||||
|
||||
Assert.Contains(hosted, h => h is LocalDbReplicationStatusReporter);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Site_Replication_HealthBridge_PublishesStatusOntoTheHealthReport()
|
||||
{
|
||||
// End-to-end through the REAL collector: probe once, then collect. This is what
|
||||
// catches a bridge that is registered but wired to the wrong provider — the
|
||||
// registration test above would pass either way.
|
||||
var reporter = _host.Services.GetServices<IHostedService>()
|
||||
.OfType<LocalDbReplicationStatusReporter>()
|
||||
.Single();
|
||||
var collector = _host.Services.GetRequiredService<ISiteHealthCollector>();
|
||||
|
||||
reporter.Probe();
|
||||
var report = collector.CollectReport("TestSite");
|
||||
|
||||
// No peer configured: not connected, and a REAL 0 backlog rather than null.
|
||||
Assert.False(report.LocalDbReplicationConnected);
|
||||
Assert.Equal(0L, report.LocalDbOplogBacklog);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Site_HealthReport_LocalDbFields_AreNull_BeforeTheReporterHasRun()
|
||||
{
|
||||
// Null means "no data yet", and must be distinguishable from a real
|
||||
// "disconnected, zero backlog". A collector that defaulted these to false/0 would
|
||||
// report an unwired node as a healthy connected one.
|
||||
var collector = new SiteHealthCollector();
|
||||
|
||||
var report = collector.CollectReport("TestSite");
|
||||
|
||||
Assert.Null(report.LocalDbReplicationConnected);
|
||||
Assert.Null(report.LocalDbOplogBacklog);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Site_LocalDb_CreatesTheConfiguredFile()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user