Merge pull request 'Preserve mobile share capture through sign-in' (#314) from timmy/313-preserve-mobile-share-sign-in into main

frontend/login.js

@@ -2,15 +2,33 @@
   if (typeof module === 'object' && module.exports) module.exports = factory;
   else root.createLoginController = factory;
 }(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 params = new URLSearchParams(value.slice(3));
+    const entries = Array.from(params.entries());
+    if (!entries.length) return './';
+    const invalid = entries.some(([name, content]) => (
+      !Object.prototype.hasOwnProperty.call(limits, name)
+      || !content
+      || content.length > limits[name]
+    ));
+    if (invalid) return './';
+    return value;
+  }
+
   const form = options.form;
   const status = options.status;
   const button = options.button;
   const fetchImpl = options.fetchImpl;
   const location = options.location;
+  const continuation = validShareContinuation(options.continuation);
   const setIntervalImpl = options.setIntervalImpl || setInterval;
   const clearIntervalImpl = options.clearIntervalImpl || clearInterval;
   let timer = null;
 
+  if (continuation !== './') status.textContent = 'Sign in to continue your shared capture.';
+
   function showReason(reason) {
     if (reason !== 'session-expired') return;
     status.textContent = 'Your session expired. Private drafts remain on this device. Sign in to continue.';
@@ -49,7 +67,7 @@
     }
     form.reset();
     if (response.ok) {
-      location.replace('./');
+      location.replace(continuation);
       return;
     }
     if (response.status === 429) {
@@ -66,14 +84,16 @@ if (typeof document !== 'undefined') {
   const form = document.getElementById('sign-in');
   const status = document.getElementById('status');
   const button = document.getElementById('submit-sign-in');
+  const loginParams = new URLSearchParams(window.location.search);
   const controller = createLoginController({
     form,
     status,
     button,
     fetchImpl: fetch.bind(window),
     location: window.location,
+    continuation: loginParams.get('continue'),
   });
-  controller.showReason(new URLSearchParams(window.location.search).get('reason'));
+  controller.showReason(loginParams.get('reason'));
   form.addEventListener('submit', event => {
     event.preventDefault();
     const accessToken = new FormData(form).get('access_token');

frontend/service-worker.js

@@ -1,6 +1,6 @@
 const BASE = new URL('./', self.location.href).pathname;
 importScripts(BASE + 'static/background-issue-sync.js');
-const CACHE = 'stackchain-dashboard-shell-v31';
+const CACHE = 'stackchain-dashboard-shell-v32';
 const OUTAGE_STATUSES = new Set([500, 502, 503, 504]);
 const SHELL = [
   BASE,
@@ -163,7 +163,9 @@ self.addEventListener('fetch', event => {
     event.respondWith(
       fetch(request).then(async response => {
         const cache = await caches.open(CACHE);
-        if (response.ok) {
+        const responseUrl = new URL(response.url || request.url);
+        const isDashboardShell = responseUrl.origin === self.location.origin && responseUrl.pathname === BASE;
+        if (response.ok && !response.redirected && isDashboardShell) {
           await cache.put(BASE, response.clone());
         } else if (OUTAGE_STATUSES.has(response.status)) {
           const cached = await cache.match(BASE);

src/main.py

@@ -8,6 +8,7 @@ from contextlib import asynccontextmanager
 from datetime import datetime
 from pathlib import Path
 from typing import Any, Literal
+from urllib.parse import urlencode
 
 from fastapi import FastAPI, Header, HTTPException, Path as PathParam, Query, Request, Response
 from fastapi.responses import JSONResponse, RedirectResponse
@@ -452,6 +453,24 @@ app.mount("/static", StaticFiles(directory=FRONTEND_DIR), name="static")
 app.include_router(frontend_router)
 
 
+def _share_target_login_redirect(request: Request) -> str:
+    limits = {"title": 200, "text": 8000, "url": 2048}
+    if any(
+        len(request.query_params.get(name, "")) > limit
+        for name, limit in limits.items()
+    ):
+        return "login"
+    shared = [
+        (name, request.query_params[name])
+        for name in limits
+        if request.query_params.get(name)
+    ]
+    if dashboard_auth.application_path(request) != "/" or not shared:
+        return "login"
+    continuation = f"./?{urlencode(shared)}"
+    return f"login?{urlencode({'continue': continuation})}"
+
+
 @app.middleware("http")
 async def require_operator_session(request: Request, call_next):
     path = dashboard_auth.application_path(request)
@@ -496,7 +515,11 @@ async def require_operator_session(request: Request, call_next):
                 status_code=401,
                 headers={"Cache-Control": "no-store"},
             )
-        return RedirectResponse("login", status_code=303, headers={"Cache-Control": "no-store"})
+        return RedirectResponse(
+            _share_target_login_redirect(request),
+            status_code=303,
+            headers={"Cache-Control": "no-store"},
+        )
 
     if not public and request.method not in {"GET", "HEAD", "OPTIONS"}:
         supplied_csrf = request.headers.get("x-csrf-token", "")

tests/test_dashboard_auth.py

@@ -1,5 +1,6 @@
 import asyncio
 import time
+from urllib.parse import parse_qs, urlsplit
 
 import httpx
 import pytest
@@ -100,6 +101,43 @@ async def test_anonymous_private_request_is_rejected_before_gitea(access_control
     assert called is False
 
 
+@pytest.mark.anyio
+async def test_anonymous_share_target_redirect_preserves_only_bounded_capture_fields(access_control):
+    transport = httpx.ASGITransport(app=main.app)
+    async with httpx.AsyncClient(transport=transport, base_url="https://test") as client:
+        response = await client.get(
+            "/",
+            params={
+                "title": "Production crash",
+                "text": "Steps from the mobile app",
+                "url": "https://example.com/incidents/42",
+                "next": "https://evil.example/steal",
+            },
+        )
+
+    assert response.status_code == 303
+    login_query = parse_qs(urlsplit(response.headers["location"]).query)
+    assert login_query == {
+        "continue": [
+            "./?title=Production+crash&text=Steps+from+the+mobile+app&url="
+            "https%3A%2F%2Fexample.com%2Fincidents%2F42"
+        ]
+    }
+    assert response.headers["cache-control"] == "no-store"
+
+
+@pytest.mark.anyio
+async def test_oversized_share_target_is_not_carried_through_login(access_control):
+    transport = httpx.ASGITransport(app=main.app)
+    async with httpx.AsyncClient(transport=transport, base_url="https://test") as client:
+        response = await client.get(
+            "/", params={"title": "x" * 201, "text": "keep me"}
+        )
+
+    assert response.status_code == 303
+    assert response.headers["location"] == "login"
+
+
 @pytest.mark.anyio
 async def test_sign_in_creates_secure_session_without_echoing_access_token(access_control):
     transport = httpx.ASGITransport(app=main.app)

tests/test_login_frontend.py

@@ -78,6 +78,122 @@ const controller = createLoginController({{
     }
 
 
+def test_successful_login_resumes_valid_shared_capture_continuation():
+    harness = f"""
+const createLoginController = require({json.dumps(str(LOGIN_JS))});
+const state = {{ replaced: null }};
+const controller = createLoginController({{
+  form: {{ reset: () => {{}} }}, status: {{ textContent: '' }}, button: {{ disabled: false }},
+  fetchImpl: async () => new Response('{{}}', {{ status: 200 }}),
+  location: {{ replace: value => state.replaced = value }},
+  continuation: './?title=Production+crash&url=https%3A%2F%2Fexample.com',
+}});
+(async () => {{
+  await controller.submit('operator-token');
+  process.stdout.write(JSON.stringify(state));
+}})().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)["replaced"] == (
+        "./?title=Production+crash&url=https%3A%2F%2Fexample.com"
+    )
+
+
+def test_shared_capture_login_explains_why_sign_in_is_required():
+    harness = f"""
+const createLoginController = require({json.dumps(str(LOGIN_JS))});
+const status = {{ textContent: '' }};
+createLoginController({{
+  form: {{ reset: () => {{}} }}, status, button: {{ disabled: false }},
+  fetchImpl: async () => new Response('{{}}', {{ status: 200 }}),
+  location: {{ replace: () => {{}} }}, continuation: './?title=Shared',
+}});
+process.stdout.write(status.textContent);
+"""
+    result = subprocess.run(
+        ["node", "-e", harness], text=True, capture_output=True, check=True
+    )
+
+    assert result.stdout == "Sign in to continue your shared capture."
+
+
+def test_login_rejects_untrusted_or_non_share_continuations():
+    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({{
+    crossOrigin: await destination('https://evil.example/steal'),
+    otherRoute: await destination('./settings?title=Shared'),
+    extraField: await destination('./?title=Shared&next=https%3A%2F%2Fevil.example'),
+    prototypeField: await destination('./?toString=Shared'),
+    oversized: await destination('./?title=' + 'x'.repeat(201)),
+  }}));
+}})().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) == {
+        "crossOrigin": "./",
+        "otherRoute": "./",
+        "extraField": "./",
+        "prototypeField": "./",
+        "oversized": "./",
+    }
+
+
+def test_login_bootstrap_continues_shared_capture_after_submit():
+    harness = f"""
+const fs = require('fs');
+const vm = require('vm');
+const state = {{ replaced: null, submit: null }};
+const elements = {{
+  'sign-in': {{ reset: () => {{}}, addEventListener: (_name, handler) => state.submit = handler }},
+  'status': {{ textContent: '' }},
+  'submit-sign-in': {{ disabled: false }},
+}};
+const context = {{
+  URLSearchParams, Response, setInterval, clearInterval,
+  FormData: function () {{ return {{ get: () => 'operator-token' }}; }},
+  fetch: async () => new Response('{{}}', {{ status: 200 }}),
+  document: {{ getElementById: id => elements[id] }},
+  window: {{ location: {{
+    search: '?continue=.%2F%3Ftitle%3DShared%2Blink%26url%3Dhttps%253A%252F%252Fexample.com',
+    replace: value => state.replaced = value,
+  }} }},
+}};
+context.fetch.bind = Function.prototype.bind;
+vm.createContext(context);
+vm.runInContext(fs.readFileSync({json.dumps(str(LOGIN_JS))}, 'utf8'), context);
+state.submit({{ preventDefault: () => {{}} }});
+setTimeout(() => process.stdout.write(JSON.stringify({{
+  replaced: state.replaced, status: elements.status.textContent,
+}})), 0);
+"""
+    result = subprocess.run(
+        ["node", "-e", harness], text=True, capture_output=True, check=True
+    )
+
+    assert json.loads(result.stdout) == {
+        "replaced": "./?title=Shared+link&url=https%3A%2F%2Fexample.com",
+        "status": "Signing in…",
+    }
+
+
 @pytest.mark.anyio
 async def test_login_page_loads_rate_limit_controller():
     html = await login()

tests/test_service_worker.py

@@ -13,7 +13,7 @@ def run_worker_scenario(scenario: str) -> dict:
 const fs = require('fs');
 const vm = require('vm');
 const listeners = {{}};
-const state = {{ added: [], deleted: [], claimed: false, skipped: false, fetches: [], puts: [], backgroundFlushes: 0, outboxPurges: 0, notifications: [], focused: [], opened: [], failFetch: false, fetchStatus: 200, cachedBody: null }};
+const state = {{ added: [], deleted: [], claimed: false, skipped: false, fetches: [], puts: [], backgroundFlushes: 0, outboxPurges: 0, notifications: [], focused: [], opened: [], failFetch: false, fetchStatus: 200, fetchRedirected: false, cachedBody: null }};
 const cache = {{
   addAll: async urls => {{ state.added = urls; }},
   match: async request => state.cachedBody === null ? null : new Response(state.cachedBody),
@@ -48,7 +48,9 @@ const context = {{
   fetch: async request => {{
     state.fetches.push(String(request.url || request));
     if (state.failFetch) throw new Error('offline');
-    return new Response('network', {{ status: state.fetchStatus }});
+    const response = new Response('network', {{ status: state.fetchStatus }});
+    Object.defineProperty(response, 'redirected', {{ value: state.fetchRedirected }});
+    return response;
   }},
 }};
 vm.createContext(context);
@@ -92,10 +94,10 @@ async function dispatchNotificationClick(route) {{
     return json.loads(completed.stdout)
 
 
-def test_session_expiry_recovery_ships_in_a_new_shell_cache():
+def test_share_target_sign_in_fix_ships_in_a_new_shell_cache():
     source = WORKER.read_text()
 
-    assert "stackchain-dashboard-shell-v31" in source
+    assert "stackchain-dashboard-shell-v32" in source
     assert "BASE + 'static/dashboard.css'" in source
     assert "BASE + 'static/dashboard.js'" in source
     assert "BASE + 'static/install-app.js'" in source
@@ -301,6 +303,22 @@ def test_offline_navigation_returns_cached_shell_for_share_target_url():
     ]
 
 
+def test_redirected_login_navigation_does_not_replace_cached_dashboard_shell():
+    result = run_worker_scenario(
+        """
+  state.fetchRedirected = true;
+  const response = await dispatch('fetch', {
+    method: 'GET', mode: 'navigate',
+    url: 'https://forge.example/dashboard/?title=Shared',
+  });
+  process.stdout.write(JSON.stringify({ status: response.status, state }));
+"""
+    )
+
+    assert result["status"] == 200
+    assert result["state"]["puts"] == []
+
+
 def test_cross_origin_navigation_is_not_intercepted_or_cached():
     result = run_worker_scenario(
         """