From 030bd0eede8cd87243c83dd1cec7d042d025ec87 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 12:16:18 +0000 Subject: [PATCH 1/7] Members: demoting an admin derives the flags from the roles left after the removal Co-Authored-By: Claude Sonnet 5.5 --- src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs b/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs index e9b536b..f761a3f 100644 --- a/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs +++ b/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs @@ -177,6 +177,8 @@ public sealed class IndexModel( } else if (!(user.Id == Actor.UserAccount.Id && role == MemberRole.PlatformAdmin)) { + // Out of the collection too, so the flags below derive from the roles as they will stand after the save. + user.Membership.RoleAssignments.Remove(existingAssignment); dbContext.RoleAssignments.Remove(existingAssignment); } -- 2.47.3 From 27ca04f1d6ef1180b29ad55cbf62e408fec76a72 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 12:16:18 +0000 Subject: [PATCH 2/7] Bootstrap admin promotion never lifts a suspension and writes a receipt when it changes status (tests cover this and the demotion fix) Co-Authored-By: Claude Sonnet 5.5 --- .../MembershipSynchronizationService.cs | 31 +-- .../Shared/ApplicationBootstrapper.cs | 51 ++++- .../Bifrost.Web.Tests/BootstrapAdminTests.cs | 176 ++++++++++++++++++ 3 files changed, 233 insertions(+), 25 deletions(-) create mode 100644 tests/Bifrost.Web.Tests/BootstrapAdminTests.cs diff --git a/src/Bifrost.Web/Features/Membership/MembershipSynchronizationService.cs b/src/Bifrost.Web/Features/Membership/MembershipSynchronizationService.cs index 8ee13a0..6d41d73 100644 --- a/src/Bifrost.Web/Features/Membership/MembershipSynchronizationService.cs +++ b/src/Bifrost.Web/Features/Membership/MembershipSynchronizationService.cs @@ -12,7 +12,9 @@ public sealed class MembershipSynchronizationService( BifrostDbContext dbContext, BifrostIdentityService bifrostIdentityService, IOptions bootstrapOptions, - TimeProvider timeProvider) + TimeProvider timeProvider, + AuditTrailService auditTrailService, + ILogger logger) { public async Task SynchronizeAsync(ClaimsPrincipal principal, CancellationToken cancellationToken) { @@ -109,9 +111,14 @@ public sealed class MembershipSynchronizationService( invitation.AcceptedAtUtc = now; } + MembershipStatus? promotedFrom = null; if (IsBootstrapAdmin(gitHubLogin, bootstrapOptions.Value)) { - PromoteToBootstrapAdmin(userAccount.Membership!, now); + promotedFrom = userAccount.Membership!.Status; + if (!ApplicationBootstrapper.TryPromoteBootstrapAdmin(userAccount, now, logger)) + { + promotedFrom = null; + } } else if (userAccount.Membership!.Status == MembershipStatus.Active) { @@ -125,22 +132,16 @@ public sealed class MembershipSynchronizationService( userAccount.MemberProfile!.UpdatedAtUtc = now; + if (promotedFrom is { } before && before != userAccount.Membership!.Status) + { + // One save: the promotion and its receipt stand or fall together. + await ApplicationBootstrapper.RecordPromotionAsync(auditTrailService, userAccount, before, cancellationToken); + return; + } + await dbContext.SaveChangesAsync(cancellationToken); } - private static void PromoteToBootstrapAdmin(Bifrost.Web.Domain.Membership membership, DateTimeOffset now) - { - membership.Status = MembershipStatus.Active; - membership.IsPlatformAdmin = true; - membership.CanManageProjects = true; - membership.CanManageLedger = true; - membership.CanModerateMotions = true; - membership.ApprovedAtUtc ??= now; - - ApplicationBootstrapper.EnsureRole(membership, MemberRole.PlatformAdmin, null, now, "Bootstrap admin"); - ApplicationBootstrapper.EnsureRole(membership, MemberRole.StandardMember, null, now, "Default active member role"); - } - private static bool TryReadGitHubIdentity( ClaimsPrincipal principal, out long gitHubUserId, diff --git a/src/Bifrost.Web/Features/Shared/ApplicationBootstrapper.cs b/src/Bifrost.Web/Features/Shared/ApplicationBootstrapper.cs index 1a3cd0f..04879e2 100644 --- a/src/Bifrost.Web/Features/Shared/ApplicationBootstrapper.cs +++ b/src/Bifrost.Web/Features/Shared/ApplicationBootstrapper.cs @@ -10,6 +10,7 @@ public sealed class ApplicationBootstrapper( BifrostDbContext dbContext, IOptions bootstrapOptions, TimeProvider timeProvider, + AuditTrailService auditTrailService, ILogger logger) { public async Task RunAsync(CancellationToken cancellationToken) @@ -40,16 +41,11 @@ public sealed class ApplicationBootstrapper( foreach (var adminUser in adminUsers.Where(x => x.Membership is not null)) { - var membership = adminUser.Membership!; - membership.Status = MembershipStatus.Active; - membership.IsPlatformAdmin = true; - membership.CanManageProjects = true; - membership.CanManageLedger = true; - membership.CanModerateMotions = true; - membership.ApprovedAtUtc ??= now; - - EnsureRole(membership, MemberRole.PlatformAdmin, null, now, "Bootstrap admin"); - EnsureRole(membership, MemberRole.StandardMember, null, now, "Default active member role"); + var before = adminUser.Membership!.Status; + if (TryPromoteBootstrapAdmin(adminUser, now, logger) && before != adminUser.Membership.Status) + { + await RecordPromotionAsync(auditTrailService, adminUser, before, cancellationToken); + } } if (dbContext.ChangeTracker.HasChanges()) @@ -58,6 +54,41 @@ public sealed class ApplicationBootstrapper( } } + /// + /// The one bootstrap-admin promotion, used at startup and at sign-in. A Suspended account is left exactly as it is (an admin's + /// decision; only the suspension service lifts it) and the refusal is logged by account id. Returns whether it promoted. + /// + public static bool TryPromoteBootstrapAdmin(UserAccount account, DateTimeOffset now, ILogger logger) + { + var membership = account.Membership!; + if (membership.Status == MembershipStatus.Suspended) + { + logger.LogWarning("Bootstrap admin promotion skipped for suspended account {UserAccountId}.", account.Id); + return false; + } + + membership.Status = MembershipStatus.Active; + membership.IsPlatformAdmin = true; + membership.CanManageProjects = true; + membership.CanManageLedger = true; + membership.CanModerateMotions = true; + membership.ApprovedAtUtc ??= now; + + EnsureRole(membership, MemberRole.PlatformAdmin, null, now, "Bootstrap admin"); + EnsureRole(membership, MemberRole.StandardMember, null, now, "Default active member role"); + return true; + } + + /// The receipt for a bootstrap promotion that changed membership status; saves together with the promotion. + public static Task RecordPromotionAsync(AuditTrailService auditTrailService, UserAccount account, MembershipStatus before, CancellationToken cancellationToken) => + auditTrailService.RecordAsync( + null, + nameof(Bifrost.Web.Domain.Membership), + account.Membership!.Id, + "membership.bootstrap-promoted", + $"Bootstrap admin promotion of {account.DisplayName} ({before} -> {account.Membership.Status}).", + cancellationToken); + public static void EnsureRole( Bifrost.Web.Domain.Membership membership, MemberRole role, diff --git a/tests/Bifrost.Web.Tests/BootstrapAdminTests.cs b/tests/Bifrost.Web.Tests/BootstrapAdminTests.cs new file mode 100644 index 0000000..a980dd6 --- /dev/null +++ b/tests/Bifrost.Web.Tests/BootstrapAdminTests.cs @@ -0,0 +1,176 @@ +using System.Security.Claims; +using Bifrost.Web.Data; +using Bifrost.Web.Domain; +using Bifrost.Web.Features.Membership; +using Bifrost.Web.Features.Shared; +using Bifrost.Web.Pages.App.Members; +using Bifrost.Web.Tests.Support; +using Microsoft.AspNetCore.Mvc; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Bifrost.Web.Tests; + +/// Bootstrap promotion never lifts a suspension and leaves a receipt when it changes status; demoting an admin clears the admin flags. +public sealed class BootstrapAdminTests +{ + private const long BootstrapGitHubId = 4242; + + private static (Microsoft.AspNetCore.Mvc.Testing.WebApplicationFactory Factory, CapturingLoggerProvider Logs) NewFactory() + { + var logs = new CapturingLoggerProvider(); + var factory = new TestWebApplicationFactory().WithWebHostBuilder(b => b.ConfigureServices(s => s.AddLogging(l => l.AddProvider(logs)))); + return (factory, logs); + } + + private static async Task SeedBootstrapLoginAsync(IServiceProvider services, MembershipStatus status) + { + using var scope = services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var account = new UserAccount + { + DisplayName = "Test Admin", + GitHubUserId = BootstrapGitHubId, + GitHubLogin = "test-admin", + NormalizedGitHubLogin = "TEST-ADMIN", + BifrostIdentity = "testadmin", + NormalizedBifrostIdentity = "testadmin", + MemberProfile = new MemberProfile { Nickname = "ta" }, + Membership = new Bifrost.Web.Domain.Membership { Status = status, ApprovedAtUtc = status == MembershipStatus.Suspended ? DateTimeOffset.UtcNow : null } + }; + db.UserAccounts.Add(account); + await db.SaveChangesAsync(); + return account.Id; + } + + private static async Task<(Bifrost.Web.Domain.Membership Membership, List Receipts)> ReadAsync(IServiceProvider services, Guid accountId) + { + using var scope = services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var membership = await db.Memberships.AsNoTracking().Include(x => x.RoleAssignments).SingleAsync(x => x.UserAccountId == accountId); + var receipts = await db.AuditEvents.AsNoTracking().Where(x => x.EntityId == membership.Id.ToString()).ToListAsync(); + return (membership, receipts); + } + + private static ClaimsPrincipal BootstrapPrincipal() => new(new ClaimsIdentity( + [ + new Claim(ClaimTypes.NameIdentifier, BootstrapGitHubId.ToString()), + new Claim(ClaimTypes.Name, "test-admin") + ], "test")); + + [Theory] + [InlineData("startup")] + [InlineData("signin")] + public async Task A_suspended_bootstrap_login_stays_suspended_and_unpromoted(string path) + { + var (factory, logs) = NewFactory(); + using var _ = factory; + var id = await SeedBootstrapLoginAsync(factory.Services, MembershipStatus.Suspended); + var before = (await ReadAsync(factory.Services, id)).Receipts.Count; + + await RunPathAsync(factory.Services, path); + + var (membership, receipts) = await ReadAsync(factory.Services, id); + Assert.Equal(MembershipStatus.Suspended, membership.Status); + Assert.False(membership.IsPlatformAdmin); + Assert.Empty(membership.RoleAssignments); + Assert.Equal(before, receipts.Count); + Assert.Contains(logs.Lines, x => x.Contains("suspended account") && x.Contains(id.ToString())); + } + + [Theory] + [InlineData("startup")] + [InlineData("signin")] + public async Task A_bootstrap_promotion_that_changes_status_writes_a_receipt(string path) + { + var (factory, _) = NewFactory(); + using var _f = factory; + var id = await SeedBootstrapLoginAsync(factory.Services, MembershipStatus.Authenticated); + + await RunPathAsync(factory.Services, path); + + var (membership, receipts) = await ReadAsync(factory.Services, id); + Assert.Equal(MembershipStatus.Active, membership.Status); + Assert.True(membership.IsPlatformAdmin); + var receipt = Assert.Single(receipts, x => x.Action == "membership.bootstrap-promoted"); + Assert.Contains("Authenticated -> Active", receipt.Detail); + + await RunPathAsync(factory.Services, path); // already Active: no second receipt + Assert.Single((await ReadAsync(factory.Services, id)).Receipts, x => x.Action == "membership.bootstrap-promoted"); + } + + private static async Task RunPathAsync(IServiceProvider services, string path) + { + using var scope = services.CreateScope(); + if (path == "startup") + { + await scope.ServiceProvider.GetRequiredService().RunAsync(CancellationToken.None); + } + else + { + await scope.ServiceProvider.GetRequiredService().SynchronizeAsync(BootstrapPrincipal(), CancellationToken.None); + } + } + + private sealed class FixedActor(CurrentBifrostActor actor) : ICurrentBifrostActorAccessor + { + public Task GetAsync(CancellationToken cancellationToken) => Task.FromResult(actor); + } + + private static async Task SeedAsync(IServiceProvider services, string name, bool admin) + { + using var scope = services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var membership = new Bifrost.Web.Domain.Membership { Status = MembershipStatus.Active, ApprovedAtUtc = DateTimeOffset.UtcNow }; + if (admin) + { + membership.IsPlatformAdmin = membership.CanManageProjects = membership.CanManageLedger = membership.CanModerateMotions = true; + membership.RoleAssignments.Add(new RoleAssignment { Role = MemberRole.PlatformAdmin }); + } + + var account = new UserAccount + { + DisplayName = name, + BifrostIdentity = $"{name}-{Guid.NewGuid():N}"[..20], + NormalizedBifrostIdentity = $"{name}-{Guid.NewGuid():N}"[..20], + Membership = membership + }; + db.UserAccounts.Add(account); + await db.SaveChangesAsync(); + return account.Id; + } + + private static async Task ToggleAsync(IServiceProvider services, Guid by, Guid target, MemberRole role) + { + using var scope = services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var actor = new CurrentBifrostActor(true, await db.UserAccounts.Include(x => x.Membership!).ThenInclude(x => x.RoleAssignments).SingleAsync(x => x.Id == by)); + var page = new IndexModel(db, new FixedActor(actor), scope.ServiceProvider.GetRequiredService(), + scope.ServiceProvider.GetRequiredService(), TimeProvider.System); + Assert.IsType(await page.OnPostToggleRoleAsync(target, role, CancellationToken.None)); + } + + [Fact] + public async Task Demoting_an_admin_through_the_page_clears_the_admin_flags_and_promoting_sets_them() + { + var (factory, _) = NewFactory(); + using var _f = factory; + var actingAdmin = await SeedAsync(factory.Services, "acting", admin: true); + var otherAdmin = await SeedAsync(factory.Services, "other", admin: true); + var member = await SeedAsync(factory.Services, "member", admin: false); + + await ToggleAsync(factory.Services, actingAdmin, otherAdmin, MemberRole.PlatformAdmin); + var (demoted, _) = await ReadAsync(factory.Services, otherAdmin); + Assert.False(demoted.IsPlatformAdmin); + Assert.False(demoted.CanManageProjects); + Assert.False(demoted.CanManageLedger); + Assert.False(demoted.CanModerateMotions); + Assert.DoesNotContain(demoted.RoleAssignments, x => x.Role == MemberRole.PlatformAdmin); + + await ToggleAsync(factory.Services, actingAdmin, member, MemberRole.PlatformAdmin); + var (promoted, _) = await ReadAsync(factory.Services, member); + Assert.True(promoted.IsPlatformAdmin); + Assert.Contains(promoted.RoleAssignments, x => x.Role == MemberRole.PlatformAdmin); + } +} -- 2.47.3 From c1c057b43c5d2cc1bcf5dff56769b65f0923233b Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 12:17:36 +0000 Subject: [PATCH 3/7] Members page: a refused suspend or unsuspend shows the admin why (fixed message via TempData, role=alert) Co-Authored-By: Claude Sonnet 5.5 --- .../Pages/App/Members/Index.cshtml | 5 + .../Pages/App/Members/Index.cshtml.cs | 20 ++- .../MembersPageNoticeTests.cs | 130 ++++++++++++++++++ 3 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 tests/Bifrost.Web.Tests/MembersPageNoticeTests.cs diff --git a/src/Bifrost.Web/Pages/App/Members/Index.cshtml b/src/Bifrost.Web/Pages/App/Members/Index.cshtml index b4d67e5..91f04c4 100644 --- a/src/Bifrost.Web/Pages/App/Members/Index.cshtml +++ b/src/Bifrost.Web/Pages/App/Members/Index.cshtml @@ -14,6 +14,11 @@ because trust and contributor accountability are product concerns, not just auth concerns.

+ @if (!string.IsNullOrEmpty(Model.Notice)) + { + + } + @if (Model.Actor.CanManageMembership) {
diff --git a/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs b/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs index f761a3f..ba0715c 100644 --- a/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs +++ b/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs @@ -22,6 +22,18 @@ public sealed class IndexModel( [BindProperty] public UpdateProfileInput Profile { get; set; } = new(); + /// Why the last suspend or unsuspend did nothing; one of , carried across the redirect. + [TempData] + public string? Notice { get; set; } + + private static readonly IReadOnlyDictionary RefusalNotices = new Dictionary + { + [SuspensionOutcome.NoReason] = "Nothing changed: a reason is required to suspend or unsuspend an account.", + [SuspensionOutcome.ProtectedAccount] = "Nothing changed: you cannot suspend yourself or another platform admin.", + [SuspensionOutcome.NoSuchAccount] = "Nothing changed: that account or its membership does not exist.", + [SuspensionOutcome.NotApplicable] = "Nothing changed: the account is already in that state." + }; + public CurrentBifrostActor Actor { get; private set; } = CurrentBifrostActor.Anonymous; public IReadOnlyList Users { get; private set; } = []; @@ -143,7 +155,13 @@ public sealed class IndexModel( var outcome = suspend ? await suspension.SuspendAsync(Actor, userAccountId, reason, cancellationToken) : await suspension.UnsuspendAsync(Actor, userAccountId, reason, cancellationToken); - return outcome == SuspensionOutcome.NotAdmin ? Forbid() : RedirectToPage(); + if (outcome == SuspensionOutcome.NotAdmin) + { + return Forbid(); + } + + Notice = RefusalNotices.GetValueOrDefault(outcome); + return RedirectToPage(); } public async Task OnPostToggleRoleAsync(Guid userAccountId, MemberRole role, CancellationToken cancellationToken) diff --git a/tests/Bifrost.Web.Tests/MembersPageNoticeTests.cs b/tests/Bifrost.Web.Tests/MembersPageNoticeTests.cs new file mode 100644 index 0000000..b84c3ce --- /dev/null +++ b/tests/Bifrost.Web.Tests/MembersPageNoticeTests.cs @@ -0,0 +1,130 @@ +using System.Net; +using System.Text.RegularExpressions; +using Bifrost.Web.Data; +using Bifrost.Web.Domain; +using Bifrost.Web.Tests.Support; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; + +namespace Bifrost.Web.Tests; + +/// A refused suspend or unsuspend tells the admin why on the page the redirect lands on, and shows only a fixed message. +public sealed class MembersPageNoticeTests(TestWebApplicationFactory factory) : IClassFixture +{ + private HttpClient Client() => factory.CreateClient(new WebApplicationFactoryClientOptions + { + AllowAutoRedirect = false, + BaseAddress = new Uri("https://localhost"), + HandleCookies = true + }); + + private async Task SeedMemberAsync(MembershipStatus status) + { + using var scope = factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var id = $"notice-{Guid.NewGuid():N}"[..20]; + var account = new UserAccount + { + DisplayName = id, BifrostIdentity = id, NormalizedBifrostIdentity = id, + Membership = new Bifrost.Web.Domain.Membership { Status = status, ApprovedAtUtc = DateTimeOffset.UtcNow } + }; + db.UserAccounts.Add(account); + await db.SaveChangesAsync(); + return account.Id; + } + + private async Task SignedInAdminIdAsync(HttpClient client) + { + using var _ = await client.GetAsync("/App/Members"); // sign-in sync creates and promotes the bootstrap admin + using var scope = factory.Services.CreateScope(); + return (await scope.ServiceProvider.GetRequiredService().UserAccounts.AsNoTracking().SingleAsync(x => x.GitHubUserId == 12345)).Id; + } + + private static async Task TokenAsync(HttpClient client) + { + var html = await (await client.GetAsync("/App/Members")).Content.ReadAsStringAsync(); + return Regex.Match(html, "name=\"__RequestVerificationToken\"[^>]*value=\"([^\"]+)\"").Groups[1].Value; + } + + private static async Task PostThenFollowAsync(HttpClient client, string handler, Guid target, string reason) + { + var token = await TokenAsync(client); + Assert.NotEmpty(token); + using var post = await client.PostAsync($"/App/Members?handler={handler}", new FormUrlEncodedContent(new Dictionary + { + ["userAccountId"] = target.ToString(), + ["reason"] = reason, + ["__RequestVerificationToken"] = token + })); + Assert.Equal(HttpStatusCode.Redirect, post.StatusCode); + using var landed = await client.GetAsync(post.Headers.Location); + Assert.Equal(HttpStatusCode.OK, landed.StatusCode); + return await landed.Content.ReadAsStringAsync(); + } + + private static readonly Regex Alert = new("]*role=\"alert\"[^>]*>(.*?)

", RegexOptions.Singleline); + + [Fact] + public async Task A_suspend_without_a_reason_says_a_reason_is_required_and_changes_nothing() + { + using var client = Client(); + _ = await SignedInAdminIdAsync(client); + var member = await SeedMemberAsync(MembershipStatus.Active); + + var html = await PostThenFollowAsync(client, "Suspend", member, " "); + + Assert.Contains("a reason is required", Alert.Match(html).Groups[1].Value); + using var scope = factory.Services.CreateScope(); + Assert.Equal(MembershipStatus.Active, (await scope.ServiceProvider.GetRequiredService().Memberships.AsNoTracking().SingleAsync(x => x.UserAccountId == member)).Status); + } + + [Fact] + public async Task A_suspend_of_the_admin_themselves_says_it_is_protected() + { + using var client = Client(); + var admin = await SignedInAdminIdAsync(client); + + var html = await PostThenFollowAsync(client, "Suspend", admin, "oops"); + + Assert.Contains("cannot suspend yourself or another platform admin", Alert.Match(html).Groups[1].Value); + } + + [Fact] + public async Task A_suspend_of_a_missing_account_says_so_and_never_echoes_the_submitted_text() + { + using var client = Client(); + _ = await SignedInAdminIdAsync(client); + const string canary = ""; + + var html = await PostThenFollowAsync(client, "Suspend", Guid.NewGuid(), canary); + + Assert.Contains("does not exist", Alert.Match(html).Groups[1].Value); + Assert.DoesNotContain("CANARY-notice-77", html); + } + + [Fact] + public async Task An_unsuspend_of_an_active_account_says_it_is_already_in_that_state_and_the_notice_shows_once() + { + using var client = Client(); + _ = await SignedInAdminIdAsync(client); + var member = await SeedMemberAsync(MembershipStatus.Active); + + var html = await PostThenFollowAsync(client, "Unsuspend", member, "appeal"); + + Assert.Contains("already in that state", Alert.Match(html).Groups[1].Value); + Assert.DoesNotContain("role=\"alert\"", await (await client.GetAsync("/App/Members")).Content.ReadAsStringAsync()); + } + + [Fact] + public async Task A_successful_suspend_shows_no_refusal() + { + using var client = Client(); + _ = await SignedInAdminIdAsync(client); + var member = await SeedMemberAsync(MembershipStatus.Active); + + var html = await PostThenFollowAsync(client, "Suspend", member, "spam"); + + Assert.DoesNotContain("role=\"alert\"", html); + } +} -- 2.47.3 From 71ba999f9db8a9e68976e196cabaf3965e3da3e3 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 12:22:20 +0000 Subject: [PATCH 4/7] Suspension reason moves to an admin-read column on the membership; the receipt holds no reason Co-Authored-By: Claude Sonnet 5.5 --- docs/registration-gates.md | 2 +- ...111110000_AddMembershipSuspensionReason.cs | 25 ++++++ .../BifrostDbContextModelSnapshot.cs | 3 + src/Bifrost.Web/Domain/BifrostModels.cs | 3 + .../Membership/MembershipSuspensionService.cs | 8 +- .../Pages/App/Members/Index.cshtml | 4 + .../Pages/App/Members/Index.cshtml.cs | 2 + .../MembershipSuspensionTests.cs | 25 +++++- .../MigrationDiscoveryTests.cs | 3 +- .../Support/OidcLogoutKit.cs | 20 ++++- .../SuspensionReasonExposureTests.cs | 84 +++++++++++++++++++ 11 files changed, 168 insertions(+), 11 deletions(-) create mode 100644 src/Bifrost.Web/Data/Migrations/20261111110000_AddMembershipSuspensionReason.cs create mode 100644 tests/Bifrost.Web.Tests/SuspensionReasonExposureTests.cs diff --git a/docs/registration-gates.md b/docs/registration-gates.md index fd039be..5564b17 100644 --- a/docs/registration-gates.md +++ b/docs/registration-gates.md @@ -11,7 +11,7 @@ This page scopes the three that did not exist, says where each lives, and record | Rate limits and brake | `OidcGuardMiddleware` (`docs/oidc-issuer.md`) | Merged. Covers `/connect/*` only, not the register endpoint. | | Reserved names | `BifrostIdentityService.IsReserved`, 28 names | Merged, at creation and at authorize. | | Chat tier, no vote | `Authenticated` / `PendingApproval` accounts chat and open no member or governance surface | Merged. | -| Suspension | `MembershipSuspensionService` | Merged. | +| Suspension | `MembershipSuspensionService` | Merged. The reason is `Membership.SuspensionReason`, written with the status change and its receipt, cleared on unsuspend, projected only to admins on the members page; the receipt (`AuditEvent.Detail`) holds no reason text. Receipts written before the column existed still hold their reasons as history; the member dashboard keeps filtering suspension receipts out. An unsuspend still requires a reason, but nothing stores it. | | CAPTCHA | `Features/Registration/RegistrationChallengeService` | Built here. | | Consent receipt | `ConsentReceipt`, `TrainingNotice` | Built here. | | Email verification | nowhere | Scoped below, not built. | diff --git a/src/Bifrost.Web/Data/Migrations/20261111110000_AddMembershipSuspensionReason.cs b/src/Bifrost.Web/Data/Migrations/20261111110000_AddMembershipSuspensionReason.cs new file mode 100644 index 0000000..4dc7719 --- /dev/null +++ b/src/Bifrost.Web/Data/Migrations/20261111110000_AddMembershipSuspensionReason.cs @@ -0,0 +1,25 @@ +using Bifrost.Web.Data; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace Bifrost.Web.Data.Migrations +{ + // Hand-written like ConsentReceiptsOneLivePerVersion: no dotnet-ef on the authoring host; the snapshot was edited to match. + // Reasons already written to AuditEvent.Detail stay there as history; new ones live only in this admin-read column. + [DbContext(typeof(BifrostDbContext))] + [Migration("20261111110000_AddMembershipSuspensionReason")] + public partial class AddMembershipSuspensionReason : Migration + { + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.AddColumn(name: "SuspensionReason", table: "Memberships", type: "text", nullable: true); + } + + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropColumn(name: "SuspensionReason", table: "Memberships"); + } + } +} diff --git a/src/Bifrost.Web/Data/Migrations/BifrostDbContextModelSnapshot.cs b/src/Bifrost.Web/Data/Migrations/BifrostDbContextModelSnapshot.cs index 851318c..042e2f0 100644 --- a/src/Bifrost.Web/Data/Migrations/BifrostDbContextModelSnapshot.cs +++ b/src/Bifrost.Web/Data/Migrations/BifrostDbContextModelSnapshot.cs @@ -821,6 +821,9 @@ namespace Bifrost.Web.Data.Migrations .IsRequired() .HasColumnType("text"); + b.Property("SuspensionReason") + .HasColumnType("text"); + b.Property("UserAccountId") .HasColumnType("uuid"); diff --git a/src/Bifrost.Web/Domain/BifrostModels.cs b/src/Bifrost.Web/Domain/BifrostModels.cs index 2e5d680..00e3f9c 100644 --- a/src/Bifrost.Web/Domain/BifrostModels.cs +++ b/src/Bifrost.Web/Domain/BifrostModels.cs @@ -334,6 +334,9 @@ public sealed class Membership public MembershipStatus Status { get; set; } = MembershipStatus.Authenticated; + /// Why an admin suspended the account; set and cleared only by MembershipSuspensionService and projected only to admins. + public string? SuspensionReason { get; set; } + public bool IsPlatformAdmin { get; set; } public bool CanManageProjects { get; set; } diff --git a/src/Bifrost.Web/Features/Membership/MembershipSuspensionService.cs b/src/Bifrost.Web/Features/Membership/MembershipSuspensionService.cs index cb5626c..870ca4f 100644 --- a/src/Bifrost.Web/Features/Membership/MembershipSuspensionService.cs +++ b/src/Bifrost.Web/Features/Membership/MembershipSuspensionService.cs @@ -17,7 +17,7 @@ public enum SuspensionOutcome /// /// The one place a membership becomes Suspended or stops being. Only a platform admin may call it, every use leaves an audit -/// receipt naming the admin, the account and the reason, and no sign-in path writes or lifts a suspension (Heimdall and GitHub +/// receipt naming the admin and the account (the reason is kept on the membership row, readable by admins only), and no sign-in path writes or lifts a suspension (Heimdall and GitHub /// sign-in leave a Suspended membership as it is). Unsuspending restores Active for an account that was once approved, /// Authenticated for one that never was; a pending request is not remembered. /// @@ -65,14 +65,16 @@ public sealed class MembershipSuspensionService(BifrostDbContext dbContext, Audi membership.Status = suspend ? MembershipStatus.Suspended : membership.ApprovedAtUtc is null ? MembershipStatus.Authenticated : MembershipStatus.Active; + // The reason is a column on the membership, for admins; the receipt names who and what changed, never why. var clipped = reason.Trim().Length > MaxReasonLength ? reason.Trim()[..MaxReasonLength] : reason.Trim(); - // One save: the status change and its receipt stand or fall together (RecordAsync saves the shared context). + membership.SuspensionReason = suspend ? clipped : null; + // One save: the status change, the reason and the receipt stand or fall together (RecordAsync saves the shared context). await auditTrailService.RecordAsync( actor.UserAccount.Id, nameof(Membership), membership.Id, suspend ? "membership.suspended" : "membership.unsuspended", - $"{(suspend ? "Suspended" : "Unsuspended")} {user.DisplayName} ({before} -> {membership.Status}): {clipped}", + $"{(suspend ? "Suspended" : "Unsuspended")} {user.DisplayName} ({before} -> {membership.Status}).", cancellationToken); return SuspensionOutcome.Done; } diff --git a/src/Bifrost.Web/Pages/App/Members/Index.cshtml b/src/Bifrost.Web/Pages/App/Members/Index.cshtml index 91f04c4..94e791d 100644 --- a/src/Bifrost.Web/Pages/App/Members/Index.cshtml +++ b/src/Bifrost.Web/Pages/App/Members/Index.cshtml @@ -191,6 +191,10 @@ @if (Model.Actor.CanManageMembership) { + @if (user.Status == MembershipStatus.Suspended && !string.IsNullOrEmpty(user.SuspensionReason)) + { +
Suspended: @user.SuspensionReason
+ } diff --git a/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs b/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs index ba0715c..9a0a57a 100644 --- a/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs +++ b/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs @@ -359,6 +359,7 @@ public sealed class IndexModel( user.MemberProfile?.Skills ?? string.Empty, user.MemberProfile?.Availability ?? string.Empty, user.Membership?.Status ?? MembershipStatus.Authenticated, + Actor.CanManageMembership ? user.Membership?.SuspensionReason ?? string.Empty : string.Empty, user.Membership?.RoleAssignments.Select(x => x.Role).OrderBy(x => x.ToString()).ToArray() ?? [], patronTier?.Label ?? string.Empty, patronTier?.Weight ?? 0m, @@ -446,6 +447,7 @@ public sealed class IndexModel( string Skills, string Availability, MembershipStatus Status, + string SuspensionReason, IReadOnlyList Roles, string PatronTierLabel, decimal PatronTierWeight, diff --git a/tests/Bifrost.Web.Tests/MembershipSuspensionTests.cs b/tests/Bifrost.Web.Tests/MembershipSuspensionTests.cs index 85e12ca..97f0c99 100644 --- a/tests/Bifrost.Web.Tests/MembershipSuspensionTests.cs +++ b/tests/Bifrost.Web.Tests/MembershipSuspensionTests.cs @@ -57,6 +57,12 @@ public sealed class MembershipSuspensionTests : IClassFixture StatusAsync(Guid id) => InScopeAsync(async (_, db) => (await db.Memberships.AsNoTracking().SingleAsync(x => x.UserAccountId == id)).Status); + private Task ReasonAsync(Guid id) => InScopeAsync(async (_, db) => + (await db.Memberships.AsNoTracking().SingleAsync(x => x.UserAccountId == id)).SuspensionReason); + + private Task MembershipIdAsync(Guid id) => InScopeAsync(async (_, db) => + (await db.Memberships.AsNoTracking().SingleAsync(x => x.UserAccountId == id)).Id); + private Task> AuditAsync(string action) => InScopeAsync((_, db) => db.AuditEvents.AsNoTracking().Where(x => x.Action == action).ToListAsync()); @@ -78,9 +84,12 @@ public sealed class MembershipSuspensionTests : IClassFixture x.Detail.Contains("spam in chat")); + var membershipId = (await MembershipIdAsync(seed.MemberId)).ToString(); + var receipt = Assert.Single(await AuditAsync("membership.suspended"), x => x.EntityId == membershipId); Assert.Equal(seed.AdminId, receipt.ActorUserAccountId); Assert.Contains("Active -> Suspended", receipt.Detail); + Assert.DoesNotContain("spam in chat", receipt.Detail); + Assert.Equal("spam in chat", await ReasonAsync(seed.MemberId)); } [Fact] @@ -95,7 +104,7 @@ public sealed class MembershipSuspensionTests : IClassFixture await sp.GetRequiredService().GetAsync(seed.OrdinaryId, CancellationToken.None)); Assert.DoesNotContain(snapshot.RecentActivity, x => x.Detail.Contains("private-reason-canary")); - Assert.Contains(await AuditAsync("membership.suspended"), x => x.Detail.Contains("private-reason-canary")); + Assert.DoesNotContain(await AuditAsync("membership.suspended"), x => x.Detail.Contains("private-reason-canary")); + Assert.Equal("private-reason-canary", await ReasonAsync(seed.MemberId)); } [Fact] @@ -132,9 +142,16 @@ public sealed class MembershipSuspensionTests : IClassFixture + { + (await db.Memberships.SingleAsync(x => x.UserAccountId == seed.MemberId)).SuspensionReason = "old reason"; + await db.SaveChangesAsync(); + return 0; + }); Assert.Equal(SuspensionOutcome.Done, await UnsuspendAsync(seed.AdminId, seed.MemberId, "appeal upheld")); Assert.Equal(restored, await StatusAsync(seed.MemberId)); - Assert.Contains(await AuditAsync("membership.unsuspended"), x => x.Detail.Contains("appeal upheld") && x.ActorUserAccountId == seed.AdminId); + Assert.Contains(await AuditAsync("membership.unsuspended"), x => x.ActorUserAccountId == seed.AdminId && !x.Detail.Contains("appeal upheld")); + Assert.Null(await ReasonAsync(seed.MemberId)); Assert.Equal(SuspensionOutcome.NotApplicable, await UnsuspendAsync(seed.AdminId, seed.MemberId, "again")); } diff --git a/tests/Bifrost.Web.Tests/MigrationDiscoveryTests.cs b/tests/Bifrost.Web.Tests/MigrationDiscoveryTests.cs index 1fc57d2..012b061 100644 --- a/tests/Bifrost.Web.Tests/MigrationDiscoveryTests.cs +++ b/tests/Bifrost.Web.Tests/MigrationDiscoveryTests.cs @@ -26,6 +26,7 @@ public sealed class MigrationDiscoveryTests "20261109210000_AddMindKeys", "20261111090000_AddConsentReceipts", "20261111100000_ConsentReceiptsOneLivePerVersion", + "20261111110000_AddMembershipSuspensionReason", ]; // Database.GetMigrations() reads assembly metadata only; no connection is opened. @@ -55,7 +56,7 @@ public sealed class MigrationDiscoveryTests } [Fact] - public void All_sixteen_migrations_are_listed_in_order() + public void All_migrations_are_listed_in_order() { Assert.Equal(ExpectedMigrations, Discovered()); } diff --git a/tests/Bifrost.Web.Tests/Support/OidcLogoutKit.cs b/tests/Bifrost.Web.Tests/Support/OidcLogoutKit.cs index 582fa04..1ef58c0 100644 --- a/tests/Bifrost.Web.Tests/Support/OidcLogoutKit.cs +++ b/tests/Bifrost.Web.Tests/Support/OidcLogoutKit.cs @@ -359,6 +359,11 @@ public sealed class OidcLogoutFactory : OidcWebApplicationFactory public FakeMasHandler Mas { get; } = new(); + /// The raw token response and access token of the last . + public string LastTokenResponse { get; private set; } = string.Empty; + + public string LastAccessToken { get; private set; } = string.Empty; + public ManualClock Time { get; } = new(); public OidcLogoutReconciler Reconciler => Services.GetServices().OfType().Single(); @@ -399,7 +404,7 @@ public sealed class OidcLogoutFactory : OidcWebApplicationFactory }; /// Registers a member, sets , and completes one authorization-code flow for them. - public async Task<(string Identity, string IdToken)> GrantAsync(MembershipStatus status = MembershipStatus.Active) + public async Task<(string Identity, string IdToken)> GrantAsync(MembershipStatus status = MembershipStatus.Active, string? suspensionReason = null) { using var client = CreateClient(HostClientOptions()); var identity = $"member-{Guid.NewGuid():N}"; @@ -409,6 +414,14 @@ public sealed class OidcLogoutFactory : OidcWebApplicationFactory } await SetMembershipAsync(identity, MembershipStatus.Active); + if (suspensionReason is not null) + { + using var scope = Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var account = db.UserAccounts.Single(x => x.NormalizedBifrostIdentity == identity); + db.Memberships.Single(x => x.UserAccountId == account.Id).SuspensionReason = suspensionReason; + await db.SaveChangesAsync(); + } var verifier = Base64UrlEncoder.Encode(RandomNumberGenerator.GetBytes(48)); var state = Base64UrlEncoder.Encode(RandomNumberGenerator.GetBytes(24)); @@ -444,7 +457,10 @@ public sealed class OidcLogoutFactory : OidcWebApplicationFactory "Basic", Convert.ToBase64String(Encoding.ASCII.GetBytes($"{OidcTestKit.ClientId}:{Kit.ClientSecret}"))); using var tokens = await bare.SendAsync(exchange); Assert.Equal(HttpStatusCode.OK, tokens.StatusCode); - var idToken = (await tokens.Content.ReadFromJsonAsync()).GetProperty("id_token").GetString()!; + var body = await tokens.Content.ReadFromJsonAsync(); + var idToken = body.GetProperty("id_token").GetString()!; + LastTokenResponse = body.GetRawText(); + LastAccessToken = body.GetProperty("access_token").GetString()!; if (status != MembershipStatus.Active) { diff --git a/tests/Bifrost.Web.Tests/SuspensionReasonExposureTests.cs b/tests/Bifrost.Web.Tests/SuspensionReasonExposureTests.cs new file mode 100644 index 0000000..f4f5d30 --- /dev/null +++ b/tests/Bifrost.Web.Tests/SuspensionReasonExposureTests.cs @@ -0,0 +1,84 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Text; +using Microsoft.IdentityModel.JsonWebTokens; +using Microsoft.IdentityModel.Tokens; +using Bifrost.Web.Data; +using Bifrost.Web.Domain; +using Bifrost.Web.Tests.Support; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; + +namespace Bifrost.Web.Tests; + +/// The suspension reason is for admins: no member-facing page or the OIDC token and userinfo output carries it. +public sealed class SuspensionReasonExposureTests +{ + private const string Canary = "REASON-CANARY-9d2f-do-not-leak"; + + private static async Task SeedAsync(IServiceProvider services, string identity, MembershipStatus status, string? reason) + { + using var scope = services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + db.UserAccounts.Add(new UserAccount + { + DisplayName = identity, BifrostIdentity = identity, NormalizedBifrostIdentity = identity, + Membership = new Bifrost.Web.Domain.Membership { Status = status, ApprovedAtUtc = DateTimeOffset.UtcNow, SuspensionReason = reason } + }); + await db.SaveChangesAsync(); + } + + private static async Task GetAsync(HttpClient client, string path, string? identity) + { + using var request = new HttpRequestMessage(HttpMethod.Get, path); + request.Headers.Add("X-Test-Bifrost-Identity", identity); + using var response = await client.SendAsync(request); + Assert.True(response.StatusCode == HttpStatusCode.OK, path); + return await response.Content.ReadAsStringAsync(); + } + + [Fact] + public async Task Member_facing_pages_never_carry_the_reason_and_the_admin_members_page_does() + { + using var factory = new TestWebApplicationFactory(); + using var client = factory.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false, BaseAddress = new Uri("https://localhost") }); + await SeedAsync(factory.Services, "suspendedone", MembershipStatus.Suspended, Canary); + await SeedAsync(factory.Services, "ordinaryone", MembershipStatus.Active, null); + + foreach (var path in new[] { "/App", "/App/Members", "/Membership/Status" }) + { + Assert.DoesNotContain(Canary, await GetAsync(client, path, "ordinaryone")); + } + + Assert.DoesNotContain(Canary, await GetAsync(client, "/Membership/Status", "suspendedone")); + + using var admin = factory.CreateClient(new WebApplicationFactoryClientOptions { BaseAddress = new Uri("https://localhost") }); + Assert.Contains(Canary, await admin.GetStringAsync("/App/Members")); // the default test sign-in is a bootstrap admin + } + + [Fact] + public async Task The_OIDC_token_response_and_id_token_never_carry_the_reason() + { + using var factory = new OidcLogoutFactory(); + var (_, idToken) = await factory.GrantAsync(suspensionReason: Canary); + + Assert.DoesNotContain(Canary, factory.LastTokenResponse); + var payload = new JsonWebToken(idToken).EncodedPayload; + Assert.DoesNotContain(Canary, Encoding.UTF8.GetString(Base64UrlEncoder.DecodeBytes(payload))); + } + + // Userinfo is broken in some environments before this change (OidcIssuerTests' userinfo cases fail identically there). + [Fact] + public async Task OIDC_userinfo_never_carries_the_reason() + { + using var factory = new OidcLogoutFactory(); + _ = await factory.GrantAsync(suspensionReason: Canary); + + using var client = factory.CreateClient(factory.HostClientOptions()); + client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", factory.LastAccessToken); + using var userinfo = await client.GetAsync("/connect/userinfo"); + Assert.Equal(HttpStatusCode.OK, userinfo.StatusCode); + Assert.DoesNotContain(Canary, await userinfo.Content.ReadAsStringAsync()); + } +} -- 2.47.3 From d91d4a3da61c3796621ac94aa7c6489c69c3ed39 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 12:23:06 +0000 Subject: [PATCH 5/7] OIDC: warn once at startup that suspension cannot end live chat sessions without BackchannelLogoutUri Co-Authored-By: Claude Sonnet 5.5 --- docs/oidc-issuer.md | 4 ++ .../Features/Oidc/OidcClientSeeder.cs | 9 +++- .../OidcLogoutReachWarningTests.cs | 50 +++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 tests/Bifrost.Web.Tests/OidcLogoutReachWarningTests.cs diff --git a/docs/oidc-issuer.md b/docs/oidc-issuer.md index d6cb22d..b3c4121 100644 --- a/docs/oidc-issuer.md +++ b/docs/oidc-issuer.md @@ -103,6 +103,10 @@ to 15 minutes; a 400 (token refused) or 404 (unknown provider ULID) is a configu and retried at the 15 minute cap. Nothing is revoked until MAS answers 2xx. A restart loses the backoff and nothing else: the authorizations table is the queue, so a later-set URI also catches up on earlier demotions. +Without `BackchannelLogoutUri` none of this runs: suspension stops new sign-ins and token refresh (the authorize, token and userinfo +endpoints refuse a suspended account) but does not end a chat session that is already live, and the issuer logs one warning at +startup saying so. The other lever is MAS admin deactivation of the user (ops, not built here). + Tradeoff: delivery latency is up to one tick, and a member demoted and re-promoted inside one tick keeps their sessions. MAS verified at v1.28.0 (`crates/handlers/src/upstream_oauth2/backchannel_logout.rs`): the token is checked like an diff --git a/src/Bifrost.Web/Features/Oidc/OidcClientSeeder.cs b/src/Bifrost.Web/Features/Oidc/OidcClientSeeder.cs index edf1dd3..c393afd 100644 --- a/src/Bifrost.Web/Features/Oidc/OidcClientSeeder.cs +++ b/src/Bifrost.Web/Features/Oidc/OidcClientSeeder.cs @@ -14,10 +14,17 @@ public sealed class OidcClientSeeder( IServiceScopeFactory scopeFactory, OidcIssuerOptions options, OidcKeyMaterial material, - IOptions serverOptions) : IHostedService + IOptions serverOptions, + ILogger logger) : IHostedService { public async Task StartAsync(CancellationToken cancellationToken) { + if (string.IsNullOrWhiteSpace(options.Clients.Mas.BackchannelLogoutUri)) + { + logger.LogWarning( + "Bifrost:Oidc:Clients:Mas:BackchannelLogoutUri is not set: suspending an account stops new sign-ins and token refresh but does not end its existing chat sessions."); + } + // Reading the server options now runs their configuration, so a bad issuer stops startup rather than the first request. _ = serverOptions.Value; diff --git a/tests/Bifrost.Web.Tests/OidcLogoutReachWarningTests.cs b/tests/Bifrost.Web.Tests/OidcLogoutReachWarningTests.cs new file mode 100644 index 0000000..acd4cbe --- /dev/null +++ b/tests/Bifrost.Web.Tests/OidcLogoutReachWarningTests.cs @@ -0,0 +1,50 @@ +using Bifrost.Web.Tests.Support; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Bifrost.Web.Tests; + +/// Without a back-channel logout URL suspension cannot end live chat sessions, and the host says so once at startup. +public sealed class OidcLogoutReachWarningTests +{ + private const string Marker = "does not end its existing chat sessions"; + + private static int Warnings(Microsoft.AspNetCore.Mvc.Testing.WebApplicationFactory factory, CapturingLoggerProvider logs) + { + _ = factory.Services; // starts the host and its hosted services + return logs.Lines.Count(x => x.Contains(Marker)); + } + + private static Microsoft.AspNetCore.Mvc.Testing.WebApplicationFactory WithLogs(Microsoft.AspNetCore.Mvc.Testing.WebApplicationFactory factory, CapturingLoggerProvider logs) => + factory.WithWebHostBuilder(b => b.ConfigureServices(s => s.AddLogging(l => l.AddProvider(logs)))); + + [Fact] + public void OIDC_configured_without_the_logout_url_warns_exactly_once() + { + var logs = new CapturingLoggerProvider(); + using var plain = new OidcWebApplicationFactory(); + using var factory = WithLogs(plain, logs); + + Assert.Equal(1, Warnings(factory, logs)); + } + + [Fact] + public void OIDC_with_the_logout_url_does_not_warn() + { + var logs = new CapturingLoggerProvider(); + using var plain = new OidcLogoutFactory(); + using var factory = WithLogs(plain, logs); + + Assert.Equal(0, Warnings(factory, logs)); + } + + [Fact] + public void OIDC_unconfigured_does_not_warn() + { + var logs = new CapturingLoggerProvider(); + using var plain = new TestWebApplicationFactory(); + using var factory = WithLogs(plain, logs); + + Assert.Equal(0, Warnings(factory, logs)); + } +} -- 2.47.3 From 5a3d36ed1b4d2ee49339e8f2db1b8d61046932dc Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 14:10:30 +0000 Subject: [PATCH 6/7] Members page: approve/role refusals (suspended, no membership, own admin role) show a fixed notice; role toggle on a suspended account is refused like approve Co-Authored-By: Claude Sonnet 5.5 --- .../Pages/App/Members/Index.cshtml.cs | 25 +++++- .../MembersPageNoticeTests.cs | 78 ++++++++++++++++++- 2 files changed, 97 insertions(+), 6 deletions(-) diff --git a/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs b/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs index 9a0a57a..55fa91f 100644 --- a/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs +++ b/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs @@ -22,7 +22,7 @@ public sealed class IndexModel( [BindProperty] public UpdateProfileInput Profile { get; set; } = new(); - /// Why the last suspend or unsuspend did nothing; one of , carried across the redirect. + /// Why the last membership action did nothing; one of , carried across the redirect. [TempData] public string? Notice { get; set; } @@ -34,6 +34,11 @@ public sealed class IndexModel( [SuspensionOutcome.NotApplicable] = "Nothing changed: the account is already in that state." }; + internal const string ApproveSuspendedNotice = "Nothing changed: a suspended account can only be lifted with Unsuspend."; + internal const string NoMembershipNotice = "Nothing changed: that account has no membership yet."; + internal const string OwnAdminRoleNotice = "Nothing changed: you cannot remove or change your own platform admin role."; + internal const string RoleOnSuspendedNotice = "Nothing changed: a suspended account's roles cannot be changed until it is unsuspended."; + public CurrentBifrostActor Actor { get; private set; } = CurrentBifrostActor.Anonymous; public IReadOnlyList Users { get; private set; } = []; @@ -118,7 +123,8 @@ public sealed class IndexModel( if (user.Membership!.Status == MembershipStatus.Suspended) { - return RedirectToPage(); // only Unsuspend lifts a suspension + Notice = ApproveSuspendedNotice; // only Unsuspend lifts a suspension + return RedirectToPage(); } user.Membership.Status = MembershipStatus.Active; @@ -179,6 +185,19 @@ public sealed class IndexModel( if (user.Membership is null) { + Notice = NoMembershipNotice; + return RedirectToPage(); + } + + if (user.Id == Actor.UserAccount.Id && role == MemberRole.PlatformAdmin) + { + Notice = OwnAdminRoleNotice; + return RedirectToPage(); + } + + if (user.Membership.Status == MembershipStatus.Suspended) + { + Notice = RoleOnSuspendedNotice; return RedirectToPage(); } @@ -193,7 +212,7 @@ public sealed class IndexModel( Notes = "Assigned from the member admin page." }); } - else if (!(user.Id == Actor.UserAccount.Id && role == MemberRole.PlatformAdmin)) + else { // Out of the collection too, so the flags below derive from the roles as they will stand after the save. user.Membership.RoleAssignments.Remove(existingAssignment); diff --git a/tests/Bifrost.Web.Tests/MembersPageNoticeTests.cs b/tests/Bifrost.Web.Tests/MembersPageNoticeTests.cs index b84c3ce..b4261d8 100644 --- a/tests/Bifrost.Web.Tests/MembersPageNoticeTests.cs +++ b/tests/Bifrost.Web.Tests/MembersPageNoticeTests.cs @@ -19,7 +19,7 @@ public sealed class MembersPageNoticeTests(TestWebApplicationFactory factory) : HandleCookies = true }); - private async Task SeedMemberAsync(MembershipStatus status) + private async Task SeedMemberAsync(MembershipStatus status, bool withMembership = true) { using var scope = factory.Services.CreateScope(); var db = scope.ServiceProvider.GetRequiredService(); @@ -27,7 +27,7 @@ public sealed class MembersPageNoticeTests(TestWebApplicationFactory factory) : var account = new UserAccount { DisplayName = id, BifrostIdentity = id, NormalizedBifrostIdentity = id, - Membership = new Bifrost.Web.Domain.Membership { Status = status, ApprovedAtUtc = DateTimeOffset.UtcNow } + Membership = withMembership ? new Bifrost.Web.Domain.Membership { Status = status, ApprovedAtUtc = DateTimeOffset.UtcNow } : null }; db.UserAccounts.Add(account); await db.SaveChangesAsync(); @@ -47,7 +47,7 @@ public sealed class MembersPageNoticeTests(TestWebApplicationFactory factory) : return Regex.Match(html, "name=\"__RequestVerificationToken\"[^>]*value=\"([^\"]+)\"").Groups[1].Value; } - private static async Task PostThenFollowAsync(HttpClient client, string handler, Guid target, string reason) + private static async Task PostThenFollowAsync(HttpClient client, string handler, Guid target, string reason, string? role = null) { var token = await TokenAsync(client); Assert.NotEmpty(token); @@ -55,6 +55,7 @@ public sealed class MembersPageNoticeTests(TestWebApplicationFactory factory) : { ["userAccountId"] = target.ToString(), ["reason"] = reason, + ["role"] = role ?? "Producer", ["__RequestVerificationToken"] = token })); Assert.Equal(HttpStatusCode.Redirect, post.StatusCode); @@ -127,4 +128,75 @@ public sealed class MembersPageNoticeTests(TestWebApplicationFactory factory) : Assert.DoesNotContain("role=\"alert\"", html); } + + private async Task<(MembershipStatus Status, int Roles)> StateAsync(Guid user) + { + using var scope = factory.Services.CreateScope(); + var m = await scope.ServiceProvider.GetRequiredService().Memberships.AsNoTracking().Include(x => x.RoleAssignments).SingleAsync(x => x.UserAccountId == user); + return (m.Status, m.RoleAssignments.Count); + } + + [Fact] + public async Task Approving_a_suspended_account_says_only_unsuspend_lifts_it_and_leaves_it_suspended() + { + using var client = Client(); + _ = await SignedInAdminIdAsync(client); + var member = await SeedMemberAsync(MembershipStatus.Suspended); + + var html = await PostThenFollowAsync(client, "Approve", member, ""); + + Assert.Contains("only be lifted with Unsuspend", Alert.Match(html).Groups[1].Value); + Assert.Equal(MembershipStatus.Suspended, (await StateAsync(member)).Status); + } + + [Fact] + public async Task Toggling_a_role_on_a_suspended_account_is_refused_with_a_notice_and_changes_nothing() + { + using var client = Client(); + _ = await SignedInAdminIdAsync(client); + var member = await SeedMemberAsync(MembershipStatus.Suspended); + + var html = await PostThenFollowAsync(client, "ToggleRole", member, "", role: "Producer"); + + Assert.Contains("roles cannot be changed until it is unsuspended", Alert.Match(html).Groups[1].Value); + Assert.Equal(0, (await StateAsync(member)).Roles); + } + + [Fact] + public async Task Toggling_a_role_on_an_account_without_a_membership_says_so() + { + using var client = Client(); + _ = await SignedInAdminIdAsync(client); + var member = await SeedMemberAsync(MembershipStatus.Active, withMembership: false); + + var html = await PostThenFollowAsync(client, "ToggleRole", member, "", role: "Producer"); + + Assert.Contains("has no membership yet", Alert.Match(html).Groups[1].Value); + } + + [Fact] + public async Task An_admin_toggling_their_own_platform_admin_role_is_refused_and_keeps_the_role() + { + using var client = Client(); + var admin = await SignedInAdminIdAsync(client); + var before = (await StateAsync(admin)).Roles; + + var html = await PostThenFollowAsync(client, "ToggleRole", admin, "", role: "PlatformAdmin"); + + Assert.Contains("cannot remove or change your own platform admin role", Alert.Match(html).Groups[1].Value); + Assert.Equal(before, (await StateAsync(admin)).Roles); + } + + [Fact] + public async Task A_role_toggle_on_an_active_account_still_works_without_a_refusal() + { + using var client = Client(); + _ = await SignedInAdminIdAsync(client); + var member = await SeedMemberAsync(MembershipStatus.Active); + + var html = await PostThenFollowAsync(client, "ToggleRole", member, "", role: "Producer"); + + Assert.DoesNotContain("role=\"alert\"", html); + Assert.True((await StateAsync(member)).Roles >= 1); + } } -- 2.47.3 From f1fb886f719c9519eca8a9ba48a2c57d793a3baf Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 14:10:30 +0000 Subject: [PATCH 7/7] Member dashboard hides the bootstrap-promotion receipt; page-model test guards the reason projection on its own Co-Authored-By: Claude Sonnet 5.5 --- .../Shared/DashboardSnapshotService.cs | 2 +- .../SuspensionReasonExposureTests.cs | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/Bifrost.Web/Features/Shared/DashboardSnapshotService.cs b/src/Bifrost.Web/Features/Shared/DashboardSnapshotService.cs index cd0d8f2..0837f7e 100644 --- a/src/Bifrost.Web/Features/Shared/DashboardSnapshotService.cs +++ b/src/Bifrost.Web/Features/Shared/DashboardSnapshotService.cs @@ -92,7 +92,7 @@ public sealed class DashboardSnapshotService(BifrostDbContext dbContext, TimePro var recentActivity = await dbContext.AuditEvents .AsNoTracking() .Include(x => x.ActorUserAccount) - .Where(x => !x.Action.StartsWith("membership.suspended") && !x.Action.StartsWith("membership.unsuspended")) // a reason is for admins, not the member dashboard + .Where(x => !x.Action.StartsWith("membership.suspended") && !x.Action.StartsWith("membership.unsuspended") && x.Action != "membership.bootstrap-promoted") // suspension reasons and the bootstrap-admin promotion are for admins, not the member dashboard .OrderByDescending(x => x.OccurredAtUtc) .Take(8) .Select(x => new DashboardActivity( diff --git a/tests/Bifrost.Web.Tests/SuspensionReasonExposureTests.cs b/tests/Bifrost.Web.Tests/SuspensionReasonExposureTests.cs index f4f5d30..0921036 100644 --- a/tests/Bifrost.Web.Tests/SuspensionReasonExposureTests.cs +++ b/tests/Bifrost.Web.Tests/SuspensionReasonExposureTests.cs @@ -5,6 +5,8 @@ using Microsoft.IdentityModel.JsonWebTokens; using Microsoft.IdentityModel.Tokens; using Bifrost.Web.Data; using Bifrost.Web.Domain; +using Bifrost.Web.Features.Membership; +using Bifrost.Web.Features.Shared; using Bifrost.Web.Tests.Support; using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.EntityFrameworkCore; @@ -38,6 +40,54 @@ public sealed class SuspensionReasonExposureTests return await response.Content.ReadAsStringAsync(); } + private sealed class FixedActor(CurrentBifrostActor actor) : ICurrentBifrostActorAccessor + { + public Task GetAsync(CancellationToken cancellationToken) => Task.FromResult(actor); + } + + // Guards the projection on its own: the view's admin gate cannot be the only thing keeping the reason off a non-admin's page model. + [Fact] + public async Task The_members_page_model_carries_no_reason_for_a_non_admin_viewer_and_does_for_an_admin() + { + using var factory = new TestWebApplicationFactory(); + await SeedAsync(factory.Services, "suspendedtwo", MembershipStatus.Suspended, Canary); + using var scope = factory.Services.CreateScope(); + var sp = scope.ServiceProvider; + var db = sp.GetRequiredService(); + var viewer = await db.UserAccounts.Include(x => x.Membership!).ThenInclude(x => x.RoleAssignments).SingleAsync(x => x.BifrostIdentity == "suspendedtwo"); + + async Task> UsersSeenBy(bool admin) + { + viewer.Membership!.IsPlatformAdmin = admin; + var model = new Bifrost.Web.Pages.App.Members.IndexModel(db, new FixedActor(new CurrentBifrostActor(true, viewer)), + sp.GetRequiredService(), sp.GetRequiredService(), TimeProvider.System); + await model.OnGetAsync(CancellationToken.None); + return model.Users; + } + + Assert.All(await UsersSeenBy(admin: false), u => Assert.DoesNotContain(Canary, u.SuspensionReason)); + Assert.Contains(await UsersSeenBy(admin: true), u => u.SuspensionReason == Canary); + } + + [Fact] + public async Task The_member_dashboard_hides_the_bootstrap_promotion_receipt_but_shows_ordinary_activity() + { + using var factory = new TestWebApplicationFactory(); + using (var scope = factory.Services.CreateScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + db.AuditEvents.Add(new AuditEvent { Action = "membership.bootstrap-promoted", Detail = "PROMO-CANARY-41", EntityType = "Membership", EntityId = "x", OccurredAtUtc = DateTimeOffset.UtcNow }); + db.AuditEvents.Add(new AuditEvent { Action = "membership.approved", Detail = "ORDINARY-CANARY-41", EntityType = "Membership", EntityId = "x", OccurredAtUtc = DateTimeOffset.UtcNow.AddMinutes(-1) }); + await db.SaveChangesAsync(); + } + + using var client = factory.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false, BaseAddress = new Uri("https://localhost") }); + var html = await client.GetStringAsync("/App"); + + Assert.Contains("ORDINARY-CANARY-41", html); + Assert.DoesNotContain("PROMO-CANARY-41", html); + } + [Fact] public async Task Member_facing_pages_never_carry_the_reason_and_the_admin_members_page_does() { -- 2.47.3