diff --git a/frontend/login.js b/frontend/login.js index 3b77af4..bcf0149 100644 --- a/frontend/login.js +++ b/frontend/login.js @@ -4,7 +4,7 @@ }(typeof self !== 'undefined' ? self : this, function createLoginController(options) { function validShareContinuation(value) { if (typeof value !== 'string' || !value.startsWith('./?') || value.includes('#')) return './'; - const limits = { title: 200, text: 8000, url: 2048 }; + const limits = { title: 200, text: 8000, url: 2048, launch: 8, shared: 5 }; const params = new URLSearchParams(value.slice(3)); const entries = Array.from(params.entries()); if (!entries.length) return './'; @@ -14,6 +14,11 @@ || content.length > limits[name] )); if (invalid) return './'; + if (Object.keys(limits).some(name => params.getAll(name).length > 1)) return './'; + const launch = params.get('launch'); + const shared = params.get('shared'); + if (launch && !['continue', 'new', 'agenda'].includes(launch)) return './'; + if (shared !== null && (shared !== 'image' || launch !== 'new')) return './'; return value; } diff --git a/src/main.py b/src/main.py index 7982a48..6cdca0e 100644 --- a/src/main.py +++ b/src/main.py @@ -1015,6 +1015,14 @@ def _share_target_login_redirect(request: Request) -> str: launch = request.query_params.get("launch", "") if launch in {"continue", "new", "agenda"}: shared.append(("launch", launch)) + image_markers = request.query_params.getlist("shared") + if image_markers: + if ( + image_markers != ["image"] + or request.query_params.getlist("launch") != ["new"] + ): + return "login" + shared.append(("shared", "image")) if dashboard_auth.application_path(request) != "/" or not shared: return "login" continuation = f"./?{urlencode(shared)}" diff --git a/tests/test_dashboard_auth.py b/tests/test_dashboard_auth.py index e218d5e..20a24c1 100644 --- a/tests/test_dashboard_auth.py +++ b/tests/test_dashboard_auth.py @@ -891,6 +891,40 @@ async def test_anonymous_app_shortcut_preserves_only_a_known_launch_action(acces assert invalid.headers["location"] == "login" +@pytest.mark.anyio +async def test_anonymous_shared_screenshot_preserves_bounded_sign_in_continuation(access_control): + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="https://test") as client: + valid = await client.get( + "/", + params={ + "launch": "new", + "shared": "image", + "title": "Broken checkout", + "text": "Steps to reproduce", + }, + ) + wrong_launch = await client.get( + "/", params={"launch": "agenda", "shared": "image"} + ) + wrong_marker = await client.get( + "/", params={"launch": "new", "shared": "document"} + ) + duplicate_marker = await client.get( + "/?launch=new&shared=image&shared=image" + ) + + assert valid.status_code == 303 + assert parse_qs(urlsplit(valid.headers["location"]).query) == { + "continue": [ + "./?title=Broken+checkout&text=Steps+to+reproduce&launch=new&shared=image" + ] + } + assert wrong_launch.headers["location"] == "login" + assert wrong_marker.headers["location"] == "login" + assert duplicate_marker.headers["location"] == "login" + + @pytest.mark.anyio async def test_oversized_share_target_is_not_carried_through_login(access_control): transport = httpx.ASGITransport(app=main.app) diff --git a/tests/test_login_frontend.py b/tests/test_login_frontend.py index 3a38cb1..f74e207 100644 --- a/tests/test_login_frontend.py +++ b/tests/test_login_frontend.py @@ -370,6 +370,44 @@ setTimeout(() => process.stdout.write(JSON.stringify({{ } +def test_login_continues_only_bounded_shared_screenshot_capture(): + harness = f""" +const createLoginController = require({json.dumps(str(LOGIN_JS))}); +async function destination(continuation) {{ + let replaced = null; + const controller = createLoginController({{ + form: {{ reset: () => {{}} }}, status: {{ textContent: '' }}, button: {{ disabled: false }}, + fetchImpl: async () => new Response('{{}}', {{ status: 200 }}), + location: {{ replace: value => replaced = value }}, continuation, + }}); + await controller.submit('operator-token'); + return replaced; +}} +(async () => {{ + process.stdout.write(JSON.stringify({{ + valid: await destination('./?title=Broken&launch=new&shared=image'), + wrongLaunch: await destination('./?launch=agenda&shared=image'), + missingLaunch: await destination('./?shared=image'), + wrongMarker: await destination('./?launch=new&shared=document'), + duplicateMarker: await destination('./?launch=new&shared=image&shared=image'), + launchWithoutMarker: await destination('./?launch=new'), + }})); +}})().catch(error => {{ console.error(error); process.exit(1); }}); +""" + result = subprocess.run( + ["node", "-e", harness], text=True, capture_output=True, check=True + ) + + assert json.loads(result.stdout) == { + "valid": "./?title=Broken&launch=new&shared=image", + "wrongLaunch": "./", + "missingLaunch": "./", + "wrongMarker": "./", + "duplicateMarker": "./", + "launchWithoutMarker": "./?launch=new", + } + + @pytest.mark.anyio async def test_login_page_loads_rate_limit_controller(): html = await login()