fix(management): elide DataConnection config secrets from responses and audit (arch-review C3)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
@@ -134,6 +134,8 @@ Both endpoints honour any site-scope rules attached to the caller's audit role b
|
||||
- **CreateDataConnection** / **UpdateDataConnection** / **DeleteDataConnection**: Manage data connection definitions.
|
||||
- **AssignDataConnectionToSite** / **UnassignDataConnectionFromSite**: Manage site assignments.
|
||||
|
||||
> **Secret elision (arch-review C3).** The `PrimaryConfiguration`/`BackupConfiguration` JSON blobs can embed OPC UA user and certificate passwords, yet List/Get are readable by any authenticated user. All command responses **and** audit `afterState` are projected through a secret-elided shape (`DataConnectionPublicShape`): secret-bearing values inside the config JSON are replaced with a redaction sentinel and a `hasSecrets` flag is surfaced (mirrors the SMTP/SMS `HasCredentials` pattern). **Update** merges the sentinel via `ConfigSecretScrubber.MergeSentinels` — a client that round-trips a scrubbed config keeps the stored secret rather than wiping it.
|
||||
|
||||
### Deployments
|
||||
|
||||
- **DeployInstance**: Deploy configuration to a specific instance (includes pre-deployment validation).
|
||||
|
||||
@@ -1445,12 +1445,37 @@ public class ManagementActor : ReceiveActor
|
||||
// Data Connection handlers
|
||||
// ========================================================================
|
||||
|
||||
/// <summary>
|
||||
/// Secret-elided projection of a DataConnection (arch-review C3): the raw
|
||||
/// PrimaryConfiguration/BackupConfiguration JSON can embed OPC UA user and
|
||||
/// certificate passwords, and List/Get are readable by any authenticated user.
|
||||
/// Mirrors <see cref="SmtpConfigPublicShape"/>. The secret-bearing config blobs
|
||||
/// are scrubbed to the redaction sentinel; every non-config field is projected
|
||||
/// verbatim so callers keep the full shape they had before.
|
||||
/// </summary>
|
||||
private static object DataConnectionPublicShape(DataConnection c)
|
||||
{
|
||||
var (primary, s1) = ConfigSecretScrubber.Scrub(c.PrimaryConfiguration);
|
||||
var (backup, s2) = ConfigSecretScrubber.Scrub(c.BackupConfiguration);
|
||||
return new
|
||||
{
|
||||
c.Id,
|
||||
c.SiteId,
|
||||
c.Name,
|
||||
c.Protocol,
|
||||
c.FailoverRetryCount,
|
||||
PrimaryConfiguration = primary,
|
||||
BackupConfiguration = backup,
|
||||
HasSecrets = s1 || s2,
|
||||
};
|
||||
}
|
||||
|
||||
private static async Task<object?> HandleListDataConnections(IServiceProvider sp, ListDataConnectionsCommand cmd)
|
||||
{
|
||||
var repo = sp.GetRequiredService<ISiteRepository>();
|
||||
if (cmd.SiteId.HasValue)
|
||||
return await repo.GetDataConnectionsBySiteIdAsync(cmd.SiteId.Value);
|
||||
return await repo.GetAllDataConnectionsAsync();
|
||||
return (await repo.GetDataConnectionsBySiteIdAsync(cmd.SiteId.Value)).Select(DataConnectionPublicShape).ToList();
|
||||
return (await repo.GetAllDataConnectionsAsync()).Select(DataConnectionPublicShape).ToList();
|
||||
}
|
||||
|
||||
private static async Task<object?> HandleGetDataConnection(IServiceProvider sp, GetDataConnectionCommand cmd, AuthenticatedUser user)
|
||||
@@ -1459,7 +1484,7 @@ public class ManagementActor : ReceiveActor
|
||||
var conn = await repo.GetDataConnectionByIdAsync(cmd.DataConnectionId);
|
||||
if (conn != null)
|
||||
EnforceSiteScope(user, conn.SiteId);
|
||||
return conn;
|
||||
return conn is null ? null : DataConnectionPublicShape(conn);
|
||||
}
|
||||
|
||||
private static async Task<object?> HandleCreateDataConnection(IServiceProvider sp, CreateDataConnectionCommand cmd, string user)
|
||||
@@ -1473,8 +1498,9 @@ public class ManagementActor : ReceiveActor
|
||||
};
|
||||
await repo.AddDataConnectionAsync(conn);
|
||||
await repo.SaveChangesAsync();
|
||||
await AuditAsync(sp, user, "Create", "DataConnection", conn.Id.ToString(), conn.Name, conn);
|
||||
return conn;
|
||||
var publicShape = DataConnectionPublicShape(conn);
|
||||
await AuditAsync(sp, user, "Create", "DataConnection", conn.Id.ToString(), conn.Name, publicShape);
|
||||
return publicShape;
|
||||
}
|
||||
|
||||
private static async Task<object?> HandleUpdateDataConnection(IServiceProvider sp, UpdateDataConnectionCommand cmd, string user)
|
||||
@@ -1484,13 +1510,17 @@ public class ManagementActor : ReceiveActor
|
||||
?? throw new ManagementCommandException($"DataConnection with ID {cmd.DataConnectionId} not found.");
|
||||
conn.Name = cmd.Name;
|
||||
conn.Protocol = cmd.Protocol;
|
||||
conn.PrimaryConfiguration = cmd.PrimaryConfiguration;
|
||||
conn.BackupConfiguration = cmd.BackupConfiguration;
|
||||
// Sentinel-preserving merge: a client that round-trips a scrubbed config
|
||||
// (secrets replaced with the redaction sentinel) keeps the stored secret
|
||||
// rather than wiping it. Non-sentinel values overwrite as usual.
|
||||
conn.PrimaryConfiguration = ConfigSecretScrubber.MergeSentinels(cmd.PrimaryConfiguration, conn.PrimaryConfiguration);
|
||||
conn.BackupConfiguration = ConfigSecretScrubber.MergeSentinels(cmd.BackupConfiguration, conn.BackupConfiguration);
|
||||
conn.FailoverRetryCount = cmd.FailoverRetryCount;
|
||||
await repo.UpdateDataConnectionAsync(conn);
|
||||
await repo.SaveChangesAsync();
|
||||
await AuditAsync(sp, user, "Update", "DataConnection", conn.Id.ToString(), conn.Name, conn);
|
||||
return conn;
|
||||
var publicShape = DataConnectionPublicShape(conn);
|
||||
await AuditAsync(sp, user, "Update", "DataConnection", conn.Id.ToString(), conn.Name, publicShape);
|
||||
return publicShape;
|
||||
}
|
||||
|
||||
private static async Task<object?> HandleDeleteDataConnection(IServiceProvider sp, DeleteDataConnectionCommand cmd, string user)
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
using Akka.Actor;
|
||||
using Akka.TestKit.Xunit2;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using NSubstitute;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.ManagementService.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Secret-elision projection tests (arch-review C3): connection-config secrets
|
||||
/// (OPC UA passwords, external-system auth config, DB connection strings) must
|
||||
/// never leave the management boundary via List/Get responses or audit afterState,
|
||||
/// and sentinel/omitted round-trips through Update must preserve the stored secret.
|
||||
/// </summary>
|
||||
public class SecretProjectionTests : TestKit, IDisposable
|
||||
{
|
||||
private readonly IAuditService _auditService;
|
||||
private readonly ServiceCollection _services;
|
||||
|
||||
public SecretProjectionTests()
|
||||
{
|
||||
_auditService = Substitute.For<IAuditService>();
|
||||
_services = new ServiceCollection();
|
||||
_services.AddScoped(_ => _auditService);
|
||||
}
|
||||
|
||||
private IActorRef CreateActor()
|
||||
{
|
||||
var sp = _services.BuildServiceProvider();
|
||||
return Sys.ActorOf(Props.Create(() => new ManagementActor(
|
||||
sp, NullLogger<ManagementActor>.Instance)));
|
||||
}
|
||||
|
||||
private static ManagementEnvelope Envelope(object command, params string[] roles) =>
|
||||
new(new AuthenticatedUser("testuser", "Test User", roles, Array.Empty<string>()),
|
||||
command, Guid.NewGuid().ToString("N"));
|
||||
|
||||
void IDisposable.Dispose() => Shutdown();
|
||||
|
||||
// ========================================================================
|
||||
// Task 7 — DataConnection config secret elision
|
||||
// ========================================================================
|
||||
|
||||
private ISiteRepository AddSiteRepoWithConnection(DataConnection conn)
|
||||
{
|
||||
var siteRepo = Substitute.For<ISiteRepository>();
|
||||
siteRepo.GetDataConnectionByIdAsync(conn.Id, Arg.Any<CancellationToken>()).Returns(conn);
|
||||
siteRepo.GetAllDataConnectionsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<DataConnection> { conn });
|
||||
_services.AddScoped(_ => siteRepo);
|
||||
return siteRepo;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetDataConnection_ResponseOmitsPasswords()
|
||||
{
|
||||
AddSiteRepoWithConnection(new DataConnection("c1", "OpcUa", 1)
|
||||
{
|
||||
Id = 1,
|
||||
PrimaryConfiguration = """{"Endpoint":"opc.tcp://x","UserIdentity":{"Password":"opc-secret"}}""",
|
||||
});
|
||||
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Envelope(new GetDataConnectionCommand(1), "Viewer"));
|
||||
|
||||
var resp = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||
Assert.DoesNotContain("opc-secret", resp.JsonData);
|
||||
Assert.Contains("hasSecrets", resp.JsonData); // camelCase serializer
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ListDataConnections_ResponseOmitsPasswords()
|
||||
{
|
||||
AddSiteRepoWithConnection(new DataConnection("c1", "OpcUa", 1)
|
||||
{
|
||||
Id = 1,
|
||||
PrimaryConfiguration = """{"UserIdentity":{"Password":"opc-secret"}}""",
|
||||
});
|
||||
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Envelope(new ListDataConnectionsCommand(null), "Viewer"));
|
||||
|
||||
var resp = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||
Assert.DoesNotContain("opc-secret", resp.JsonData);
|
||||
Assert.Contains("hasSecrets", resp.JsonData);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task UpdateDataConnection_SentinelConfig_PreservesStoredSecret()
|
||||
{
|
||||
var stored = new DataConnection("c1", "OpcUa", 1)
|
||||
{
|
||||
Id = 1,
|
||||
PrimaryConfiguration = """{"Endpoint":"opc.tcp://old","UserIdentity":{"Password":"opc-secret"}}""",
|
||||
};
|
||||
var siteRepo = AddSiteRepoWithConnection(stored);
|
||||
|
||||
var actor = CreateActor();
|
||||
var incoming = """{"Endpoint":"opc.tcp://new","UserIdentity":{"Password":"***REDACTED***"}}""";
|
||||
actor.Tell(Envelope(new UpdateDataConnectionCommand(1, "c1", "OpcUa", incoming, null, 3), "Designer"));
|
||||
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||
|
||||
await siteRepo.Received(1).UpdateDataConnectionAsync(
|
||||
Arg.Is<DataConnection>(c => c.PrimaryConfiguration!.Contains("opc-secret")
|
||||
&& !c.PrimaryConfiguration.Contains("***REDACTED***")),
|
||||
Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UpdateDataConnection_AuditAfterState_OmitsPasswords()
|
||||
{
|
||||
var stored = new DataConnection("c1", "OpcUa", 1)
|
||||
{
|
||||
Id = 1,
|
||||
PrimaryConfiguration = """{"Password":"opc-secret"}""",
|
||||
};
|
||||
AddSiteRepoWithConnection(stored);
|
||||
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Envelope(new UpdateDataConnectionCommand(1, "c1", "OpcUa", """{"Password":"newpw"}""", null, 3), "Designer"));
|
||||
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||
|
||||
_auditService.Received().LogAsync(Arg.Any<string>(), "Update", "DataConnection",
|
||||
Arg.Any<string>(), Arg.Any<string>(),
|
||||
Arg.Is<object>(o => !ManagementActor.SerializeResult(o).Contains("newpw")),
|
||||
Arg.Any<CancellationToken>());
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user