From 42dc46dbc6837513d9cdb67b2a58cbccbdbc6ef9 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 06:43:05 +0000 Subject: [PATCH 1/4] WIP: OIDC guard (brake marker, per-address and per-client budgets, token-row cap), pruner; tests written, not yet run green Co-Authored-By: Claude Sonnet 5.5 --- .../Configuration/OidcIssuerOptions.cs | 31 ++++ src/Bifrost.Web/Features/Oidc/OidcGuard.cs | 170 ++++++++++++++++++ .../Oidc/OidcServiceCollectionExtensions.cs | 3 + src/Bifrost.Web/Program.cs | 5 + tests/Bifrost.Web.Tests/OidcIssuerTests.cs | 157 +++++++++++++++- .../Bifrost.Web.Tests/Support/OidcTestKit.cs | 31 +++- 6 files changed, 395 insertions(+), 2 deletions(-) create mode 100644 src/Bifrost.Web/Features/Oidc/OidcGuard.cs diff --git a/src/Bifrost.Web/Configuration/OidcIssuerOptions.cs b/src/Bifrost.Web/Configuration/OidcIssuerOptions.cs index 13ba192..860c776 100644 --- a/src/Bifrost.Web/Configuration/OidcIssuerOptions.cs +++ b/src/Bifrost.Web/Configuration/OidcIssuerOptions.cs @@ -16,6 +16,15 @@ public sealed class OidcIssuerOptions public OidcClientsOptions Clients { get; init; } = new(); + public OidcLimitsOptions Limits { get; init; } = new(); + + /// + /// Optional path of the issuer's brake marker. While a file exists there, /connect/* answers 503 and nothing is issued; + /// remove it to release. Checked on every request (one stat), so engaging and releasing need no restart. + /// Unset: the issuer has no marker brake (removing this section and restarting is the other off switch). + /// + public string BrakePath { get; init; } = string.Empty; + /// Null when the section is absent. public static OidcIssuerOptions? FromConfiguration(IConfiguration configuration) { @@ -42,3 +51,25 @@ public sealed class OidcClientOptions /// MAS's back-channel logout URL for this provider. Empty: no logout is sent and no session is ended. public string BackchannelLogoutUri { get; init; } = string.Empty; } + +/// +/// Budgets on /connect/authorize, /connect/token and /connect/userinfo, and the bound on what the anonymous authorize +/// endpoint can park. Per source address and per client id, one fixed window; a refused caller gets 429 with Retry-After. +/// The defaults suit one client (MAS) behind one proxy. +/// +public sealed class OidcLimitsOptions +{ + public int PerAddressPermits { get; init; } = 30; + + public int PerClientPermits { get; init; } = 300; + + public TimeSpan Window { get; init; } = TimeSpan.FromMinutes(1); + + public int MaxTrackedKeys { get; init; } = 10_000; + + /// The most OpenIddict token rows (parked authorize requests, codes, access tokens) allowed to exist; a new authorize request beyond it is refused until the prune frees room. + public int MaxTokenRows { get; init; } = 20_000; + + /// How often expired and orphaned authorizations and tokens are deleted. + public TimeSpan PruneInterval { get; init; } = TimeSpan.FromMinutes(5); +} diff --git a/src/Bifrost.Web/Features/Oidc/OidcGuard.cs b/src/Bifrost.Web/Features/Oidc/OidcGuard.cs new file mode 100644 index 0000000..f07ac71 --- /dev/null +++ b/src/Bifrost.Web/Features/Oidc/OidcGuard.cs @@ -0,0 +1,170 @@ +using System.Net; +using System.Text; +using Bifrost.Web.Configuration; +using Bifrost.Web.Features.Shared; +using Microsoft.Extensions.Options; +using OpenIddict.Abstractions; +using OpenIddict.Server.AspNetCore; + +namespace Bifrost.Web.Features.Oidc; + +/// +/// The issuer's front door, ahead of OpenIddict: the marker brake, the per-address and per-client budgets, and the bound +/// on token rows. The address is the connection's, already corrected by the forwarded-headers handling that trusts only +/// the local proxy, so a caller cannot pick its own. It fails closed: anything this middleware cannot decide is a 503, never a pass. +/// +public sealed class OidcGuardMiddleware(RequestDelegate next, OidcIssuerOptions options, OidcRateLimit limit, ILogger logger) +{ + private const int MaxClientIdLength = 128; + + public async Task InvokeAsync(HttpContext http, IOpenIddictTokenManager tokens) + { + var endpoint = EndpointOf(http.Request.Path); + if (endpoint is null) + { + await next(http); + return; + } + + try + { + if (BrakeEngaged(options.BrakePath)) + { + await RefuseAsync(http, StatusCodes.Status503ServiceUnavailable, "temporarily_unavailable", TimeSpan.FromMinutes(1)); + return; + } + + if (limit.Spend($"address:{AddressKey(http.Connection.RemoteIpAddress)}") is { } byAddress) + { + await RefuseAsync(http, StatusCodes.Status429TooManyRequests, "too_many_requests", byAddress.RetryAfter); + return; + } + + if (limit.Spend($"client:{endpoint}:{await ClientKeyAsync(http, endpoint)}") is { } byClient) + { + await RefuseAsync(http, StatusCodes.Status429TooManyRequests, "too_many_requests", byClient.RetryAfter); + return; + } + + // A new authorize request parks a row for anyone who can name the (public) client; a resumed one carries request_uri and parks none. + if (endpoint == "authorize" && !http.Request.Query.ContainsKey("request_uri") && await tokens.CountAsync(http.RequestAborted) >= options.Limits.MaxTokenRows) + { + await RefuseAsync(http, StatusCodes.Status503ServiceUnavailable, "temporarily_unavailable", options.Limits.PruneInterval); + return; + } + } + catch (Exception exception) when (exception is not OperationCanceledException) + { + logger.LogError("OIDC guard could not decide ({ExceptionType}); the request is refused.", exception.GetType().Name); + await RefuseAsync(http, StatusCodes.Status503ServiceUnavailable, "temporarily_unavailable", TimeSpan.FromMinutes(1)); + return; + } + + await next(http); + } + + private static string? EndpointOf(PathString path) => + path.Equals(OidcServiceCollectionExtensions.AuthorizationPath, StringComparison.OrdinalIgnoreCase) ? "authorize" + : path.Equals(OidcServiceCollectionExtensions.TokenPath, StringComparison.OrdinalIgnoreCase) ? "token" + : path.Equals(OidcServiceCollectionExtensions.UserInfoPath, StringComparison.OrdinalIgnoreCase) ? "userinfo" + : null; + + private static bool BrakeEngaged(string path) => !string.IsNullOrWhiteSpace(path) && File.Exists(path); + + // An IPv6 caller owns its /64, so rotating the low bits buys nothing. + private static string AddressKey(IPAddress? address) + { + if (address is null) + { + return "unknown"; + } + + if (address.IsIPv4MappedToIPv6) + { + address = address.MapToIPv4(); + } + + if (address.AddressFamily != System.Net.Sockets.AddressFamily.InterNetworkV6) + { + return address.ToString(); + } + + var bytes = address.GetAddressBytes(); + Array.Clear(bytes, 8, 8); + return new IPAddress(bytes).ToString() + "/64"; + } + + // The client the caller names, not one it proved: authorize names it in the query or form, token in its Basic header. + // An unreadable value shares one key, so garbage cannot mint keys. + private static async Task ClientKeyAsync(HttpContext http, string endpoint) + { + string? clientId = null; + if (endpoint == "authorize") + { + clientId = http.Request.Query["client_id"].FirstOrDefault() + ?? (http.Request.HasFormContentType ? (await http.Request.ReadFormAsync(http.RequestAborted))["client_id"].FirstOrDefault() : null); + } + else if (endpoint == "token" && http.Request.Headers.Authorization.FirstOrDefault() is { } header + && header.StartsWith("Basic ", StringComparison.OrdinalIgnoreCase)) + { + try + { + var decoded = Encoding.UTF8.GetString(Convert.FromBase64String(header["Basic ".Length..].Trim())); + clientId = Uri.UnescapeDataString(decoded.Split(':', 2)[0]); + } + catch (FormatException) + { + } + } + + return string.IsNullOrEmpty(clientId) || clientId.Length > MaxClientIdLength ? "unnamed" : clientId; + } + + private static Task RefuseAsync(HttpContext http, int status, string error, TimeSpan retryAfter) + { + http.Response.StatusCode = status; + http.Response.Headers.RetryAfter = Math.Max(1, (int)Math.Ceiling(retryAfter.TotalSeconds)).ToString(System.Globalization.CultureInfo.InvariantCulture); + http.Response.ContentType = "application/json"; + return http.Response.WriteAsync($"{{\"error\":\"{error}\"}}", http.RequestAborted); + } +} + +/// The issuer's per-address and per-client permits, on the shared fixed-window budget. +public sealed class OidcRateLimit(OidcIssuerOptions options, TimeProvider clock) +{ + private readonly FixedWindowBudget addresses = new(options.Limits.PerAddressPermits, options.Limits.Window, options.Limits.MaxTrackedKeys, clock); + private readonly FixedWindowBudget clients = new(options.Limits.PerClientPermits, options.Limits.Window, options.Limits.MaxTrackedKeys, clock); + + public BudgetDenial? Spend(string key) => key.StartsWith("address:", StringComparison.Ordinal) ? addresses.Spend(key) : clients.Spend(key); +} + +/// +/// Deletes expired and orphaned authorizations and tokens on a schedule (OpenIddict's own prune: tokens that are no longer +/// valid or have expired, and authorizations that are ad hoc or invalid with no live token). Live rows are never touched. +/// +public sealed class OidcPruner(IServiceScopeFactory scopes, OidcIssuerOptions options, TimeProvider clock, ILogger logger) : BackgroundService +{ + public async Task PruneAsync(CancellationToken cancellationToken) + { + await using var scope = scopes.CreateAsyncScope(); + var threshold = clock.GetUtcNow(); + await scope.ServiceProvider.GetRequiredService().PruneAsync(threshold, cancellationToken); + await scope.ServiceProvider.GetRequiredService().PruneAsync(threshold, cancellationToken); + } + + protected override async Task ExecuteAsync(CancellationToken stoppingToken) + { + using var timer = new PeriodicTimer(options.Limits.PruneInterval, clock); + while (await timer.WaitForNextTickAsync(stoppingToken)) + { + try + { + await PruneAsync(stoppingToken); + } + catch (Exception exception) when (exception is not OperationCanceledException) + { + logger.LogError("OIDC prune failed ({ExceptionType}); the next tick tries again.", exception.GetType().Name); + } + } + } +} diff --git a/src/Bifrost.Web/Features/Oidc/OidcServiceCollectionExtensions.cs b/src/Bifrost.Web/Features/Oidc/OidcServiceCollectionExtensions.cs index 5ad30b8..78526af 100644 --- a/src/Bifrost.Web/Features/Oidc/OidcServiceCollectionExtensions.cs +++ b/src/Bifrost.Web/Features/Oidc/OidcServiceCollectionExtensions.cs @@ -67,6 +67,9 @@ public static class OidcServiceCollectionExtensions server.Issuer = issuer; }); + services.AddSingleton(); + services.AddSingleton(); + services.AddHostedService(provider => provider.GetRequiredService()); services.AddHostedService(); if (!string.IsNullOrWhiteSpace(options.Clients.Mas.BackchannelLogoutUri)) { diff --git a/src/Bifrost.Web/Program.cs b/src/Bifrost.Web/Program.cs index 87d8088..2a5ff5f 100644 --- a/src/Bifrost.Web/Program.cs +++ b/src/Bifrost.Web/Program.cs @@ -285,6 +285,11 @@ if (!app.Environment.IsDevelopment()) app.UseHttpLogging(); app.UseHttpsRedirection(); +if (oidcOptions is not null) +{ + app.UseMiddleware(); // before authentication: OpenIddict answers /connect/* inside it +} + app.UseStaticFiles(); app.UseRouting(); app.UseConceptTallyCors(); diff --git a/tests/Bifrost.Web.Tests/OidcIssuerTests.cs b/tests/Bifrost.Web.Tests/OidcIssuerTests.cs index 30098f5..e9ead63 100644 --- a/tests/Bifrost.Web.Tests/OidcIssuerTests.cs +++ b/tests/Bifrost.Web.Tests/OidcIssuerTests.cs @@ -19,7 +19,7 @@ namespace Bifrost.Web.Tests; public sealed class OidcIssuerTests : IClassFixture { private static readonly Uri Origin = new("https://localhost"); - private readonly OidcWebApplicationFactory _factory; + private OidcWebApplicationFactory _factory; public OidcIssuerTests(OidcWebApplicationFactory factory) => _factory = factory; @@ -178,6 +178,161 @@ public sealed class OidcIssuerTests : IClassFixture Assert.False(redirect.ContainsKey("error")); } + private static IReadOnlyDictionary Settings(params (string Key, string Value)[] pairs) => + pairs.ToDictionary(p => $"Bifrost:Oidc:{p.Key}", p => (string?)p.Value); + + [Fact] + public async Task A_flood_from_one_address_is_refused_with_429_and_a_retry_after() + { + using var factory = new OidcSettingsFactory(Settings(("Limits:PerAddressPermits", "5"))); + using var client = factory.CreateClient(ClientOptions()); + + var statuses = new List(); + HttpResponseMessage? last = null; + for (var i = 0; i < 8; i++) + { + last?.Dispose(); + last = await client.GetAsync(NewRequest().Path); + statuses.Add(last.StatusCode); + } + + Assert.DoesNotContain(HttpStatusCode.TooManyRequests, statuses.Take(5)); + Assert.All(statuses.Skip(5), status => Assert.Equal(HttpStatusCode.TooManyRequests, status)); + Assert.True(int.Parse(last!.Headers.GetValues("Retry-After").Single()) >= 1); + using var tokenFlood = await client.PostAsync("/connect/token", new FormUrlEncodedContent(new Dictionary())); + Assert.Equal(HttpStatusCode.TooManyRequests, tokenFlood.StatusCode); // one budget over all three endpoints + using var userinfoFlood = await client.GetAsync("/connect/userinfo"); + Assert.Equal(HttpStatusCode.TooManyRequests, userinfoFlood.StatusCode); + last.Dispose(); + } + + [Fact] + public async Task A_flood_naming_one_client_is_refused_for_that_client_only() + { + using var factory = new OidcSettingsFactory(Settings(("Limits:PerClientPermits", "3"))); + using var client = factory.CreateClient(ClientOptions()); + + var flooded = new List(); + for (var i = 0; i < 5; i++) + { + using var response = await client.GetAsync(NewRequest("noisy-client").Path); + flooded.Add(response.StatusCode); + } + + using var other = await client.GetAsync(NewRequest().Path); + Assert.Equal([HttpStatusCode.TooManyRequests, HttpStatusCode.TooManyRequests], flooded.Skip(3)); + Assert.NotEqual(HttpStatusCode.TooManyRequests, other.StatusCode); + Assert.NotEqual(HttpStatusCode.TooManyRequests, flooded[0]); + } + + [Fact] + public async Task A_mas_flow_fits_the_default_budgets_and_the_window_reopens() + { + using var factory = new OidcSettingsFactory(Settings(("Limits:PerAddressPermits", "30"), ("Limits:PerClientPermits", "300"))); + _factory = factory; + var (client, identity) = await SignedInAsync(MembershipStatus.Active, "Within Budget"); + + var verified = await ValidateIdTokenAsync((await CompleteFlowAsync(client)).IdToken, null); + Assert.Equal(identity, verified.Claim("preferred_username")); + + for (var i = 0; i < 40; i++) + { + using var burst = await client.GetAsync("/connect/userinfo"); + } + + using var refused = await client.GetAsync("/connect/userinfo"); + Assert.Equal(HttpStatusCode.TooManyRequests, refused.StatusCode); + factory.Clock.Advance(TimeSpan.FromMinutes(1)); + using var reopened = await client.GetAsync("/connect/userinfo"); + Assert.NotEqual(HttpStatusCode.TooManyRequests, reopened.StatusCode); + } + + [Fact] + public async Task The_brake_marker_stops_the_issuer_and_releasing_it_needs_no_restart() + { + var marker = Path.Combine(Path.GetTempPath(), $"bifrost-oidc-brake-{Guid.NewGuid():N}"); + using var factory = new OidcSettingsFactory(Settings(("BrakePath", marker))); + _factory = factory; + var (client, _) = await SignedInAsync(MembershipStatus.Active, "Braked"); + try + { + Assert.True((await CompleteFlowAsync(client)).IdToken.Length > 0, "released: the flow passes"); + + await File.WriteAllTextAsync(marker, "held"); + foreach (var path in new[] { NewRequest().Path, "/connect/userinfo" }) + { + using var held = await client.GetAsync(path); + Assert.Equal(HttpStatusCode.ServiceUnavailable, held.StatusCode); + Assert.True(held.Headers.Contains("Retry-After")); + } + + using var token = await client.PostAsync("/connect/token", new FormUrlEncodedContent(new Dictionary())); + Assert.Equal(HttpStatusCode.ServiceUnavailable, token.StatusCode); + using var discovery = await client.GetAsync("/.well-known/openid-configuration"); + Assert.Equal(HttpStatusCode.OK, discovery.StatusCode); // the brake stops issuing, not the published keys + + File.Delete(marker); + Assert.True((await CompleteFlowAsync(client)).IdToken.Length > 0, "released again, same process"); + } + finally + { + File.Delete(marker); + } + } + + [Fact] + public async Task Anonymous_authorize_requests_cannot_park_more_than_the_row_cap_and_the_prune_gives_the_room_back() + { + using var factory = new OidcSettingsFactory(Settings(("Limits:MaxTokenRows", "3"))); + _factory = factory; + using var client = factory.CreateClient(ClientOptions()); + + var parked = new List(); + for (var i = 0; i < 5; i++) + { + using var response = await client.GetAsync(NewRequest().Path); + parked.Add(response.StatusCode); + } + + Assert.Equal(3L, await factory.TokenRowsAsync()); + Assert.All(parked.Skip(3), status => Assert.Equal(HttpStatusCode.ServiceUnavailable, status)); + Assert.All(parked.Take(3), status => Assert.NotEqual(HttpStatusCode.ServiceUnavailable, status)); + + factory.Clock.Advance(TimeSpan.FromMinutes(11)); // past the ten-minute parked lifetime + await factory.PruneAsync(); + Assert.Equal(0L, await factory.TokenRowsAsync()); + using var again = await client.GetAsync(NewRequest().Path); + Assert.NotEqual(HttpStatusCode.ServiceUnavailable, again.StatusCode); + } + + [Fact] + public async Task The_prune_removes_expired_rows_and_leaves_a_live_token_alone() + { + using var factory = new OidcSettingsFactory(Settings()); + _factory = factory; + var (client, _) = await SignedInAsync(MembershipStatus.Active, "Pruned"); + var tokens = await CompleteFlowAsync(client); // a parked request, a code and an access token existed along the way + using (var anonymous = factory.CreateClient(ClientOptions())) + { + using var parked = await anonymous.GetAsync(NewRequest().Path); // a parked request that will expire + } + + var before = await factory.TokenRowsAsync(); + await factory.PruneAsync(); + Assert.Equal(before, await factory.TokenRowsAsync()); // nothing has expired yet: nothing goes + + factory.Clock.Advance(TimeSpan.FromMinutes(11)); // parked requests and codes are dead, the access token (an hour) is not + await factory.PruneAsync(); + var after = await factory.TokenRowsAsync(); + Assert.True(after < before, "expired rows went"); + Assert.True(after >= 1, "the live access token stayed"); + + using var userinfo = new HttpRequestMessage(HttpMethod.Get, "/connect/userinfo"); + userinfo.Headers.Authorization = new AuthenticationHeaderValue("Bearer", tokens.AccessToken); + using var response = await factory.CreateClient(ClientOptions()).SendAsync(userinfo); + Assert.NotEqual(HttpStatusCode.Unauthorized, response.StatusCode); + } + [Theory] [InlineData("unknown-client", OidcTestKit.RedirectUri, true)] [InlineData(OidcTestKit.ClientId, "https://evil.example.test/callback", true)] diff --git a/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs b/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs index 0b14d18..902dbda 100644 --- a/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs +++ b/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs @@ -92,7 +92,10 @@ public sealed class OidcTestKit : IDisposable ["Bifrost:Oidc:EncryptionKeyPath"] = EncryptionKeyPath, ["Bifrost:Oidc:Clients:Mas:ClientId"] = ClientId, ["Bifrost:Oidc:Clients:Mas:ClientSecretPath"] = ClientSecretPath, - ["Bifrost:Oidc:Clients:Mas:RedirectUri"] = RedirectUri + ["Bifrost:Oidc:Clients:Mas:RedirectUri"] = RedirectUri, + // The shared test client has no remote address, so every request is one caller: the budgets are out of the way unless a test sets them. + ["Bifrost:Oidc:Limits:PerAddressPermits"] = "100000", + ["Bifrost:Oidc:Limits:PerClientPermits"] = "100000" }; public void Dispose() @@ -114,6 +117,11 @@ public sealed class OidcTestClock : TimeProvider public override DateTimeOffset GetUtcNow() => _now; + // The monotonic reading follows the moved time, so a budget's window can be aged the same way as a parked request. + public override long TimestampFrequency => TimeSpan.TicksPerSecond; + + public override long GetTimestamp() => _now.UtcTicks; + public void Advance(TimeSpan by) => _now += by; } @@ -258,6 +266,27 @@ public sealed class OidcFixedIdentityFactory(string identity) : OidcWebApplicati } } +/// An OIDC host with extra settings (limits, a brake path) applied after the kit's own. +public sealed class OidcSettingsFactory(IReadOnlyDictionary settings) : OidcWebApplicationFactory +{ + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + base.ConfigureWebHost(builder); + foreach (var (key, value) in settings) + { + builder.UseSetting(key, value); + } + } + + public Task PruneAsync() => Services.GetRequiredService().PruneAsync(CancellationToken.None); + + public async Task TokenRowsAsync() + { + using var scope = Services.CreateScope(); + return await scope.ServiceProvider.GetRequiredService().CountAsync(); + } +} + /// An OIDC host whose public base URL differs from the origin its requests arrive on, so the issuer cannot be request-derived. public sealed class OidcForeignIssuerFactory : OidcWebApplicationFactory { -- 2.47.3 From d0749d4022a31fcb197f37780555b0787d1ff28e Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 07:56:09 +0000 Subject: [PATCH 2/4] OIDC issuer: per-address and per-client limits, token-row cap, scheduled prune, marker brake Co-Authored-By: Claude Sonnet 5.5 --- docs/oidc-issuer.md | 23 +++++++--- docs/oidc-reserved-accounts.sql | 12 +++++ src/Bifrost.Web/Features/Oidc/OidcGuard.cs | 45 ++++++++++++++++--- tests/Bifrost.Web.Tests/OidcIssuerTests.cs | 4 +- .../Bifrost.Web.Tests/Support/OidcTestKit.cs | 4 +- 5 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 docs/oidc-reserved-accounts.sql diff --git a/docs/oidc-issuer.md b/docs/oidc-issuer.md index beaa6df..8d4e301 100644 --- a/docs/oidc-issuer.md +++ b/docs/oidc-issuer.md @@ -44,6 +44,8 @@ echoes its value. | `Bifrost:Oidc:Clients:Mas:ClientId` | The client id MAS presents. | | `Bifrost:Oidc:Clients:Mas:ClientSecretPath` | File holding the client secret. | | `Bifrost:Oidc:Clients:Mas:RedirectUri` | MAS's callback URL, registered exactly. | +| `Bifrost:Oidc:BrakePath` | Optional. While a file exists at this path, `/connect/authorize`, `/token` and `/userinfo` answer 503 `temporarily_unavailable` and nothing is issued; delete the file to release. Checked per request, so no restart. Unset: no marker brake. | +| `Bifrost:Oidc:Limits:*` | `PerAddressPermits` (30), `PerClientPermits` (300), `Window` (1 min), `MaxTrackedKeys` (10000), `MaxTokenRows` (20000), `PruneInterval` (5 min). | | `Bifrost:Oidc:Clients:Mas:BackchannelLogoutUri` | Optional. MAS's back-channel logout URL, `https:///upstream/backchannel-logout/`; absolute http(s) or startup fails. Unset: no logout is sent and no authorization is revoked. | There are no development or ephemeral keys in the host. Key files are provisioned like the other @@ -111,17 +113,28 @@ Configure MAS's upstream provider with the issuer above, the client id and secre from `preferred_username`. MAS mints the Matrix localpart on first sign-in, so the claim must never change for an account. +## Limits, prune, brake + +`OidcGuardMiddleware` sits ahead of OpenIddict and guards the three endpoints. In order: the marker brake (503); a per-address +budget (429 with `Retry-After`; the address is the connection's after forwarded-headers handling that trusts only the local +proxy, IPv6 keyed by /64); a per-client budget keyed by the client id the caller names (authorize query or form, token Basic +header; unreadable or over-long names share one key); and a cap on token rows: a new authorize request (no `request_uri`) +is refused with 503 while the table holds `MaxTokenRows`. Any failure inside the guard is a 503, never a pass. MAS's flow is +a handful of requests per sign-in from one address, far under the defaults. + +`OidcPruner` runs every `PruneInterval` and deletes tokens past expiry (parked authorize requests, codes, access tokens) or no +longer valid, then authorizations no longer valid or ad hoc with no token left (younger than five minutes are left alone). +Live rows are untouched. Plain EF queries rather than OpenIddict's own prune, which uses ExecuteDelete and cannot run on the +in-memory provider the tests use. + +`docs/oidc-reserved-accounts.sql` is the read-only query for existing accounts that hold one of the 28 reserved names. + ## Not built Checked against the Window B list (2026-11-11). Present and tested: discovery, JWKS, authorize/token/userinfo, PKCE S256, the one client, claims to a Matrix-legal localpart (`[a-z0-9._-]`, 3-80, lower case), reserved names at creation and now at issuance, back-channel logout. Not present: -- **Rate limits and pruning.** No limiter sits on `/connect/*`, and expired OpenIddict rows (parked authorize requests, - tokens, authorizations) are never deleted. The authorize endpoint is anonymous and its client id and redirect URI are - public, so a caller can park rows without bound. Needs a budget (`FixedWindowBudget`) and a prune. -- **A brake.** The Bifrost process has none; the deploy brake belongs to Idunn. The only off switch is removing - `Bifrost:Oidc` and restarting, or membership leaving `Active` (the reconciler then logs the person out). - **Non-member sign-in.** The gate is `Active` membership, so an account with no membership gets `access_denied`. A "Users" tier for chat registration, with no vote, is a policy choice not yet made; nothing here writes membership or votes. - **MAS upstream block.** `gamecult-ops` `chat/matrix-auth/config.yaml` has no `upstream_oauth2` (provider ULID, diff --git a/docs/oidc-reserved-accounts.sql b/docs/oidc-reserved-accounts.sql new file mode 100644 index 0000000..8845053 --- /dev/null +++ b/docs/oidc-reserved-accounts.sql @@ -0,0 +1,12 @@ +-- Read-only. Which existing UserAccounts hold one of the 28 reserved localparts +-- (BifrostIdentityService.ReservedIdentities). Written for review; not run on live. +-- Run it in a read-only transaction by whoever holds the database: +-- psql -v ON_ERROR_STOP=1 -c 'BEGIN READ ONLY' -f docs/oidc-reserved-accounts.sql +-- Keep the list in step with ReservedIdentities; the issuer refuses these at authorize (#49). +SELECT a."Id", a."BifrostIdentity", a."NormalizedBifrostIdentity", a."CreatedAtUtc", a."LastSeenAtUtc" +FROM "UserAccounts" a +WHERE lower(a."NormalizedBifrostIdentity") IN ( + 'admin', 'aqua', 'deru', 'eureka', 'heimdall', 'ivixey', 'kiko', 'libby', 'meta', 'nibu', 'unnie', 'void', 'weksa', + 'root', 'system', 'matrix', 'synapse', 'support', 'bot', 'mod', 'moderator', 'postmaster', 'gamecult', + 'bifrost', 'idunn', 'odin', 'huginn', 'ratatoskr') +ORDER BY a."NormalizedBifrostIdentity"; diff --git a/src/Bifrost.Web/Features/Oidc/OidcGuard.cs b/src/Bifrost.Web/Features/Oidc/OidcGuard.cs index f07ac71..7349693 100644 --- a/src/Bifrost.Web/Features/Oidc/OidcGuard.cs +++ b/src/Bifrost.Web/Features/Oidc/OidcGuard.cs @@ -1,9 +1,13 @@ using System.Net; using System.Text; using Bifrost.Web.Configuration; +using Bifrost.Web.Data; +using Microsoft.EntityFrameworkCore; +using OpenIddict.EntityFrameworkCore.Models; using Bifrost.Web.Features.Shared; using Microsoft.Extensions.Options; using OpenIddict.Abstractions; +using static OpenIddict.Abstractions.OpenIddictConstants; using OpenIddict.Server.AspNetCore; namespace Bifrost.Web.Features.Oidc; @@ -139,17 +143,48 @@ public sealed class OidcRateLimit(OidcIssuerOptions options, TimeProvider clock) } /// -/// Deletes expired and orphaned authorizations and tokens on a schedule (OpenIddict's own prune: tokens that are no longer -/// valid or have expired, and authorizations that are ad hoc or invalid with no live token). Live rows are never touched. +/// Deletes expired and orphaned authorizations and tokens on a schedule: tokens past their expiry (parked authorize +/// requests, codes, access tokens) or no longer valid, then authorizations that are no longer valid or are ad hoc with no +/// token left. Live rows are never touched. Plain queries, not OpenIddict's ExecuteDelete prune, so the same code runs on +/// Postgres and on the in-memory test provider. /// public sealed class OidcPruner(IServiceScopeFactory scopes, OidcIssuerOptions options, TimeProvider clock, ILogger logger) : BackgroundService { + private const int Batch = 500; + + // A fresh authorization briefly has no token yet; leave anything younger than this alone. + private static readonly TimeSpan AuthorizationGrace = TimeSpan.FromMinutes(5); + public async Task PruneAsync(CancellationToken cancellationToken) { await using var scope = scopes.CreateAsyncScope(); - var threshold = clock.GetUtcNow(); - await scope.ServiceProvider.GetRequiredService().PruneAsync(threshold, cancellationToken); - await scope.ServiceProvider.GetRequiredService().PruneAsync(threshold, cancellationToken); + var db = scope.ServiceProvider.GetRequiredService(); + var now = clock.GetUtcNow(); + var young = now - AuthorizationGrace; + + int removed; + do + { + var tokens = await db.Set() + .Where(token => token.ExpirationDate < now || (token.Status != Statuses.Valid && token.CreationDate < young)) + .Take(Batch).ToListAsync(cancellationToken); + db.RemoveRange(tokens); + await db.SaveChangesAsync(cancellationToken); + removed = tokens.Count; + } + while (removed == Batch); + + do + { + var authorizations = await db.Set() + .Where(authorization => authorization.CreationDate < young + && (authorization.Status != Statuses.Valid || (authorization.Type == AuthorizationTypes.AdHoc && !authorization.Tokens.Any()))) + .Take(Batch).ToListAsync(cancellationToken); + db.RemoveRange(authorizations); + await db.SaveChangesAsync(cancellationToken); + removed = authorizations.Count; + } + while (removed == Batch); } protected override async Task ExecuteAsync(CancellationToken stoppingToken) diff --git a/tests/Bifrost.Web.Tests/OidcIssuerTests.cs b/tests/Bifrost.Web.Tests/OidcIssuerTests.cs index e9ead63..b2346e6 100644 --- a/tests/Bifrost.Web.Tests/OidcIssuerTests.cs +++ b/tests/Bifrost.Web.Tests/OidcIssuerTests.cs @@ -294,13 +294,13 @@ public sealed class OidcIssuerTests : IClassFixture parked.Add(response.StatusCode); } - Assert.Equal(3L, await factory.TokenRowsAsync()); + Assert.Equal(3, await factory.TokenRowsAsync()); Assert.All(parked.Skip(3), status => Assert.Equal(HttpStatusCode.ServiceUnavailable, status)); Assert.All(parked.Take(3), status => Assert.NotEqual(HttpStatusCode.ServiceUnavailable, status)); factory.Clock.Advance(TimeSpan.FromMinutes(11)); // past the ten-minute parked lifetime await factory.PruneAsync(); - Assert.Equal(0L, await factory.TokenRowsAsync()); + Assert.Equal(0, await factory.TokenRowsAsync()); using var again = await client.GetAsync(NewRequest().Path); Assert.NotEqual(HttpStatusCode.ServiceUnavailable, again.StatusCode); } diff --git a/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs b/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs index 902dbda..f413c08 100644 --- a/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs +++ b/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs @@ -280,10 +280,10 @@ public sealed class OidcSettingsFactory(IReadOnlyDictionary set public Task PruneAsync() => Services.GetRequiredService().PruneAsync(CancellationToken.None); - public async Task TokenRowsAsync() + public async Task TokenRowsAsync() { using var scope = Services.CreateScope(); - return await scope.ServiceProvider.GetRequiredService().CountAsync(); + return (int)await scope.ServiceProvider.GetRequiredService().CountAsync(); } } -- 2.47.3 From fa920725083d4c04aaa21515ef5521e246d03d6d Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 08:08:45 +0000 Subject: [PATCH 3/4] OIDC guard: match endpoints by segment, drop the per-client budget, brake fails closed, prune takes an authorization's tokens; tests pin each Co-Authored-By: Claude Sonnet 5.5 --- docs/oidc-issuer.md | 21 ++-- .../Configuration/OidcIssuerOptions.cs | 7 +- src/Bifrost.Web/Features/Oidc/OidcGuard.cs | 84 ++++++------- tests/Bifrost.Web.Tests/OidcIssuerTests.cs | 114 ++++++++++++++++-- .../Bifrost.Web.Tests/Support/OidcTestKit.cs | 38 ++++++ 5 files changed, 193 insertions(+), 71 deletions(-) diff --git a/docs/oidc-issuer.md b/docs/oidc-issuer.md index 8d4e301..cb2ee08 100644 --- a/docs/oidc-issuer.md +++ b/docs/oidc-issuer.md @@ -45,7 +45,7 @@ echoes its value. | `Bifrost:Oidc:Clients:Mas:ClientSecretPath` | File holding the client secret. | | `Bifrost:Oidc:Clients:Mas:RedirectUri` | MAS's callback URL, registered exactly. | | `Bifrost:Oidc:BrakePath` | Optional. While a file exists at this path, `/connect/authorize`, `/token` and `/userinfo` answer 503 `temporarily_unavailable` and nothing is issued; delete the file to release. Checked per request, so no restart. Unset: no marker brake. | -| `Bifrost:Oidc:Limits:*` | `PerAddressPermits` (30), `PerClientPermits` (300), `Window` (1 min), `MaxTrackedKeys` (10000), `MaxTokenRows` (20000), `PruneInterval` (5 min). | +| `Bifrost:Oidc:Limits:*` | `PerAddressPermits` (30), `Window` (1 min), `MaxTrackedKeys` (10000), `MaxTokenRows` (20000), `PruneInterval` (5 min). | | `Bifrost:Oidc:Clients:Mas:BackchannelLogoutUri` | Optional. MAS's back-channel logout URL, `https:///upstream/backchannel-logout/`; absolute http(s) or startup fails. Unset: no logout is sent and no authorization is revoked. | There are no development or ephemeral keys in the host. Key files are provisioned like the other @@ -115,12 +115,19 @@ account. ## Limits, prune, brake -`OidcGuardMiddleware` sits ahead of OpenIddict and guards the three endpoints. In order: the marker brake (503); a per-address -budget (429 with `Retry-After`; the address is the connection's after forwarded-headers handling that trusts only the local -proxy, IPv6 keyed by /64); a per-client budget keyed by the client id the caller names (authorize query or form, token Basic -header; unreadable or over-long names share one key); and a cap on token rows: a new authorize request (no `request_uri`) -is refused with 503 while the table holds `MaxTokenRows`. Any failure inside the guard is a 503, never a pass. MAS's flow is -a handful of requests per sign-in from one address, far under the defaults. +`OidcGuardMiddleware` sits ahead of OpenIddict and guards the three endpoints, matched by path segment so a trailing slash +is no way around it. In order: the marker brake (503; a marker that cannot be read is treated as engaged); one per-address +budget over the three endpoints together (429 with `Retry-After`; the address is the connection's after forwarded-headers +handling that trusts only the local proxy, IPv6 keyed by /64); and a cap on token rows: a new authorize request (no +`request_uri`) is refused with 503 while the table holds `MaxTokenRows`. Any failure inside the guard is a 503, never a pass. +MAS's flow is a handful of requests per sign-in from one address, far under the defaults (30 a minute is about 15 sign-ins). + +There is no per-client budget. The one client's id is public and what a caller names is unverified, so a budget keyed on it +would let anyone spend the real client's permits and lock sign-in out. The bound on the anonymous endpoint is the address +budget plus the row cap, and the cap is itself a switch an attacker with many addresses can flip: it refuses new sign-ins +(never resumed ones) until the next prune frees rows, which is the bounded failure chosen over unbounded parked rows. +Not parking state before the user authenticates (as pushed authorization requests avoid by authenticating the client first) +would remove the switch, and OpenIddict's request caching does not offer that. `OidcPruner` runs every `PruneInterval` and deletes tokens past expiry (parked authorize requests, codes, access tokens) or no longer valid, then authorizations no longer valid or ad hoc with no token left (younger than five minutes are left alone). diff --git a/src/Bifrost.Web/Configuration/OidcIssuerOptions.cs b/src/Bifrost.Web/Configuration/OidcIssuerOptions.cs index 860c776..017648e 100644 --- a/src/Bifrost.Web/Configuration/OidcIssuerOptions.cs +++ b/src/Bifrost.Web/Configuration/OidcIssuerOptions.cs @@ -53,16 +53,13 @@ public sealed class OidcClientOptions } /// -/// Budgets on /connect/authorize, /connect/token and /connect/userinfo, and the bound on what the anonymous authorize -/// endpoint can park. Per source address and per client id, one fixed window; a refused caller gets 429 with Retry-After. -/// The defaults suit one client (MAS) behind one proxy. +/// One budget on /connect/authorize, /connect/token and /connect/userinfo together, per source address in one fixed window (a refused caller gets 429 with Retry-After), +/// and the bound on what the anonymous authorize endpoint can park. No per-client budget: the client id is public. The defaults suit one client (MAS) behind one proxy. /// public sealed class OidcLimitsOptions { public int PerAddressPermits { get; init; } = 30; - public int PerClientPermits { get; init; } = 300; - public TimeSpan Window { get; init; } = TimeSpan.FromMinutes(1); public int MaxTrackedKeys { get; init; } = 10_000; diff --git a/src/Bifrost.Web/Features/Oidc/OidcGuard.cs b/src/Bifrost.Web/Features/Oidc/OidcGuard.cs index 7349693..ccfd5e1 100644 --- a/src/Bifrost.Web/Features/Oidc/OidcGuard.cs +++ b/src/Bifrost.Web/Features/Oidc/OidcGuard.cs @@ -1,5 +1,4 @@ using System.Net; -using System.Text; using Bifrost.Web.Configuration; using Bifrost.Web.Data; using Microsoft.EntityFrameworkCore; @@ -13,14 +12,12 @@ using OpenIddict.Server.AspNetCore; namespace Bifrost.Web.Features.Oidc; /// -/// The issuer's front door, ahead of OpenIddict: the marker brake, the per-address and per-client budgets, and the bound +/// The issuer's front door, ahead of OpenIddict: the marker brake, the per-address budget, and the bound /// on token rows. The address is the connection's, already corrected by the forwarded-headers handling that trusts only /// the local proxy, so a caller cannot pick its own. It fails closed: anything this middleware cannot decide is a 503, never a pass. /// public sealed class OidcGuardMiddleware(RequestDelegate next, OidcIssuerOptions options, OidcRateLimit limit, ILogger logger) { - private const int MaxClientIdLength = 128; - public async Task InvokeAsync(HttpContext http, IOpenIddictTokenManager tokens) { var endpoint = EndpointOf(http.Request.Path); @@ -38,20 +35,14 @@ public sealed class OidcGuardMiddleware(RequestDelegate next, OidcIssuerOptions return; } - if (limit.Spend($"address:{AddressKey(http.Connection.RemoteIpAddress)}") is { } byAddress) + if (limit.Spend(AddressKey(http.Connection.RemoteIpAddress)) is { } byAddress) { await RefuseAsync(http, StatusCodes.Status429TooManyRequests, "too_many_requests", byAddress.RetryAfter); return; } - if (limit.Spend($"client:{endpoint}:{await ClientKeyAsync(http, endpoint)}") is { } byClient) - { - await RefuseAsync(http, StatusCodes.Status429TooManyRequests, "too_many_requests", byClient.RetryAfter); - return; - } - // A new authorize request parks a row for anyone who can name the (public) client; a resumed one carries request_uri and parks none. - if (endpoint == "authorize" && !http.Request.Query.ContainsKey("request_uri") && await tokens.CountAsync(http.RequestAborted) >= options.Limits.MaxTokenRows) + if (endpoint == Authorize && !http.Request.Query.ContainsKey("request_uri") && await tokens.CountAsync(http.RequestAborted) >= options.Limits.MaxTokenRows) { await RefuseAsync(http, StatusCodes.Status503ServiceUnavailable, "temporarily_unavailable", options.Limits.PruneInterval); return; @@ -67,13 +58,33 @@ public sealed class OidcGuardMiddleware(RequestDelegate next, OidcIssuerOptions await next(http); } + private const string Authorize = "authorize"; + + // OpenIddict answers these paths with or without a trailing slash, so the match is by segments, never exact equality. private static string? EndpointOf(PathString path) => - path.Equals(OidcServiceCollectionExtensions.AuthorizationPath, StringComparison.OrdinalIgnoreCase) ? "authorize" - : path.Equals(OidcServiceCollectionExtensions.TokenPath, StringComparison.OrdinalIgnoreCase) ? "token" - : path.Equals(OidcServiceCollectionExtensions.UserInfoPath, StringComparison.OrdinalIgnoreCase) ? "userinfo" + path.StartsWithSegments(OidcServiceCollectionExtensions.AuthorizationPath, StringComparison.OrdinalIgnoreCase) ? Authorize + : path.StartsWithSegments(OidcServiceCollectionExtensions.TokenPath, StringComparison.OrdinalIgnoreCase) ? "token" + : path.StartsWithSegments(OidcServiceCollectionExtensions.UserInfoPath, StringComparison.OrdinalIgnoreCase) ? "userinfo" : null; - private static bool BrakeEngaged(string path) => !string.IsNullOrWhiteSpace(path) && File.Exists(path); + // A marker that cannot be told apart from an absent one (a permission or I/O error) throws, and the guard answers 503: the brake never releases by accident. + private static bool BrakeEngaged(string path) + { + if (string.IsNullOrWhiteSpace(path)) + { + return false; + } + + try + { + File.GetAttributes(path); + return true; + } + catch (Exception exception) when (exception is FileNotFoundException or DirectoryNotFoundException) + { + return false; + } + } // An IPv6 caller owns its /64, so rotating the low bits buys nothing. private static string AddressKey(IPAddress? address) @@ -98,32 +109,6 @@ public sealed class OidcGuardMiddleware(RequestDelegate next, OidcIssuerOptions return new IPAddress(bytes).ToString() + "/64"; } - // The client the caller names, not one it proved: authorize names it in the query or form, token in its Basic header. - // An unreadable value shares one key, so garbage cannot mint keys. - private static async Task ClientKeyAsync(HttpContext http, string endpoint) - { - string? clientId = null; - if (endpoint == "authorize") - { - clientId = http.Request.Query["client_id"].FirstOrDefault() - ?? (http.Request.HasFormContentType ? (await http.Request.ReadFormAsync(http.RequestAborted))["client_id"].FirstOrDefault() : null); - } - else if (endpoint == "token" && http.Request.Headers.Authorization.FirstOrDefault() is { } header - && header.StartsWith("Basic ", StringComparison.OrdinalIgnoreCase)) - { - try - { - var decoded = Encoding.UTF8.GetString(Convert.FromBase64String(header["Basic ".Length..].Trim())); - clientId = Uri.UnescapeDataString(decoded.Split(':', 2)[0]); - } - catch (FormatException) - { - } - } - - return string.IsNullOrEmpty(clientId) || clientId.Length > MaxClientIdLength ? "unnamed" : clientId; - } - private static Task RefuseAsync(HttpContext http, int status, string error, TimeSpan retryAfter) { http.Response.StatusCode = status; @@ -133,20 +118,22 @@ public sealed class OidcGuardMiddleware(RequestDelegate next, OidcIssuerOptions } } -/// The issuer's per-address and per-client permits, on the shared fixed-window budget. +/// +/// The issuer's per-address permits on the shared fixed-window budget. There is no per-client budget: the one client's id is +/// public and what a caller names unverified, so a budget on it would let anyone spend the real client's permits. +/// public sealed class OidcRateLimit(OidcIssuerOptions options, TimeProvider clock) { private readonly FixedWindowBudget addresses = new(options.Limits.PerAddressPermits, options.Limits.Window, options.Limits.MaxTrackedKeys, clock); - private readonly FixedWindowBudget clients = new(options.Limits.PerClientPermits, options.Limits.Window, options.Limits.MaxTrackedKeys, clock); - public BudgetDenial? Spend(string key) => key.StartsWith("address:", StringComparison.Ordinal) ? addresses.Spend(key) : clients.Spend(key); + public BudgetDenial? Spend(string address) => addresses.Spend(address); } /// /// Deletes expired and orphaned authorizations and tokens on a schedule: tokens past their expiry (parked authorize /// requests, codes, access tokens) or no longer valid, then authorizations that are no longer valid or are ad hoc with no -/// token left. Live rows are never touched. Plain queries, not OpenIddict's ExecuteDelete prune, so the same code runs on -/// Postgres and on the in-memory test provider. +/// token left. Live rows are never touched. Plain queries, not OpenIddict's ExecuteDelete prune, which the in-memory test provider +/// cannot run; an authorization takes its remaining tokens with it, because the foreign key has no cascade on Postgres. /// public sealed class OidcPruner(IServiceScopeFactory scopes, OidcIssuerOptions options, TimeProvider clock, ILogger logger) : BackgroundService { @@ -176,10 +163,13 @@ public sealed class OidcPruner(IServiceScopeFactory scopes, OidcIssuerOptions op do { + // A token row references its authorization (no cascade in the database), so the tokens go with it. var authorizations = await db.Set() + .Include(authorization => authorization.Tokens) .Where(authorization => authorization.CreationDate < young && (authorization.Status != Statuses.Valid || (authorization.Type == AuthorizationTypes.AdHoc && !authorization.Tokens.Any()))) .Take(Batch).ToListAsync(cancellationToken); + db.RemoveRange(authorizations.SelectMany(authorization => authorization.Tokens)); db.RemoveRange(authorizations); await db.SaveChangesAsync(cancellationToken); removed = authorizations.Count; diff --git a/tests/Bifrost.Web.Tests/OidcIssuerTests.cs b/tests/Bifrost.Web.Tests/OidcIssuerTests.cs index b2346e6..56f2b9a 100644 --- a/tests/Bifrost.Web.Tests/OidcIssuerTests.cs +++ b/tests/Bifrost.Web.Tests/OidcIssuerTests.cs @@ -7,6 +7,7 @@ using System.Text.Json; using Bifrost.Web.Domain; using Bifrost.Web.Features.Shared; using Bifrost.Web.Tests.Support; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; @@ -207,28 +208,97 @@ public sealed class OidcIssuerTests : IClassFixture } [Fact] - public async Task A_flood_naming_one_client_is_refused_for_that_client_only() + public async Task A_flood_naming_the_real_client_does_not_spend_anyone_elses_permits_and_a_trailing_slash_is_no_way_around() { - using var factory = new OidcSettingsFactory(Settings(("Limits:PerClientPermits", "3"))); + using var factory = new OidcSettingsFactory(Settings(("Limits:PerAddressPermits", "4"))); using var client = factory.CreateClient(ClientOptions()); - var flooded = new List(); - for (var i = 0; i < 5; i++) + var statuses = new List(); + foreach (var path in new[] { NewRequest().Path, NewRequest().Path, "/connect/authorize/?client_id=x", "/connect/token/", "/connect/userinfo/" }) { - using var response = await client.GetAsync(NewRequest("noisy-client").Path); - flooded.Add(response.StatusCode); + using var response = await client.GetAsync(path); + statuses.Add(response.StatusCode); } - using var other = await client.GetAsync(NewRequest().Path); - Assert.Equal([HttpStatusCode.TooManyRequests, HttpStatusCode.TooManyRequests], flooded.Skip(3)); - Assert.NotEqual(HttpStatusCode.TooManyRequests, other.StatusCode); - Assert.NotEqual(HttpStatusCode.TooManyRequests, flooded[0]); + Assert.Equal(HttpStatusCode.TooManyRequests, statuses[4]); // the slashed forms spend the same budget + Assert.DoesNotContain(HttpStatusCode.TooManyRequests, statuses.Take(4)); + } + + [Theory] + [InlineData("/connect/authorize")] + [InlineData("/connect/authorize/")] + [InlineData("/connect/token/")] + [InlineData("/connect/userinfo/")] + [InlineData("/CONNECT/Userinfo")] + public async Task The_brake_holds_for_every_spelling_of_an_endpoint(string path) + { + var marker = Path.Combine(Path.GetTempPath(), $"oidc-brake-{Guid.NewGuid():N}"); + File.WriteAllText(marker, "braked"); + try + { + using var factory = new OidcSettingsFactory(Settings(("BrakePath", marker))); + using var client = factory.CreateClient(ClientOptions()); + using var response = await client.GetAsync(path == "/connect/authorize" || path == "/connect/authorize/" ? path + "?client_id=x" : path); + Assert.Equal(HttpStatusCode.ServiceUnavailable, response.StatusCode); + } + finally + { + File.Delete(marker); + } + } + + [Fact] + public async Task A_guard_that_cannot_decide_answers_503_and_never_passes_the_request_on() + { + var reached = false; + var options = new Bifrost.Web.Configuration.OidcIssuerOptions(); + var middleware = new Bifrost.Web.Features.Oidc.OidcGuardMiddleware( + _ => { reached = true; return Task.CompletedTask; }, options, + new Bifrost.Web.Features.Oidc.OidcRateLimit(options, TimeProvider.System), + Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + var http = new DefaultHttpContext(); + http.Request.Path = "/connect/authorize"; + http.Request.QueryString = new QueryString("?client_id=x"); + http.Response.Body = new MemoryStream(); + + await middleware.InvokeAsync(http, System.Reflection.DispatchProxy.Create()); + + Assert.False(reached); + Assert.Equal(StatusCodes.Status503ServiceUnavailable, http.Response.StatusCode); + } + + /// Every call fails: the row count the guard asks of the token table cannot be answered. + public class ThrowingProxy : System.Reflection.DispatchProxy + { + protected override object? Invoke(System.Reflection.MethodInfo? targetMethod, object?[]? args) => throw new InvalidOperationException("the token table is down"); + } + + [Fact] + public async Task A_resumed_authorize_request_is_not_refused_by_the_row_cap_but_a_fresh_one_is() + { + using var factory = new OidcSettingsFactory(Settings(("Limits:MaxTokenRows", "3"))); + _factory = factory; + var (client, _) = await SignedInAsync(MembershipStatus.Active, "Resumer"); + using var first = await client.GetAsync(NewRequest().Path); // parks one row, answers with a redirect carrying the request_uri + var resume = first.Headers.Location!.ToString(); + Assert.Contains("request_uri", resume); + + using var anonymous = factory.CreateClient(ClientOptions()); + for (var i = 0; i < 2; i++) + { + using var filler = await anonymous.GetAsync(NewRequest().Path); + } + + using var fresh = await anonymous.GetAsync(NewRequest().Path); + Assert.Equal(HttpStatusCode.ServiceUnavailable, fresh.StatusCode); + using var resumed = await client.GetAsync(resume); + Assert.NotEqual(HttpStatusCode.ServiceUnavailable, resumed.StatusCode); } [Fact] public async Task A_mas_flow_fits_the_default_budgets_and_the_window_reopens() { - using var factory = new OidcSettingsFactory(Settings(("Limits:PerAddressPermits", "30"), ("Limits:PerClientPermits", "300"))); + using var factory = new OidcSettingsFactory(Settings(("Limits:PerAddressPermits", "30"))); _factory = factory; var (client, identity) = await SignedInAsync(MembershipStatus.Active, "Within Budget"); @@ -325,7 +395,7 @@ public sealed class OidcIssuerTests : IClassFixture await factory.PruneAsync(); var after = await factory.TokenRowsAsync(); Assert.True(after < before, "expired rows went"); - Assert.True(after >= 1, "the live access token stayed"); + Assert.Contains("urn:ietf:params:oauth:token-type:access_token", await factory.TokenTypesAsync()); // the live one, not merely some row using var userinfo = new HttpRequestMessage(HttpMethod.Get, "/connect/userinfo"); userinfo.Headers.Authorization = new AuthenticationHeaderValue("Bearer", tokens.AccessToken); @@ -333,6 +403,26 @@ public sealed class OidcIssuerTests : IClassFixture Assert.NotEqual(HttpStatusCode.Unauthorized, response.StatusCode); } + [Fact] + public async Task The_prune_takes_a_revoked_authorization_with_its_tokens_and_touches_no_live_authorization() + { + using var factory = new OidcSettingsFactory(Settings()); + _factory = factory; + var (revoked, _) = await SignedInAsync(MembershipStatus.Active, "Revoked"); + await CompleteFlowAsync(revoked); + var (live, _) = await SignedInAsync(MembershipStatus.Active, "Live"); + await CompleteFlowAsync(live); + Assert.Equal(2, await factory.AuthorizationRowsAsync()); + Assert.Equal(6, await factory.TokensOfAuthorizationsAsync()); // a spent code, an access token and an id token each + await factory.RevokeFirstAuthorizationAsync(); + + factory.Clock.Advance(TimeSpan.FromMinutes(6)); // past the grace for a young row; the access tokens (an hour) are still alive + await factory.PruneAsync(); + + Assert.Equal(1, await factory.AuthorizationRowsAsync()); + Assert.Equal(2, await factory.TokenRowsAsync()); // no orphan stays either: the revoked one's tokens went with it; the live one's access and id tokens stayed, spent codes go + } + [Theory] [InlineData("unknown-client", OidcTestKit.RedirectUri, true)] [InlineData(OidcTestKit.ClientId, "https://evil.example.test/callback", true)] diff --git a/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs b/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs index f413c08..c6e9575 100644 --- a/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs +++ b/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs @@ -5,6 +5,7 @@ using Bifrost.Web.Features.Shared; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Hosting; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Options; @@ -280,6 +281,43 @@ public sealed class OidcSettingsFactory(IReadOnlyDictionary set public Task PruneAsync() => Services.GetRequiredService().PruneAsync(CancellationToken.None); + public async Task> TokenTypesAsync() + { + using var scope = Services.CreateScope(); + var manager = scope.ServiceProvider.GetRequiredService(); + var types = new List(); + await foreach (var token in manager.ListAsync()) + { + types.Add(await manager.GetTypeAsync(token)); + } + + return types; + } + + public async Task AuthorizationRowsAsync() + { + using var scope = Services.CreateScope(); + return (int)await scope.ServiceProvider.GetRequiredService().CountAsync(); + } + + /// Tokens that belong to some authorization (the request and code rows carry none once spent). + public async Task TokensOfAuthorizationsAsync() + { + using var scope = Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + return await db.Set().CountAsync(token => token.Authorization != null); + } + + public async Task RevokeFirstAuthorizationAsync() + { + using var scope = Services.CreateScope(); + var manager = scope.ServiceProvider.GetRequiredService(); + await foreach (var authorization in manager.ListAsync(count: 1, offset: 0)) + { + Assert.True(await manager.TryRevokeAsync(authorization)); + } + } + public async Task TokenRowsAsync() { using var scope = Services.CreateScope(); -- 2.47.3 From 3dc70d0c6d0c645ed5a53defc9b6421a1f4e1be0 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 08:12:27 +0000 Subject: [PATCH 4/4] OIDC test kit: drop the unread PerClientPermits setting Co-Authored-By: Claude Sonnet 5.5 --- tests/Bifrost.Web.Tests/Support/OidcTestKit.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs b/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs index c6e9575..351ac79 100644 --- a/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs +++ b/tests/Bifrost.Web.Tests/Support/OidcTestKit.cs @@ -96,7 +96,6 @@ public sealed class OidcTestKit : IDisposable ["Bifrost:Oidc:Clients:Mas:RedirectUri"] = RedirectUri, // The shared test client has no remote address, so every request is one caller: the budgets are out of the way unless a test sets them. ["Bifrost:Oidc:Limits:PerAddressPermits"] = "100000", - ["Bifrost:Oidc:Limits:PerClientPermits"] = "100000" }; public void Dispose() -- 2.47.3