fix(provisioning): a Management API consumer's first run is one file, one apply

docs/admin/realm-provisioning.md

@@ -100,12 +100,17 @@ happens to share it. Two forms of `Id`, and nothing else:
 
 **Cross-references between entities follow the same rule:**
 
-- Groups list **`Members`** (users) and **`Roles`**; positions list **`Grants`** (users).
-  Each entry is a real id (`{ "Key": "acme/Author", "Id": "…" }`, where the `Key` is a
-  *verified hint* — never followed, and reported when it disagrees), or a `"#handle"`.
-  A **bare name is an error**: the "acme/Author" in the target realm need not be the one
-  the file was written against. Exports write the object form. Group membership is the
-  *only* way users get roles.
+- Groups list **`Members`** (users, nested groups, service accounts) and **`Roles`**;
+  positions list **`Grants`** (users). Each entry is a real id (`{ "Key": "acme/Author",
+  "Id": "…" }`, where the `Key` is a *verified hint* — never followed, and reported when it
+  disagrees), or a `"#handle"`. A **bare name is an error**: the "acme/Author" in the
+  target realm need not be the one the file was written against. Exports write the object
+  form. Group membership is the *only* way users get roles.
+- Sections apply in dependency order — apps, APIs, scopes, clients, login providers,
+  roles, users, **service accounts, then groups**, positions — so a group may name a user
+  *or a service account* this same file creates by `#handle`. "A machine identity, a
+  role and a group binding the two" is one file, one run. A role may sit on the system
+  app (`"App": "modgud"`) — that is where the Management API permissions live.
 - A real id the target realm does not have is **skipped and reported**; a `#handle` the
   file never declares is an **error**. A missing target is a fact about the target; a
   dangling handle is the file contradicting itself.

src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestApplierTests.cs

@@ -1513,6 +1513,110 @@ await InTenantAsync(factory, slug, async sp =>
         });
     }
 
+    /// <summary>
+    /// The first run of a Management API consumer, reported from the field: a role on
+    /// the SYSTEM app (that is where <c>oauth-authorization:*</c> lives), a service
+    /// account, and a group binding the two — one file, handles throughout. Three things
+    /// used to stand in the way, none of them in the apply itself: service accounts applied
+    /// AFTER groups, so <c>"#mgmt-sa"</c> in Members was "applied too late"; the planner
+    /// resolved members against users only, so a service-account member — written by the
+    /// export in its own <c>{ Key, Id }</c> form — read as "no such entity"; and the
+    /// planner took its app catalog off the export, which omits the system app, so a role
+    /// on it was announced as skipped while the apply created it fine.
+    /// </summary>
+    [Fact]
+    public async Task A_management_api_consumer_provisions_role_service_account_and_group_in_one_run()
+    {
+        await using var host = await Fixture.CreateIsolatedHostAsync();
+        var factory = host.Factory;
+        var ct = TestContext.Current.CancellationToken;
+        var applier = factory.Services.GetRequiredService<RealmManifestApplier>();
+        var planner = factory.Services.GetRequiredService<RealmManifestPlanner>();
+        var exporter = factory.Services.GetRequiredService<RealmManifestExporter>();
+
+        const string slug = "consumer";
+        Assert.False((await ProvisionRealmAsync(factory, Shell(slug), new RealmManifest(), ct)).IsError);
+
+        var manifest = new RealmManifest
+        {
+            Roles =
+            [
+                new RealmManifestRole
+                {
+                    Id = "#mgmt-role", Name = "consumer-management", App = AppSlugs.Modgud,
+                    Permissions = [new("oauth-authorization", "read"), new("oauth-authorization", "revoke")],
+                },
+            ],
+            ServiceAccounts =
+            [
+                new RealmManifestServiceAccount
+                {
+                    Id = "#mgmt-sa", AccountName = "consumer-management",
+                    Credentials = [new RealmManifestServiceAccountCredential { ClientId = "consumer-management" }],
+                },
+            ],
+            Groups = [new RealmManifestGroup { Name = "consumer-management", Members = ["#mgmt-sa"], Roles = ["#mgmt-role"] }],
+        };
+
+        // ── Plan: no contradiction, no phantom skip — the plan sees the system app and
+        //    the account exactly as the apply will. ─────────────────────────────────────
+        var plan = await planner.PlanAsync(slug, manifest, prune: false, ct: ct);
+        Assert.False(plan.IsError, plan.IsError ? plan.FirstError.Description : string.Empty);
+        Assert.DoesNotContain(plan.Value.Sections, s => s.Name == "manifest");
+        var roleEntry = Assert.Single(plan.Value.Sections.Single(s => s.Name == "roles").Entries);
+        Assert.Equal("create", roleEntry.Action);
+        Assert.DoesNotContain(roleEntry.Notes, n => n.Contains("does not exist here"));
+        var groupEntry = Assert.Single(plan.Value.Sections.Single(s => s.Name == "groups").Entries);
+        Assert.Equal("create", groupEntry.Action);
+        Assert.DoesNotContain(groupEntry.Notes, n => n.Contains("no such entity"));
+
+        // ── Apply: everything lands in one run. ────────────────────────────────────────
+        var applied = await applier.UpdateRealmAsync(slug, manifest, ct: ct);
+        Assert.False(applied.IsError, applied.IsError ? applied.FirstError.Description : string.Empty);
+        Assert.True(applied.Value.ClientSecrets.ContainsKey("consumer-management"));
+        Assert.Empty(applied.Value.SkippedReferences);
+        Guid saId = default;
+        await InTenantAsync(factory, slug, async sp =>
+        {
+            var session = sp.GetRequiredService<IDocumentSession>();
+            var sa = await session.Query<ServiceAccount>().SingleAsync(s => !s.IsDeleted && s.AccountName == "consumer-management", ct);
+            saId = sa.Id;
+            var group = await session.Query<Group>().SingleAsync(g => !g.IsDeleted && g.Name == "consumer-management", ct);
+            Assert.Contains(sa.Id, group.MemberIds);
+            var role = await session.Query<PermissionRole>().SingleAsync(r => !r.IsDeleted && r.Name == "consumer-management", ct);
+            Assert.Contains(role.Id, group.RoleIds);
+            var system = await session.Query<App>().SingleAsync(a => !a.IsDeleted && a.Slug == AppSlugs.Modgud, ct);
+            Assert.Equal(system.Id, role.AppId);
+            Assert.Equal(2, role.PermissionIds.Count);
+        });
+
+        // ── Export → plan: the account is a member in the export's own form and reads
+        //    as unchanged, without a note; so does the role on the system app. ─────────
+        var exported = await exporter.ExportRealmAsync(slug, ct);
+        Assert.False(exported.IsError);
+        var exGroup = Assert.Single(exported.Value.Groups, g => g.Name == "consumer-management");
+        Assert.Equal(["consumer-management"], exGroup.Members!.Select(m => m.Key));
+        var again = await planner.PlanAsync(slug, exported.Value, prune: false, ct: ct);
+        Assert.False(again.IsError);
+        var againGroup = Assert.Single(again.Value.Sections.Single(s => s.Name == "groups").Entries, e => e.Key == "consumer-management");
+        Assert.Equal("unchanged", againGroup.Action);
+        Assert.Empty(againGroup.Notes);
+        var againRole = Assert.Single(again.Value.Sections.Single(s => s.Name == "roles").Entries, e => e.Key == $"{AppSlugs.Modgud}/consumer-management");
+        Assert.Equal("unchanged", againRole.Action);
+        Assert.Empty(againRole.Notes);
+
+        // The id-only spelling of the same member is the same membership, not a change.
+        var idOnly = exported.Value with
+        {
+            Groups = [exGroup with { Members = [new ManifestRef { Id = new ShortGuid(saId).ToString() }] }],
+        };
+        var idPlan = await planner.PlanAsync(slug, idOnly, prune: false, ct: ct);
+        Assert.False(idPlan.IsError);
+        var idGroup = Assert.Single(idPlan.Value.Sections.Single(s => s.Name == "groups").Entries, e => e.Key == "consumer-management");
+        Assert.Equal("unchanged", idGroup.Action);
+        Assert.Empty(idGroup.Notes);
+    }
+
     /// <summary>
     /// A `#handle` proves nothing about what it names — unlike a real id, which proves its
     /// type by loading the document. Three ways that can go wrong, all of which used to

src/dotnet/Modgud.Api/Features/Admin/Provisioning/ManifestIdentity.cs

@@ -162,9 +162,10 @@ void CheckRef(ManifestRef? reference, string what, string wantedSection, string
 
         foreach (var g in manifest.Groups)
         {
-            // A member may be a user, a nested group, or a service account. Service
-            // accounts apply AFTER groups, so one of those has to be named by its real id
-            // — the order check says so up front rather than dropping it at apply time.
+            // A member may be a user, a nested group, or a service account. Users and
+            // service accounts both apply BEFORE groups, so either may be a '#handle' this
+            // file declares; a nested group's handle resolves only when that group comes
+            // earlier in the list (same section — the order check lets it through).
             foreach (var m in g.Members ?? [])
                 CheckRef(m, "user, group or service account",
                     $"{Sections.Users}|{Sections.Groups}|{Sections.ServiceAccounts}",
@@ -246,8 +247,8 @@ void CheckVocabulary(string? value, string what, string ctx)
         Sections.LoginProviders => 5,
         Sections.Roles => 6,
         Sections.Users => 7,
-        Sections.Groups => 8,
-        Sections.ServiceAccounts => 9,
+        Sections.ServiceAccounts => 8,
+        Sections.Groups => 9,
         Sections.Positions => 10,
         _ => int.MaxValue,
     };

src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifest.cs

@@ -482,7 +482,7 @@ public sealed record RealmManifestGroup
     [Description("Optional description. Absent = unchanged; explicit null clears.")]
     public Optional<string?> Description { get; init; }
 
-    [Description("Members for MembershipMode=Manual — a user, a NESTED GROUP, or a service account. Each entry names one by IDENTITY (ADR 0024): { \"Key\": \"alice\", \"Id\": \"<id>\" } for one that exists here (missing = reported skip), or \"#alice\" for one this same manifest creates (undeclared = error). A bare name is an error. A service-account member must use a real id — service accounts apply after groups, so a handle would not exist yet. Absent = unchanged; [] clears the member list.")]
+    [Description("Members for MembershipMode=Manual — a user, a NESTED GROUP, or a service account. Each entry names one by IDENTITY (ADR 0024): { \"Key\": \"alice\", \"Id\": \"<id>\" } for one that exists here (missing = reported skip), or \"#alice\" for one this same manifest creates (undeclared = error). A bare name is an error. Users and service accounts apply before groups, so either may be a handle; a nested group's handle resolves only when that group is listed earlier. Absent = unchanged; [] clears the member list.")]
     public List<ManifestRef>? Members { get; init; }
 
     [Description("Roles this group grants to its members. Each entry names a role by IDENTITY (ADR 0024): { \"Key\": \"acme/Author\", \"Id\": \"<role id>\" } for a role that exists here (missing = reported skip), or \"#author\" for a role this same manifest creates (undeclared = error). A bare name is an error. Absent = unchanged; [] clears.")]

src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestApplier.cs

@@ -644,6 +644,12 @@ await SetUserTwoFactorPolicyAsync(session, existing.Id,
             }
         }
 
+        // ── Service accounts (hulls, id-pinned creates) — BEFORE groups, like users: a
+        //    group's Members may name an account this same file creates by '#handle', and
+        //    "M2M identity + role + group" is the standard first run of every Management
+        //    API consumer. Credentials only reference apps and scopes, both applied above.
+        await ApplyServiceAccountsAsync(sp, manifest, identity, apps, secrets, skips, ct);
+
         // ── Groups (identity = Id; the Name is mutable) ───────────────────────────
         if (manifest.Groups.Count > 0)
         {
@@ -741,9 +747,6 @@ await SetUserTwoFactorPolicyAsync(session, existing.Id,
             }
         }
 
-        // ── Service accounts (hulls, id-pinned creates) ───────────────────────────
-        await ApplyServiceAccountsAsync(sp, manifest, identity, apps, secrets, skips, ct);
-
         // ── Positions (MG-FT) — after users so grants can resolve their handles ───
         await ApplyPositionsAsync(sp, manifest, identity, apps, secrets, skips, ct);
 

src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs

@@ -330,8 +330,15 @@ List<RealmManifestApp> CanonApps(List<RealmManifestApp> apps) => [.. apps.Select
         foreach (var r in current.Roles) roleRefs.Add(r.NaturalKey, r.Id);
         var userRefs = new RefCanonicalizer();
         foreach (var u in current.Users) userRefs.Add(u.Key ?? u.UserName ?? u.Email, u.Id);
+        // A group's Members are users, nested groups AND service accounts — the exporter
+        // writes all three as { Key, Id }, so all three have to canonicalize, or a service
+        // account written id-only reads as a change the apply would never make.
+        var memberRefs = new RefCanonicalizer();
+        foreach (var u in current.Users) memberRefs.Add(u.Key ?? u.UserName ?? u.Email, u.Id);
+        foreach (var g in current.Groups) memberRefs.Add(g.Name, g.Id);
+        foreach (var s in current.ServiceAccounts) memberRefs.Add(s.AccountName, s.Id);
         RealmManifestGroup CanonGroup(RealmManifestGroup g)
-            => g with { Members = userRefs.Canon(g.Members), Roles = roleRefs.Canon(g.Roles) };
+            => g with { Members = memberRefs.Canon(g.Members), Roles = roleRefs.Canon(g.Roles) };
         var positionRefs = new RefCanonicalizer();
         foreach (var p in current.Positions) positionRefs.Add(p.AccountName.Trim().ToLowerInvariant(), p.Id);
         RealmManifestPosition CanonPosition(RealmManifestPosition p)
@@ -398,7 +405,12 @@ await GroupMembershipGuards.GroupConfersRealmAdminAsync(session, perms, doc, ct)
                 },
             }));
 
-        AddSkippedReferenceNotes(manifest, current, result);
+        // The realm's apps are read LIVE, not off the export: the export leaves the system
+        // app out on purpose (it is seeded, not authored), but a role, client, API or scope
+        // may point at it all the same — and the applier resolves it, because its app
+        // resolver is seeded from the same query. The plan has to see what the apply sees.
+        var liveApps = await session.Query<App>().Where(a => !a.IsDeleted).ToListAsync(ct);
+        AddSkippedReferenceNotes(manifest, current, liveApps, result);
         return result;
     }
 
@@ -414,15 +426,23 @@ await GroupMembershipGuards.GroupConfersRealmAdminAsync(session, perms, doc, ct)
     /// reference is resolved they exist.</para>
     /// </summary>
     private static void AddSkippedReferenceNotes(
-        RealmManifest manifest, RealmManifest current, RealmPlanResult result)
+        RealmManifest manifest, RealmManifest current, IReadOnlyList<App> liveApps, RealmPlanResult result)
     {
-        var appSlugs = manifest.Apps.Select(a => a.Slug)
-            .Concat(current.Apps.Select(a => a.Slug))
+        // Every app the realm HAS (system app included — the export omits it, the applier's
+        // resolver does not) plus every app the manifest creates.
+        var appSlugs = liveApps.Select(a => a.Slug)
+            .Concat(manifest.Apps.Select(a => a.Slug))
             .ToHashSet(StringComparer.Ordinal);
 
-        // resource:action keys per app slug, from both sides.
+        // resource:action keys per app slug, live catalog first, manifest entries on top
+        // (a manifest may add entries to an app it also updates).
         var catalog = new Dictionary<string, HashSet<string>>(StringComparer.Ordinal);
-        foreach (var app in current.Apps.Concat(manifest.Apps))
+        foreach (var app in liveApps)
+        {
+            var set = catalog[app.Slug] = new HashSet<string>(StringComparer.Ordinal);
+            foreach (var perm in app.Permissions) set.Add($"{perm.Resource}:{perm.Action}");
+        }
+        foreach (var app in manifest.Apps)
         {
             if (app.Permissions is null) continue;
             if (!catalog.TryGetValue(app.Slug, out var set))
@@ -439,13 +459,26 @@ private static void AddSkippedReferenceNotes(
         var userIds = manifest.Users.Select(u => NormalizedId(u.Id))
             .Concat(current.Users.Select(u => NormalizedId(u.Id)))
             .OfType<string>().ToHashSet(StringComparer.Ordinal);
+        // A group member is a user, a nested group or a service account — the same three
+        // kinds the applier's member resolver accepts.
+        var memberIds = userIds
+            .Concat(manifest.Groups.Select(g => NormalizedId(g.Id)))
+            .Concat(current.Groups.Select(g => NormalizedId(g.Id)))
+            .Concat(manifest.ServiceAccounts.Select(s => NormalizedId(s.Id)))
+            .Concat(current.ServiceAccounts.Select(s => NormalizedId(s.Id)))
+            .OfType<string>().ToHashSet(StringComparer.Ordinal);
         // The readable name each live id actually carries, for the verified-hint check.
         var roleKeyById = current.Roles
             .Where(r => NormalizedId(r.Id) is not null)
             .ToDictionary(r => NormalizedId(r.Id)!, r => r.NaturalKey, StringComparer.Ordinal);
         var userKeyById = current.Users
             .Where(u => NormalizedId(u.Id) is not null)
             .ToDictionary(u => NormalizedId(u.Id)!, u => u.ResolveKey(), StringComparer.Ordinal);
+        var memberKeyById = new Dictionary<string, string>(userKeyById, StringComparer.Ordinal);
+        foreach (var g in current.Groups)
+            if (NormalizedId(g.Id) is { } gid) memberKeyById.TryAdd(gid, g.Name);
+        foreach (var s in current.ServiceAccounts)
+            if (NormalizedId(s.Id) is { } sid) memberKeyById.TryAdd(sid, s.AccountName);
 
         RealmPlanEntry? Entry(string section, string key)
         {
@@ -533,7 +566,7 @@ static string Skipped(string what) =>
                 if (ReferenceNote(reference, "Role", roleIds, roleKeyById) is { } note)
                     Note("groups", group.Name, note);
             foreach (var reference in group.Members ?? [])
-                if (ReferenceNote(reference, "Member", userIds, userKeyById) is { } note)
+                if (ReferenceNote(reference, "Member", memberIds, memberKeyById) is { } note)
                     Note("groups", group.Name, note);
         }