fix(management): elide DatabaseConnection ConnectionString from List/Get (arch-review C3)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
@@ -190,6 +190,8 @@ The two-person authorization workflow for writes through the MxAccess Gateway. B
|
||||
- **ListDatabaseConnections** / **GetDatabaseConnection**: Query database connection definitions.
|
||||
- **CreateDatabaseConnection** / **UpdateDatabaseConnection** / **DeleteDatabaseConnection**: Manage database connections.
|
||||
|
||||
> **Secret elision (arch-review C3).** The ADO.NET `ConnectionString` can embed the SQL password. List/Get responses are projected through `DatabaseConnectionPublicShape` (`{ id, name, hasConnectionString }`) — the connection string itself is never emitted. `UpdateDatabaseConnectionDefCommand.ConnectionString` is nullable and preserve-if-null: an omitted (`null`) value leaves the stored connection string intact, so a round-trip of the elided shape does not wipe the secret.
|
||||
|
||||
### Inbound API Methods
|
||||
|
||||
- **ListApiMethods** / **GetApiMethod**: Query inbound API method definitions.
|
||||
|
||||
+4
-1
@@ -3,5 +3,8 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Management;
|
||||
public record ListDatabaseConnectionsCommand;
|
||||
public record GetDatabaseConnectionCommand(int DatabaseConnectionId);
|
||||
public record CreateDatabaseConnectionDefCommand(string Name, string ConnectionString);
|
||||
public record UpdateDatabaseConnectionDefCommand(int DatabaseConnectionId, string Name, string ConnectionString);
|
||||
// ConnectionString is nullable (arch-review C3): an omitted (null) value preserves
|
||||
// the stored secret rather than wiping it — the connection string is elided from
|
||||
// List/Get responses, so a round-trip must not require re-sending it.
|
||||
public record UpdateDatabaseConnectionDefCommand(int DatabaseConnectionId, string Name, string? ConnectionString);
|
||||
public record DeleteDatabaseConnectionDefCommand(int DatabaseConnectionId);
|
||||
|
||||
@@ -2637,16 +2637,30 @@ public class ManagementActor : ReceiveActor
|
||||
// Database Connection Definition handlers
|
||||
// ========================================================================
|
||||
|
||||
/// <summary>
|
||||
/// Secret-elided DatabaseConnectionDefinition projection (arch-review C3): the
|
||||
/// ADO.NET <c>ConnectionString</c> can embed the SQL password and must never
|
||||
/// leave this boundary via List/Get responses. Presence is surfaced as
|
||||
/// <c>HasConnectionString</c>. Mirrors <see cref="SmtpConfigPublicShape"/>.
|
||||
/// </summary>
|
||||
private static object DatabaseConnectionPublicShape(DatabaseConnectionDefinition d) => new
|
||||
{
|
||||
d.Id,
|
||||
d.Name,
|
||||
HasConnectionString = !string.IsNullOrEmpty(d.ConnectionString),
|
||||
};
|
||||
|
||||
private static async Task<object?> HandleListDatabaseConnections(IServiceProvider sp)
|
||||
{
|
||||
var repo = sp.GetRequiredService<IExternalSystemRepository>();
|
||||
return await repo.GetAllDatabaseConnectionsAsync();
|
||||
return (await repo.GetAllDatabaseConnectionsAsync()).Select(DatabaseConnectionPublicShape).ToList();
|
||||
}
|
||||
|
||||
private static async Task<object?> HandleGetDatabaseConnection(IServiceProvider sp, GetDatabaseConnectionCommand cmd)
|
||||
{
|
||||
var repo = sp.GetRequiredService<IExternalSystemRepository>();
|
||||
return await repo.GetDatabaseConnectionByIdAsync(cmd.DatabaseConnectionId);
|
||||
var def = await repo.GetDatabaseConnectionByIdAsync(cmd.DatabaseConnectionId);
|
||||
return def is null ? null : DatabaseConnectionPublicShape(def);
|
||||
}
|
||||
|
||||
private static async Task<object?> HandleCreateDatabaseConnection(IServiceProvider sp, CreateDatabaseConnectionDefCommand cmd, string user)
|
||||
@@ -2656,7 +2670,7 @@ public class ManagementActor : ReceiveActor
|
||||
await repo.AddDatabaseConnectionAsync(def);
|
||||
await repo.SaveChangesAsync();
|
||||
await AuditAsync(sp, user, "Create", "DatabaseConnection", def.Id.ToString(), def.Name, new { def.Id, def.Name });
|
||||
return def;
|
||||
return DatabaseConnectionPublicShape(def);
|
||||
}
|
||||
|
||||
private static async Task<object?> HandleUpdateDatabaseConnection(IServiceProvider sp, UpdateDatabaseConnectionDefCommand cmd, string user)
|
||||
@@ -2665,11 +2679,14 @@ public class ManagementActor : ReceiveActor
|
||||
var def = await repo.GetDatabaseConnectionByIdAsync(cmd.DatabaseConnectionId)
|
||||
?? throw new ManagementCommandException($"DatabaseConnection with ID {cmd.DatabaseConnectionId} not found.");
|
||||
def.Name = cmd.Name;
|
||||
def.ConnectionString = cmd.ConnectionString;
|
||||
// Preserve-if-null: an update that omits ConnectionString leaves the stored
|
||||
// secret intact — the connection string is elided from List/Get, so a
|
||||
// round-trip must not be required to re-send it.
|
||||
if (cmd.ConnectionString is not null) def.ConnectionString = cmd.ConnectionString;
|
||||
await repo.UpdateDatabaseConnectionAsync(def);
|
||||
await repo.SaveChangesAsync();
|
||||
await AuditAsync(sp, user, "Update", "DatabaseConnection", def.Id.ToString(), def.Name, new { def.Id, def.Name });
|
||||
return def;
|
||||
return DatabaseConnectionPublicShape(def);
|
||||
}
|
||||
|
||||
private static async Task<object?> HandleDeleteDatabaseConnection(IServiceProvider sp, DeleteDatabaseConnectionDefCommand cmd, string user)
|
||||
|
||||
@@ -218,4 +218,59 @@ public class SecretProjectionTests : TestKit, IDisposable
|
||||
Arg.Is<object>(o => !ManagementActor.SerializeResult(o).Contains("newsecret")),
|
||||
Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Task 9 — DatabaseConnection ConnectionString elision
|
||||
// ========================================================================
|
||||
|
||||
private IExternalSystemRepository AddDatabaseConnectionRepo(DatabaseConnectionDefinition def)
|
||||
{
|
||||
var repo = Substitute.For<IExternalSystemRepository>();
|
||||
repo.GetDatabaseConnectionByIdAsync(def.Id, Arg.Any<CancellationToken>()).Returns(def);
|
||||
repo.GetAllDatabaseConnectionsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<DatabaseConnectionDefinition> { def });
|
||||
_services.AddScoped(_ => repo);
|
||||
return repo;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetDatabaseConnection_ResponseOmitsConnectionString()
|
||||
{
|
||||
AddDatabaseConnectionRepo(new DatabaseConnectionDefinition("db", "Server=db;Password=sql-secret") { Id = 1 });
|
||||
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Envelope(new GetDatabaseConnectionCommand(1), "Viewer"));
|
||||
|
||||
var resp = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||
Assert.DoesNotContain("sql-secret", resp.JsonData);
|
||||
Assert.Contains("hasConnectionString", resp.JsonData);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ListDatabaseConnections_ResponseOmitsConnectionString()
|
||||
{
|
||||
AddDatabaseConnectionRepo(new DatabaseConnectionDefinition("db", "Server=db;Password=sql-secret") { Id = 1 });
|
||||
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Envelope(new ListDatabaseConnectionsCommand(), "Viewer"));
|
||||
|
||||
var resp = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||
Assert.DoesNotContain("sql-secret", resp.JsonData);
|
||||
Assert.Contains("hasConnectionString", resp.JsonData);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task UpdateDatabaseConnection_NullConnectionString_PreservesStoredSecret()
|
||||
{
|
||||
var stored = new DatabaseConnectionDefinition("db", "Server=db;Password=sql-secret") { Id = 1 };
|
||||
var repo = AddDatabaseConnectionRepo(stored);
|
||||
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Envelope(new UpdateDatabaseConnectionDefCommand(1, "db-renamed", null), "Designer"));
|
||||
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||
|
||||
await repo.Received(1).UpdateDatabaseConnectionAsync(
|
||||
Arg.Is<DatabaseConnectionDefinition>(d => d.ConnectionString == "Server=db;Password=sql-secret"),
|
||||
Arg.Any<CancellationToken>());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user