diff --git a/backend/app/services/push_service.py b/backend/app/services/push_service.py index a267835..6ef7885 100644 --- a/backend/app/services/push_service.py +++ b/backend/app/services/push_service.py @@ -2,6 +2,7 @@ import asyncio import json import logging import uuid +from urllib.parse import urlparse from pywebpush import WebPushException, webpush from sqlalchemy import delete, select @@ -53,7 +54,23 @@ async def unsubscribe(db: AsyncSession, user_id: uuid.UUID, endpoint: str) -> No await db.commit() +# #56 correction: bumping to pywebpush's latest release (2.4.0) turned out +# not to actually fix WNS -- checked the installed package's own source +# directly and it has no WNS-specific code anywhere; the upstream +# discussion (web-push-libs/pywebpush#162) apparently never shipped. +# Worked around here instead, using the `headers` param webpush() already +# exposes for exactly this: WNS (Windows/Edge push, +# *.notify.windows.com) has required this header since April 2024, or it +# 400s with no useful body -- "cache" for a non-zero TTL, "no-cache" for +# zero (this app never sets a TTL, so always the latter). +def _is_wns_endpoint(endpoint: str) -> bool: + return urlparse(endpoint).hostname is not None and urlparse(endpoint).hostname.endswith( + "notify.windows.com" + ) + + def _send_one(subscription: PushSubscription, payload: dict) -> None: + extra_headers = {"X-WNS-Cache-Policy": "no-cache"} if _is_wns_endpoint(subscription.endpoint) else None webpush( subscription_info={ "endpoint": subscription.endpoint, @@ -62,6 +79,7 @@ def _send_one(subscription: PushSubscription, payload: dict) -> None: data=json.dumps(payload), vapid_private_key=settings.vapid_private_key, vapid_claims={"sub": settings.vapid_subject}, + headers=extra_headers, ) diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 92b34cb..d03723d 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -15,13 +15,13 @@ dependencies = [ "pydantic-settings>=2.6", "argon2-cffi>=23.1", "itsdangerous>=2.2", - # #56: >=2.0 let the production venv sit on an old 2.0.x that predates - # the fix for web-push-libs/pywebpush#162 -- WNS (Windows/Edge push) - # has required an X-WNS-Cache-Policy header since April 2024, and an - # old pywebpush that never sends it gets a bodyless 400 on every send. - # Floored at the actual latest release (merged/shipped 2026-01 through - # 2026-08) rather than pinning to whichever exact point release first - # included the fix, since that wasn't independently confirmed. + # #56: >=2.0 let the production venv sit on an old 2.0.x with no real + # downside to bumping the floor -- worth keeping current regardless. + # Doesn't by itself fix WNS (Windows/Edge push): despite + # web-push-libs/pywebpush#162's discussion, even the latest release + # (2.4.0) has no WNS-specific header handling in its own source. The + # actual fix is app/services/push_service.py adding the required + # X-WNS-Cache-Policy header itself via webpush()'s `headers` param. "pywebpush>=2.4.0", "redis>=5.0", "httpx>=0.27", diff --git a/backend/tests/test_push.py b/backend/tests/test_push.py index 5ea5232..6003870 100644 --- a/backend/tests/test_push.py +++ b/backend/tests/test_push.py @@ -272,3 +272,61 @@ def test_non_gone_push_failure_logs_response_detail_and_keeps_subscription(ws_cl assert len(calls) == 1 assert "Bad Request" in calls[0] assert "Ttl value conflicts with X-WNS-Cache-Policy" in calls[0] + + +def test_wns_endpoint_gets_cache_policy_header(ws_client, monkeypatch): + calls = [] + monkeypatch.setattr("app.services.push_service.webpush", lambda **kw: calls.append(kw)) + + alice = _register_ws(ws_client, _unique("alice")) + room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json() + + bob = _register_ws(ws_client, _unique("bob")) + ws_client.post(f"/api/rooms/{room['id']}/join") + ws_client.post( + "/api/push/subscribe", + json={ + "endpoint": f"https://wns2-by3p.notify.windows.com/w/{_unique('bob')}", + "keys": {"p256dh": "p256dh-bob", "auth": "auth-bob"}, + }, + ) + + ws_client.post( + "/api/auth/login", json={"username_or_email": alice["username"], "password": "password123"} + ) + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + ws.send_json({"type": "message", "room_id": room["id"], "content": "hello"}) + assert ws.receive_json()["type"] == "message" + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + + assert len(calls) == 1 + assert calls[0]["headers"] == {"X-WNS-Cache-Policy": "no-cache"} + + +def test_non_wns_endpoint_gets_no_extra_headers(ws_client, monkeypatch): + calls = [] + monkeypatch.setattr("app.services.push_service.webpush", lambda **kw: calls.append(kw)) + + alice = _register_ws(ws_client, _unique("alice")) + room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json() + + bob = _register_ws(ws_client, _unique("bob")) + ws_client.post(f"/api/rooms/{room['id']}/join") + ws_client.post("/api/push/subscribe", json=_subscription_payload(_unique("bob"))) + + ws_client.post( + "/api/auth/login", json={"username_or_email": alice["username"], "password": "password123"} + ) + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + ws.send_json({"type": "message", "room_id": room["id"], "content": "hello"}) + assert ws.receive_json()["type"] == "message" + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + + assert len(calls) == 1 + assert calls[0]["headers"] is None diff --git a/frontend/src/lib/push.ts b/frontend/src/lib/push.ts index cab55b7..685cf41 100644 --- a/frontend/src/lib/push.ts +++ b/frontend/src/lib/push.ts @@ -1,5 +1,33 @@ import { getVapidPublicKey, subscribePush, unsubscribePush } from '../api/push' +// A stuck permission prompt or service-worker-ready wait would otherwise +// leave TopBar's "Enable notifications" toggle permanently disabled with +// no feedback at all -- confirmed live on a fresh Windows/Edge install +// (never been used before): clicking it greyed the button out and never +// showed the OS permission prompt, with no error and no way out short of +// reloading. Most likely cause is Windows' own per-app notification +// permission being off for Edge (Settings > System > Notifications), which +// some Chromium versions handle by just never resolving the request +// instead of rejecting it -- but whatever the cause, the UI should never +// hang forever waiting on a browser API that may simply never settle. +class PushTimeoutError extends Error {} + +function withTimeout(promise: Promise, ms: number, message: string): Promise { + return new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new PushTimeoutError(message)), ms) + promise.then( + (value) => { + clearTimeout(timer) + resolve(value) + }, + (err: unknown) => { + clearTimeout(timer) + reject(err) + }, + ) + }) +} + function urlBase64ToUint8Array(base64String: string): Uint8Array { const padding = '='.repeat((4 - (base64String.length % 4)) % 4) const base64 = (base64String + padding).replace(/-/g, '+').replace(/_/g, '/') @@ -27,7 +55,11 @@ export async function subscribeToPush(): Promise { throw new Error('Push notifications are not supported in this browser') } - const permission = await Notification.requestPermission() + const permission = await withTimeout( + Notification.requestPermission(), + 20_000, + "The browser never responded to the notification permission request. Check this browser's notification permission for this site, and your OS-level notification settings for the browser, then try again.", + ) if (permission !== 'granted') { throw new Error('Notification permission was not granted') } @@ -37,7 +69,11 @@ export async function subscribeToPush(): Promise { throw new Error('Push notifications are not configured on the server') } - const registration = await navigator.serviceWorker.ready + const registration = await withTimeout( + navigator.serviceWorker.ready, + 20_000, + 'The browser never finished setting up its background service worker. Try reloading the page.', + ) const subscription = await registration.pushManager.subscribe({ userVisibleOnly: true, applicationServerKey: urlBase64ToUint8Array(public_key),