1392fd144a
- Add WhitespaceAuthorization_ValidXApiKey_Returns200: pins the IsNullOrWhiteSpace
fall-through — a present-but-blank Authorization header is treated as absent so a
valid X-API-Key still authenticates (200).
- Remove MissingBearer_Returns401 (added in 510559e): identical path to
NeitherHeader_Returns401 (no Authorization + no X-API-Key → 401); keep the
descriptively-named NeitherHeader variant.
- Change "legacy 'X-API-Key'" -> "alternate 'X-API-Key'" in EndpointExtensions.cs and
the BuildPostWithApiKeyHeader/HappyPath doc comments to avoid implying Bearer is
the older transport (Bearer was itself introduced by the prior auth re-arch).
254 lines
12 KiB
C#
254 lines
12 KiB
C#
using System.Text.Json;
|
|
using Microsoft.AspNetCore.Builder;
|
|
using Microsoft.AspNetCore.Http;
|
|
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;
|
|
|
|
namespace ZB.MOM.WW.ScadaBridge.InboundAPI;
|
|
|
|
/// <summary>
|
|
/// Review N1: log-category marker for the inbound API endpoint. Used as the type
|
|
/// argument to <see cref="ILogger{TCategoryName}"/> so inbound-API auth/authz log
|
|
/// lines are categorized under this ScadaBridge.InboundAPI type rather than under
|
|
/// the shared <c>ZB.MOM.WW.Auth</c> library's <c>IApiKeyVerifier</c>. Exists only
|
|
/// to name the log category (<see cref="EndpointExtensions"/> is static and cannot
|
|
/// be a generic type argument); it is never instantiated.
|
|
/// </summary>
|
|
public sealed class InboundApiEndpoint
|
|
{
|
|
private InboundApiEndpoint() { }
|
|
}
|
|
|
|
/// <summary>
|
|
/// WP-1: POST /api/{methodName} endpoint registration.
|
|
/// WP-2: Method routing and parameter validation.
|
|
/// WP-3: Script execution on central.
|
|
/// WP-5: Error handling — 401, 403, 400, 500.
|
|
/// </summary>
|
|
public static class EndpointExtensions
|
|
{
|
|
/// <summary>
|
|
/// 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.
|
|
/// </summary>
|
|
private const string NotApprovedMessage = "API key not approved for this method";
|
|
|
|
/// <summary>
|
|
/// 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.
|
|
/// </summary>
|
|
private const string UnauthorizedMessage = "Invalid or missing API key";
|
|
|
|
/// <summary>Registers the <c>POST /api/{methodName}</c> inbound API endpoint with the active-node gate and body-size filter applied.</summary>
|
|
/// <param name="endpoints">The route builder to add the endpoint to.</param>
|
|
/// <returns>The same <paramref name="endpoints"/> instance for chaining.</returns>
|
|
public static IEndpointRouteBuilder MapInboundAPI(this IEndpointRouteBuilder endpoints)
|
|
{
|
|
endpoints.MapPost("/api/{methodName}", HandleInboundApiRequest)
|
|
// InboundAPI-006 / InboundAPI-008: active-node gating + request body
|
|
// size cap are enforced by the endpoint filter before the handler runs.
|
|
.AddEndpointFilter<InboundApiEndpointFilter>();
|
|
return endpoints;
|
|
}
|
|
|
|
private static async Task<IResult> HandleInboundApiRequest(
|
|
HttpContext httpContext,
|
|
string methodName)
|
|
{
|
|
// Review N1: log under an endpoint-owned category (a marker type in the
|
|
// ScadaBridge.InboundAPI namespace) rather than ILogger<IApiKeyVerifier>,
|
|
// so inbound-API auth log lines land under ScadaBridge — not the shared
|
|
// ZB.MOM.WW.Auth library namespace — when operators filter by category.
|
|
// (EndpointExtensions is static and cannot be an ILogger type argument, so
|
|
// the namespace-local InboundApiEndpoint marker carries the category.)
|
|
var logger = httpContext.RequestServices.GetRequiredService<ILogger<InboundApiEndpoint>>();
|
|
var verifier = httpContext.RequestServices.GetRequiredService<IApiKeyVerifier>();
|
|
var repository = httpContext.RequestServices.GetRequiredService<IInboundApiRepository>();
|
|
var executor = httpContext.RequestServices.GetRequiredService<InboundScriptExecutor>();
|
|
var routeHelper = httpContext.RequestServices.GetRequiredService<RouteHelper>();
|
|
var options = httpContext.RequestServices.GetRequiredService<IOptions<InboundApiOptions>>().Value;
|
|
|
|
// Auth re-arch (A+B) + X-API-Key restore: the inbound credential is accepted
|
|
// from EITHER the Authorization header ("Bearer sbk_<keyId>_<secret>") OR the
|
|
// alternate "X-API-Key: sbk_<keyId>_<secret>" header (raw token). Both are passed
|
|
// to the SAME shared ZB.MOM.WW.Auth.ApiKeys verifier — the parser strips an
|
|
// optional "Bearer " prefix and otherwise accepts a bare token, so the
|
|
// peppered-HMAC constant-time secret compare is identical for both transports.
|
|
// Authorization takes precedence when both headers are present.
|
|
var authorizationHeader = httpContext.Request.Headers.Authorization.ToString();
|
|
var credential = !string.IsNullOrWhiteSpace(authorizationHeader)
|
|
? authorizationHeader
|
|
: httpContext.Request.Headers["X-API-Key"].ToString();
|
|
var verification = await verifier.VerifyAsync(
|
|
credential, httpContext.RequestAborted);
|
|
|
|
if (!verification.Succeeded)
|
|
{
|
|
// 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("<unauthorized>");
|
|
|
|
// WP-5: Failures-only logging.
|
|
logger.LogWarning(
|
|
"Inbound API auth failure for method {Method}: {Failure} (status 401)",
|
|
methodName, verification.Failure);
|
|
|
|
return Results.Json(
|
|
new { error = UnauthorizedMessage },
|
|
statusCode: StatusCodes.Status401Unauthorized);
|
|
}
|
|
|
|
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.
|
|
// Both the not-found and not-in-scope branches return 403 with the SAME
|
|
// body; 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.
|
|
//
|
|
// Review #3 (scope-check-before-DB-lookup): the in-memory scope check runs
|
|
// FIRST and the DB GetMethodByNameAsync only runs when the caller is in
|
|
// scope. This removes a timing oracle — a not-in-scope caller no longer
|
|
// pays a DB round-trip whose latency could distinguish "valid scope but no
|
|
// such method" from "not in scope" — and saves the round-trip entirely on
|
|
// the reject path. The combined `method == null || !inScope` guard still
|
|
// emits the single identical 403 body, preserving enumeration-safety.
|
|
//
|
|
// Review #2 (scope case-policy): scope strings ARE the method names, and
|
|
// identity.Scopes.Contains(methodName) is an ordinal, case-SENSITIVE
|
|
// comparison (HashSet<string> with the default StringComparer.Ordinal). A
|
|
// scope must therefore match the registered method name's casing EXACTLY —
|
|
// "Echo" does not grant "echo". This is the intended invariant: method
|
|
// names are case-sensitive identifiers, and a key's granted scopes must be
|
|
// provisioned with the exact casing of the methods they authorize.
|
|
var inScope = identity.Scopes.Contains(methodName);
|
|
var method = inScope
|
|
? await repository.GetMethodByNameAsync(methodName, httpContext.RequestAborted)
|
|
: null;
|
|
|
|
if (method == null || !inScope)
|
|
{
|
|
ScadaBridgeTelemetry.RecordInboundApiRequest("<forbidden>");
|
|
|
|
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
|
|
// catalogue (an exact-name lookup), so the `method` tag is bounded to the
|
|
// set of configured API methods — never the raw caller-supplied route value.
|
|
ScadaBridgeTelemetry.RecordInboundApiRequest(method.Name);
|
|
|
|
// 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 auth+authz succeeded — auth failures leave the
|
|
// slot empty and the middleware records the row with Actor=null.
|
|
httpContext.Items[AuditWriteMiddleware.AuditActorItemKey] =
|
|
identity.DisplayName;
|
|
|
|
// WP-2: Deserialize and validate parameters
|
|
JsonElement? body = null;
|
|
try
|
|
{
|
|
// InboundAPI-020: the content-type sniff must be case-insensitive — a
|
|
// request with `application/JSON` or `Application/Json` is still JSON
|
|
// and must enter the body-parsing path. The previous case-sensitive
|
|
// `Contains("json")` silently skipped JSON deserialization for any
|
|
// capitalised value, leaving `body = null` and surfacing required
|
|
// parameters as 400 "missing" even though the caller sent a valid body.
|
|
if (httpContext.Request.ContentLength > 0
|
|
|| httpContext.Request.ContentType?.Contains("json", StringComparison.OrdinalIgnoreCase) == true)
|
|
{
|
|
using var doc = await JsonDocument.ParseAsync(
|
|
httpContext.Request.Body, cancellationToken: httpContext.RequestAborted);
|
|
body = doc.RootElement.Clone();
|
|
}
|
|
}
|
|
catch (JsonException)
|
|
{
|
|
return Results.Json(
|
|
new { error = "Invalid JSON in request body" },
|
|
statusCode: 400);
|
|
}
|
|
|
|
var paramResult = ParameterValidator.Validate(body, method.ParameterDefinitions);
|
|
if (!paramResult.IsValid)
|
|
{
|
|
return Results.Json(
|
|
new { error = paramResult.ErrorMessage },
|
|
statusCode: 400);
|
|
}
|
|
|
|
// WP-3: Execute the method's script
|
|
var timeout = method.TimeoutSeconds > 0
|
|
? TimeSpan.FromSeconds(method.TimeoutSeconds)
|
|
: options.DefaultMethodTimeout;
|
|
|
|
// Audit Log #23 (ParentExecutionId): the inbound request's per-request
|
|
// ExecutionId was minted early by AuditWriteMiddleware and stashed on
|
|
// HttpContext.Items. Thread it into the executor so a routed
|
|
// Route.To(...).Call(...) carries it as RouteToCallRequest.ParentExecutionId
|
|
// — the spawned site script execution points back at this inbound request.
|
|
var parentExecutionId =
|
|
httpContext.Items.TryGetValue(
|
|
AuditWriteMiddleware.InboundExecutionIdItemKey, out var stashedExecutionId)
|
|
&& stashedExecutionId is Guid inboundExecutionId
|
|
? inboundExecutionId
|
|
: (Guid?)null;
|
|
|
|
var scriptResult = await executor.ExecuteAsync(
|
|
method, paramResult.Parameters, routeHelper, timeout,
|
|
httpContext.RequestAborted, parentExecutionId);
|
|
|
|
if (!scriptResult.Success)
|
|
{
|
|
// InboundAPI-004: a client-aborted request is not a script failure.
|
|
// Do not pollute the failure log (reserved for genuine script errors)
|
|
// and do not attempt to write a 500 body to an already-gone connection.
|
|
if (httpContext.RequestAborted.IsCancellationRequested)
|
|
{
|
|
return Results.Empty;
|
|
}
|
|
|
|
// WP-5: 500 for script failures, safe error message
|
|
logger.LogWarning(
|
|
"Inbound API script failure for method {Method}: {Error}",
|
|
methodName, scriptResult.ErrorMessage);
|
|
|
|
return Results.Json(
|
|
new { error = scriptResult.ErrorMessage ?? "Internal server error" },
|
|
statusCode: 500);
|
|
}
|
|
|
|
// Return the script result as JSON
|
|
if (scriptResult.ResultJson != null)
|
|
{
|
|
return Results.Text(scriptResult.ResultJson, "application/json", statusCode: 200);
|
|
}
|
|
|
|
return Results.Ok();
|
|
}
|
|
}
|