| Username | +Name | +Groups | +Status | +Created | +
|---|
ID: {{ target_user.userid }} · Created {{ target_user.created_at.strftime('%Y-%m-%d %H:%M') }}
Password is set. + +
+ {% else %} +No password set.
+ {% endif %} + +No security keys registered.
+ {% endif %} +Password is set. + +
+{% else %} +No password set.
+{% endif %} + +No security keys registered.
+{% endif %} +``` + +**Step 3: Run full test suite** + +Run: `uv run python -m pytest -v` +Expected: All tests pass + +**Step 4: Commit** + +``` +git add src/porchlight/admin/routes.py src/porchlight/templates/admin/ +git commit -m "feat: add admin action routes (profile, groups, activate, credentials, invite, delete)" +``` + +--- + +### Task 8: Python unit tests for admin routes + +**Files:** +- Create: `tests/test_admin/test_admin_routes.py` + +**Step 1: Write comprehensive tests** + +Test all admin routes using the same `httpx.AsyncClient` + `ASGITransport` pattern used elsewhere in the test suite. Cover: + +- Admin guard: redirect for unauthenticated, 403 for non-admin +- User list: returns HTML with user table +- User detail: returns HTML with user data +- Profile update: modifies user profile +- Groups update: modifies user groups +- Activate/deactivate: toggles user active status +- Delete user: removes user +- Delete credentials: removes password/webauthn +- Create invite: generates magic link +- Re-invite: generates magic link for existing user +- Self-deletion prevention + +Look at the existing test patterns in `tests/test_auth_routes/` for fixture and session patterns. + +**Step 2: Run tests** + +Run: `uv run python -m pytest tests/test_admin/ -v` +Expected: All pass + +**Step 3: Commit** + +``` +git add tests/test_admin/ +git commit -m "test: add unit tests for admin routes" +``` + +--- + +### Task 9: Add admin link to manage nav for admin users + +**Files:** +- Modify: `src/porchlight/templates/manage/base.html` +- Modify: `src/porchlight/manage/routes.py` + +**Step 1: Pass `is_admin` to manage templates** + +In `src/porchlight/manage/routes.py`, update `credentials_page` and `profile_page` to check if user is admin and pass `is_admin` to context: + +```python +# In credentials_page and profile_page, after fetching the user: +user = await user_repo.get_by_userid(userid) +is_admin = user is not None and "admin" in user.groups +# Add is_admin=is_admin to template context +``` + +**Step 2: Update manage/base.html** + +```html +{% extends "base.html" %} + +{% block content %} + +{% block manage_content %}{% endblock %} +{% endblock %} +``` + +**Step 3: Run full test suite** + +Run: `uv run python -m pytest -v` +Expected: All tests pass + +**Step 4: Commit** + +``` +git add src/porchlight/manage/routes.py src/porchlight/templates/manage/base.html +git commit -m "feat: show admin link in manage nav for admin users" +``` + +--- + +### Task 10: Seed admin test user + E2E tests + +**Files:** +- Modify: `tests/e2e/setup_db.py` +- Create: `tests/e2e/admin.spec.js` + +**Step 1: Seed test data** + +In `tests/e2e/setup_db.py`, add an admin user and additional regular users: + +```python +# Admin user for admin page tests +admin_user = User( + userid="test-user-05", + username="adminuser", + given_name="Admin", + family_name="User", + email="admin@example.com", + groups=["admin", "users"], +) +await user_repo.create(admin_user) +admin_password_hash = password_service.hash("adminpass123") +await cred_repo.create_password( + PasswordCredential(user_id=admin_user.userid, password_hash=admin_password_hash) +) +result["admin_username"] = "adminuser" +result["admin_password"] = "adminpass123" +result["admin_userid"] = "test-user-05" +``` + +**Step 2: Write E2E tests** + +Create `tests/e2e/admin.spec.js` covering: + +- Auth guard: unauthenticated redirect, non-admin 403 +- User list: page structure, search, pagination +- User detail: page structure, all sections visible +- Profile update: modify and verify +- Groups update: modify and verify +- Activate/deactivate: toggle and verify +- Create invite: generate link and verify URL +- Re-invite: generate link for existing user +- Delete credential: remove password +- Delete user: remove user and verify redirect + +Use `helpers.login(page, username, password)` pattern from existing tests. + +**Step 3: Run E2E tests** + +Run from `tests/e2e/`: `bash run.sh admin.spec.js` +Expected: All pass + +**Step 4: Run full E2E suite** + +Run from `tests/e2e/`: `bash run.sh` +Expected: All 76+ existing tests pass + new admin tests + +**Step 5: Commit** + +``` +git add tests/e2e/setup_db.py tests/e2e/admin.spec.js +git commit -m "test: add E2E tests for admin pages" +``` + +--- + +### Task 11: Final verification + +**Step 1: Run full Python test suite** + +Run: `uv run python -m pytest -v` +Expected: All tests pass + +**Step 2: Run full E2E test suite** + +Run from `tests/e2e/`: `bash run.sh` +Expected: All tests pass + +**Step 3: Run linter** + +Run: `uv run ruff check src/ tests/ --fix` +Expected: No errors + +**Step 4: Final commit if needed** + +Fix any lint issues and commit. diff --git a/docs/plans/2026-02-18-consent-screen-plan.md b/docs/plans/2026-02-18-consent-screen-plan.md new file mode 100644 index 0000000..95e7e49 --- /dev/null +++ b/docs/plans/2026-02-18-consent-screen-plan.md @@ -0,0 +1,851 @@ +# Consent Screen Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Add a consent screen to the OIDC authorization flow so users +approve scopes before tokens are issued. Consent is saved per user+client +and only re-shown when requested scopes change. + +**Architecture:** New `user_consents` SQLite table stores approved scopes +per user+client. A consent check runs after authentication in the +authorization flow. If consent is needed, the user sees a scope approval +page. The manage-app client bypasses consent. Partial consent (unchecking +optional scopes) is supported. + +**Tech Stack:** SQLite migration, aiosqlite repository, Pydantic model, +Jinja2 template, existing FastAPI routing patterns. + +--- + +### Task 1: Add Consent model and SQLite migration + +**Files:** +- Modify: `src/porchlight/models.py` +- Create: `src/porchlight/store/sqlite/migrations/002_user_consents.sql` +- Modify: `src/porchlight/store/protocols.py` +- Modify: `src/porchlight/store/sqlite/repositories.py` +- Test: `tests/test_store/test_sqlite_consent_repo.py` + +**Step 1: Write the failing tests** + +Add `tests/test_store/test_sqlite_consent_repo.py`: + +```python +from datetime import UTC, datetime + +from porchlight.models import Consent +from porchlight.store.protocols import ConsentRepository +from porchlight.store.sqlite.repositories import SQLiteConsentRepository + + +async def test_implements_protocol(db_connection) -> None: + repo = SQLiteConsentRepository(db_connection) + assert isinstance(repo, ConsentRepository) + + +async def test_set_and_get_consent(db_connection, sample_user) -> None: + repo = SQLiteConsentRepository(db_connection) + await repo.set_consent(sample_user.userid, "test-rp", ["openid", "profile"]) + + consent = await repo.get_consent(sample_user.userid, "test-rp") + assert consent is not None + assert consent.userid == sample_user.userid + assert consent.client_id == "test-rp" + assert consent.scopes == ["openid", "profile"] + assert isinstance(consent.created_at, datetime) + assert isinstance(consent.updated_at, datetime) + + +async def test_get_consent_not_found(db_connection, sample_user) -> None: + repo = SQLiteConsentRepository(db_connection) + consent = await repo.get_consent(sample_user.userid, "nonexistent") + assert consent is None + + +async def test_set_consent_upserts(db_connection, sample_user) -> None: + repo = SQLiteConsentRepository(db_connection) + await repo.set_consent(sample_user.userid, "test-rp", ["openid"]) + await repo.set_consent(sample_user.userid, "test-rp", ["openid", "profile", "email"]) + + consent = await repo.get_consent(sample_user.userid, "test-rp") + assert consent is not None + assert consent.scopes == ["openid", "profile", "email"] + + +async def test_delete_consent(db_connection, sample_user) -> None: + repo = SQLiteConsentRepository(db_connection) + await repo.set_consent(sample_user.userid, "test-rp", ["openid"]) + + result = await repo.delete_consent(sample_user.userid, "test-rp") + assert result is True + + consent = await repo.get_consent(sample_user.userid, "test-rp") + assert consent is None + + +async def test_delete_consent_not_found(db_connection, sample_user) -> None: + repo = SQLiteConsentRepository(db_connection) + result = await repo.delete_consent(sample_user.userid, "nonexistent") + assert result is False + + +async def test_list_consents(db_connection, sample_user) -> None: + repo = SQLiteConsentRepository(db_connection) + await repo.set_consent(sample_user.userid, "rp-a", ["openid"]) + await repo.set_consent(sample_user.userid, "rp-b", ["openid", "profile"]) + + consents = await repo.list_consents(sample_user.userid) + assert len(consents) == 2 + client_ids = {c.client_id for c in consents} + assert client_ids == {"rp-a", "rp-b"} + + +async def test_list_consents_empty(db_connection, sample_user) -> None: + repo = SQLiteConsentRepository(db_connection) + consents = await repo.list_consents(sample_user.userid) + assert consents == [] + + +async def test_consent_deleted_on_user_cascade(db_connection, sample_user) -> None: + """Consent records are deleted when the user is deleted (CASCADE).""" + from porchlight.store.sqlite.repositories import SQLiteUserRepository + + repo = SQLiteConsentRepository(db_connection) + user_repo = SQLiteUserRepository(db_connection) + + await repo.set_consent(sample_user.userid, "test-rp", ["openid"]) + await user_repo.delete(sample_user.userid) + + consent = await repo.get_consent(sample_user.userid, "test-rp") + assert consent is None +``` + +This test file uses fixtures `db_connection` and `sample_user` that should +already exist in `tests/conftest.py`. Check `tests/conftest.py` for the +exact fixture names — they may be named differently (e.g. `db` instead of +`db_connection`). Adapt the test to match. + +**Step 2: Run tests to verify they fail** + +Run: `uv run python -m pytest tests/test_store/test_sqlite_consent_repo.py -v` +Expected: FAIL — `Consent` model not defined, `ConsentRepository` not +defined, `SQLiteConsentRepository` not defined. + +**Step 3: Write the implementation** + +Add to `src/porchlight/models.py`: + +```python +class Consent(BaseModel): + userid: str + client_id: str + scopes: list[str] + created_at: datetime = Field(default_factory=_utcnow) + updated_at: datetime = Field(default_factory=_utcnow) +``` + +Create `src/porchlight/store/sqlite/migrations/002_user_consents.sql`: + +```sql +CREATE TABLE user_consents ( + userid TEXT NOT NULL REFERENCES users(userid) ON DELETE CASCADE, + client_id TEXT NOT NULL, + scopes TEXT NOT NULL, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + PRIMARY KEY (userid, client_id) +); +``` + +Add to `src/porchlight/store/protocols.py`: + +```python +from porchlight.models import Consent # add to existing imports + +@runtime_checkable +class ConsentRepository(Protocol): + async def get_consent(self, userid: str, client_id: str) -> Consent | None: ... + + async def set_consent(self, userid: str, client_id: str, scopes: list[str]) -> None: ... + + async def delete_consent(self, userid: str, client_id: str) -> bool: ... + + async def list_consents(self, userid: str) -> list[Consent]: ... +``` + +Add to `src/porchlight/store/sqlite/repositories.py`: + +```python +import json # add to existing imports +from porchlight.models import Consent # add to existing imports + + +class SQLiteConsentRepository: + def __init__(self, db: aiosqlite.Connection) -> None: + self._db = db + + async def get_consent(self, userid: str, client_id: str) -> Consent | None: + async with self._db.execute( + "SELECT * FROM user_consents WHERE userid = ? AND client_id = ?", + (userid, client_id), + ) as cursor: + row = await cursor.fetchone() + if row is None: + return None + return Consent( + userid=row["userid"], + client_id=row["client_id"], + scopes=json.loads(row["scopes"]), + created_at=datetime.fromisoformat(row["created_at"]), + updated_at=datetime.fromisoformat(row["updated_at"]), + ) + + async def set_consent(self, userid: str, client_id: str, scopes: list[str]) -> None: + now = datetime.now(UTC).isoformat() + await self._db.execute( + """ + INSERT INTO user_consents (userid, client_id, scopes, created_at, updated_at) + VALUES (?, ?, ?, ?, ?) + ON CONFLICT (userid, client_id) + DO UPDATE SET scopes = excluded.scopes, updated_at = excluded.updated_at + """, + (userid, client_id, json.dumps(scopes), now, now), + ) + await self._db.commit() + + async def delete_consent(self, userid: str, client_id: str) -> bool: + cursor = await self._db.execute( + "DELETE FROM user_consents WHERE userid = ? AND client_id = ?", + (userid, client_id), + ) + await self._db.commit() + return cursor.rowcount > 0 + + async def list_consents(self, userid: str) -> list[Consent]: + async with self._db.execute( + "SELECT * FROM user_consents WHERE userid = ? ORDER BY client_id", + (userid,), + ) as cursor: + rows = await cursor.fetchall() + return [ + Consent( + userid=row["userid"], + client_id=row["client_id"], + scopes=json.loads(row["scopes"]), + created_at=datetime.fromisoformat(row["created_at"]), + updated_at=datetime.fromisoformat(row["updated_at"]), + ) + for row in rows + ] +``` + +**Step 4: Run tests to verify they pass** + +Run: `uv run python -m pytest tests/test_store/test_sqlite_consent_repo.py -v` +Expected: All PASS. + +**Step 5: Run full test suite** + +Run: `uv run python -m pytest -v` +Expected: All tests PASS (existing + new). + +**Step 6: Commit** + +```bash +git add src/porchlight/models.py src/porchlight/store/protocols.py \ + src/porchlight/store/sqlite/migrations/002_user_consents.sql \ + src/porchlight/store/sqlite/repositories.py \ + tests/test_store/test_sqlite_consent_repo.py +git commit -m "feat: add Consent model, migration, and repository" +``` + +--- + +### Task 2: Add consent check to authorization flow + +**Files:** +- Modify: `src/porchlight/app.py:36-38` (add consent_repo to app.state) +- Modify: `src/porchlight/oidc/endpoints.py:45-97,100-149` +- Test: `tests/test_oidc/test_consent_flow.py` + +**Step 1: Write the failing tests** + +Add `tests/test_oidc/test_consent_flow.py`: + +```python +import secrets +from datetime import UTC, datetime +from urllib.parse import parse_qs, urlparse + +from argon2 import PasswordHasher +from httpx import AsyncClient + +from porchlight.authn.password import PasswordService +from porchlight.models import PasswordCredential, User + + +async def test_authorization_shows_consent_for_new_client(client: AsyncClient) -> None: + """First-time authorization for an RP should redirect to /consent.""" + app = client._transport.app # type: ignore[union-attr] + _register_test_rp(app) + await _create_test_user(app) + + # Login + await client.post( + "/login/password", + data={"username": "consentuser", "password": "testpass"}, + headers={"HX-Request": "true"}, + ) + + # Authorization request + res = await client.get( + "/authorization", + params={ + "response_type": "code", + "client_id": "consent-rp", + "redirect_uri": "http://localhost:9000/callback", + "scope": "openid profile", + "state": "teststate", + }, + follow_redirects=False, + ) + assert res.status_code == 303 + assert "/consent" in res.headers["location"] + + +async def test_consent_page_renders(client: AsyncClient) -> None: + """GET /consent should render the consent form.""" + app = client._transport.app # type: ignore[union-attr] + _register_test_rp(app) + await _create_test_user(app) + await _login_and_start_auth(client) + + res = await client.get("/consent") + assert res.status_code == 200 + assert "consent-rp" in res.text + assert "profile" in res.text.lower() + + +async def test_consent_allow_redirects_with_code(client: AsyncClient) -> None: + """Approving consent should complete the authorization flow.""" + app = client._transport.app # type: ignore[union-attr] + _register_test_rp(app) + await _create_test_user(app) + await _login_and_start_auth(client) + + res = await client.post( + "/consent", + data={"action": "allow", "scope": ["openid", "profile"]}, + follow_redirects=False, + ) + assert res.status_code == 303 + location = res.headers["location"] + parsed = urlparse(location) + params = parse_qs(parsed.query) + assert "code" in params + + +async def test_consent_deny_redirects_with_error(client: AsyncClient) -> None: + """Denying consent should redirect with access_denied error.""" + app = client._transport.app # type: ignore[union-attr] + _register_test_rp(app) + await _create_test_user(app) + await _login_and_start_auth(client) + + res = await client.post( + "/consent", + data={"action": "deny"}, + follow_redirects=False, + ) + assert res.status_code == 303 + location = res.headers["location"] + parsed = urlparse(location) + params = parse_qs(parsed.query) + assert params["error"] == ["access_denied"] + + +async def test_saved_consent_skips_consent_screen(client: AsyncClient) -> None: + """Second authorization with same scopes should skip consent.""" + app = client._transport.app # type: ignore[union-attr] + _register_test_rp(app) + await _create_test_user(app) + + # First flow: login, authorize, consent + await _login_and_start_auth(client) + await client.post( + "/consent", + data={"action": "allow", "scope": ["openid", "profile"]}, + follow_redirects=False, + ) + + # Second flow: same scopes, should skip consent + res = await client.get( + "/authorization", + params={ + "response_type": "code", + "client_id": "consent-rp", + "redirect_uri": "http://localhost:9000/callback", + "scope": "openid profile", + "state": "teststate2", + }, + follow_redirects=False, + ) + assert res.status_code == 303 + location = res.headers["location"] + # Should redirect directly to callback, not /consent + assert "callback" in location + assert "code" in location + + +async def test_new_scopes_reshows_consent(client: AsyncClient) -> None: + """If RP requests new scopes, consent screen should reappear.""" + app = client._transport.app # type: ignore[union-attr] + _register_test_rp(app) + await _create_test_user(app) + + # First flow: consent to openid only + await _login_and_start_auth(client, scope="openid") + await client.post( + "/consent", + data={"action": "allow", "scope": ["openid"]}, + follow_redirects=False, + ) + + # Second flow: request openid + profile (new scope) + res = await client.get( + "/authorization", + params={ + "response_type": "code", + "client_id": "consent-rp", + "redirect_uri": "http://localhost:9000/callback", + "scope": "openid profile", + "state": "teststate2", + }, + follow_redirects=False, + ) + assert res.status_code == 303 + assert "/consent" in res.headers["location"] + + +async def test_manage_app_skips_consent(client: AsyncClient) -> None: + """The manage-app client should bypass consent entirely.""" + app = client._transport.app # type: ignore[union-attr] + settings = app.state.settings + await _create_test_user(app) + + await client.post( + "/login/password", + data={"username": "consentuser", "password": "testpass"}, + headers={"HX-Request": "true"}, + ) + + manage_cdb = app.state.oidc_server.context.cdb[settings.manage_client_id] + redirect_uri = manage_cdb["redirect_uris"][0][0] + + res = await client.get( + "/authorization", + params={ + "response_type": "code", + "client_id": settings.manage_client_id, + "redirect_uri": redirect_uri, + "scope": "openid profile email", + "state": "teststate", + }, + follow_redirects=False, + ) + assert res.status_code == 303 + location = res.headers["location"] + # Should redirect directly to callback, not /consent + assert "code" in location + assert "/consent" not in location + + +async def test_partial_consent_filters_scopes(client: AsyncClient) -> None: + """User can approve only some scopes (partial consent).""" + app = client._transport.app # type: ignore[union-attr] + _register_test_rp(app) + await _create_test_user(app) + + # Request openid + profile + email, approve only openid + profile + await _login_and_start_auth(client, scope="openid profile email") + res = await client.post( + "/consent", + data={"action": "allow", "scope": ["openid", "profile"]}, + follow_redirects=False, + ) + assert res.status_code == 303 + location = res.headers["location"] + assert "code" in location + + # Verify consent was saved with only the approved scopes + consent_repo = app.state.consent_repo + consent = await consent_repo.get_consent("lusab-consent", "consent-rp") + assert consent is not None + assert set(consent.scopes) == {"openid", "profile"} + + +# -- Test helpers -- + +def _register_test_rp(app) -> None: + oidc_server = app.state.oidc_server + if "consent-rp" in oidc_server.context.cdb: + return + client_id = "consent-rp" + client_secret = "consent-secret-0123456789abcdef" + oidc_server.context.cdb[client_id] = { + "client_id": client_id, + "client_secret": client_secret, + "redirect_uris": [("http://localhost:9000/callback", {})], + "response_types_supported": ["code"], + "token_endpoint_auth_method": "client_secret_basic", + "scope": ["openid", "profile", "email"], + "allowed_scopes": ["openid", "profile", "email"], + "client_salt": secrets.token_hex(8), + } + oidc_server.keyjar.add_symmetric(client_id, client_secret) + + +async def _create_test_user(app) -> None: + user_repo = app.state.user_repo + existing = await user_repo.get_by_username("consentuser") + if existing: + return + user = User( + userid="lusab-consent", + username="consentuser", + email="consent@example.com", + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + await user_repo.create(user) + svc = PasswordService(hasher=PasswordHasher(time_cost=1, memory_cost=8192)) + cred_repo = app.state.credential_repo + await cred_repo.create_password( + PasswordCredential(user_id=user.userid, password_hash=svc.hash("testpass")) + ) + + +async def _login_and_start_auth(client: AsyncClient, scope: str = "openid profile") -> None: + await client.post( + "/login/password", + data={"username": "consentuser", "password": "testpass"}, + headers={"HX-Request": "true"}, + ) + await client.get( + "/authorization", + params={ + "response_type": "code", + "client_id": "consent-rp", + "redirect_uri": "http://localhost:9000/callback", + "scope": scope, + "state": "teststate", + }, + follow_redirects=False, + ) +``` + +**Step 2: Run tests to verify they fail** + +Run: `uv run python -m pytest tests/test_oidc/test_consent_flow.py -v` +Expected: FAIL — consent_repo not on app.state, no `/consent` route. + +**Step 3: Write the implementation** + +**3a. Wire up consent_repo in `src/porchlight/app.py`** + +Add after `app.state.magic_link_repo` (around line 38): + +```python +from porchlight.store.sqlite.repositories import SQLiteConsentRepository # add to imports + +app.state.consent_repo = SQLiteConsentRepository(db) +``` + +**3b. Add consent routes and modify authorization flow in `src/porchlight/oidc/endpoints.py`** + +Add scope descriptions constant: + +```python +SCOPE_DESCRIPTIONS: dict[str, str] = { + "openid": "Sign you in (required)", + "profile": "Your name and profile information", + "email": "Your email address", + "phone": "Your phone number", +} +``` + +Modify `authorization()` (line 65-66) — when user is authenticated, instead +of calling `_complete_authorization()` directly, call a new +`_check_consent_or_complete()` helper: + +```python +if userid and username: + return await _check_consent_or_complete( + request, oidc_server, endpoint, parsed, userid, username, query_params + ) +``` + +Similarly modify `authorization_complete()` (line 97) to call +`_check_consent_or_complete()` instead of `_complete_authorization()`. + +Add the consent check helper: + +```python +async def _check_consent_or_complete( + request: Request, + oidc_server: object, + endpoint: object, + parsed: object, + userid: str, + username: str, + auth_params: dict, +) -> Response: + """Check if consent is needed; if so redirect to /consent, otherwise complete.""" + settings = request.app.state.settings + client_id = auth_params.get("client_id", "") + + # Manage-app bypasses consent + if client_id == settings.manage_client_id: + return await _complete_authorization(request, oidc_server, endpoint, parsed, userid, username) + + # Check stored consent + consent_repo = request.app.state.consent_repo + requested_scopes = auth_params.get("scope", "openid").split() + stored_consent = await consent_repo.get_consent(userid, client_id) + + if stored_consent and set(requested_scopes) <= set(stored_consent.scopes): + # All requested scopes already approved + return await _complete_authorization(request, oidc_server, endpoint, parsed, userid, username) + + # Consent needed — store auth state and redirect + request.session["consent_auth_request"] = auth_params + return RedirectResponse("/consent", status_code=303) +``` + +Add the consent page routes: + +```python +@router.get("/consent") +async def consent_page(request: Request) -> Response: + """Show the consent form.""" + auth_params = request.session.get("consent_auth_request") + if auth_params is None: + return HTMLResponse("No pending consent request
", status_code=400) + + userid = request.session.get("userid") + if not userid: + return RedirectResponse("/login", status_code=303) + + client_id = auth_params.get("client_id", "") + requested_scopes = auth_params.get("scope", "openid").split() + + scope_info = [ + {"name": s, "description": SCOPE_DESCRIPTIONS.get(s, s), "required": s == "openid"} + for s in requested_scopes + ] + + templates = request.app.state.templates + return templates.TemplateResponse( + request, + "consent.html", + {"client_id": client_id, "scopes": scope_info}, + ) + + +@router.post("/consent") +async def consent_submit(request: Request) -> Response: + """Handle consent form submission.""" + auth_params = request.session.pop("consent_auth_request", None) + if auth_params is None: + return HTMLResponse("No pending consent request
", status_code=400) + + userid = request.session.get("userid") + username = request.session.get("username") + if not userid or not username: + return RedirectResponse("/login", status_code=303) + + form = await request.form() + action = form.get("action") + client_id = auth_params.get("client_id", "") + redirect_uri = auth_params.get("redirect_uri", "") + state = auth_params.get("state", "") + + if action == "deny": + params = urlencode({"error": "access_denied", "state": state}) + return RedirectResponse(f"{redirect_uri}?{params}", status_code=303) + + # Allow — collect approved scopes + approved_scopes = form.getlist("scope") + if "openid" not in approved_scopes: + approved_scopes = ["openid"] + list(approved_scopes) + + # Save consent + consent_repo = request.app.state.consent_repo + await consent_repo.set_consent(userid, client_id, list(approved_scopes)) + + # Filter auth request scopes to only approved + auth_params["scope"] = " ".join(approved_scopes) + + # Re-parse and complete + oidc_server = request.app.state.oidc_server + endpoint = oidc_server.get_endpoint("authorization") + + try: + parsed = endpoint.parse_request(auth_params) + except Exception as exc: + return HTMLResponse(f"{exc}
", status_code=400) + + return await _complete_authorization(request, oidc_server, endpoint, parsed, userid, username) +``` + +**Step 4: Run tests to verify they pass** + +Run: `uv run python -m pytest tests/test_oidc/test_consent_flow.py -v` +Expected: All PASS. + +**Step 5: Run full test suite** + +Run: `uv run python -m pytest -v` +Expected: All tests PASS. Note: `test_full_authorization_code_flow` in +`tests/test_oidc/test_e2e_flow.py` will need updating since it now hits the +consent screen. Either pre-seed consent in the test setup, or add the consent +step to the test flow. + +**Step 6: Commit** + +```bash +git add src/porchlight/app.py src/porchlight/oidc/endpoints.py \ + tests/test_oidc/test_consent_flow.py +git commit -m "feat: add consent check to authorization flow" +``` + +--- + +### Task 3: Add consent page template + +**Files:** +- Create: `src/porchlight/templates/consent.html` +- Modify: `src/porchlight/static/style.css` (if needed) + +**Step 1: Create the consent template** + +Create `src/porchlight/templates/consent.html`: + +```html +{% extends "base.html" %} + +{% block title %}Authorize — Porchlight{% endblock %} + +{% block content %} + +{% endblock %} +``` + +Note: The `openid` checkbox is `checked disabled` so the user sees it but +can't uncheck it. A hidden input ensures the value is still submitted. + +**Step 2: Verify consent page renders** + +Run: `uv run python -m pytest tests/test_oidc/test_consent_flow.py::test_consent_page_renders -v` +Expected: PASS. + +**Step 3: Commit** + +```bash +git add src/porchlight/templates/consent.html +git commit -m "feat: add consent page template" +``` + +--- + +### Task 4: Update existing E2E test and add consent E2E test + +**Files:** +- Modify: `tests/test_oidc/test_e2e_flow.py` + +The existing `test_full_authorization_code_flow` test directly calls +`/authorization/complete` which now redirects to `/consent` instead of the RP +callback. This test needs to handle the consent step. + +**Step 1: Update the existing E2E flow test** + +In `tests/test_oidc/test_e2e_flow.py`, after step 2 (login) and step 3 +(complete authorization), add the consent step. The test should: + +1. Follow the redirect to `/consent` when `/authorization/complete` + returns 303 to `/consent` +2. POST to `/consent` with `action=allow` and the requested scopes +3. Follow the redirect to the RP callback + +Alternatively, pre-seed consent in the test setup using the consent repo: + +```python +consent_repo = app.state.consent_repo +await consent_repo.set_consent(user.userid, client_id, ["openid", "profile", "email"]) +``` + +This approach is simpler and keeps the existing test focused on the OIDC +token flow rather than the consent UI. + +**Step 2: Run full test suite** + +Run: `uv run python -m pytest -v` +Expected: All PASS. + +**Step 3: Commit** + +```bash +git add tests/test_oidc/test_e2e_flow.py +git commit -m "test: update E2E flow test to handle consent" +``` + +--- + +### Task 5: Quality check + +**Step 1: Run formatter and linter** + +Run: `uv run ruff format src/ tests/ && uv run ruff check src/ tests/ --fix` + +**Step 2: Run type checker** + +Run: `uv run ty check src/` + +**Step 3: Run full test suite** + +Run: `uv run python -m pytest -v` + +**Step 4: Fix any issues and commit** + +```bash +git add -A +git commit -m "refactor: fix lint and type check issues" +``` diff --git a/docs/plans/2026-02-18-playwright-migration-webauthn-e2e-design.md b/docs/plans/2026-02-18-playwright-migration-webauthn-e2e-design.md new file mode 100644 index 0000000..ef8aaa4 --- /dev/null +++ b/docs/plans/2026-02-18-playwright-migration-webauthn-e2e-design.md @@ -0,0 +1,117 @@ +# Playwright Test Migration + WebAuthn E2E Tests + +## Problem + +The existing 7 E2E tests use a hand-rolled test runner (`helpers.js` with +`run()`/`assert()`). This works but lacks structured reporting, retry logic, +parallel execution, and proper lifecycle hooks. + +Additionally, the WebAuthn (passkey) authentication flow has no E2E coverage. +The existing tests acknowledge this gap -- `test_login.js` checks that the +WebAuthn button exists but doesn't exercise the actual flow. + +## Decision + +Two-phase approach: + +1. Migrate all existing E2E tests from the custom runner to `@playwright/test` +2. Add comprehensive WebAuthn E2E tests using CDP virtual authenticators + +## Phase 1: Migrate to @playwright/test + +### Infrastructure + +- Add `@playwright/test` to `package.json` +- Create `playwright.config.js` with: + - `baseURL` from `TARGET_URL` env var (default `http://localhost:8099`) + - Chromium-only project (WebAuthn CDP requires Chromium) + - `testDir` pointing to the e2e directory + - `testMatch` for `*.spec.js` files +- Update `run.sh` to call `npx playwright test` instead of looping over `test_*.js` + +### Test conversion + +Each `test_*.js` becomes `*.spec.js`: +- `run(async (page, assert) => { ... })` becomes `test('...', async ({ page }) => { ... })` +- `assert(condition, msg)` becomes `expect(condition).toBeTruthy()` or specific matchers +- Shared setup moves to `test.beforeAll()` / `test.beforeEach()` +- `TARGET_URL` usage replaced by Playwright's `baseURL` (use relative paths) + +### What stays the same + +- `run.sh` still starts the app, seeds data, runs tests, tears down +- `setup_db.py` unchanged +- Test logic/assertions are equivalent + +### Files removed + +- `helpers.js` -- replaced by Playwright Test's built-in fixtures and `expect` + +## Phase 2: WebAuthn E2E tests + +### Approach: CDP Virtual Authenticator + Route Interception + +Chromium DevTools Protocol exposes `WebAuthn.addVirtualAuthenticator` which +creates a software authenticator that the browser's WebAuthn API treats as real. +This lets us test the full stack: button click -> `navigator.credentials` -> +server round-trip -> redirect. + +For error scenarios, we use Playwright's `page.route()` to intercept network +requests and return error responses. + +### Virtual authenticator configuration + +```js +const cdpSession = await page.context().newCDPSession(page); +await cdpSession.send('WebAuthn.enable'); +const { authenticatorId } = await cdpSession.send('WebAuthn.addVirtualAuthenticator', { + options: { + protocol: 'ctap2', + transport: 'internal', + hasResidentKey: true, + hasUserVerification: true, + isUserVerified: true, + automaticPresenceSimulation: true, + } +}); +``` + +### Test scenarios + +**Registration (from credentials page, requires active session):** +- Register a passkey via the "Add security key" button +- Verify the new passkey appears in the credential list +- Verify registration uses resident key (discoverable credential) + +**Authentication (usernameless login):** +- Full round-trip: register passkey -> logout -> login via passkey button +- No username needed -- browser's passkey picker selects the credential +- Verify redirect to `/manage/credentials` after successful login +- Verify session is established (can access protected pages) + +**Deletion:** +- Register passkey + have password, delete the passkey +- Cannot delete last credential (only has passkey, no password) + +**Error handling (route interception):** +- Server error on authentication begin +- Server error on authentication complete +- Expired session (complete without prior begin) + +### Test data + +Extend `setup_db.py` to create a user for WebAuthn tests: +- User with password credential (for logging in to register a passkey) +- The test flow: login with password -> register passkey -> logout -> login with passkey + +### Files changed/created + +| File | Change | +|------|--------| +| `tests/e2e/package.json` | Add `@playwright/test` dependency | +| `tests/e2e/playwright.config.js` | New: Playwright Test configuration | +| `tests/e2e/run.sh` | Update to use `npx playwright test` | +| `tests/e2e/helpers.js` | Remove (replaced by Playwright Test) | +| `tests/e2e/test_*.js` -> `*.spec.js` | Migrate all 7 tests to Playwright Test syntax | +| `tests/e2e/test_webauthn.spec.js` | New: WebAuthn E2E test suite | +| `tests/e2e/setup_db.py` | Add WebAuthn test user fixture | diff --git a/docs/plans/2026-02-18-playwright-migration-webauthn-e2e-plan.md b/docs/plans/2026-02-18-playwright-migration-webauthn-e2e-plan.md new file mode 100644 index 0000000..c5f81b3 --- /dev/null +++ b/docs/plans/2026-02-18-playwright-migration-webauthn-e2e-plan.md @@ -0,0 +1,913 @@ +# Playwright Test Migration + WebAuthn E2E Tests — Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Migrate existing E2E tests from custom runner to @playwright/test, then add comprehensive WebAuthn E2E tests using CDP virtual authenticators. + +**Architecture:** Two-phase migration. Phase 1 replaces the hand-rolled `helpers.js` runner with Playwright Test's built-in `test()`/`expect()` framework. Phase 2 adds a new `test_webauthn.spec.js` using CDP `WebAuthn.addVirtualAuthenticator` for real passkey simulation, plus `page.route()` for error scenarios. + +**Tech Stack:** Playwright Test (`@playwright/test`), Chrome DevTools Protocol (CDP) WebAuthn API, Node.js + +--- + +## Task 1: Update package.json and add Playwright config + +**Files:** +- Modify: `tests/e2e/package.json` +- Create: `tests/e2e/playwright.config.js` + +**Step 1: Update package.json** + +Replace `tests/e2e/package.json` with: + +```json +{ + "private": true, + "name": "porchlight-e2e", + "description": "End-to-end browser tests for Porchlight", + "scripts": { + "test": "npx playwright test", + "setup": "npx playwright install chromium" + }, + "dependencies": { + "@playwright/test": "^1.52.0" + } +} +``` + +Note: `@playwright/test` includes `playwright` as a dependency, so we replace the direct `playwright` dep. + +**Step 2: Create playwright.config.js** + +Create `tests/e2e/playwright.config.js`: + +```js +// @ts-check +const { defineConfig } = require('@playwright/test'); + +module.exports = defineConfig({ + testDir: '.', + testMatch: '*.spec.js', + timeout: 30_000, + retries: 0, + workers: 1, // Serial execution — tests share one seeded database + reporter: [['list']], + use: { + baseURL: process.env.TARGET_URL || 'http://localhost:8099', + browserName: 'chromium', + headless: process.env.E2E_HEADLESS !== '0', + }, +}); +``` + +**Step 3: Install updated dependencies** + +Run: `cd tests/e2e && npm install` + +**Step 4: Verify config loads** + +Run: `cd tests/e2e && npx playwright test --list` +Expected: No errors (will say "no tests found" since *.spec.js files don't exist yet) + +**Step 5: Commit** + +``` +feat(e2e): add @playwright/test and config +``` + +--- + +## Task 2: Update run.sh for Playwright Test + +**Files:** +- Modify: `tests/e2e/run.sh` + +**Step 1: Update run.sh** + +The script still starts the app, seeds data, and runs tests — but now calls `npx playwright test` instead of looping over individual files. + +Replace the "Run tests" section (lines 63-81) and the final summary (lines 83-90) in `run.sh`: + +```bash +# --- Run tests --- +echo "" +echo "=== Running Playwright tests ===" +cd "$SCRIPT_DIR" +if [ $# -gt 0 ]; then + npx playwright test "$@" +else + npx playwright test +fi +EXIT_CODE=$? + +exit "$EXIT_CODE" +``` + +The rest of the script (server start, seed, cleanup) stays the same. + +**Step 2: Verify run.sh still starts/stops correctly** + +Don't run the full suite yet (no spec files), just verify the script structure is correct by reading it. + +**Step 3: Commit** + +``` +feat(e2e): update run.sh for Playwright Test runner +``` + +--- + +## Task 3: Migrate test_health.js -> health.spec.js + +**Files:** +- Create: `tests/e2e/health.spec.js` +- Remove: `tests/e2e/test_health.js` + +**Step 1: Create health.spec.js** + +```js +// @ts-check +const { test, expect } = require('@playwright/test'); + +test.describe('Health endpoint', () => { + test('returns OK status', async ({ request }) => { + const resp = await request.get('/health'); + expect(resp.ok()).toBe(true); + + const body = await resp.json(); + expect(body.status).toBe('ok'); + }); +}); +``` + +Note: `request` fixture uses `baseURL` from config automatically. + +**Step 2: Delete test_health.js** + +**Step 3: Run the test** + +Run: `cd tests/e2e && TARGET_URL=http://localhost:8099 npx playwright test health.spec.js` +(Server must be running separately for this step) + +**Step 4: Commit** + +``` +refactor(e2e): migrate health test to Playwright Test +``` + +--- + +## Task 4: Migrate test_login.js -> login.spec.js + +**Files:** +- Create: `tests/e2e/login.spec.js` +- Remove: `tests/e2e/test_login.js` + +**Step 1: Create login.spec.js** + +```js +// @ts-check +const { test, expect } = require('@playwright/test'); + +test.describe('Login page', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/login'); + }); + + test.describe('Branding', () => { + test('page title contains Porchlight', async ({ page }) => { + await expect(page).toHaveTitle(/Porchlight/); + }); + + test('page title does not contain FastAPI', async ({ page }) => { + const title = await page.title(); + expect(title).not.toContain('FastAPI'); + }); + + test('favicon is served', async ({ page, request }) => { + const favicon = await page.locator('link[rel="icon"]').getAttribute('href'); + expect(favicon).toBe('/static/favicon.png'); + + const resp = await request.get('/static/favicon.png'); + expect(resp.ok()).toBe(true); + }); + + test('site header and logo are visible', async ({ page, request }) => { + await expect(page.locator('.site-header')).toBeVisible(); + await expect(page.locator('.site-logo')).toBeVisible(); + + const logoSrc = await page.locator('.site-logo').getAttribute('src'); + expect(logoSrc).toBe('/static/logo.svg'); + + const resp = await request.get('/static/logo.svg'); + expect(resp.ok()).toBe(true); + + await expect(page.locator('.site-title')).toHaveText('Porchlight'); + }); + }); + + test.describe('Accessibility', () => { + test('skip link is present', async ({ page }) => { + await expect(page.locator('.skip-link')).toHaveCount(1); + }); + + test('main landmark exists', async ({ page }) => { + await expect(page.locator('main#main')).toHaveCount(1); + }); + + test('polite live region exists', async ({ page }) => { + await expect(page.locator('#live[aria-live="polite"]')).toHaveCount(1); + }); + }); + + test.describe('Login form structure', () => { + test('heading says Sign in', async ({ page }) => { + await expect(page.locator('h1')).toHaveText('Sign in'); + }); + + test('password login form exists with inputs', async ({ page }) => { + await expect(page.locator('form[hx-post="/login/password"]')).toHaveCount(1); + await expect(page.locator('#username')).toBeVisible(); + await expect(page.locator('#password')).toBeVisible(); + await expect(page.locator('form[hx-post="/login/password"] button[type="submit"]')).toBeVisible(); + }); + + test('WebAuthn login form exists', async ({ page }) => { + await expect(page.locator('#webauthn-login-btn')).toBeVisible(); + }); + }); + + test.describe('Theme / styling', () => { + test('body has themed background', async ({ page }) => { + const bgColor = await page.evaluate(() => getComputedStyle(document.body).backgroundColor); + expect(['rgb(250, 250, 249)', 'rgb(28, 25, 23)']).toContain(bgColor); + }); + + test('button uses amber accent', async ({ page }) => { + const btnBg = await page.evaluate(() => { + const btn = document.querySelector('button[type="submit"]'); + return btn ? getComputedStyle(btn).backgroundColor : ''; + }); + expect(['rgb(217, 119, 6)', 'rgb(245, 158, 11)']).toContain(btnBg); + }); + + test('sections have surface background and border', async ({ page }) => { + const sectionBg = await page.evaluate(() => { + const section = document.querySelector('section'); + return section ? getComputedStyle(section).backgroundColor : ''; + }); + expect(['rgb(245, 245, 244)', 'rgb(41, 37, 36)']).toContain(sectionBg); + + const sectionBorder = await page.evaluate(() => { + const section = document.querySelector('section'); + return section ? getComputedStyle(section).borderStyle : ''; + }); + expect(sectionBorder).toBe('solid'); + }); + }); + + test.describe('Static assets', () => { + test('CSS is served with expected content', async ({ request }) => { + const resp = await request.get('/static/style.css'); + expect(resp.ok()).toBe(true); + const css = await resp.text(); + expect(css).toContain('--accent'); + expect(css).toContain('#d97706'); + expect(css).toContain('prefers-color-scheme: dark'); + expect(css).toContain('prefers-reduced-motion'); + }); + }); +}); +``` + +**Step 2: Delete test_login.js** + +**Step 3: Commit** + +``` +refactor(e2e): migrate login page test to Playwright Test +``` + +--- + +## Task 5: Migrate test_password_auth.js -> password-auth.spec.js + +**Files:** +- Create: `tests/e2e/password-auth.spec.js` +- Remove: `tests/e2e/test_password_auth.js` + +**Step 1: Create password-auth.spec.js** + +```js +// @ts-check +const { test, expect } = require('@playwright/test'); + +const fixtures = JSON.parse(process.env.E2E_FIXTURES || '{}'); + +test.describe('Password authentication', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/login'); + }); + + test('shows error for nonexistent user', async ({ page }) => { + await page.fill('#username', 'nobody'); + await page.fill('#password', 'whatever'); + await page.click('form[hx-post="/login/password"] button[type="submit"]'); + + const alert = page.locator('[role="alert"]'); + await expect(alert).toBeVisible({ timeout: 5000 }); + await expect(alert).toContainText('Invalid username or password'); + }); + + test('shows error for wrong password', async ({ page }) => { + await page.fill('#username', fixtures.login_username); + await page.fill('#password', 'wrongpassword'); + await page.click('form[hx-post="/login/password"] button[type="submit"]'); + + const alert = page.locator('[role="alert"]'); + await expect(alert).toBeVisible({ timeout: 5000 }); + await expect(alert).toContainText('Invalid username or password'); + }); + + test('successful login redirects to credentials', async ({ page }) => { + await page.fill('#username', fixtures.login_username); + await page.fill('#password', fixtures.login_password); + await page.click('form[hx-post="/login/password"] button[type="submit"]'); + + await page.waitForURL('**/manage/credentials', { timeout: 5000 }); + expect(page.url()).toContain('/manage/credentials'); + }); + + test('form has required and autocomplete attributes', async ({ page }) => { + await expect(page.locator('#username')).toHaveAttribute('required', ''); + await expect(page.locator('#password')).toHaveAttribute('required', ''); + await expect(page.locator('#username')).toHaveAttribute('autocomplete', 'username'); + await expect(page.locator('#password')).toHaveAttribute('autocomplete', 'current-password'); + }); +}); +``` + +**Step 2: Delete test_password_auth.js** + +**Step 3: Commit** + +``` +refactor(e2e): migrate password auth test to Playwright Test +``` + +--- + +## Task 6: Migrate test_registration.js -> registration.spec.js + +**Files:** +- Create: `tests/e2e/registration.spec.js` +- Remove: `tests/e2e/test_registration.js` + +**Step 1: Create registration.spec.js** + +```js +// @ts-check +const { test, expect } = require('@playwright/test'); + +const fixtures = JSON.parse(process.env.E2E_FIXTURES || '{}'); + +test.describe('Magic link registration', () => { + test('invalid token returns 400', async ({ page }) => { + const resp = await page.goto('/register/invalid-token-12345'); + expect(resp.status()).toBe(400); + await expect(page.locator('body')).toContainText('Invalid or expired'); + }); + + test('used token returns 400', async ({ page }) => { + expect(fixtures.used_token).toBeTruthy(); + const resp = await page.goto(`/register/${fixtures.used_token}`); + expect(resp.status()).toBe(400); + await expect(page.locator('body')).toContainText('Invalid or expired'); + }); +}); +``` + +**Step 2: Delete test_registration.js** + +**Step 3: Commit** + +``` +refactor(e2e): migrate registration test to Playwright Test +``` + +--- + +## Task 7: Migrate test_credentials.js -> credentials.spec.js + +**Files:** +- Create: `tests/e2e/credentials.spec.js` +- Remove: `tests/e2e/test_credentials.js` + +**Step 1: Create credentials.spec.js** + +```js +// @ts-check +const { test, expect } = require('@playwright/test'); + +const fixtures = JSON.parse(process.env.E2E_FIXTURES || '{}'); + +test.describe('Credential management', () => { + test.beforeEach(async ({ page }) => { + // Login with dedicated credentials user + await page.goto('/login'); + await page.fill('#username', fixtures.cred_username); + await page.fill('#password', fixtures.cred_password); + await page.click('form[hx-post="/login/password"] button[type="submit"]'); + await page.waitForURL('**/manage/credentials', { timeout: 5000 }); + }); + + test.describe('Page structure', () => { + test('page title contains Credentials and Porchlight', async ({ page }) => { + await expect(page).toHaveTitle(/Credentials/); + await expect(page).toHaveTitle(/Porchlight/); + }); + + test('heading says Credentials', async ({ page }) => { + await expect(page.locator('h1')).toHaveText('Credentials'); + }); + + test('security keys section is visible', async ({ page }) => { + await expect(page.locator('h2:has-text("Security keys")')).toBeVisible(); + await expect(page.locator('#webauthn-register-btn')).toBeVisible(); + }); + + test('password section is visible', async ({ page }) => { + await expect(page.locator('h2:has-text("Password")')).toBeVisible(); + await expect(page.locator('#password-section')).toBeVisible(); + }); + }); + + test.describe('Password validation', () => { + test('shows mismatch error', async ({ page }) => { + await page.fill('#password', 'newpassword1'); + await page.fill('#confirm', 'newpassword2'); + await page.click('#password-section button[type="submit"]'); + + const alert = page.locator('#password-section [role="alert"]'); + await expect(alert).toBeVisible({ timeout: 5000 }); + await expect(alert).toContainText('do not match'); + }); + + test('password inputs have minlength attribute', async ({ page }) => { + await expect(page.locator('#password')).toHaveAttribute('minlength', '8'); + await expect(page.locator('#confirm')).toHaveAttribute('minlength', '8'); + }); + }); + + test.describe('Password change', () => { + test('successful password change shows confirmation', async ({ page }) => { + await page.fill('#password', 'newpassword123'); + await page.fill('#confirm', 'newpassword123'); + await page.click('#password-section button[type="submit"]'); + + const status = page.locator('#password-section [role="status"]'); + await expect(status).toBeVisible({ timeout: 5000 }); + await expect(status).toContainText('Password updated'); + }); + }); +}); +``` + +**Step 2: Delete test_credentials.js** + +**Step 3: Commit** + +``` +refactor(e2e): migrate credentials test to Playwright Test +``` + +--- + +## Task 8: Migrate test_auth_guard.js -> auth-guard.spec.js + +**Files:** +- Create: `tests/e2e/auth-guard.spec.js` +- Remove: `tests/e2e/test_auth_guard.js` + +**Step 1: Create auth-guard.spec.js** + +```js +// @ts-check +const { test, expect } = require('@playwright/test'); + +test.describe('Auth guard', () => { + test('unauthenticated /manage/credentials redirects to login', async ({ page }) => { + await page.goto('/manage/credentials'); + await page.waitForURL('**/login', { timeout: 5000 }); + expect(page.url()).toContain('/login'); + }); + + test('unauthenticated /manage/credentials?setup=1 redirects to login', async ({ page }) => { + await page.goto('/manage/credentials?setup=1'); + await page.waitForURL('**/login', { timeout: 5000 }); + expect(page.url()).toContain('/login'); + }); +}); +``` + +**Step 2: Delete test_auth_guard.js** + +**Step 3: Commit** + +``` +refactor(e2e): migrate auth guard test to Playwright Test +``` + +--- + +## Task 9: Migrate test_full_flow.js -> full-flow.spec.js + +**Files:** +- Create: `tests/e2e/full-flow.spec.js` +- Remove: `tests/e2e/test_full_flow.js` + +**Step 1: Create full-flow.spec.js** + +```js +// @ts-check +const { test, expect } = require('@playwright/test'); + +const fixtures = JSON.parse(process.env.E2E_FIXTURES || '{}'); + +test.describe('Full user journey', () => { + test('magic link register -> set password -> logout -> login', async ({ page, request }) => { + // Step 1: Register via magic link + expect(fixtures.register_token).toBeTruthy(); + await page.goto(`/register/${fixtures.register_token}`); + await page.waitForURL('**/manage/credentials?setup=1', { timeout: 5000 }); + expect(page.url()).toContain('/manage/credentials'); + + // Welcome message visible + const welcome = page.locator('[role="status"]'); + await expect(welcome).toBeVisible(); + await expect(welcome).toContainText('Welcome'); + await expect(page).toHaveTitle(/Porchlight/); + + // Step 2: Set password + await expect(page.locator('#password')).toBeVisible(); + await expect(page.locator('#confirm')).toBeVisible(); + + await page.fill('#password', 'mypassword123'); + await page.fill('#confirm', 'mypassword123'); + await page.click('#password-section button[type="submit"]'); + + const successMsg = page.locator('#password-section [role="status"]'); + await expect(successMsg).toBeVisible({ timeout: 5000 }); + await expect(successMsg).toContainText('Password updated'); + + // Step 3: Logout + await request.post('/logout'); + await page.goto('/manage/credentials'); + await page.waitForURL('**/login', { timeout: 5000 }); + expect(page.url()).toContain('/login'); + + // Step 4: Login with new password + await page.fill('#username', fixtures.register_username); + await page.fill('#password', 'mypassword123'); + await page.click('form[hx-post="/login/password"] button[type="submit"]'); + + await page.waitForURL('**/manage/credentials', { timeout: 5000 }); + expect(page.url()).toContain('/manage/credentials'); + + // No setup message on normal login + await expect(page.locator('[role="status"]:has-text("Welcome")')).toHaveCount(0); + }); +}); +``` + +**Step 2: Delete test_full_flow.js** + +**Step 3: Commit** + +``` +refactor(e2e): migrate full flow test to Playwright Test +``` + +--- + +## Task 10: Remove old helpers.js and run the full suite + +**Files:** +- Remove: `tests/e2e/helpers.js` + +**Step 1: Delete helpers.js** + +The old custom runner is no longer used by any test. + +**Step 2: Run the full e2e suite** + +Run: `./tests/e2e/run.sh` +Expected: All tests pass with Playwright Test's list reporter output. + +**Step 3: Commit** + +``` +refactor(e2e): remove old custom test runner +``` + +--- + +## Task 11: Extend setup_db.py with WebAuthn test user + +**Files:** +- Modify: `tests/e2e/setup_db.py` + +**Step 1: Add WebAuthn test user** + +Add after the "Create a separate user for credentials management test" block (after line 63): + +```python + # 5. Create a user with password for WebAuthn registration tests + # (login with password first, then register a passkey) + webauthn_user = User(userid="test-user-03", username="webauthnuser", groups=["users"]) + await user_repo.create(webauthn_user) + webauthn_password_hash = password_service.hash("webauthnpass123") + await cred_repo.create_password( + PasswordCredential(user_id=webauthn_user.userid, password_hash=webauthn_password_hash) + ) + result["webauthn_username"] = "webauthnuser" + result["webauthn_password"] = "webauthnpass123" + result["webauthn_userid"] = "test-user-03" +``` + +**Step 2: Verify seeding still works** + +The server startup in run.sh will exercise this. + +**Step 3: Commit** + +``` +feat(e2e): add WebAuthn test user to fixture seeding +``` + +--- + +## Task 12: Create WebAuthn E2E tests + +**Files:** +- Create: `tests/e2e/webauthn.spec.js` + +**Step 1: Create webauthn.spec.js** + +```js +// @ts-check +const { test, expect } = require('@playwright/test'); + +const fixtures = JSON.parse(process.env.E2E_FIXTURES || '{}'); + +/** + * Set up a CDP virtual authenticator for WebAuthn testing. + * Returns { cdpSession, authenticatorId } for cleanup. + */ +async function addVirtualAuthenticator(page) { + const cdpSession = await page.context().newCDPSession(page); + await cdpSession.send('WebAuthn.enable'); + const { authenticatorId } = await cdpSession.send('WebAuthn.addVirtualAuthenticator', { + options: { + protocol: 'ctap2', + transport: 'internal', + hasResidentKey: true, + hasUserVerification: true, + isUserVerified: true, + automaticPresenceSimulation: true, + }, + }); + return { cdpSession, authenticatorId }; +} + +/** + * Log in with password and navigate to credentials page. + */ +async function loginWithPassword(page, username, password) { + await page.goto('/login'); + await page.fill('#username', username); + await page.fill('#password', password); + await page.click('form[hx-post="/login/password"] button[type="submit"]'); + await page.waitForURL('**/manage/credentials', { timeout: 5000 }); +} + +test.describe('WebAuthn', () => { + test.describe('Registration', () => { + test('register a passkey from credentials page', async ({ page }) => { + const { cdpSession, authenticatorId } = await addVirtualAuthenticator(page); + + try { + await loginWithPassword(page, fixtures.webauthn_username, fixtures.webauthn_password); + + // Verify no security keys initially + await expect(page.locator('#webauthn-list')).toContainText('No security keys registered'); + + // Click register button + await page.click('#webauthn-register-btn'); + + // Wait for page reload (registration success triggers reload) + await page.waitForURL('**/manage/credentials', { timeout: 10000 }); + + // Verify the passkey now appears in the list + await expect(page.locator('#webauthn-list')).not.toContainText('No security keys registered'); + await expect(page.locator('#webauthn-list li')).toHaveCount(1); + } finally { + await cdpSession.send('WebAuthn.removeVirtualAuthenticator', { authenticatorId }); + } + }); + }); + + test.describe('Authentication (usernameless)', () => { + test('full round-trip: register passkey, logout, login with passkey', async ({ page, request }) => { + const { cdpSession, authenticatorId } = await addVirtualAuthenticator(page); + + try { + // Step 1: Login with password and register a passkey + await loginWithPassword(page, fixtures.webauthn_username, fixtures.webauthn_password); + await page.click('#webauthn-register-btn'); + await page.waitForURL('**/manage/credentials', { timeout: 10000 }); + + // Verify passkey registered + await expect(page.locator('#webauthn-list li')).toHaveCount(1); + + // Step 2: Logout + await request.post('/logout'); + + // Step 3: Login with passkey (no username) + await page.goto('/login'); + await page.click('#webauthn-login-btn'); + + // Wait for redirect to credentials page + await page.waitForURL('**/manage/credentials', { timeout: 10000 }); + expect(page.url()).toContain('/manage/credentials'); + + // Verify we're authenticated — page shows credential management + await expect(page.locator('h1')).toHaveText('Credentials'); + } finally { + await cdpSession.send('WebAuthn.removeVirtualAuthenticator', { authenticatorId }); + } + }); + + test('shows error when session expired (complete without begin)', async ({ page }) => { + const { cdpSession, authenticatorId } = await addVirtualAuthenticator(page); + + try { + await page.goto('/login'); + + // Intercept the begin request to skip it but still try complete + await page.route('/login/webauthn/complete', async (route) => { + // Simulate server response for missing state + await route.fulfill({ + status: 400, + contentType: 'application/json', + body: JSON.stringify({ error: 'Authentication session expired' }), + }); + }); + + // Directly POST to complete endpoint (skipping begin) + await page.evaluate(async () => { + const res = await fetch('/login/webauthn/complete', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + id: 'fake', + rawId: 'fake', + type: 'public-key', + response: {}, + }), + }); + const data = await res.json(); + const el = document.getElementById('webauthn-login-status'); + if (el) el.innerHTML = '