46cb6965ac
Security-sensitive batch, handled main-thread for careful judgment on secret-leak and pepper-bypass paths. Secret leak / pepper bypass: - CD-016 (pepper bypass): InboundApiRepository's GetApiKeyByValueAsync no longer hashes the candidate with the unpeppered ApiKeyHasher.Default — ctor takes a lazy Func<IApiKeyHasher> accessor (lazy so test composition roots without a pepper still bring up the repository), and the DI registration wires sp.GetService<IApiKeyHasher>() so the production peppered hasher matches the stored KeyHash. Regression test asserts positive (peppered roundtrip) AND negative (Default hasher misses the same key — proving the lookup uses the injected hasher). - MgmtSvc-020 (SMTP credential leak): UpdateSmtpConfig/ListSmtpConfigs now project through SmtpConfigPublicShape so the response payload and audit-row afterState never carry the Credentials field — only a HasCredentials bool. The SMTP password / OAuth2 client secret no longer leaves the Admin-only UpdateSmtpConfig boundary the caller already supplied it to. Redaction: - AuditLog-008 (test-fixture under-redact): new SafeDefaultAuditPayloadFilter (stateless singleton) does HTTP header redaction for the always-sensitive defaults (Authorization, X-Api-Key, Cookie, Set-Cookie). FallbackAuditWriter, CentralAuditWriter, and AuditLogIngestActor (both ingest paths) default to it instead of null — composition roots that bypass AddAuditLog can no longer write unredacted auth headers to the audit store. - NotifService-025 (over-mask): CredentialRedactor.Scrub now only masks the last colon-separated component (password / clientSecret) AND only if it's >= 12 chars (typical password heuristic). Short user names like "root" no longer become global redaction tokens that eat unrelated diagnostic text. The full packed string is always masked regardless of length. 3 new negative tests pin the no-over-mask contract. Audit-row correctness / fail-loud: - InboundAPI-025: Program.cs UseWhen predicate now excludes /api/audit, /api/management, /api/centralui, /api/script-analysis AND requires POST — the AuditWriteMiddleware no longer emits spurious ApiInbound rows for audit-log query/export endpoints (write-on-read recursion broken). - ESG-021: ApplyAuth now logs Warning (not silent) on empty AuthConfiguration for apikey/basic, unknown AuthType, and malformed Basic config. AuthConfiguration value NEVER logged. AuthType=none remains silent (documented unauthenticated sentinel). - Security-021: AddSecurity now logs a startup Warning when RequireHttpsCookie=false — an HTTP-only deployment that previously transmitted the cookie-embedded JWT silently in cleartext is now audible in the log. Defensive: - CD-021: SwitchOutPartitionAsync's monthBoundary format string now yyyy-MM-dd HH:mm:ss.fffffff (datetime2(7) precision) so a future sub-second / non-midnight boundary doesn't silently round to the wrong partition. Plus reconciled stale per-module Open-findings counters that had drifted from earlier sessions (AuditLog, CD, ESG, IAPI, MgmtSvc, NotifService, Security). Build clean; all affected test projects green (Host 208, ConfigDB 242, ESG 69, IAPI 151, MgmtSvc 100, NotifService 55, Security 85, AuditLog 247/248 — 1 pre-existing date-sensitive integration test flake on PartitionPurgeTests, unrelated). README regenerated: 46 open (was 54).
184 lines
6.6 KiB
C#
184 lines
6.6 KiB
C#
using Microsoft.EntityFrameworkCore;
|
|
using Microsoft.Extensions.Logging;
|
|
using ScadaLink.Commons.Entities.InboundApi;
|
|
using ScadaLink.ConfigurationDatabase;
|
|
using ScadaLink.ConfigurationDatabase.Repositories;
|
|
|
|
namespace ScadaLink.ConfigurationDatabase.Tests;
|
|
|
|
public class InboundApiRepositoryTests : IDisposable
|
|
{
|
|
private readonly ScadaLinkDbContext _context;
|
|
private readonly CapturingLogger<InboundApiRepository> _logger = new();
|
|
private readonly InboundApiRepository _repository;
|
|
|
|
public InboundApiRepositoryTests()
|
|
{
|
|
_context = SqliteTestHelper.CreateInMemoryContext();
|
|
_repository = new InboundApiRepository(_context, hasherAccessor: null, logger: _logger);
|
|
}
|
|
|
|
public void Dispose()
|
|
{
|
|
_context.Database.CloseConnection();
|
|
_context.Dispose();
|
|
}
|
|
|
|
[Fact]
|
|
public async Task AddApiKey_AndGetById_RoundTrips()
|
|
{
|
|
var key = new ApiKey("Key1", "secret-value-1") { IsEnabled = true };
|
|
await _repository.AddApiKeyAsync(key);
|
|
await _repository.SaveChangesAsync();
|
|
|
|
var loaded = await _repository.GetApiKeyByIdAsync(key.Id);
|
|
Assert.NotNull(loaded);
|
|
Assert.Equal("Key1", loaded!.Name);
|
|
|
|
var byValue = await _repository.GetApiKeyByValueAsync("secret-value-1");
|
|
Assert.NotNull(byValue);
|
|
Assert.Equal(key.Id, byValue!.Id);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task CD016_GetApiKeyByValue_UsesInjectedPepperedHasher_NotDefault()
|
|
{
|
|
// CD-016 regression: stored KeyHash is produced by a peppered hasher.
|
|
// A repository whose lookup uses ApiKeyHasher.Default (the pre-fix
|
|
// behaviour) would compute a different digest and return null. With the
|
|
// pepper-aware hasherAccessor wired in, the lookup must round-trip.
|
|
var peppered = new Commons.Types.InboundApi.ApiKeyHasher("a-strong-test-pepper-of-sufficient-length");
|
|
var pepperedHash = peppered.Hash("secret-with-pepper");
|
|
var key = ApiKey.FromHash("Peppered", pepperedHash);
|
|
key.IsEnabled = true;
|
|
|
|
using var ctx = SqliteTestHelper.CreateInMemoryContext();
|
|
var repo = new InboundApiRepository(ctx, hasherAccessor: () => peppered, logger: _logger);
|
|
await repo.AddApiKeyAsync(key);
|
|
await repo.SaveChangesAsync();
|
|
|
|
var byValue = await repo.GetApiKeyByValueAsync("secret-with-pepper");
|
|
Assert.NotNull(byValue);
|
|
Assert.Equal(key.Id, byValue!.Id);
|
|
|
|
// And: a repository wired with the Default (unpeppered) hasher MUST
|
|
// NOT find the same key — proving the lookup actually uses the
|
|
// injected hasher and the original bug shape.
|
|
var defaultRepo = new InboundApiRepository(ctx,
|
|
hasherAccessor: () => Commons.Types.InboundApi.ApiKeyHasher.Default,
|
|
logger: _logger);
|
|
var missByDefault = await defaultRepo.GetApiKeyByValueAsync("secret-with-pepper");
|
|
Assert.Null(missByDefault);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task AddApiMethod_AndGetByName_RoundTrips()
|
|
{
|
|
var method = new ApiMethod("DoThing", "return 1;");
|
|
await _repository.AddApiMethodAsync(method);
|
|
await _repository.SaveChangesAsync();
|
|
|
|
var loaded = await _repository.GetMethodByNameAsync("DoThing");
|
|
Assert.NotNull(loaded);
|
|
Assert.Equal(method.Id, loaded!.Id);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task GetApprovedKeysForMethod_WithValidCsv_ReturnsAllKeys()
|
|
{
|
|
var k1 = new ApiKey("K1", "v1");
|
|
var k2 = new ApiKey("K2", "v2");
|
|
await _repository.AddApiKeyAsync(k1);
|
|
await _repository.AddApiKeyAsync(k2);
|
|
await _repository.SaveChangesAsync();
|
|
|
|
var method = new ApiMethod("M", "return 1;") { ApprovedApiKeyIds = $"{k1.Id}, {k2.Id}" };
|
|
await _repository.AddApiMethodAsync(method);
|
|
await _repository.SaveChangesAsync();
|
|
|
|
var keys = await _repository.GetApprovedKeysForMethodAsync(method.Id);
|
|
|
|
Assert.Equal(2, keys.Count);
|
|
Assert.Empty(_logger.Warnings);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task GetApprovedKeysForMethod_WithMalformedCsvToken_LogsWarningAndDropsToken()
|
|
{
|
|
// Regression guard for ConfigurationDatabase-008: a corrupt token (a name where an
|
|
// integer id is expected) must not be dropped silently — the corruption must be
|
|
// observable via a logged warning, while the valid ids still resolve.
|
|
var k1 = new ApiKey("K1", "v1");
|
|
await _repository.AddApiKeyAsync(k1);
|
|
await _repository.SaveChangesAsync();
|
|
|
|
var method = new ApiMethod("M", "return 1;") { ApprovedApiKeyIds = $"{k1.Id},not-an-id" };
|
|
await _repository.AddApiMethodAsync(method);
|
|
await _repository.SaveChangesAsync();
|
|
|
|
var keys = await _repository.GetApprovedKeysForMethodAsync(method.Id);
|
|
|
|
Assert.Single(keys);
|
|
Assert.Equal(k1.Id, keys[0].Id);
|
|
Assert.Single(_logger.Warnings);
|
|
Assert.Contains("not-an-id", _logger.Warnings[0]);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task GetApprovedKeysForMethod_WithNullOrEmptyCsv_ReturnsEmptyWithoutWarning()
|
|
{
|
|
var method = new ApiMethod("M", "return 1;");
|
|
await _repository.AddApiMethodAsync(method);
|
|
await _repository.SaveChangesAsync();
|
|
|
|
var keys = await _repository.GetApprovedKeysForMethodAsync(method.Id);
|
|
|
|
Assert.Empty(keys);
|
|
Assert.Empty(_logger.Warnings);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task DeleteApiMethod_RemovesEntity()
|
|
{
|
|
var method = new ApiMethod("ToDelete", "return 1;");
|
|
await _repository.AddApiMethodAsync(method);
|
|
await _repository.SaveChangesAsync();
|
|
|
|
await _repository.DeleteApiMethodAsync(method.Id);
|
|
await _repository.SaveChangesAsync();
|
|
|
|
Assert.Null(await _repository.GetApiMethodByIdAsync(method.Id));
|
|
}
|
|
|
|
[Fact]
|
|
public void Constructor_NullContext_Throws()
|
|
{
|
|
Assert.Throws<ArgumentNullException>(() => new InboundApiRepository(null!));
|
|
}
|
|
}
|
|
|
|
/// <summary>Minimal ILogger that captures warning-level messages for assertions.</summary>
|
|
internal sealed class CapturingLogger<T> : ILogger<T>
|
|
{
|
|
public List<string> Warnings { get; } = new();
|
|
|
|
public IDisposable BeginScope<TState>(TState state) where TState : notnull => NullScope.Instance;
|
|
|
|
public bool IsEnabled(LogLevel logLevel) => true;
|
|
|
|
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception,
|
|
Func<TState, Exception?, string> formatter)
|
|
{
|
|
if (logLevel == LogLevel.Warning)
|
|
{
|
|
Warnings.Add(formatter(state, exception));
|
|
}
|
|
}
|
|
|
|
private sealed class NullScope : IDisposable
|
|
{
|
|
public static readonly NullScope Instance = new();
|
|
public void Dispose() { }
|
|
}
|
|
}
|