fix(oauth): backfill ept:pushed_authorization on pre-existing clients (ID2183)#169
Merged
Merged
Conversation
Since Modgud advertises the RFC-9126 /connect/par endpoint, current .NET OIDC clients call it by default (PushedAuthorizationBehavior.UseIfAvailable). BuildClientPermissions() grants ept:pushed_authorization as a baseline, but OAuthAdminService only recalculates permissions when a grant/scope change (or an unrelated re-save) comes through — so clients persisted before the baseline kept their old permission list and failed the PAR challenge with unauthorized_client / OpenIddict ID2183 until an admin re-saved them. Add PushedAuthorizationPermissionBackfill, a one-time idempotent boot migration (same cold-start-walks-every-realm pattern as LegacyAdminGroupRenameBootstrap): for every active realm it enters the tenant context and appends a SetPermissions event adding the missing PAR endpoint permission to each non-deleted client that lacks it. Add-only — grants, scopes, response types and other endpoints are carried through unchanged, and the separate ft:par requirement (RequirePushedAuthorizationRequests) is never touched, so this grants "may use PAR", never "must". The projection carries the authoritative permission set, so only streams that actually need it are aggregated + appended; a repeat boot finds none and appends nothing. Tests (Modgud.Api.Tests/Authorization/PushedAuthorizationPermissionBackfillTests): adds the permission while preserving the rest + ft:par and is idempotent (one append, second boot none); a client that already has it is untouched; a stripped legacy client is refused by /connect/par (ID2183) before the backfill and accepted (201) after — no manual re-save required. Ref: Atlas modgud/engineering/par-endpoint-permission-backfill-missing (DevOrbs incident 2026-07-22). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes the upgrade/backfill bug reported in Atlas
modgud/engineering/par-endpoint-permission-backfill-missing(verified via the DevOrbs incident 2026-07-22).Since Modgud advertises the RFC-9126
/connect/parendpoint, current .NET OIDC clients call it by default (PushedAuthorizationBehavior.UseIfAvailable).BuildClientPermissions()grantsept:pushed_authorizationas a baseline, butOAuthAdminServiceonly recomputes permissions when a grant/scope change (or an unrelated re-save) is sent. Clients persisted before that baseline kept their old permission list and failed the PAR challenge withunauthorized_client/ OpenIddict ID2183 until an admin re-saved them.Fix
PushedAuthorizationPermissionBackfill— a one-time, idempotent boot migration (same cold-start-walks-every-realm shape asLegacyAdminGroupRenameBootstrap):SetPermissionsevent adding the missingept:pushed_authorizationto every non-deleted client that lacks it.ft:parrequirement (RequirePushedAuthorizationRequests) lives in the separateRequirementslist and is never touched — this grants may use PAR, never must.Acceptance criteria → tests
Modgud.Api.Tests/Authorization/PushedAuthorizationPermissionBackfillTests.cs:ept:pushed_authorizationadded, exactly (previous perms + PAR), withft:parand all requirements byte-identical; one append happened; a second boot appends nothing (idempotent)./connect/par(ID2183) before the backfill and accepted (201) after — no manual re-save.Criterion "no cross-tenant write" is structurally guaranteed by the per-realm
TenantContext.Enterpattern (same as the tested group-rename bootstrap).Verification
🤖 Generated with Claude Code