diff --git a/src/porchlight/authn/routes.py b/src/porchlight/authn/routes.py index cf851ea..606b315 100644 --- a/src/porchlight/authn/routes.py +++ b/src/porchlight/authn/routes.py @@ -86,7 +86,28 @@ async def logout(request: Request) -> Response: return response -@router.get("/register/{token}") +@router.get("/register/{token}", response_class=HTMLResponse) +async def register_magic_link_page(request: Request, token: str) -> Response: + """Show the registration confirmation page. + + GET is side-effect free: it only validates the token (without consuming it) + and renders a form. The token is consumed by the POST below, so that simply + visiting the link (email scanners, prefetchers) cannot create a session. + """ + magic_link_service = request.app.state.magic_link_service + link = await magic_link_service.validate(token) + if link is None: + return HTMLResponse("
Invalid or expired registration link.
", status_code=400) + + templates = request.app.state.templates + return templates.TemplateResponse( + request, + "register.html", + {"token": token, "username": link.username}, + ) + + +@router.post("/register/{token}") async def register_magic_link(request: Request, token: str) -> Response: magic_link_service = request.app.state.magic_link_service user_repo = request.app.state.user_repo @@ -118,8 +139,7 @@ async def register_magic_link(request: Request, token: str) -> Response: user = User(userid=userid, username=link.username, groups=["users"]) await user_repo.create(user) - request.session["userid"] = user.userid - request.session["username"] = user.username + _establish_authenticated_session(request, user) return RedirectResponse("/manage/credentials?setup=1", status_code=303) diff --git a/src/porchlight/templates/register.html b/src/porchlight/templates/register.html new file mode 100644 index 0000000..6b9a668 --- /dev/null +++ b/src/porchlight/templates/register.html @@ -0,0 +1,14 @@ +{% extends "base.html" %} + +{% block title %}Set up your account — Porchlight{% endblock %} + +{% block content %} +You're about to set up the account for {{ username }}.
+ + +{% endblock %} diff --git a/tests/e2e/full-flow.spec.js b/tests/e2e/full-flow.spec.js index be797d2..584078f 100644 --- a/tests/e2e/full-flow.spec.js +++ b/tests/e2e/full-flow.spec.js @@ -9,7 +9,9 @@ test.describe('Full user journey', () => { expect(fixtures.register_token).toBeTruthy(); // ---- Step 1: Register via magic link ---- + // GET shows a confirmation page; submitting it (POST) consumes the token. await page.goto(`/register/${fixtures.register_token}`); + await page.click('button[type="submit"]'); // Should redirect to /manage/credentials?setup=1 await page.waitForURL('**/manage/credentials?setup=1', { timeout: 5000 }); diff --git a/tests/test_auth_routes/test_register_magic_link.py b/tests/test_auth_routes/test_register_magic_link.py index 4a79408..7ff47ba 100644 --- a/tests/test_auth_routes/test_register_magic_link.py +++ b/tests/test_auth_routes/test_register_magic_link.py @@ -4,6 +4,7 @@ from httpx import AsyncClient from porchlight.authn.password import PasswordService from porchlight.invite.service import MagicLinkService from porchlight.models import PasswordCredential, User +from tests.conftest import get_csrf_token async def test_register_invalid_token_returns_error_page(client: AsyncClient) -> None: @@ -23,14 +24,33 @@ async def test_register_expired_token_returns_error_page(client: AsyncClient) -> assert "Invalid or expired" in res.text -async def test_register_valid_token_creates_user_and_redirects(client: AsyncClient) -> None: +async def test_register_get_does_not_consume_token(client: AsyncClient) -> None: + """GET is side-effect free: it shows a confirmation form and must not + consume the token or create a session.""" + app = client._transport.app # type: ignore[union-attr] + magic_link_service = app.state.magic_link_service + + link = await magic_link_service.create(username="newuser") + res = await client.get(f"/register/{link.token}", follow_redirects=False) + assert res.status_code == 200 + assert f'action="/register/{link.token}"' in res.text + # Token still valid (not consumed by the GET). + assert await magic_link_service.validate(link.token) is not None + + +async def test_register_post_creates_user_and_redirects(client: AsyncClient) -> None: app = client._transport.app # type: ignore[union-attr] magic_link_service = app.state.magic_link_service user_repo = app.state.user_repo link = await magic_link_service.create(username="newuser") - res = await client.get(f"/register/{link.token}", follow_redirects=False) + csrf = await get_csrf_token(client) + res = await client.post( + f"/register/{link.token}", + headers={"X-CSRF-Token": csrf}, + follow_redirects=False, + ) assert res.status_code in (302, 303) assert "/manage/credentials" in res.headers["location"] assert "setup=1" in res.headers["location"] @@ -44,6 +64,17 @@ async def test_register_valid_token_creates_user_and_redirects(client: AsyncClie assert "users" in user.groups +async def test_register_post_requires_csrf(client: AsyncClient) -> None: + app = client._transport.app # type: ignore[union-attr] + magic_link_service = app.state.magic_link_service + link = await magic_link_service.create(username="newuser") + + res = await client.post(f"/register/{link.token}", follow_redirects=False) + assert res.status_code == 403 + # Token must not have been consumed by the rejected request. + assert await magic_link_service.validate(link.token) is not None + + async def test_register_used_token_returns_error(client: AsyncClient) -> None: app = client._transport.app # type: ignore[union-attr] magic_link_service = app.state.magic_link_service @@ -67,7 +98,12 @@ async def test_register_existing_user_logs_in_and_redirects(client: AsyncClient) link = await magic_link_service.create(username="admin") - res = await client.get(f"/register/{link.token}", follow_redirects=False) + csrf = await get_csrf_token(client) + res = await client.post( + f"/register/{link.token}", + headers={"X-CSRF-Token": csrf}, + follow_redirects=False, + ) assert res.status_code in (302, 303) assert "/manage/credentials" in res.headers["location"] assert "setup=1" in res.headers["location"] @@ -96,7 +132,12 @@ async def test_register_rejects_user_that_already_has_credentials(client: AsyncC await cred_repo.create_password(PasswordCredential(user_id=user.userid, password_hash=svc.hash("existing-pass"))) link = await magic_link_service.create(username="hascreds") - res = await client.get(f"/register/{link.token}", follow_redirects=False) + csrf = await get_csrf_token(client) + res = await client.post( + f"/register/{link.token}", + headers={"X-CSRF-Token": csrf}, + follow_redirects=False, + ) # No passwordless login: rejected, not redirected to setup. assert res.status_code == 400 diff --git a/tests/test_authn_active.py b/tests/test_authn_active.py index 3564bb5..6126343 100644 --- a/tests/test_authn_active.py +++ b/tests/test_authn_active.py @@ -61,6 +61,11 @@ async def test_inactive_user_cannot_register_magic_link(client: AsyncClient) -> link = await magic_link_service.create(username="deactivated", created_by="admin", note="test") - response = await client.get(f"/register/{link.token}", follow_redirects=False) + csrf = await get_csrf_token(client) + response = await client.post( + f"/register/{link.token}", + headers={"X-CSRF-Token": csrf}, + follow_redirects=False, + ) assert response.status_code == 400 or "deactivated" in response.text.lower() or "Invalid" in response.text diff --git a/tests/test_password_change.py b/tests/test_password_change.py index 0e003e2..d9ffb5b 100644 --- a/tests/test_password_change.py +++ b/tests/test_password_change.py @@ -48,11 +48,15 @@ async def _login_user_without_password(client: AsyncClient) -> str: ) await user_repo.create(user) - # Simulate session login via magic link + # Simulate session login via magic link (GET shows the form, POST consumes) token = await get_csrf_token(client) magic_link_service = app.state.magic_link_service link = await magic_link_service.create(username="newuser", created_by="admin", note="test") - await client.get(f"/register/{link.token}", follow_redirects=False) + await client.post( + f"/register/{link.token}", + headers={"X-CSRF-Token": token}, + follow_redirects=False, + ) # Re-fetch CSRF token after session change token = await get_csrf_token(client) return token