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); + } +}