Repository navigation
fix(oauth): a loopback redirect URI matches on any port — local MCP clients can log in - #238
Merged
Merged
Conversation
…lients can log in Reported from the field (amZettel, 2026-09-19, priority high): Claude Code against an MCP server behind Modgud failed at /connect/authorize with invalid_request (ID2043), "'http://localhost:40489/callback' was not a valid redirect_uri". Its CIMD document registers http://localhost/callback port-less, as every local MCP client does (Cursor, VS Code, the MCP Inspector too): a native app cannot reserve a port, it takes an ephemeral one at request time, and RFC 8252 §7.3 requires the server to accept whatever port arrives. That blocked EVERY local MCP client — the one class of client the DCR/CIMD work exists for. OpenIddict already implements the tolerance (loopback on both sides, registered URI without a port, same scheme and path), but only for a client whose application type is "native" — and nothing in Modgud ever set it. The synthesized CIMD client was hard-wired "web"; DCR never read application_type and created with null; admin-created clients had no way to say it. Three places, one gap. One rule, in the one place OpenIddict asks. MartenApplicationStore.GetApplicationTypeAsync now returns the EFFECTIVE type (OAuthApplicationTypes.Effective): a client with a loopback http redirect URI is native whatever it declared — only a native app can have such a URI in the first place (OIDC DCR §2) — otherwise the declared type stands. That covers CIMD documents, DCR registrations old and new (no re-registration needed) and admin-created clients alike, is rolling-safe (no stored value changes), and loosens nothing else: OpenIddict relaxes the port only between loopback URIs, https URIs still match exactly. Around it: the CIMD parser reads an optional application_type (web | native, anything else invalid) and Synthesize uses the effective type instead of "web"; DCR reads application_type, rejects anything but the two literal values as invalid_client_metadata, records the declaration on the client and echoes the effective value in the response; CreateOAuthClientDto carries ApplicationType through so DCR (and the admin API) can set it. The registration validators are unchanged — they already knew the rule the runtime check did not. Tests: Loopback_redirect_with_an_ephemeral_port_is_accepted_for_a_cimd_client (the exact Claude Code document; random ports on both loopback hosts and the port-less form pass, another path or a non-loopback host is still refused, and the code exchange completes with the ported URI — red with the store and CIMD resolver reverted, green with the fix), _for_a_dcr_client (native echoed, ephemeral port accepted, bogus type rejected, an https-only client gets nothing invented), plus the unit tests on Effective/IsKnown and the two parsers. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
4 of 5 tasks
windischb
added a commit
that referenced
this pull request
Sep 24, 2026
…e, one draft bar (#242) * fix(oauth): a dynamic client remembers consent when its identity is assured Since #236 AllowRememberConsent is enforced, and DCR and CIMD always stored it false, so every dynamic client saw the consent screen on every authorize - claude.ai and ChatGPT included. For a dynamic client the stored flag is no longer consulted; RFC 8252 §8.6 decides (DynamicClientConsent): a remembered authorization skips the screen when the request's redirect is https on a real host, or the client is confidential (private_key_jwt, a DCR secret). A public client redirecting to loopback or a private-use scheme (Claude Code, VS Code, Zed, Cursor) is still asked every time: its client_id is public and any local process can listen on a loopback port - on any port since #238. Admin-created clients keep their own flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(provisioning)!: an apply never deletes what the manifest leaves out Full-sync prune is removed from every manifest route (control plane, data plane, drafts), together with the two-step confirmation the 0.14 betas carried (409 Manifest.ConfirmationRequired, confirmation tokens, parked review drafts, PruneOnApply). An apply is an additive merge by id; deleting is a staged deletion in a draft, which the plan shows before the apply - for the admin UI and a Management API client alike. The plan loses its Prune field and the "protected" action. A service-account credential is never deleted through the clients section: it leaves through its account's Credentials list, the only place the plan shows it. Found in the pre-release review and fixed on the way: - Draft endpoints took the caller from NameIdentifier, which a bearer principal does not carry: every Management API call answered 500. They read sub first now. - Staging matched a draft entry by natural key only, so renaming an entity inside a draft appended a second entry with the same id. Staging matches by Id first. - A credential with an Id in the manifest was issued under a random id; the next apply of the same file re-issued its client_id and failed on ClientIdAlreadyExists. The issue op now takes the pinned id. - About fifteen strings of the staging UI had no German translation, and a few English fallbacks were German. BREAKING CHANGE: ?prune=true is no longer read; a script sending it gets the additive merge and no deletions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(provisioning): what a click-through of the draft UI turned up - A dormant group (empty BoundTo) came out of export -> apply into another realm bound to the system app: the export wrote the empty list as absent, and a create reads absent as ['modgud']. Empty BoundTo, Capabilities and job Parameters now export as empty, which also removes the phantom "(empty) -> []" plan changes for untouched groups and jobs. - A credential secret created by a draft apply vanished when the apply ran from the staging bar on another admin page (toast only). The bar now opens the drafts workspace, which shows the one-time secrets. - The service-account grid was empty after an apply until a reload: the post-apply refresh skipped service accounts, jobs and the inbox policy, and the applier dispatched no ServiceAccount event. Both fixed; the refresh no longer requests positions with the feature off (404). - A staged deletion can be taken back from its plan entry (the plan note said "unstage", the modal had no way to), and the previous apply's outcome no longer stands above the next draft. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(ui): one footer bar for the draft, the export basket moves to the header A UI review of the draft surface (screenshots of every state) found two look-alike footer bars stacked on each other doing unrelated things: the export basket's "Exportieren…" sat right above "Draft anwenden", both primary, and a draft could only be discarded from inside the workspace or after parking it. - The export selection is a header chip (icon + count) with a panel: per-entry remove, Clear, and a secondary "Download as manifest…". - The staging bar is the only footer bar: accent stripe, the draft name plus "n staged" / "m errors" as one link to the review, Discard (with a danger confirm, from any admin page), Park, and Apply as the single primary. A disabled Apply says why. On narrow windows Discard and Park move into a "⋯" menu. The workspace no longer repeats Apply / Discard. - Confirmations say what they do ("Anwenden", "Verwerfen", "Abbrechen") instead of the library's "OK" / "Cancel"; the library's popconfirm arrow (never positioned, it covered the title's first letter) is hidden. - Lists show the staged state as "New" / "Changed" / "To be deleted", and their delete action reads "Stage deletion" while staging. - A refused staged deletion shows its intent and reason on the plan card; its modal shows the refusal as an error and "Undo the deletion" as the primary action, without the contradicting "applying deletes it" note. - Auto-named drafts read "Draft by <user> · <local time>" (the stored name carries UTC); parked/shared drafts are counted in the sidebar. - Selective export: group roles no longer render as [object Object], the hint wraps, singular/plural wording. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <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
1 participant
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.
Summary
Reported from the field (amZettel, priority high): Claude Code could not log in against an MCP server behind Modgud —
/connect/authorizeansweredinvalid_request(ID2043),'http://localhost:40489/callback' was not a valid redirect_uri. Its CIMD document registershttp://localhost/callbackport-less, as every local MCP client does (Cursor, VS Code, the MCP Inspector too): a native app cannot reserve a port, it takes an ephemeral one at request time, and RFC 8252 §7.3 requires the server to accept whatever port arrives. That blocked the one class of client the DCR/CIMD work exists for.Cause
OpenIddict already implements the tolerance (loopback on both sides, registered URI without a port, same scheme and path) — but only for a client whose application type is
native, and nothing in Modgud ever set it: the synthesized CIMD client was hard-wiredweb, DCR ignoredapplication_typeand created withnull, admin-created clients had no way to say it. The field existed in the domain and the store served it; it was just nevernative.Fix — one rule, in the one place OpenIddict asks
MartenApplicationStore.GetApplicationTypeAsyncreturns the effective type (OAuthApplicationTypes.Effective): a client with a loopbackhttpredirect URI isnativewhatever it declared — only a native app can have such a URI in the first place (OIDC DCR §2) — otherwise the declared type stands.nativeat synthesizeapplication_type/ URIsRolling-safe (no stored value changes). Nothing is loosened: OpenIddict relaxes the port only between loopback URIs, only when the registered one carries none; https URIs still match exactly — the tests assert another path and a non-loopback host stay refused.
Around it: the CIMD parser reads an optional
application_type(web|native, anything else invalid) andSynthesizeuses the effective type instead ofweb; DCR readsapplication_type, rejects anything but the two literal values asinvalid_client_metadata, records the declaration and echoes the effective value;CreateOAuthClientDto.ApplicationTypecarries it through. The registration validators are unchanged — they already knew the rule the runtime check did not.Test plan
CimdFullFlowTests.Loopback_redirect_with_an_ephemeral_port_is_accepted_for_a_cimd_client— the exact Claude Code document; random ports on both loopback hosts and the port-less form pass, another path / a non-loopback host are refused, and the code exchange completes with the ported URI. Red with the store and CIMD resolver reverted, green with the fix.DcrFullFlowTests.Loopback_redirect_with_an_ephemeral_port_is_accepted_for_a_dcr_client—nativeechoed, ephemeral port accepted, bogusapplication_type→invalid_client_metadata, an https-only client gets nothing inventedOAuthApplicationTypes.Effective/IsKnown, CIMD parser, DCR validator🤖 Generated with Claude Code