dohertj2/ScadaBridge
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

feat(auth)!: ScadaBridge retire SQL Server ApiKey entity + ApprovedApiKeyIds + legacy hashing; EF migration RetireInboundApiKeyStore; re-issue runbook + CHANGELOG (re-arch C5/E) — BREAKING: X-API-Key -> Bearer sbk_, keys re-issued

Browse Source
This commit is contained in:
Joseph Doherty
2026-06-02 05:39:59 -04:00
parent b13d7b3d28
commit afa55981d5
32 changed files with 2117 additions and 1193 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/InboundApiRepositoryTests.cs
+6 -129
View File
@@ -1,21 +1,24 @@
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase;
using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Repositories;
namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests;
// Auth re-arch (C5): the SQL Server ApiKey entity and the repository's key methods
// (Add/Get/Update/Delete/GetApprovedKeysForMethod) were retired — inbound API keys
// now live in the shared ZB.MOM.WW.Auth.ApiKeys SQLite store. The former key
// round-trip, peppered-hasher, and ApprovedApiKeyIds CSV tests were removed with
// them; only the API-method catalogue remains here.
public class InboundApiRepositoryTests : IDisposable
{
private readonly ScadaBridgeDbContext _context;
private readonly CapturingLogger<InboundApiRepository> _logger = new();
private readonly InboundApiRepository _repository;
public InboundApiRepositoryTests()
{
_context = SqliteTestHelper.CreateInMemoryContext();
_repository = new InboundApiRepository(_context, hasherAccessor: null, logger: _logger);
_repository = new InboundApiRepository(_context);
}
public void Dispose()
@@ -24,53 +27,6 @@ public class InboundApiRepositoryTests : IDisposable
_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()
{
@@ -83,60 +39,6 @@ public class InboundApiRepositoryTests : IDisposable
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()
{
@@ -156,28 +58,3 @@ public class InboundApiRepositoryTests : IDisposable
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() { }
}
}
tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/SchemaConfigurationTests.cs
+4 -25
View File
@@ -135,29 +135,8 @@ public class SplitQueryBehaviourTests : IDisposable
Assert.Single(loaded.Scripts);
}
// ConfigurationDatabase-012: the ApiKey table must persist the bearer credential
// as a hash column (KeyHash) and must NOT carry a plaintext KeyValue column.
[Fact]
public void ApiKey_KeyHashColumn_IsMappedAndUniquelyIndexed()
{
var entityType = _context.Model.FindEntityType(typeof(ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi.ApiKey))!;
var keyHash = entityType.FindProperty("KeyHash");
Assert.NotNull(keyHash);
Assert.False(keyHash!.IsNullable);
var hashIndex = entityType.GetIndexes()
.FirstOrDefault(i => i.Properties.Any(p => p.Name == "KeyHash"));
Assert.NotNull(hashIndex);
Assert.True(hashIndex!.IsUnique);
}
[Fact]
public void ApiKey_HasNoPlaintextKeyValueColumn()
{
var entityType = _context.Model.FindEntityType(typeof(ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi.ApiKey))!;
Assert.Null(entityType.FindProperty("KeyValue"));
}
// Auth re-arch (C5): the SQL Server ApiKey entity was retired (inbound keys now
// live in the shared ZB.MOM.WW.Auth.ApiKeys SQLite store), so the former
// ApiKey_KeyHashColumn_IsMappedAndUniquelyIndexed and
// ApiKey_HasNoPlaintextKeyValueColumn schema assertions were removed with it.
}
tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/UnitTest1.cs
+6 -5
View File
@@ -56,7 +56,8 @@ public class DbContextTests : IDisposable
Assert.NotNull(_context.SharedScripts);
Assert.NotNull(_context.LdapGroupMappings);
Assert.NotNull(_context.SiteScopeRules);
Assert.NotNull(_context.ApiKeys);
// Auth re-arch (C5): the ApiKeys DbSet was retired (inbound keys moved to the
// shared ZB.MOM.WW.Auth.ApiKeys SQLite store); only ApiMethods remains.
Assert.NotNull(_context.ApiMethods);
Assert.NotNull(_context.AuditLogEntries);
@@ -264,16 +265,16 @@ public class DbContextTests : IDisposable
}
[Fact]
public void InboundApi_ApiKeyAndMethod()
public void InboundApi_Method()
{
var key = new ApiKey("TestKey", "sk-test-123") { IsEnabled = true };
// Auth re-arch (C5): the SQL Server ApiKey entity was retired (inbound keys
// now live in the shared ZB.MOM.WW.Auth.ApiKeys SQLite store), so this case
// covers only the surviving ApiMethod catalogue.
var method = new ApiMethod("GetStatus", "return \"ok\";") { TimeoutSeconds = 30 };
_context.ApiKeys.Add(key);
_context.ApiMethods.Add(method);
_context.SaveChanges();
Assert.Single(_context.ApiKeys.Where(k => k.Name == "TestKey"));
Assert.Single(_context.ApiMethods.Where(m => m.Name == "GetStatus"));
}