Suspension survives sign-in; admin-only Suspend/Unsuspend with a receipt #53
7 changed files with 290 additions and 4 deletions
|
|
@ -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!,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
public sealed class MembershipSuspensionService(BifrostDbContext dbContext, AuditTrailService auditTrailService, TimeProvider timeProvider)
|
||||
{
|
||||
public const int MaxReasonLength = 500;
|
||||
|
||||
public Task<SuspensionOutcome> SuspendAsync(CurrentBifrostActor actor, Guid userAccountId, string? reason, CancellationToken cancellationToken) =>
|
||||
ChangeAsync(actor, userAccountId, reason, suspend: true, cancellationToken);
|
||||
|
||||
public Task<SuspensionOutcome> UnsuspendAsync(CurrentBifrostActor actor, Guid userAccountId, string? reason, CancellationToken cancellationToken) =>
|
||||
ChangeAsync(actor, userAccountId, reason, suspend: false, cancellationToken);
|
||||
|
||||
private async Task<SuspensionOutcome> 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;
|
||||
}
|
||||
}
|
||||
|
|
@ -176,7 +176,7 @@
|
|||
</div>
|
||||
</td>
|
||||
<td>
|
||||
@if (Model.Actor.CanManageMembership && user.Status != MembershipStatus.Active)
|
||||
@if (Model.Actor.CanManageMembership && user.Status != MembershipStatus.Active && user.Status != MembershipStatus.Suspended)
|
||||
{
|
||||
<form method="post" asp-page-handler="Approve">
|
||||
<input type="hidden" name="userAccountId" value="@user.UserAccountId" />
|
||||
|
|
@ -184,6 +184,15 @@
|
|||
</form>
|
||||
}
|
||||
|
||||
@if (Model.Actor.CanManageMembership)
|
||||
{
|
||||
<form method="post" asp-page-handler="@(user.Status == MembershipStatus.Suspended ? "Unsuspend" : "Suspend")" style="margin-top: 0.8rem;">
|
||||
<input type="hidden" name="userAccountId" value="@user.UserAccountId" />
|
||||
<input type="text" name="reason" maxlength="500" required placeholder="Reason" />
|
||||
<button class="button warm" type="submit">@(user.Status == MembershipStatus.Suspended ? "Unsuspend" : "Suspend")</button>
|
||||
</form>
|
||||
}
|
||||
|
||||
@if (Model.Actor.CanManageMembership)
|
||||
{
|
||||
<div class="stack-form" style="margin-top: 0.8rem;">
|
||||
|
|
|
|||
|
|
@ -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<IActionResult> OnPostSuspendAsync(Guid userAccountId, string? reason, CancellationToken cancellationToken) =>
|
||||
await ChangeSuspensionAsync(userAccountId, reason, suspend: true, cancellationToken);
|
||||
|
||||
public async Task<IActionResult> OnPostUnsuspendAsync(Guid userAccountId, string? reason, CancellationToken cancellationToken) =>
|
||||
await ChangeSuspensionAsync(userAccountId, reason, suspend: false, cancellationToken);
|
||||
|
||||
private async Task<IActionResult> 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<IActionResult> OnPostToggleRoleAsync(Guid userAccountId, MemberRole role, CancellationToken cancellationToken)
|
||||
{
|
||||
Actor = await actorAccessor.GetAsync(cancellationToken);
|
||||
|
|
|
|||
|
|
@ -117,6 +117,7 @@ builder.Services.AddHttpContextAccessor();
|
|||
builder.Services.AddSingleton(TimeProvider.System);
|
||||
builder.Services.AddScoped<ICurrentBifrostActorAccessor, CurrentBifrostActorAccessor>();
|
||||
builder.Services.AddScoped<AuditTrailService>();
|
||||
builder.Services.AddScoped<Bifrost.Web.Features.Membership.MembershipSuspensionService>();
|
||||
builder.Services.AddScoped<DashboardSnapshotService>();
|
||||
builder.Services.AddScoped<BridgeActionService>();
|
||||
builder.Services.AddScoped<AgentDispatchRunService>();
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
{
|
||||
|
|
|
|||
152
tests/Bifrost.Web.Tests/MembershipSuspensionTests.cs
Normal file
152
tests/Bifrost.Web.Tests/MembershipSuspensionTests.cs
Normal file
|
|
@ -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;
|
||||
|
||||
/// <summary>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.</summary>
|
||||
public sealed class MembershipSuspensionTests : IClassFixture<TestWebApplicationFactory>
|
||||
{
|
||||
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<Seeded> SeedAsync(MembershipStatus memberStatus = MembershipStatus.Active, bool approved = true)
|
||||
{
|
||||
using var scope = _factory.Services.CreateScope();
|
||||
var db = scope.ServiceProvider.GetRequiredService<BifrostDbContext>();
|
||||
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<T> InScopeAsync<T>(Func<IServiceProvider, BifrostDbContext, Task<T>> body)
|
||||
{
|
||||
using var scope = _factory.Services.CreateScope();
|
||||
return await body(scope.ServiceProvider, scope.ServiceProvider.GetRequiredService<BifrostDbContext>());
|
||||
}
|
||||
|
||||
private Task<CurrentBifrostActor> 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<MembershipStatus> StatusAsync(Guid id) => InScopeAsync(async (_, db) =>
|
||||
(await db.Memberships.AsNoTracking().SingleAsync(x => x.UserAccountId == id)).Status);
|
||||
|
||||
private Task<List<AuditEvent>> AuditAsync(string action) => InScopeAsync((_, db) =>
|
||||
db.AuditEvents.AsNoTracking().Where(x => x.Action == action).ToListAsync());
|
||||
|
||||
private Task<SuspensionOutcome> SuspendAsync(Guid by, Guid target, string? reason) => InScopeAsync(async (sp, _) =>
|
||||
await sp.GetRequiredService<MembershipSuspensionService>().SuspendAsync(await ActorAsync(by), target, reason, CancellationToken.None));
|
||||
|
||||
private Task<SuspensionOutcome> UnsuspendAsync(Guid by, Guid target, string? reason) => InScopeAsync(async (sp, _) =>
|
||||
await sp.GetRequiredService<MembershipSuspensionService>().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<AuditTrailService>(),
|
||||
sp.GetRequiredService<MembershipSuspensionService>(), TimeProvider.System);
|
||||
return await page.OnPostApproveAsync(seed.MemberId, CancellationToken.None);
|
||||
});
|
||||
|
||||
Assert.IsType<RedirectToPageResult>(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<AuditTrailService>(),
|
||||
sp.GetRequiredService<MembershipSuspensionService>(), TimeProvider.System);
|
||||
return await page.OnPostSuspendAsync(seed.MemberId, "nope", CancellationToken.None);
|
||||
});
|
||||
|
||||
Assert.IsType<ForbidResult>(result);
|
||||
Assert.Equal(MembershipStatus.Active, await StatusAsync(seed.MemberId));
|
||||
}
|
||||
|
||||
private sealed class FixedActor(CurrentBifrostActor actor) : ICurrentBifrostActorAccessor
|
||||
{
|
||||
public Task<CurrentBifrostActor> GetAsync(CancellationToken cancellationToken) => Task.FromResult(actor);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue