diff --git a/Directory.Packages.props b/Directory.Packages.props index a81b4c54..51f96913 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -80,10 +80,10 @@ - - - - + + + + diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs index deedecf9..07fa8e26 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs @@ -1,3 +1,4 @@ +using ZB.MOM.WW.Auth.ApiKeys.DependencyInjection; using ZB.MOM.WW.Health; using ZB.MOM.WW.Health.Akka; using ZB.MOM.WW.Health.EntityFrameworkCore; @@ -106,6 +107,43 @@ try builder.Services.AddSecurity(builder.Configuration); builder.Services.AddCentralUI(); builder.Services.AddInboundAPI(); + + // Inbound-API auth re-arch (A+B), additive: stand up the shared + // ZB.MOM.WW.Auth.ApiKeys verifier + SQLite store + startup migration + // ALONGSIDE the legacy peppered-HMAC X-API-Key path. The POST + // /api/{methodName} endpoint now authenticates Bearer tokens + // (sbk__) and authorizes by scope == method name through + // this verifier. The legacy ApiKeyValidator/IApiKeyHasher remain + // registered (unused by the endpoint) until a later sub-task retires the + // SQL Server ApiKey entity. + // + // ApiKeyOptions is an init-only record, so the contract-mandated values + // are injected as in-memory configuration UNDER the bound section path + // (ScadaBridge:InboundApi:ApiKeyStore) rather than mutated post-bind: + // - TokenPrefix = "sbk" (the inbound token prefix) + // - PepperSecretName points at the EXISTING inbound-API pepper config key + // (reused so one secret backs both the legacy and new path during the + // additive window) + // - RunMigrationsOnStartup = true (hosted service creates the schema) + // - SqlitePath defaults under the content root's data/ directory, but any + // value already supplied via appsettings/env wins (AddInMemoryCollection + // is registered last, but only fills keys the operator did not set + // because we read the existing value first). + const string apiKeyStoreSection = "ScadaBridge:InboundApi:ApiKeyStore"; + var configuredSqlitePath = builder.Configuration[$"{apiKeyStoreSection}:SqlitePath"]; + var apiKeyStoreDefaults = new Dictionary + { + [$"{apiKeyStoreSection}:TokenPrefix"] = "sbk", + [$"{apiKeyStoreSection}:PepperSecretName"] = "ScadaBridge:InboundApi:ApiKeyPepper", + [$"{apiKeyStoreSection}:RunMigrationsOnStartup"] = "true", + [$"{apiKeyStoreSection}:SqlitePath"] = string.IsNullOrWhiteSpace(configuredSqlitePath) + ? Path.Combine(builder.Environment.ContentRootPath, "data", "inbound-api-keys.sqlite") + : configuredSqlitePath, + }; + builder.Configuration.AddInMemoryCollection(apiKeyStoreDefaults); + + builder.Services.AddZbApiKeyAuth(builder.Configuration, apiKeyStoreSection); + builder.Services.AddManagementService(); var configDbConnectionString = configuration["ScadaBridge:Database:ConfigurationDb"] diff --git a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/EndpointExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/EndpointExtensions.cs index a04c3f2e..fe34b238 100644 --- a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/EndpointExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/EndpointExtensions.cs @@ -5,6 +5,8 @@ using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using ZB.MOM.WW.Auth.Abstractions.ApiKeys; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; using ZB.MOM.WW.ScadaBridge.Commons.Observability; using ZB.MOM.WW.ScadaBridge.InboundAPI.Middleware; @@ -18,6 +20,24 @@ namespace ZB.MOM.WW.ScadaBridge.InboundAPI; /// public static class EndpointExtensions { + /// + /// Auth re-arch (A+B), InboundAPI-011 successor: the single message used for + /// BOTH "method not found" and "key not in scope for this method" so the two + /// outcomes are indistinguishable to the caller. A caller holding any valid key + /// must not be able to enumerate which method names exist by observing a + /// status/message difference, so both branches return 403 with this identical + /// body and the caller-supplied method name is never echoed back. + /// + private const string NotApprovedMessage = "API key not approved for this method"; + + /// + /// Auth re-arch (A+B): the generic 401 message. Every verifier failure reason + /// (missing/malformed token, unknown key, revoked key, pepper unavailable, + /// secret mismatch) maps to this one message so the auth stage is never leaked + /// to the caller. + /// + private const string UnauthorizedMessage = "Invalid or missing API key"; + /// Registers the POST /api/{methodName} inbound API endpoint with the active-node gate and body-size filter applied. /// The route builder to add the endpoint to. public static IEndpointRouteBuilder MapInboundAPI(this IEndpointRouteBuilder endpoints) @@ -33,39 +53,65 @@ public static class EndpointExtensions HttpContext httpContext, string methodName) { - var logger = httpContext.RequestServices.GetRequiredService>(); - var validator = httpContext.RequestServices.GetRequiredService(); + var logger = httpContext.RequestServices.GetRequiredService>(); + var verifier = httpContext.RequestServices.GetRequiredService(); + var repository = httpContext.RequestServices.GetRequiredService(); var executor = httpContext.RequestServices.GetRequiredService(); var routeHelper = httpContext.RequestServices.GetRequiredService(); var options = httpContext.RequestServices.GetRequiredService>().Value; - // WP-1: Extract and validate API key - var apiKeyValue = httpContext.Request.Headers["X-API-Key"].FirstOrDefault(); - var validationResult = await validator.ValidateAsync(apiKeyValue, methodName, httpContext.RequestAborted); + // Auth re-arch (A+B): the inbound credential is now a Bearer token + // (Authorization: Bearer sbk__) verified by the shared + // ZB.MOM.WW.Auth.ApiKeys verifier — peppered-HMAC constant-time secret + // compare is handled inside the library verifier. The raw X-API-Key header + // and the in-repo ApiKeyValidator are retired on this path. + var authorizationHeader = httpContext.Request.Headers.Authorization.ToString(); + var verification = await verifier.VerifyAsync( + authorizationHeader, httpContext.RequestAborted); - if (!validationResult.IsValid) + if (!verification.Succeeded) { - // Telemetry follow-on: count every inbound request, including auth - // failures. The raw {methodName} route value is arbitrary caller input - // and would be high-cardinality, so failures are tagged with a small - // bounded set of sentinels keyed off the validator's status code rather - // than the unvalidated name (401 → "", 403 → ""). - ScadaBridgeTelemetry.RecordInboundApiRequest( - validationResult.StatusCode == StatusCodes.Status401Unauthorized - ? "" - : ""); + // WP-5: 401 for any verifier failure. The failure reason is + // discriminated for our own logs/telemetry but NEVER surfaced to the + // caller — every reason maps to the one generic message so the auth + // stage (missing vs unknown-key vs revoked vs secret-mismatch) is not + // leaked. + ScadaBridgeTelemetry.RecordInboundApiRequest(""); - // WP-5: Failures-only logging + // WP-5: Failures-only logging. logger.LogWarning( - "Inbound API auth failure for method {Method}: {Error} (status {StatusCode})", - methodName, validationResult.ErrorMessage, validationResult.StatusCode); + "Inbound API auth failure for method {Method}: {Failure} (status 401)", + methodName, verification.Failure); return Results.Json( - new { error = validationResult.ErrorMessage }, - statusCode: validationResult.StatusCode); + new { error = UnauthorizedMessage }, + statusCode: StatusCodes.Status401Unauthorized); } - var method = validationResult.Method!; + var identity = verification.Identity!; + + // Auth re-arch (A+B), enumeration-safety: "method not found" and "key not + // in scope for this method" MUST produce an indistinguishable response. + // The method is resolved for execution (script / parameters / timeout), but + // the not-found and not-in-scope branches both return 403 with the SAME + // body. Resolving the method first and folding both negatives into one + // branch keeps the two cases byte-identical (status + message), so a caller + // holding a valid key cannot probe which method names exist. + var method = await repository.GetMethodByNameAsync(methodName, httpContext.RequestAborted); + var inScope = identity.Scopes.Contains(methodName); + + if (method == null || !inScope) + { + ScadaBridgeTelemetry.RecordInboundApiRequest(""); + + logger.LogWarning( + "Inbound API authz failure for method {Method}: not approved (status 403)", + methodName); + + return Results.Json( + new { error = NotApprovedMessage }, + statusCode: StatusCodes.Status403Forbidden); + } // Telemetry follow-on: count this inbound request against the resolved, // registered method name. method.Name comes from the repository's method @@ -73,12 +119,12 @@ public static class EndpointExtensions // set of configured API methods — never the raw caller-supplied route value. ScadaBridgeTelemetry.RecordInboundApiRequest(method.Name); - // Audit Log (#23 M4 Bundle D): publish the resolved API key name so + // Audit Log (#23 M4 Bundle D): publish the verified key's display name so // AuditWriteMiddleware can populate AuditEvent.Actor in its finally - // block. Done AFTER validation succeeded — auth failures leave the + // block. Done AFTER auth+authz succeeded — auth failures leave the // slot empty and the middleware records the row with Actor=null. httpContext.Items[AuditWriteMiddleware.AuditActorItemKey] = - validationResult.ApiKey!.Name; + identity.DisplayName; // WP-2: Deserialize and validate parameters JsonElement? body = null; diff --git a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ZB.MOM.WW.ScadaBridge.InboundAPI.csproj b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ZB.MOM.WW.ScadaBridge.InboundAPI.csproj index 3af1480c..b6307013 100644 --- a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ZB.MOM.WW.ScadaBridge.InboundAPI.csproj +++ b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ZB.MOM.WW.ScadaBridge.InboundAPI.csproj @@ -25,6 +25,12 @@ + + diff --git a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointContentTypeTests.cs b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointContentTypeTests.cs index 31050b49..5c4a3a29 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointContentTypeTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointContentTypeTests.cs @@ -1,15 +1,20 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using NSubstitute; +using ZB.MOM.WW.Auth.Abstractions.ApiKeys; +using ZB.MOM.WW.Auth.ApiKeys; +using ZB.MOM.WW.Auth.ApiKeys.Admin; +using ZB.MOM.WW.Auth.ApiKeys.DependencyInjection; +using ZB.MOM.WW.Auth.ApiKeys.Sqlite; using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; -using ZB.MOM.WW.ScadaBridge.Commons.Types.InboundApi; using System.Net; using System.Net.Http.Headers; using System.Text; @@ -24,18 +29,22 @@ namespace ZB.MOM.WW.ScadaBridge.InboundAPI.Tests; /// was case-sensitive so a capitalised value silently skipped body parsing /// and any required parameters surfaced as a 400 even though the caller /// sent a valid JSON body. +/// +/// +/// Auth re-arch (A+B): the request carries a Bearer token verified by the shared +/// ZB.MOM.WW.Auth.ApiKeys verifier (scope == method name), not the legacy X-API-Key +/// header. The content-type behaviour under test is downstream of auth and unchanged. +/// /// -public class EndpointContentTypeTests +public sealed class EndpointContentTypeTests : IDisposable { - /// - /// Stub hasher that returns its input unchanged. Lets the test pre-seed the - /// repository with a known "hash" value without depending on the real - /// HMAC-with-pepper hasher. - /// - private sealed class IdentityHasher : IApiKeyHasher - { - public string Hash(string keyValue) => keyValue; - } + private const string Pepper = "test-pepper-at-least-16-chars-long"; + private const string PepperConfigKey = "ScadaBridge:InboundApi:ApiKeyPepper"; + private const string TokenPrefix = "sbk"; + private const string ApiKeyStoreSection = "ScadaBridge:InboundApi:ApiKeyStore"; + + private readonly string _sqlitePath = + Path.Combine(Path.GetTempPath(), $"inbound-api-keys-ct-{Guid.NewGuid():N}.sqlite"); [Theory] [InlineData("application/json")] @@ -44,13 +53,8 @@ public class EndpointContentTypeTests [InlineData("APPLICATION/JSON")] public async Task ContentTypeCheck_IsCaseInsensitive_ParsesBodyForAnyCasing(string contentType) { - const string apiKeyValue = "test-key"; const string methodName = "echoParam"; - var key = ApiKey.FromHash("test", apiKeyValue); - key.IsEnabled = true; - key.Id = 1; - var method = new ApiMethod(methodName, "return Parameters[\"value\"];") { Id = 1, @@ -62,25 +66,22 @@ public class EndpointContentTypeTests }; var repo = Substitute.For(); - repo.GetAllApiKeysAsync(Arg.Any()) - .Returns(new List { key }); repo.GetMethodByNameAsync(methodName, Arg.Any()) .Returns(method); - repo.GetApprovedKeysForMethodAsync(method.Id, Arg.Any()) - .Returns(new List { key }); using var host = await BuildHostAsync(repo); + var token = await SeedKeyAsync(host, methodName); var client = host.GetTestClient(); var request = new HttpRequestMessage(HttpMethod.Post, "/api/" + methodName) { // Bypass HttpClient's MediaTypeHeaderValue auto-normalization by - // setting the header through TryAddWithoutValidation — we need the - // exact casing reach the server intact. + // setting the header through MediaTypeHeaderValue.Parse — we need the + // exact casing to reach the server intact. Content = new ByteArrayContent(Encoding.UTF8.GetBytes("{\"value\":42}")) }; request.Content.Headers.ContentType = MediaTypeHeaderValue.Parse(contentType); - request.Headers.Add("X-API-Key", apiKeyValue); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); var response = await client.SendAsync(request); var body = await response.Content.ReadAsStringAsync(); @@ -91,33 +92,56 @@ public class EndpointContentTypeTests Assert.Contains("42", body); } - private static async Task BuildHostAsync(IInboundApiRepository repo) + /// Seeds a key scoped for and returns its Bearer token. + private static async Task SeedKeyAsync(IHost host, string methodName) { + var services = host.Services; + var commands = new ApiKeyAdminCommands( + services.GetRequiredService>().Value, + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService()); + + var result = await commands.CreateKeyAsync( + "key1", "ct-caller", new HashSet { methodName }, + constraintsJson: null, remoteAddress: null, CancellationToken.None); + return result.Token!; + } + + private async Task BuildHostAsync(IInboundApiRepository repo) + { + // The pepper provider reads the HOST's IConfiguration (AddZbApiKeyAuth only + // TryAdds its own), so the api-key settings — pepper included — must live in + // the host configuration. + var apiKeySettings = new Dictionary + { + [PepperConfigKey] = Pepper, + [$"{ApiKeyStoreSection}:TokenPrefix"] = TokenPrefix, + [$"{ApiKeyStoreSection}:PepperSecretName"] = PepperConfigKey, + [$"{ApiKeyStoreSection}:SqlitePath"] = _sqlitePath, + [$"{ApiKeyStoreSection}:RunMigrationsOnStartup"] = "true", + }; + var hostBuilder = new HostBuilder() + .ConfigureAppConfiguration(config => config.AddInMemoryCollection(apiKeySettings)) .ConfigureWebHost(webBuilder => { webBuilder .UseTestServer() - .ConfigureServices(services => + .ConfigureServices((context, services) => { services.AddRouting(); services.AddSingleton(repo); // RouteHelper depends on IInstanceLocator + IInstanceRouter // (InboundAPI-017). Tests for content-type handling never - // route, so both can be no-op stubs — the production - // CommunicationServiceInstanceRouter would need a real - // CommunicationService which isn't wired here. + // route, so both can be no-op stubs. services.AddSingleton(Substitute.For()); services.Configure(_ => { }); services.AddInboundAPI(); + services.AddZbApiKeyAuth(context.Configuration, ApiKeyStoreSection); services.RemoveAll(); services.AddSingleton(Substitute.For()); - // The production AddInboundAPI registration of IApiKeyHasher - // requires a configured pepper. Replace it with the identity - // stub so the seeded ApiKey.KeyHash matches "test-key" - // deterministically without depending on configuration. - services.RemoveAll(); - services.AddSingleton(new IdentityHasher()); services.AddLogging(); }) .Configure(app => @@ -129,4 +153,24 @@ public class EndpointContentTypeTests return await hostBuilder.StartAsync(); } + + public void Dispose() + { + try + { + Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools(); + foreach (var suffix in new[] { "", "-wal", "-shm" }) + { + var path = _sqlitePath + suffix; + if (File.Exists(path)) + { + File.Delete(path); + } + } + } + catch + { + // Best-effort cleanup. + } + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointExtensionsTests.cs b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointExtensionsTests.cs index a5a14fcb..526be1e4 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointExtensionsTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointExtensionsTests.cs @@ -2,50 +2,66 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options; using NSubstitute; +using ZB.MOM.WW.Auth.Abstractions.ApiKeys; +using ZB.MOM.WW.Auth.ApiKeys; +using ZB.MOM.WW.Auth.ApiKeys.Admin; +using ZB.MOM.WW.Auth.ApiKeys.DependencyInjection; +using ZB.MOM.WW.Auth.ApiKeys.Sqlite; using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; using ZB.MOM.WW.ScadaBridge.Commons.Observability; -using ZB.MOM.WW.ScadaBridge.Commons.Types.InboundApi; using ZB.MOM.WW.ScadaBridge.InboundAPI.Middleware; using System.Diagnostics.Metrics; using System.Net; +using System.Net.Http.Headers; using System.Text; namespace ZB.MOM.WW.ScadaBridge.InboundAPI.Tests; /// -/// InboundAPI-023: is -/// the composition wiring that ties validator → JSON parse → ParameterValidator → -/// InboundScriptExecutor → response shaping together. Each composed component -/// has its own unit tests, but the wiring itself was uncovered. These tests -/// drive the end-to-end POST /api/{methodName} flow through a TestServer so a -/// regression in any of the seams below would be caught here: +/// Auth re-arch (A+B): now +/// authenticates with the shared ZB.MOM.WW.Auth.ApiKeys verifier — a Bearer +/// token (Authorization: Bearer sbk_<keyId>_<secret>) replaces the +/// raw X-API-Key header, and a key's Scopes set (the API-method names it may +/// call) replaces the per-method approval table for authorization. These tests drive +/// the end-to-end POST /api/{methodName} flow through a TestServer, seeding the +/// library SQLite store via , so a regression in any +/// of the seams below would be caught here: /// -/// 1. happy path — 200 + script result body -/// 2. auth failures — validator status code propagates verbatim -/// 3. invalid JSON body — 400 + sanitized error -/// 4. parameter validation failure — 400 + ParameterValidator's error message -/// 5. script failure — 500 + ErrorMessage in body -/// 6. successful auth must publish the resolved API key name into +/// 1. happy path — valid Bearer + in-scope → 200 + script result body. +/// 2. valid key, method NOT in scope → 403 (authz). +/// 3. unknown method → 403 with the SAME body as not-in-scope +/// (enumeration-safety — neither the status nor the message reveals which). +/// 4. missing / garbage Bearer → 401 (generic, no stage leak). +/// 5. revoked key → 401. +/// 6. invalid JSON body → 400 + sanitized error. +/// 7. parameter validation failure → 400 + ParameterValidator's error message. +/// 8. script failure → 500 + ErrorMessage in body. +/// 9. successful auth must publish the verified key's DISPLAY NAME into /// HttpContext.Items[AuditWriteMiddleware.AuditActorItemKey] (so the /// AuditWriteMiddleware sees a non-null Actor when it emits the audit row). /// -public class EndpointExtensionsTests +public sealed class EndpointExtensionsTests : IDisposable { - /// - /// Stub hasher that returns its input unchanged. Same pattern as - /// — lets us seed an ApiKey with a - /// known "hash" without depending on the configured HMAC pepper. - /// - private sealed class IdentityHasher : IApiKeyHasher - { - public string Hash(string keyValue) => keyValue; - } + // The pepper used to back the seeded keys. The verifier resolves it from + // configuration under the name ApiKeyOptions.PepperSecretName; the seed path + // resolves it through the same IApiKeyPepperProvider, so both agree. + private const string Pepper = "test-pepper-at-least-16-chars-long"; + private const string PepperConfigKey = "ScadaBridge:InboundApi:ApiKeyPepper"; + private const string TokenPrefix = "sbk"; + private const string ApiKeyStoreSection = "ScadaBridge:InboundApi:ApiKeyStore"; + + // Each test gets its own throwaway SQLite database so seeded keys never leak + // between tests; the file is deleted on Dispose. + private readonly string _sqlitePath = + Path.Combine(Path.GetTempPath(), $"inbound-api-keys-{Guid.NewGuid():N}.sqlite"); /// /// Inline middleware that captures the value at @@ -58,16 +74,6 @@ public class EndpointExtensionsTests public string? CapturedActor { get; set; } } - private const string ApiKeyValue = "test-key"; - - private static ApiKey SeedKey(int id = 1, string name = "test") - { - var key = ApiKey.FromHash(name, ApiKeyValue); - key.IsEnabled = true; - key.Id = id; - return key; - } - private static ApiMethod SeedMethod( int id, string name, string script, string? paramDefs = null) { @@ -80,16 +86,18 @@ public class EndpointExtensionsTests } [Fact] - public async Task HappyPath_Returns200WithScriptResultJson() + public async Task HappyPath_ValidBearerInScope_Returns200WithScriptResultJson() { - var key = SeedKey(); var method = SeedMethod(1, "echo", "return Parameters[\"value\"];", """[{"name":"value","type":"Integer","required":true}]"""); - using var host = await BuildHostAsync(key, method); + using var host = await BuildHostAsync(method); + // Seed a key whose scope set contains the method name "echo". + var token = await SeedKeyAsync(host, keyId: "key1", displayName: "echo-caller", + scopes: new[] { "echo" }); var client = host.GetTestClient(); - var request = BuildPost("echo", """{"value":7}"""); + var request = BuildPost("echo", """{"value":7}""", token); var response = await client.SendAsync(request); var body = await response.Content.ReadAsStringAsync(); @@ -98,15 +106,15 @@ public class EndpointExtensionsTests } [Fact] - public async Task MissingApiKey_Returns401() + public async Task MissingBearer_Returns401() { - var key = SeedKey(); var method = SeedMethod(1, "noKey", "return 1;"); - using var host = await BuildHostAsync(key, method); + using var host = await BuildHostAsync(method); + await SeedKeyAsync(host, "key1", "noKey-caller", new[] { "noKey" }); var client = host.GetTestClient(); - // No X-API-Key header — auth should reject with 401. + // No Authorization header — auth should reject with 401. var request = new HttpRequestMessage(HttpMethod.Post, "/api/noKey") { Content = new StringContent("{}", Encoding.UTF8, "application/json"), @@ -117,32 +125,104 @@ public class EndpointExtensionsTests } [Fact] - public async Task UnknownMethod_Returns403_IndistinguishableFromNotApproved() + public async Task GarbageBearer_Returns401() { - // InboundAPI-011: method existence is intentionally not observable — - // both "method not found" and "key not approved" surface as 403. - var key = SeedKey(); - var method = SeedMethod(1, "knownMethod", "return 1;"); + var method = SeedMethod(1, "noKey", "return 1;"); - using var host = await BuildHostAsync(key, method); + using var host = await BuildHostAsync(method); + await SeedKeyAsync(host, "key1", "noKey-caller", new[] { "noKey" }); var client = host.GetTestClient(); - var request = BuildPost("unknownMethod", "{}"); + // A well-formed Authorization header carrying a non-parseable / wrong-prefix + // token is indistinguishable from a missing credential — still 401. + var request = new HttpRequestMessage(HttpMethod.Post, "/api/noKey") + { + Content = new StringContent("{}", Encoding.UTF8, "application/json"), + }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", "not-a-real-token"); + var response = await client.SendAsync(request); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [Fact] + public async Task RevokedKey_Returns401() + { + var method = SeedMethod(1, "echo", "return 1;"); + + using var host = await BuildHostAsync(method); + // Seed-then-revoke: the token is well-formed and the secret matches, but + // the key is revoked — the verifier fails closed and the endpoint maps it + // to the generic 401 (no "revoked" stage leak). + var token = await SeedKeyAsync(host, "key1", "echo-caller", new[] { "echo" }); + await RevokeKeyAsync(host, "key1"); + var client = host.GetTestClient(); + + var request = BuildPost("echo", "{}", token); + var response = await client.SendAsync(request); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [Fact] + public async Task ValidKey_MethodNotInScope_Returns403() + { + // The key authenticates fine, but its scope set does NOT contain the + // requested method name — authorization fails with 403. + var method = SeedMethod(1, "secured", "return 1;"); + + using var host = await BuildHostAsync(method); + // Scope grants "otherMethod", not "secured". + var token = await SeedKeyAsync(host, "key1", "limited-caller", new[] { "otherMethod" }); + var client = host.GetTestClient(); + + var request = BuildPost("secured", "{}", token); var response = await client.SendAsync(request); Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); } [Fact] - public async Task InvalidJsonBody_Returns400() + public async Task UnknownMethod_Returns403_WithIdenticalBodyToNotInScope() { - var key = SeedKey(); - var method = SeedMethod(1, "badJson", "return 1;"); + // Enumeration-safety: "method not found" and "key not in scope" must be + // indistinguishable — same 403 status AND same response body. We drive both + // cases through the same valid key and assert byte-identical bodies. + var method = SeedMethod(1, "knownMethod", "return 1;"); - using var host = await BuildHostAsync(key, method); + using var host = await BuildHostAsync(method); + // Key is in scope for "knownMethod" only — so: + // - posting "unknownMethod" → method-not-found 403 + // - posting "knownMethod" with a key scoped elsewhere would be not-in-scope 403 + var unknownToken = await SeedKeyAsync(host, "key1", "caller", new[] { "unknownMethod" }); + var notInScopeToken = await SeedKeyAsync(host, "key2", "caller2", new[] { "somethingElse" }); var client = host.GetTestClient(); - var request = BuildPost("badJson", "{ not json"); + // (a) unknown method — the key IS in scope for "unknownMethod" (so scope + // passes) but no such method exists in the repository → method-not-found 403. + var unknownResponse = await client.SendAsync(BuildPost("unknownMethod", "{}", unknownToken)); + var unknownBody = await unknownResponse.Content.ReadAsStringAsync(); + + // (b) known method but key not in scope for it → not-in-scope 403. + var notInScopeResponse = await client.SendAsync(BuildPost("knownMethod", "{}", notInScopeToken)); + var notInScopeBody = await notInScopeResponse.Content.ReadAsStringAsync(); + + Assert.Equal(HttpStatusCode.Forbidden, unknownResponse.StatusCode); + Assert.Equal(HttpStatusCode.Forbidden, notInScopeResponse.StatusCode); + // The crux of the enumeration-safety invariant: identical bodies. + Assert.Equal(notInScopeBody, unknownBody); + } + + [Fact] + public async Task InvalidJsonBody_Returns400() + { + var method = SeedMethod(1, "badJson", "return 1;"); + + using var host = await BuildHostAsync(method); + var token = await SeedKeyAsync(host, "key1", "caller", new[] { "badJson" }); + var client = host.GetTestClient(); + + var request = BuildPost("badJson", "{ not json", token); var response = await client.SendAsync(request); var body = await response.Content.ReadAsStringAsync(); @@ -153,15 +233,15 @@ public class EndpointExtensionsTests [Fact] public async Task MissingRequiredParameter_Returns400_FromParameterValidator() { - var key = SeedKey(); var method = SeedMethod(1, "needsParam", "return Parameters[\"value\"];", """[{"name":"value","type":"Integer","required":true}]"""); - using var host = await BuildHostAsync(key, method); + using var host = await BuildHostAsync(method); + var token = await SeedKeyAsync(host, "key1", "caller", new[] { "needsParam" }); var client = host.GetTestClient(); // Body is empty object — required parameter "value" is missing. - var request = BuildPost("needsParam", "{}"); + var request = BuildPost("needsParam", "{}", token); var response = await client.SendAsync(request); var body = await response.Content.ReadAsStringAsync(); @@ -173,19 +253,19 @@ public class EndpointExtensionsTests [Fact] public async Task ScriptThrows_Returns500_WithSanitizedErrorBody() { - var key = SeedKey(); // Throws inside the script body — InboundScriptExecutor catches the // exception, logs it server-side, and surfaces the generic "Internal // script error" message to the caller (the executor deliberately does - // not leak raw exception details — see InboundScriptExecutor.ExecuteAsync's - // catch block). The endpoint maps the script failure to HTTP 500. + // not leak raw exception details). The endpoint maps the script failure + // to HTTP 500. var method = SeedMethod(1, "boom", """throw new System.InvalidOperationException("boom-msg");"""); - using var host = await BuildHostAsync(key, method); + using var host = await BuildHostAsync(method); + var token = await SeedKeyAsync(host, "key1", "caller", new[] { "boom" }); var client = host.GetTestClient(); - var request = BuildPost("boom", "{}"); + var request = BuildPost("boom", "{}", token); var response = await client.SendAsync(request); var body = await response.Content.ReadAsStringAsync(); @@ -197,19 +277,18 @@ public class EndpointExtensionsTests } [Fact] - public async Task SuccessfulAuth_StashesResolvedApiKeyNameOnHttpContextItems() + public async Task SuccessfulAuth_StashesVerifiedKeyDisplayNameOnHttpContextItems() { - // InboundAPI-023: the handler stashes the resolved API key's display name - // at HttpContext.Items[AuditWriteMiddleware.AuditActorItemKey] AFTER auth - // succeeded, so AuditWriteMiddleware sees a populated Actor when it - // emits the audit row. A capture middleware reads the slot once the - // endpoint finishes, proving the wiring still publishes it. - var key = SeedKey(id: 99, name: "audit-actor-name"); + // The handler stashes the VERIFIED key's display name at + // HttpContext.Items[AuditWriteMiddleware.AuditActorItemKey] AFTER auth+authz + // succeeded, so AuditWriteMiddleware sees a populated Actor when it emits + // the audit row. A capture middleware reads the slot once the endpoint + // finishes, proving the wiring still publishes it. var method = SeedMethod(1, "stamp", "return 1;"); var capture = new AuditActorCapture(); - using var host = await BuildHostAsync(key, method, customize: builder => + using var host = await BuildHostAsync(method, customize: builder => { builder.Use(async (ctx, next) => { @@ -225,9 +304,10 @@ public class EndpointExtensionsTests { services.AddSingleton(capture); }); + var token = await SeedKeyAsync(host, "key1", "audit-actor-name", new[] { "stamp" }); var client = host.GetTestClient(); - var request = BuildPost("stamp", "{}"); + var request = BuildPost("stamp", "{}", token); var response = await client.SendAsync(request); Assert.Equal(HttpStatusCode.OK, response.StatusCode); @@ -241,16 +321,16 @@ public class EndpointExtensionsTests // scadabridge.inbound_api.requests once, tagged with the resolved, // registered method name (method.Name) — the bounded identifier, not the // raw route value. - var key = SeedKey(); var method = SeedMethod(1, "echo", "return Parameters[\"value\"];", """[{"name":"value","type":"Integer","required":true}]"""); using var collector = new InboundApiRequestCounterCollector(); - using var host = await BuildHostAsync(key, method); + using var host = await BuildHostAsync(method); + var token = await SeedKeyAsync(host, "key1", "caller", new[] { "echo" }); var client = host.GetTestClient(); - var response = await client.SendAsync(BuildPost("echo", """{"value":7}""")); + var response = await client.SendAsync(BuildPost("echo", """{"value":7}""", token)); Assert.Equal(HttpStatusCode.OK, response.StatusCode); // Filter by the method tag this test produced: the counter is a process-wide @@ -264,19 +344,21 @@ public class EndpointExtensionsTests [Fact] public async Task UnknownMethod_EmitsInboundApiRequestCounter_WithBoundedForbiddenSentinel() { - // Telemetry follow-on: an auth/authz failure is still counted, but the - // tag is a bounded sentinel ("") rather than the arbitrary - // caller-supplied route value — so an attacker posting random method - // names cannot blow up the `method` tag cardinality. - var key = SeedKey(); + // Telemetry follow-on: an authz failure is still counted, but the tag is a + // bounded sentinel ("") rather than the arbitrary caller-supplied + // route value — so an attacker posting random method names cannot blow up + // the `method` tag cardinality. var method = SeedMethod(1, "knownMethod", "return 1;"); using var collector = new InboundApiRequestCounterCollector(); - using var host = await BuildHostAsync(key, method); + using var host = await BuildHostAsync(method); + // Key is in scope for the made-up name, so scope passes and the request + // falls through to method-not-found (403) — exercising the forbidden path. + var token = await SeedKeyAsync(host, "key1", "caller", new[] { "totally-made-up-name" }); var client = host.GetTestClient(); - var response = await client.SendAsync(BuildPost("totally-made-up-name", "{}")); + var response = await client.SendAsync(BuildPost("totally-made-up-name", "{}", token)); Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); var measurements = collector.Measurements; @@ -342,49 +424,100 @@ public class EndpointExtensionsTests public void Dispose() => _listener.Dispose(); } - private static HttpRequestMessage BuildPost(string methodName, string body) + private static HttpRequestMessage BuildPost(string methodName, string body, string bearerToken) { var request = new HttpRequestMessage(HttpMethod.Post, "/api/" + methodName) { Content = new StringContent(body, Encoding.UTF8, "application/json"), }; - request.Headers.Add("X-API-Key", ApiKeyValue); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", bearerToken); return request; } - private static async Task BuildHostAsync( - ApiKey key, + /// + /// Seeds a key with the given scopes into the library SQLite store via + /// (the public admin seam) and + /// returns the assembled Bearer token (sbk_<keyId>_<secret>) — + /// the only moment the secret is ever available. The verifier registered in the + /// host's DI shares the same SQLite path + pepper, so it accepts this token. + /// + private static async Task SeedKeyAsync( + IHost host, string keyId, string displayName, IReadOnlyCollection scopes) + { + var commands = BuildAdminCommands(host); + var result = await commands.CreateKeyAsync( + keyId, displayName, new HashSet(scopes), + constraintsJson: null, remoteAddress: null, CancellationToken.None); + Assert.NotNull(result.Token); + return result.Token!; + } + + private static async Task RevokeKeyAsync(IHost host, string keyId) + { + var commands = BuildAdminCommands(host); + await commands.RevokeKeyAsync(keyId, remoteAddress: null, CancellationToken.None); + } + + /// + /// Builds an over the stores + pepper provider + + /// migrator that AddZbApiKeyAuth registered in the host's DI, so seeding writes to + /// the exact same database/pepper the verifier reads from. + /// + private static ApiKeyAdminCommands BuildAdminCommands(IHost host) + { + var services = host.Services; + return new ApiKeyAdminCommands( + services.GetRequiredService>().Value, + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService()); + } + + private async Task BuildHostAsync( ApiMethod method, Action? customize = null, Action? additionalServices = null) { var repo = Substitute.For(); - repo.GetAllApiKeysAsync(Arg.Any()) - .Returns(new List { key }); repo.GetMethodByNameAsync(method.Name, Arg.Any()) .Returns(method); - repo.GetApprovedKeysForMethodAsync(method.Id, Arg.Any()) - .Returns(new List { key }); + + // The pepper provider (ConfigurationApiKeyPepperProvider) reads the HOST's + // IConfiguration, and AddZbApiKeyAuth only TryAdds its own config (so the + // host registration wins). The api-key settings — including the pepper — + // must therefore live in the host configuration, not a separate object. + var apiKeySettings = new Dictionary + { + [PepperConfigKey] = Pepper, + [$"{ApiKeyStoreSection}:TokenPrefix"] = TokenPrefix, + [$"{ApiKeyStoreSection}:PepperSecretName"] = PepperConfigKey, + [$"{ApiKeyStoreSection}:SqlitePath"] = _sqlitePath, + [$"{ApiKeyStoreSection}:RunMigrationsOnStartup"] = "true", + }; var hostBuilder = new HostBuilder() + .ConfigureAppConfiguration(config => config.AddInMemoryCollection(apiKeySettings)) .ConfigureWebHost(webBuilder => { webBuilder .UseTestServer() - .ConfigureServices(services => + .ConfigureServices((context, services) => { services.AddRouting(); services.AddSingleton(repo); services.AddSingleton(Substitute.For()); services.Configure(_ => { }); services.AddInboundAPI(); - // Replace the production CommunicationService-backed - // router and the configured HMAC hasher with test stubs - // (same pattern as EndpointContentTypeTests). + // Stand up the shared verifier + SQLite store + migration + // hosted service against the per-test database and pepper, + // binding from the host configuration the pepper provider reads. + services.AddZbApiKeyAuth(context.Configuration, ApiKeyStoreSection); + // Replace the production CommunicationService-backed router + // with a test stub (it would otherwise need a real + // CommunicationService which isn't wired here). services.RemoveAll(); services.AddSingleton(Substitute.For()); - services.RemoveAll(); - services.AddSingleton(new IdentityHasher()); services.AddLogging(); additionalServices?.Invoke(services); }) @@ -398,4 +531,26 @@ public class EndpointExtensionsTests return await hostBuilder.StartAsync(); } + + public void Dispose() + { + try + { + // SqliteConnection pooling can hold the file open; clear pools before + // deleting so the temp DB (and its -wal/-shm sidecars) are removed. + Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools(); + foreach (var suffix in new[] { "", "-wal", "-shm" }) + { + var path = _sqlitePath + suffix; + if (File.Exists(path)) + { + File.Delete(path); + } + } + } + catch + { + // Best-effort cleanup; a leaked temp file is harmless. + } + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests.csproj b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests.csproj index 6a23f981..2af82258 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests.csproj +++ b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests.csproj @@ -19,6 +19,11 @@ + +