3a31dd2de7
SitePskProvider cached each site's gRPC preshared key forever; only site removal
invalidated it, so rotating SB-GRPC-PSK-{site} broke the central<->site link on
both central nodes until they restarted. The cached key is now dropped when:
- the secret is written through /admin/secrets: central replaces the secrets
library's ISecretCacheInvalidator with SitePskRotationInvalidator, which also
publishes SitePskInvalidated on a new DistributedPubSub topic
(site-psk-invalidation) so the peer central node drops its copy and its
resolver cache entry too (SitePskInvalidationSubscriber);
- a site rejects a central->site call with PermissionDenied/Unauthenticated:
WithSiteCredentials wraps every per-site channel's handler in
PskRejectionRefreshHandler, so the next call re-reads the key (covers
out-of-process writes such as the secret CLI);
- a site->central call presents a token that does not match the cached key:
CentralControlAuthInterceptor invalidates and re-reads once before refusing;
- ScadaBridge:Communication:SitePsks reloads.
Invalidation is case-insensitive because SecretName lowercases names. Rotation
runbook (topology-guide), Component-Communication/Host and the #85 cache audit
(D2) updated.
Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
265 lines
12 KiB
C#
265 lines
12 KiB
C#
using Microsoft.Extensions.Logging.Abstractions;
|
|
using Microsoft.Extensions.Options;
|
|
using NSubstitute;
|
|
using ZB.MOM.WW.ScadaBridge.Communication;
|
|
using ZB.MOM.WW.Secrets.Abstractions;
|
|
|
|
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
|
|
|
/// <summary>
|
|
/// Central's per-site gRPC preshared key resolution (ClusterClient→gRPC migration, T0.3).
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// The property that matters most here is the negative one: a site whose key cannot be found
|
|
/// must produce a throw, never an unauthenticated channel. Everything else — the two sources,
|
|
/// the caching, the invalidation — exists to make that behaviour usable in practice.
|
|
/// </remarks>
|
|
public class SitePskProviderTests
|
|
{
|
|
private static SitePskProvider Create(
|
|
ISecretResolver resolver, CommunicationOptions? options = null)
|
|
=> new(
|
|
resolver,
|
|
new StaticOptionsMonitor(options ?? new CommunicationOptions()),
|
|
NullLogger<SitePskProvider>.Instance);
|
|
|
|
private sealed class StaticOptionsMonitor(CommunicationOptions value)
|
|
: IOptionsMonitor<CommunicationOptions>
|
|
{
|
|
/// <summary>Gets the fixed options instance.</summary>
|
|
public CommunicationOptions CurrentValue => value;
|
|
/// <summary>Returns the fixed options instance regardless of name.</summary>
|
|
/// <param name="name">The named-options key, ignored.</param>
|
|
/// <returns>The fixed options instance.</returns>
|
|
public CommunicationOptions Get(string? name) => value;
|
|
/// <summary>Does nothing; the fake never raises change notifications.</summary>
|
|
/// <param name="listener">The change listener, ignored.</param>
|
|
/// <returns>Always null.</returns>
|
|
public IDisposable? OnChange(Action<CommunicationOptions, string?> listener) => null;
|
|
}
|
|
|
|
/// <summary>Verifies that the key is resolved from the secret store under the site-qualified name.</summary>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
[Fact]
|
|
public async Task ResolvesFromTheSecretStore_UnderTheSiteQualifiedName()
|
|
{
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(new SecretName("SB-GRPC-PSK-site-a"), Arg.Any<CancellationToken>())
|
|
.Returns("key-for-a");
|
|
|
|
var psk = await Create(resolver).GetAsync("site-a", CancellationToken.None);
|
|
|
|
Assert.Equal("key-for-a", psk);
|
|
}
|
|
|
|
/// <summary>Verifies that a configured key map takes precedence so a host without a master key can still dial.</summary>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
[Fact]
|
|
public async Task ConfiguredMapWins_SoAHostWithNoMasterKeyCanStillDial()
|
|
{
|
|
// The development rig runs with no secrets master key at all — every credential
|
|
// arrives as an environment override. Without this source, the rig could not use the
|
|
// gated control plane and the fail-closed design would be untestable there.
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
|
.Returns("from-the-store");
|
|
|
|
var options = new CommunicationOptions();
|
|
options.SitePsks["site-a"] = "from-config";
|
|
|
|
var psk = await Create(resolver, options).GetAsync("site-a", CancellationToken.None);
|
|
|
|
Assert.Equal("from-config", psk);
|
|
await resolver.DidNotReceive().GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>());
|
|
}
|
|
|
|
/// <summary>Verifies that the store is still consulted for a site missing from the configured map.</summary>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
[Fact]
|
|
public async Task StoreIsStillConsulted_ForASiteMissingFromTheMap()
|
|
{
|
|
// Sites are added at runtime from the Central UI, so the map can never be complete.
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(new SecretName("SB-GRPC-PSK-site-b"), Arg.Any<CancellationToken>())
|
|
.Returns("key-for-b");
|
|
|
|
var options = new CommunicationOptions();
|
|
options.SitePsks["site-a"] = "from-config";
|
|
|
|
var psk = await Create(resolver, options).GetAsync("site-b", CancellationToken.None);
|
|
|
|
Assert.Equal("key-for-b", psk);
|
|
}
|
|
|
|
/// <summary>Verifies that a missing secret throws and never yields an empty key.</summary>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
[Fact]
|
|
public async Task MissingSecret_Throws_AndNeverYieldsAnEmptyKey()
|
|
{
|
|
// Fail-closed. The alternative — returning "" — would build a channel that presents
|
|
// "Bearer " and gets PermissionDenied anyway, but with a far less diagnosable error.
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
|
.Returns((string?)null);
|
|
|
|
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
|
|
async () => await Create(resolver).GetAsync("site-a", CancellationToken.None));
|
|
|
|
// The message must name both sources — either one fixes it.
|
|
Assert.Contains("SB-GRPC-PSK-site-a", ex.Message);
|
|
Assert.Contains("SitePsks", ex.Message);
|
|
}
|
|
|
|
/// <summary>Verifies that an empty secret is treated as missing.</summary>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
[Fact]
|
|
public async Task EmptySecret_IsTreatedAsMissing()
|
|
{
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>()).Returns("");
|
|
|
|
await Assert.ThrowsAsync<InvalidOperationException>(
|
|
async () => await Create(resolver).GetAsync("site-a", CancellationToken.None));
|
|
}
|
|
|
|
/// <summary>Verifies that a faulting store surfaces as a distinct store-unavailable error.</summary>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
[Fact]
|
|
public async Task AFaultingStore_SurfacesAsStoreUnavailable_NotAsMissingSecret()
|
|
{
|
|
// Review 2026-10-21 05/U2: a store fault must not read as "secret missing".
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
|
.Returns<string?>(_ => throw new InvalidOperationException("no master key configured"));
|
|
|
|
var ex = await Assert.ThrowsAsync<SecretStoreUnavailableException>(
|
|
async () => await Create(resolver).GetAsync("site-a", CancellationToken.None));
|
|
|
|
Assert.Contains("SB-GRPC-PSK-site-a", ex.Message);
|
|
Assert.Contains("store fault", ex.Message);
|
|
Assert.IsType<InvalidOperationException>(ex.InnerException);
|
|
}
|
|
|
|
/// <summary>Verifies that successful resolves are cached so not every call hits the store.</summary>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
[Fact]
|
|
public async Task SuccessfulResolvesAreCached_SoEveryCallDoesNotHitTheStore()
|
|
{
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>()).Returns("k");
|
|
var provider = Create(resolver);
|
|
|
|
await provider.GetAsync("site-a", CancellationToken.None);
|
|
await provider.GetAsync("site-a", CancellationToken.None);
|
|
await provider.GetAsync("site-a", CancellationToken.None);
|
|
|
|
await resolver.Received(1).GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>());
|
|
}
|
|
|
|
/// <summary>Verifies that failures are not cached so a newly seeded key is picked up without a restart.</summary>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
[Fact]
|
|
public async Task FailuresAreNotCached_SoASeededKeyIsPickedUpWithoutARestart()
|
|
{
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
|
.Returns((string?)null, "seeded-later");
|
|
var provider = Create(resolver);
|
|
|
|
await Assert.ThrowsAsync<InvalidOperationException>(
|
|
async () => await provider.GetAsync("site-a", CancellationToken.None));
|
|
|
|
Assert.Equal("seeded-later", await provider.GetAsync("site-a", CancellationToken.None));
|
|
}
|
|
|
|
/// <summary>Verifies that invalidating drops the cached key so a rotated key is read.</summary>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
[Fact]
|
|
public async Task Invalidate_DropsTheCachedKey_SoARotatedKeyIsRead()
|
|
{
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
|
.Returns("old", "rotated");
|
|
var provider = Create(resolver);
|
|
|
|
Assert.Equal("old", await provider.GetAsync("site-a", CancellationToken.None));
|
|
provider.Invalidate("site-a");
|
|
|
|
Assert.Equal("rotated", await provider.GetAsync("site-a", CancellationToken.None));
|
|
}
|
|
|
|
/// <summary>Verifies that keys are scoped per site so one site never presents another site's key.</summary>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
[Fact]
|
|
public async Task KeysAreScopedPerSite_SoOneSiteNeverPresentsAnothersKey()
|
|
{
|
|
// The whole reason the design rejected a single fleet-wide key: blast radius.
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(new SecretName("SB-GRPC-PSK-site-a"), Arg.Any<CancellationToken>())
|
|
.Returns("key-a");
|
|
resolver.GetAsync(new SecretName("SB-GRPC-PSK-site-b"), Arg.Any<CancellationToken>())
|
|
.Returns("key-b");
|
|
var provider = Create(resolver);
|
|
|
|
Assert.Equal("key-a", await provider.GetAsync("site-a", CancellationToken.None));
|
|
Assert.Equal("key-b", await provider.GetAsync("site-b", CancellationToken.None));
|
|
}
|
|
/// <summary>
|
|
/// Verifies that a reload of the configured <c>SitePsks</c> map drops cached keys, so a key
|
|
/// rotated in configuration is used without a restart (Gitea #96).
|
|
/// </summary>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
[Fact]
|
|
public async Task ReloadedSitePsks_DropsTheCache_SoAKeyRotatedInConfigIsUsed()
|
|
{
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
var options = new CommunicationOptions();
|
|
options.SitePsks["site-a"] = "old";
|
|
Action<CommunicationOptions, string?>? listener = null;
|
|
var monitor = Substitute.For<IOptionsMonitor<CommunicationOptions>>();
|
|
monitor.CurrentValue.Returns(_ => options);
|
|
monitor.OnChange(Arg.Do<Action<CommunicationOptions, string?>>(l => listener = l));
|
|
using var provider = new SitePskProvider(resolver, monitor, NullLogger<SitePskProvider>.Instance);
|
|
|
|
Assert.Equal("old", await provider.GetAsync("site-a", CancellationToken.None));
|
|
|
|
options = new CommunicationOptions();
|
|
options.SitePsks["site-a"] = "rotated";
|
|
Assert.NotNull(listener);
|
|
listener!(options, null);
|
|
|
|
Assert.Equal("rotated", await provider.GetAsync("site-a", CancellationToken.None));
|
|
}
|
|
|
|
/// <summary>Verifies the secret-name to site-id mapping the rotation hook relies on.</summary>
|
|
/// <param name="name">A secret name.</param>
|
|
/// <param name="expected">The expected site id, or null.</param>
|
|
[Theory]
|
|
[InlineData("SB-GRPC-PSK-site-a", "site-a")]
|
|
[InlineData("SB-GRPC-PSK-", null)]
|
|
[InlineData("OTHER-SECRET", null)]
|
|
[InlineData("sb-grpc-psk-site-a", "site-a")]
|
|
public void SiteIdFromSecretName_MapsOnlyPskNames(string name, string? expected)
|
|
{
|
|
Assert.Equal(expected, SitePskProvider.SiteIdFromSecretName(name));
|
|
}
|
|
/// <summary>
|
|
/// Verifies that invalidation ignores case: the rotation hook derives the site id from a
|
|
/// lowercased secret name, but the site may be cached under its mixed-case identifier.
|
|
/// </summary>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
[Fact]
|
|
public async Task Invalidate_IgnoresCase_BecauseSecretNamesAreLowercased()
|
|
{
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
|
.Returns("old", "rotated");
|
|
var provider = Create(resolver);
|
|
|
|
Assert.Equal("old", await provider.GetAsync("Site-A", CancellationToken.None));
|
|
provider.Invalidate("site-a");
|
|
|
|
Assert.Equal("rotated", await provider.GetAsync("Site-A", CancellationToken.None));
|
|
}
|
|
}
|