mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-25 05:00:07 +00:00
fix(backend): derive AuthenticationMiddleware JWT-exempt set from policy catalog
The ticket-gated diagnostics/download route (PublicAnonymous, MCP session_debug) 401'd on the deployed backend: AuthenticationMiddleware kept its OWN hardcoded _anonymousRoutes allowlist, parallel to EndpointAccessPolicyCatalog, and the new route was added only to the catalog. Since AuthenticationMiddleware runs before PolicyEnforcementMiddleware, the missing entry rejected the request with a JWT 401 before the PublicAnonymous policy was ever honored. Classic hand-synced parallel-catalog drift. Delete _anonymousRoutes / _anonymousPrefixes and derive the JWT-exempt decision from the catalog: a route skips JWT iff its policy is PublicAnonymous or DeviceOrBootstrapAuth (authenticated in-function via cert/bootstrap token, not JWT). Now method-aware and fail-closed on unregistered routes. Behavior- preserving: the old 14 routes + 1 prefix equal exactly the catalog's PublicAnonymous u DeviceOrBootstrapAuth set. Add drift guard in EndpointPolicyCatalogCompletenessTests: AnonymousAndDeviceRoutes_AreJwtExemptInAuthMiddleware loops every such catalog entry and asserts the middleware exempts it (would have caught this bug); SkipsJwtValidation_MatchesPolicyTier locks the positive/negative tiers incl. the diagnostics/download vs diagnostics/download-url contrast. Full suite 2393 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+55
@@ -1,4 +1,5 @@
|
||||
using System.Reflection;
|
||||
using System.Text.RegularExpressions;
|
||||
using AutopilotMonitor.Functions.Middleware;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using Microsoft.Azure.Functions.Worker;
|
||||
@@ -290,6 +291,60 @@ public class EndpointPolicyCatalogCompletenessTests
|
||||
Assert.Equal(TenantScoping.None, entry.TenantScoping);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// REGRESSION GUARD (drift bug 2026-06-22): AuthenticationMiddleware runs BEFORE
|
||||
/// PolicyEnforcementMiddleware, so every catalog route whose policy is PublicAnonymous (fully
|
||||
/// public) or DeviceOrBootstrapAuth (authenticated in-function via cert/bootstrap token, not
|
||||
/// JWT) MUST be JWT-exempt in the middleware — otherwise it is 401'd before its policy can ever
|
||||
/// be honored. That is exactly how the ticket-gated diagnostics/download route broke: it was
|
||||
/// registered PublicAnonymous in the catalog but missing from the middleware's (former) parallel
|
||||
/// allowlist. Deriving the exempt set from the catalog makes the drift structurally impossible;
|
||||
/// this test locks the invariant so a future refactor can't reintroduce a hand-kept list.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void AnonymousAndDeviceRoutes_AreJwtExemptInAuthMiddleware()
|
||||
{
|
||||
var notExempt = new List<string>();
|
||||
|
||||
foreach (var entry in EndpointAccessPolicyCatalog.Entries)
|
||||
{
|
||||
if (entry.Policy is not (EndpointPolicy.PublicAnonymous or EndpointPolicy.DeviceOrBootstrapAuth))
|
||||
continue;
|
||||
|
||||
// Build a concrete request path from the template (sample value for any {param}).
|
||||
var concrete = "/api/" + Regex.Replace(entry.RouteTemplate, @"\{[^}]+}", "sample");
|
||||
if (!AuthenticationMiddleware.SkipsJwtValidation(entry.HttpMethod, concrete))
|
||||
notExempt.Add($"{entry.HttpMethod} {entry.RouteTemplate} [{entry.Policy}]");
|
||||
}
|
||||
|
||||
Assert.True(notExempt.Count == 0,
|
||||
"These PublicAnonymous/DeviceOrBootstrapAuth routes are NOT JWT-exempt in " +
|
||||
"AuthenticationMiddleware — they will be 401'd before their policy is honored:\n" +
|
||||
string.Join("\n", notExempt.Select(r => $" - {r}")));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Conversely, JWT-gated tiers (MemberRead and up) must NOT be exempt — otherwise the middleware
|
||||
/// would wave authenticated traffic through unvalidated. Includes the deliberate contrast pair:
|
||||
/// the ticket-gated diagnostics/download IS exempt (its HMAC ticket is the sole authority), but
|
||||
/// its JWT sibling diagnostics/download-url (MemberRead) stays gated.
|
||||
/// </summary>
|
||||
[Theory]
|
||||
[InlineData("GET", "/api/diagnostics/download", true)] // PublicAnonymous (ticket-gated)
|
||||
[InlineData("GET", "/api/diagnostics/download-url", false)] // MemberRead (JWT)
|
||||
[InlineData("POST", "/api/diagnostics/download-ticket", false)]// MemberRead (JWT) — mints the ticket
|
||||
[InlineData("GET", "/api/health", true)] // PublicAnonymous
|
||||
[InlineData("POST", "/api/agent/ingest", true)] // DeviceOrBootstrapAuth
|
||||
[InlineData("GET", "/api/bootstrap/validate/ABC123", true)] // PublicAnonymous (param route)
|
||||
[InlineData("GET", "/api/sessions", false)] // MemberRead
|
||||
[InlineData("GET", "/api/config/00000000-0000-0000-0000-000000000001", false)] // TenantAdminOrGlobalReader
|
||||
[InlineData("GET", "/api/global/sessions", false)] // GlobalReadOrAdmin
|
||||
[InlineData("GET", "/api/nonexistent", false)] // unregistered → fail-closed
|
||||
public void SkipsJwtValidation_MatchesPolicyTier(string method, string path, bool expectedExempt)
|
||||
{
|
||||
Assert.Equal(expectedExempt, AuthenticationMiddleware.SkipsJwtValidation(method, path));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Named capture group for {tenantId} correctly extracts the value from request paths.
|
||||
/// </summary>
|
||||
|
||||
@@ -12,6 +12,7 @@ using System.Net;
|
||||
using System.Security.Claims;
|
||||
using AutopilotMonitor.Functions.Extensions;
|
||||
using AutopilotMonitor.Functions.Helpers;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using AutopilotMonitor.Shared.DataAccess;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Middleware;
|
||||
@@ -33,43 +34,6 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware
|
||||
private readonly Dictionary<string, (IConfigurationManager<OpenIdConnectConfiguration> Manager, DateTime LastAccessed)> _configManagerCache;
|
||||
private readonly SemaphoreSlim _cacheLock = new SemaphoreSlim(1, 1);
|
||||
|
||||
/// <summary>
|
||||
/// Routes that do not require JWT authentication (exact match).
|
||||
/// Device-to-cloud routes use their own certificate/hardware validation via SecurityValidator.
|
||||
/// Bootstrap device routes use bootstrap token validation via SecurityValidator.
|
||||
/// </summary>
|
||||
private static readonly string[] _anonymousRoutes =
|
||||
{
|
||||
"/api/health",
|
||||
"/api/stats/platform",
|
||||
// Ticket-gated diagnostics download (MCP/AI clients hold no JWT). Authority is the
|
||||
// HMAC-signed ?t= ticket, validated inside DiagnosticsTicketDownloadFunction; authz
|
||||
// already happened at mint time (diagnostics/download-ticket, MemberRead). Must mirror
|
||||
// the PublicAnonymous entry in EndpointAccessPolicyCatalog — this middleware runs BEFORE
|
||||
// PolicyEnforcementMiddleware, so a missing entry here 401s before the policy is honored.
|
||||
"/api/diagnostics/download",
|
||||
"/api/agent/register-session",
|
||||
"/api/agent/ingest",
|
||||
"/api/agent/telemetry",
|
||||
"/api/agent/config",
|
||||
"/api/agent/upload-url",
|
||||
"/api/agent/error",
|
||||
"/api/agent/distress",
|
||||
"/api/bootstrap/register-session",
|
||||
"/api/bootstrap/ingest",
|
||||
"/api/bootstrap/config",
|
||||
"/api/bootstrap/error",
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
/// Route prefixes that do not require JWT authentication (prefix match).
|
||||
/// Used for routes with path parameters where exact match is impossible.
|
||||
/// </summary>
|
||||
private static readonly string[] _anonymousPrefixes =
|
||||
{
|
||||
"/api/bootstrap/validate/", // /api/bootstrap/validate/{code}
|
||||
};
|
||||
|
||||
public AuthenticationMiddleware(
|
||||
ILogger<AuthenticationMiddleware> logger,
|
||||
IConfiguration configuration,
|
||||
@@ -95,11 +59,13 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware
|
||||
return;
|
||||
}
|
||||
|
||||
// Allow anonymous routes without JWT validation
|
||||
// Allow JWT-exempt routes (public + device/bootstrap-authenticated) through without
|
||||
// JWT validation. The exempt set is DERIVED from EndpointAccessPolicyCatalog — never a
|
||||
// hand-kept parallel list — so a new anonymous/device route can't drift out of sync here.
|
||||
var requestPath = httpContext.Request.Path.Value ?? string.Empty;
|
||||
if (IsAnonymousRoute(requestPath))
|
||||
if (SkipsJwtValidation(httpContext.Request.Method, requestPath))
|
||||
{
|
||||
_logger.LogDebug("[Auth Middleware] Anonymous route: {Path}", requestPath);
|
||||
_logger.LogDebug("[Auth Middleware] JWT-exempt route: {Method} {Path}", httpContext.Request.Method, requestPath);
|
||||
await next(context);
|
||||
return;
|
||||
}
|
||||
@@ -302,18 +268,21 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware
|
||||
await next(context);
|
||||
}
|
||||
|
||||
private static bool IsAnonymousRoute(string path)
|
||||
/// <summary>
|
||||
/// Whether a route is exempt from JWT validation in this middleware. SINGLE SOURCE OF TRUTH is
|
||||
/// <see cref="EndpointAccessPolicyCatalog"/>: a route is JWT-exempt iff its policy is
|
||||
/// <see cref="EndpointPolicy.PublicAnonymous"/> (fully public) or
|
||||
/// <see cref="EndpointPolicy.DeviceOrBootstrapAuth"/> (authenticated later, in-function, via
|
||||
/// device certificate / bootstrap token — not JWT). Deriving from the catalog instead of a
|
||||
/// hand-kept parallel allowlist prevents drift: a new anonymous/device route registered in the
|
||||
/// catalog is automatically exempt here, so it can never be 401'd before
|
||||
/// <c>PolicyEnforcementMiddleware</c> honors its policy (the bug that broke the ticket-gated
|
||||
/// <c>diagnostics/download</c> route). Unregistered routes (FindPolicy == null) are fail-closed:
|
||||
/// JWT required. Method-aware so e.g. an unexpected verb on a public path is not waved through.
|
||||
/// </summary>
|
||||
internal static bool SkipsJwtValidation(string httpMethod, string path)
|
||||
{
|
||||
foreach (var route in _anonymousRoutes)
|
||||
{
|
||||
if (path.Equals(route, StringComparison.OrdinalIgnoreCase))
|
||||
return true;
|
||||
}
|
||||
foreach (var prefix in _anonymousPrefixes)
|
||||
{
|
||||
if (path.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
var entry = EndpointAccessPolicyCatalog.FindPolicy(httpMethod, path);
|
||||
return entry is { Policy: EndpointPolicy.PublicAnonymous or EndpointPolicy.DeviceOrBootstrapAuth };
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user