From 05925b1786c84bf091a6400b4e6e76be19d485e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20G=C3=B6pel?= Date: Sun, 19 Jul 2026 18:34:58 +0800 Subject: [PATCH] fix: guarantee root admin and Administrator role are non-deletable The first-run setup produced a *deletable* root admin and a *deletable* Administrator role, so an administrator could delete the main admin account and/or the Administrator role and orphan all admin access. Two layers, one defect: - Library: CreateAsync now forces Deletable=false for root users (mirroring the existing UpdateAsync guard) so the default User.Deletable=true can no longer mint a deletable root. EnsureAdministratorRoleAsync now hardens a pre-existing deletable Administrator role to non-deletable via a RoleChanged event, so the guarantee holds regardless of how the role first came to exist. - Sample: Setup.razor creates the first admin with RootUser=true and lets the library own the "un-deletable root administrator" guarantee (protected role creation + assignment + non-deletability) in one transaction. The manual role creation and AddToRole that left both deletable are removed. Tests: integration coverage for both invariants (root created non-deletable; existing deletable Administrator role hardened) and an E2E assertion that the /Setup-created admin surfaces as non-deletable. Closes #117 Closes #118 --- .../Components/Pages/Setup.razor | 32 ++++--------- .../Users/UserStore.cs | 30 +++++++++++- .../Tests/AdministrationTests.cs | 20 ++++++++ .../Users/UserStoreTests.cs | 46 +++++++++++++++++++ 4 files changed, 105 insertions(+), 23 deletions(-) diff --git a/samples/MartenIdentity.Aspire.Web/Components/Pages/Setup.razor b/samples/MartenIdentity.Aspire.Web/Components/Pages/Setup.razor index faa6acd..6e31881 100644 --- a/samples/MartenIdentity.Aspire.Web/Components/Pages/Setup.razor +++ b/samples/MartenIdentity.Aspire.Web/Components/Pages/Setup.razor @@ -10,7 +10,6 @@ @using Microsoft.AspNetCore.Identity @inject UserManager UserManager -@inject RoleManager RoleManager @inject IIdentityAuthorizer Authorizer @inject IQuerySession QuerySession @inject NavigationManager Nav @@ -90,22 +89,18 @@ busy = true; try { - // Trusted first-run bootstrap: BeginSystemScope lets the store operations that - // normally require an administrator (creating and assigning the role) run while - // no administrator exists yet. The scope is ambient across the awaits below. + // Trusted first-run bootstrap: BeginSystemScope lets the privileged store + // operations run while no administrator exists yet. The scope is ambient across + // the awaits below. using (Authorizer.BeginSystemScope()) { - if (!await RoleManager.RoleExistsAsync(Roles.Administrator)) - { - var roleResult = await RoleManager.CreateAsync(new Role { Name = Roles.Administrator }); - if (!roleResult.Succeeded) - { - AddIdentityErrors(roleResult); - return; - } - } - - var user = new User { UserName = model.Email, Email = model.Email }; + // Mark this account as the root user. The library then owns the whole + // "un-deletable root administrator" guarantee in one transaction: it forces + // the account non-deletable, creates the Administrator role as a protected, + // non-deletable role, and assigns it — so the first admin and the role that + // anchors it can never be deleted (#41). Do NOT create the role or assign it + // manually here; that historically left both deletable. + var user = new User { UserName = model.Email, Email = model.Email, RootUser = true }; var createResult = await UserManager.CreateAsync(user, model.Password); if (!createResult.Succeeded) { @@ -121,13 +116,6 @@ AddIdentityErrors(confirmResult); return; } - - var roleAssign = await UserManager.AddToRoleAsync(user, Roles.Administrator); - if (!roleAssign.Succeeded) - { - AddIdentityErrors(roleAssign); - return; - } } // Setup is complete. A full-page navigation lets the redirect middleware diff --git a/src/AndreGoepel.Marten.Identity/Users/UserStore.cs b/src/AndreGoepel.Marten.Identity/Users/UserStore.cs index 976c44f..e0a7e80 100644 --- a/src/AndreGoepel.Marten.Identity/Users/UserStore.cs +++ b/src/AndreGoepel.Marten.Identity/Users/UserStore.cs @@ -146,6 +146,14 @@ public async Task CreateAsync(TUser user, CancellationToken canc user.LockoutEnabled = !user.RootUser && identityOptions.Value.Lockout.AllowedForNewUsers; + // Domain-layer invariant (#41): the root user must be non-deletable so it can + // never be removed (which would orphan administration). User.Deletable defaults + // to true, so a host that sets RootUser but leaves Deletable at its default + // would otherwise mint a *deletable* root. Force it here, mirroring the same + // guard on the UpdateAsync path — the guarantee must not depend on host code. + if (user.RootUser) + user.Deletable = false; + using var session = documentStore.LightweightSession(); session.Events.Append( @@ -194,7 +202,10 @@ public async Task CreateAsync(TUser user, CancellationToken canc /// /// Returns the id of the (non-deleted) Administrator role, creating it as a /// protected, non-deletable role in the supplied session if it does not yet - /// exist. Used to guarantee the root user is always an administrator. + /// exist. If it already exists but is (incorrectly) deletable — e.g. a host + /// setup flow created it manually before assigning the root user — it is + /// hardened to non-deletable. Used to guarantee the root user is always an + /// administrator anchored by a role that can never be removed. /// private async Task EnsureAdministratorRoleAsync( IDocumentSession session, @@ -210,7 +221,24 @@ CancellationToken cancellationToken cancellationToken ); if (existing is not null) + { + // The Administrator role that anchors the root user must be non-deletable. + // A host that created it manually (as the sample setup flow historically did) + // leaves it at the default Deletable = true; upgrade it here so the guarantee + // holds regardless of how the role first came to exist (#41). + if (existing.Deletable) + { + session.Events.Append( + existing.RoleId.Value, + new RoleChanged(existing.RoleId, existing.Name!, createdBy) + { + Deletable = false, + } + ); + } + return existing.RoleId; + } var roleId = RoleId.New(); session.Events.Append( diff --git a/tests/AndreGoepel.Marten.Identity.E2ETests/Tests/AdministrationTests.cs b/tests/AndreGoepel.Marten.Identity.E2ETests/Tests/AdministrationTests.cs index eced507..3620c5e 100644 --- a/tests/AndreGoepel.Marten.Identity.E2ETests/Tests/AdministrationTests.cs +++ b/tests/AndreGoepel.Marten.Identity.E2ETests/Tests/AdministrationTests.cs @@ -21,6 +21,26 @@ await Expect(Page.Locator(".rz-data-grid").GetByText(TestData.AdminEmail)) .ToBeVisibleAsync(); } + [Fact] + public async Task Admin_RootAccount_IsNonDeletable() + { + // #41 / #117: the account created by the first-run /Setup flow is the root admin + // and must be non-deletable, so administration can never be orphaned. The Setup + // flow drives this via RootUser = true; here we assert it surfaces end-to-end as + // Deletable = "No" in the Users grid (ProvisionAdminAsync ran the real /Setup page). + // Arrange + await LoginAsAdminAsync(); + + // Act + await Page.GotoAsync("/Administration/Users"); + await Page.WaitForBlazorAsync(); + + // Assert — within the admin's own grid row, the Deletable column reads "No". + var adminRow = Page.Locator(".rz-data-grid tr") + .Filter(new() { HasText = TestData.AdminEmail }); + await Expect(adminRow.GetByText("No", new() { Exact = true })).ToBeVisibleAsync(); + } + [Fact] public async Task Admin_CanCreateRole_AppearsInGrid() { diff --git a/tests/AndreGoepel.Marten.Identity.IntegrationTests/Users/UserStoreTests.cs b/tests/AndreGoepel.Marten.Identity.IntegrationTests/Users/UserStoreTests.cs index 7f58d20..e6e599c 100644 --- a/tests/AndreGoepel.Marten.Identity.IntegrationTests/Users/UserStoreTests.cs +++ b/tests/AndreGoepel.Marten.Identity.IntegrationTests/Users/UserStoreTests.cs @@ -187,6 +187,52 @@ public async Task CreateAsync_RootUser_ReusesExistingAdministratorRole() Assert.Equal(1, await CountAdministratorRolesAsync()); } + [Fact] + public async Task CreateAsync_RootUser_IsNonDeletable() + { + // #41 domain-layer invariant: the root user must be created non-deletable so the + // admin anchor can never be removed. User.Deletable defaults to true, so a host + // that sets RootUser but leaves Deletable at its default must NOT mint a deletable + // root — CreateAsync forces it, mirroring the UpdateAsync guard. + // Arrange + var store = UserStoreTestHelpers.BuildStore(fixture.Store); + var user = UserStoreTestHelpers.NewUser(); + user.RootUser = true; // Deletable intentionally left at its default (true) + + // Act + var result = await store.CreateAsync(user, Ct); + + // Assert + Assert.True(result.Succeeded); + Assert.False(user.Deletable); + await using var session = fixture.Store.QuerySession(); + var persisted = await session.LoadAsync(user.UserId.Value, Ct); + Assert.False(persisted!.Deletable); + } + + [Fact] + public async Task CreateAsync_RootUser_HardensExistingDeletableAdministratorRole() + { + // If a host created the Administrator role manually first, it is deletable by + // default. Assigning the root user must harden it to non-deletable so the role + // that anchors administration can never be removed (#41). + // Arrange + var store = UserStoreTestHelpers.BuildStore(fixture.Store); + await SeedRoleAsync(RoleNames.Administrator); // RoleCreated defaults Deletable = true + var user = UserStoreTestHelpers.NewUser(); + user.RootUser = true; + + // Act + await store.CreateAsync(user, Ct); + + // Assert + await using var session = fixture.Store.QuerySession(); + var role = await session + .Query() + .SingleAsync(r => r.NormalizedName == RoleNames.Administrator.ToUpperInvariant(), Ct); + Assert.False(role.Deletable); + } + [Fact] public async Task CreateAsync_SecondRootUser_IsRejected() {