fix(templateengine): delete dead SiteService data-connection create/update that audited raw credentials (review 2026-10-21 08/C1)

This commit is contained in:
Joseph Doherty
2026-10-22 06:26:03 -04:00
parent 4102922640
commit c0792cff3e
2 changed files with 16 additions and 80 deletions
@@ -124,73 +124,10 @@ public class SiteService
// --- Data Connection CRUD ---
/// <summary>Creates a new data connection owned by the specified site.</summary>
/// <param name="siteId">Primary key of the owning site.</param>
/// <param name="name">Display name of the data connection.</param>
/// <param name="protocol">Protocol identifier (e.g., "OpcUa").</param>
/// <param name="primaryConfiguration">JSON configuration for the primary endpoint; may be null.</param>
/// <param name="backupConfiguration">JSON configuration for the backup endpoint; may be null.</param>
/// <param name="failoverRetryCount">Number of retries before switching to the backup endpoint.</param>
/// <param name="user">Authenticated username for audit logging.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A successful result containing the new <see cref="DataConnection"/>, or a failure result.</returns>
public async Task<Result<DataConnection>> CreateDataConnectionAsync(
int siteId, string name, string protocol, string? primaryConfiguration,
string? backupConfiguration, int failoverRetryCount, string user,
CancellationToken cancellationToken = default)
{
if (string.IsNullOrWhiteSpace(name))
return Result<DataConnection>.Failure("Data connection name is required.");
if (string.IsNullOrWhiteSpace(protocol))
return Result<DataConnection>.Failure("Protocol is required.");
var connection = new DataConnection(name, protocol, siteId)
{
PrimaryConfiguration = primaryConfiguration,
BackupConfiguration = backupConfiguration,
FailoverRetryCount = failoverRetryCount
};
await _repository.AddDataConnectionAsync(connection, cancellationToken);
await _repository.SaveChangesAsync(cancellationToken);
await _auditService.LogAsync(user, "Create", "DataConnection",
connection.Id.ToString(), name, connection, cancellationToken);
return Result<DataConnection>.Success(connection);
}
/// <summary>Updates an existing data connection's configuration.</summary>
/// <param name="connectionId">Primary key of the data connection to update.</param>
/// <param name="name">New display name.</param>
/// <param name="protocol">New protocol identifier.</param>
/// <param name="primaryConfiguration">New primary endpoint JSON configuration; may be null.</param>
/// <param name="backupConfiguration">New backup endpoint JSON configuration; may be null.</param>
/// <param name="failoverRetryCount">New failover retry count.</param>
/// <param name="user">Authenticated username for audit logging.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A successful result containing the updated <see cref="DataConnection"/>, or a failure result if not found.</returns>
public async Task<Result<DataConnection>> UpdateDataConnectionAsync(
int connectionId, string name, string protocol, string? primaryConfiguration,
string? backupConfiguration, int failoverRetryCount, string user,
CancellationToken cancellationToken = default)
{
var connection = await _repository.GetDataConnectionByIdAsync(connectionId, cancellationToken);
if (connection == null)
return Result<DataConnection>.Failure($"Data connection with ID {connectionId} not found.");
connection.Name = name;
connection.Protocol = protocol;
connection.PrimaryConfiguration = primaryConfiguration;
connection.BackupConfiguration = backupConfiguration;
connection.FailoverRetryCount = failoverRetryCount;
await _repository.UpdateDataConnectionAsync(connection, cancellationToken);
await _repository.SaveChangesAsync(cancellationToken);
await _auditService.LogAsync(user, "Update", "DataConnection",
connection.Id.ToString(), name, connection, cancellationToken);
return Result<DataConnection>.Success(connection);
}
// Create/update of data connections deliberately live only in the Management
// Service, which audits the secret-scrubbed public shape. The SiteService versions
// (no production callers) audited the raw entity — Primary/BackupConfiguration can
// embed protocol credentials — and were removed (review 2026-10-21 08/C1).
/// <summary>Deletes a data connection by its primary key.</summary>
/// <param name="connectionId">Primary key of the data connection to delete.</param>
@@ -98,20 +98,19 @@ public class SiteServiceTests
Assert.True(result.IsSuccess);
}
/// <summary>Verifies that creating a data connection with valid input succeeds.</summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task CreateDataConnection_ValidInput_Success()
/// <summary>
/// Review 2026-10-21 08/C1: SiteService must not offer a data-connection create/update
/// that audits the raw <c>DataConnection</c> entity — its Primary/BackupConfiguration
/// can embed protocol credentials, which would land in plaintext in the indefinitely
/// retained config audit. The Management Service path (which audits the scrubbed
/// public shape) is the only writer.
/// </summary>
[Theory]
[InlineData("CreateDataConnectionAsync")]
[InlineData("UpdateDataConnectionAsync")]
public void SiteService_HasNoRawDataConnectionWriter(string method)
{
_repoMock.Setup(r => r.SaveChangesAsync(It.IsAny<CancellationToken>()))
.ReturnsAsync(1);
var result = await _sut.CreateDataConnectionAsync(1, "OPC-Server1", "OpcUa", "{\"url\":\"opc.tcp://localhost\"}", null, 3, "admin");
Assert.True(result.IsSuccess);
Assert.Equal("OPC-Server1", result.Value.Name);
Assert.Equal("OpcUa", result.Value.Protocol);
Assert.Equal(1, result.Value.SiteId);
Assert.Null(typeof(SiteService).GetMethod(method));
}
/// <summary>Verifies that updating a site with valid input succeeds.</summary>