From 5918a1fd80777bb5e73bf80a453b7ba80fee4675 Mon Sep 17 00:00:00 2001 From: Bernhard Windisch Date: Wed, 9 Sep 2026 08:36:51 +0200 Subject: [PATCH 1/7] fix(provisioning): a binned user in the manifest no longer fails the whole apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user in the recycle bin keeps IsDeleted=false — that is what reserves their email for a restore — so the exporter still lists them and every manifest written afterwards carries them along. UpdateUserHandler refuses to edit a pending-deletion user, and because one failed op rolls the whole apply transaction back, a single binned user blocked EVERY later apply with User.DeletionPending. Staged deletes go through that same path, so it also blocked deleting any OTHER user — the error named a lifecycle state the admin had never touched for the user they were actually trying to delete. The upsert now skips such an entry instead of failing: the lifecycle state is left alone, the way back is a restore. The plan carries a matching note on update entries so an edit that will be skipped is visible beforehand, added after the unchanged→update promotion so an untouched binned user does not read as a pending change for the whole retention window. Co-Authored-By: Claude Opus 5 --- .../ColdStart/RealmManifestApplierTests.cs | 89 +++++++++++++++++++ .../Provisioning/RealmManifestApplier.cs | 16 ++++ .../Provisioning/RealmManifestPlanner.cs | 13 +++ 3 files changed, 118 insertions(+) diff --git a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs index 58fbb1a0..d51d2467 100644 --- a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs +++ b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs @@ -9,6 +9,7 @@ using Modgud.Application.DTOs.Realms; using Modgud.Application.Services; using Modgud.Authentication.Domain; +using Modgud.Authentication.Gdpr; using Modgud.Authorization.Apps; using Modgud.Authorization.Principals; using Modgud.Authorization.Roles; @@ -805,6 +806,94 @@ await perms.HasPermissionAsync(adminPerson.Id, AppSlugs.Modgud, PermissionEvalua }); } + /// + /// Regression: a user in the recycle bin keeps IsDeleted=false (that is what + /// reserves their email for a restore), so the exporter still lists them and every + /// manifest written afterwards carries them along. UpdateUserHandler refuses to edit a + /// pending-deletion user — and because one failed op rolls the WHOLE apply transaction + /// back, a single binned user used to block EVERY later apply with + /// User.DeletionPending, including the staged deletion of any OTHER user (staged + /// deletes apply through this same path). The entry must be skipped instead: the apply + /// succeeds, and the binned user's profile and lifecycle state are left untouched. + /// + [Fact] + public async Task A_binned_user_carried_in_the_manifest_is_skipped_instead_of_failing_the_apply() + { + await using var host = await Fixture.CreateIsolatedHostAsync(); + var factory = host.Factory; + var ct = TestContext.Current.CancellationToken; + var applier = factory.Services.GetRequiredService(); + var exporter = factory.Services.GetRequiredService(); + + const string slug = "binned"; + var full = new RealmManifest + { + Realm = new CreateRealmDto + { + Slug = slug, + DisplayName = "Binned", + Domains = [$"{slug}.localhost"], + InitialAdmin = new InitialAdminDto { UserName = "boot", Email = "boot@binned.test" }, + }, + Users = + [ + new RealmManifestUser { Key = "first", Email = "first@binned.test", UserName = "first", Firstname = "First", Password = "Passw0rd!23" }, + new RealmManifestUser { Key = "second", Email = "second@binned.test", UserName = "second", Firstname = "Second", Password = "Passw0rd!23" }, + ], + }; + var import = await applier.ImportNewRealmAsync(full, ct); + Assert.False(import.IsError, import.IsError ? import.FirstError.Description : string.Empty); + + // ── 1. The admin deletes "first" — the normal staged-delete path. ────────── + var binned = await applier.UpdateRealmAsync( + full with { Users = [full.Users[1]] }, + deletions: [new RealmDraftDeletion("users", "first")], + ct: ct); + Assert.False(binned.IsError, binned.IsError ? binned.FirstError.Description : string.Empty); + + // ── 2. The premise: the bin is reversible, so the export still lists them. ── + var export = await exporter.ExportRealmAsync(slug, ct); + Assert.False(export.IsError, export.IsError ? export.FirstError.Description : string.Empty); + Assert.Contains(export.Value.Users, u => u.UserName == "first"); + + // ── 3. Re-applying that export must NOT fail. The manifest even carries a + // changed profile for the binned user — read-only means it is skipped, + // not applied, so the stored Firstname survives untouched. ───────────── + var carried = export.Value with + { + Users = [.. export.Value.Users.Select(u => + u.UserName == "first" ? u with { Firstname = "Overwritten" } : u)], + }; + var reapplied = await applier.UpdateRealmAsync(carried, ct: ct); + Assert.False(reapplied.IsError, reapplied.IsError ? reapplied.FirstError.Description : string.Empty); + + // ── 4. The real-world symptom: deleting ANOTHER user while the first sits in + // the bin. This is what failed in production. ──────────────────────────── + var second = await applier.UpdateRealmAsync( + carried with { Users = [.. carried.Users.Where(u => u.UserName != "second")] }, + deletions: [new RealmDraftDeletion("users", "second")], + ct: ct); + Assert.False(second.IsError, second.IsError ? second.FirstError.Description : string.Empty); + + await InTenantAsync(factory, slug, async sp => + { + var session = sp.GetRequiredService(); + + var first = await session.Query().SingleAsync(p => p.AccountName == "first", ct); + Assert.Equal("First", first.Firstname); // skipped, not updated + Assert.False(first.IsDeleted, "the bin is reversible — IsDeleted stays false"); + var firstState = await session.LoadAsync(first.Id, ct); + Assert.True(firstState!.IsDeletionPending, "lifecycle state untouched by the apply"); + Assert.False((await session.LoadAsync(first.Id, ct))!.IsActive); + + // And the second delete really landed rather than silently no-opping. + var secondPerson = await session.Query().SingleAsync(p => p.AccountName == "second", ct); + var secondState = await session.LoadAsync(secondPerson.Id, ct); + Assert.True(secondState!.IsDeletionPending, "the other user was binned"); + Assert.False((await session.LoadAsync(secondPerson.Id, ct))!.IsActive); + }); + } + /// /// Builds the Globex manifest. 1 is the import baseline; /// version 2 changes every existing entity (display names, catalog, redirect, role diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs index 3ac3897d..1c7f64c7 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs @@ -16,6 +16,7 @@ using Modgud.Authentication.Applications; using Modgud.Authentication.Domain; using Modgud.Authentication.Domain.LoginProviders; +using Modgud.Authentication.Gdpr; using Modgud.Authentication.Identity.LoginProviders; using Modgud.Authentication.Identity.LoginProviders.Saml; using Modgud.Authentication.RealmSettings; @@ -718,6 +719,21 @@ r.Permissions is null && existing is not null EnsureOk(created, ctx); uid = ShortGuid.TryParse(created.Value.Id, out Guid cid) ? cid : null; } + // A user with a pending deletion (admin recycle bin OR self-service grace) is + // READ-ONLY: UpdateUserHandler rejects every edit until the user is restored. + // Such a user still has IsDeleted=false, so the exporter lists them and every + // later manifest carries them along — without this skip a single binned user + // would fail EVERY subsequent apply (the whole transaction rolls back), which + // also blocks deleting any OTHER user, because staged deletes apply through + // this same path. Leave the lifecycle state alone: the way back is a restore, + // then the next apply updates the profile again. + else if (await session.LoadAsync(existing.Id, ct) is { IsDeletionPending: true }) + { + logger.LogInformation( + "Manifest apply skipped {Context}: the user has a pending deletion and is read-only. " + + "Restore the user (or let the retention purge finish) to make it writable again.", ctx); + uid = existing.Id; + } else { // UpdateUserCommand mutates only the profile fields. Password / EmailConfirmed diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs index a29e5c3e..bc6eab52 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs @@ -12,6 +12,7 @@ using Modgud.Authorization.Roles; using Modgud.Authorization.Services; using Modgud.Authentication.Domain.LoginProviders; +using Modgud.Authentication.Gdpr; using Modgud.Domain.OAuth.Apis; using Modgud.Domain.OAuth.Applications; using Modgud.Domain.OAuth.Scopes; @@ -447,6 +448,18 @@ static bool IsTargeted(RealmManifestUser u, HashSet? keys) => entry.Notes.Add("EmailConfirmed is not changed on apply — the differing manifest value is ignored."); if ((entry.Notes.Count > 0 || entry.Conflicts.Count > 0) && entry.Action == "unchanged") entry = entry with { Action = "update" }; + // Mirrors the applier's read-only skip: a user with a pending deletion (recycle + // bin or self-service grace) cannot be edited, so an update entry for them is a + // no-op until they are restored. Deliberately added AFTER the promotion above — + // an otherwise untouched binned user must keep reading as "unchanged" rather + // than showing a pending change on every plan for the whole retention window. + if (entry.Action == "update" && existing is not null + && ShortGuid.TryParse(existing.Id, out Guid existingId) + && await session.LoadAsync(existingId, ct) is { IsDeletionPending: true }) + { + entry.Notes.Add("This user has a pending deletion and is read-only — the apply SKIPS this entry. " + + "Restore the user first, then re-apply."); + } section.Entries.Add(entry); } From 62aad3865d821931bb40c8d9aa93cb26a0632b1f Mon Sep 17 00:00:00 2001 From: Bernhard Windisch Date: Wed, 9 Sep 2026 09:27:55 +0200 Subject: [PATCH 2/7] refactor(provisioning)!: a manifest carries content, never a realm identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RealmManifest.Realm was dead weight that actively got in the way. The applier never touched the realm shell — the planner even emitted a warning saying so ("the realm shell is not modified by apply") — yet the field forced every export to name a realm, and forced whoever exported to decide, up front, where the file would eventually land. Slug, routing domains and primary domain are deployment identity; carrying them between environments is exactly what you do not want. The shell is gone from the manifest. The target realm now comes from the route alone, so one file applies unchanged to any realm, and the data-plane boundary stops being a guard and becomes structural: a realm admin cannot aim a manifest elsewhere because a manifest has no field to aim with. Manifest.SlugMismatch and the draft service's PinSlug are gone with it. Older files that still carry a "Realm" object stay loadable — the property is simply ignored. POST /api/admin/realms/import is retired. It only ever wrapped two steps that both already existed (create the realm, then apply), and it wrapped them badly: its "all-or-nothing" was not a transaction but a compensating hard-delete, so a typo in the manifest cost you the whole tenant database. Split, the apply runs in its own transaction and rolls back cleanly, leaving the realm intact to retry a corrected file against — a better failure model, not a worse one. Modgud.Provisioning.TestKit keeps its one-call convenience: ImportRealmAsync now takes the RealmSpec alongside the manifest and performs both calls, tearing the realm down itself if the manifest step fails (tests want isolation; the API no longer imposes it). Its error parser also learned the second error shape it now meets — POST /api/admin/realms answers { error: } while the manifest endpoints answer { Error: , Message: }, and reading the first as a code passed a description off as a machine-readable error. BREAKING: RealmManifest.Realm removed; POST /api/admin/realms/import removed; RealmManifestApplier.UpdateRealmAsync and RealmManifestPlanner.PlanAsync take the slug as their first argument; TestKit ImportRealmAsync takes a RealmSpec. The duplicate-slug error code on the create path is Realm.DuplicateSlug (the canonical one) rather than the import wrapper's Realm.AlreadyExists. Co-Authored-By: Claude Opus 5 --- .../ColdStart/ProvisioningTestKitTests.cs | 31 ++- .../ColdStart/RealmConfigEndpointsTests.cs | 22 +- .../ColdStart/RealmManifestApplierTests.cs | 179 ++++--------- .../ColdStart/RealmManifestExportTests.cs | 15 +- .../ColdStart/RealmManifestParityTests.cs | 11 +- .../ColdStart/RealmManifestSectionsTests.cs | 41 +-- .../RealmProvisioningEndpointsTests.cs | 108 +++++--- .../Infrastructure/ColdStartTestBase.cs | 38 +++ .../Provisioning/RealmConfigEndpoints.cs | 56 +--- .../Admin/Provisioning/RealmDraftService.cs | 23 +- .../Admin/Provisioning/RealmManifest.cs | 11 +- .../Provisioning/RealmManifestApplier.cs | 247 ++---------------- .../Provisioning/RealmManifestExporter.cs | 12 +- .../Provisioning/RealmManifestPlanner.cs | 23 +- .../Admin/Provisioning/RealmManifestSchema.cs | 6 - .../Features/Admin/RealmsEndpoints.cs | 39 +-- .../ModgudProvisioningClient.cs | 75 ++++-- .../ProvisionedRealm.cs | 11 +- .../RealmManifest.cs | 24 +- src/frontend-vue/public/i18n/de.json | 3 +- .../src/stores/realmDraft.store.ts | 1 - .../realm-config/SelectiveExportModal.vue | 24 +- .../admin/realm-config/manifestSelection.ts | 9 +- 23 files changed, 346 insertions(+), 663 deletions(-) diff --git a/src/dotnet/Modgud.Api.Tests/ColdStart/ProvisioningTestKitTests.cs b/src/dotnet/Modgud.Api.Tests/ColdStart/ProvisioningTestKitTests.cs index a08edc08..d2163f27 100644 --- a/src/dotnet/Modgud.Api.Tests/ColdStart/ProvisioningTestKitTests.cs +++ b/src/dotnet/Modgud.Api.Tests/ColdStart/ProvisioningTestKitTests.cs @@ -4,6 +4,7 @@ using Modgud.Authorization.Apps; using Modgud.Infrastructure.Persistence.Tenancy; using Modgud.Infrastructure.Realms; +using System.Net; using Modgud.Provisioning.TestKit; namespace Modgud.Api.Tests.ColdStart; @@ -28,7 +29,7 @@ public async Task TestKit_imports_applies_and_hard_deletes_a_realm() var kit = new ModgudProvisioningClient(httpClient); const string slug = "kittest"; - var realm = await kit.ImportRealmAsync(BuildManifest(slug, "Kit App"), ct); + var realm = await kit.ImportRealmAsync(BuildSpec(slug), BuildManifest(slug, "Kit App"), ct); // The handle surfaces everything an app-under-test needs. Assert.Equal(slug, realm.Slug); @@ -60,22 +61,30 @@ public async Task TestKit_surfaces_the_server_error_code_on_duplicate_import() var kit = new ModgudProvisioningClient(await factory.CreateRealmAdminAndLoginAsync()); const string slug = "kitdup"; - await using var first = await kit.ImportRealmAsync(BuildManifest(slug, "Dup"), ct); + await using var first = await kit.ImportRealmAsync(BuildSpec(slug), BuildManifest(slug, "Dup"), ct); var ex = await Assert.ThrowsAsync( - () => kit.ImportRealmAsync(BuildManifest(slug, "Dup"), ct)); - Assert.Equal("Realm.AlreadyExists", ex.Code); + () => kit.ImportRealmAsync(BuildSpec(slug), BuildManifest(slug, "Dup"), ct)); + + // The duplicate is caught by the create step, which answers in the canonical + // ErrorOr shape — a description, no machine-readable code. The kit must surface + // that as-is rather than passing the description off as a Code. + Assert.Equal("create-realm", ex.Operation); + Assert.Equal(HttpStatusCode.Conflict, ex.StatusCode); + Assert.Null(ex.Code); + Assert.Contains(slug, ex.Message); } + private static RealmSpec BuildSpec(string slug) => new() + { + Slug = slug, + DisplayName = slug, + Domains = [$"{slug}.localhost"], + InitialAdmin = new InitialAdmin { UserName = "admin", Email = $"admin@{slug}.test" }, + }; + private static RealmManifest BuildManifest(string slug, string appDisplayName) => new() { - Realm = new RealmSpec - { - Slug = slug, - DisplayName = slug, - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdmin { UserName = "admin", Email = $"admin@{slug}.test" }, - }, Apps = [ new RealmManifestApp diff --git a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmConfigEndpointsTests.cs b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmConfigEndpointsTests.cs index 1c8060e0..bd37db46 100644 --- a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmConfigEndpointsTests.cs +++ b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmConfigEndpointsTests.cs @@ -56,25 +56,33 @@ await InTenantAsync(factory, TenantConstants.SystemTenantId, async sp => } [Fact] - public async Task Apply_refuses_a_manifest_targeting_a_different_realm() + public async Task Apply_ignores_a_stray_realm_object_and_lands_in_the_callers_own_realm() { await using var host = await Fixture.CreateIsolatedHostAsync(); var factory = host.Factory; var ct = TestContext.Current.CancellationToken; var client = await factory.CreateRealmAdminAndLoginAsync(); - // A realm admin may only manage their own realm — a foreign slug is the data-plane boundary. - var foreign = new + // A manifest names no realm any more, so the data-plane boundary is structural rather + // than a guard: there is no field a caller could aim elsewhere. A body that still + // carries the retired "Realm" object (an older export, a hand-written file) stays + // loadable — the property is simply ignored, NOT honoured. + var stray = new { Realm = new { Slug = "some-other-realm" }, - Apps = new[] { new { Slug = "x", DisplayName = "X", Permissions = new object[0] } }, + Apps = new[] { new { Slug = "stray-app", DisplayName = "Stray", Permissions = new object[0] } }, }; var resp = await client.PostAsJsonAsync( - "/api/admin/realm-config/apply", foreign, factory.JsonOptions, ct); + "/api/admin/realm-config/apply", stray, factory.JsonOptions, ct); + Assert.Equal(HttpStatusCode.OK, resp.StatusCode); - Assert.Equal(HttpStatusCode.BadRequest, resp.StatusCode); - Assert.Contains("Manifest.SlugMismatch", await resp.Content.ReadAsStringAsync(ct)); + // It landed in the caller's realm, not the one the stray object named. + await InTenantAsync(factory, TenantConstants.SystemTenantId, async sp => + { + var session = sp.GetRequiredService(); + Assert.True(await session.Query().AnyAsync(a => !a.IsDeleted && a.Slug == "stray-app", ct)); + }); } [Fact] diff --git a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs index d51d2467..9b359849 100644 --- a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs +++ b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs @@ -44,13 +44,6 @@ public async Task Import_provisions_a_fully_configured_realm_with_resolved_cross const string slug = "acme"; var manifest = new RealmManifest { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = "Acme", - Domains = ["acme.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "admin", Email = "admin@acme.test" }, - }, Apps = [ new RealmManifestApp @@ -116,7 +109,7 @@ public async Task Import_provisions_a_fully_configured_realm_with_resolved_cross var applier = factory.Services.GetRequiredService(); - var result = await applier.ImportNewRealmAsync(manifest, ct); + var result = await ProvisionRealmAsync(factory, Shell(slug), manifest, ct); Assert.False(result.IsError, result.IsError ? result.FirstError.Description : string.Empty); Assert.Equal(slug, result.Value.Slug); @@ -167,25 +160,17 @@ public async Task Import_rejects_a_slug_that_already_exists() var factory = host.Factory; var ct = TestContext.Current.CancellationToken; - var manifest = new RealmManifest - { - Realm = new CreateRealmDto - { - Slug = "dup", - DisplayName = "Dup", - Domains = ["dup.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "admin", Email = "admin@dup.test" }, - }, - }; + const string slug = "dupe"; + var manifest = new RealmManifest(); - var applier = factory.Services.GetRequiredService(); - - var first = await applier.ImportNewRealmAsync(manifest, ct); + var first = await ProvisionRealmAsync(factory, Shell(slug), manifest, ct); Assert.False(first.IsError, first.IsError ? first.FirstError.Description : string.Empty); - var second = await applier.ImportNewRealmAsync(manifest, ct); + var second = await ProvisionRealmAsync(factory, Shell(slug), manifest, ct); Assert.True(second.IsError); - Assert.Equal("Realm.AlreadyExists", second.FirstError.Code); + // The canonical create op owns this rule now — one code for a duplicate slug, + // whichever way a realm is created. + Assert.Equal("Realm.DuplicateSlug", second.FirstError.Code); } [Fact] @@ -206,13 +191,6 @@ public async Task Re_importing_a_deleted_entity_under_its_pinned_id_revives_it_e // come back — consuming apps persist them as foreign keys. RealmManifest Manifest(string appSlug, string roleName, string groupName) => new() { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = "Revive", - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "boot", Email = "boot@revive.test" }, - }, Apps = [ new RealmManifestApp @@ -232,7 +210,7 @@ public async Task Re_importing_a_deleted_entity_under_its_pinned_id_revives_it_e Groups = [new RealmManifestGroup { Name = groupName, Id = new ShortGuid(groupId).ToString(), Roles = [roleName] }], }; - var imported = await applier.ImportNewRealmAsync(Manifest("rev-app", "rev-role", "RevGroup"), ct); + var imported = await ProvisionRealmAsync(factory, Shell(slug), Manifest("rev-app", "rev-role", "RevGroup"), ct); Assert.False(imported.IsError, imported.IsError ? imported.FirstError.Description : string.Empty); // RENAME live in the admin UI (the manifest can't rename — its natural key IS the @@ -262,7 +240,7 @@ await InTenantAsync(factory, slug, async sp => }); // Delete all three under their CURRENT keys (staged deletes, the normal admin path). - var deleted = await applier.UpdateRealmAsync( + var deleted = await applier.UpdateRealmAsync(slug, Manifest("rev-app", "rev-role", "RevGroup") with { Apps = [], Roles = [], Groups = [] }, deletions: [ @@ -284,7 +262,7 @@ await InTenantAsync(factory, slug, async sp => // Re-import the ORIGINAL manifest: the pinned ids point at soft-deleted streams, so // the apply revives them — under the manifest's (original) names, not the renamed ones. - var reimported = await applier.UpdateRealmAsync(Manifest("rev-app", "rev-role", "RevGroup"), ct: ct); + var reimported = await applier.UpdateRealmAsync(slug, Manifest("rev-app", "rev-role", "RevGroup"), ct: ct); Assert.False(reimported.IsError, reimported.IsError ? reimported.FirstError.Description : string.Empty); await InTenantAsync(factory, slug, async sp => @@ -319,13 +297,6 @@ public async Task A_pinned_id_matching_a_live_entity_updates_it_and_renames_wher RealmManifest Manifest(string roleName, string groupName, string? groupDescription = null) => new() { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = "Id match", - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "boot", Email = "boot@idmatch.test" }, - }, Apps = [new RealmManifestApp { Slug = "im-app", DisplayName = "Id Match App", Permissions = [new RealmManifestPermission("im", "read")] }], Roles = [ @@ -345,12 +316,12 @@ public async Task A_pinned_id_matching_a_live_entity_updates_it_and_renames_wher ], }; - var imported = await applier.ImportNewRealmAsync(Manifest("im-role", "ImGroup"), ct); + var imported = await ProvisionRealmAsync(factory, Shell(slug), Manifest("im-role", "ImGroup"), ct); Assert.False(imported.IsError, imported.IsError ? imported.FirstError.Description : string.Empty); // Same ids, DIFFERENT names: the id names the entity, so this is an update that // renames — not a second entity next to the old one. - var renamed = await applier.UpdateRealmAsync(Manifest("im-role-v2", "ImGroupV2", "renamed via id"), ct: ct); + var renamed = await applier.UpdateRealmAsync(slug, Manifest("im-role-v2", "ImGroupV2", "renamed via id"), ct: ct); Assert.False(renamed.IsError, renamed.IsError ? renamed.FirstError.Description : string.Empty); await InTenantAsync(factory, slug, async sp => @@ -374,7 +345,7 @@ await InTenantAsync(factory, slug, async sp => // The plan makes the rename visible before it happens. var planner = factory.Services.GetRequiredService(); - var plan = await planner.PlanAsync(Manifest("im-role-v3", "ImGroupV3"), prune: false, ct: ct); + var plan = await planner.PlanAsync(slug, Manifest("im-role-v3", "ImGroupV3"), prune: false, ct: ct); Assert.False(plan.IsError, plan.IsError ? plan.FirstError.Description : string.Empty); var roleEntry = Assert.Single(plan.Value.Sections.Single(s => s.Name == "roles").Entries); @@ -398,27 +369,20 @@ public async Task A_pinned_id_naming_an_entity_with_an_immutable_key_fails_with_ RealmManifest Manifest(string appSlug) => new() { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = "Immutable key", - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "boot", Email = "boot@immutablekey.test" }, - }, Apps = [new RealmManifestApp { Slug = appSlug, Id = new ShortGuid(appId).ToString(), DisplayName = "App" }], }; - var imported = await applier.ImportNewRealmAsync(Manifest("ik-app"), ct); + var imported = await ProvisionRealmAsync(factory, Shell(slug), Manifest("ik-app"), ct); Assert.False(imported.IsError, imported.IsError ? imported.FirstError.Description : string.Empty); // An app slug cannot be renamed through the canonical update — the id and the slug // name two different things, which is never a silent merge. - var renamed = await applier.UpdateRealmAsync(Manifest("ik-app-renamed"), ct: ct); + var renamed = await applier.UpdateRealmAsync(slug, Manifest("ik-app-renamed"), ct: ct); Assert.True(renamed.IsError); Assert.Equal("Manifest.ImmutableKey", renamed.FirstError.Code); var planner = factory.Services.GetRequiredService(); - var plan = await planner.PlanAsync(Manifest("ik-app-renamed"), prune: false, ct: ct); + var plan = await planner.PlanAsync(slug, Manifest("ik-app-renamed"), prune: false, ct: ct); var entry = Assert.Single(plan.Value.Sections.Single(s => s.Name == "apps").Entries); Assert.Equal("error", entry.Action); Assert.Contains(entry.Notes, n => n.Contains("Slug is immutable")); @@ -437,13 +401,6 @@ public async Task Pinning_an_id_owned_by_another_entity_type_fails_the_apply() RealmManifest Manifest(bool withRole) => new() { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = "Id clash", - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "boot", Email = "boot@idclash.test" }, - }, Apps = [new RealmManifestApp { Slug = "first", Id = new ShortGuid(appId).ToString(), DisplayName = "First" }], // Claims the APP's id for a role — appending role events onto an app's stream // would corrupt it, so this stays a hard conflict (no revive, no update). @@ -452,10 +409,10 @@ public async Task Pinning_an_id_owned_by_another_entity_type_fails_the_apply() : [], }; - var imported = await applier.ImportNewRealmAsync(Manifest(withRole: false), ct); + var imported = await ProvisionRealmAsync(factory, Shell(slug), Manifest(withRole: false), ct); Assert.False(imported.IsError, imported.IsError ? imported.FirstError.Description : string.Empty); - var clash = await applier.UpdateRealmAsync(Manifest(withRole: true), ct: ct); + var clash = await applier.UpdateRealmAsync(slug, Manifest(withRole: true), ct: ct); Assert.True(clash.IsError); Assert.Equal("Role.PinnedIdTaken", clash.FirstError.Code); @@ -472,7 +429,7 @@ public async Task Update_merges_in_place_keeping_ids_and_upserts_new_entities() var applier = factory.Services.GetRequiredService(); // ── Import the baseline realm ────────────────────────────────────────── - var imported = await applier.ImportNewRealmAsync(BuildGlobexManifest(slug, version: 1), ct); + var imported = await ProvisionRealmAsync(factory, Shell(slug), BuildGlobexManifest(slug, version: 1), ct); Assert.False(imported.IsError, imported.IsError ? imported.FirstError.Description : string.Empty); // Capture the stable ids so we can prove the update was IN PLACE (not drop+recreate). @@ -491,7 +448,7 @@ await InTenantAsync(factory, slug, async sp => }); // ── Apply the v2 manifest: changes every existing entity + adds a new role ── - var updated = await applier.UpdateRealmAsync(BuildGlobexManifest(slug, version: 2), ct: ct); + var updated = await applier.UpdateRealmAsync(slug, BuildGlobexManifest(slug, version: 2), ct: ct); Assert.False(updated.IsError, updated.IsError ? updated.FirstError.Description : string.Empty); // The realm DB was never dropped. @@ -552,13 +509,6 @@ public async Task Update_omitting_a_bool_leaves_it_unchanged() // Import a DISABLED confidential client (Enabled explicitly false). var manifest = new RealmManifest { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = slug, - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "admin", Email = $"admin@{slug}.test" }, - }, Apps = [new RealmManifestApp { Slug = "bp-app", DisplayName = "BP", Permissions = [new RealmManifestPermission("bp", "read")] }], Clients = [ @@ -574,12 +524,11 @@ public async Task Update_omitting_a_bool_leaves_it_unchanged() }, ], }; - Assert.False((await applier.ImportNewRealmAsync(manifest, ct)).IsError); + Assert.False((await ProvisionRealmAsync(factory, Shell(slug), manifest, ct)).IsError); // Apply a partial update: change the redirect URI, OMIT Enabled (null = no change). var patch = new RealmManifest { - Realm = manifest.Realm, Clients = [ new RealmManifestClient @@ -592,7 +541,7 @@ public async Task Update_omitting_a_bool_leaves_it_unchanged() }, ], }; - Assert.False((await applier.UpdateRealmAsync(patch, ct: ct)).IsError); + Assert.False((await applier.UpdateRealmAsync(slug, patch, ct: ct)).IsError); await InTenantAsync(factory, slug, async sp => { @@ -623,16 +572,9 @@ public async Task Import_and_update_apply_the_client_access_token_type() }; var manifest = new RealmManifest { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = slug, - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "admin", Email = $"admin@{slug}.test" }, - }, Clients = [Client("Jwt")], }; - Assert.False((await applier.ImportNewRealmAsync(manifest, ct)).IsError); + Assert.False((await ProvisionRealmAsync(factory, Shell(slug), manifest, ct)).IsError); async Task GetTokenTypeAsync() { @@ -650,15 +592,15 @@ await InTenantAsync(factory, slug, async sp => Assert.Equal(AccessTokenType.Jwt, await GetTokenTypeAsync()); // Apply with the field OMITTED: no change (same patch semantics as the bool flags). - Assert.False((await applier.UpdateRealmAsync(manifest with { Clients = [Client(null)] }, ct: ct)).IsError); + Assert.False((await applier.UpdateRealmAsync(slug, manifest with { Clients = [Client(null)] }, ct: ct)).IsError); Assert.Equal(AccessTokenType.Jwt, await GetTokenTypeAsync()); // Apply with an explicit 'Reference': the merge flips it back. - Assert.False((await applier.UpdateRealmAsync(manifest with { Clients = [Client("Reference")] }, ct: ct)).IsError); + Assert.False((await applier.UpdateRealmAsync(slug, manifest with { Clients = [Client("Reference")] }, ct: ct)).IsError); Assert.Equal(AccessTokenType.Reference, await GetTokenTypeAsync()); // An invalid value is a contextual validation error, not a silent default. - var invalid = await applier.UpdateRealmAsync(manifest with { Clients = [Client("Bogus")] }, ct: ct); + var invalid = await applier.UpdateRealmAsync(slug, manifest with { Clients = [Client("Bogus")] }, ct: ct); Assert.True(invalid.IsError); Assert.Equal("Manifest.InvalidEnum", invalid.FirstError.Code); } @@ -671,7 +613,7 @@ public async Task Update_rejects_a_slug_that_does_not_exist() var ct = TestContext.Current.CancellationToken; var applier = factory.Services.GetRequiredService(); - var result = await applier.UpdateRealmAsync(BuildGlobexManifest("ghost", version: 1), ct: ct); + var result = await applier.UpdateRealmAsync("ghost", BuildGlobexManifest("ghost", version: 1), ct: ct); Assert.True(result.IsError); Assert.Equal("Realm.NotFound", result.FirstError.Code); @@ -694,13 +636,6 @@ public async Task Prune_removes_absent_entities_but_protects_infra_and_admins() // all dropped too → exercises reverse-dependency-order pruning. var full = new RealmManifest { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = "Prune", - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "boot", Email = "boot@prune.test" }, - }, Apps = [ new RealmManifestApp { Slug = "keep-app", DisplayName = "Keep", Permissions = [new RealmManifestPermission("keep", "read")] }, @@ -740,13 +675,12 @@ public async Task Prune_removes_absent_entities_but_protects_infra_and_admins() new RealmManifestGroup { Name = "AdminGroup", Members = ["adminuser"], Roles = ["super-admin"] }, ], }; - var import = await applier.ImportNewRealmAsync(full, ct); + var import = await ProvisionRealmAsync(factory, Shell(slug), full, ct); Assert.False(import.IsError, import.IsError ? import.FirstError.Description : string.Empty); // The prune manifest keeps only the keep-* entities; everything else is absent. var keepOnly = new RealmManifest { - Realm = full.Realm, Apps = [full.Apps[0]], Apis = [full.Apis[0]], Scopes = [full.Scopes[0]], @@ -756,7 +690,7 @@ public async Task Prune_removes_absent_entities_but_protects_infra_and_admins() Groups = [full.Groups[0]], }; - var pruned = await applier.UpdateRealmAsync(keepOnly, prune: true, deletions: null, ct); + var pruned = await applier.UpdateRealmAsync(slug, keepOnly, prune: true, deletions: null, ct); Assert.False(pruned.IsError, pruned.IsError ? pruned.FirstError.Description : string.Empty); // The realm DB was never dropped. @@ -828,24 +762,17 @@ public async Task A_binned_user_carried_in_the_manifest_is_skipped_instead_of_fa const string slug = "binned"; var full = new RealmManifest { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = "Binned", - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "boot", Email = "boot@binned.test" }, - }, Users = [ new RealmManifestUser { Key = "first", Email = "first@binned.test", UserName = "first", Firstname = "First", Password = "Passw0rd!23" }, new RealmManifestUser { Key = "second", Email = "second@binned.test", UserName = "second", Firstname = "Second", Password = "Passw0rd!23" }, ], }; - var import = await applier.ImportNewRealmAsync(full, ct); + var import = await ProvisionRealmAsync(factory, Shell(slug), full, ct); Assert.False(import.IsError, import.IsError ? import.FirstError.Description : string.Empty); // ── 1. The admin deletes "first" — the normal staged-delete path. ────────── - var binned = await applier.UpdateRealmAsync( + var binned = await applier.UpdateRealmAsync(slug, full with { Users = [full.Users[1]] }, deletions: [new RealmDraftDeletion("users", "first")], ct: ct); @@ -864,12 +791,12 @@ public async Task A_binned_user_carried_in_the_manifest_is_skipped_instead_of_fa Users = [.. export.Value.Users.Select(u => u.UserName == "first" ? u with { Firstname = "Overwritten" } : u)], }; - var reapplied = await applier.UpdateRealmAsync(carried, ct: ct); + var reapplied = await applier.UpdateRealmAsync(slug, carried, ct: ct); Assert.False(reapplied.IsError, reapplied.IsError ? reapplied.FirstError.Description : string.Empty); // ── 4. The real-world symptom: deleting ANOTHER user while the first sits in // the bin. This is what failed in production. ──────────────────────────── - var second = await applier.UpdateRealmAsync( + var second = await applier.UpdateRealmAsync(slug, carried with { Users = [.. carried.Users.Where(u => u.UserName != "second")] }, deletions: [new RealmDraftDeletion("users", "second")], ct: ct); @@ -928,11 +855,6 @@ public async Task Roles_are_keyed_per_app_and_round_trip_through_export() var manifest = new RealmManifest { - Realm = new CreateRealmDto - { - Slug = slug, DisplayName = "Authors", Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "boot", Email = "boot@authors.test" }, - }, Apps = [AppOf("alpha"), AppOf("beta")], Roles = [Author("alpha"), Author("beta"), new RealmManifestRole { Name = "Reviewer", App = "beta", Permissions = [] }], Groups = @@ -943,7 +865,7 @@ public async Task Roles_are_keyed_per_app_and_round_trip_through_export() new RealmManifestGroup { Name = "Reviewers", Roles = ["Reviewer"] }, ], }; - var imported = await applier.ImportNewRealmAsync(manifest, ct); + var imported = await ProvisionRealmAsync(factory, Shell(slug), manifest, ct); Assert.False(imported.IsError, imported.IsError ? imported.FirstError.Description : string.Empty); async Task<(Guid Alpha, Guid Beta, Guid Reviewer)> RoleIdsAsync() @@ -972,17 +894,16 @@ await InTenantAsync(factory, slug, async sp => Assert.False(export.IsError, export.IsError ? export.FirstError.Description : string.Empty); var betaRef = Assert.Single(export.Value.Groups.Single(g => g.Name == "Beta writers").Roles!); Assert.Equal(("beta/Author", before.Beta), (betaRef.Key, betaRef.ParsedId)); - var reapplied = await applier.UpdateRealmAsync(export.Value, ct: ct); + var reapplied = await applier.UpdateRealmAsync(slug, export.Value, ct: ct); Assert.False(reapplied.IsError, reapplied.IsError ? reapplied.FirstError.Description : string.Empty); Assert.Equal(before, await RoleIdsAsync()); // A patch without App (absent = unchanged) updates a uniquely-named role... var patch = new RealmManifest { - Realm = manifest.Realm, Roles = [new RealmManifestRole { Name = "Reviewer", Description = "Reviews drafts" }], }; - var patched = await applier.UpdateRealmAsync(patch, ct: ct); + var patched = await applier.UpdateRealmAsync(slug, patch, ct: ct); Assert.False(patched.IsError, patched.IsError ? patched.FirstError.Description : string.Empty); await InTenantAsync(factory, slug, async sp => { @@ -991,18 +912,16 @@ await InTenantAsync(factory, slug, async sp => }); // ...but is refused for an ambiguous one instead of silently picking the first. - var ambiguous = await applier.UpdateRealmAsync(new RealmManifest + var ambiguous = await applier.UpdateRealmAsync(slug, new RealmManifest { - Realm = manifest.Realm, Roles = [new RealmManifestRole { Name = "Author", Description = "Which one?" }], }, ct: ct); Assert.True(ambiguous.IsError); Assert.Equal("Manifest.AmbiguousRole", ambiguous.FirstError.Code); // So is a group reference to the bare name. - var ambiguousRef = await applier.UpdateRealmAsync(new RealmManifest + var ambiguousRef = await applier.UpdateRealmAsync(slug, new RealmManifest { - Realm = manifest.Realm, Groups = [new RealmManifestGroup { Name = "Anyone", Roles = ["Author"] }], }, ct: ct); Assert.True(ambiguousRef.IsError); @@ -1098,9 +1017,8 @@ public async Task References_follow_the_id_and_fall_back_to_the_key() Slug = slug, DisplayName = "Refs", Domains = [$"{slug}.localhost"], InitialAdmin = new InitialAdminDto { UserName = "boot", Email = "boot@refs.test" }, }; - var imported = await applier.ImportNewRealmAsync(new RealmManifest + var imported = await ProvisionRealmAsync(factory, realm, new RealmManifest { - Realm = realm, Apps = [new RealmManifestApp { Slug = "alpha", DisplayName = "alpha", Permissions = [new RealmManifestPermission("doc", "write")] }, new RealmManifestApp { Slug = "beta", DisplayName = "beta", Permissions = [new RealmManifestPermission("doc", "write")] }], Roles = [new RealmManifestRole { Name = "Author", App = "alpha", Permissions = [] }, @@ -1133,10 +1051,9 @@ await InTenantAsync(factory, slug, async sp => } async Task ApplyGroupAsync(List? roles = null, List? members = null) { - var result = await applier.UpdateRealmAsync(new RealmManifest + var result = await applier.UpdateRealmAsync(slug, new RealmManifest { - Realm = realm, - Groups = [new RealmManifestGroup { Name = "Beta writers", Roles = roles, Members = members }], + Groups = [new RealmManifestGroup { Name = "Beta writers", Roles = roles, Members = members }], }, ct: ct); Assert.False(result.IsError, result.IsError ? result.FirstError.Description : string.Empty); } @@ -1154,9 +1071,8 @@ async Task ApplyGroupAsync(List? roles = null, List? m await ApplyGroupAsync(members: [new ManifestRef { Key = "nobody", Id = new ShortGuid(alice).ToString() }]); Assert.Equal([alice], (await GroupAsync()).MemberIds); // A bare string is ALWAYS a key — even when it happens to be an id. - var idAsKey = await applier.UpdateRealmAsync(new RealmManifest + var idAsKey = await applier.UpdateRealmAsync(slug, new RealmManifest { - Realm = realm, Groups = [new RealmManifestGroup { Name = "Beta writers", Roles = [new ShortGuid(betaAuthor).ToString()] }], }, ct: ct); Assert.True(idAsKey.IsError); @@ -1173,7 +1089,7 @@ await InTenantAsync(factory, slug, async sp => }); // ...so the plan sees no change on the group (the role entry itself flags the rename), // and the apply keeps the group on the SAME role, stale key notwithstanding. - var plan = await planner.PlanAsync(exportBefore.Value, prune: false, baseline: exportBefore.Value, ct: ct); + var plan = await planner.PlanAsync(slug, exportBefore.Value, prune: false, baseline: exportBefore.Value, ct: ct); Assert.False(plan.IsError, plan.IsError ? plan.FirstError.Description : string.Empty); var groupEntry = plan.Value.Sections.Single(s => s.Name == "groups").Entries.Single(e => e.Key == "Beta writers"); Assert.Equal("unchanged", groupEntry.Action); @@ -1213,13 +1129,6 @@ private static RealmManifest BuildGlobexManifest(string slug, int version) return new RealmManifest { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = "Globex", - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "admin", Email = $"admin@{slug}.test" }, - }, Apps = [ new RealmManifestApp diff --git a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestExportTests.cs b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestExportTests.cs index 1288f1c6..3f776625 100644 --- a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestExportTests.cs +++ b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestExportTests.cs @@ -30,13 +30,6 @@ public async Task Export_is_structure_only_and_round_trips_with_apply_and_passwo const string slug = "exporttest"; var manifest = new RealmManifest { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = slug, - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "admin", Email = $"admin@{slug}.test" }, - }, Apps = [ new RealmManifestApp { Slug = "ex-app", DisplayName = "Ex App", @@ -59,7 +52,7 @@ public async Task Export_is_structure_only_and_round_trips_with_apply_and_passwo ], Users = [new RealmManifestUser { Key = "bob", Email = "bob@ex.test", UserName = "bob" }], // passwordless }; - Assert.False((await applier.ImportNewRealmAsync(manifest, ct)).IsError); + Assert.False((await ProvisionRealmAsync(factory, Shell(slug), manifest, ct)).IsError); // ── Export ──────────────────────────────────────────────────────────── var exported = await exporter.ExportRealmAsync(slug, ct); @@ -93,7 +86,7 @@ public async Task Export_is_structure_only_and_round_trips_with_apply_and_passwo Assert.NotNull(m.Settings.PositionSecurity); // ── Re-apply the UNEDITED export = idempotent ────────────────────────── - Assert.False((await applier.UpdateRealmAsync(m, ct: ct)).IsError); + Assert.False((await applier.UpdateRealmAsync(slug, m, ct: ct)).IsError); // ── Edit a setting and re-apply → it round-trips ─────────────────────── var withSetting = m with @@ -103,7 +96,7 @@ public async Task Export_is_structure_only_and_round_trips_with_apply_and_passwo RegistrationFields = new UpdateRegistrationFieldsSettingsDto { Username = "Required" }, }, }; - Assert.False((await applier.UpdateRealmAsync(withSetting, ct: ct)).IsError); + Assert.False((await applier.UpdateRealmAsync(slug, withSetting, ct: ct)).IsError); var reexport = await exporter.ExportRealmAsync(slug, ct); Assert.Equal("Required", reexport.Value.Settings!.RegistrationFields!.Username); @@ -112,7 +105,7 @@ public async Task Export_is_structure_only_and_round_trips_with_apply_and_passwo { Users = m.Users.Select(u => u.UserName == "bob" ? u with { Password = "Bobsecret1!" } : u).ToList(), }; - Assert.False((await applier.UpdateRealmAsync(withPassword, ct: ct)).IsError); + Assert.False((await applier.UpdateRealmAsync(slug, withPassword, ct: ct)).IsError); await InTenantAsync(factory, slug, async sp => { diff --git a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestParityTests.cs b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestParityTests.cs index 7631e719..15b78008 100644 --- a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestParityTests.cs +++ b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestParityTests.cs @@ -36,7 +36,7 @@ public async Task Client_built_via_admin_service_and_via_manifest_have_identical // Realm A: import just the realm + app, then create the client via the canonical // OAuthAdminService with an explicit DTO (the admin-API path). - var importA = await applier.ImportNewRealmAsync(BaseManifest(slugA), ct); + var importA = await ProvisionRealmAsync(factory, Shell(slugA), BaseManifest(slugA), ct); Assert.False(importA.IsError, importA.IsError ? importA.FirstError.Description : string.Empty); await InTenantAsync(factory, slugA, async sp => { @@ -99,7 +99,7 @@ await InTenantAsync(factory, slugA, async sp => }, ], }; - var importB = await applier.ImportNewRealmAsync(manifestB, ct); + var importB = await ProvisionRealmAsync(factory, Shell(slugB), manifestB, ct); Assert.False(importB.IsError, importB.IsError ? importB.FirstError.Description : string.Empty); var shapeA = await GetClientShapeAsync(factory, slugA, "parity-web", ct); @@ -185,13 +185,6 @@ await InTenantAsync(factory, slug, async sp => private static RealmManifest BaseManifest(string slug) => new() { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = slug, - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "admin", Email = $"admin@{slug}.test" }, - }, Apps = [ new RealmManifestApp diff --git a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestSectionsTests.cs b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestSectionsTests.cs index 66f53c61..c1cac6ad 100644 --- a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestSectionsTests.cs +++ b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestSectionsTests.cs @@ -51,16 +51,9 @@ public async Task LoginProviders_import_apply_export_and_prune() }; var manifest = new RealmManifest { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = slug, - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "admin", Email = $"admin@{slug}.test" }, - }, LoginProviders = [Provider("corp-idp", "Corp IdP"), Provider("legacy-idp", "Legacy IdP")], }; - var import = await applier.ImportNewRealmAsync(manifest, ct); + var import = await ProvisionRealmAsync(factory, Shell(slug), manifest, ct); Assert.False(import.IsError, import.IsError ? import.FirstError.Description : string.Empty); await InTenantAsync(factory, slug, async sp => @@ -92,7 +85,7 @@ await InTenantAsync(factory, slug, async sp => { LoginProviders = [Provider("corp-idp", "Corp IdP v2") with { ClientSecret = null }], }; - var applied = await applier.UpdateRealmAsync(v2, prune: true, deletions: null, ct); + var applied = await applier.UpdateRealmAsync(slug, v2, prune: true, deletions: null, ct); Assert.False(applied.IsError, applied.IsError ? applied.FirstError.Description : string.Empty); await InTenantAsync(factory, slug, async sp => @@ -115,7 +108,7 @@ await InTenantAsync(factory, slug, async sp => Slug = "my-internal", Flavor = "internal", DisplayName = "Nope", Type = "Internal", }], }; - var rejected = await applier.UpdateRealmAsync(withInternal, ct: ct); + var rejected = await applier.UpdateRealmAsync(slug, withInternal, ct: ct); Assert.True(rejected.IsError); Assert.Equal("Manifest.InternalProviderReserved", rejected.FirstError.Code); } @@ -132,13 +125,6 @@ public async Task App_settings_override_applies_routes_and_exports() const string slug = "appset"; RealmManifest Manifest(string productName) => new() { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = slug, - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "admin", Email = $"admin@{slug}.test" }, - }, Apps = [ new RealmManifestApp @@ -156,7 +142,7 @@ public async Task App_settings_override_applies_routes_and_exports() new RealmManifestApp { Slug = "plain", DisplayName = "Plain" }, ], }; - var import = await applier.ImportNewRealmAsync(Manifest("Shop!"), ct); + var import = await ProvisionRealmAsync(factory, Shell(slug), Manifest("Shop!"), ct); Assert.False(import.IsError, import.IsError ? import.FirstError.Description : string.Empty); Guid shopAppId = default; @@ -186,7 +172,7 @@ await InTenantAsync(factory, slug, async sp => Assert.Null(Assert.Single(exported.Value.Apps, a => a.Slug == "plain").Settings); // ── Apply: the settings patch updates in place. ──────────────────────────── - Assert.False((await applier.UpdateRealmAsync(Manifest("Shop v2"), ct: ct)).IsError); + Assert.False((await applier.UpdateRealmAsync(slug, Manifest("Shop v2"), ct: ct)).IsError); await InTenantAsync(factory, slug, async sp => { var settings = await sp.GetRequiredService().GetAsync(shopAppId, ct); @@ -208,13 +194,6 @@ public async Task Positions_are_feature_gated_and_import_apply_export_prune() RealmManifest Manifest(string purpose, params string[] grants) => ManifestFor(slug, purpose, grants); RealmManifest ManifestFor(string realmSlug, string purpose, params string[] grants) => new() { - Realm = new CreateRealmDto - { - Slug = realmSlug, - DisplayName = realmSlug, - Domains = [$"{realmSlug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "admin", Email = $"admin@{realmSlug}.test" }, - }, Users = [ new RealmManifestUser { Key = "alice", Email = $"alice@{slug}.test", UserName = "alice", Password = "Passw0rd!23" }, @@ -244,13 +223,15 @@ public async Task Positions_are_feature_gated_and_import_apply_export_prune() // the rollback hard-deletes the tenant DB, and recreating the same slug in // the same host would hit the disposed tenant data source. appSettings.Features.PositionTerminals = false; - var gated = await applier.ImportNewRealmAsync(ManifestFor("posgate", "Gate", "alice"), ct); + // Its own realm — the feature-gate probe must not consume the slug the real + // import below needs (creating a realm and filling it are two operations now). + var gated = await ProvisionRealmAsync(factory, Shell("posgate"), ManifestFor("posgate", "Gate", "alice"), ct); Assert.True(gated.IsError); Assert.Equal("Manifest.FeatureDisabled", gated.FirstError.Code); // ── Feature on: import creates position + policy + grant. ────────────────── appSettings.Features.PositionTerminals = true; - var import = await applier.ImportNewRealmAsync(Manifest("Gate", "alice"), ct); + var import = await ProvisionRealmAsync(factory, Shell(slug), Manifest("Gate", "alice"), ct); Assert.False(import.IsError, import.IsError ? import.FirstError.Description : string.Empty); Guid positionId = default, aliceId = default, bobId = default; @@ -282,7 +263,7 @@ await InTenantAsync(factory, slug, async sp => Assert.Equal(["alice"], exPos.Grants!.Select(g => g.Key)); // ── Apply: merge purpose + REPLACE the grant set (alice → bob). ──────────── - var applied = await applier.UpdateRealmAsync(Manifest("Gate v2", "bob"), ct: ct); + var applied = await applier.UpdateRealmAsync(slug, Manifest("Gate v2", "bob"), ct: ct); Assert.False(applied.IsError, applied.IsError ? applied.FirstError.Description : string.Empty); await InTenantAsync(factory, slug, async sp => { @@ -298,7 +279,7 @@ await InTenantAsync(factory, slug, async sp => // ── Prune: a position absent from the manifest is deleted via the canonical // cascade (soft delete; grants stay history). ───────────────────────────── var noPositions = Manifest("unused") with { Positions = [] }; - Assert.False((await applier.UpdateRealmAsync(noPositions, prune: true, deletions: null, ct)).IsError); + Assert.False((await applier.UpdateRealmAsync(slug, noPositions, prune: true, deletions: null, ct)).IsError); await InTenantAsync(factory, slug, async sp => { var session = sp.GetRequiredService(); diff --git a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmProvisioningEndpointsTests.cs b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmProvisioningEndpointsTests.cs index a4e561a9..2b2873bf 100644 --- a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmProvisioningEndpointsTests.cs +++ b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmProvisioningEndpointsTests.cs @@ -21,7 +21,7 @@ namespace Modgud.Api.Tests.ColdStart; public class RealmProvisioningEndpointsTests(ColdStartFixture fixture) : ColdStartTestBase(fixture) { [Fact] - public async Task Import_then_apply_then_hard_delete_round_trip() + public async Task Create_then_apply_then_hard_delete_round_trip() { await using var host = await Fixture.CreateIsolatedHostAsync(); var factory = host.Factory; @@ -32,15 +32,17 @@ public async Task Import_then_apply_then_hard_delete_round_trip() const string slug = "initech"; // ── Import ──────────────────────────────────────────────────────────── - var importResp = await client.PostAsJsonAsync( - "/api/admin/realms/import", BuildManifest(slug, "Initech App"), factory.JsonOptions, ct); - Assert.Equal(HttpStatusCode.Created, importResp.StatusCode); + await CreateRealmAsync(client, slug, factory.JsonOptions, ct); + Assert.NotNull(await svc.GetRealmBySlugAsync(slug, ct)); + + var fillResp = await client.PostAsJsonAsync( + $"/api/admin/realms/{slug}/apply", BuildManifest(slug, "Initech App"), factory.JsonOptions, ct); + Assert.Equal(HttpStatusCode.OK, fillResp.StatusCode); - var imported = await importResp.Content.ReadFromJsonAsync(factory.JsonOptions, ct); + var imported = await fillResp.Content.ReadFromJsonAsync(factory.JsonOptions, ct); Assert.NotNull(imported); Assert.Equal(slug, imported!.Slug); Assert.True(imported.ClientSecrets.ContainsKey("initech-web")); - Assert.NotNull(await svc.GetRealmBySlugAsync(slug, ct)); // ── Apply (in-place update: change the app display name) ─────────────── var applyResp = await client.PostAsJsonAsync( @@ -69,15 +71,18 @@ public async Task Export_endpoint_returns_a_structure_only_manifest() var client = await factory.CreateRealmAdminAndLoginAsync(); const string slug = "exportep"; - Assert.Equal(HttpStatusCode.Created, (await client.PostAsJsonAsync( - "/api/admin/realms/import", BuildManifest(slug, "Ex EP App"), factory.JsonOptions, ct)).StatusCode); + await CreateRealmAsync(client, slug, factory.JsonOptions, ct); + Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( + $"/api/admin/realms/{slug}/apply", BuildManifest(slug, "Ex EP App"), factory.JsonOptions, ct)).StatusCode); var resp = await client.GetAsync($"/api/admin/realms/{slug}/export", ct); Assert.Equal(HttpStatusCode.OK, resp.StatusCode); using var json = JsonDocument.Parse(await resp.Content.ReadAsStringAsync(ct)); var root = json.RootElement; - Assert.Equal(slug, root.GetProperty("Realm").GetProperty("Slug").GetString()); + // No realm shell: an export is content, not identity — that is what lets the same + // file be applied to a different realm than the one it came from. + Assert.False(root.TryGetProperty("Realm", out _), "the export must not carry a realm shell"); // The confidential client is present but its secret is omitted (structure-only). var web = root.GetProperty("Clients").EnumerateArray() @@ -86,7 +91,7 @@ public async Task Export_endpoint_returns_a_structure_only_manifest() } [Fact] - public async Task Import_rejects_duplicate_slug_with_409() + public async Task Creating_a_realm_twice_rejects_the_duplicate_slug_with_409() { await using var host = await Fixture.CreateIsolatedHostAsync(); var factory = host.Factory; @@ -94,14 +99,11 @@ public async Task Import_rejects_duplicate_slug_with_409() var client = await factory.CreateRealmAdminAndLoginAsync(); const string slug = "dup-ep"; - var first = await client.PostAsJsonAsync( - "/api/admin/realms/import", BuildManifest(slug, "Dup"), factory.JsonOptions, ct); - Assert.Equal(HttpStatusCode.Created, first.StatusCode); + await CreateRealmAsync(client, slug, factory.JsonOptions, ct); - var second = await client.PostAsJsonAsync( - "/api/admin/realms/import", BuildManifest(slug, "Dup"), factory.JsonOptions, ct); + var second = await client.PostAsJsonAsync("/api/admin/realms", RealmShell(slug), factory.JsonOptions, ct); Assert.Equal(HttpStatusCode.Conflict, second.StatusCode); - Assert.Contains("Realm.AlreadyExists", await second.Content.ReadAsStringAsync(ct)); + Assert.Contains(slug, await second.Content.ReadAsStringAsync(ct)); } [Fact] @@ -118,18 +120,40 @@ public async Task Apply_to_missing_realm_returns_404() Assert.Contains("Realm.NotFound", await resp.Content.ReadAsStringAsync(ct)); } + /// + /// The invariant that replaced the old slug-mismatch guard: a manifest names no realm, so + /// ONE file applies to any number of realms and the route alone decides where it lands. + /// There is nothing left to mismatch. + /// [Fact] - public async Task Apply_with_route_slug_not_matching_manifest_returns_400() + public async Task One_manifest_applies_unchanged_to_two_different_realms() { await using var host = await Fixture.CreateIsolatedHostAsync(); var factory = host.Factory; var ct = TestContext.Current.CancellationToken; var client = await factory.CreateRealmAdminAndLoginAsync(); - var resp = await client.PostAsJsonAsync( - "/api/admin/realms/other-slug/apply", BuildManifest("manifest-slug", "X"), factory.JsonOptions, ct); - Assert.Equal(HttpStatusCode.BadRequest, resp.StatusCode); - Assert.Contains("Manifest.SlugMismatch", await resp.Content.ReadAsStringAsync(ct)); + const string one = "twinsone"; + const string two = "twinstwo"; + var manifest = BuildManifest(one, "Twins App"); + + foreach (var slug in new[] { one, two }) + { + await CreateRealmAsync(client, slug, factory.JsonOptions, ct); + var resp = await client.PostAsJsonAsync( + $"/api/admin/realms/{slug}/apply", manifest, factory.JsonOptions, ct); + Assert.Equal(HttpStatusCode.OK, resp.StatusCode); + } + + foreach (var slug in new[] { one, two }) + { + await InTenantAsync(factory, slug, async sp => + { + var session = sp.GetRequiredService(); + var app = await session.Query().SingleAsync(a => !a.IsDeleted && a.Slug == "initech-app", ct); + Assert.Equal("Twins App", app.DisplayName); + }); + } } [Fact] @@ -141,8 +165,9 @@ public async Task Apply_with_prune_true_removes_a_client_absent_from_the_manifes var client = await factory.CreateRealmAdminAndLoginAsync(); const string slug = "pruneep"; - Assert.Equal(HttpStatusCode.Created, (await client.PostAsJsonAsync( - "/api/admin/realms/import", BuildManifest(slug, "Prune EP"), factory.JsonOptions, ct)).StatusCode); + await CreateRealmAsync(client, slug, factory.JsonOptions, ct); + Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( + $"/api/admin/realms/{slug}/apply", BuildManifest(slug, "Prune EP"), factory.JsonOptions, ct)).StatusCode); // Re-apply with ?prune=true a manifest that drops the client → it must be pruned. var withoutClient = BuildManifest(slug, "Prune EP") with { Clients = [] }; @@ -181,13 +206,14 @@ public async Task Manifest_schema_endpoint_returns_a_described_json_schema_with_ Assert.Equal("object", root.GetProperty("type").GetString()); Assert.True(root.TryGetProperty("$schema", out _)); var props = root.GetProperty("properties"); - foreach (var section in new[] { "Realm", "Settings", "Apps", "Apis", "Scopes", "Clients", "Roles", "Users", "Groups" }) + foreach (var section in new[] { "Settings", "Apps", "Apis", "Scopes", "Clients", "Roles", "Users", "Groups" }) Assert.True(props.TryGetProperty(section, out _), $"schema missing '{section}'"); - // Only the realm shell is required; the entity lists default to empty. - var required = root.GetProperty("required").EnumerateArray().Select(e => e.GetString()).ToList(); - Assert.Contains("Realm", required); - Assert.DoesNotContain("Apps", required); + // A manifest is content only — no realm shell, and nothing is required: every section + // defaults to empty, so a partial manifest is a first-class payload. + Assert.False(props.TryGetProperty("Realm", out _), "the schema must not describe a realm shell"); + if (root.TryGetProperty("required", out var required)) + Assert.DoesNotContain("Realm", required.EnumerateArray().Select(e => e.GetString())); // Field-level [Description]s are injected (proves the docs ride along). Assert.Contains("permission namespace", props.GetProperty("Apps").GetProperty("description").GetString()); @@ -196,7 +222,7 @@ public async Task Manifest_schema_endpoint_returns_a_described_json_schema_with_ // A worked example is attached so a consumer can author a manifest from the schema alone. var examples = root.GetProperty("examples"); Assert.True(examples.GetArrayLength() >= 1); - Assert.Equal("acme-test", examples[0].GetProperty("Realm").GetProperty("Slug").GetString()); + Assert.Equal("acme", examples[0].GetProperty("Apps")[0].GetProperty("Slug").GetString()); } [Fact] @@ -213,15 +239,25 @@ public async Task Manifest_schema_endpoint_is_gated_for_an_unauthenticated_calle Assert.Contains(resp.StatusCode, new[] { HttpStatusCode.Unauthorized, HttpStatusCode.Forbidden }); } + private static CreateRealmDto RealmShell(string slug) => new() + { + Slug = slug, + DisplayName = slug, + Domains = [$"{slug}.localhost"], + }; + + /// Creates the realm shell — the first half of provisioning, now that a manifest + /// carries no realm identity. The second half is a POST to {slug}/apply. + private static async Task CreateRealmAsync( + HttpClient client, string slug, JsonSerializerOptions json, CancellationToken ct) + { + var resp = await client.PostAsJsonAsync("/api/admin/realms", RealmShell(slug), json, ct); + Assert.True(resp.IsSuccessStatusCode, + $"creating realm '{slug}' failed with {(int)resp.StatusCode}: {await resp.Content.ReadAsStringAsync(ct)}"); + } + private static RealmManifest BuildManifest(string slug, string appDisplayName) => new() { - Realm = new CreateRealmDto - { - Slug = slug, - DisplayName = slug, - Domains = [$"{slug}.localhost"], - InitialAdmin = new InitialAdminDto { UserName = "admin", Email = $"admin@{slug}.test" }, - }, Apps = [ new RealmManifestApp diff --git a/src/dotnet/Modgud.Api.Tests/Infrastructure/ColdStartTestBase.cs b/src/dotnet/Modgud.Api.Tests/Infrastructure/ColdStartTestBase.cs index fbc72d15..cc1f7ea2 100644 --- a/src/dotnet/Modgud.Api.Tests/Infrastructure/ColdStartTestBase.cs +++ b/src/dotnet/Modgud.Api.Tests/Infrastructure/ColdStartTestBase.cs @@ -1,5 +1,10 @@ using System.Text.Json; using Cocoar.Configuration.Testing; +using ErrorOr; +using Microsoft.Extensions.DependencyInjection; +using Modgud.Api.Features.Admin.Provisioning; +using Modgud.Application.DTOs.Realms; +using Modgud.Infrastructure.Realms; namespace Modgud.Api.Tests.Infrastructure; @@ -31,4 +36,37 @@ protected ColdStartTestBase(ColdStartFixture fixture) } public void Dispose() => CocoarTestConfiguration.Clear(); + + /// + /// Provisions a realm from scratch: create the shell, then fill it from the manifest. + /// A manifest carries no realm identity, so these are two operations — this helper is + /// the in-process twin of what a caller does over HTTP (POST /api/admin/realms + /// then POST /api/admin/realms/{slug}/apply) and of + /// Modgud.Provisioning.TestKit's ImportRealmAsync. + /// + /// + /// Unlike the TestKit it does NOT tear the realm down when the manifest step fails — + /// tests that assert on a failed apply want to inspect the realm it left behind. + /// + protected static async Task> ProvisionRealmAsync( + ColdStartWebApplicationFactory factory, CreateRealmDto realm, RealmManifest manifest, + CancellationToken ct) + { + var created = await factory.Services + .GetRequiredService().CreateRealmAsync(realm, ct); + if (created.IsError) return created.Errors; + + return await factory.Services + .GetRequiredService().UpdateRealmAsync(realm.Slug, manifest, ct: ct); + } + + /// The boilerplate realm shell most provisioning tests want: one routing + /// domain and an initial admin derived from the slug. + protected static CreateRealmDto Shell(string slug, string? displayName = null) => new() + { + Slug = slug, + DisplayName = displayName ?? slug, + Domains = [$"{slug}.localhost"], + InitialAdmin = new InitialAdminDto { UserName = "boot", Email = $"boot@{slug}.test" }, + }; } diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmConfigEndpoints.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmConfigEndpoints.cs index 588f571b..f24fc93a 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmConfigEndpoints.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmConfigEndpoints.cs @@ -20,7 +20,8 @@ namespace Modgud.Api.Features.Admin.Provisioning; /// and require realm:admin in THAT realm. So a delegated per-realm credential /// (a service account or user holding realm:admin in one realm) can fully manage that realm's /// config + entities, but CANNOT create or delete realms (those stay control-plane-only) and -/// cannot touch any other realm (tenant isolation + the slug guard below). Prune is allowed, +/// cannot touch any other realm — a manifest names no realm at all, so the target is always +/// the host-routed one and there is nothing to aim elsewhere. Prune is allowed, /// but only within the realm and with the same lockout/infra protections as the control-plane /// path (system app, standard scopes, SA clients, and every realm:admin path are never pruned). /// @@ -54,15 +55,13 @@ public static WebApplication MapRealmConfigEndpoints(this WebApplication applica .WithName("RealmConfig_Export") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); - // Dry-run: what WOULD an apply of this manifest change? Same slug guard as apply, - // same ?prune= semantics (adds delete candidates + protections) — writes nothing. + // Dry-run: what WOULD an apply of this manifest change? Same ?prune= semantics + // (adds delete candidates + protections) — writes nothing. group.MapPost("plan", async ( RealmManifest manifest, RealmManifestPlanner planner, CancellationToken ct, bool prune = false) => { - var currentSlug = TenantContext.Current; - if (SlugMismatch(manifest, currentSlug) is { } mismatch) return mismatch; - var scoped = manifest with { Realm = manifest.Realm with { Slug = currentSlug } }; - var result = await planner.PlanAsync(scoped, prune, baseline: null, deletions: null, ct); + var result = await planner.PlanAsync( + TenantContext.Current, manifest, prune, baseline: null, deletions: null, ct); return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Plan") @@ -76,25 +75,12 @@ public static WebApplication MapRealmConfigEndpoints(this WebApplication applica group.MapPost("apply", async ( RealmManifest manifest, RealmManifestApplier applier, CancellationToken ct, bool prune = false) => { - var currentSlug = TenantContext.Current; - - // A realm admin may only manage their OWN realm. A manifest aimed at a different - // slug is refused — this is the data-plane safety boundary (cross-realm writes and - // realm lifecycle stay control-plane-only). - if (!string.IsNullOrEmpty(manifest.Realm.Slug) && - !string.Equals(manifest.Realm.Slug, currentSlug, StringComparison.Ordinal)) - { - return Results.BadRequest(new - { - Error = "Manifest.SlugMismatch", - Message = $"This realm is '{currentSlug}'. A realm admin can only manage their own realm; the manifest targets '{manifest.Realm.Slug}'.", - }); - } - - // Pin the manifest to the current realm (covers an empty slug in the body). The - // realm shell (domains/display name) is not mutated by apply — only in-realm config. - var scoped = manifest with { Realm = manifest.Realm with { Slug = currentSlug } }; - var result = await applier.UpdateRealmAsync(scoped, prune, deletions: null, ct); + // The target is the caller's own realm, always — a manifest carries no realm + // identity, so there is nothing here that could aim at a different one. That + // IS the data-plane safety boundary: cross-realm writes and realm lifecycle + // are only reachable through the control-plane routes. + var result = await applier.UpdateRealmAsync( + TenantContext.Current, manifest, prune, deletions: null, ct); return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Apply") @@ -120,8 +106,6 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) drafts.MapPost("", async ( CreateRealmDraftDto dto, HttpContext http, RealmDraftService service, CancellationToken ct) => { - if (dto.Manifest is not null && SlugMismatch(dto.Manifest, TenantContext.Current) is { } mismatch) - return mismatch; var result = await service.CreateAsync( dto, TenantContext.Current, RequireUserId(http), UserName(http), ct); return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); @@ -141,8 +125,6 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) drafts.MapPut("{id:guid}", async ( Guid id, UpdateRealmDraftDto dto, HttpContext http, RealmDraftService service, CancellationToken ct) => { - if (dto.Manifest is not null && SlugMismatch(dto.Manifest, TenantContext.Current) is { } mismatch) - return mismatch; var result = await service.UpdateAsync( id, dto, TenantContext.Current, RequireUserId(http), UserName(http), ct); return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); @@ -281,20 +263,6 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); } - /// The data-plane safety boundary shared by apply/plan/drafts: a manifest - /// aimed at a different realm slug is refused. - private static IResult? SlugMismatch(RealmManifest manifest, string currentSlug) - { - if (string.IsNullOrEmpty(manifest.Realm.Slug) || - string.Equals(manifest.Realm.Slug, currentSlug, StringComparison.Ordinal)) - return null; - return Results.BadRequest(new - { - Error = "Manifest.SlugMismatch", - Message = $"This realm is '{currentSlug}'. A realm admin can only manage their own realm; the manifest targets '{manifest.Realm.Slug}'.", - }); - } - private static Guid RequireUserId(HttpContext http) => http.GetUserId() ?? throw new InvalidOperationException( "Realm-config draft endpoints require an authenticated user principal."); diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmDraftService.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmDraftService.cs index c1954c5e..3aef0b58 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmDraftService.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmDraftService.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Http.Json; using Microsoft.Extensions.Options; +using Modgud.Infrastructure.Persistence.Tenancy; namespace Modgud.Api.Features.Admin.Provisioning; @@ -60,7 +61,7 @@ public async Task> CreateAsync( var manifest = dto.Source.ToLowerInvariant() switch { "export" => baseline, - "empty" => new RealmManifest { Realm = new() { Slug = slug } }, + "empty" => new RealmManifest(), "manifest" when dto.Manifest is not null => dto.Manifest, "manifest" => null, _ => null, @@ -70,7 +71,7 @@ public async Task> CreateAsync( "Source must be 'export', 'empty', or 'manifest' (with a Manifest payload)."); var secrets = new Dictionary(StringComparer.Ordinal); - manifest = SanitizeManifest(PinSlug(manifest, slug), secrets); + manifest = SanitizeManifest(manifest, secrets); var now = time.GetUtcNow(); var draft = new RealmDraft @@ -116,7 +117,7 @@ public async Task> UpdateAsync( } if (dto.Shared.HasValue) draft.Shared = dto.Shared.Value; if (dto.Manifest is not null) - draft.Manifest = SanitizeManifest(PinSlug(dto.Manifest, slug), draft.Secrets); + draft.Manifest = SanitizeManifest(dto.Manifest, draft.Secrets); draft.LastModifiedBy = userId; draft.LastModifiedByName = userName; @@ -245,7 +246,7 @@ public async Task> StageEntityAsync( draft.Deletions.RemoveAll(d => d.Section == section && d.Key == key); draft.Manifest = SanitizeManifest( - PinSlug(root.Deserialize(json)!, slug), draft.Secrets); + root.Deserialize(json)!, draft.Secrets); Touch(draft, userId, userName); session.Store(draft); await session.SaveChangesAsync(ct); @@ -322,7 +323,7 @@ public async Task> UnstageDeleteAsync( } draft.Manifest = SanitizeManifest( - PinSlug(root.Deserialize(json)!, slug), draft.Secrets); + root.Deserialize(json)!, draft.Secrets); Touch(draft, userId, userName); session.Store(draft); await session.SaveChangesAsync(ct); @@ -431,7 +432,8 @@ public async Task> PlanAsync( var draft = await LoadVisibleAsync(id, userId, ct); if (draft is null) return NotFound; var manifest = MergeSecrets(draft.Manifest, draft.Secrets); - return await planner.PlanAsync(manifest, prune, draft.Baseline, draft.Deletions, ct); + return await planner.PlanAsync( + TenantContext.Current, manifest, prune, draft.Baseline, draft.Deletions, ct); } /// @@ -447,14 +449,16 @@ public async Task> ApplyAsync( if (draft is null) return NotFound; var manifest = MergeSecrets(draft.Manifest, draft.Secrets); - var planResult = await planner.PlanAsync(manifest, prune, draft.Baseline, draft.Deletions, ct); + var planResult = await planner.PlanAsync( + TenantContext.Current, manifest, prune, draft.Baseline, draft.Deletions, ct); if (planResult.IsError) return planResult.Errors; var plan = planResult.Value; var hasErrors = plan.Sections.Any(s => s.Entries.Any(e => e.Action == "error")); if (hasErrors || plan.HasConflicts) return new RealmDraftApplyResult { Refused = true, Plan = plan }; - var applyResult = await applier.UpdateRealmAsync(manifest, prune, draft.Deletions, ct); + var applyResult = await applier.UpdateRealmAsync( + TenantContext.Current, manifest, prune, draft.Deletions, ct); if (applyResult.IsError) return applyResult.Errors; session.Delete(draft); @@ -568,9 +572,6 @@ private RealmManifest MergeSecrets(RealmManifest manifest, IReadOnlyDictionary manifest with { Realm = manifest.Realm with { Slug = slug } }; - private static RealmDraftSummaryDto Summary(RealmDraft d, Guid userId) => new( d.Id, d.Name, d.Shared, d.CreatedBy == userId, d.CreatedByName, d.CreatedAt, d.LastModifiedByName, d.LastModifiedAt, d.Version); diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifest.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifest.cs index 2f838b42..7b5bd28d 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifest.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifest.cs @@ -29,12 +29,15 @@ namespace Modgud.Api.Features.Admin.Provisioning; /// SAME canonical operation the admin UI/API uses, so the manifest path and the manual /// path can never diverge. /// -[Description("A complete, declarative realm configuration. POST to /api/admin/realms/import to create a new realm, or to /{slug}/apply to merge into an existing one (add ?prune=true for a full sync that also deletes entities absent from the manifest). Cross-references use stable keys (app slug, role/user key, permission 'resource:action'), never server ids.")] +[Description("A declarative realm configuration. POST to /api/admin/realms/{slug}/apply (control plane) or /api/admin/realm-config/apply (realm admin) to merge it into that realm; add ?prune=true for a full sync that also deletes entities absent from the manifest. The TARGET realm comes from the route alone — a manifest carries content, never an identity, so the same file applies to any realm. Cross-references use stable keys (app slug, role/user key, permission 'resource:action'), never server ids.")] public sealed record RealmManifest { - /// Realm shell + initial admin (reuses ). - [Description("REQUIRED. The realm shell (slug, display name, routing domains) and its first admin.")] - public required CreateRealmDto Realm { get; init; } + // NOTE: a manifest deliberately carries NO realm shell. The target is named by the + // route, so where a file is applied is decided independently of where it was + // exported — and the shell (slug, routing domains, primary domain) is deployment + // identity that must never travel between environments. Creating a realm is its own + // operation (POST /api/admin/realms), followed by an apply. Older manifests that + // still carry a "Realm" object stay loadable: the property is simply ignored. /// Optional realm settings patch (self-registration, native grants, ...). [Description("Optional. Realm-settings patch (self-registration, registration fields, native grants, branding, auth rate limits, deletion, audit, DCR, CIMD). Omit to keep defaults; only the sections/fields you include are changed. Mirrors the realm-settings PATCH shape.")] diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs index 1c7f64c7..0b756b3e 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs @@ -48,8 +48,13 @@ namespace Modgud.Api.Features.Admin.Provisioning; /// , , the user/group /// command handlers), so the manifest path and the manual path can never drift. /// -/// Tenant routing: the realm shell is created via the global store, then the -/// per-tenant config runs inside TenantContext.Enter(slug) + a fresh DI scope — +/// A manifest describes CONTENT only — it carries no realm shell, so the target is +/// named by the caller (the route) and the same file applies to any realm. Creating a +/// realm is a separate operation (POST /api/admin/realms); this type only ever +/// fills an existing one. +/// +/// Tenant routing: the per-tenant config runs inside TenantContext.Enter(slug) +/// + a fresh DI scope — /// TenantedSessionFactory prefers the AsyncLocal TenantContext over the /// ambient (control-plane) HttpContext. Handlers resolved in that fresh scope /// therefore write to the newly provisioned tenant. @@ -65,53 +70,8 @@ public sealed partial class RealmManifestApplier( ILogger logger) { /// - /// Imports a brand-new realm: the slug must NOT already exist. Provisions the realm - /// shell (tenant DB + seed) then applies the manifest. If any step fails the whole - /// partially-provisioned realm is hard-deleted, so a failed import leaves nothing - /// behind (all-or-nothing). - /// - public async Task> ImportNewRealmAsync( - RealmManifest manifest, CancellationToken ct = default) - { - var slug = manifest.Realm.Slug; - - if (await realms.GetRealmBySlugAsync(slug, ct) is not null) - return Error.Conflict("Realm.AlreadyExists", - $"Realm '{slug}' already exists. Use UpdateRealm to modify an existing realm."); - - var realmResult = await realms.CreateRealmAsync(manifest.Realm, ct); - if (realmResult.IsError) return realmResult.Errors; - var realm = realmResult.Value; - - try - { - var secrets = await ApplyTenantConfigAsync(slug, manifest, ct); - logger.LogInformation( - "Imported realm {Slug}: {Apps} apps, {Apis} apis, {Scopes} scopes, {Clients} clients, {Roles} roles, {Users} users, {Groups} groups, {Providers} login providers, {Positions} positions.", - slug, manifest.Apps.Count, manifest.Apis.Count, manifest.Scopes.Count, - manifest.Clients.Count, manifest.Roles.Count, manifest.Users.Count, manifest.Groups.Count, - manifest.LoginProviders.Count, manifest.Positions.Count); - return new RealmImportResult - { - Slug = slug, - PrimaryDomain = realm.PrimaryDomain, - ClientSecrets = secrets, - }; - } - catch (ManifestApplyException ex) - { - // A failed import must leave nothing behind: roll the whole realm back via - // the prod-safe hard-delete (drops the tenant DB + the global record). - logger.LogError(ex, - "Manifest apply failed for realm {Slug} ({What}); hard-deleting the partially-provisioned realm.", - slug, ex.What); - await realms.HardDeleteRealmAsync(slug, ct); - return ex.Errors; - } - } - - /// - /// Updates an existing realm in place: the slug MUST already exist. Each entity in the + /// Updates the realm named by in place; it MUST already exist + /// (create it via POST /api/admin/realms first). Each entity in the /// manifest is upserted by its natural key (app slug, api/scope/role/group name, client /// id, user email/username) — created if absent, otherwise updated through the SAME /// canonical Update operation the admin API uses. The realm database is NEVER dropped @@ -150,15 +110,13 @@ public async Task> ImportNewRealmAsync( /// beforehand). /// public async Task> UpdateRealmAsync( - RealmManifest manifest, bool prune = false, + string slug, RealmManifest manifest, bool prune = false, IReadOnlyCollection? deletions = null, CancellationToken ct = default) { - var slug = manifest.Realm.Slug; - var realm = await realms.GetRealmBySlugAsync(slug, ct); if (realm is null) return Error.NotFound("Realm.NotFound", - $"Realm '{slug}' does not exist. Use ImportNewRealm to create it."); + $"Realm '{slug}' does not exist. Create it first (POST /api/admin/realms), then apply."); try { @@ -187,184 +145,13 @@ public async Task> UpdateRealmAsync( } } - private async Task> ApplyTenantConfigAsync( - string slug, RealmManifest manifest, CancellationToken ct) - { - var secrets = new Dictionary(StringComparer.Ordinal); - var apps = new Dictionary(StringComparer.Ordinal); // slug → App (id + catalog) - var roleIds = new Dictionary(StringComparer.Ordinal); // role key → id (for groups) - var userIds = new Dictionary(StringComparer.Ordinal); // user key → id (for groups) - - // Enter the new realm's tenant context, then resolve the per-tenant services in - // a FRESH scope so their IDocumentSession binds to this tenant. - using var _ = TenantContext.Enter(slug); - using var scope = scopeFactory.CreateScope(); - var sp = scope.ServiceProvider; - - if (manifest.Settings is not null) - EnsureOk(await sp.GetRequiredService().PatchAsync(manifest.Settings, ct), "settings"); - - // ── Apps (+ permission catalog) — referenced by everything below ────────── - var appAdmin = sp.GetRequiredService(); - foreach (var app in manifest.Apps) - { - var dto = new CreateAppDto(app.Slug, app.DisplayName, OrNull(app.Description), - (app.Permissions ?? []).Select(p => new AppPermissionDto(null, p.Resource, p.Action, p.Description)).ToList(), - app.Settings, app.Id); - var created = await appAdmin.CreateAppAsync(dto, ct); - EnsureOk(created, $"app '{app.Slug}'"); - apps[app.Slug] = created.Value; - } - - var oauth = sp.GetRequiredService(); - - // ── OAuth APIs ──────────────────────────────────────────────────────────── - foreach (var api in manifest.Apis) - { - EnsureOk(await oauth.CreateApiAsync(new CreateOAuthApiDto - { - Id = api.Id, - Name = api.Name, - DisplayName = OrNull(api.DisplayName), - Description = OrNull(api.Description), - Enabled = api.Enabled ?? true, - Scopes = api.Scopes ?? [], - UserClaims = api.UserClaims ?? [], - AppId = ResolveAppId(apps, OrNull(api.App), $"api '{api.Name}'"), - PermissionIds = ResolvePermissionIds(apps, OrNull(api.App), api.Permissions, $"api '{api.Name}'"), - AllowDynamicRegistration = api.AllowDynamicRegistration ?? false, - }, ct), $"api '{api.Name}'"); - } - - // ── OAuth scopes ────────────────────────────────────────────────────────── - foreach (var s in manifest.Scopes) - { - EnsureOk(await oauth.CreateScopeAsync(new CreateOAuthScopeDto - { - Id = s.Id, - Name = s.Name, - DisplayName = OrNull(s.DisplayName), - Description = OrNull(s.Description), - Resources = s.Resources ?? [], - UserClaims = s.UserClaims ?? [], - Enabled = s.Enabled ?? true, - Required = s.Required ?? false, - Emphasize = s.Emphasize ?? false, - ShowInDiscoveryDocument = s.ShowInDiscoveryDocument ?? true, - AllowDynamicRegistrationClients = s.AllowDynamicRegistrationClients ?? false, - AppId = ResolveAppId(apps, OrNull(s.App), $"scope '{s.Name}'"), - }, ct), $"scope '{s.Name}'"); - } - - // ── OAuth clients ───────────────────────────────────────────────────────── - foreach (var c in manifest.Clients) - { - var created = await oauth.CreateClientAsync( - BuildClientCreateDto(c, apps, $"client '{c.ClientId}'"), ct); - EnsureOk(created, $"client '{c.ClientId}'"); - if (created.Value.ClientSecret is not null) - secrets[c.ClientId] = created.Value.ClientSecret; - } - - // ── Login providers — canonical handler on the manifest's tenant session ── - foreach (var lp in manifest.LoginProviders) - { - var ctx = $"login provider '{lp.Slug}'"; - var pinnedLp = await ResolvePinnedAsync( - sp.GetRequiredService(), lp.Id, "LoginProvider", ctx, - x => x.IsDeleted, ct); - EnsureOk(await BuildCreateProviderHandler(sp).Handle( - BuildCreateProviderCommand(lp, ctx, pinnedLp), ct), ctx); - } - - // ── Roles (app-scoped or realm-admin) ───────────────────────────────────── - var roleAdmin = sp.GetRequiredService(); - foreach (var r in manifest.Roles) - { - var payload = new RolePayload( - r.Name, - OrNull(r.Description), - ResolveAppId(apps, r.App, $"role '{r.Name}'"), - r.IsRealmAdmin ?? false, - ResolvePermissionIds(apps, r.App, r.Permissions, $"role '{r.Name}'"), - r.Id); - // Control-plane provisioning is trusted, so the realm-admin guard is satisfied. - var created = await roleAdmin.CreateRoleAsync(payload, callerIsRealmAdmin: true, ct); - EnsureOk(created, $"role '{r.Name}'"); - RegisterRole(roleIds, manifest.Roles, r, created.Value.Id); - } - - // ── Users — canonical handler on the manifest's tenant session ──────────── - // Realm provisioning has already initialized Wolverine's inbox/outbox. - // Direct invocation keeps manifest application sequential and exposes the - // canonical handler result immediately for contextual import errors. - var userSession = sp.GetRequiredService(); - var createUser = new CreateUserHandler( - userSession, - sp.GetRequiredService>(), - sp.GetRequiredService()); - foreach (var u in manifest.Users) - { - var ctx = $"user '{u.Email}'"; - var cmd = new CreateUserCommand(OrNull(u.Firstname), OrNull(u.Lastname), OrNull(u.Acronym), u.Email, - u.UserName ?? string.Empty, u.Password, u.EmailConfirmed ?? false, - Id: await ResolvePinnedUserAsync(userSession, u.Id, ctx, ct)); - var created = await createUser.Handle(cmd, ct); - EnsureOk(created, ctx); - if (ShortGuid.TryParse(created.Value.Id, out Guid uid)) - userIds[u.ResolveKey()] = uid; - } - - // ── Groups — canonical handler on the manifest's tenant session ─────────── - // Keep the same explicit, sequential dispatch used for users so reference - // resolution and contextual import failures remain deterministic. - if (manifest.Groups.Count > 0) - { - var groupSession = sp.GetRequiredService(); - var groupHandler = new CreateGroupHandler( - groupSession, - sp.GetRequiredService(), - sp.GetRequiredService()); - - foreach (var g in manifest.Groups) - { - var memberIds = new List((g.Members ?? []).Count); - foreach (var m in g.Members ?? []) - memberIds.Add(await ResolveUserRefAsync(groupSession, userIds, m, $"group '{g.Name}' member '{m}'", ct)); - var groupRoleIds = new List((g.Roles ?? []).Count); - foreach (var rk in g.Roles ?? []) - groupRoleIds.Add(await ResolveRoleRefAsync(groupSession, roleIds, rk, $"group '{g.Name}' role '{rk}'", ct)); - var pinnedGroup = await ResolvePinnedAsync( - groupSession, g.Id, "Group", $"group '{g.Name}'", x => x.IsDeleted, ct); - var cmd = new CreateGroupCommand( - g.Name, OrNull(g.Description), memberIds, groupRoleIds, - ParseEnum(g.MembershipMode ?? "Manual", $"group '{g.Name}' membershipMode"), - g.MembershipScript, OrNull(g.Email), - ParseEnum(g.EmailMode ?? "Shared", $"group '{g.Name}' emailMode"), - // Mirror the create endpoint's default (GroupEndpoints: dto.BoundTo ?? [Modgud]) - // so a manifest group is bound to the IdP and actually confers its roles — - // CreateGroupHandler itself defaults null to [] (dormant), which would make an - // imported admin group silently grant nothing. - g.BoundTo ?? [AppSlugs.Modgud], g.ExternallyDrivable ?? false, CallerIsRealmAdmin: true, - Id: pinnedGroup.Id, ReviveExistingStream: pinnedGroup.Revive); - EnsureOk(await groupHandler.Handle(cmd, ct), $"group '{g.Name}'"); - } - } - - // ── Service accounts (hulls, id-pinned creates) ─────────────────────────── - await ApplyServiceAccountsAsync(sp, manifest, ct); - - // ── Positions (MG-FT) — after users so grants can resolve their keys ────── - await ApplyPositionsAsync(sp, manifest, userIds, ct); - - return secrets; - } - /// - /// In-place upsert of every entity in the manifest against an already-provisioned realm. - /// Mirrors but reads current state by natural key - /// and dispatches to the canonical Update op when the entity exists, the Create op when - /// it doesn't. See for the field-level merge semantics. + /// In-place upsert of every entity in the manifest against an already-provisioned realm: + /// reads current state by natural key and dispatches to the canonical Update op when the + /// entity exists, the Create op when it doesn't. This is the ONLY apply path — a fresh + /// realm is created first (POST /api/admin/realms) and then filled through exactly these + /// upserts, so provisioning and merging can never drift apart. See + /// for the field-level merge semantics. /// private async Task> ApplyTenantUpdateAsync( string slug, RealmManifest manifest, bool prune, diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestExporter.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestExporter.cs index 5beeb842..c1688f56 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestExporter.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestExporter.cs @@ -313,15 +313,9 @@ public async Task> ExportRealmAsync(string slug, Cancella return new RealmManifest { - Realm = new CreateRealmDto - { - Slug = realm.Slug, - DisplayName = realm.DisplayName, - Description = realm.Description, - Domains = realm.Domains, - PrimaryDomain = realm.PrimaryDomain, - // InitialAdmin is meaningless for an existing realm; left default (ignored on apply). - }, + // No realm shell: slug, routing domains and primary domain are deployment + // identity, not configuration. Leaving them out is what makes an export + // portable — the target realm is chosen at apply time, by the route. Settings = settings, Apps = manifestApps, Apis = manifestApis, diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs index bc6eab52..742c19bc 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs @@ -60,7 +60,7 @@ public sealed class RealmManifestPlanner( /// modify/delete conflict). /// public async Task> PlanAsync( - RealmManifest manifest, bool prune, RealmManifest? baseline = null, + string slug, RealmManifest manifest, bool prune, RealmManifest? baseline = null, IReadOnlyCollection? deletions = null, CancellationToken ct = default) { var deleteKeys = (deletions ?? []) @@ -69,14 +69,12 @@ public async Task> PlanAsync( StringComparer.Ordinal); HashSet? DeletesFor(string section) => deleteKeys.GetValueOrDefault(section); - var slug = manifest.Realm.Slug; var exported = await exporter.ExportRealmAsync(slug, ct); if (exported.IsError) return exported.Errors; var current = exported.Value; var json = jsonOptions.Value.SerializerOptions; var result = new RealmPlanResult { Slug = slug, Prune = prune }; - AddRealmShellWarnings(manifest, current, result.Warnings); // The protection checks for prune candidates (does this user/group confer // realm:admin?) need tenant-scoped queries — same scoping as the exporter. @@ -288,25 +286,6 @@ await GroupMembershipGuards.GroupConfersRealmAdminAsync(session, perms, doc, ct) return result; } - // ── Realm shell — apply never mutates it; differing values are warnings. ───────── - - private static void AddRealmShellWarnings(RealmManifest manifest, RealmManifest current, List warnings) - { - var ignored = new List(); - if (!string.IsNullOrEmpty(manifest.Realm.DisplayName) && - !string.Equals(manifest.Realm.DisplayName, current.Realm.DisplayName, StringComparison.Ordinal)) - ignored.Add("DisplayName"); - if (manifest.Realm.Domains is { Length: > 0 } && - !manifest.Realm.Domains.ToHashSet(StringComparer.Ordinal) - .SetEquals(current.Realm.Domains ?? [])) - ignored.Add("Domains"); - if (!string.IsNullOrEmpty(manifest.Realm.PrimaryDomain) && - !string.Equals(manifest.Realm.PrimaryDomain, current.Realm.PrimaryDomain, StringComparison.Ordinal)) - ignored.Add("PrimaryDomain"); - if (ignored.Count > 0) - warnings.Add($"The realm shell is not modified by apply — differing value(s) for {string.Join(", ", ignored)} are ignored."); - } - // ── Settings — one pseudo-entity, nested patch diff with dotted paths. ─────────── private static RealmPlanSection PlanSettings( diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestSchema.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestSchema.cs index 6995e066..e5c37a0c 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestSchema.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestSchema.cs @@ -126,12 +126,6 @@ private static JsonNode InjectDescriptions(JsonSchemaExporterContext context, Js private static JsonNode Example() => JsonNode.Parse( """ { - "Realm": { - "Slug": "acme-test", - "DisplayName": "Acme Test", - "Domains": ["acme-test.localhost"], - "InitialAdmin": { "UserName": "admin", "Email": "admin@acme-test.local" } - }, "Apps": [ { "Slug": "acme", "DisplayName": "Acme", "Permissions": [ { "Resource": "invoice", "Action": "read" }, diff --git a/src/dotnet/Modgud.Api/Features/Admin/RealmsEndpoints.cs b/src/dotnet/Modgud.Api/Features/Admin/RealmsEndpoints.cs index 26605d31..3c680fe9 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/RealmsEndpoints.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/RealmsEndpoints.cs @@ -302,30 +302,12 @@ await RecordControlPlaneRealmOperationAsync( .RequiresPermission("realm:write", AppSlugs.ControlPlane); // ── Declarative provisioning (RealmManifestApplier) ───────────────────────── - // Import a brand-new realm from a complete manifest (realm + settings + apps + - // apis + scopes + clients + roles + users + groups), all via the canonical admin - // operations. The slug must NOT already exist; a failed import rolls the whole - // realm back (hard-delete). Returns the created slug + primary domain + the - // plaintext secrets of any confidential clients (only available at create time). - group.MapPost("import", async ( - RealmManifest manifest, - RealmManifestApplier applier, - HttpContext http, - ISecurityAuditLog securityAudit, - CancellationToken ct) => - { - var result = await applier.ImportNewRealmAsync(manifest, ct); - if (result.IsError) return ManifestError(result.Errors); - ModgudMeters.RecordRealmProvisioned(); - await RecordControlPlaneRealmOperationAsync( - securityAudit, - http, - result.Value.Slug, - "import-realm"); - return Results.Created($"{path}/admin/realms/{result.Value.Slug}", result.Value); - }) - .WithName("Realms_Import") - .RequiresPermission("realm:write", AppSlugs.ControlPlane); + // A realm is created by POST "" above; a manifest then FILLS it via {slug}/apply. + // There is deliberately no combined "import" endpoint: a manifest carries content + // only (no realm shell), so the two steps cannot be collapsed without smuggling a + // realm identity into the file — and keeping them apart means a failed apply rolls + // back inside its own transaction and leaves the realm intact to retry against, + // instead of tearing down the tenant database over a typo. // Apply a manifest to an EXISTING realm: in-place merge/upsert per entity (never // drops the DB). The route slug must match the manifest's realm slug. Default is an @@ -341,14 +323,7 @@ await RecordControlPlaneRealmOperationAsync( CancellationToken ct, bool prune = false) => { - if (!string.Equals(slug, manifest.Realm.Slug, StringComparison.Ordinal)) - return Results.BadRequest(new - { - Error = "Manifest.SlugMismatch", - Message = $"Route slug '{slug}' does not match the manifest realm slug '{manifest.Realm.Slug}'.", - }); - - var result = await applier.UpdateRealmAsync(manifest, prune, deletions: null, ct); + var result = await applier.UpdateRealmAsync(slug, manifest, prune, deletions: null, ct); if (!result.IsError) { await RecordControlPlaneRealmOperationAsync( diff --git a/src/dotnet/Modgud.Provisioning.TestKit/ModgudProvisioningClient.cs b/src/dotnet/Modgud.Provisioning.TestKit/ModgudProvisioningClient.cs index 43ea0a32..281e097d 100644 --- a/src/dotnet/Modgud.Provisioning.TestKit/ModgudProvisioningClient.cs +++ b/src/dotnet/Modgud.Provisioning.TestKit/ModgudProvisioningClient.cs @@ -8,9 +8,9 @@ namespace Modgud.Provisioning.TestKit; /// /// Thin client over the Modgud control-plane provisioning API. Wraps an /// the caller has already pointed at a running Modgud instance and -/// authenticated as a control-plane admin (cookie or bearer). The only entry point is -/// , which provisions a fresh realm and hands back a -/// disposable handle. +/// authenticated as a control-plane admin (cookie or bearer). The entry point is +/// , which creates a fresh realm, fills it from a manifest and +/// hands back a disposable handle. /// public sealed class ModgudProvisioningClient { @@ -30,26 +30,51 @@ public ModgudProvisioningClient(HttpClient http) => _http = http ?? throw new ArgumentNullException(nameof(http)); /// - /// Provisions a brand-new realm from (the slug must not - /// already exist) and returns a handle that hard-deletes the realm on dispose. Throws - /// if the server rejects the import. + /// Provisions a brand-new realm: creates the realm named by + /// (its slug must not already exist), then fills it from . + /// Returns a handle that hard-deletes the realm on dispose. Throws + /// if the server rejects either step. + /// + /// The realm shell is a SEPARATE argument because a manifest deliberately carries + /// no realm identity — the same manifest file provisions any realm you name here. Two + /// server calls, one method: the kit keeps the convenience without the file having to + /// know where it will land. + /// + /// If the manifest step fails, the just-created realm is torn down before the + /// exception propagates, so a failed provision leaves no orphan behind for the next + /// test run. (The server itself no longer does this — driving the API directly keeps + /// the realm so a corrected manifest can simply be re-applied.) /// public async Task ImportRealmAsync( - RealmManifest manifest, CancellationToken ct = default) + RealmSpec realm, RealmManifest manifest, CancellationToken ct = default) { + ArgumentNullException.ThrowIfNull(realm); ArgumentNullException.ThrowIfNull(manifest); - using var response = await _http.PostAsJsonAsync( - "api/admin/realms/import", manifest, JsonOptions, ct); - var result = await ReadResultOrThrowAsync(response, "import", manifest.Realm.Slug, ct); - return new ProvisionedRealm(this, result); + using (var created = await _http.PostAsJsonAsync("api/admin/realms", realm, JsonOptions, ct)) + { + if (!created.IsSuccessStatusCode) + await ThrowFromResponseAsync(created, "create-realm", realm.Slug, ct); + } + + try + { + return new ProvisionedRealm(this, await ApplyAsync(realm.Slug, manifest, ct)); + } + catch + { + // Best-effort teardown: never let a cleanup failure mask the real error. + try { await HardDeleteAsync(realm.Slug, ct); } catch { /* ignored */ } + throw; + } } - internal async Task ApplyAsync(string slug, RealmManifest manifest, CancellationToken ct) + internal async Task ApplyAsync( + string slug, RealmManifest manifest, CancellationToken ct) { using var response = await _http.PostAsJsonAsync( $"api/admin/realms/{slug}/apply", manifest, JsonOptions, ct); - await ReadResultOrThrowAsync(response, "apply", slug, ct); + return await ReadResultOrThrowAsync(response, "apply", slug, ct); } internal async Task HardDeleteAsync(string slug, CancellationToken ct) @@ -79,9 +104,21 @@ private static async Task ThrowFromResponseAsync( var body = await response.Content.ReadAsStringAsync(ct); try { - var error = JsonSerializer.Deserialize(body, JsonOptions); - code = error?.Error; - message = error?.Message; + // Two error shapes reach this client, both using a field named "error": + // manifest endpoints → { "Error": "", "Message": "" } + // canonical ErrorOr → { "error": "" } (no code at all) + // Telling them apart by whether "Message" is present keeps a description from + // masquerading as an error code on the second shape. + var error = JsonSerializer.Deserialize(body, JsonOptions); + if (error?.Message is { Length: > 0 }) + { + code = error.Error; + message = error.Message; + } + else + { + message = error?.Error; + } } catch (JsonException) { /* non-JSON body — fall back to the raw text below */ } @@ -89,7 +126,7 @@ private static async Task ThrowFromResponseAsync( message ?? $"Realm {op} for '{slug}' failed with {(int)response.StatusCode}: {body}"); } - private sealed record ManifestErrorBody(string? Error, string? Message); + private sealed record ErrorBody(string? Error, string? Message); } /// The successful-provisioning response: the realm's slug + canonical host and the @@ -101,9 +138,9 @@ public sealed record RealmImportResult public Dictionary ClientSecrets { get; init; } = []; } -/// Thrown when the provisioning API rejects an import / apply / hard-delete. Carries +/// Thrown when the provisioning API rejects a create / apply / hard-delete. Carries /// the HTTP status and the server's error (e.g. Realm.AlreadyExists, -/// Realm.NotFound, Manifest.SlugMismatch) when present. +/// Realm.NotFound, Manifest.UnknownReference) when present. public sealed class ModgudProvisioningException( HttpStatusCode statusCode, string operation, string slug, string? code, string message) : Exception(message) diff --git a/src/dotnet/Modgud.Provisioning.TestKit/ProvisionedRealm.cs b/src/dotnet/Modgud.Provisioning.TestKit/ProvisionedRealm.cs index 58814a44..34c830a4 100644 --- a/src/dotnet/Modgud.Provisioning.TestKit/ProvisionedRealm.cs +++ b/src/dotnet/Modgud.Provisioning.TestKit/ProvisionedRealm.cs @@ -44,15 +44,10 @@ public string SecretFor(string clientId) $"No client secret for '{clientId}' in realm '{Slug}'. Known clients: {string.Join(", ", ClientSecrets.Keys)}."); /// Applies to this realm in place (merge/upsert). - /// The manifest's realm slug must match this realm. New confidential-client secrets are - /// NOT surfaced — existing clients keep their secret. + /// A manifest names no realm, so it always targets THIS one — nothing to mismatch. New + /// confidential-client secrets are NOT surfaced — existing clients keep their secret. public Task ApplyAsync(RealmManifest manifest, CancellationToken ct = default) - { - if (!string.Equals(manifest.Realm.Slug, Slug, StringComparison.Ordinal)) - throw new ArgumentException( - $"Manifest realm slug '{manifest.Realm.Slug}' does not match this realm '{Slug}'.", nameof(manifest)); - return _client.ApplyAsync(Slug, manifest, ct); - } + => _client.ApplyAsync(Slug, manifest, ct); /// Hard-deletes the realm (drops the tenant database). Idempotent — a second call /// is a no-op. Called automatically by . diff --git a/src/dotnet/Modgud.Provisioning.TestKit/RealmManifest.cs b/src/dotnet/Modgud.Provisioning.TestKit/RealmManifest.cs index 4b2ee5ab..1fa63f1a 100644 --- a/src/dotnet/Modgud.Provisioning.TestKit/RealmManifest.cs +++ b/src/dotnet/Modgud.Provisioning.TestKit/RealmManifest.cs @@ -7,16 +7,16 @@ namespace Modgud.Provisioning.TestKit; /// control-plane provisioning API. This is the client-side mirror of the server's manifest /// contract — cross-references use stable KEYS (apps by slug, roles/users by key, /// permissions by resource:action), never server-generated ids. The JSON shape is -/// what POST /api/admin/realms/import and POST /{slug}/apply bind; the -/// round-trip is exercised end-to-end by the IdP repo's own provisioning tests so the two -/// sides can't silently drift. +/// what POST /api/admin/realms/{slug}/apply binds; the round-trip is exercised +/// end-to-end by the IdP repo's own provisioning tests so the two sides can't silently drift. +/// +/// A manifest describes CONTENT only and carries no realm shell — the target realm is +/// named by the route, so the same file applies to any realm. Create the realm separately +/// with a (see +/// ). /// public sealed record RealmManifest { - /// Realm shell + (for import) the initial-admin placeholder. On apply only - /// is read. - public required RealmSpec Realm { get; init; } - /// Optional raw realm-settings patch (self-registration, native grants, …). /// Left as a free-form JSON object so the kit doesn't have to mirror the full settings /// surface; null = no settings change. @@ -31,6 +31,10 @@ public sealed record RealmManifest public List Groups { get; init; } = []; } +/// The realm shell — the payload of POST /api/admin/realms. Deliberately NOT +/// part of : slug, routing domains and primary domain are +/// deployment identity, and keeping them out of the manifest is what makes a manifest +/// portable between realms and environments. public sealed record RealmSpec { public required string Slug { get; init; } @@ -41,9 +45,9 @@ public sealed record RealmSpec public InitialAdmin InitialAdmin { get; init; } = new(); } -/// Initial-admin placeholder. Required JSON-shape-wise on import (the realm shell -/// reuses the create-realm DTO) but ignored by the manifest flow, which provisions admins -/// directly via + . +/// The realm's first admin, issued as a pending admin invite by the create-realm +/// call. Optional — a manifest can equally provision admins outright via +/// + . public sealed record InitialAdmin { public string UserName { get; init; } = "admin"; diff --git a/src/frontend-vue/public/i18n/de.json b/src/frontend-vue/public/i18n/de.json index 7d165bea..c4815956 100644 --- a/src/frontend-vue/public/i18n/de.json +++ b/src/frontend-vue/public/i18n/de.json @@ -1820,8 +1820,7 @@ "selective": { "title": "Selektiver Export", "subTitle": "Entities auswählen — Referenzen werden automatisch mitgenommen", - "hint": "Secrets werden nie exportiert. Das Teil-Manifest auf dem Ziel OHNE Prune anwenden — mit Prune würde alles gelöscht, was nicht in der Datei steht.", - "targetSlug": "Ziel-Realm-Slug", + "hint": "Secrets werden nie exportiert. Die Datei benennt keinen Realm — das Ziel wählst du beim Import. Auf dem Ziel OHNE Prune anwenden; mit Prune würde alles gelöscht, was nicht in der Datei steht.", "includeSettings": "Realm-Einstellungen einschließen", "includeUsers": "User-Referenzen einschließen (Gruppen-Mitglieder, Positions-Grants)", "download": "Manifest herunterladen", diff --git a/src/frontend-vue/src/stores/realmDraft.store.ts b/src/frontend-vue/src/stores/realmDraft.store.ts index 878a1dc4..14d1cadc 100644 --- a/src/frontend-vue/src/stores/realmDraft.store.ts +++ b/src/frontend-vue/src/stores/realmDraft.store.ts @@ -17,7 +17,6 @@ import { useHttpClient, HttpClientError } from '@/composables/useHttpClient' export type ManifestEntity = Record export interface DraftManifest extends ManifestEntity { - Realm?: ManifestEntity Settings?: ManifestEntity | null Apps?: ManifestEntity[] Apis?: ManifestEntity[] diff --git a/src/frontend-vue/src/views/admin/realm-config/SelectiveExportModal.vue b/src/frontend-vue/src/views/admin/realm-config/SelectiveExportModal.vue index 3a577f82..4b339162 100644 --- a/src/frontend-vue/src/views/admin/realm-config/SelectiveExportModal.vue +++ b/src/frontend-vue/src/views/admin/realm-config/SelectiveExportModal.vue @@ -9,7 +9,7 @@ * the plan and applies. */ import { computed, ref } from 'vue' -import { CoarButton, CoarCheckbox, CoarIcon, CoarNotice, CoarTag, CoarTextInput } from '@cocoar/vue-ui' +import { CoarButton, CoarCheckbox, CoarIcon, CoarNotice, CoarTag } from '@cocoar/vue-ui' import { useI18n } from '@cocoar/vue-localization' import ModalLayout from '@/components/ModalLayout.vue' import { SECTION_META, type DraftManifest, type ManifestEntity } from '@/stores/realmDraft.store' @@ -38,7 +38,6 @@ const { t } = useI18n() const includeUsers = ref(false) const includeSettings = ref(false) -const targetSlug = ref(String(props.manifest.Realm?.Slug ?? '')) function keyExists(sel: SelectionKey): boolean { const slash = sel.indexOf('/') @@ -157,7 +156,7 @@ function selectAppBundle(appKey: string) { } const totalCount = computed(() => effectiveSelection.value.size + (includeSettings.value ? 1 : 0)) -const canDownload = computed(() => totalCount.value > 0 && targetSlug.value.trim().length > 0) +const canDownload = computed(() => totalCount.value > 0) // ── Download ───────────────────────────────────────────────────────────────── @@ -166,13 +165,12 @@ function download() { const partial = buildSelectiveManifest(props.manifest, effectiveSelection.value, { includeUsers: includeUsers.value, includeSettings: includeSettings.value, - targetSlug: targetSlug.value.trim(), }) const url = URL.createObjectURL(new Blob( [JSON.stringify(partial, null, 2)], { type: 'application/json' })) const anchor = document.createElement('a') anchor.href = url - anchor.download = `realm-${targetSlug.value.trim()}-partial.json` + anchor.download = 'realm-partial.json' anchor.click() URL.revokeObjectURL(url) props.close() @@ -203,7 +201,7 @@ function sectionLabel(name: string): string {
{{ t('admin.realmConfig.selective.hint', {}, - 'Secrets are never exported. Apply the partial manifest on the target WITHOUT prune — with prune it would delete everything not in the file.') }} + 'Secrets are never exported. The file names no realm — you pick the target when you import it. Apply it WITHOUT prune; with prune it would delete everything not in the file.') }} {{ t('admin.realmConfig.selective.dropped', { count: droppedCount }, @@ -211,10 +209,6 @@ function sectionLabel(name: string): string {
-
- - -
@@ -275,16 +269,6 @@ function sectionLabel(name: string): string { align-items: flex-end; gap: 16px; } -.option-slug { - display: flex; - flex-direction: column; - gap: 4px; - min-width: 220px; -} -.option-label { - font-size: 12px; - color: var(--coar-text-secondary, #6b7280); -} .selective-section { border: 1px solid var(--coar-border-neutral, #e5e7eb); border-radius: 8px; diff --git a/src/frontend-vue/src/views/admin/realm-config/manifestSelection.ts b/src/frontend-vue/src/views/admin/realm-config/manifestSelection.ts index bdf94ee5..b8c40ce6 100644 --- a/src/frontend-vue/src/views/admin/realm-config/manifestSelection.ts +++ b/src/frontend-vue/src/views/admin/realm-config/manifestSelection.ts @@ -175,17 +175,14 @@ export function relatedToApp(manifest: DraftManifest, appSlug: string): EntityRe } export interface BuildOptions extends ClosureOptions { - /** Realm slug written into the manifest — the apply guard on the target - * rejects a foreign slug, so this is set to the TARGET realm. */ - targetSlug: string /** Include the realm-settings patch. Off by default. */ includeSettings: boolean } /** * Builds the partial manifest for the given selection (checked + closure). - * The Realm shell is reduced to the slug (everything else in it is ignored on - * apply and would only produce warnings). When user references are excluded, + * The file carries CONTENT only — no realm shell, so where it is applied is + * decided at import time, not here. When user references are excluded, * group `Members` and position `Grants` are stripped ENTIRELY — under v2 * merge-patch an absent list means "unchanged" on the target (empty on * create), while `[]` would actively clear it. @@ -195,7 +192,7 @@ export function buildSelectiveManifest( selectedWithClosure: ReadonlySet, opts: BuildOptions, ): DraftManifest { - const out: DraftManifest = { Realm: { Slug: opts.targetSlug } } + const out: DraftManifest = {} if (opts.includeSettings && manifest.Settings) { out.Settings = JSON.parse(JSON.stringify(manifest.Settings)) as ManifestEntity } From d99440467e8e6c5234ffeb3b3e4976bcfd11921b Mon Sep 17 00:00:00 2001 From: Bernhard Windisch Date: Wed, 9 Sep 2026 10:48:47 +0200 Subject: [PATCH 3/7] feat(provisioning): unresolvable manifest references are skipped and reported, not fatal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A manifest is content, and content travels. Applied to a realm that does not have every app, role or user it names — a partial export, a hand-written file, a realm whose apps moved on — the apply used to throw on the first unknown reference and roll the whole thing back. That made the export UI compensate: it locked dependencies in as "Required", though most of them are not. A client without its app is an ordinary client, and the applier itself seeds its resolver with every app already in the realm, so a file omitting an app the target has resolved fine anyway. The strictness rested on an assumption — that a manifest is always a complete closure — that the format never guaranteed. References now resolve one by one. What resolves is applied, what does not is skipped and recorded. Ten role references where eight exist means eight roles. Two rules keep "skipped" from turning into "cleared": - A reference list stays a REPLACE. Applying [A, B, C] to a group holding [A, B, D] leaves [A, B]: C is skipped, D goes because the manifest did not ask for it. Anything else would make removal via manifest impossible. - If a non-empty list resolves to NOTHING, the field is left unchanged rather than written empty. An empty list is an instruction ("clear this"); a failed lookup is not one. Without this, exporting a role without its app would silently strip an existing role of every permission on the target. A role's app is not an ordinary reference and gets its own handling: the app owns the catalog the role grants from, and the domain refuses a role that is neither app-linked nor realm-admin. An existing role (matched by its exported id) keeps the app and permissions it has; a NEW role naming an absent app cannot exist at all, so the whole role is skipped rather than half-created. Skips are reported at both ends, because the result alone cannot show them — a group left with two of three roles looks exactly like a group that asked for two. The apply returns RealmImportResult.SkippedReferences and logs a warning; the plan predicts the same set as per-entry notes, so the review step in front of every draft apply shows what will not land. With nothing genuinely required any more, the selective-export modal stops claiming otherwise: closure entries are pre-checked and labelled "referenced" instead of locked and labelled "required". Keeping the lock while the server no longer needs it would have been a lie about the server; keeping the label while allowing it to be unchecked would have been a lie about the word. Co-Authored-By: Claude Opus 5 --- .../ColdStart/RealmConfigEndpointsTests.cs | 23 +- .../ColdStart/RealmManifestApplierTests.cs | 205 +++++++++++++++++- .../Provisioning/ManifestReferenceSkips.cs | 44 ++++ .../Admin/Provisioning/RealmManifest.cs | 9 + .../RealmManifestApplier.Positions.cs | 14 +- .../Provisioning/RealmManifestApplier.cs | 196 +++++++++++++---- .../Provisioning/RealmManifestPlanner.cs | 121 +++++++++++ src/frontend-vue/public/i18n/de.json | 6 +- .../realm-config/SelectiveExportModal.vue | 81 +++++-- 9 files changed, 614 insertions(+), 85 deletions(-) create mode 100644 src/dotnet/Modgud.Api/Features/Admin/Provisioning/ManifestReferenceSkips.cs diff --git a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmConfigEndpointsTests.cs b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmConfigEndpointsTests.cs index bd37db46..0e91d90c 100644 --- a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmConfigEndpointsTests.cs +++ b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmConfigEndpointsTests.cs @@ -214,27 +214,37 @@ public async Task Apply_is_atomic_a_failing_section_rolls_back_earlier_writes() var ct = TestContext.Current.CancellationToken; var client = await factory.CreateRealmAdminAndLoginAsync(); - // Apps apply before roles. The role references an unknown app, so the apply - // fails AFTER the app section already ran — ADR-0017 Phase 0 demands the + // Apps apply before clients. The client carries an unparseable AccessTokenType, so + // the apply fails AFTER the app section already ran — ADR-0017 Phase 0 demands the // whole transaction rolls back and the app never materializes. + // + // NOTE: an unresolvable REFERENCE is deliberately not usable as the trigger here — + // those are skipped rather than fatal. A malformed value still is: the manifest says + // something the domain cannot mean, as opposed to naming something absent. var manifest = new { - Realm = new { }, Apps = new[] { new { Slug = "atomic-app", DisplayName = "Atomic App", Permissions = new[] { new { Resource = "atomic", Action = "read" } } }, }, - Roles = new[] + Clients = new[] { - new { Name = "Broken Role", App = "no-such-app", Permissions = new object[0] }, + new + { + ClientId = "atomic-web", DisplayName = "Atomic Web", ClientType = "public", + RedirectUris = new[] { "https://atomic.test/cb" }, + Scopes = new[] { "openid" }, + AllowedGrantTypes = new[] { "authorization_code" }, + AccessTokenType = "Bogus", + }, }, }; var resp = await client.PostAsJsonAsync( "/api/admin/realm-config/apply", manifest, factory.JsonOptions, ct); Assert.Equal(HttpStatusCode.BadRequest, resp.StatusCode); - Assert.Contains("Manifest.UnknownApp", await resp.Content.ReadAsStringAsync(ct)); + Assert.Contains("Manifest.InvalidEnum", await resp.Content.ReadAsStringAsync(ct)); await InTenantAsync(factory, TenantConstants.SystemTenantId, async sp => { @@ -246,7 +256,6 @@ await InTenantAsync(factory, TenantConstants.SystemTenantId, async sp => // The realm is untouched, so the SAME manifest minus the broken role applies cleanly. var fixedManifest = new { - Realm = new { }, Apps = new[] { new { Slug = "atomic-app", DisplayName = "Atomic App", diff --git a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs index 9b359849..4ff3eed9 100644 --- a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs +++ b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs @@ -1070,13 +1070,17 @@ async Task ApplyGroupAsync(List? roles = null, List? m // Members follow the same rule. await ApplyGroupAsync(members: [new ManifestRef { Key = "nobody", Id = new ShortGuid(alice).ToString() }]); Assert.Equal([alice], (await GroupAsync()).MemberIds); - // A bare string is ALWAYS a key — even when it happens to be an id. + // A bare string is ALWAYS a key — even when it happens to be an id. It therefore + // resolves to no role and is SKIPPED; and because it was the list's only entry, + // nothing resolved at all, so the stored roles survive instead of being cleared. var idAsKey = await applier.UpdateRealmAsync(slug, new RealmManifest { Groups = [new RealmManifestGroup { Name = "Beta writers", Roles = [new ShortGuid(betaAuthor).ToString()] }], }, ct: ct); - Assert.True(idAsKey.IsError); - Assert.Equal("Manifest.UnknownReference", idAsKey.FirstError.Code); + Assert.False(idAsKey.IsError, idAsKey.IsError ? idAsKey.FirstError.Description : string.Empty); + Assert.Contains(idAsKey.Value.SkippedReferences, + x => x.Contains(new ShortGuid(betaAuthor).ToString())); + Assert.Equal([betaAuthor], (await GroupAsync()).RoleIds); // Rename beta's Author live. The export taken BEFORE still references it by id... await InTenantAsync(factory, slug, async sp => @@ -1196,6 +1200,201 @@ private static RealmManifest BuildGlobexManifest(string slug, int version) }; } + /// + /// A manifest travels, so its references routinely name things the target realm does not + /// have. The apply resolves what it can and SKIPS the rest instead of failing — a client + /// arrives without the app that is not here, a group gets the roles that exist. What was + /// skipped is reported, because the result alone cannot show it (a group left with two of + /// three roles looks exactly like a group that asked for two). + /// + [Fact] + public async Task Unresolvable_references_are_skipped_and_reported_rather_than_failing_the_apply() + { + await using var host = await Fixture.CreateIsolatedHostAsync(); + var factory = host.Factory; + var ct = TestContext.Current.CancellationToken; + var applier = factory.Services.GetRequiredService(); + + const string slug = "skipref"; + var seed = new RealmManifest + { + Apps = + [ + new RealmManifestApp { Slug = "here", DisplayName = "Here", + Permissions = [new RealmManifestPermission("doc", "read")] }, + ], + Roles = [new RealmManifestRole { Name = "Reader", App = "here", + Permissions = [new RealmManifestPermission("doc", "read")] }], + Users = [new RealmManifestUser { Key = "alice", Email = "alice@skipref.test", UserName = "alice" }], + Groups = [new RealmManifestGroup { Name = "Readers", Roles = ["here/Reader"], Members = ["alice"] }], + }; + var seeded = await ProvisionRealmAsync(factory, Shell(slug), seed, ct); + Assert.False(seeded.IsError, seeded.IsError ? seeded.FirstError.Description : string.Empty); + + // Everything below names something this realm does NOT have, alongside something it does. + var mixed = new RealmManifest + { + Clients = + [ + new RealmManifestClient + { + ClientId = "mixed-web", DisplayName = "Mixed", ClientType = "public", + RedirectUris = ["https://mixed.test/cb"], Scopes = ["openid"], + AllowedGrantTypes = ["authorization_code"], + Apps = ["here", "elsewhere"], // one known, one not + }, + ], + Roles = + [ + new RealmManifestRole { Name = "Reader", App = "here", + Permissions = + [ + new RealmManifestPermission("doc", "read"), // in the catalog + new RealmManifestPermission("doc", "publish"), // not in the catalog + ] }, + ], + Groups = + [ + new RealmManifestGroup { Name = "Readers", + Roles = ["here/Reader", "here/Ghost"], Members = ["alice", "nobody"] }, + ], + }; + + var applied = await applier.UpdateRealmAsync(slug, mixed, ct: ct); + Assert.False(applied.IsError, applied.IsError ? applied.FirstError.Description : string.Empty); + + var skipped = applied.Value.SkippedReferences; + Assert.Contains(skipped, x => x.Contains("elsewhere")); + Assert.Contains(skipped, x => x.Contains("doc:publish")); + Assert.Contains(skipped, x => x.Contains("Ghost")); + Assert.Contains(skipped, x => x.Contains("nobody")); + + await InTenantAsync(factory, slug, async sp => + { + var session = sp.GetRequiredService(); + + // The client exists and is linked to the app that DOES exist here. + var app = await session.Query().SingleAsync(a => !a.IsDeleted && a.Slug == "here", ct); + var client = await session.Query() + .SingleAsync(c => !c.IsDeleted && c.ClientId == "mixed-web", ct); + Assert.Equal([app.Id], client.AppIds); + + // The role kept the permission that resolved; the unknown one just dropped. + var role = await session.Query().SingleAsync(r => !r.IsDeleted && r.Name == "Reader", ct); + Assert.Single(role.PermissionIds); + + // The group kept the resolvable role + member. + var group = await session.Query().SingleAsync(g => !g.IsDeleted && g.Name == "Readers", ct); + Assert.Equal([role.Id], group.RoleIds); + Assert.Single(group.MemberIds); + }); + } + + /// + /// A reference list is a REPLACE, and skipping does not turn it into a merge: applying + /// roles [A, B, C] to a group that holds [A, B, D] where C does not exist here leaves + /// [A, B] — C is skipped because it cannot resolve, D goes because the manifest did not + /// ask for it. Anything else would make removal via manifest impossible. + /// + [Fact] + public async Task A_reference_list_stays_a_replace_even_when_part_of_it_is_skipped() + { + await using var host = await Fixture.CreateIsolatedHostAsync(); + var factory = host.Factory; + var ct = TestContext.Current.CancellationToken; + var applier = factory.Services.GetRequiredService(); + + const string slug = "replacelist"; + RealmManifestRole Role(string name) => new() + { + Name = name, App = "rlist", + Permissions = [new RealmManifestPermission("doc", "read")], + }; + var seed = new RealmManifest + { + Apps = [new RealmManifestApp { Slug = "rlist", DisplayName = "RList", + Permissions = [new RealmManifestPermission("doc", "read")] }], + Roles = [Role("A"), Role("B"), Role("D")], + Groups = [new RealmManifestGroup { Name = "G", Roles = ["rlist/A", "rlist/B", "rlist/D"] }], + }; + var seeded = await ProvisionRealmAsync(factory, Shell(slug), seed, ct); + Assert.False(seeded.IsError, seeded.IsError ? seeded.FirstError.Description : string.Empty); + + var applied = await applier.UpdateRealmAsync(slug, new RealmManifest + { + Groups = [new RealmManifestGroup { Name = "G", Roles = ["rlist/A", "rlist/B", "rlist/C"] }], + }, ct: ct); + Assert.False(applied.IsError, applied.IsError ? applied.FirstError.Description : string.Empty); + Assert.Contains(applied.Value.SkippedReferences, x => x.Contains("rlist/C")); + + await InTenantAsync(factory, slug, async sp => + { + var session = sp.GetRequiredService(); + var roles = (await session.Query().Where(r => !r.IsDeleted).ToListAsync(ct)) + .ToDictionary(r => r.Name, r => r.Id, StringComparer.Ordinal); + var group = await session.Query().SingleAsync(g => !g.IsDeleted && g.Name == "G", ct); + + Assert.Equal([roles["A"], roles["B"]], group.RoleIds); // C skipped, D removed + }); + } + + /// + /// The one carve-out: when NOTHING in a non-empty reference list resolves, the field is + /// left unchanged instead of written empty. An empty list is an instruction ("clear + /// this") and a failed lookup is not one — otherwise exporting a role without its app + /// would silently strip an existing role of every permission on the target. + /// + [Fact] + public async Task A_list_that_resolves_to_nothing_leaves_the_stored_value_alone() + { + await using var host = await Fixture.CreateIsolatedHostAsync(); + var factory = host.Factory; + var ct = TestContext.Current.CancellationToken; + var applier = factory.Services.GetRequiredService(); + + const string slug = "nothingresolves"; + var seed = new RealmManifest + { + Apps = [new RealmManifestApp { Slug = "keep", DisplayName = "Keep", + Permissions = [new RealmManifestPermission("doc", "read"), new RealmManifestPermission("doc", "write")] }], + Roles = [new RealmManifestRole { Name = "Editor", App = "keep", + Permissions = [new RealmManifestPermission("doc", "read"), new RealmManifestPermission("doc", "write")] }], + }; + var seeded = await ProvisionRealmAsync(factory, Shell(slug), seed, ct); + Assert.False(seeded.IsError, seeded.IsError ? seeded.FirstError.Description : string.Empty); + + Guid roleId = default; + await InTenantAsync(factory, slug, async sp => + { + var session = sp.GetRequiredService(); + var role = await session.Query().SingleAsync(r => !r.IsDeleted && r.Name == "Editor", ct); + roleId = role.Id; + Assert.Equal(2, role.PermissionIds.Count); + }); + + // The same role — matched by its exported Id, which is what makes this the SAME + // entity even though its App no longer resolves here (an app slug is part of a + // role's natural key, so without the id this would read as a different role). + var orphaned = await applier.UpdateRealmAsync(slug, new RealmManifest + { + Roles = [new RealmManifestRole + { + Id = new ShortGuid(roleId).ToString(), + Name = "Editor", App = "gone", + Permissions = [new RealmManifestPermission("doc", "read")], + }], + }, ct: ct); + Assert.False(orphaned.IsError, orphaned.IsError ? orphaned.FirstError.Description : string.Empty); + Assert.Contains(orphaned.Value.SkippedReferences, x => x.Contains("app 'gone'")); + + await InTenantAsync(factory, slug, async sp => + { + var session = sp.GetRequiredService(); + var role = await session.LoadAsync(roleId, ct); + Assert.Equal(2, role!.PermissionIds.Count); // NOT stripped + }); + } + private static async Task InTenantAsync( ColdStartWebApplicationFactory factory, string slug, Func body) { diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/ManifestReferenceSkips.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/ManifestReferenceSkips.cs new file mode 100644 index 00000000..611c2a7a --- /dev/null +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/ManifestReferenceSkips.cs @@ -0,0 +1,44 @@ +namespace Modgud.Api.Features.Admin.Provisioning; + +/// +/// Collects the manifest references an apply could not resolve on the target realm. +/// +/// A manifest is content, and content travels: a partial export, a hand-written file, +/// or a realm whose apps have moved on since the file was written all produce references +/// that name something the target does not have. The applier does NOT fail on those — a +/// reference that cannot be resolved is SKIPPED, and what could be resolved is applied. +/// Ten role ids where eight exist means eight roles, not an aborted apply. +/// +/// Two rules keep "skipped" from quietly turning into "cleared": +/// +/// Each reference resolves on its own. Missing ones drop out of the list; the rest land. +/// If a NON-EMPTY reference list resolves to nothing at all, the field is treated as +/// ABSENT rather than as an empty list. Under v2 merge-patch semantics an empty list is an +/// instruction ("clear this"), and the manifest never asked for that — zero resolved +/// references are a failed lookup, not an expressed intent. On a create absent and empty +/// coincide (there is nothing to keep); on an update the stored value survives, so an +/// existing role cannot be silently stripped of its permissions because its app was left +/// out of the file. +/// +/// +/// Everything skipped is recorded here so the apply can report it — silence is the +/// only genuinely dangerous outcome, and the plan shows the same thing beforehand. +/// +public sealed class ManifestReferenceSkips +{ + private readonly List _skips = []; + + public IReadOnlyList Skips => _skips; + + /// Records one unresolvable reference. is the entity + /// the reference sits on (e.g. role 'acme/Author'). + public void Skip(string context, string reference, string reason) + => _skips.Add($"{context}: {reference} — {reason}"); + + /// Records that a whole reference list came back empty and is therefore left + /// unchanged rather than cleared (rule 2 above). + public void SkipWholeList(string context, string field, int listed) + => _skips.Add( + $"{context}: none of the {listed} {field} reference(s) exist in this realm — " + + $"{field} is left unchanged rather than cleared."); +} diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifest.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifest.cs index 7b5bd28d..060cb6f0 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifest.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifest.cs @@ -640,4 +640,13 @@ public sealed record RealmImportResult /// surfaced here for a test-kit / caller to use without a separate fetch. /// public Dictionary ClientSecrets { get; init; } = []; + + /// + /// Manifest references this realm could not resolve and that the apply therefore + /// SKIPPED — a role naming an app that lives elsewhere, a group member who is not a + /// user here, a permission outside the target app's catalog. The apply succeeded; these + /// are the parts of it that did not land, reported because a silent skip is the one + /// genuinely dangerous outcome. Empty on a clean apply. + /// + public List SkippedReferences { get; init; } = []; } diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.Positions.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.Positions.cs index 7fdb5c19..4e74e43c 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.Positions.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.Positions.cs @@ -40,7 +40,7 @@ public sealed partial class RealmManifestApplier /// private static async Task ApplyPositionsAsync( IServiceProvider sp, RealmManifest manifest, - IReadOnlyDictionary userIds, CancellationToken ct) + IReadOnlyDictionary userIds, ManifestReferenceSkips skips, CancellationToken ct) { if (manifest.Positions.Count == 0) return; @@ -65,7 +65,17 @@ private static async Task ApplyPositionsAsync( { grantUserIds = new List(pos.Grants.Count); foreach (var key in pos.Grants) - grantUserIds.Add(await ResolveUserRefAsync(session, userIds, key, $"{ctx} grant '{key}'", ct)); + { + if (await ResolveUserRefAsync(session, userIds, key, $"{ctx} grant '{key}'", skips, ct) is { } uid) + grantUserIds.Add(uid); + } + // Revoking every grant is a real instruction ([]), resolving none of them is + // not — leave the grant set alone rather than ending everyone's shifts. + if (grantUserIds.Count == 0 && pos.Grants.Count > 0) + { + skips.SkipWholeList(ctx, "grant", pos.Grants.Count); + grantUserIds = null; + } } // Id first — the account name is mutable through the canonical update, so an diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs index 0b756b3e..f852a374 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs @@ -118,19 +118,27 @@ public async Task> UpdateRealmAsync( return Error.NotFound("Realm.NotFound", $"Realm '{slug}' does not exist. Create it first (POST /api/admin/realms), then apply."); + var skips = new ManifestReferenceSkips(); try { - var secrets = await ApplyTenantUpdateAsync(slug, manifest, prune, deletions, ct); + var secrets = await ApplyTenantUpdateAsync(slug, manifest, prune, deletions, skips, ct); logger.LogInformation( "Updated realm {Slug}: {Apps} apps, {Apis} apis, {Scopes} scopes, {Clients} clients, {Roles} roles, {Users} users, {Groups} groups, {Providers} login providers, {Positions} positions (in-place merge).", slug, manifest.Apps.Count, manifest.Apis.Count, manifest.Scopes.Count, manifest.Clients.Count, manifest.Roles.Count, manifest.Users.Count, manifest.Groups.Count, manifest.LoginProviders.Count, manifest.Positions.Count); + if (skips.Skips.Count > 0) + { + logger.LogWarning( + "Apply of realm {Slug} skipped {Count} unresolvable manifest reference(s): {Skips}", + slug, skips.Skips.Count, string.Join(" | ", skips.Skips)); + } return new RealmImportResult { Slug = slug, PrimaryDomain = realm.PrimaryDomain, ClientSecrets = secrets, + SkippedReferences = [.. skips.Skips], }; } catch (ManifestApplyException ex) @@ -155,7 +163,8 @@ public async Task> UpdateRealmAsync( /// private async Task> ApplyTenantUpdateAsync( string slug, RealmManifest manifest, bool prune, - IReadOnlyCollection? deletions, CancellationToken ct) + IReadOnlyCollection? deletions, ManifestReferenceSkips skips, + CancellationToken ct) { var secrets = new Dictionary(StringComparer.Ordinal); @@ -170,7 +179,7 @@ private async Task> ApplyTenantUpdateAsync( await using var applyTx = await TenantApplyTransaction.BeginAsync(store, slug, ct); using (applyTx.Activate()) { - await ApplyTenantUpdateSectionsAsync(manifest, prune, deletions, secrets, ct); + await ApplyTenantUpdateSectionsAsync(manifest, prune, deletions, secrets, skips, ct); await applyTx.CommitAsync(ct); } @@ -183,7 +192,7 @@ private async Task> ApplyTenantUpdateAsync( private async Task ApplyTenantUpdateSectionsAsync( RealmManifest manifest, bool prune, IReadOnlyCollection? deletions, - Dictionary secrets, CancellationToken ct) + Dictionary secrets, ManifestReferenceSkips skips, CancellationToken ct) { var apps = new Dictionary(StringComparer.Ordinal); // slug → App (id + catalog) var roleIds = new Dictionary(StringComparer.Ordinal); // role key → id (for groups) @@ -269,8 +278,8 @@ private async Task ApplyTenantUpdateSectionsAsync( Enabled = api.Enabled ?? true, Scopes = api.Scopes ?? [], UserClaims = api.UserClaims ?? [], - AppId = ResolveAppId(apps, OrNull(api.App), ctx), - PermissionIds = ResolvePermissionIds(apps, OrNull(api.App), api.Permissions, ctx), + AppId = ResolveAppId(apps, OrNull(api.App), ctx, skips), + PermissionIds = ResolvePermissionIds(apps, OrNull(api.App), api.Permissions, ctx, skips) ?? [], AllowDynamicRegistration = api.AllowDynamicRegistration ?? false, }, ct), ctx); } @@ -287,11 +296,12 @@ private async Task ApplyTenantUpdateSectionsAsync( Scopes = api.Scopes, UserClaims = api.UserClaims, AppId = api.App.HasValue - ? new Optional(ResolveAppId(apps, api.App.Value, ctx)) + ? new Optional(ResolveAppId(apps, api.App.Value, ctx, skips)) : default, + // null = unchanged, which is also what an all-unresolvable list becomes. PermissionIds = api.Permissions is null ? null - : ResolvePermissionIds(apps, OrNull(api.App), api.Permissions, ctx), + : ResolvePermissionIds(apps, OrNull(api.App), api.Permissions, ctx, skips), AllowDynamicRegistration = api.AllowDynamicRegistration, }, ct), ctx); } @@ -320,7 +330,7 @@ private async Task ApplyTenantUpdateSectionsAsync( Emphasize = s.Emphasize ?? false, ShowInDiscoveryDocument = s.ShowInDiscoveryDocument ?? true, AllowDynamicRegistrationClients = s.AllowDynamicRegistrationClients ?? false, - AppId = ResolveAppId(apps, OrNull(s.App), ctx), + AppId = ResolveAppId(apps, OrNull(s.App), ctx, skips), }, ct), ctx); } else @@ -339,7 +349,7 @@ private async Task ApplyTenantUpdateSectionsAsync( ShowInDiscoveryDocument = s.ShowInDiscoveryDocument, AllowDynamicRegistrationClients = s.AllowDynamicRegistrationClients, AppId = s.App.HasValue - ? new Optional(ResolveAppId(apps, s.App.Value, ctx)) + ? new Optional(ResolveAppId(apps, s.App.Value, ctx, skips)) : default, }, ct), ctx); } @@ -355,7 +365,7 @@ private async Task ApplyTenantUpdateSectionsAsync( if (existing is not null) EnsureRenameable(false, c.ClientId, existing.ClientId, "ClientId", ctx); if (existing is null) { - var created = await oauth.CreateClientAsync(BuildClientCreateDto(c, apps, ctx), ct); + var created = await oauth.CreateClientAsync(BuildClientCreateDto(c, apps, ctx, skips), ct); EnsureOk(created, ctx); if (created.Value.ClientSecret is not null) secrets[c.ClientId] = created.Value.ClientSecret; @@ -365,7 +375,7 @@ private async Task ApplyTenantUpdateSectionsAsync( // ClientType + secret are immutable through the canonical update path; an // existing client keeps its secret (rotate via the dedicated endpoint). EnsureOk(await oauth.UpdateClientAsync( - existing.Id.ToString(), BuildClientUpdateDto(c, apps, ctx), ct), ctx); + existing.Id.ToString(), BuildClientUpdateDto(c, apps, ctx, skips), ct), ctx); } } @@ -458,16 +468,37 @@ await ResolvePinnedAsync( // v2 merge-patch: absent fields keep the existing role's values (the // canonical update is a full payload replace, so merge here). var isRealmAdmin = r.IsRealmAdmin ?? existing?.IsRealmAdmin ?? false; + + // A role's app is not an ordinary skippable reference: the app OWNS the catalog + // the role grants from, and the domain refuses a role that is neither app-linked + // nor realm-admin. So an app this realm does not have cannot simply "drop out" — + // an existing role keeps the app it already has, and a NEW one cannot be created + // at all, which makes the whole role the thing that gets skipped. + var appId = r.App is null + ? isRealmAdmin || existing?.AppId is not { } keptId ? null : new ShortGuid(keptId).ToString() + : ResolveAppId(apps, r.App, ctx, skips) + ?? (existing?.AppId is { } fallbackId ? new ShortGuid(fallbackId).ToString() : null); + + if (!isRealmAdmin && appId is null && r.App is not null) + { + skips.Skip(ctx, $"app '{r.App}'", + "no such app in this realm, and a role must belong to one — the whole role is skipped"); + continue; + } + var payload = new RolePayload( r.Name, r.Description.HasValue ? r.Description.Value : existing?.Description, - r.App is null - ? isRealmAdmin || existing?.AppId is not { } appId ? null : new ShortGuid(appId).ToString() - : ResolveAppId(apps, r.App, ctx), + appId, isRealmAdmin, + // Absent permissions keep the stored set — and so does a list that resolved + // to nothing (its app is not in this realm), which is why the null coalesces + // onto the existing ids rather than onto an empty list. r.Permissions is null && existing is not null ? existing.PermissionIds.Select(pid => new ShortGuid(pid).ToString()).ToList() - : ResolvePermissionIds(apps, r.App, r.Permissions ?? [], ctx), + : ResolvePermissionIds(apps, r.App, r.Permissions ?? [], ctx, skips) + ?? existing?.PermissionIds.Select(pid => new ShortGuid(pid).ToString()).ToList() + ?? [], r.Id); // Control-plane provisioning is trusted, so the realm-admin guard is satisfied. ErrorOr result = existing is null @@ -580,7 +611,15 @@ r.Permissions is null && existing is not null { memberIds = new List((g.Members ?? []).Count); foreach (var m in g.Members ?? []) - memberIds.Add(await ResolveUserRefAsync(session, userIds, m, $"{ctx} member '{m}'", ct)); + { + if (await ResolveUserRefAsync(session, userIds, m, $"{ctx} member '{m}'", skips, ct) is { } uid) + memberIds.Add(uid); + } + // Every listed member was unresolvable → keep the stored membership + // instead of emptying it (a replace-list of nothing is not an intent). + memberIds = OrUnchangedWhenNothingResolved( + memberIds, (g.Members ?? []).Count, ctx, "member", skips) + ?? existing?.MemberIds.ToList() ?? []; } List groupRoleIds; if (g.Roles is null && existing is not null) @@ -591,7 +630,13 @@ r.Permissions is null && existing is not null { groupRoleIds = new List((g.Roles ?? []).Count); foreach (var rk in g.Roles ?? []) - groupRoleIds.Add(await ResolveRoleRefAsync(session, roleIds, rk, $"{ctx} role '{rk}'", ct)); + { + if (await ResolveRoleRefAsync(session, roleIds, rk, $"{ctx} role '{rk}'", skips, ct) is { } rid) + groupRoleIds.Add(rid); + } + groupRoleIds = OrUnchangedWhenNothingResolved( + groupRoleIds, (g.Roles ?? []).Count, ctx, "role", skips) + ?? existing?.RoleIds.ToList() ?? []; } var mode = g.MembershipMode is null @@ -630,7 +675,7 @@ r.Permissions is null && existing is not null await ApplyServiceAccountsAsync(sp, manifest, ct); // ── Positions (MG-FT) — after users so grants can resolve their keys ────── - await ApplyPositionsAsync(sp, manifest, userIds, ct); + await ApplyPositionsAsync(sp, manifest, userIds, skips, ct); // ── Prune / staged deletions: removal of entities absent from the manifest. Runs // AFTER the upsert so the protection checks see the desired (post-merge) role @@ -803,7 +848,8 @@ bool WantsUser(Person p) /// path can never diverge (guarded by RealmManifestParityTests). /// private static CreateOAuthClientDto BuildClientCreateDto( - RealmManifestClient c, IReadOnlyDictionary apps, string ctx) => new() + RealmManifestClient c, IReadOnlyDictionary apps, string ctx, + ManifestReferenceSkips skips) => new() { Id = c.Id, ClientId = c.ClientId, @@ -843,9 +889,13 @@ private static CreateOAuthClientDto BuildClientCreateDto( ClientClaimsPrefix = OrNull(c.ClientClaimsPrefix), AlwaysSendClientClaims = c.AlwaysSendClientClaims ?? false, UpdateAccessTokenClaimsOnRefresh = c.UpdateAccessTokenClaimsOnRefresh ?? false, + // A client's app link is optional, so an app this realm does not have simply + // drops out; an all-unresolvable list leaves the link alone rather than clearing it. AppIds = c.Apps is not { Count: > 0 } ? null - : c.Apps.Select(appSlug => ResolveAppId(apps, appSlug, ctx)!).ToList(), + : OrUnchangedWhenNothingResolved( + c.Apps.Select(slug => ResolveAppId(apps, slug, ctx, skips)).OfType().ToList(), + c.Apps.Count, ctx, "app", skips), }; /// @@ -854,7 +904,8 @@ private static CreateOAuthClientDto BuildClientCreateDto( /// Optionals/lists stay unchanged, explicit null clears, [] clears a list. /// private static UpdateOAuthClientDto BuildClientUpdateDto( - RealmManifestClient c, IReadOnlyDictionary apps, string ctx) => new() + RealmManifestClient c, IReadOnlyDictionary apps, string ctx, + ManifestReferenceSkips skips) => new() { DisplayName = c.DisplayName, ConsentType = c.ConsentType, @@ -889,7 +940,11 @@ private static UpdateOAuthClientDto BuildClientUpdateDto( ClientClaimsPrefix = c.ClientClaimsPrefix, AlwaysSendClientClaims = c.AlwaysSendClientClaims, UpdateAccessTokenClaimsOnRefresh = c.UpdateAccessTokenClaimsOnRefresh, - AppIds = c.Apps?.Select(appSlug => ResolveAppId(apps, appSlug, ctx)!).ToList(), + AppIds = c.Apps is null + ? null + : OrUnchangedWhenNothingResolved( + c.Apps.Select(slug => ResolveAppId(apps, slug, ctx, skips)).OfType().ToList(), + c.Apps.Count, ctx, "app", skips), }; /// @@ -973,15 +1028,18 @@ private static Optional OptThrough(Optional value) /// otherwise the Key (username or email) — first among the users this apply /// created/matched, then live. A bare string is always a key. /// - private static async Task ResolveUserRefAsync( - IDocumentSession session, IReadOnlyDictionary map, ManifestRef reference, string context, CancellationToken ct) + private static async Task ResolveUserRefAsync( + IDocumentSession session, IReadOnlyDictionary map, ManifestRef reference, + string context, ManifestReferenceSkips skips, CancellationToken ct) { if (reference.ParsedId is { } byId && await session.LoadAsync(byId, ct) is { IsDeleted: false }) return byId; if (reference.Key is null) - throw new ManifestApplyException(context, - [Error.Validation("Manifest.UnknownReference", $"{context} names no live user by id and carries no key.")]); + { + skips.Skip(context, "user", "names no live user by id and carries no key"); + return null; + } var key = reference.Key; if (map.TryGetValue(key, out var id)) return id; var lowered = key.ToLowerInvariant(); @@ -989,8 +1047,10 @@ await session.LoadAsync(byId, ct) is { IsDeleted: false }) var person = await session.Query() .FirstOrDefaultAsync(p => !p.IsDeleted && (p.AccountName == lowered || p.NormalizedEmail == upper), ct); if (person is null) - throw new ManifestApplyException(context, - [Error.Validation("Manifest.UnknownReference", $"{context} resolves to no user.")]); + { + skips.Skip(context, $"user '{key}'", "no such user in this realm"); + return null; + } return person.Id; } @@ -1041,8 +1101,9 @@ private static void RegisterRole( /// live — a qualified app/name exactly, a bare name only while it names exactly /// one live role. /// - private static async Task ResolveRoleRefAsync( - IDocumentSession session, IReadOnlyDictionary map, ManifestRef reference, string context, CancellationToken ct) + private static async Task ResolveRoleRefAsync( + IDocumentSession session, IReadOnlyDictionary map, ManifestRef reference, + string context, ManifestReferenceSkips skips, CancellationToken ct) { // An explicit Id wins when a live role carries it (rename-proof); the Key is the // fallback for a hand-edited or cross-environment manifest. A bare string is a key. @@ -1050,8 +1111,10 @@ private static async Task ResolveRoleRefAsync( await session.LoadAsync(byId, ct) is { IsDeleted: false }) return byId; if (reference.Key is null) - throw new ManifestApplyException(context, - [Error.Validation("Manifest.UnknownReference", $"{context} names no live role by id and carries no key.")]); + { + skips.Skip(context, "role", "names no live role by id and carries no key"); + return null; + } var key = reference.Key; if (map.TryGetValue(key, out var id)) return id; var (appSlug, name) = RoleKeys.Split(key); @@ -1070,40 +1133,79 @@ await session.LoadAsync(byId, ct) is { IsDeleted: false }) throw new ManifestApplyException(context, [Error.Validation("Manifest.AmbiguousReference", $"{context} names {byName.Count} live roles — qualify it as /{key}.")]); - throw new ManifestApplyException(context, - [Error.Validation("Manifest.UnknownReference", $"{context} resolves to no role.")]); + skips.Skip(context, $"role '{key}'", "no such role in this realm"); + return null; } - private static string? ResolveAppId(IReadOnlyDictionary apps, string? slug, string context) + /// An app reference: resolves to the app's id, or to null when this realm has no + /// such app. A missing app is SKIPPED, not an error — the app link is an optional + /// grouping (a client with no app is a legal, ordinary client), and the applier seeds its + /// resolver with every app already in the realm, so a file that omits an app the target + /// already has resolves fine. See . + private static string? ResolveAppId( + IReadOnlyDictionary apps, string? slug, string context, ManifestReferenceSkips skips) { if (string.IsNullOrEmpty(slug)) return null; if (!apps.TryGetValue(slug, out var app)) - throw new ManifestApplyException(context, - [Error.Validation("Manifest.UnknownApp", $"{context} references unknown app '{slug}'.")]); + { + skips.Skip(context, $"app '{slug}'", "no such app in this realm"); + return null; + } return new ShortGuid(app.Id).ToString(); } - private static List ResolvePermissionIds( - IReadOnlyDictionary apps, string? appSlug, List? perms, string context) + /// + /// Resolves resource:action permission keys against the catalog of + /// . Permissions the catalog does not carry are skipped, the + /// rest resolve. + /// + /// Returns null — meaning "leave the stored value alone" — when a non-empty + /// permission list yields nothing, which happens either because the app itself is absent + /// (no catalog to look in at all) or because none of the listed permissions are in it. + /// Writing [] there would CLEAR an existing role's permissions on the strength of + /// a lookup that never happened; rule 2 in . + /// + private static List? ResolvePermissionIds( + IReadOnlyDictionary apps, string? appSlug, List? perms, + string context, ManifestReferenceSkips skips) { if (perms is not { Count: > 0 }) return []; + if (string.IsNullOrEmpty(appSlug) || !apps.TryGetValue(appSlug, out var app)) - throw new ManifestApplyException(context, - [Error.Validation("Manifest.PermissionsNeedApp", $"{context} lists permissions but has no resolvable app.")]); + { + skips.SkipWholeList(context, "permission", perms.Count); + return null; + } var catalog = app.Permissions.ToDictionary(p => $"{p.Resource}:{p.Action}", p => p.Id); var ids = new List(perms.Count); foreach (var p in perms) { - if (!catalog.TryGetValue($"{p.Resource}:{p.Action}", out var pid)) - throw new ManifestApplyException(context, - [Error.Validation("Manifest.UnknownPermission", - $"{context} references permission '{p.Resource}:{p.Action}' not in app '{appSlug}' catalog.")]); - ids.Add(new ShortGuid(pid).ToString()); + if (catalog.TryGetValue($"{p.Resource}:{p.Action}", out var pid)) + ids.Add(new ShortGuid(pid).ToString()); + else + skips.Skip(context, $"permission '{p.Resource}:{p.Action}'", + $"not in the catalog of app '{appSlug}'"); + } + + if (ids.Count == 0) + { + skips.SkipWholeList(context, "permission", perms.Count); + return null; } return ids; } + /// Applies rule 2 to a resolved reference list: a non-empty input that resolved + /// to nothing becomes null (unchanged) instead of [] (cleared). + private static List? OrUnchangedWhenNothingResolved( + List resolved, int listed, string context, string field, ManifestReferenceSkips skips) + { + if (listed == 0 || resolved.Count > 0) return resolved; + skips.SkipWholeList(context, field, listed); + return null; + } + /// Null stays null — the manifest's "omitted = no change on apply / shipped /// default on create" semantics for optional enum fields. diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs index 742c19bc..5a805201 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs @@ -283,9 +283,130 @@ await GroupMembershipGuards.GroupConfersRealmAdminAsync(session, perms, doc, ct) }, })); + AddSkippedReferenceNotes(manifest, current, result); return result; } + /// + /// Predicts what the applier will SKIP: references naming something this realm does not + /// have. The applier resolves what it can and drops the rest, so these are the parts of + /// the manifest that will not land — invisible in the result afterwards (a group left + /// with two of three roles looks exactly like a group that asked for two), which is + /// precisely why the plan has to say so beforehand. + /// + /// Resolution is checked against the manifest UNION the current realm, mirroring + /// the apply: entities the same file creates count as present, because by the time the + /// reference is resolved they exist. + /// + private static void AddSkippedReferenceNotes( + RealmManifest manifest, RealmManifest current, RealmPlanResult result) + { + var appSlugs = manifest.Apps.Select(a => a.Slug) + .Concat(current.Apps.Select(a => a.Slug)) + .ToHashSet(StringComparer.Ordinal); + + // resource:action keys per app slug, from both sides. + var catalog = new Dictionary>(StringComparer.Ordinal); + foreach (var app in current.Apps.Concat(manifest.Apps)) + { + if (app.Permissions is null) continue; + if (!catalog.TryGetValue(app.Slug, out var set)) + catalog[app.Slug] = set = new HashSet(StringComparer.Ordinal); + foreach (var perm in app.Permissions) set.Add($"{perm.Resource}:{perm.Action}"); + } + + var roleKeys = manifest.Roles.Select(r => r.NaturalKey) + .Concat(current.Roles.Select(r => r.NaturalKey)) + .ToHashSet(StringComparer.Ordinal); + var bareRoleNames = manifest.Roles.Select(r => r.Name) + .Concat(current.Roles.Select(r => r.Name)) + .ToHashSet(StringComparer.Ordinal); + var userKeys = manifest.Users.Select(u => u.ResolveKey()) + .Concat(current.Users.Select(u => u.ResolveKey())) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + var userEmails = manifest.Users.Select(u => u.Email) + .Concat(current.Users.Select(u => u.Email)) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + + void Note(string section, string key, string note) + { + var entry = result.Sections.FirstOrDefault(x => x.Name == section)? + .Entries.FirstOrDefault(e => string.Equals(e.Key, key, StringComparison.Ordinal)); + if (entry is not null && entry.Action != "delete" && entry.Action != "protected") + entry.Notes.Add(note); + } + + static string Skipped(string what) => + $"{what} — this realm has no such entity, so the apply SKIPS the reference and applies the rest."; + + // ── Roles: the app carries the permission catalog the entries resolve against ── + foreach (var role in manifest.Roles) + { + if (role.IsRealmAdmin == true || role.App is null) continue; + if (!appSlugs.Contains(role.App)) + { + Note("roles", role.NaturalKey, + $"App '{role.App}' does not exist here, so its permission catalog cannot be read: " + + "the role's permissions are left UNCHANGED (an existing role keeps what it has; " + + "a new one is created without permissions). Import the app first to get them."); + continue; + } + var known = catalog.GetValueOrDefault(role.App) ?? []; + foreach (var perm in role.Permissions ?? []) + { + var key = $"{perm.Resource}:{perm.Action}"; + if (!known.Contains(key)) + Note("roles", role.NaturalKey, Skipped($"Permission '{key}' is not in app '{role.App}'")); + } + } + + // ── Clients / APIs / scopes: the app link is optional, a missing one just drops ── + foreach (var client in manifest.Clients) + { + foreach (var slug in client.Apps ?? []) + if (!appSlugs.Contains(slug)) + Note("clients", client.ClientId, Skipped($"App '{slug}'")); + } + foreach (var api in manifest.Apis) + { + if (api.App.HasValue && api.App.Value is { } appSlug && !appSlugs.Contains(appSlug)) + Note("apis", api.Name, Skipped($"App '{appSlug}'")); + } + foreach (var scope in manifest.Scopes) + { + if (scope.App.HasValue && scope.App.Value is { } appSlug && !appSlugs.Contains(appSlug)) + Note("scopes", scope.Name, Skipped($"App '{appSlug}'")); + } + + // ── Groups: roles and members resolve one by one ────────────────────────────── + foreach (var group in manifest.Groups) + { + foreach (var reference in group.Roles ?? []) + { + if (reference.Key is not { } key) continue; + if (!roleKeys.Contains(key) && !bareRoleNames.Contains(key)) + Note("groups", group.Name, Skipped($"Role '{key}'")); + } + foreach (var reference in group.Members ?? []) + { + if (reference.Key is not { } key) continue; + if (!userKeys.Contains(key) && !userEmails.Contains(key)) + Note("groups", group.Name, Skipped($"Member '{key}'")); + } + } + + // ── Positions: staffing grants are user references too ──────────────────────── + foreach (var position in manifest.Positions) + { + foreach (var reference in position.Grants ?? []) + { + if (reference.Key is not { } key) continue; + if (!userKeys.Contains(key) && !userEmails.Contains(key)) + Note("positions", position.AccountName, Skipped($"Grant for user '{key}'")); + } + } + } + // ── Settings — one pseudo-entity, nested patch diff with dotted paths. ─────────── private static RealmPlanSection PlanSettings( diff --git a/src/frontend-vue/public/i18n/de.json b/src/frontend-vue/public/i18n/de.json index c4815956..88070c52 100644 --- a/src/frontend-vue/public/i18n/de.json +++ b/src/frontend-vue/public/i18n/de.json @@ -1820,12 +1820,12 @@ "selective": { "title": "Selektiver Export", "subTitle": "Entities auswählen — Referenzen werden automatisch mitgenommen", - "hint": "Secrets werden nie exportiert. Die Datei benennt keinen Realm — das Ziel wählst du beim Import. Auf dem Ziel OHNE Prune anwenden; mit Prune würde alles gelöscht, was nicht in der Datei steht.", + "hint": "Secrets werden nie exportiert. Die Datei benennt keinen Realm — das Ziel wählst du beim Import. Referenzierte Entities sind vorausgewählt; nimm raus, was du nicht willst — das Ziel überspringt Referenzen, die es nicht auflösen kann, und weist im Plan darauf hin. OHNE Prune anwenden; mit Prune würde alles gelöscht, was nicht in der Datei steht.", "includeSettings": "Realm-Einstellungen einschließen", "includeUsers": "User-Referenzen einschließen (Gruppen-Mitglieder, Positions-Grants)", "download": "Manifest herunterladen", - "required": "Benötigt", - "requiredBy": "Benötigt von: ", + "referenced": "Referenziert", + "referencedBy": "Referenziert von: ", "selectRelated": "Zugehörige auswählen", "showAll": "Alle Entities anzeigen (weitere hinzufügen)", "dropped": "{count} gesammelte Einträge existieren nicht mehr im Export und wurden entfernt." diff --git a/src/frontend-vue/src/views/admin/realm-config/SelectiveExportModal.vue b/src/frontend-vue/src/views/admin/realm-config/SelectiveExportModal.vue index 4b339162..a4e29d1f 100644 --- a/src/frontend-vue/src/views/admin/realm-config/SelectiveExportModal.vue +++ b/src/frontend-vue/src/views/admin/realm-config/SelectiveExportModal.vue @@ -3,10 +3,18 @@ * Selective ("cart") export — pick entities from the current export and * download a partial manifest for another realm/instance. The dependency * closure is computed live: whatever the checked entities reference - * (transitively) is pulled in automatically and shown as "required", so the - * downloaded manifest always applies cleanly. Nothing here writes anywhere — - * the output is a JSON download; the target imports it as a draft, reviews - * the plan and applies. + * (transitively) is pulled in and pre-checked, because that is nearly always + * what you want. + * + * It is a DEFAULT, not a lock. Nothing here is truly required: the target + * resolves what it can and skips the rest, so a client can travel without its + * app (it simply arrives with no app linked) and its plan says so before the + * apply. Labelling those entries "required" while letting them be unchecked + * would be a lie, and labelling them required while forcing them in would be a + * lie about the server — so they are labelled "referenced" and left to you. + * + * Nothing here writes anywhere — the output is a JSON download; the target + * imports it as a draft, reviews the plan and applies. */ import { computed, ref } from 'vue' import { CoarButton, CoarCheckbox, CoarIcon, CoarNotice, CoarTag } from '@cocoar/vue-ui' @@ -54,6 +62,9 @@ const validPreselected = (props.preselected ?? []).filter(keyExists) const droppedCount = (props.preselected?.length ?? 0) - validPreselected.length const selected = ref>(new Set(validPreselected)) +/** Closure entries the user deliberately unchecked — see the header note. */ +const excluded = ref>(new Set()) + /** Review mode (preselected): list only the selection + closure. */ const showAll = ref(!props.preselected) @@ -78,7 +89,9 @@ const sections = computed(() => SELECTABLE_SECTIONS const key = meta.key(e) return { key, sel: selectionKey(section, key), info: rowInfo(section, e) } }) - if (!showAll.value) rows = rows.filter((r) => isChecked(r.sel)) + // Keep an unchecked closure entry on screen — otherwise it vanishes the moment + // you uncheck it and there is no way back to it. + if (!showAll.value) rows = rows.filter((r) => isChecked(r.sel) || closure.value.has(r.sel)) return { section, rows } }) .filter((s) => s.rows.length > 0)) @@ -108,6 +121,7 @@ const closure = computed(() => const effectiveSelection = computed>(() => { const all = new Set(selected.value) for (const key of closure.value.keys()) all.add(key) + for (const key of excluded.value) all.delete(key) return all }) @@ -115,22 +129,31 @@ function isChecked(sel: SelectionKey): boolean { return effectiveSelection.value.has(sel) } -/** Required by the closure but not explicitly checked — locked on. */ -function isRequired(sel: SelectionKey): boolean { +/** Pulled in by the closure rather than picked by hand — pre-checked, not locked. */ +function isReferenced(sel: SelectionKey): boolean { return closure.value.has(sel) && !selected.value.has(sel) } -function requiredBy(sel: SelectionKey): string { +function referencedBy(sel: SelectionKey): string { const by = closure.value.get(sel) if (!by) return '' return [...by].map((k) => k.slice(k.indexOf('/') + 1)).join(', ') } function toggle(sel: SelectionKey, on: boolean) { - const next = new Set(selected.value) - if (on) next.add(sel) - else next.delete(sel) - selected.value = next + const nextSelected = new Set(selected.value) + const nextExcluded = new Set(excluded.value) + if (on) { + nextSelected.add(sel) + nextExcluded.delete(sel) + } else { + // Unchecking must also override the closure, or a referenced entry would + // silently reappear in the file. + nextSelected.delete(sel) + nextExcluded.add(sel) + } + selected.value = nextSelected + excluded.value = nextExcluded } function sectionAllChecked(rows: Row[]): boolean { @@ -138,21 +161,34 @@ function sectionAllChecked(rows: Row[]): boolean { } function toggleSection(rows: Row[], on: boolean) { - const next = new Set(selected.value) + const nextSelected = new Set(selected.value) + const nextExcluded = new Set(excluded.value) for (const r of rows) { - if (on) next.add(r.sel) - else next.delete(r.sel) + if (on) { + nextSelected.add(r.sel) + nextExcluded.delete(r.sel) + } else { + nextSelected.delete(r.sel) + nextExcluded.add(r.sel) + } } - selected.value = next + selected.value = nextSelected + excluded.value = nextExcluded } /** "Everything belonging to this app": check the app plus its clients, APIs, * scopes and roles — their own references then flow through the closure. */ function selectAppBundle(appKey: string) { const next = new Set(selected.value) - next.add(selectionKey('apps', appKey)) - for (const ref of relatedToApp(props.manifest, appKey)) next.add(selectionKey(ref.section, ref.key)) + const nextExcluded = new Set(excluded.value) + const add = (sel: SelectionKey) => { + next.add(sel) + nextExcluded.delete(sel) + } + add(selectionKey('apps', appKey)) + for (const ref of relatedToApp(props.manifest, appKey)) add(selectionKey(ref.section, ref.key)) selected.value = next + excluded.value = nextExcluded } const totalCount = computed(() => effectiveSelection.value.size + (includeSettings.value ? 1 : 0)) @@ -201,7 +237,7 @@ function sectionLabel(name: string): string {
{{ t('admin.realmConfig.selective.hint', {}, - 'Secrets are never exported. The file names no realm — you pick the target when you import it. Apply it WITHOUT prune; with prune it would delete everything not in the file.') }} + 'Secrets are never exported. The file names no realm — you pick the target when you import it. Referenced entities are pre-selected; uncheck any you do not want, the target skips references it cannot resolve and reports them in the plan. Apply WITHOUT prune; with prune it would delete everything not in the file.') }} {{ t('admin.realmConfig.selective.dropped', { count: droppedCount }, @@ -233,15 +269,14 @@ function sectionLabel(name: string): string {
{{ row.key }} {{ row.info }} - {{ t('admin.realmConfig.selective.required', {}, 'Required') }} + :title="t('admin.realmConfig.selective.referencedBy', {}, 'Referenced by: ') + referencedBy(row.sel)"> + {{ t('admin.realmConfig.selective.referenced', {}, 'Referenced') }} Date: Wed, 9 Sep 2026 15:48:05 +0200 Subject: [PATCH 4/7] fix(provisioning): the plan's role note matches what the apply actually does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Driving the UI turned up a stale note: the plan told a create entry its role would be "created without permissions" when a missing app makes the applier skip the role outright. The two branches now say what happens — an existing role keeps its app and permissions, a new one is skipped whole. Co-Authored-By: Claude Opus 5 --- .../Provisioning/RealmManifestPlanner.cs | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs index 5a805201..21036424 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs @@ -328,14 +328,15 @@ private static void AddSkippedReferenceNotes( .Concat(current.Users.Select(u => u.Email)) .ToHashSet(StringComparer.OrdinalIgnoreCase); - void Note(string section, string key, string note) + RealmPlanEntry? Entry(string section, string key) { var entry = result.Sections.FirstOrDefault(x => x.Name == section)? .Entries.FirstOrDefault(e => string.Equals(e.Key, key, StringComparison.Ordinal)); - if (entry is not null && entry.Action != "delete" && entry.Action != "protected") - entry.Notes.Add(note); + return entry is null || entry.Action == "delete" || entry.Action == "protected" ? null : entry; } + void Note(string section, string key, string note) => Entry(section, key)?.Notes.Add(note); + static string Skipped(string what) => $"{what} — this realm has no such entity, so the apply SKIPS the reference and applies the rest."; @@ -345,10 +346,18 @@ static string Skipped(string what) => if (role.IsRealmAdmin == true || role.App is null) continue; if (!appSlugs.Contains(role.App)) { - Note("roles", role.NaturalKey, - $"App '{role.App}' does not exist here, so its permission catalog cannot be read: " - + "the role's permissions are left UNCHANGED (an existing role keeps what it has; " - + "a new one is created without permissions). Import the app first to get them."); + // A role must belong to an app, so a missing one is not a droppable + // reference: an existing role keeps the app it already has, and a role that + // would have to be CREATED cannot exist at all and is skipped whole. + if (Entry("roles", role.NaturalKey) is { } roleEntry) + { + roleEntry.Notes.Add(roleEntry.Action == "create" + ? $"App '{role.App}' does not exist here, and a role must belong to one — " + + "the apply SKIPS this role entirely. Import the app first." + : $"App '{role.App}' does not exist here, so its permission catalog cannot be " + + "read: the role keeps its current app and permissions. Import the app first " + + "to apply the listed permissions."); + } continue; } var known = catalog.GetValueOrDefault(role.App) ?? []; From c3e5b3c8711b298cfc73ac0e072c3c2a3f1db0b2 Mon Sep 17 00:00:00 2001 From: Bernhard Windisch Date: Wed, 9 Sep 2026 19:42:58 +0200 Subject: [PATCH 5/7] test(provisioning): the last assertion on the retired slug guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 62aad386 removed Manifest.SlugMismatch, but this test still asserted it — it lives under Authorization/ and the verification run for that commit was filtered to ColdStart/, so it was never executed. The full suite caught it. It now pins the invariant that replaced the guard: a delegated management bearer cannot write into another realm because a manifest names no realm at all. A body still carrying the retired "Realm" object is accepted and IGNORED, and the export afterwards shows the write landed in the caller's own realm. Co-Authored-By: Claude Opus 5 --- .../ManagementApiAuthorizationTests.cs | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/src/dotnet/Modgud.Api.Tests/Authorization/ManagementApiAuthorizationTests.cs b/src/dotnet/Modgud.Api.Tests/Authorization/ManagementApiAuthorizationTests.cs index 5fe505c4..c55e5e68 100644 --- a/src/dotnet/Modgud.Api.Tests/Authorization/ManagementApiAuthorizationTests.cs +++ b/src/dotnet/Modgud.Api.Tests/Authorization/ManagementApiAuthorizationTests.cs @@ -264,18 +264,34 @@ await CreateClientCredentialsClientAsync( Assert.Contains(appSlug, body); } - using var foreignApply = await bearerClient.PostAsJsonAsync( - "/api/admin/realm-config/apply", - new - { - Realm = new { Slug = $"foreign-{Guid.NewGuid():N}" }, - Apps = Array.Empty(), - }, - JsonOptions, - ct); - var foreignBody = await foreignApply.Content.ReadAsStringAsync(ct); - Assert.Equal(HttpStatusCode.BadRequest, foreignApply.StatusCode); - Assert.Contains("Manifest.SlugMismatch", foreignBody); + // A delegated bearer cannot reach another realm — and since a manifest carries no + // realm identity, that is now structural rather than guarded: there is no field to + // aim elsewhere with. A body still carrying the retired "Realm" object (an older + // export, a hand-written file) is accepted and IGNORED, and the write lands in the + // caller's own realm, which the export below proves. + var strayApp = $"realm-config-stray-{Guid.NewGuid():N}"; + using (var stray = await bearerClient.PostAsJsonAsync( + "/api/admin/realm-config/apply", + new + { + Realm = new { Slug = $"foreign-{Guid.NewGuid():N}" }, + Apps = new[] + { + new { Slug = strayApp, DisplayName = "Stray", Permissions = Array.Empty() }, + }, + }, + JsonOptions, + ct)) + { + var strayBody = await stray.Content.ReadAsStringAsync(ct); + Assert.True(stray.IsSuccessStatusCode, + $"stray-realm apply should be accepted and the Realm object ignored ({(int)stray.StatusCode}): {strayBody}"); + } + + using (var reexport = await bearerClient.GetAsync("/api/admin/realm-config/export", ct)) + { + Assert.Contains(strayApp, await reexport.Content.ReadAsStringAsync(ct)); + } } [Fact] From 590a908d0121f61624e28375178ad480ea60e737 Mon Sep 17 00:00:00 2001 From: Bernhard Windisch Date: Wed, 9 Sep 2026 19:42:58 +0200 Subject: [PATCH 6/7] refactor(api)!: one error body for the whole API, with the code in it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two error shapes were in play, both using a field named "error" and meaning different things by it. The canonical renderer sent { "error": "" }; the manifest endpoints sent { "Error": "", "Message": "" }. Reading the first as if it were the second passes prose off as a machine-readable code, which is exactly what Modgud.Provisioning.TestKit started doing once it spoke to both. Both halves of the canonical shape were wrong. It dropped Error.Code entirely, so a client could only branch on prose — poor for any API, worse for an IdP. And it put a description under `error`, the key OAuth reserves for a CODE, colliding with the convention its own token endpoints follow. Everything now answers { "Error": "", "Message": "" }. The frontend already expected this. useAssets, usePagesApi, usePageCompositionsApi and the user grid all read body?.Message, so every error from a ToResult endpoint has been silently degrading to "HTTP 400" in the UI. No frontend change was needed to fix it. Forbidden and Unauthorized are rendered here too instead of delegating to Results.Forbid() / Results.Unauthorized(). Under this app's cookie auth, Forbid() turns an /api/* response into an empty-body 403 and throws away the code — which is why THREE endpoint files had each grown a private copy of this renderer (RolesEndpoints, RealmsEndpoints, RealmConfigEndpoints, the first of which documents the reason). With the shared one fixed, all three are gone. Status codes are unchanged; only the body appears. Deliberately untouched: OAuth/OIDC/DCR keep their RFC-mandated { error, error_description }, and the rate-limit filter keeps its documented { error: "rate_limited", … } — both use lowercase `error` for a CODE, which is the convention our own endpoints were breaking, not following. The unit tests for the mapper asserted result TYPES only, which is how the shape drifted unnoticed; they now read the payload. BREAKING: any consumer reading `error` from a non-OAuth endpoint gets `Error` + `Message` instead. 401 and 403 now carry a body where they carried none. Co-Authored-By: Claude Opus 5 --- .../ColdStart/ProvisioningTestKitTests.cs | 8 +-- .../Provisioning/RealmConfigEndpoints.cs | 48 +++++--------- .../Features/Admin/RealmsEndpoints.cs | 19 +----- .../Features/Roles/RolesEndpoints.cs | 19 +----- .../ExtensionMethods/ErrorOrExtensions.cs | 36 +++++++--- .../ModgudProvisioningClient.cs | 10 +-- .../ErrorOrExtensionsTests.cs | 66 +++++++++++++------ 7 files changed, 103 insertions(+), 103 deletions(-) diff --git a/src/dotnet/Modgud.Api.Tests/ColdStart/ProvisioningTestKitTests.cs b/src/dotnet/Modgud.Api.Tests/ColdStart/ProvisioningTestKitTests.cs index d2163f27..c5d60040 100644 --- a/src/dotnet/Modgud.Api.Tests/ColdStart/ProvisioningTestKitTests.cs +++ b/src/dotnet/Modgud.Api.Tests/ColdStart/ProvisioningTestKitTests.cs @@ -66,12 +66,12 @@ public async Task TestKit_surfaces_the_server_error_code_on_duplicate_import() var ex = await Assert.ThrowsAsync( () => kit.ImportRealmAsync(BuildSpec(slug), BuildManifest(slug, "Dup"), ct)); - // The duplicate is caught by the create step, which answers in the canonical - // ErrorOr shape — a description, no machine-readable code. The kit must surface - // that as-is rather than passing the description off as a Code. + // The duplicate is caught by the create step. Every endpoint now answers in one + // shape, so the machine-readable code survives the whole way to the caller — + // before the shapes were unified this step could only hand back prose. Assert.Equal("create-realm", ex.Operation); Assert.Equal(HttpStatusCode.Conflict, ex.StatusCode); - Assert.Null(ex.Code); + Assert.Equal("Realm.DuplicateSlug", ex.Code); Assert.Contains(slug, ex.Message); } diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmConfigEndpoints.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmConfigEndpoints.cs index f24fc93a..0e7777c8 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmConfigEndpoints.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmConfigEndpoints.cs @@ -1,3 +1,4 @@ +using static Modgud.Authentication.ExtensionMethods.ErrorOrExtensions; using System.Security.Claims; using ErrorOr; using Microsoft.AspNetCore.Http.Json; @@ -50,7 +51,7 @@ public static WebApplication MapRealmConfigEndpoints(this WebApplication applica group.MapGet("export", async (RealmManifestExporter exporter, CancellationToken ct) => { var result = await exporter.ExportRealmAsync(TenantContext.Current, ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Export") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -62,7 +63,7 @@ public static WebApplication MapRealmConfigEndpoints(this WebApplication applica { var result = await planner.PlanAsync( TenantContext.Current, manifest, prune, baseline: null, deletions: null, ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Plan") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -81,7 +82,7 @@ public static WebApplication MapRealmConfigEndpoints(this WebApplication applica // are only reachable through the control-plane routes. var result = await applier.UpdateRealmAsync( TenantContext.Current, manifest, prune, deletions: null, ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Apply") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -108,7 +109,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) { var result = await service.CreateAsync( dto, TenantContext.Current, RequireUserId(http), UserName(http), ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Drafts_Create") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -117,7 +118,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) Guid id, HttpContext http, RealmDraftService service, CancellationToken ct) => { var result = await service.GetAsync(id, RequireUserId(http), ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Drafts_Get") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -127,7 +128,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) { var result = await service.UpdateAsync( id, dto, TenantContext.Current, RequireUserId(http), UserName(http), ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Drafts_Update") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -136,7 +137,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) Guid id, HttpContext http, RealmDraftService service, CancellationToken ct) => { var result = await service.DeleteAsync(id, RequireUserId(http), ct); - return result.IsError ? ManifestError(result.Errors) : Results.NoContent(); + return result.IsError ? ToErrorResult(result.Errors) : Results.NoContent(); }) .WithName("RealmConfig_Drafts_Delete") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -146,7 +147,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) Guid id, string slot, HttpContext http, RealmDraftService service, CancellationToken ct) => { var result = await service.ClearSecretAsync(id, slot, RequireUserId(http), UserName(http), ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Drafts_ClearSecret") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -169,7 +170,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) Guid id, HttpContext http, RealmDraftService service, CancellationToken ct) => { var result = await service.SwitchAsync(id, RequireUserId(http), ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Drafts_Switch") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -183,7 +184,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) { var result = await service.StageEntityAsync( section, entity, TenantContext.Current, RequireUserId(http), UserName(http), ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Drafts_StageEntity") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -193,7 +194,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) { var result = await service.UnstageEntityAsync( section, key, TenantContext.Current, RequireUserId(http), UserName(http), ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Drafts_UnstageEntity") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -206,7 +207,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) { var result = await service.StageDeleteAsync( section, key, TenantContext.Current, RequireUserId(http), UserName(http), ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Drafts_StageDelete") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -216,7 +217,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) { var result = await service.UnstageDeleteAsync( section, key, TenantContext.Current, RequireUserId(http), UserName(http), ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Drafts_UnstageDelete") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -228,7 +229,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) { var result = await service.RebaseAsync( id, TenantContext.Current, RequireUserId(http), UserName(http), ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Drafts_Rebase") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -238,7 +239,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) Guid id, HttpContext http, RealmDraftService service, CancellationToken ct, bool prune = false) => { var result = await service.PlanAsync(id, prune, RequireUserId(http), ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("RealmConfig_Drafts_Plan") .RequiresManagementPermission(PermissionEvaluator.RealmAdminPermission); @@ -249,7 +250,7 @@ private static void MapDraftEndpoints(IEndpointRouteBuilder group) Guid id, HttpContext http, RealmDraftService service, CancellationToken ct, bool prune = false) => { var result = await service.ApplyAsync(id, prune, RequireUserId(http), ct); - if (result.IsError) return ManifestError(result.Errors); + if (result.IsError) return ToErrorResult(result.Errors); return result.Value.Refused ? Results.Json(new { @@ -269,19 +270,4 @@ private static Guid RequireUserId(HttpContext http) private static string UserName(HttpContext http) => http.User.FindFirstValue(ClaimTypes.Name) ?? http.User.Identity?.Name ?? "unknown"; - - // Renders a manifest ErrorOr with the error code in the body (mirrors RealmsEndpoints). - private static IResult ManifestError(List errors) - { - var error = errors[0]; - var status = error.Type switch - { - ErrorType.NotFound => StatusCodes.Status404NotFound, - ErrorType.Conflict => StatusCodes.Status409Conflict, - ErrorType.Validation => StatusCodes.Status400BadRequest, - ErrorType.Forbidden => StatusCodes.Status403Forbidden, - _ => StatusCodes.Status500InternalServerError, - }; - return Results.Json(new { Error = error.Code, Message = error.Description }, statusCode: status); - } } diff --git a/src/dotnet/Modgud.Api/Features/Admin/RealmsEndpoints.cs b/src/dotnet/Modgud.Api/Features/Admin/RealmsEndpoints.cs index 3c680fe9..4a5da0b3 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/RealmsEndpoints.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/RealmsEndpoints.cs @@ -1,3 +1,4 @@ +using static Modgud.Authentication.ExtensionMethods.ErrorOrExtensions; using System.Diagnostics; using System.Security.Claims; using ErrorOr; @@ -332,7 +333,7 @@ await RecordControlPlaneRealmOperationAsync( slug, prune ? "apply-manifest-prune" : "apply-manifest"); } - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("Realms_Apply") .RequiresPermission("realm:write", AppSlugs.ControlPlane); @@ -344,7 +345,7 @@ await RecordControlPlaneRealmOperationAsync( string slug, RealmManifestExporter exporter, CancellationToken ct) => { var result = await exporter.ExportRealmAsync(slug, ct); - return result.IsError ? ManifestError(result.Errors) : Results.Ok(result.Value); + return result.IsError ? ToErrorResult(result.Errors) : Results.Ok(result.Value); }) .WithName("Realms_Export") .RequiresPermission("realm:read", AppSlugs.ControlPlane); @@ -491,20 +492,6 @@ await securityAudit.RecordRequiredAsync(new SecurityAuditRecord // Renders a RealmManifestApplier ErrorOr error with the code in the body — the manifest // codes (Realm.AlreadyExists / Realm.NotFound / Manifest.*) are how a test-kit / caller // distinguishes outcomes, so don't collapse them through the shared ToResult. - private static IResult ManifestError(List errors) - { - var error = errors[0]; - var status = error.Type switch - { - ErrorType.NotFound => StatusCodes.Status404NotFound, - ErrorType.Conflict => StatusCodes.Status409Conflict, - ErrorType.Validation => StatusCodes.Status400BadRequest, - ErrorType.Forbidden => StatusCodes.Status403Forbidden, - _ => StatusCodes.Status500InternalServerError, - }; - return Results.Json(new { Error = error.Code, Message = error.Description }, statusCode: status); - } - internal static RealmDto MapToDto(Realm realm) => new() { Id = realm.Id, diff --git a/src/dotnet/Modgud.Api/Features/Roles/RolesEndpoints.cs b/src/dotnet/Modgud.Api/Features/Roles/RolesEndpoints.cs index 4825374f..792d83f6 100644 --- a/src/dotnet/Modgud.Api/Features/Roles/RolesEndpoints.cs +++ b/src/dotnet/Modgud.Api/Features/Roles/RolesEndpoints.cs @@ -1,3 +1,4 @@ +using static Modgud.Authentication.ExtensionMethods.ErrorOrExtensions; using BuildingBlocks.Helper; using ErrorOr; using Marten; @@ -109,24 +110,6 @@ public static WebApplication MapRolesEndpoints(this WebApplication application, return application; } - // Renders a RoleAdminService ErrorOr error as HTTP with the error code in the body. - // The shared ErrorOrExtensions.ToResult maps Forbidden → Results.Forbid(), which under - // this app's cookie auth turns /api/* into an empty-body 403 (OnRedirectToAccessDenied) - // — losing the code the SPA + RealmAdminEscalationGuardTests rely on. This local - // renderer keeps the {Error,Message} body the role endpoints have always returned. - private static IResult ToErrorResult(List errors) - { - var error = errors[0]; - var status = error.Type switch - { - ErrorType.NotFound => StatusCodes.Status404NotFound, - ErrorType.Forbidden => StatusCodes.Status403Forbidden, - ErrorType.Conflict => StatusCodes.Status409Conflict, - _ => StatusCodes.Status400BadRequest, - }; - return Results.Json(new { Error = error.Code, Message = error.Description }, statusCode: status); - } - private static object MapToResponse(PermissionRole r) => new { Id = new ShortGuid(r.Id).ToString(), diff --git a/src/dotnet/Modgud.Authentication/ExtensionMethods/ErrorOrExtensions.cs b/src/dotnet/Modgud.Authentication/ExtensionMethods/ErrorOrExtensions.cs index 7755281a..a4d861cc 100644 --- a/src/dotnet/Modgud.Authentication/ExtensionMethods/ErrorOrExtensions.cs +++ b/src/dotnet/Modgud.Authentication/ExtensionMethods/ErrorOrExtensions.cs @@ -36,7 +36,25 @@ public static IResult ToNoContentResult(this ErrorOr errorOr) return Results.NoContent(); } - private static IResult ToErrorResult(List errors) + /// + /// The application's single error body: { "Error": "<code>", "Message": "<description>" }. + /// + /// It used to be { "error": "<description>" }, which was wrong twice over. + /// It dropped the machine-readable entirely, so a client could only + /// match on prose; and it put a description under a key named error, which is exactly + /// the key OAuth reserves for an error CODE. Anything reading both surfaces — the frontend, + /// Modgud.Provisioning.TestKit — got a description handed to it as if it were a code. + /// The OAuth/OIDC/DCR endpoints keep their RFC-mandated { error, error_description } + /// and are untouched by this: they are a different contract, and the collision was ours. + /// + /// Forbidden and Unauthorized are rendered here too, rather than delegated to + /// / . Under this app's + /// cookie auth, Forbid() turns an /api/* response into an EMPTY-body 403, which + /// throws away the very code the SPA needs — and that is why three endpoint files had each + /// grown their own private copy of this renderer. Emitting the body directly is what lets + /// them collapse back into one. The status codes are unchanged; only the body appears. + /// + public static IResult ToErrorResult(List errors) { if (errors.Count == 0) { @@ -45,14 +63,16 @@ private static IResult ToErrorResult(List errors) var firstError = errors[0]; - return firstError.Type switch + var status = firstError.Type switch { - ErrorType.NotFound => Results.NotFound(new { error = firstError.Description }), - ErrorType.Validation => Results.BadRequest(new { error = firstError.Description }), - ErrorType.Unauthorized => Results.Unauthorized(), - ErrorType.Forbidden => Results.Forbid(), - ErrorType.Conflict => Results.Conflict(new { error = firstError.Description }), - _ => Results.Problem(detail: firstError.Description, statusCode: 500) + ErrorType.NotFound => StatusCodes.Status404NotFound, + ErrorType.Validation => StatusCodes.Status400BadRequest, + ErrorType.Conflict => StatusCodes.Status409Conflict, + ErrorType.Unauthorized => StatusCodes.Status401Unauthorized, + ErrorType.Forbidden => StatusCodes.Status403Forbidden, + _ => StatusCodes.Status500InternalServerError, }; + + return Results.Json(new { Error = firstError.Code, Message = firstError.Description }, statusCode: status); } } diff --git a/src/dotnet/Modgud.Provisioning.TestKit/ModgudProvisioningClient.cs b/src/dotnet/Modgud.Provisioning.TestKit/ModgudProvisioningClient.cs index 281e097d..e390dea5 100644 --- a/src/dotnet/Modgud.Provisioning.TestKit/ModgudProvisioningClient.cs +++ b/src/dotnet/Modgud.Provisioning.TestKit/ModgudProvisioningClient.cs @@ -104,11 +104,11 @@ private static async Task ThrowFromResponseAsync( var body = await response.Content.ReadAsStringAsync(ct); try { - // Two error shapes reach this client, both using a field named "error": - // manifest endpoints → { "Error": "", "Message": "" } - // canonical ErrorOr → { "error": "" } (no code at all) - // Telling them apart by whether "Message" is present keeps a description from - // masquerading as an error code on the second shape. + // Modgud answers in ONE shape: { "Error": "", "Message": "" }. + // The description-only fallback below is for older servers, which sent + // { "error": "" } from the non-manifest endpoints — reading that as + // a code would pass prose off as machine-readable. Presence of "Message" tells + // the two apart. var error = JsonSerializer.Deserialize(body, JsonOptions); if (error?.Message is { Length: > 0 }) { diff --git a/src/dotnet/Modgud.Tests.Unit/Authentication/ExtensionMethods/ErrorOrExtensionsTests.cs b/src/dotnet/Modgud.Tests.Unit/Authentication/ExtensionMethods/ErrorOrExtensionsTests.cs index 959d57cc..8d1b159c 100644 --- a/src/dotnet/Modgud.Tests.Unit/Authentication/ExtensionMethods/ErrorOrExtensionsTests.cs +++ b/src/dotnet/Modgud.Tests.Unit/Authentication/ExtensionMethods/ErrorOrExtensionsTests.cs @@ -10,6 +10,13 @@ namespace Modgud.Tests.Unit.Authentication.ExtensionMethods; /// minimal-API . Every error type maps to a specific status /// code and shape — admin endpoints rely on these to surface inline form errors, /// so a regression that downgrades a 400 to a 500 hides validation messages. +/// +/// The BODY is pinned here too, not just the status. It is one shape for every +/// error — { "Error": "<code>", "Message": "<description>" } — and it used +/// to be neither: the code was dropped entirely and the description sat under a key +/// named error, the key OAuth reserves for a code. Three endpoint files had each +/// grown a private copy of this mapper to work around that. Asserting only the result +/// TYPE is what let the shape drift unnoticed, so these tests read the payload. /// public class ErrorOrExtensionsTests { @@ -118,23 +125,21 @@ public void Validation_maps_to_400() } [Fact] - public void Unauthorized_maps_to_401() + public void Unauthorized_maps_to_401_with_a_body() { - ErrorOr input = Error.Unauthorized(description: "no auth"); + ErrorOr input = Error.Unauthorized("Auth.Missing", "no auth"); - // UnauthorizedHttpResult exposes 401 via IStatusCodeHttpResult. - Assert.IsType(input.ToResult()); + AssertError(input.ToResult(), StatusCodes.Status401Unauthorized, "Auth.Missing", "no auth"); } [Fact] - public void Forbidden_maps_to_403() + public void Forbidden_maps_to_403_with_a_body() { - ErrorOr input = Error.Forbidden(description: "denied"); + // Deliberately NOT Results.Forbid(): under this app's cookie auth that renders + // an /api/* response as an EMPTY-body 403, throwing away the code the SPA needs. + ErrorOr input = Error.Forbidden("Role.RealmAdminForbidden", "denied"); - // Results.Forbid() returns ForbidHttpResult — note: it does NOT carry a - // status code property (status is set by the auth handler), so we just - // pin the type. - Assert.IsType(input.ToResult()); + AssertError(input.ToResult(), StatusCodes.Status403Forbidden, "Role.RealmAdminForbidden", "denied"); } [Fact] @@ -146,26 +151,30 @@ public void Conflict_maps_to_409() } [Fact] - public void Failure_maps_to_problem_500() + public void Failure_maps_to_500_with_a_body() { - ErrorOr input = Error.Failure(description: "boom"); + ErrorOr input = Error.Failure("Some.Failure", "boom"); - var result = input.ToResult(); - - var problem = Assert.IsType(result); - Assert.Equal(500, problem.StatusCode); - Assert.Equal("boom", problem.ProblemDetails.Detail); + AssertError(input.ToResult(), StatusCodes.Status500InternalServerError, "Some.Failure", "boom"); } [Fact] - public void Unexpected_maps_to_problem_500() + public void Unexpected_maps_to_500_with_a_body() { ErrorOr input = Error.Unexpected(description: "weird"); - var result = input.ToResult(); + AssertError(input.ToResult(), StatusCodes.Status500InternalServerError, message: "weird"); + } + + [Fact] + public void The_body_carries_the_machine_readable_code_not_just_prose() + { + // The whole point of the shape: a client can branch on Error without matching + // on a human-readable Message that is free to change. + ErrorOr input = Error.Conflict("Group.NameTaken", "A group of that name already exists."); - var problem = Assert.IsType(result); - Assert.Equal(500, problem.StatusCode); + AssertError(input.ToResult(), StatusCodes.Status409Conflict, + "Group.NameTaken", "A group of that name already exists."); } [Fact] @@ -189,4 +198,19 @@ private static void AssertStatus(IResult result, int expected) var status = Assert.IsAssignableFrom(result); Assert.Equal(expected, status.StatusCode); } + + /// Pins the status AND the one error body every endpoint answers with. + private static void AssertError(IResult result, int expectedStatus, string? code = null, string? message = null) + { + AssertStatus(result, expectedStatus); + + var value = Assert.IsAssignableFrom(result).Value; + Assert.NotNull(value); + + var type = value.GetType(); + if (code is not null) + Assert.Equal(code, type.GetProperty("Error")!.GetValue(value)); + if (message is not null) + Assert.Equal(message, type.GetProperty("Message")!.GetValue(value)); + } } From c96737a42c7389184d32638ccba5b2f252eb4cb9 Mon Sep 17 00:00:00 2001 From: Bernhard Windisch Date: Wed, 9 Sep 2026 21:45:49 +0200 Subject: [PATCH 7/7] ci: audit CVEs through restore, so a reviewed exception can be recorded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit gate failed on GHSA-23fw-v26w-5fgq — Microsoft.Build.Tasks.Git 10.0.301, pulled in transitively by Microsoft.SourceLink.GitHub in the two packable projects. It has been latent on develop and only surfaced now: this is the first backend-touching PR since the advisory appeared, and the backend job is skipped for frontend-only changes. There is no fix to take — every version above 10.0.301 is an 11.0.100 preview or rc. And the exposure is build-time only: SourceLink is referenced with PrivateAssets=all and runs as an MSBuild task at pack time to embed git metadata, reaching neither the produced package, nor the running product, nor any consumer's dependency graph. So the exception is worth recording. Recording one was impossible with the old gate. The `dotnet list package` vulnerability report queries the advisory database directly and ignores , so the only ways past it were to disable the step or to take a pre-release dependency. The gate is now NuGet's restore-time audit (NU1901-NU1904), which honours suppressions: an exception has to be written down in Directory.Build.props next to its justification, and everything else still fails the build. Audit settings are pinned there rather than inherited from SDK defaults, at mode 'all' (transitive included) and level 'low' (nothing filtered out by severity). Restore and audit are deliberately ONE step: a second `dotnet restore` is incremental and emits no warnings, so a separate audit step would have passed by doing nothing. The step pipes through tee rather than capturing into a variable. Under `bash -e`, `out=$(dotnet restore ...)` aborts the step before anything is echoed, so a restore that fails outright prints nothing at all and the log shows only an exit code — which is exactly how a malformed props file cost a diagnosis round here. Verified by removing the suppression and checking the exit code, not just grepping for a pattern: without it restore reports 2 NU1902 warnings naming GHSA-23fw-v26w-5fgq and the gate fails; with it, zero. The audit has teeth and the suppression is what silences it. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 22 +++++++++++++-------- src/dotnet/Directory.Build.props | 33 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0f857c4b..3514e90d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,16 +79,22 @@ jobs: - name: Log version run: echo "Building version ${{ steps.dotnet.outputs.version }}" - - name: Restore backend dependencies - run: dotnet restore Modgud.slnx - working-directory: ./src/dotnet - - - name: Audit NuGet dependencies for known CVEs + # Restore IS the CVE audit. NuGet's restore-time audit (NU1901-NU1904, configured + # in src/dotnet/Directory.Build.props) is used rather than + # `dotnet list package --vulnerable`, because that command queries the advisory + # database directly and IGNORES — leaving no supported way to + # record a reviewed, justified exception. Restore honours it. + # + # The two steps are deliberately one: a second `dotnet restore` is incremental and + # emits no warnings at all, so a separate audit step would pass by doing nothing. + - name: Restore backend dependencies (audits for known CVEs) run: | set -o pipefail - out=$(dotnet list package --vulnerable --include-transitive 2>&1) - echo "$out" - if echo "$out" | grep -qE "has the following vulnerable packages"; then + # tee, not command substitution: a captured $(...) under `bash -e` aborts the + # step BEFORE anything is echoed, so a restore that fails outright prints + # nothing at all and the log shows only an exit code. + dotnet restore Modgud.slnx 2>&1 | tee restore.log + if grep -qE "warning NU190[1-4]" restore.log; then echo "::error ::Vulnerable NuGet packages detected. See log above." exit 1 fi diff --git a/src/dotnet/Directory.Build.props b/src/dotnet/Directory.Build.props index b92f572c..5e7cae27 100644 --- a/src/dotnet/Directory.Build.props +++ b/src/dotnet/Directory.Build.props @@ -23,4 +23,37 @@ runtime; build; native; contentfiles; analyzers; buildtransitive + + + + true + all + low + + + + + +