Merge pull request 'Preserve shared screenshots through operator sign-in' (#742) from timmy/741-preserve-shared-screenshot-signin into main

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;
   }
 

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)}"

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)

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()