diff --git a/backend/.env.example b/backend/.env.example index 66ac7d8..08e2c46 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -1,3 +1,9 @@ DATABASE_URL=postgresql+asyncpg://chatapp:chatapp@localhost:5432/chatapp SESSION_SECRET=change-me-to-a-long-random-string SESSION_HTTPS_ONLY=false + +# Optional: push notifications are skipped if unset. Generate with: +# python -m app.cli generate-vapid-keys +VAPID_PUBLIC_KEY= +VAPID_PRIVATE_KEY= +VAPID_SUBJECT=mailto:you@example.com diff --git a/backend/README.md b/backend/README.md index c21d379..bb8ccc2 100644 --- a/backend/README.md +++ b/backend/README.md @@ -1,9 +1,10 @@ -# KeepItTalking backend (Phase 1 + 2) +# KeepItTalking backend (Phase 1 + 2 + 4) FastAPI + SQLAlchemy 2.0 (async) + PostgreSQL. Implements auth, room CRUD -(open and private), room roles (owner/admin/member) and invites, and a -single-instance WebSocket chat endpoint. See `../ARCHITECTURE.md` for the -full system design and the phased build plan. +(open and private), room roles (owner/admin/member) and invites, a +single-instance WebSocket chat endpoint, and Web Push notifications for +offline room members. See `../ARCHITECTURE.md` for the full system design +and the phased build plan. This is an **invite-only site**: there is no public registration endpoint. Accounts are created by an operator on the app server — see step 4 below. @@ -55,7 +56,17 @@ There's no public sign-up. Create accounts directly with the CLI (add .venv/bin/python -m app.cli create-user alice alice@example.com "some-password" ``` -### 5. Run the dev server +### 5. (Optional) Set up push notifications + +Push works without any setup — `VAPID_PUBLIC_KEY`/`VAPID_PRIVATE_KEY` are +unset by default and push delivery is silently skipped. To enable it: + +```bash +.venv/bin/python -m app.cli generate-vapid-keys +# paste the three printed lines into backend/.env +``` + +### 6. Run the dev server ```bash .venv/bin/uvicorn app.main:app --reload @@ -63,7 +74,7 @@ There's no public sign-up. Create accounts directly with the CLI (add API docs: http://localhost:8000/docs. WebSocket chat endpoint: `ws://localhost:8000/ws/chat`. -### 6. Run tests +### 7. Run tests Tests run against a real Postgres database (`chatapp_test` by default — native `ENUM`/`UUID` types aren't faithfully reproduced by SQLite), with each test @@ -82,17 +93,32 @@ app/ database.py async engine/session, get_db() dependency dependencies.py get_current_user, require_room_member, require_room_role security.py argon2 password hashing - cli.py `python -m app.cli create-user` (account provisioning) + cli.py `python -m app.cli create-user` / `generate-vapid-keys` models/ SQLAlchemy models (users, rooms, room_memberships, - messages, room_invites) + messages, room_invites, push_subscriptions) schemas/ Pydantic request/response models - routers/ auth, rooms, invites, health + routers/ auth, rooms, invites, push, health services/ business logic called by routers ws/ WebSocket connection manager + /ws/chat handler alembic/ migrations tests/ pytest + httpx/TestClient tests ``` +## Push notifications (Phase 4) + +`POST /api/push/subscribe` (upserts by `endpoint`) / `DELETE /api/push/subscribe` +manage a user's `push_subscriptions` rows; `GET /api/push/vapid-public-key` gives +the frontend the key it needs for `PushManager.subscribe()`. On every chat +message, `app/ws/chat.py` computes `room members - ConnectionManager. +connected_user_ids(room_id)` (who's actually connected to *that room* right +now, tracked alongside the existing WebSocket registry) and sends each +offline member a push via `pywebpush`, awaited inline against the same +request-scoped session rather than fired as a background task — the +broadcast to online members already happened by that point, so nothing +online-facing is delayed, and it sidesteps `asyncio.create_task()`s outliving +the session/event loop they were created on. An expired/invalid subscription +(pywebpush 404/410) is deleted automatically. + ## Room roles and invites (Phase 2) Rooms can be `open` (anyone can join via `POST /api/rooms/{id}/join`) or diff --git a/backend/alembic/versions/8a55c5254edb_push_subscriptions.py b/backend/alembic/versions/8a55c5254edb_push_subscriptions.py new file mode 100644 index 0000000..beda5c7 --- /dev/null +++ b/backend/alembic/versions/8a55c5254edb_push_subscriptions.py @@ -0,0 +1,45 @@ +"""push subscriptions + +Revision ID: 8a55c5254edb +Revises: 0699d20789b3 +Create Date: 2026-08-14 06:24:35.973594 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = '8a55c5254edb' +down_revision: Union[str, Sequence[str], None] = '0699d20789b3' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('push_subscriptions', + sa.Column('id', sa.Uuid(), nullable=False), + sa.Column('user_id', sa.Uuid(), nullable=False), + sa.Column('endpoint', sa.String(length=1024), nullable=False), + sa.Column('p256dh_key', sa.String(length=255), nullable=False), + sa.Column('auth_key', sa.String(length=255), nullable=False), + sa.Column('created_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False), + sa.ForeignKeyConstraint(['user_id'], ['users.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_push_subscriptions_endpoint'), 'push_subscriptions', ['endpoint'], unique=True) + op.create_index(op.f('ix_push_subscriptions_user_id'), 'push_subscriptions', ['user_id'], unique=False) + # ### end Alembic commands ### + + +def downgrade() -> None: + """Downgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f('ix_push_subscriptions_user_id'), table_name='push_subscriptions') + op.drop_index(op.f('ix_push_subscriptions_endpoint'), table_name='push_subscriptions') + op.drop_table('push_subscriptions') + # ### end Alembic commands ### diff --git a/backend/app/cli.py b/backend/app/cli.py index 11bd224..2a870fa 100644 --- a/backend/app/cli.py +++ b/backend/app/cli.py @@ -6,6 +6,7 @@ created by an operator running this script directly on the app server. import argparse import asyncio +import base64 from pydantic import ValidationError @@ -33,6 +34,34 @@ async def _create_user(username: str, email: str, password: str, is_admin: bool) print(f"Created user {username!r} (id={user.id}, admin={is_admin})") +def _generate_vapid_keys() -> None: + # py_vapid works in DER/PEM internally, but both pywebpush's + # vapid_private_key argument and the browser's PushManager + # applicationServerKey expect base64url-encoded *raw* key bytes -- the + # format used in every Web Push tutorial/example. Encode explicitly + # rather than relying on py_vapid's own (PEM-oriented) save helpers. + from py_vapid import Vapid02 + + vapid = Vapid02() + vapid.generate_keys() + + private_raw = vapid.private_key.private_numbers().private_value.to_bytes(32, "big") + private_b64 = base64.urlsafe_b64encode(private_raw).decode().rstrip("=") + + from cryptography.hazmat.primitives import serialization + + public_raw = vapid.public_key.public_bytes( + encoding=serialization.Encoding.X962, + format=serialization.PublicFormat.UncompressedPoint, + ) + public_b64 = base64.urlsafe_b64encode(public_raw).decode().rstrip("=") + + print("Add these to backend/.env:") + print(f"VAPID_PUBLIC_KEY={public_b64}") + print(f"VAPID_PRIVATE_KEY={private_b64}") + print("VAPID_SUBJECT=mailto:you@example.com") + + def main() -> None: parser = argparse.ArgumentParser(prog="python -m app.cli") subparsers = parser.add_subparsers(dest="command", required=True) @@ -43,10 +72,14 @@ def main() -> None: create_user.add_argument("password") create_user.add_argument("--admin", action="store_true", help="Grant is_site_admin") + subparsers.add_parser("generate-vapid-keys", help="Generate a VAPID key pair for push notifications") + args = parser.parse_args() if args.command == "create-user": asyncio.run(_create_user(args.username, args.email, args.password, args.admin)) + elif args.command == "generate-vapid-keys": + _generate_vapid_keys() if __name__ == "__main__": diff --git a/backend/app/config.py b/backend/app/config.py index 6abed64..9fd890b 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -9,5 +9,12 @@ class Settings(BaseSettings): session_https_only: bool = True session_max_age_seconds: int = 60 * 60 * 24 * 14 + # Optional: push notifications are skipped (logged, not an error) if + # unset, so existing deployments don't have to configure this to keep + # running. Generate a pair with `python -m app.cli generate-vapid-keys`. + vapid_public_key: str | None = None + vapid_private_key: str | None = None + vapid_subject: str = "mailto:admin@example.com" + settings = Settings() diff --git a/backend/app/main.py b/backend/app/main.py index 195b630..666f244 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -2,7 +2,7 @@ from fastapi import FastAPI from starlette.middleware.sessions import SessionMiddleware from app.config import settings -from app.routers import auth, health, invites, rooms +from app.routers import auth, health, invites, push, rooms from app.ws.chat import router as ws_router from app.ws.connection_manager import ConnectionManager @@ -24,6 +24,7 @@ def create_app() -> FastAPI: app.include_router(auth.router) app.include_router(rooms.router) app.include_router(invites.router) + app.include_router(push.router) app.include_router(ws_router) return app diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index cd1b937..475d351 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -2,6 +2,7 @@ from app.models.base import Base from app.models.invite import InviteStatus, RoomInvite from app.models.membership import RoomMembership, RoomRole from app.models.message import Message +from app.models.push_subscription import PushSubscription from app.models.room import Room from app.models.user import User @@ -14,4 +15,5 @@ __all__ = [ "Message", "RoomInvite", "InviteStatus", + "PushSubscription", ] diff --git a/backend/app/models/push_subscription.py b/backend/app/models/push_subscription.py new file mode 100644 index 0000000..345f484 --- /dev/null +++ b/backend/app/models/push_subscription.py @@ -0,0 +1,22 @@ +import uuid +from datetime import datetime + +from sqlalchemy import DateTime, ForeignKey, String, func +from sqlalchemy.orm import Mapped, mapped_column, relationship + +from app.models.base import Base + + +class PushSubscription(Base): + __tablename__ = "push_subscriptions" + + id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=uuid.uuid4) + user_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("users.id"), index=True, nullable=False) + endpoint: Mapped[str] = mapped_column(String(1024), unique=True, index=True, nullable=False) + p256dh_key: Mapped[str] = mapped_column(String(255), nullable=False) + auth_key: Mapped[str] = mapped_column(String(255), nullable=False) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now(), nullable=False + ) + + user = relationship("User") diff --git a/backend/app/routers/push.py b/backend/app/routers/push.py new file mode 100644 index 0000000..44c1acd --- /dev/null +++ b/backend/app/routers/push.py @@ -0,0 +1,40 @@ +from fastapi import APIRouter, Depends, Response +from sqlalchemy.ext.asyncio import AsyncSession + +from app.config import settings +from app.database import get_db +from app.dependencies import get_current_user +from app.models import User +from app.schemas.push import ( + PushSubscriptionCreate, + PushUnsubscribeRequest, + VapidPublicKeyRead, +) +from app.services.push_service import subscribe, unsubscribe + +router = APIRouter(prefix="/api/push", tags=["push"]) + + +@router.get("/vapid-public-key", response_model=VapidPublicKeyRead) +async def get_vapid_public_key(current_user: User = Depends(get_current_user)): + return VapidPublicKeyRead(public_key=settings.vapid_public_key) + + +@router.post("/subscribe", status_code=204) +async def subscribe_endpoint( + data: PushSubscriptionCreate, + current_user: User = Depends(get_current_user), + db: AsyncSession = Depends(get_db), +) -> Response: + await subscribe(db, current_user.id, data) + return Response(status_code=204) + + +@router.delete("/subscribe", status_code=204) +async def unsubscribe_endpoint( + data: PushUnsubscribeRequest, + current_user: User = Depends(get_current_user), + db: AsyncSession = Depends(get_db), +) -> Response: + await unsubscribe(db, current_user.id, data.endpoint) + return Response(status_code=204) diff --git a/backend/app/schemas/push.py b/backend/app/schemas/push.py new file mode 100644 index 0000000..cf2ea02 --- /dev/null +++ b/backend/app/schemas/push.py @@ -0,0 +1,19 @@ +from pydantic import BaseModel + + +class PushSubscriptionKeys(BaseModel): + p256dh: str + auth: str + + +class PushSubscriptionCreate(BaseModel): + endpoint: str + keys: PushSubscriptionKeys + + +class PushUnsubscribeRequest(BaseModel): + endpoint: str + + +class VapidPublicKeyRead(BaseModel): + public_key: str | None diff --git a/backend/app/services/push_service.py b/backend/app/services/push_service.py new file mode 100644 index 0000000..9f27aee --- /dev/null +++ b/backend/app/services/push_service.py @@ -0,0 +1,98 @@ +import asyncio +import json +import logging +import uuid + +from pywebpush import WebPushException, webpush +from sqlalchemy import delete, select +from sqlalchemy.dialects.postgresql import insert as pg_insert +from sqlalchemy.ext.asyncio import AsyncSession + +from app.config import settings +from app.models import PushSubscription +from app.schemas.push import PushSubscriptionCreate + +logger = logging.getLogger(__name__) + + +async def subscribe( + db: AsyncSession, user_id: uuid.UUID, data: PushSubscriptionCreate +) -> PushSubscription: + # Upsert by endpoint: the same device/browser re-subscribing (e.g. after + # a key rotation, or logging in as someone else on a shared device) + # updates the existing row rather than erroring on the unique constraint. + stmt = ( + pg_insert(PushSubscription) + .values( + user_id=user_id, + endpoint=data.endpoint, + p256dh_key=data.keys.p256dh, + auth_key=data.keys.auth, + ) + .on_conflict_do_update( + index_elements=[PushSubscription.endpoint], + set_={ + "user_id": user_id, + "p256dh_key": data.keys.p256dh, + "auth_key": data.keys.auth, + }, + ) + .returning(PushSubscription) + ) + result = await db.execute(stmt) + await db.commit() + return result.scalar_one() + + +async def unsubscribe(db: AsyncSession, user_id: uuid.UUID, endpoint: str) -> None: + await db.execute( + delete(PushSubscription).where( + PushSubscription.user_id == user_id, PushSubscription.endpoint == endpoint + ) + ) + await db.commit() + + +def _send_one(subscription: PushSubscription, payload: dict) -> None: + webpush( + subscription_info={ + "endpoint": subscription.endpoint, + "keys": {"p256dh": subscription.p256dh_key, "auth": subscription.auth_key}, + }, + data=json.dumps(payload), + vapid_private_key=settings.vapid_private_key, + vapid_claims={"sub": settings.vapid_subject}, + ) + + +async def send_push_to_user(db: AsyncSession, user_id: uuid.UUID, payload: dict) -> None: + """Called (awaited) from the WS handler after broadcasting to connected + clients, so it never delays delivery to anyone actually online. Runs + sequentially against the caller's session rather than firing background + asyncio.create_task()s -- those can easily outlive the request/test event + loop they were created on, and AsyncSession isn't safe to touch from two + coroutines concurrently, so a fire-and-forget task per subscription would + risk exactly that. Each webpush() call itself still runs off the event + loop via asyncio.to_thread (pywebpush is synchronous).""" + if not settings.vapid_private_key: + logger.debug("VAPID keys not configured; skipping push to %s", user_id) + return + + result = await db.execute( + select(PushSubscription).where(PushSubscription.user_id == user_id) + ) + subscriptions = list(result.scalars().all()) + + for subscription in subscriptions: + try: + await asyncio.to_thread(_send_one, subscription, payload) + except WebPushException as exc: + status = exc.response.status_code if exc.response is not None else None + if status in (404, 410): + # Subscription is gone (browser unsubscribed, expired, etc.) + await db.execute( + delete(PushSubscription).where(PushSubscription.id == subscription.id) + ) + await db.commit() + else: + logger.warning("Push delivery failed for %s: %s", subscription.id, exc) diff --git a/backend/app/ws/chat.py b/backend/app/ws/chat.py index c9f929f..26269b9 100644 --- a/backend/app/ws/chat.py +++ b/backend/app/ws/chat.py @@ -6,8 +6,10 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from app.database import get_db -from app.models import RoomMembership, User +from app.models import Room, RoomMembership, User from app.services.message_service import create_message +from app.services.push_service import send_push_to_user +from app.ws.connection_manager import ConnectionManager router = APIRouter(tags=["ws"]) @@ -29,6 +31,31 @@ async def _is_room_member(db: AsyncSession, room_id: uuid.UUID, user_id: uuid.UU return result.scalar_one_or_none() is not None +async def _notify_offline_members( + db: AsyncSession, + manager: ConnectionManager, + room_id: uuid.UUID, + sender: User, + content: str, +) -> None: + result = await db.execute( + select(RoomMembership.user_id).where(RoomMembership.room_id == room_id) + ) + member_ids = {row[0] for row in result.all()} + offline_ids = member_ids - manager.connected_user_ids(room_id) + if not offline_ids: + return + + room = await db.get(Room, room_id) + payload = { + "title": f"#{room.name}" if room else "New message", + "body": f"{sender.username}: {content}"[:120], + "room_id": str(room_id), + } + for user_id in offline_ids: + await send_push_to_user(db, user_id, payload) + + @router.websocket("/ws/chat") async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db)) -> None: user_id_raw = websocket.session.get("user_id") @@ -63,7 +90,7 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db) {"type": "error", "detail": "Not a member of this room"} ) continue - manager.join(envelope.room_id, websocket) + manager.join(envelope.room_id, websocket, user.id) joined_rooms.add(envelope.room_id) await websocket.send_json({"type": "joined", "room_id": str(envelope.room_id)}) @@ -100,6 +127,9 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db) "created_at": message.created_at.isoformat(), }, ) + await _notify_offline_members( + db, manager, envelope.room_id, user, envelope.content + ) else: await websocket.send_json( diff --git a/backend/app/ws/connection_manager.py b/backend/app/ws/connection_manager.py index 43d94c9..720634a 100644 --- a/backend/app/ws/connection_manager.py +++ b/backend/app/ws/connection_manager.py @@ -13,9 +13,14 @@ class ConnectionManager: def __init__(self) -> None: self._rooms: dict[uuid.UUID, set[WebSocket]] = defaultdict(set) + # A single connection can be joined to multiple rooms at once (one + # `join` message per room over the same socket), so this is keyed on + # the socket alone, not per-room. + self._ws_user: dict[WebSocket, uuid.UUID] = {} - def join(self, room_id: uuid.UUID, websocket: WebSocket) -> None: + def join(self, room_id: uuid.UUID, websocket: WebSocket, user_id: uuid.UUID) -> None: self._rooms[room_id].add(websocket) + self._ws_user[websocket] = user_id def leave(self, room_id: uuid.UUID, websocket: WebSocket) -> None: self._rooms[room_id].discard(websocket) @@ -25,6 +30,15 @@ class ConnectionManager: def leave_all(self, websocket: WebSocket) -> None: for room_id in list(self._rooms.keys()): self.leave(room_id, websocket) + self._ws_user.pop(websocket, None) + + def connected_user_ids(self, room_id: uuid.UUID) -> set[uuid.UUID]: + """Users (not just sockets) with an active connection to this room -- + used to skip push notifications for anyone already watching, per + ARCHITECTURE.md's "members with no active connection" push flow.""" + return { + self._ws_user[ws] for ws in self._rooms.get(room_id, ()) if ws in self._ws_user + } async def broadcast(self, room_id: uuid.UUID, payload: dict) -> None: for websocket in list(self._rooms.get(room_id, ())): diff --git a/backend/pyproject.toml b/backend/pyproject.toml index ec6b3b4..88474bd 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -14,6 +14,7 @@ dependencies = [ "pydantic-settings>=2.6", "argon2-cffi>=23.1", "itsdangerous>=2.2", + "pywebpush>=2.0", ] [project.scripts] diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 50c7e30..18fee11 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -14,6 +14,7 @@ from alembic.config import Config from fastapi.testclient import TestClient from httpx import ASGITransport, AsyncClient from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine +from sqlalchemy.pool import NullPool from app.database import get_db from app.main import create_app @@ -84,8 +85,17 @@ def ws_client(): # the `db_session`/`app` fixtures' engine, which belongs to pytest's # loop. No per-test rollback here (see test_ws_chat.py for the # unique-name convention that keeps tests independent without it). + # + # poolclass=NullPool: with pooling, a WS test that does more than one + # DB round trip per message (e.g. the offline-push lookup) can hit a + # race where the pooled connection returned by the WS handler's session + # close hasn't finished being checked back in before the test's next + # (synchronous, same-portal) REST call checks a connection back out -- + # surfaces as "connection is closed". A fresh connection per session + # sidesteps it; fine for tests, not something prod needs (prod isn't + # juggling a background portal thread against the main test thread). application = create_app() - test_engine = create_async_engine(TEST_DATABASE_URL) + test_engine = create_async_engine(TEST_DATABASE_URL, poolclass=NullPool) test_session_factory = async_sessionmaker(test_engine, expire_on_commit=False) async def _get_db(): diff --git a/backend/tests/test_push.py b/backend/tests/test_push.py new file mode 100644 index 0000000..8b7c79f --- /dev/null +++ b/backend/tests/test_push.py @@ -0,0 +1,210 @@ +import uuid + +from pywebpush import WebPushException +from sqlalchemy import select + +from app.models import PushSubscription +from tests.conftest import login_as, register_and_login + + +def _subscription_payload(suffix: str = "a") -> dict: + return { + "endpoint": f"https://push.example.com/ep-{suffix}", + "keys": {"p256dh": f"p256dh-{suffix}", "auth": f"auth-{suffix}"}, + } + + +async def test_vapid_public_key_endpoint(client, db_session): + await register_and_login(client, db_session, username="alice") + resp = await client.get("/api/push/vapid-public-key") + assert resp.status_code == 200 + assert "public_key" in resp.json() + + +def _unique_suffix() -> str: + return uuid.uuid4().hex[:8] + + +async def test_subscribe_creates_row(client, db_session): + await register_and_login(client, db_session, username="alice") + payload = _subscription_payload(_unique_suffix()) + resp = await client.post("/api/push/subscribe", json=payload) + assert resp.status_code == 204 + + # The chatapp_test database is shared across the whole suite and the + # ws_client-based tests below intentionally don't roll back (see + # conftest.ws_client), so a unique endpoint keeps this test independent + # of leftover rows from those instead of asserting on the total count. + result = await db_session.execute( + select(PushSubscription).where(PushSubscription.endpoint == payload["endpoint"]) + ) + rows = result.scalars().all() + assert len(rows) == 1 + + +async def test_subscribe_upserts_by_endpoint(client, db_session): + await register_and_login(client, db_session, username="alice") + payload = _subscription_payload(_unique_suffix()) + assert (await client.post("/api/push/subscribe", json=payload)).status_code == 204 + + updated = {**payload, "keys": {"p256dh": "new-p256dh", "auth": "new-auth"}} + assert (await client.post("/api/push/subscribe", json=updated)).status_code == 204 + + result = await db_session.execute( + select(PushSubscription).where(PushSubscription.endpoint == payload["endpoint"]) + ) + rows = result.scalars().all() + assert len(rows) == 1 + assert rows[0].p256dh_key == "new-p256dh" + + +async def test_unsubscribe_removes_row(client, db_session): + await register_and_login(client, db_session, username="alice") + payload = _subscription_payload(_unique_suffix()) + await client.post("/api/push/subscribe", json=payload) + + resp = await client.request( + "DELETE", "/api/push/subscribe", json={"endpoint": payload["endpoint"]} + ) + assert resp.status_code == 204 + + result = await db_session.execute( + select(PushSubscription).where(PushSubscription.endpoint == payload["endpoint"]) + ) + assert result.scalars().all() == [] + + +def _register_ws(ws_client, username: str) -> dict: + from app.schemas.user import UserCreate + from app.services.auth_service import register_user + + async def _seed(): + async with ws_client.session_factory() as session: + await register_user( + session, + UserCreate(username=username, email=f"{username}@example.com", password="password123"), + ) + + ws_client.portal.call(_seed) + resp = ws_client.post( + "/api/auth/login", json={"username_or_email": username, "password": "password123"} + ) + assert resp.status_code == 200, resp.text + return resp.json() + + +def _unique(prefix: str) -> str: + return f"{prefix}-{uuid.uuid4().hex[:8]}" + + +def _fetch_subscriptions(ws_client, user_id: str) -> list[PushSubscription]: + async def _query(): + async with ws_client.session_factory() as session: + result = await session.execute( + select(PushSubscription).where(PushSubscription.user_id == uuid.UUID(user_id)) + ) + return list(result.scalars().all()) + + return ws_client.portal.call(_query) + + +def test_ws_message_pushes_offline_member_only(ws_client, monkeypatch): + calls = [] + + def fake_webpush(**kwargs): + calls.append(kwargs) + + monkeypatch.setattr("app.services.push_service.webpush", fake_webpush) + + 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"))) + + login_resp = ws_client.post( + "/api/auth/login", json={"username_or_email": alice["username"], "password": "password123"} + ) + assert login_resp.status_code == 200 + + 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" + # The handler processes frames strictly sequentially, so a second + # (idempotent) join only gets acked once the "message" frame's full + # handling -- including the offline-push step -- has completed. A + # plain `with` block exit doesn't guarantee that: closing can race + # ahead of (and cancel) still-in-flight server-side work. + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + + assert len(calls) == 1 + assert calls[0]["subscription_info"]["endpoint"].startswith("https://push.example.com/ep-bob") + assert "hello" in calls[0]["data"] + assert alice["username"] in calls[0]["data"] # sender attribution in the payload + + +def test_ws_message_no_push_when_member_connected(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"))) + + with ws_client.websocket_connect("/ws/chat") as bob_ws: + bob_ws.send_json({"type": "join", "room_id": room["id"]}) + assert bob_ws.receive_json()["type"] == "joined" + + ws_client.post( + "/api/auth/login", + json={"username_or_email": alice["username"], "password": "password123"}, + ) + with ws_client.websocket_connect("/ws/chat") as alice_ws: + alice_ws.send_json({"type": "join", "room_id": room["id"]}) + assert alice_ws.receive_json()["type"] == "joined" + alice_ws.send_json({"type": "message", "room_id": room["id"], "content": "hi"}) + assert alice_ws.receive_json()["type"] == "message" + # bob is connected too -- he should get the broadcast, not a push + assert bob_ws.receive_json()["type"] == "message" + + assert calls == [] + + +def test_expired_subscription_is_cleaned_up(ws_client, monkeypatch): + class FakeResponse: + status_code = 410 + + def fake_webpush(**kwargs): + raise WebPushException("gone", response=FakeResponse()) + + monkeypatch.setattr("app.services.push_service.webpush", fake_webpush) + + 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"))) + assert len(_fetch_subscriptions(ws_client, bob["id"])) == 1 + + 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" + # See test_ws_message_pushes_offline_member_only for why this sync + # barrier is needed before checking server-side push side effects. + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + + assert _fetch_subscriptions(ws_client, bob["id"]) == [] diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 80b9eef..e65f589 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -20,7 +20,12 @@ "@vitejs/plugin-react": "^6.0.4", "oxlint": "^1.75.0", "typescript": "~6.0.2", - "vite": "^8.2.0" + "vite": "^8.2.0", + "workbox-cacheable-response": "^7.4.1", + "workbox-expiration": "^7.4.1", + "workbox-precaching": "^7.4.1", + "workbox-routing": "^7.4.1", + "workbox-strategies": "^7.4.1" } }, "node_modules/@apideck/better-ajv-errors": { diff --git a/frontend/package.json b/frontend/package.json index f261de0..9c36f66 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -22,6 +22,11 @@ "@vitejs/plugin-react": "^6.0.4", "oxlint": "^1.75.0", "typescript": "~6.0.2", - "vite": "^8.2.0" + "vite": "^8.2.0", + "workbox-cacheable-response": "^7.4.1", + "workbox-expiration": "^7.4.1", + "workbox-precaching": "^7.4.1", + "workbox-routing": "^7.4.1", + "workbox-strategies": "^7.4.1" } } diff --git a/frontend/src/api/push.ts b/frontend/src/api/push.ts new file mode 100644 index 0000000..de3eccb --- /dev/null +++ b/frontend/src/api/push.ts @@ -0,0 +1,28 @@ +import { apiFetch } from './client' + +export interface PushSubscriptionPayload { + endpoint: string + keys: { p256dh: string; auth: string } +} + +interface VapidPublicKeyResponse { + public_key: string | null +} + +export function getVapidPublicKey(): Promise { + return apiFetch('/api/push/vapid-public-key') +} + +export function subscribePush(subscription: PushSubscriptionPayload): Promise { + return apiFetch('/api/push/subscribe', { + method: 'POST', + body: JSON.stringify(subscription), + }) +} + +export function unsubscribePush(endpoint: string): Promise { + return apiFetch('/api/push/subscribe', { + method: 'DELETE', + body: JSON.stringify({ endpoint }), + }) +} diff --git a/frontend/src/components/TopBar.css b/frontend/src/components/TopBar.css index c882f5b..0b2ac0e 100644 --- a/frontend/src/components/TopBar.css +++ b/frontend/src/components/TopBar.css @@ -94,3 +94,15 @@ .top-bar-menu button[role='menuitem']:hover { background: var(--ds-surface-2); } + +.top-bar-menu button[role='menuitem']:disabled { + opacity: 0.5; + cursor: not-allowed; +} + +.top-bar-menu-error { + color: var(--ds-danger); + font-size: 0.74rem; + padding: 2px 8px 6px; + max-width: 220px; +} diff --git a/frontend/src/components/TopBar.tsx b/frontend/src/components/TopBar.tsx index f189822..3607309 100644 --- a/frontend/src/components/TopBar.tsx +++ b/frontend/src/components/TopBar.tsx @@ -1,12 +1,38 @@ -import { useState } from 'react' +import { useEffect, useState } from 'react' import logo from '../assets/logo.png' import { useAuth } from '../context/AuthContext' import { initials } from '../lib/avatar' +import { getPushSubscriptionStatus, isPushSupported, subscribeToPush, unsubscribeFromPush } from '../lib/push' import './TopBar.css' export function TopBar() { const { user, logout } = useAuth() const [menuOpen, setMenuOpen] = useState(false) + const [pushSubscribed, setPushSubscribed] = useState(false) + const [pushBusy, setPushBusy] = useState(false) + const [pushError, setPushError] = useState(null) + + useEffect(() => { + getPushSubscriptionStatus().then(setPushSubscribed) + }, []) + + async function handleTogglePush() { + setPushBusy(true) + setPushError(null) + try { + if (pushSubscribed) { + await unsubscribeFromPush() + setPushSubscribed(false) + } else { + await subscribeToPush() + setPushSubscribed(true) + } + } catch (err) { + setPushError(err instanceof Error ? err.message : String(err)) + } finally { + setPushBusy(false) + } + } if (!user) return null @@ -32,6 +58,17 @@ export function TopBar() {
setMenuOpen(false)} />
{user.username}
+ {isPushSupported() && ( + + )} + {pushError &&
{pushError}
} diff --git a/frontend/src/lib/push.ts b/frontend/src/lib/push.ts new file mode 100644 index 0000000..cab55b7 --- /dev/null +++ b/frontend/src/lib/push.ts @@ -0,0 +1,64 @@ +import { getVapidPublicKey, subscribePush, unsubscribePush } from '../api/push' + +function urlBase64ToUint8Array(base64String: string): Uint8Array { + const padding = '='.repeat((4 - (base64String.length % 4)) % 4) + const base64 = (base64String + padding).replace(/-/g, '+').replace(/_/g, '/') + const rawData = atob(base64) + const outputArray = new Uint8Array(new ArrayBuffer(rawData.length)) + for (let i = 0; i < rawData.length; i++) { + outputArray[i] = rawData.charCodeAt(i) + } + return outputArray +} + +export function isPushSupported(): boolean { + return 'serviceWorker' in navigator && 'PushManager' in window +} + +export async function getPushSubscriptionStatus(): Promise { + if (!isPushSupported()) return false + const registration = await navigator.serviceWorker.ready + const subscription = await registration.pushManager.getSubscription() + return subscription !== null +} + +export async function subscribeToPush(): Promise { + if (!isPushSupported()) { + throw new Error('Push notifications are not supported in this browser') + } + + const permission = await Notification.requestPermission() + if (permission !== 'granted') { + throw new Error('Notification permission was not granted') + } + + const { public_key } = await getVapidPublicKey() + if (!public_key) { + throw new Error('Push notifications are not configured on the server') + } + + const registration = await navigator.serviceWorker.ready + const subscription = await registration.pushManager.subscribe({ + userVisibleOnly: true, + applicationServerKey: urlBase64ToUint8Array(public_key), + }) + + const json = subscription.toJSON() + if (!json.endpoint || !json.keys?.p256dh || !json.keys?.auth) { + throw new Error('Push subscription is missing required fields') + } + + await subscribePush({ + endpoint: json.endpoint, + keys: { p256dh: json.keys.p256dh, auth: json.keys.auth }, + }) +} + +export async function unsubscribeFromPush(): Promise { + if (!isPushSupported()) return + const registration = await navigator.serviceWorker.ready + const subscription = await registration.pushManager.getSubscription() + if (!subscription) return + await unsubscribePush(subscription.endpoint) + await subscription.unsubscribe() +} diff --git a/frontend/src/sw.ts b/frontend/src/sw.ts new file mode 100644 index 0000000..09bcf35 --- /dev/null +++ b/frontend/src/sw.ts @@ -0,0 +1,111 @@ +/// +import { CacheableResponsePlugin } from 'workbox-cacheable-response' +import { ExpirationPlugin } from 'workbox-expiration' +import { cleanupOutdatedCaches, createHandlerBoundToURL, precacheAndRoute } from 'workbox-precaching' +import { NavigationRoute, registerRoute } from 'workbox-routing' +import { NetworkOnly, StaleWhileRevalidate } from 'workbox-strategies' + +declare let self: ServiceWorkerGlobalScope + +self.skipWaiting() +cleanupOutdatedCaches() + +// The app shell -- same effect generateSW gave us automatically in Phase 3. +precacheAndRoute(self.__WB_MANIFEST) +registerRoute( + new NavigationRoute(createHandlerBoundToURL('index.html'), { + denylist: [/^\/api/, /^\/ws/], + }), +) + +const READ_CACHE_EXPIRATION = { maxEntries: 50, maxAgeSeconds: 7 * 24 * 60 * 60 } +const cacheableResponse = new CacheableResponsePlugin({ statuses: [0, 200] }) + +// Ported from Phase 3's vite.config.ts `workbox.runtimeCaching` -- that +// option only applies to the generateSW strategy, so with a hand-written +// service worker (required below for the push/notificationclick handlers) +// these routes have to be registered explicitly instead. +registerRoute(({ url }) => url.pathname.startsWith('/api/auth/'), new NetworkOnly()) + +registerRoute( + ({ url }) => url.pathname === '/api/rooms/mine', + new StaleWhileRevalidate({ + cacheName: 'api-rooms-mine', + plugins: [cacheableResponse, new ExpirationPlugin(READ_CACHE_EXPIRATION)], + }), +) +registerRoute( + ({ url }) => url.pathname === '/api/rooms', + new StaleWhileRevalidate({ + cacheName: 'api-rooms-open', + plugins: [cacheableResponse, new ExpirationPlugin(READ_CACHE_EXPIRATION)], + }), +) +registerRoute( + ({ url }) => /^\/api\/rooms\/[^/]+\/messages$/.test(url.pathname), + new StaleWhileRevalidate({ + cacheName: 'api-room-messages', + plugins: [cacheableResponse, new ExpirationPlugin(READ_CACHE_EXPIRATION)], + }), +) +registerRoute( + ({ url }) => /^\/api\/rooms\/[^/]+\/members$/.test(url.pathname), + new StaleWhileRevalidate({ + cacheName: 'api-room-members', + plugins: [cacheableResponse, new ExpirationPlugin(READ_CACHE_EXPIRATION)], + }), +) +registerRoute( + ({ url }) => url.pathname === '/api/invites/mine', + new StaleWhileRevalidate({ + cacheName: 'api-invites-mine', + plugins: [cacheableResponse, new ExpirationPlugin(READ_CACHE_EXPIRATION)], + }), +) +// Defensive default: anything else under /api/ stays network-only until +// explicitly opted in above. +registerRoute(({ url }) => url.pathname.startsWith('/api/'), new NetworkOnly()) + +interface PushPayload { + title: string + body: string + room_id: string +} + +self.addEventListener('push', (event) => { + if (!event.data) return + let payload: PushPayload + try { + payload = event.data.json() + } catch { + return + } + + event.waitUntil( + self.registration.showNotification(payload.title, { + body: payload.body, + icon: '/icons/icon-192.png', + badge: '/icons/icon-192.png', + data: { room_id: payload.room_id }, + }), + ) +}) + +self.addEventListener('notificationclick', (event) => { + event.notification.close() + const roomId = (event.notification.data as { room_id?: string } | undefined)?.room_id + const targetUrl = roomId ? `/rooms/${roomId}` : '/rooms' + + event.waitUntil( + (async () => { + const clientsList = await self.clients.matchAll({ type: 'window', includeUncontrolled: true }) + for (const client of clientsList) { + if ('focus' in client) { + await client.navigate(targetUrl) + return client.focus() + } + } + return self.clients.openWindow(targetUrl) + })(), + ) +}) diff --git a/frontend/tsconfig.app.json b/frontend/tsconfig.app.json index 6830b6f..b455f55 100644 --- a/frontend/tsconfig.app.json +++ b/frontend/tsconfig.app.json @@ -22,5 +22,6 @@ "erasableSyntaxOnly": true, "noFallthroughCasesInSwitch": true }, - "include": ["src"] + "include": ["src"], + "exclude": ["src/sw.ts"] } diff --git a/frontend/tsconfig.json b/frontend/tsconfig.json index 1ffef60..661d0c8 100644 --- a/frontend/tsconfig.json +++ b/frontend/tsconfig.json @@ -2,6 +2,7 @@ "files": [], "references": [ { "path": "./tsconfig.app.json" }, - { "path": "./tsconfig.node.json" } + { "path": "./tsconfig.node.json" }, + { "path": "./tsconfig.sw.json" } ] } diff --git a/frontend/tsconfig.sw.json b/frontend/tsconfig.sw.json new file mode 100644 index 0000000..b017ddd --- /dev/null +++ b/frontend/tsconfig.sw.json @@ -0,0 +1,15 @@ +{ + "compilerOptions": { + "tsBuildInfoFile": "./node_modules/.tmp/tsconfig.sw.tsbuildinfo", + "target": "es2023", + "lib": ["ES2023", "WebWorker"], + "module": "esnext", + "moduleResolution": "bundler", + "types": ["vite/client"], + "skipLibCheck": true, + "noEmit": true, + "moduleDetection": "force", + "erasableSyntaxOnly": true + }, + "include": ["src/sw.ts"] +} diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index 8a16b7c..a225d5f 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -2,16 +2,25 @@ import { defineConfig } from 'vite' import react from '@vitejs/plugin-react' import { VitePWA } from 'vite-plugin-pwa' -const READ_CACHE_EXPIRATION = { - maxEntries: 50, - maxAgeSeconds: 7 * 24 * 60 * 60, // 7 days -} - // https://vite.dev/config/ export default defineConfig({ plugins: [ react(), VitePWA({ + // generateSW (Phase 3) can't add custom event listeners, and push / + // notificationclick need exactly that -- injectManifest means we hand- + // write the service worker (src/sw.ts); its runtime-caching routes are + // registered there directly instead of via the `workbox` option below + // (which only applies to generateSW). + strategies: 'injectManifest', + srcDir: 'src', + filename: 'sw.ts', + injectManifest: { + // Workbox's default globPatterns exclude the manifest's own output + // dir, which is fine, but be explicit about what the app shell + // precache should contain. + globPatterns: ['**/*.{js,css,html,ico,png,svg,webmanifest}'], + }, registerType: 'autoUpdate', manifest: { name: 'KeepItTalking', @@ -31,77 +40,6 @@ export default defineConfig({ }, ], }, - workbox: { - navigateFallbackDenylist: [/^\/api/, /^\/ws/], - // urlPattern uses function matchers against url.pathname rather than - // RegExp (which Workbox tests against the *full href*, origin - // included -- a `^/api/` anchor would silently never match). - runtimeCaching: [ - // Never serve a stale cached "who am I" response. - { - urlPattern: ({ url }) => url.pathname.startsWith('/api/auth/'), - handler: 'NetworkOnly', - }, - // Cache-and-refresh: show the last known list/history immediately, - // update from the network in the background. Routes default to - // matching GET only, so mutations to these same paths are - // untouched and still go straight to network. - { - urlPattern: ({ url }) => url.pathname === '/api/rooms/mine', - handler: 'StaleWhileRevalidate', - options: { - cacheName: 'api-rooms-mine', - expiration: READ_CACHE_EXPIRATION, - cacheableResponse: { statuses: [0, 200] }, - }, - }, - { - urlPattern: ({ url }) => url.pathname === '/api/rooms', - handler: 'StaleWhileRevalidate', - options: { - cacheName: 'api-rooms-open', - expiration: READ_CACHE_EXPIRATION, - cacheableResponse: { statuses: [0, 200] }, - }, - }, - { - urlPattern: ({ url }) => - /^\/api\/rooms\/[^/]+\/messages$/.test(url.pathname), - handler: 'StaleWhileRevalidate', - options: { - cacheName: 'api-room-messages', - expiration: READ_CACHE_EXPIRATION, - cacheableResponse: { statuses: [0, 200] }, - }, - }, - { - urlPattern: ({ url }) => - /^\/api\/rooms\/[^/]+\/members$/.test(url.pathname), - handler: 'StaleWhileRevalidate', - options: { - cacheName: 'api-room-members', - expiration: READ_CACHE_EXPIRATION, - cacheableResponse: { statuses: [0, 200] }, - }, - }, - { - urlPattern: ({ url }) => url.pathname === '/api/invites/mine', - handler: 'StaleWhileRevalidate', - options: { - cacheName: 'api-invites-mine', - expiration: READ_CACHE_EXPIRATION, - cacheableResponse: { statuses: [0, 200] }, - }, - }, - // Defensive default: anything else under /api/ (including any - // future GET endpoint) stays network-only until explicitly opted - // in above. - { - urlPattern: ({ url }) => url.pathname.startsWith('/api/'), - handler: 'NetworkOnly', - }, - ], - }, }), ], server: {