diff --git a/src/Bifrost.Web/Features/Oidc/OidcServiceCollectionExtensions.cs b/src/Bifrost.Web/Features/Oidc/OidcServiceCollectionExtensions.cs index 0e3bbf1..5ad30b8 100644 --- a/src/Bifrost.Web/Features/Oidc/OidcServiceCollectionExtensions.cs +++ b/src/Bifrost.Web/Features/Oidc/OidcServiceCollectionExtensions.cs @@ -70,7 +70,9 @@ public static class OidcServiceCollectionExtensions services.AddHostedService(); if (!string.IsNullOrWhiteSpace(options.Clients.Mas.BackchannelLogoutUri)) { - services.AddHttpClient(OidcLogoutReconciler.HttpClientName, client => client.Timeout = OidcLogoutReconciler.RequestTimeout); + // MAS answers the logout POST itself: a redirect is a non-delivery, never followed to some other 2xx. + services.AddHttpClient(OidcLogoutReconciler.HttpClientName, client => client.Timeout = OidcLogoutReconciler.RequestTimeout) + .ConfigurePrimaryHttpMessageHandler(() => new SocketsHttpHandler { AllowAutoRedirect = false }); services.AddHostedService(); } diff --git a/tests/Bifrost.Web.Tests/OidcLogoutTests.cs b/tests/Bifrost.Web.Tests/OidcLogoutTests.cs index cdbea0f..c357507 100644 --- a/tests/Bifrost.Web.Tests/OidcLogoutTests.cs +++ b/tests/Bifrost.Web.Tests/OidcLogoutTests.cs @@ -319,26 +319,89 @@ public sealed class OidcLogoutTests } [Fact] - public async Task An_unexpected_exception_is_not_a_network_fault_and_a_cancelled_post_is_not_a_failed_delivery() + public async Task One_subject_that_throws_unexpectedly_does_not_stop_the_pass_and_keeps_its_authorizations() + { + using var factory = new OidcLogoutFactory(); + var log = new ListLogger(); + var reconciler = factory.NewReconciler(logger: log); + var (first, _) = await factory.GrantAsync(MembershipStatus.Suspended); + var (second, _) = await factory.GrantAsync(MembershipStatus.Suspended); + var subjects = new[] { first, second }.Select(x => factory.AccountId(x).ToString("D")).ToArray(); + + // Whichever subject the pass reaches first throws; the other is still logged out and revoked. + factory.Mas.Script(FakeMasHandler.Unexpected); + await reconciler.ReconcileOnceAsync(CancellationToken.None); + + Assert.Equal(2, factory.Mas.Bodies.Count); + var firstToken = factory.Mas.LogoutTokens()[0]; + var thrown = new JsonWebToken(firstToken).Subject; + var other = subjects.Single(x => x != thrown); + Assert.Equal(0, factory.ValidAuthorizations(other)); + Assert.True(factory.ValidAuthorizations(thrown) > 0); + var failure = Assert.Single(log.Entries, x => x.Level == LogLevel.Error); + Assert.Contains(thrown, failure.Message, StringComparison.Ordinal); + Assert.Contains(nameof(InvalidOperationException), failure.Message, StringComparison.Ordinal); + Assert.DoesNotContain("not a network fault", failure.Message, StringComparison.Ordinal); + Assert.DoesNotContain(firstToken, failure.Message, StringComparison.Ordinal); + + // The failed subject was not backed off: the next pass tries it again and delivers. + await reconciler.ReconcileOnceAsync(CancellationToken.None); + Assert.Equal(3, factory.Mas.Bodies.Count); + Assert.Equal(0, factory.ValidAuthorizations(thrown)); + } + + [Fact] + public async Task A_cancelled_post_stops_the_pass_and_is_not_a_failed_delivery() { using var factory = new OidcLogoutFactory(); var (identity, _) = await factory.GrantAsync(MembershipStatus.Suspended); var subject = factory.AccountId(identity).ToString("D"); var reconciler = factory.Reconciler; - factory.Mas.Script(FakeMasHandler.Unexpected); - await Assert.ThrowsAsync(() => reconciler.ReconcileOnceAsync(CancellationToken.None)); - // Stopping while the post is in flight surfaces as cancellation and records no backoff: the next pass posts at once. factory.Mas.Script(FakeMasHandler.Hang); using var stop = new CancellationTokenSource(TimeSpan.FromMilliseconds(300)); await Assert.ThrowsAnyAsync(() => reconciler.ReconcileOnceAsync(stop.Token)); - Assert.Equal(2, factory.Mas.Bodies.Count); + Assert.Single(factory.Mas.Bodies); await reconciler.ReconcileOnceAsync(CancellationToken.None); - Assert.Equal(3, factory.Mas.Bodies.Count); + Assert.Equal(2, factory.Mas.Bodies.Count); Assert.Equal(0, factory.ValidAuthorizations(subject)); } + [Fact] + public async Task A_redirect_from_mas_is_not_followed_and_is_not_delivery() + { + using var mas = new RedirectingMas(); + using var factory = new OidcLogoutFactory(mas.LogoutUri); + var (identity, _) = await factory.GrantAsync(MembershipStatus.Suspended); + var subject = factory.AccountId(identity).ToString("D"); + var before = factory.ValidAuthorizations(subject); + Assert.True(before > 0); + + await factory.Reconciler.ReconcileOnceAsync(CancellationToken.None); + + Assert.Equal(["POST /logout"], mas.Requests); + Assert.Equal(before, factory.ValidAuthorizations(subject)); + + // A redirect is a non-delivery like any non-2xx: it backs off, then posts again. + factory.Time.Advance(TimeSpan.FromSeconds(30)); + await factory.Reconciler.ReconcileOnceAsync(CancellationToken.None); + Assert.Equal(["POST /logout", "POST /logout"], mas.Requests); + Assert.Equal(before, factory.ValidAuthorizations(subject)); + } + + [Fact] + public void The_logout_client_times_out_after_ten_seconds() + { + using var factory = new OidcLogoutFactory(); + using var client = factory.CreateClient(); + + Assert.Equal(TimeSpan.FromSeconds(10), OidcLogoutReconciler.RequestTimeout); + Assert.Equal( + TimeSpan.FromSeconds(10), + factory.Services.GetRequiredService().CreateClient(OidcLogoutReconciler.HttpClientName).Timeout); + } + [Fact] public async Task The_hosted_loop_ticks_again_after_each_interval_and_survives_a_failed_tick() { diff --git a/tests/Bifrost.Web.Tests/Support/OidcLogoutKit.cs b/tests/Bifrost.Web.Tests/Support/OidcLogoutKit.cs index abb5a06..9b771fd 100644 --- a/tests/Bifrost.Web.Tests/Support/OidcLogoutKit.cs +++ b/tests/Bifrost.Web.Tests/Support/OidcLogoutKit.cs @@ -211,12 +211,134 @@ public sealed class FakeMasHandler : HttpMessageHandler } } +/// +/// A real HTTP listener on loopback standing in for a MAS that answers every POST with 302 to /login and GET /login with +/// 200: what a real HttpClient does with that answer is the registered handler's rule, which a fake handler cannot see. +/// +public sealed class RedirectingMas : IDisposable +{ + private readonly System.Net.Sockets.TcpListener _listener = new(IPAddress.Loopback, 0); + private readonly List _requests = []; + private readonly CancellationTokenSource _stop = new(); + private readonly Task _accepting; + + public RedirectingMas() + { + _listener.Start(); + _accepting = Task.Run(AcceptAsync); + } + + public string LogoutUri => $"http://127.0.0.1:{((IPEndPoint)_listener.LocalEndpoint).Port}/logout"; + + /// "METHOD path" for every request received, in order. + public IReadOnlyList Requests + { + get + { + lock (_requests) + { + return _requests.ToArray(); + } + } + } + + private async Task AcceptAsync() + { + try + { + while (!_stop.IsCancellationRequested) + { + var connection = await _listener.AcceptTcpClientAsync(_stop.Token); + _ = Task.Run(() => ServeAsync(connection)); + } + } + catch (Exception) when (_stop.IsCancellationRequested) + { + } + } + + private async Task ServeAsync(System.Net.Sockets.TcpClient connection) + { + using var owned = connection; + try + { + var stream = connection.GetStream(); + using var reader = new StreamReader(stream, Encoding.ASCII, leaveOpen: true); + var requestLine = await reader.ReadLineAsync(_stop.Token) ?? ""; + var length = 0; + while (await reader.ReadLineAsync(_stop.Token) is { Length: > 0 } header) + { + if (header.StartsWith("Content-Length:", StringComparison.OrdinalIgnoreCase)) + { + length = int.Parse(header["Content-Length:".Length..].Trim()); + } + } + + var body = new char[length]; + var read = 0; + while (read < length) + { + var n = await reader.ReadAsync(body.AsMemory(read), _stop.Token); + if (n == 0) + { + break; + } + + read += n; + } + + var parts = requestLine.Split(' '); + lock (_requests) + { + _requests.Add($"{parts[0]} {parts[1]}"); + } + + var response = parts[0] == "POST" + ? "HTTP/1.1 302 Found\r\nLocation: /login\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" + : "HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"; + await stream.WriteAsync(Encoding.ASCII.GetBytes(response), _stop.Token); + } + catch (Exception error) when (error is IOException or OperationCanceledException or ObjectDisposedException) + { + } + } + + public void Dispose() + { + _stop.Cancel(); + _listener.Stop(); + try + { + _accepting.Wait(TimeSpan.FromSeconds(2)); + } + catch (AggregateException) + { + } + } +} + /// An OIDC host with MAS's back-channel logout URI set, a fake MAS behind it and a clock the test drives. public sealed class OidcLogoutFactory : OidcWebApplicationFactory { public const string LogoutUri = "https://mas.example.test/upstream/backchannel-logout/01TESTPROVIDER"; public static readonly Uri Origin = new("https://localhost"); + private readonly string _logoutUri; + private readonly bool _fakeTransport; + + public OidcLogoutFactory() + { + _logoutUri = LogoutUri; + _fakeTransport = true; + } + + /// Keeps the real registered HttpClient and its primary handler and posts to . + public OidcLogoutFactory(string logoutUri) + { + _logoutUri = logoutUri; + _fakeTransport = false; + } + public FakeMasHandler Mas { get; } = new(); public ManualClock Time { get; } = new(); @@ -226,12 +348,15 @@ public sealed class OidcLogoutFactory : OidcWebApplicationFactory protected override void ConfigureWebHost(IWebHostBuilder builder) { base.ConfigureWebHost(builder); - builder.UseSetting("Bifrost:Oidc:Clients:Mas:BackchannelLogoutUri", LogoutUri); + builder.UseSetting("Bifrost:Oidc:Clients:Mas:BackchannelLogoutUri", _logoutUri); builder.ConfigureServices(services => { services.RemoveAll(); services.AddSingleton(Time); - services.AddHttpClient(OidcLogoutReconciler.HttpClientName).ConfigurePrimaryHttpMessageHandler(() => Mas); + if (_fakeTransport) + { + services.AddHttpClient(OidcLogoutReconciler.HttpClientName).ConfigurePrimaryHttpMessageHandler(() => Mas); + } }); }