diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs index e22e4769..3b29c7fe 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs @@ -3,6 +3,7 @@ using Microsoft.Extensions.Logging; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Protocol; using ZB.MOM.WW.ScadaBridge.Commons.Serialization; using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.Secrets.Abstractions; namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Adapters; @@ -24,6 +25,7 @@ public class MxGatewayDataConnection : IDataConnection, IBrowsableDataConnection { private readonly IMxGatewayClientFactory _clientFactory; private readonly ILogger _logger; + private readonly ISecretResolver? _secretResolver; private IMxGatewayClient? _client; private ConnectionHealth _status = ConnectionHealth.Disconnected; private CancellationTokenSource? _eventLoopCts; @@ -51,10 +53,19 @@ public class MxGatewayDataConnection : IDataConnection, IBrowsableDataConnection /// Initializes a new instance of . /// Factory used to create gateway client instances. /// Logger instance. - public MxGatewayDataConnection(IMxGatewayClientFactory clientFactory, ILogger logger) + /// + /// Optional secret resolver used to resolve a secret:<name> ApiKey reference at + /// connect time. When null, a literal ApiKey still works, but a secret: reference + /// fails closed (a connection is never established with an unresolved key). + /// + public MxGatewayDataConnection( + IMxGatewayClientFactory clientFactory, + ILogger logger, + ISecretResolver? secretResolver = null) { _clientFactory = clientFactory; _logger = logger; + _secretResolver = secretResolver; } /// @@ -68,6 +79,25 @@ public class MxGatewayDataConnection : IDataConnection, IBrowsableDataConnection { var cfg = MxGatewayEndpointConfigSerializer.FromFlatDict(connectionDetails); + // Resolve a `secret:` ApiKey reference BEFORE any teardown/client creation + // so a fail-closed throw here leaves adapter state untouched (no half-open client, + // previous connection — if any — still intact for a later retry). A literal key + // (no `secret:` prefix) passes through unchanged for back-compat. + var apiKey = cfg.ApiKey; + if (!string.IsNullOrEmpty(apiKey) && + apiKey.StartsWith("secret:", StringComparison.OrdinalIgnoreCase)) + { + if (_secretResolver is null) + { + throw new InvalidOperationException( + "MxGateway ApiKey uses a 'secret:' reference but no ISecretResolver is available to resolve it."); + } + var secretName = apiKey["secret:".Length..]; + apiKey = await _secretResolver.GetAsync(new SecretName(secretName), cancellationToken) + ?? throw new InvalidOperationException( + $"MxGateway ApiKey secret '{secretName}' could not be resolved (not found or tombstoned)."); + } + // Reconnect reuses this adapter: cancel the previous event loop and dispose // the previous client so a flapping gateway cannot accumulate orphaned // streams/sockets, and the old loop's fault path cannot signal Disconnected @@ -93,7 +123,7 @@ public class MxGatewayDataConnection : IDataConnection, IBrowsableDataConnection await _client.ConnectAsync(new MxGatewayConnectionOptions( cfg.Endpoint, - cfg.ApiKey, + apiKey, string.IsNullOrWhiteSpace(cfg.ClientName) ? "scadabridge" : cfg.ClientName, cfg.WriteUserId, cfg.UseTls, diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/DataConnectionFactory.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/DataConnectionFactory.cs index 845326b7..81b1f53f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/DataConnectionFactory.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/DataConnectionFactory.cs @@ -2,6 +2,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Protocol; using ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Adapters; +using ZB.MOM.WW.Secrets.Abstractions; namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer; @@ -13,6 +14,7 @@ public class DataConnectionFactory : IDataConnectionFactory { private readonly Dictionary, IDataConnection>> _factories = new(StringComparer.OrdinalIgnoreCase); private readonly ILoggerFactory _loggerFactory; + private readonly ISecretResolver? _secretResolver; /// /// Initializes a new with default OPC UA global options. @@ -26,9 +28,19 @@ public class DataConnectionFactory : IDataConnectionFactory /// /// Logger factory used when creating protocol adapters. /// Global OPC UA options applied to all OPC UA connections created by this factory. - public DataConnectionFactory(ILoggerFactory loggerFactory, IOptions opcUaGlobalOptions) + /// + /// Optional secret resolver threaded into the MxGateway adapter so a per-endpoint + /// secret:<name> ApiKey reference is resolved at connect time. DI supplies it on + /// the Site container (where is registered); direct + /// construction without it keeps literal-key behaviour. + /// + public DataConnectionFactory( + ILoggerFactory loggerFactory, + IOptions opcUaGlobalOptions, + ISecretResolver? secretResolver = null) { _loggerFactory = loggerFactory; + _secretResolver = secretResolver; var globalOptions = opcUaGlobalOptions.Value; // Register built-in protocols. @@ -44,7 +56,8 @@ public class DataConnectionFactory : IDataConnectionFactory // flat details dict (see MxGatewayEndpointConfigSerializer). RegisterAdapter("MxGateway", details => new MxGatewayDataConnection( new RealMxGatewayClientFactory(_loggerFactory), - _loggerFactory.CreateLogger())); + _loggerFactory.CreateLogger(), + _secretResolver)); } /// diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.csproj b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.csproj index 775ace0d..76241313 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.csproj +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.csproj @@ -17,6 +17,7 @@ + diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionSecretsTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionSecretsTests.cs new file mode 100644 index 00000000..27cca9c3 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionSecretsTests.cs @@ -0,0 +1,140 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Protocol; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Adapters; +using ZB.MOM.WW.Secrets.Abstractions; + +namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests.Adapters; + +// G-3 / Task 9: the MxGateway per-endpoint ApiKey may hold a `secret:` reference +// resolved via ISecretResolver at connect time. Literal keys pass through unchanged +// (back-compat); an unresolvable / missing secret, or a secret ref with no resolver +// available, fails closed (no connection established with an empty/null key). +[Collection("DataConnectionManagerActor")] +public class MxGatewayDataConnectionSecretsTests +{ + private static Dictionary Details(string apiKey) => new() + { + ["Endpoint"] = "http://gw:5000", + ["ApiKey"] = apiKey, + ["ClientName"] = "client-a", + ["WriteUserId"] = "0", + ["ReadTimeoutMs"] = "2000", + }; + + private sealed class FakeSecretResolver : ISecretResolver + { + private readonly string? _value; + public FakeSecretResolver(string? value) => _value = value; + public List Requested { get; } = new(); + + public Task GetAsync(SecretName name, CancellationToken ct) + { + Requested.Add(name.Value); + return Task.FromResult(_value); + } + } + + [Fact] + public async Task ConnectAsync_resolves_secret_ref_apikey() + { + var fake = new FakeMxGatewayClient(); + var resolver = new FakeSecretResolver("resolved-plaintext-key"); + var adapter = new MxGatewayDataConnection(fake, NullLogger.Instance, resolver); + + await adapter.ConnectAsync(Details("secret:mxgateway/testconn/api-key")); + + Assert.Equal(ConnectionHealth.Connected, adapter.Status); + Assert.NotNull(fake.ConnectedWith); + Assert.Equal("resolved-plaintext-key", fake.ConnectedWith!.ApiKey); + Assert.Contains("mxgateway/testconn/api-key", resolver.Requested); + } + + [Fact] + public async Task ConnectAsync_passes_literal_apikey_unchanged_and_does_not_call_resolver() + { + var fake = new FakeMxGatewayClient(); + var resolver = new FakeSecretResolver("SHOULD-NOT-BE-USED"); + var adapter = new MxGatewayDataConnection(fake, NullLogger.Instance, resolver); + + await adapter.ConnectAsync(Details("literal-api-key")); + + Assert.Equal("literal-api-key", fake.ConnectedWith!.ApiKey); + Assert.Empty(resolver.Requested); + } + + [Fact] + public async Task ConnectAsync_fails_closed_when_secret_unresolvable() + { + var fake = new FakeMxGatewayClient(); + var resolver = new FakeSecretResolver(null); // secret not found / tombstoned + var adapter = new MxGatewayDataConnection(fake, NullLogger.Instance, resolver); + + await Assert.ThrowsAsync( + () => adapter.ConnectAsync(Details("secret:mxgateway/testconn/missing"))); + + // Fail-closed: the underlying client was never connected and status is not Connected. + Assert.Null(fake.ConnectedWith); + Assert.NotEqual(ConnectionHealth.Connected, adapter.Status); + } + + [Fact] + public async Task ConnectAsync_fails_closed_on_secret_ref_with_no_resolver() + { + var fake = new FakeMxGatewayClient(); + var adapter = new MxGatewayDataConnection(fake, NullLogger.Instance, secretResolver: null); + + await Assert.ThrowsAsync( + () => adapter.ConnectAsync(Details("secret:mxgateway/testconn/api-key"))); + + Assert.Null(fake.ConnectedWith); + Assert.NotEqual(ConnectionHealth.Connected, adapter.Status); + } + + // DI-threading seam (code-review follow-up): the four tests above construct the adapter + // directly with an explicit resolver, so they don't prove the factory→adapter DI wiring + // G-3 actually relies on. This test resolves IDataConnectionFactory from a real container + // (AddDataConnectionLayer + a registered fake ISecretResolver, no AddZbSecrets), creates a + // "MxGateway" adapter through it, and drives a secret:-ref ConnectAsync. The real client + // then fails to reach a non-existent gateway (expected, ignored) — but resolution happens + // BEFORE any connect attempt, so the fake resolver being invoked proves the greediest + // 3-arg DataConnectionFactory ctor picked up the DI-registered resolver and threaded it in. + // Had it been null, ConnectAsync would have thrown the "no ISecretResolver" fail-closed + // path instead and the resolver would never have been called. + [Fact] + public async Task Factory_from_DI_threads_registered_ISecretResolver_into_MxGateway_adapter() + { + var fake = new FakeSecretResolver("resolved-from-di"); + var services = new ServiceCollection(); + services.AddLogging(); + services.AddSingleton(new ConfigurationBuilder().Build()); + services.AddDataConnectionLayer(); + services.AddSingleton(fake); + using var provider = services.BuildServiceProvider(); + + var factory = provider.GetRequiredService(); + + // Point at a fast-refusing loopback port so the real client's connect fails quickly. + var details = Details("secret:mxgateway/testconn/api-key"); + details["Endpoint"] = "http://127.0.0.1:1"; + + await using var adapter = factory.Create("MxGateway", details); + Assert.IsType(adapter); + + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10)); + try + { + await adapter.ConnectAsync(details, cts.Token); + } + catch + { + // The real RealMxGatewayClient cannot reach 127.0.0.1:1 — irrelevant to this test. + } + + // Resolution runs before the connect attempt: the resolver being invoked proves the + // factory threaded the DI-registered ISecretResolver into the adapter. + Assert.Contains("mxgateway/testconn/api-key", fake.Requested); + } +}