From f38ab061d32a82eb6de503a195613b2f7098bfdc Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 08:21:27 +0000 Subject: [PATCH] Suspension survives sign-in; admin-only Suspend/Unsuspend with a receipt Heimdall sign-in no longer lifts a Suspended membership when the member-access claim is present, the members page Approve cannot lift one, and MembershipSuspensionService (platform admin only, reason required, audit names admin and reason) is the only way to set or lift it. Co-Authored-By: Claude Sonnet 5.5 --- .../Features/Heimdall/HeimdallAuthService.cs | 5 +- .../Membership/MembershipSuspensionService.cs | 78 +++++++++ .../Pages/App/Members/Index.cshtml | 11 +- .../Pages/App/Members/Index.cshtml.cs | 24 ++- src/Bifrost.Web/Program.cs | 1 + .../HeimdallCompletionTests.cs | 23 +++ .../MembershipSuspensionTests.cs | 152 ++++++++++++++++++ 7 files changed, 290 insertions(+), 4 deletions(-) create mode 100644 src/Bifrost.Web/Features/Membership/MembershipSuspensionService.cs create mode 100644 tests/Bifrost.Web.Tests/MembershipSuspensionTests.cs diff --git a/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs b/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs index 0d39c2f..8d10b53 100644 --- a/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs +++ b/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs @@ -319,7 +319,8 @@ public sealed class HeimdallAuthService( UserAccountId = userAccount.Id, CreatedAtUtc = now }; - if (grantMemberAccess) + // A suspension is an admin's decision: the claim neither lifts it nor, absent, imposes it. + if (grantMemberAccess && userAccount.Membership.Status != MembershipStatus.Suspended) { userAccount.Membership.Status = MembershipStatus.Active; userAccount.Membership.ApprovedAtUtc ??= now; @@ -332,7 +333,7 @@ public sealed class HeimdallAuthService( userAccount, displayName, cancellationToken); - if (grantMemberAccess) + if (grantMemberAccess && userAccount.Membership!.Status == MembershipStatus.Active) { ApplicationBootstrapper.EnsureRole( userAccount.Membership!, diff --git a/src/Bifrost.Web/Features/Membership/MembershipSuspensionService.cs b/src/Bifrost.Web/Features/Membership/MembershipSuspensionService.cs new file mode 100644 index 0000000..e4bd388 --- /dev/null +++ b/src/Bifrost.Web/Features/Membership/MembershipSuspensionService.cs @@ -0,0 +1,78 @@ +using Bifrost.Web.Data; +using Bifrost.Web.Domain; +using Bifrost.Web.Features.Shared; +using Microsoft.EntityFrameworkCore; + +namespace Bifrost.Web.Features.Membership; + +public enum SuspensionOutcome +{ + Done, + NotAdmin, + NoReason, + NoSuchAccount, + NotApplicable, + ProtectedAccount +} + +/// +/// 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 +/// 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. +/// +public sealed class MembershipSuspensionService(BifrostDbContext dbContext, AuditTrailService auditTrailService, TimeProvider timeProvider) +{ + public const int MaxReasonLength = 500; + + public Task SuspendAsync(CurrentBifrostActor actor, Guid userAccountId, string? reason, CancellationToken cancellationToken) => + ChangeAsync(actor, userAccountId, reason, suspend: true, cancellationToken); + + public Task UnsuspendAsync(CurrentBifrostActor actor, Guid userAccountId, string? reason, CancellationToken cancellationToken) => + ChangeAsync(actor, userAccountId, reason, suspend: false, cancellationToken); + + private async Task ChangeAsync(CurrentBifrostActor actor, Guid userAccountId, string? reason, bool suspend, CancellationToken cancellationToken) + { + if (!actor.CanManageMembership || actor.UserAccount is null) + { + return SuspensionOutcome.NotAdmin; + } + + if (string.IsNullOrWhiteSpace(reason)) + { + return SuspensionOutcome.NoReason; + } + + var user = await dbContext.UserAccounts.Include(x => x.Membership).SingleOrDefaultAsync(x => x.Id == userAccountId, cancellationToken); + if (user?.Membership is not { } membership) + { + return SuspensionOutcome.NoSuchAccount; + } + + // An admin cannot suspend themselves or another platform admin here: bootstrap admins are re-promoted at sign-in by configuration. + if (suspend && (user.Id == actor.UserAccount.Id || membership.IsPlatformAdmin)) + { + return SuspensionOutcome.ProtectedAccount; + } + + if (suspend == (membership.Status == MembershipStatus.Suspended)) + { + return SuspensionOutcome.NotApplicable; + } + + var before = membership.Status; + membership.Status = suspend + ? MembershipStatus.Suspended + : membership.ApprovedAtUtc is null ? MembershipStatus.Authenticated : MembershipStatus.Active; + var clipped = reason.Trim().Length > MaxReasonLength ? reason.Trim()[..MaxReasonLength] : reason.Trim(); + await dbContext.SaveChangesAsync(cancellationToken); + await auditTrailService.RecordAsync( + actor.UserAccount.Id, + nameof(Membership), + membership.Id, + suspend ? "membership.suspended" : "membership.unsuspended", + $"{(suspend ? "Suspended" : "Unsuspended")} {user.DisplayName} ({before} -> {membership.Status}): {clipped}", + 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 0d9d546..b4d67e5 100644 --- a/src/Bifrost.Web/Pages/App/Members/Index.cshtml +++ b/src/Bifrost.Web/Pages/App/Members/Index.cshtml @@ -176,7 +176,7 @@ - @if (Model.Actor.CanManageMembership && user.Status != MembershipStatus.Active) + @if (Model.Actor.CanManageMembership && user.Status != MembershipStatus.Active && user.Status != MembershipStatus.Suspended) {
@@ -184,6 +184,15 @@
} + @if (Model.Actor.CanManageMembership) + { +
+ + + +
+ } + @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 2f385b7..e9b536b 100644 --- a/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs +++ b/src/Bifrost.Web/Pages/App/Members/Index.cshtml.cs @@ -1,6 +1,7 @@ using System.ComponentModel.DataAnnotations; using Bifrost.Web.Data; using Bifrost.Web.Domain; +using Bifrost.Web.Features.Membership; using Bifrost.Web.Features.Shared; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.RazorPages; @@ -12,6 +13,7 @@ public sealed class IndexModel( BifrostDbContext dbContext, ICurrentBifrostActorAccessor actorAccessor, AuditTrailService auditTrailService, + MembershipSuspensionService suspension, TimeProvider timeProvider) : PageModel { [BindProperty] @@ -102,7 +104,12 @@ public sealed class IndexModel( .ThenInclude(x => x.RoleAssignments) .SingleAsync(x => x.Id == userAccountId, cancellationToken); - user.Membership!.Status = MembershipStatus.Active; + if (user.Membership!.Status == MembershipStatus.Suspended) + { + return RedirectToPage(); // only Unsuspend lifts a suspension + } + + user.Membership.Status = MembershipStatus.Active; user.Membership.ApprovedAtUtc = timeProvider.GetUtcNow(); ApplicationBootstrapper.EnsureRole( user.Membership, @@ -124,6 +131,21 @@ public sealed class IndexModel( return RedirectToPage(); } + public async Task OnPostSuspendAsync(Guid userAccountId, string? reason, CancellationToken cancellationToken) => + await ChangeSuspensionAsync(userAccountId, reason, suspend: true, cancellationToken); + + public async Task OnPostUnsuspendAsync(Guid userAccountId, string? reason, CancellationToken cancellationToken) => + await ChangeSuspensionAsync(userAccountId, reason, suspend: false, cancellationToken); + + private async Task ChangeSuspensionAsync(Guid userAccountId, string? reason, bool suspend, CancellationToken cancellationToken) + { + Actor = await actorAccessor.GetAsync(cancellationToken); + var outcome = suspend + ? await suspension.SuspendAsync(Actor, userAccountId, reason, cancellationToken) + : await suspension.UnsuspendAsync(Actor, userAccountId, reason, cancellationToken); + return outcome == SuspensionOutcome.NotAdmin ? Forbid() : RedirectToPage(); + } + public async Task OnPostToggleRoleAsync(Guid userAccountId, MemberRole role, CancellationToken cancellationToken) { Actor = await actorAccessor.GetAsync(cancellationToken); diff --git a/src/Bifrost.Web/Program.cs b/src/Bifrost.Web/Program.cs index 2a5ff5f..e19b8d0 100644 --- a/src/Bifrost.Web/Program.cs +++ b/src/Bifrost.Web/Program.cs @@ -117,6 +117,7 @@ builder.Services.AddHttpContextAccessor(); builder.Services.AddSingleton(TimeProvider.System); builder.Services.AddScoped(); builder.Services.AddScoped(); +builder.Services.AddScoped(); builder.Services.AddScoped(); builder.Services.AddScoped(); builder.Services.AddScoped(); diff --git a/tests/Bifrost.Web.Tests/HeimdallCompletionTests.cs b/tests/Bifrost.Web.Tests/HeimdallCompletionTests.cs index 532f669..15866d1 100644 --- a/tests/Bifrost.Web.Tests/HeimdallCompletionTests.cs +++ b/tests/Bifrost.Web.Tests/HeimdallCompletionTests.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Authentication.OAuth; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.AspNetCore.WebUtilities; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -864,6 +865,28 @@ public sealed class HeimdallCompletionTests } } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task A_suspended_account_stays_suspended_through_a_sign_in_with_or_without_the_member_claim(bool asksForMemberAccess) + { + using var host = new HeimdallHost(); + var accountId = HeimdallShapes.NewAccountId(); + Assert.True((await Genuine(host, await host.StartAsync(member: true), accountId, ["member_access"])).SignedIn); + await host.Db(async db => + { + var account = await db.UserAccounts.Include(x => x.Membership).SingleAsync(x => x.HeimdallAccountId == accountId); + account.Membership!.Status = MembershipStatus.Suspended; + await db.SaveChangesAsync(); + return 0; + }); + + var again = await Genuine(host, await host.StartAsync(member: asksForMemberAccess), accountId, ["member_access"]); + + Assert.True(again.SignedIn); + Assert.Equal(MembershipStatus.Suspended, (await host.Account(accountId))!.Membership!.Status); + } + [Fact] public async Task A_token_capability_grants_no_role_to_a_sign_in_that_did_not_ask_for_member_access() { diff --git a/tests/Bifrost.Web.Tests/MembershipSuspensionTests.cs b/tests/Bifrost.Web.Tests/MembershipSuspensionTests.cs new file mode 100644 index 0000000..167d7ec --- /dev/null +++ b/tests/Bifrost.Web.Tests/MembershipSuspensionTests.cs @@ -0,0 +1,152 @@ +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; + +namespace Bifrost.Web.Tests; + +/// A suspension is an admin's decision: only the admin path sets or lifts it, with a receipt, and the members page's Approve cannot lift it. +public sealed class MembershipSuspensionTests : IClassFixture +{ + private readonly TestWebApplicationFactory _factory; + + public MembershipSuspensionTests(TestWebApplicationFactory factory) => _factory = factory; + + private sealed record Seeded(Guid AdminId, Guid MemberId, Guid OrdinaryId, Guid OtherAdminId); + + private async Task SeedAsync(MembershipStatus memberStatus = MembershipStatus.Active, bool approved = true) + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + UserAccount Account(string name, MembershipStatus status, bool admin, bool wasApproved) => new() + { + DisplayName = name, + BifrostIdentity = $"{name}-{Guid.NewGuid():N}"[..20], + NormalizedBifrostIdentity = $"{name}-{Guid.NewGuid():N}"[..20], + Membership = new Bifrost.Web.Domain.Membership + { + Status = status, + IsPlatformAdmin = admin, + ApprovedAtUtc = wasApproved ? DateTimeOffset.UtcNow : null + } + }; + + var admin = Account("admin", MembershipStatus.Active, true, true); + var otherAdmin = Account("admin2", MembershipStatus.Active, true, true); + var member = Account("member", memberStatus, false, approved); + var ordinary = Account("ordinary", MembershipStatus.Active, false, true); + db.UserAccounts.AddRange(admin, otherAdmin, member, ordinary); + await db.SaveChangesAsync(); + return new Seeded(admin.Id, member.Id, ordinary.Id, otherAdmin.Id); + } + + private async Task InScopeAsync(Func> body) + { + using var scope = _factory.Services.CreateScope(); + return await body(scope.ServiceProvider, scope.ServiceProvider.GetRequiredService()); + } + + private Task ActorAsync(Guid id) => InScopeAsync(async (_, db) => + new CurrentBifrostActor(true, await db.UserAccounts.Include(x => x.Membership!).ThenInclude(x => x.RoleAssignments).SingleAsync(x => x.Id == id))); + + private Task StatusAsync(Guid id) => InScopeAsync(async (_, db) => + (await db.Memberships.AsNoTracking().SingleAsync(x => x.UserAccountId == id)).Status); + + private Task> AuditAsync(string action) => InScopeAsync((_, db) => + db.AuditEvents.AsNoTracking().Where(x => x.Action == action).ToListAsync()); + + private Task SuspendAsync(Guid by, Guid target, string? reason) => InScopeAsync(async (sp, _) => + await sp.GetRequiredService().SuspendAsync(await ActorAsync(by), target, reason, CancellationToken.None)); + + private Task UnsuspendAsync(Guid by, Guid target, string? reason) => InScopeAsync(async (sp, _) => + await sp.GetRequiredService().UnsuspendAsync(await ActorAsync(by), target, reason, CancellationToken.None)); + + [Fact] + public async Task Only_a_platform_admin_can_suspend_and_the_receipt_names_who_and_why() + { + var seed = await SeedAsync(); + + Assert.Equal(SuspensionOutcome.NotAdmin, await SuspendAsync(seed.OrdinaryId, seed.MemberId, "because")); + Assert.Equal(MembershipStatus.Active, await StatusAsync(seed.MemberId)); + Assert.Equal(SuspensionOutcome.NoReason, await SuspendAsync(seed.AdminId, seed.MemberId, " ")); + Assert.Equal(MembershipStatus.Active, await StatusAsync(seed.MemberId)); + + Assert.Equal(SuspensionOutcome.Done, await SuspendAsync(seed.AdminId, seed.MemberId, "spam in chat")); + Assert.Equal(MembershipStatus.Suspended, await StatusAsync(seed.MemberId)); + var receipt = Assert.Single(await AuditAsync("membership.suspended"), x => x.Detail.Contains("spam in chat")); + Assert.Equal(seed.AdminId, receipt.ActorUserAccountId); + Assert.Contains("Active -> Suspended", receipt.Detail); + } + + [Fact] + public async Task An_admin_cannot_suspend_themselves_or_another_platform_admin() + { + var seed = await SeedAsync(); + + Assert.Equal(SuspensionOutcome.ProtectedAccount, await SuspendAsync(seed.AdminId, seed.AdminId, "oops")); + Assert.Equal(SuspensionOutcome.ProtectedAccount, await SuspendAsync(seed.AdminId, seed.OtherAdminId, "coup")); + Assert.Equal(MembershipStatus.Active, await StatusAsync(seed.AdminId)); + Assert.Equal(MembershipStatus.Active, await StatusAsync(seed.OtherAdminId)); + } + + [Theory] + [InlineData(true, MembershipStatus.Active)] + [InlineData(false, MembershipStatus.Authenticated)] + public async Task Unsuspend_works_only_by_the_admin_path_and_with_a_reason(bool approved, MembershipStatus restored) + { + var seed = await SeedAsync(MembershipStatus.Suspended, approved); + + Assert.Equal(SuspensionOutcome.NotAdmin, await UnsuspendAsync(seed.OrdinaryId, seed.MemberId, "please")); + Assert.Equal(SuspensionOutcome.NoReason, await UnsuspendAsync(seed.AdminId, seed.MemberId, null)); + Assert.Equal(MembershipStatus.Suspended, await StatusAsync(seed.MemberId)); + + 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.Equal(SuspensionOutcome.NotApplicable, await UnsuspendAsync(seed.AdminId, seed.MemberId, "again")); + } + + [Fact] + public async Task The_members_page_Approve_cannot_lift_a_suspension() + { + var seed = await SeedAsync(MembershipStatus.Suspended); + + var result = await InScopeAsync(async (sp, db) => + { + var page = new IndexModel( + db, new FixedActor(await ActorAsync(seed.AdminId)), sp.GetRequiredService(), + sp.GetRequiredService(), TimeProvider.System); + return await page.OnPostApproveAsync(seed.MemberId, CancellationToken.None); + }); + + Assert.IsType(result); + Assert.Equal(MembershipStatus.Suspended, await StatusAsync(seed.MemberId)); + } + + [Fact] + public async Task The_members_page_suspend_handler_forbids_a_non_admin_and_changes_nothing() + { + var seed = await SeedAsync(); + + var result = await InScopeAsync(async (sp, db) => + { + var page = new IndexModel( + db, new FixedActor(await ActorAsync(seed.OrdinaryId)), sp.GetRequiredService(), + sp.GetRequiredService(), TimeProvider.System); + return await page.OnPostSuspendAsync(seed.MemberId, "nope", CancellationToken.None); + }); + + Assert.IsType(result); + Assert.Equal(MembershipStatus.Active, await StatusAsync(seed.MemberId)); + } + + private sealed class FixedActor(CurrentBifrostActor actor) : ICurrentBifrostActorAccessor + { + public Task GetAsync(CancellationToken cancellationToken) => Task.FromResult(actor); + } +} -- 2.47.3