From 8ba4d485494b26b28d99bacea8c434be9081d77d Mon Sep 17 00:00:00 2001 From: timmy Date: Sat, 8 Aug 2026 15:18:30 +0000 Subject: [PATCH] feat: preserve mobile shares through sign-in (#313) --- frontend/login.js | 24 +++++++- frontend/service-worker.js | 6 +- src/main.py | 25 +++++++- tests/test_dashboard_auth.py | 38 ++++++++++++ tests/test_login_frontend.py | 116 +++++++++++++++++++++++++++++++++++ tests/test_service_worker.py | 26 ++++++-- 6 files changed, 226 insertions(+), 9 deletions(-) diff --git a/frontend/login.js b/frontend/login.js index 0c5c70b..2cd600a 100644 --- a/frontend/login.js +++ b/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'); diff --git a/frontend/service-worker.js b/frontend/service-worker.js index efbfb68..d41a703 100644 --- a/frontend/service-worker.js +++ b/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); diff --git a/src/main.py b/src/main.py index 2225af0..d89bc1a 100644 --- a/src/main.py +++ b/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", "") diff --git a/tests/test_dashboard_auth.py b/tests/test_dashboard_auth.py index 8cccc72..aca4497 100644 --- a/tests/test_dashboard_auth.py +++ b/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) diff --git a/tests/test_login_frontend.py b/tests/test_login_frontend.py index 113aca8..dc81d91 100644 --- a/tests/test_login_frontend.py +++ b/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() diff --git a/tests/test_service_worker.py b/tests/test_service_worker.py index d6bc7ef..0b49ce8 100644 --- a/tests/test_service_worker.py +++ b/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( """