Fix OAuth authorize 500: encode login next URL as JS, not HTML - #17
Merged
Conversation
The sign-in page threads the post-login destination back through inline JS (`const NEXT = "..."`), but built that literal with `html.escape()`. Inside a `<script>` element the browser does not decode HTML entities, so every `&` in the destination URL became a literal `&`. For a multi-parameter `next` (the OAuth `/authorize` URL), the redirect after login parsed `?response_type=code&client_id=X&redirect_uri=Y` as params `response_type`, `amp;client_id`, `amp;redirect_uri` — dropping the real names. `client_id` thus arrived empty at `/authorize`, and the file-backed client store resolved the empty key to its root directory (IsADirectoryError) → HTTP 500. A plain-path `next` (no `&`) hid the bug; the MCP OAuth flow is the first multi-param `next`. - pages.py: new `_js_string()` encodes `next_url` as a JS string literal (JSON + &/</> so it's inert to the HTML tokenizer yet decodes back at runtime). Applied to both the standard and shared login pages. - oauth_server.py: a blank/missing client_id now short-circuits to the clean 400 "unknown client" page instead of hitting the store and 500ing. - tests: new test_pages.py (JS-encoding + no-breakout + no HTML-escape) and a blank-client_id 400 regression in test_oauth_server.py. Suite 195 -> 200. Claude-Session: https://claude.ai/code/session_01Gw5RPgrQhC88Hc3DyACYWF
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.
Symptom
Completing the OAuth login for a remote MCP connector (Claude.ai) returned HTTP 500 at
/auth/oauth/authorizeright after sign-in — the browser saved the plain-textInternal Server Errorbody as anauthorize.txtdownload.Root cause
The sign-in page threads the post-login destination back through inline JS as
const NEXT = "...", but built that literal withhtml.escape(). Inside a<script>element the browser does not decode HTML entities, so every&in the destination became a literal&. For a multi-parameternext(the OAuth/authorizeURL), the post-login redirect parsed?response_type=code&client_id=X&redirect_uri=Yas paramsresponse_type,amp;client_id,amp;redirect_uri— soclient_idarrived empty. The file-backed client store then resolved the empty key to its own root directory (IsADirectoryError) → 500.A plain-path
next(no&) hid this for normal app logins; the MCP OAuth flow is the first multi-paramnextto exercise it.Fix
pages.py: new_js_string()encodesnext_urlas a proper JS string literal (JSON-encoded, with&/</>as\uXXXXso it's inert in the HTML tokenizer but decodes back at runtime). Applied to both the standard and shared login pages.oauth_server.py: defense-in-depth — a blank/missingclient_idshort-circuits to the clean 400 "unknown client" page instead of hitting the store and 500ing.test_pages.py(JS-encoding preserves query params, no</script>breakout, no HTML-escaping into JS) + a blank-client_id400 regression intest_oauth_server.py. Suite 195 → 200 passing.https://claude.ai/code/session_01Gw5RPgrQhC88Hc3DyACYWF