diff --git a/enlace_auth/auth/oauth_server.py b/enlace_auth/auth/oauth_server.py index 41039ac..c1192aa 100644 --- a/enlace_auth/auth/oauth_server.py +++ b/enlace_auth/auth/oauth_server.py @@ -280,7 +280,11 @@ def _validate_authorize( """Parse + validate /authorize params. Returns (ok, error_page).""" client_id = params.get("client_id", "") redirect_uri = params.get("redirect_uri", "") - client = client_store.get(client_id) + # Guard the empty key explicitly: a file-backed client_store resolves an + # empty client_id to its root directory and raises IsADirectoryError + # (a 500) instead of returning None — so a missing/blank client_id must + # short-circuit to the clean "unknown client" page below. + client = client_store.get(client_id) if client_id else None if not client or redirect_uri not in client.get("redirect_uris", []): # Cannot safely redirect to an unverified URI — show an error page. return None, HTMLResponse( diff --git a/enlace_auth/auth/pages.py b/enlace_auth/auth/pages.py index b8ae754..ab8d171 100644 --- a/enlace_auth/auth/pages.py +++ b/enlace_auth/auth/pages.py @@ -14,9 +14,33 @@ from __future__ import annotations +import json from html import escape from typing import Optional + +def _js_string(value: str) -> str: + """Encode a Python string as a JS string literal safe to embed inline in a + ```` breakout) while the JS engine + decodes them back to the original characters at runtime. Returns the literal + *including* its surrounding quotes. + """ + return ( + json.dumps(value) + .replace("&", "\\u0026") + .replace("<", "\\u003c") + .replace(">", "\\u003e") + .replace("\u2028", "\\u2028") + .replace("\u2029", "\\u2029") + ) + # -------------------------------------------------------------------------- # Shared chrome # -------------------------------------------------------------------------- @@ -191,7 +215,7 @@ def render_login_page( error: optional error banner (e.g. a stale-session note). show_register_hint: whether to show the "ask an admin" footnote. """ - next_js = escape(next_url, quote=True) + next_js = _js_string(next_url) hint = ( '

No account? Accounts are created by the platform admin.

' if show_register_hint @@ -218,7 +242,7 @@ def render_login_page( &c=1") + assert "" not in lit and "<" not in lit and ">" not in lit and "&" not in lit + # ...but the runtime value is exactly the original, untouched. + assert _js_runtime_value(lit) == "/x?a=&c=1" + + +def test_login_page_does_not_html_escape_next_into_js(): + page = render_login_page(next_url=AUTHZ) + # The bug's fingerprint: an HTML entity where a raw ampersand must be. + assert "&client_id" not in page + # The fix's fingerprint: query separators carried as JS unicode escapes. + assert "\\u0026client_id" in page + + +def test_shared_login_page_does_not_html_escape_next_into_js(): + page = render_shared_login_page(app="demo", next_url=AUTHZ) + assert "&client_id" not in page + assert "\\u0026client_id" in page