diff --git a/src/porchlight/invite/service.py b/src/porchlight/invite/service.py index fd5bebf..b8b5ef8 100644 --- a/src/porchlight/invite/service.py +++ b/src/porchlight/invite/service.py @@ -1,3 +1,5 @@ +import hashlib +import hmac import secrets from datetime import UTC, datetime, timedelta @@ -6,11 +8,23 @@ from porchlight.store.protocols import MagicLinkRepository class MagicLinkService: - """Magic link token lifecycle management.""" + """Magic link token lifecycle management. - def __init__(self, repo: MagicLinkRepository, ttl: int = 86400) -> None: + The raw token is only ever exposed to the user (in the registration URL). + Only a hash of the token is persisted, so a database disclosure does not + yield usable tokens. A pepper, when configured, keys the hash (HMAC); + otherwise a plain SHA-256 of the high-entropy token is stored. + """ + + def __init__(self, repo: MagicLinkRepository, ttl: int = 86400, pepper: str | None = None) -> None: self._repo = repo self._ttl = ttl + self._pepper = pepper + + def _hash_token(self, token: str) -> str: + if self._pepper: + return hmac.new(self._pepper.encode(), token.encode(), hashlib.sha256).hexdigest() + return hashlib.sha256(token.encode()).hexdigest() async def create( self, @@ -18,34 +32,39 @@ class MagicLinkService: created_by: str | None = None, note: str | None = None, ) -> MagicLink: - """Generate and store a new magic link for the given username.""" + """Generate and store a new magic link for the given username. + + Persists only the hashed token; the returned MagicLink carries the raw + token so the caller can build the registration URL. + """ token = secrets.token_urlsafe(32) expires_at = datetime.now(UTC) + timedelta(seconds=self._ttl) - link = MagicLink( - token=token, + stored = MagicLink( + token=self._hash_token(token), username=username, expires_at=expires_at, created_by=created_by, note=note, ) - return await self._repo.create(link) + await self._repo.create(stored) + return stored.model_copy(update={"token": token}) async def validate(self, token: str) -> MagicLink | None: """Validate a magic link token. - Returns the MagicLink if it exists, is not used, and has not expired. - Returns None otherwise. + Returns the MagicLink (with the raw token re-attached) if it exists, is + not used, and has not expired. Returns None otherwise. """ - link = await self._repo.get_by_token(token) + link = await self._repo.get_by_token(self._hash_token(token)) if link is None or link.used: return None if link.expires_at < datetime.now(UTC): return None - return link + return link.model_copy(update={"token": token}) async def mark_used(self, token: str) -> bool: """Mark a magic link as used. Returns True if found and marked.""" - return await self._repo.mark_used(token) + return await self._repo.mark_used(self._hash_token(token)) async def cleanup_expired(self) -> int: """Delete expired unused links. Returns count deleted.""" diff --git a/tests/test_auth_routes/test_register_magic_link.py b/tests/test_auth_routes/test_register_magic_link.py index 1a03fed..2d4ed65 100644 --- a/tests/test_auth_routes/test_register_magic_link.py +++ b/tests/test_auth_routes/test_register_magic_link.py @@ -1,8 +1,7 @@ -from datetime import UTC, datetime, timedelta - from httpx import AsyncClient -from porchlight.models import MagicLink, User +from porchlight.invite.service import MagicLinkService +from porchlight.models import User async def test_register_invalid_token_returns_error_page(client: AsyncClient) -> None: @@ -13,42 +12,29 @@ async def test_register_invalid_token_returns_error_page(client: AsyncClient) -> async def test_register_expired_token_returns_error_page(client: AsyncClient) -> None: app = client._transport.app # type: ignore[union-attr] - repo = app.state.magic_link_repo - await repo.create( - MagicLink( - token="expired", - username="newuser", - expires_at=datetime.now(UTC) - timedelta(hours=1), - ) - ) + # An already-expired link (negative TTL), stored hashed like the real service. + expired_service = MagicLinkService(repo=app.state.magic_link_repo, ttl=-3600) + link = await expired_service.create(username="newuser") - res = await client.get("/register/expired", follow_redirects=False) + res = await client.get(f"/register/{link.token}", follow_redirects=False) assert res.status_code == 400 assert "Invalid or expired" in res.text async def test_register_valid_token_creates_user_and_redirects(client: AsyncClient) -> None: app = client._transport.app # type: ignore[union-attr] - magic_link_repo = app.state.magic_link_repo + magic_link_service = app.state.magic_link_service user_repo = app.state.user_repo - await magic_link_repo.create( - MagicLink( - token="t1", - username="newuser", - expires_at=datetime.now(UTC) + timedelta(hours=1), - ) - ) + link = await magic_link_service.create(username="newuser") - res = await client.get("/register/t1", follow_redirects=False) + res = await client.get(f"/register/{link.token}", follow_redirects=False) assert res.status_code in (302, 303) assert "/manage/credentials" in res.headers["location"] assert "setup=1" in res.headers["location"] - # Token should be marked used - link = await magic_link_repo.get_by_token("t1") - assert link is not None - assert link.used is True + # Token should be consumed (no longer valid) + assert await magic_link_service.validate(link.token) is None # User should exist user = await user_repo.get_by_username("newuser") @@ -58,48 +44,34 @@ async def test_register_valid_token_creates_user_and_redirects(client: AsyncClie async def test_register_used_token_returns_error(client: AsyncClient) -> None: app = client._transport.app # type: ignore[union-attr] - repo = app.state.magic_link_repo - await repo.create( - MagicLink( - token="used", - username="newuser", - expires_at=datetime.now(UTC) + timedelta(hours=1), - used=True, - ) - ) + magic_link_service = app.state.magic_link_service + link = await magic_link_service.create(username="newuser") + await magic_link_service.mark_used(link.token) - res = await client.get("/register/used", follow_redirects=False) + res = await client.get(f"/register/{link.token}", follow_redirects=False) assert res.status_code == 400 async def test_register_existing_user_logs_in_and_redirects(client: AsyncClient) -> None: - """When initial-admin creates a user, the invite link should log them in.""" + """When initial-admin creates a user (no credentials yet), the invite link + should let them set up their account.""" app = client._transport.app # type: ignore[union-attr] - magic_link_repo = app.state.magic_link_repo + magic_link_service = app.state.magic_link_service user_repo = app.state.user_repo - # Pre-create the user (as initial-admin would) + # Pre-create the user (as initial-admin would), with no credentials. user = User(userid="lusab-bansen", username="admin", groups=["admin", "users"]) await user_repo.create(user) - # Create invite for the same username - await magic_link_repo.create( - MagicLink( - token="admin-setup", - username="admin", - expires_at=datetime.now(UTC) + timedelta(hours=1), - ) - ) + link = await magic_link_service.create(username="admin") - res = await client.get("/register/admin-setup", follow_redirects=False) + res = await client.get(f"/register/{link.token}", follow_redirects=False) assert res.status_code in (302, 303) assert "/manage/credentials" in res.headers["location"] assert "setup=1" in res.headers["location"] - # Token should be marked used - link = await magic_link_repo.get_by_token("admin-setup") - assert link is not None - assert link.used is True + # Token should be consumed + assert await magic_link_service.validate(link.token) is None # Original user should still exist with original groups existing = await user_repo.get_by_username("admin") diff --git a/tests/test_invite/test_service.py b/tests/test_invite/test_service.py index 6464fce..e7a20bd 100644 --- a/tests/test_invite/test_service.py +++ b/tests/test_invite/test_service.py @@ -42,6 +42,18 @@ async def test_create_returns_magic_link(service: MagicLinkService) -> None: assert link.expires_at > datetime.now(UTC) +async def test_token_stored_hashed_not_plaintext(service: MagicLinkService, repo: SQLiteMagicLinkRepository) -> None: + # The raw token handed to the user must never be the value persisted in + # the store; a DB read must not yield a usable token. + link = await service.create(username="alice") + raw = link.token + assert await repo.get_by_token(raw) is None + # ...but the raw token must still validate through the service. + validated = await service.validate(raw) + assert validated is not None + assert validated.username == "alice" + + async def test_create_generates_unique_tokens(service: MagicLinkService) -> None: link1 = await service.create(username="alice") link2 = await service.create(username="bob")