diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/DependencyInjection/SecretsGrpcHubServerExtensions.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/DependencyInjection/SecretsGrpcHubServerExtensions.cs index f7adba7..8026aad 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/DependencyInjection/SecretsGrpcHubServerExtensions.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/DependencyInjection/SecretsGrpcHubServerExtensions.cs @@ -37,7 +37,9 @@ public static class SecretsGrpcHubServerExtensions /// The application configuration. /// Configuration section holding the hub options. /// The same instance, for chaining. - /// No bearer token is configured. + /// + /// No bearer token is configured, or the hub has already been registered. + /// public static IServiceCollection AddZbSecretsGrpcHub( this IServiceCollection services, IConfiguration configuration, @@ -47,6 +49,24 @@ public static class SecretsGrpcHubServerExtensions ArgumentNullException.ThrowIfNull(configuration); ArgumentException.ThrowIfNullOrWhiteSpace(sectionPath); + // Registering twice is refused rather than tolerated, because the two calls do NOT compose: + // options binding MERGES (a key absent from the second section leaves the first section's + // value standing) while the SectionPath stamp below is last-one-wins. A hub registered from + // two sections could therefore serve a token bound from one of them while every failure + // message names the other — a misdirection that outlives the mistake that caused it. + if (services.Any(descriptor => descriptor.ServiceType == typeof(HubRegistrationMarker))) + { + throw new InvalidOperationException( + "The gRPC secrets hub may only be registered once; AddZbSecretsGrpcHub was called " + + "more than once. Remove the duplicate call — a second registration would merge its " + + "configuration into the first and leave the reported section path pointing at " + + "whichever call ran last."); + } + + // Registered as an instance so nothing ever has to activate it: the marker exists to be + // looked for, not resolved. + services.AddSingleton(new HubRegistrationMarker()); + IConfigurationSection section = configuration.GetSection(sectionPath); services.Configure(section); @@ -84,4 +104,12 @@ public static class SecretsGrpcHubServerExtensions endpoints.MapGrpcService(); return endpoints; } + + /// + /// Presence of this in the container means the hub is already registered. It carries no state + /// and is never resolved — a dedicated type is used rather than probing for one of the real + /// registrations because every one of those is added with TryAdd-style semantics that a + /// host may legitimately have satisfied for its own reasons. + /// + private sealed class HubRegistrationMarker; } diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/Protos/secrets_hub.proto b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/Protos/secrets_hub.proto index 6354c0c..4c9e1c8 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/Protos/secrets_hub.proto +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/Protos/secrets_hub.proto @@ -13,11 +13,13 @@ option csharp_namespace = "ZB.MOM.WW.Secrets.Replicator.Grpc.V1"; // The central-hosted secrets hub, as seen by a site node. // // PULL-ONLY BY CONSTRUCTION. This service mirrors exactly the read half of -// ISecretReplicationHub — GetManifestAsync and GetManyAsync. Its third method, -// ApplyReplicatedAsync, is deliberately NOT mirrored: in this topology every update -// originates at central, and a site node is a strict follower. The narrow contract IS the -// enforcement — with no write RPC on the wire, a mis-wired or compromised site node has no -// way to push a row into the hub, regardless of what its authorization happens to allow. +// ISecretReplicationHub, method for method: GetManifestAsync -> GetManifest, and +// GetManyAsync -> GetSecrets (the RPC is named for what it returns rather than for the +// .NET method it fronts). Its third method, ApplyReplicatedAsync, is deliberately NOT +// mirrored: in this topology every update originates at central, and a site node is a +// strict follower. The narrow contract IS the enforcement — with no write RPC on the wire, +// a mis-wired or compromised site node has no way to push a row into the hub, regardless of +// what its authorization happens to allow. // Adding a third RPC to this service would silently retire that guarantee, so do not. // // Everything crossing this boundary is ciphertext or metadata about ciphertext. The @@ -33,6 +35,9 @@ service SecretsHub { // Fetches the named encrypted rows in bulk. Names absent from the hub are omitted from // the response rather than reported as errors — the caller's manifest may be a moment // stale, and a row deleted between the two calls is not a fault. + // + // The server caps how many names one call may carry and answers RESOURCE_EXHAUSTED above + // it, so a caller must chunk large sweeps rather than send one request per manifest. rpc GetSecrets(GetSecretsRequest) returns (GetSecretsResponse); } diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/SecretsGrpcHubServerOptions.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/SecretsGrpcHubServerOptions.cs index 6a7f50c..209ea9f 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/SecretsGrpcHubServerOptions.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/SecretsGrpcHubServerOptions.cs @@ -22,12 +22,30 @@ public sealed class SecretsGrpcHubServerOptions /// The configuration section the hub is bound from unless a host overrides it. internal const string DefaultSectionPath = "Secrets:GrpcHub"; + /// + /// The default ceiling on names in one GetSecrets call. Set comfortably above the + /// follower's own fetch chunk so a compliant sweep never approaches it. + /// + internal const int DefaultMaxNamesPerRequest = 1000; + /// /// The shared bearer token a follower must present. Required — an unset token is not "open /// access", it is a hub that refuses every call (see ). /// public string BearerToken { get; set; } = string.Empty; + /// + /// The most names a single GetSecrets call may carry. A larger request is refused with + /// ResourceExhausted rather than served. + /// + /// + /// The well-behaved follower already chunks its fetches well below this, so the cap is not a + /// throughput knob — it is the server declining to let one authenticated caller turn a single + /// request into an unbounded sequential store sweep. The cap sits behind authentication, so it + /// is defence in depth against a misbehaving token holder, not an anonymous-DoS control. + /// + public int MaxNamesPerRequest { get; set; } = DefaultMaxNamesPerRequest; + /// /// The configuration section this instance was bound from, used so failure messages name the /// key an operator actually has to set. @@ -43,7 +61,9 @@ public sealed class SecretsGrpcHubServerOptions /// Throws if the options are unusable. Called by the registration extension so a hub with no /// token fails at startup, rather than starting and rejecting every follower at runtime. /// - /// The bearer token is missing or blank. + /// + /// The bearer token is missing or blank, or the request cap is not positive. + /// public void Validate() { if (string.IsNullOrWhiteSpace(BearerToken)) @@ -54,5 +74,14 @@ public sealed class SecretsGrpcHubServerOptions $"The gRPC secrets hub requires a shared bearer token ({SectionPath}:BearerToken). " + "Until it is set the hub refuses every call."); } + + if (MaxNamesPerRequest <= 0) + { + // A zero or negative cap would refuse every GetSecrets call, which reads at runtime like + // a broken follower rather than a typo in configuration — so it is refused at startup. + throw new InvalidOperationException( + $"The gRPC secrets hub request cap ({SectionPath}:MaxNamesPerRequest) must be " + + $"greater than zero; it is {MaxNamesPerRequest}."); + } } } diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/SecretsHubService.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/SecretsHubService.cs index 4eb28b4..986ecc7 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/SecretsHubService.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/SecretsHubService.cs @@ -1,5 +1,6 @@ using Grpc.Core; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using ZB.MOM.WW.Secrets.Abstractions; using ZB.MOM.WW.Secrets.Replicator.Grpc.V1; using ZB.MOM.WW.Secrets.Replicator.Grpc.Wire; @@ -27,14 +28,30 @@ public sealed class SecretsHubService : SecretsHub.SecretsHubBase { private readonly ISecretStore _store; private readonly ILogger _logger; + private readonly int _maxNamesPerRequest; /// Creates the hub service over the node's secret store. /// The store to serve. Only its read members are used. + /// The hub options, read for the per-request name cap. /// Diagnostics sink. - public SecretsHubService(ISecretStore store, ILogger logger) + public SecretsHubService( + ISecretStore store, + IOptions options, + ILogger logger) { + ArgumentNullException.ThrowIfNull(options); + _store = store ?? throw new ArgumentNullException(nameof(store)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + + // The registration extension validates the cap, so a non-positive value here means the + // options were built some other way. Falling back to the default is the safe direction: the + // alternative reading of "0" is a hub with no cap at all, which is the thing the cap exists + // to prevent. + int configured = options.Value.MaxNamesPerRequest; + _maxNamesPerRequest = configured > 0 + ? configured + : SecretsGrpcHubServerOptions.DefaultMaxNamesPerRequest; } /// @@ -66,21 +83,47 @@ public sealed class SecretsHubService : SecretsHub.SecretsHubBase /// Fetches the named encrypted rows in bulk. /// /// + /// /// A requested name the hub does not hold is omitted from the response, and an unparseable /// name is skipped — neither is an error. Both describe the same ordinary situation: the /// follower is acting on a manifest that was true a moment ago. Failing the whole call would turn /// a routine race into a stalled sweep, and would let one bad name deny the follower every other /// row in the batch. + /// + /// + /// A request carrying more than + /// names is refused, before any read runs. That is not the stale-manifest case: it is a + /// caller asking the hub to walk its store an unbounded number of times inside one call, and the + /// server declines independently of whatever chunking the client believes it is doing. + /// /// /// The names to fetch. /// The call context. /// One row per distinct name the hub holds. + /// The request carries more names than the configured cap. public override async Task GetSecrets( GetSecretsRequest request, ServerCallContext context) { ArgumentNullException.ThrowIfNull(request); ArgumentNullException.ThrowIfNull(context); + if (request.Names.Count > _maxNamesPerRequest) + { + // The cap and the count are both safe to state — neither is secret, and an operator + // cannot fix a batch that is too large without knowing by how much. The names are NOT + // stated: they are the one part of this request that identifies secrets. + _logger.LogWarning( + "Secrets hub refused a GetSecrets request of {RequestedCount} name(s); the cap is " + + "{MaxNamesPerRequest}.", + request.Names.Count, + _maxNamesPerRequest); + + throw new RpcException(new Status( + StatusCode.ResourceExhausted, + $"GetSecrets accepts at most {_maxNamesPerRequest} names per request; this one " + + $"carried {request.Names.Count}. Split the fetch into smaller batches.")); + } + var response = new GetSecretsResponse(); var seen = new HashSet(StringComparer.Ordinal); int skipped = 0; diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsGrpcHubServerOptionsTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsGrpcHubServerOptionsTests.cs index 7eb69c2..3ce01b7 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsGrpcHubServerOptionsTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsGrpcHubServerOptionsTests.cs @@ -1,3 +1,8 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using ZB.MOM.WW.Secrets.Replicator.Grpc.DependencyInjection; + namespace ZB.MOM.WW.Secrets.Replicator.Grpc.Tests; /// @@ -71,4 +76,194 @@ public sealed class SecretsGrpcHubServerOptionsTests Assert.Equal("Secrets:GrpcHub", options.SectionPath); } + + [Fact] + public void Max_names_per_request_defaults_above_the_followers_own_fetch_chunk() + { + // The follower chunks at 500; the cap sits above that so a compliant sweep never meets it, + // and meeting it therefore means something. + var options = new SecretsGrpcHubServerOptions(); + + Assert.Equal(1000, options.MaxNamesPerRequest); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + [InlineData(int.MinValue)] + public void Validate_rejects_a_nonpositive_request_cap(int cap) + { + var options = new SecretsGrpcHubServerOptions + { + BearerToken = "a-configured-hub-token", + MaxNamesPerRequest = cap, + }; + + InvalidOperationException error = + Assert.Throws(() => options.Validate()); + + Assert.Contains("Secrets:GrpcHub:MaxNamesPerRequest", error.Message, StringComparison.Ordinal); + } + + [Fact] + public void Validate_accepts_a_cap_of_one() + { + var options = new SecretsGrpcHubServerOptions + { + BearerToken = "a-configured-hub-token", + MaxNamesPerRequest = 1, + }; + + options.Validate(); + } + + [Fact] + public void Validate_names_the_section_path_on_the_request_cap_too() + { + var options = new SecretsGrpcHubServerOptions + { + BearerToken = "a-configured-hub-token", + MaxNamesPerRequest = 0, + }; + options.SectionPath = "Central:Secrets:Hub"; + + InvalidOperationException error = + Assert.Throws(() => options.Validate()); + + Assert.Contains("Central:Secrets:Hub:MaxNamesPerRequest", error.Message, StringComparison.Ordinal); + } +} + +/// +/// The registration extension itself, driven through a real and a +/// real . What is pinned here is the coherence of what a host ends up +/// holding: the token it configured, bound from the section it named, reported under that same +/// section path — and the refusal of a second registration, which cannot deliver that coherence. +/// +public sealed class SecretsGrpcHubServerExtensionsTests +{ + private const string Token = "a-configured-hub-token"; + + [Fact] + public void Registration_binds_the_token_and_stamps_the_default_section_path() + { + var services = new ServiceCollection(); + + services.AddZbSecretsGrpcHub(Configuration(("Secrets:GrpcHub:BearerToken", Token))); + + SecretsGrpcHubServerOptions options = Resolve(services); + Assert.Equal(Token, options.BearerToken); + Assert.Equal("Secrets:GrpcHub", options.SectionPath); + Assert.Equal(1000, options.MaxNamesPerRequest); + } + + [Fact] + public void Registration_binds_the_request_cap_when_configured() + { + var services = new ServiceCollection(); + + services.AddZbSecretsGrpcHub(Configuration( + ("Secrets:GrpcHub:BearerToken", Token), + ("Secrets:GrpcHub:MaxNamesPerRequest", "250"))); + + Assert.Equal(250, Resolve(services).MaxNamesPerRequest); + } + + [Fact] + public void Registration_under_a_custom_section_path_binds_and_stamps_that_path() + { + var services = new ServiceCollection(); + + services.AddZbSecretsGrpcHub( + Configuration(("Central:Secrets:Hub:BearerToken", Token)), + "Central:Secrets:Hub"); + + SecretsGrpcHubServerOptions options = Resolve(services); + Assert.Equal(Token, options.BearerToken); + Assert.Equal("Central:Secrets:Hub", options.SectionPath); + } + + [Fact] + public void Registration_under_a_custom_section_path_names_that_path_when_the_token_is_missing() + { + // End to end: the operator is told the key THIS host reads, not the library's default. + var services = new ServiceCollection(); + + InvalidOperationException error = Assert.Throws( + () => services.AddZbSecretsGrpcHub( + Configuration(("Secrets:GrpcHub:BearerToken", Token)), "Central:Secrets:Hub")); + + Assert.Contains("Central:Secrets:Hub:BearerToken", error.Message, StringComparison.Ordinal); + } + + [Fact] + public void Registration_without_a_token_throws_naming_the_exact_key() + { + var services = new ServiceCollection(); + + InvalidOperationException error = Assert.Throws( + () => services.AddZbSecretsGrpcHub(Configuration())); + + Assert.Contains("Secrets:GrpcHub:BearerToken", error.Message, StringComparison.Ordinal); + } + + [Fact] + public void Registration_with_a_nonpositive_request_cap_throws() + { + var services = new ServiceCollection(); + + InvalidOperationException error = Assert.Throws( + () => services.AddZbSecretsGrpcHub(Configuration( + ("Secrets:GrpcHub:BearerToken", Token), + ("Secrets:GrpcHub:MaxNamesPerRequest", "0")))); + + Assert.Contains("Secrets:GrpcHub:MaxNamesPerRequest", error.Message, StringComparison.Ordinal); + } + + [Fact] + public void Registering_the_hub_twice_throws() + { + var services = new ServiceCollection(); + IConfiguration configuration = Configuration(("Secrets:GrpcHub:BearerToken", Token)); + + services.AddZbSecretsGrpcHub(configuration); + + InvalidOperationException error = Assert.Throws( + () => services.AddZbSecretsGrpcHub(configuration)); + + Assert.Contains("once", error.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void Registering_the_hub_twice_under_different_sections_throws_rather_than_merging() + { + // This is the case the guard exists for: binding MERGES (the second section's absent + // BearerToken leaves the first one's standing) while the SectionPath stamp is last-one-wins, + // so the surviving options would report a section that did not supply the token they hold. + var services = new ServiceCollection(); + IConfiguration configuration = Configuration( + ("Secrets:GrpcHub:BearerToken", Token), + ("Central:Secrets:Hub:MaxNamesPerRequest", "7")); + + services.AddZbSecretsGrpcHub(configuration); + + Assert.Throws( + () => services.AddZbSecretsGrpcHub(configuration, "Central:Secrets:Hub")); + + SecretsGrpcHubServerOptions options = Resolve(services); + Assert.Equal("Secrets:GrpcHub", options.SectionPath); + Assert.Equal(Token, options.BearerToken); + Assert.Equal(1000, options.MaxNamesPerRequest); + } + + private static IConfiguration Configuration(params (string Key, string Value)[] entries) => + new ConfigurationBuilder() + .AddInMemoryCollection(entries.Select(e => new KeyValuePair(e.Key, e.Value))) + .Build(); + + private static SecretsGrpcHubServerOptions Resolve(IServiceCollection services) + { + using ServiceProvider provider = services.BuildServiceProvider(); + return provider.GetRequiredService>().Value; + } } diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsHubAuthInterceptorTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsHubAuthInterceptorTests.cs index a3c1359..aea1cd1 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsHubAuthInterceptorTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsHubAuthInterceptorTests.cs @@ -155,7 +155,7 @@ public sealed class SecretsHubAuthInterceptorTests } [Fact] - public async Task Non_unary_calls_are_gated_too() + public async Task Server_streaming_calls_are_gated_too() { SecretsHubAuthInterceptor interceptor = Build(ConfiguredToken, out _); bool invoked = false; @@ -175,6 +175,88 @@ public sealed class SecretsHubAuthInterceptorTests Assert.False(invoked); } + [Fact] + public async Task Client_streaming_calls_are_gated_too() + { + // The contract is unary-only today, so all four handler shapes are covered deliberately: + // an overload left ungated would be discovered by the first RPC that used it, in production. + SecretsHubAuthInterceptor interceptor = Build(ConfiguredToken, out _); + bool invoked = false; + + RpcException error = await Assert.ThrowsAsync( + () => interceptor.ClientStreamingServerHandler( + new HubAsyncStreamReaderStub(), + HubServerCallContextStub.WithAuthorization(header: null), + (_, _) => + { + invoked = true; + return Task.FromResult(new GetSecretsResponse()); + })); + + Assert.Equal(StatusCode.Unauthenticated, error.StatusCode); + Assert.False(invoked); + } + + [Fact] + public async Task Duplex_streaming_calls_are_gated_too() + { + SecretsHubAuthInterceptor interceptor = Build(ConfiguredToken, out _); + bool invoked = false; + + RpcException error = await Assert.ThrowsAsync( + () => interceptor.DuplexStreamingServerHandler( + new HubAsyncStreamReaderStub(), + new HubServerStreamWriterStub(), + HubServerCallContextStub.WithAuthorization(header: null), + (_, _, _) => + { + invoked = true; + return Task.CompletedTask; + })); + + Assert.Equal(StatusCode.Unauthenticated, error.StatusCode); + Assert.False(invoked); + } + + [Fact] + public async Task A_valid_token_reaches_every_streaming_continuation() + { + SecretsHubAuthInterceptor interceptor = Build(ConfiguredToken, out _); + string header = $"Bearer {ConfiguredToken}"; + var reached = new List(); + + await interceptor.ServerStreamingServerHandler( + new GetSecretsRequest(), + new HubServerStreamWriterStub(), + HubServerCallContextStub.WithAuthorization(header), + (_, _, _) => + { + reached.Add("server"); + return Task.CompletedTask; + }); + + await interceptor.ClientStreamingServerHandler( + new HubAsyncStreamReaderStub(), + HubServerCallContextStub.WithAuthorization(header), + (_, _) => + { + reached.Add("client"); + return Task.FromResult(new GetSecretsResponse()); + }); + + await interceptor.DuplexStreamingServerHandler( + new HubAsyncStreamReaderStub(), + new HubServerStreamWriterStub(), + HubServerCallContextStub.WithAuthorization(header), + (_, _, _) => + { + reached.Add("duplex"); + return Task.CompletedTask; + }); + + Assert.Equal(["server", "client", "duplex"], reached); + } + [Fact] public async Task No_log_line_ever_contains_token_material() { @@ -238,11 +320,16 @@ public sealed class SecretsHubAuthInterceptorTests internal sealed class HubServerCallContextStub : ServerCallContext { private readonly Metadata _requestHeaders; + private readonly CancellationToken _cancellationToken; private readonly Metadata _responseTrailers = new(); private readonly AuthContext _authContext = new(null, new Dictionary>()); private readonly Dictionary _userState = new(); - private HubServerCallContextStub(Metadata requestHeaders) => _requestHeaders = requestHeaders; + private HubServerCallContextStub(Metadata requestHeaders, CancellationToken cancellationToken) + { + _requestHeaders = requestHeaders; + _cancellationToken = cancellationToken; + } /// Builds a context carrying the given Authorization header, or none when null. public static HubServerCallContextStub WithAuthorization(string? header) @@ -253,18 +340,22 @@ internal sealed class HubServerCallContextStub : ServerCallContext headers.Add("Authorization", header); } - return new HubServerCallContextStub(headers); + return new HubServerCallContextStub(headers, CancellationToken.None); } /// Builds a context with no metadata at all, for service-level tests. - public static HubServerCallContextStub Empty() => new(new Metadata()); + public static HubServerCallContextStub Empty() => new(new Metadata(), CancellationToken.None); + + /// Builds a context whose call token is the given one, for propagation tests. + public static HubServerCallContextStub WithCancellation(CancellationToken cancellationToken) => + new(new Metadata(), cancellationToken); protected override string MethodCore => "/zb.mom.ww.secrets.hub.v1.SecretsHub/Test"; protected override string HostCore => "localhost"; protected override string PeerCore => "ipv4:127.0.0.1:0"; protected override DateTime DeadlineCore => DateTime.UtcNow.AddSeconds(30); protected override Metadata RequestHeadersCore => _requestHeaders; - protected override CancellationToken CancellationTokenCore => CancellationToken.None; + protected override CancellationToken CancellationTokenCore => _cancellationToken; protected override Metadata ResponseTrailersCore => _responseTrailers; protected override Status StatusCore { get; set; } protected override WriteOptions? WriteOptionsCore { get; set; } @@ -294,6 +385,17 @@ internal sealed class HubServerStreamWriterStub : IServerStreamWriter } } +/// An empty request stream, enough to drive the two client-reading interceptor paths. +internal sealed class HubAsyncStreamReaderStub : IAsyncStreamReader + where T : class +{ + /// + public T Current => throw new InvalidOperationException("The stub stream carries no messages."); + + /// + public Task MoveNext(CancellationToken cancellationToken) => Task.FromResult(false); +} + /// Captures formatted log messages so tests can assert on what was — and was not — logged. internal sealed class HubCapturingLogger : ILogger { diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsHubServiceTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsHubServiceTests.cs index 786349a..790f4d4 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsHubServiceTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/SecretsHubServiceTests.cs @@ -1,6 +1,8 @@ using System.Globalization; using Google.Protobuf; +using Grpc.Core; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using ZB.MOM.WW.Secrets.Abstractions; using ZB.MOM.WW.Secrets.Replicator.Grpc.V1; using ZB.MOM.WW.Secrets.Replicator.Grpc.Wire; @@ -142,6 +144,106 @@ public sealed class SecretsHubServiceTests Assert.Equal(0, store.GetCallCount); } + [Fact] + public async Task GetSecrets_serves_a_request_exactly_at_the_cap() + { + var store = new HubFakeSecretStore(); + store.Seed(Row("n1")); + store.Seed(Row("n2")); + store.Seed(Row("n3")); + + var request = new GetSecretsRequest { Names = { "n1", "n2", "n3" } }; + + GetSecretsResponse response = await Service(store, maxNamesPerRequest: 3) + .GetSecrets(request, HubServerCallContextStub.Empty()); + + Assert.Equal(3, response.Secrets.Count); + } + + [Fact] + public async Task GetSecrets_refuses_a_request_over_the_cap() + { + var store = new HubFakeSecretStore(); + store.Seed(Row("n1")); + + var request = new GetSecretsRequest { Names = { "n1", "n2", "n3", "n4" } }; + + RpcException error = await Assert.ThrowsAsync( + () => Service(store, maxNamesPerRequest: 3).GetSecrets(request, HubServerCallContextStub.Empty())); + + Assert.Equal(StatusCode.ResourceExhausted, error.StatusCode); + } + + [Fact] + public async Task GetSecrets_over_the_cap_reads_nothing_at_all() + { + // The refusal has to land BEFORE the sequential store loop — a cap that stops the response + // after doing the reads would defend nothing. + var store = new HubFakeSecretStore(); + store.Seed(Row("n1")); + + var request = new GetSecretsRequest { Names = { "n1", "n1", "n1" } }; + + await Assert.ThrowsAsync( + () => Service(store, maxNamesPerRequest: 2).GetSecrets(request, HubServerCallContextStub.Empty())); + + Assert.Equal(0, store.GetCallCount); + } + + [Fact] + public async Task GetSecrets_cap_refusal_states_the_cap_and_the_count_but_never_a_name() + { + var store = new HubFakeSecretStore(); + var request = new GetSecretsRequest { Names = { "db/central", "db/site-a", "db/site-b" } }; + + RpcException error = await Assert.ThrowsAsync( + () => Service(store, maxNamesPerRequest: 2).GetSecrets(request, HubServerCallContextStub.Empty())); + + Assert.Contains("2", error.Status.Detail, StringComparison.Ordinal); + Assert.Contains("3", error.Status.Detail, StringComparison.Ordinal); + foreach (string name in request.Names) + { + Assert.DoesNotContain(name, error.Status.Detail, StringComparison.OrdinalIgnoreCase); + } + } + + [Fact] + public async Task GetSecrets_falls_back_to_the_default_cap_when_options_carry_a_nonpositive_one() + { + // Registration validates the cap, so this only happens when the options were built some + // other way — and the safe reading of "0" is the default, never "no cap". + var store = new HubFakeSecretStore(); + var request = new GetSecretsRequest(); + for (int i = 0; i <= SecretsGrpcHubServerOptions.DefaultMaxNamesPerRequest; i++) + { + request.Names.Add($"n{i}"); + } + + RpcException error = await Assert.ThrowsAsync( + () => Service(store, maxNamesPerRequest: 0).GetSecrets(request, HubServerCallContextStub.Empty())); + + Assert.Equal(StatusCode.ResourceExhausted, error.StatusCode); + } + + [Fact] + public async Task GetSecrets_hands_the_call_cancellation_token_to_the_store() + { + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + + var store = new HubFakeSecretStore(); + store.Seed(Row("present")); + + await Service(store).GetSecrets( + new GetSecretsRequest { Names = { "present" } }, + HubServerCallContextStub.WithCancellation(cts.Token)); + + // The fake does not honour the token, so what is pinned here is the propagation itself: a + // real store sees the caller's cancellation, and an abandoned call stops costing reads. + Assert.True(store.LastCancellationToken.IsCancellationRequested); + Assert.Equal(cts.Token, store.LastCancellationToken); + } + [Fact] public void DomainToWire_maps_a_fully_populated_row_field_by_field() { @@ -231,8 +333,16 @@ public sealed class SecretsHubServiceTests Assert.Equal(expected, wire.ContentType); } - private static SecretsHubService Service(ISecretStore store) => - new(store, NullLogger.Instance); + private static SecretsHubService Service(ISecretStore store, int? maxNamesPerRequest = null) + { + var options = new SecretsGrpcHubServerOptions { BearerToken = "irrelevant-to-the-service" }; + if (maxNamesPerRequest is int max) + { + options.MaxNamesPerRequest = max; + } + + return new SecretsHubService(store, Options.Create(options), NullLogger.Instance); + } private static StoredSecret Row(string name, bool isDeleted = false) => new() { @@ -266,6 +376,9 @@ internal sealed class HubFakeSecretStore : ISecretStore /// How many single-row reads the service issued. public int GetCallCount { get; private set; } + /// The cancellation token the service passed on the most recent read. + public CancellationToken LastCancellationToken { get; private set; } + /// Adds a row, and the manifest entry derived from it. public void Seed(StoredSecret row) => _rows[row.Name.Value] = row; @@ -276,6 +389,7 @@ internal sealed class HubFakeSecretStore : ISecretStore public Task GetAsync(SecretName name, CancellationToken ct) { GetCallCount++; + LastCancellationToken = ct; return Task.FromResult(_rows.GetValueOrDefault(name.Value)); }