Link several roles from one grant, and name every role by id

[kisielewski]

Implements A_GRANT_LINKS_SEVERAL_ROLES_2026-09-19. This breaks the contract, which 0.2 is allowed to do.

A grant links any number of roles and holds its own entries beside them; what it confers is the union. Every role is named by its id — mapping rules, a provider's default roles, an activity's enrollment sets, the built-ins, which are found by a new BuiltInKey so a renamed shipped role keeps working.

Why, and what it fixes

An end-to-end run against a real Keycloak and a real Moodle 5.2.2 found nineteen defects in the model of 2026-09-13, two of them privilege escalations: a manager whose grant sat at system scope — which is what a directory group hands out — could edit the shipped roles and empty admin; and an activity's role could be named where an installation role was expected. The 209 tests then in this repository saw none of them. Every one has a regression test here.

The model

• GrantRoles, one row per linked role, carrying SourceProviderId (who added it) and DismissedAt. The tombstone exists for one case: an LTI launch adds the roles its rules name and never removes one, so without it a manager's correction would be undone at that person's next launch.
• Roles.BuiltInKey, filtered-unique and global-only, so every enrollment path resolves a shipped role by key rather than by name.
• IdentityProviderMappingRules name a role by id and carry a target — a role, or the activity's participant or manager enrollment set. Only a platform's rules may use the last two.
• IdentityProviders.Kind separates a sign-in provider from the row an LTI platform speaks through, so a platform no longer appears in the providers list. LtiBoundaryTests still holds: nothing outside Lti/ names LTI.
• ActivityEnrollmentRoles replaces the two role columns. Absent means unchanged, an empty list clears.
• role:manage is global and administrator-only; role:manage:activity is new, scoped, and in the manager role. The catalog is 55 keys, the manager role 37.

Two defects found while building this, which no gate would have caught

A contribution the upgrade leaves as a copy would have outlived every mapping change. Nothing recorded which claim values produced a contribution written before this change, so the migration cannot turn one into links. The sign-in that rewrites it only wrote links, so the stale copy would have kept granting what it held for as long as the account existed. It now empties the row's own entries too. Found by reading what the migration did to a development database holding real data from the end-to-end run, and pinned by a test that fails when the fix is reverted.

A role link added only through a navigation is tracked Modified, not Added. Every model here assigns its own key in the constructor, so EF issued an UPDATE for a row that was never inserted, zero rows were affected, and a second LTI launch answered 409 concurrency.conflict on a grant nobody else had touched. It affected every addition of a role to an existing grant, in the launch path and in the panel. Four call sites now write to the set as well as to the collection.

The migration

20260919220105_aGrantLinksSeveralRoles, hand-ordered: schema additions, then nine data steps, then the drops, then the constraints. The data steps rewrite role:manage in three cases, set BuiltInKey, link the grants whose set still equals the role they were made from, carry SourceProviderId onto the new links, move mapping rules from names to ids (deleting the unresolvable), and fill the enrollment tables. Down collapses the links back and folds several into Permissions, which is a widening and says so in a comment.

Rehearsed against a development database carrying the end-to-end run's data: 33 grants, 22 links, 12 roles, 6 rules, and every activity grant's copy correctly recognized and linked.

Measured

• dotnet test: 981 passed, 0 failed. dotnet build -warnaserror: 0 warnings.
• openapi.json regenerated from the container, twice — the second time after the platform mapping-rule surface.
• The end-to-end suite against a real Keycloak: 22 checks, 0 failed, including two groups resolving to a union of roles in one grant with no permissions of its own, a rename leaving the mapping alone, and an activity's role refused in a mapping and as a default.
• Every new test was sabotaged to prove it bites.

Also here

An operator can now read and write a platform's role rules. They existed and were seeded, and nothing could reach them — so "which LTI role means which role here is configuration" was true only in the database.

Not covered

Nothing re-derives a provider contribution the migration left as a copy; it is corrected at that person's next sign-in, and the release note has to say so. Roles accumulate, because nothing subtracts — pruning a role a rule no longer names is manual. Revoking a grant is not a ban: the next launch enrolls that person again.

[kisielewski]

A review round, after CI went green

Every check on this pull request passed on the first push. A review of the diff afterwards found four defects none of them could see, and they are fixed in the commits above with a regression test each — every test sabotaged to prove it bites.

An activity's enrollment roles never reached the wire. Projections.ManagedActivity read them from activity.EnrollmentRoles, a navigation nothing includes. Proved against a running Server: the PUT answered with the role, because it projects the entity it has just tracked, a fresh GET answered [], and the row was in the table the whole time. A manager would have chosen the roles a course enrolls into, saved, and watched the pickers come back blank. The test that covered this asserted the list was empty — it passed for exactly the reason the feature was broken. The roles are a parameter now, so a caller that does not supply them does not compile.

Deleting an activity's role asked for role:manage. Which is global and an administrator's since this change, so a manager could create a role in their own activity, be offered the delete button, and get a 403. It goes through RequireRoleWriteAsync like creating and editing one.

A write that said nothing about roles computed the union from nothing. "Absent leaves the roles alone" reached the stored links but not the union every rule beside them reads, so saving a grant without naming its roles recomputed the staff flag from an empty set: a jury member linking manager silently became a competitor and rejoined the ranking, and the installation's sole administrator saving their own grant was refused for losing an administratorship the write never touched. The Client's fake implemented the same contract correctly, so the Server and the fake shipped in one change disagreed about this case.

Three readers still saw a grant's own entries and not its roles. AccountDeletionService, AccountMergeService and the live ranking push. The first is the one that matters: a grant carries its permissions in links, so an administrator who signs in through a directory read as holding nothing, and a provider's deletion webhook would have anonymized their account instead of stopping for a person. That hole has been open since 2026-09-13; this pull request's own ADR claimed it was closed, which is what the review caught.

Four more, smaller, from the same round: the enrollment-role excess rule read r.Role without including it, so it never exempted what the slot already carried; AddRolesAsync returned early before activating, so a launch whose roles were all already linked left an invited grant invited; the migration identified the shipped manager role by name, so a renamed one kept the installation-wide key — in a change whose entire subject is not using names; and the platform-is-not-a-door test gained a positive control, because the list it asserts an absence from carries enabled providers only and would have been empty whatever the code did.

dotnet test is green with these in, and openapi.json was regenerated from the container again.