diff --git a/CLAUDE.md b/CLAUDE.md index fb6e47c..8a1f50c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,13 +44,18 @@ make schema # regenerates openapi.json AND the TypeScript client ## Auth The frontend never uses session cookies against `/api/`. It gets a Google ID -token, trades it at `/_allauth/app/v1/auth/provider/token` for our own JWT pair, +token (or an emailed code - below), trades it at `/_allauth/app/v1/auth/provider/token` for our own JWT pair, and sends the access token as a bearer. Refresh tokens **rotate** — the new one must replace the old. Access tokens live 15 minutes. Session cookies still exist, but only for the Django admin and the server-rendered `/accounts/` allauth views. +**Email sign-in is ours, not allauth's** (`apps/accounts/email_login.py`, next-auth provider +`email`). One mail carries a code and a link that share one row, so using either spends both. It +is sent in the request, never queued, because it carries the raw secrets. Requesting answers 204 +for every address. The verify page acts on a button, never on load. + Google credentials come from environment variables, **not** a `SocialApp` row in the admin — configuring both raises `MultipleObjectsReturned`. diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 9f080a0..a970da9 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -120,9 +120,39 @@ access, at the next request. Signing uses `JWT_SIGNING_KEY`, falling back to `SE A browser redirect flow (`/_allauth/app/v1/auth/provider/redirect`) and the server-rendered `/accounts/` views are also available; the latter is how you sign into the Django admin with Google. -Email/password and magic-link login are deliberately switched off for now -(`SOCIALACCOUNT_ONLY = True` in `config/settings/base.py`); allauth already ships both, so enabling -them later is a settings change plus frontend work. +### Signing in by email + +Readers without Google sign in by email. One message carries both a six-digit code and a link, and +either one signs in. The first sign-in creates the account (and sends `welcome`). The endpoints are +ours (`apps/accounts/email_login.py`), not allauth's: allauth's login-by-code is bound to the +`X-Session-Token` of the browser that asked, so its link could not finish on a phone a sign-in +started on a laptop. allauth's own local login stays off (`SOCIALACCOUNT_ONLY = True`). + +``` +# 1. mail a code and a link - 204 whatever the address, so the form reveals no accounts +POST /api/v1/auth/email-login { "email": "...", "next": "/tools/acme" } + +# 2a. the code, typed on /auth/signin 2b. the link, /auth/email/verify?token=...&next=... +POST /api/v1/auth/email-login/confirm { "email": "...", "code": "042917" } | { "token": "..." } +-> { "access_token", "refresh_token", "token_type", "expires_in", "user": {...} } +``` + +The pair is the same one Google's exchange returns, so refreshing it is step 3 above. On the +frontend it is a next-auth `Credentials` provider with id `email`: its `authorize` makes the confirm +call, and `resolveAuthToken` keeps the pair. + +What bites: + +- **The code and link share one row**, stored as SHA-256 only: using one spends the other, a new + request spends the previous mail's, and the row dies after 15 minutes or 5 wrong codes. +- **The mail is sent in the request, never queued**, like `claim_code`. +- **A repeat within 60 seconds sends nothing** and still answers 204. Both endpoints are also + throttled per IP (`EMAIL_LOGIN_REQUEST_RATE`, `EMAIL_LOGIN_CONFIRM_RATE`). +- **The verify page signs in on a button, never on load**, because mail scanners open links. +- **Signing in records a verified allauth `EmailAddress`.** That is what lets a later Google + sign-in with the same address land on the same account. +- **`next` is a path on this site or nothing.** The backend falls back to `/account`, and + `landingPath` (`src/lib/auth/landing.ts`) strips the origin that next-auth's `signIn()` adds. ## Adding an endpoint @@ -291,7 +321,8 @@ production. Tests always use the in-memory backend. | Email | Sent when | To | | --- | --- | --- | -| `welcome` | allauth's `user_signed_up` - a first Google sign-in, not an account made in the admin | the new user | +| `welcome` | allauth's `user_signed_up` - a first Google or email sign-in, not an account made in the admin | the new user | +| `login_link` | someone asks to sign in by email (`POST /auth/email-login`) | that address | | `submission_received` | a tool is submitted (`POST /catalog/submissions`) | `contact_email`, else the account | | `submission_staff_notice` | the same, with a link to the row in the admin | every active `is_staff` account | | `claim_code` | a vendor claims a listing | the claimant's work email | @@ -315,8 +346,8 @@ re-queued with the admin's **Re-score** action, `services.rescore`. It runs only submission that is scoring, failed or awaiting review, and settles it - emails included - like any other. -`claim_code` and `claim_invite` are sent inside the request, never queued: a task's arguments are -kept in the database, and both carry a credential the server otherwise stores only as a hash. All +`claim_code`, `claim_invite` and `login_link` are sent inside the request, never queued: a task's arguments are +kept in the database, and each carries a credential the server otherwise stores only as a hash. All the others go through the django-q2 queue after the transaction commits, so a sign-in or a submission never waits on - or fails with - the mail provider, and a rolled-back request mails nobody. On the host that means they only leave while diff --git a/backend/.env.example b/backend/.env.example index c86ecb8..727b70b 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -98,6 +98,9 @@ PUBLIC_MEDIA_URL=http://localhost:8007/media # MAX_API_KEYS_PER_USER=10 # Per-IP rate at which `pdfredeval login` may open a sign-in. # CLI_LOGIN_START_RATE=20/hour +# Per-IP rates for signing in by email: requesting a code and link, and using one. +# EMAIL_LOGIN_REQUEST_RATE=10/hour +# EMAIL_LOGIN_CONFIRM_RATE=30/hour # --- Staff MCP server ----------------------------------------------------- # /mcp, with its OAuth authorization server at /oauth/. No required variables; diff --git a/backend/apps/accounts/api.py b/backend/apps/accounts/api.py index 7ecd5bf..f2838ac 100644 --- a/backend/apps/accounts/api.py +++ b/backend/apps/accounts/api.py @@ -1,7 +1,8 @@ """API-side authentication: the bearer token scheme and the `/auth` router. -Obtaining tokens is allauth's job (`/_allauth/app/v1/...`, including -`auth/tokens/refresh`); this module only consumes them. +Obtaining tokens is mostly allauth's job (`/_allauth/app/v1/...`, including +`auth/tokens/refresh`). The exception is signing in by email (`email_login.py`), +which ends here in the same token pair. """ from django.conf import settings @@ -13,7 +14,7 @@ from ninja_apikey.models import APIKey from ninja_apikey.security import APIKeyAuth, generate_key -from . import cli_login, jwt, mcp_oauth +from . import cli_login, email_login, jwt, mcp_oauth from .schemas import ( ApiKeyCreatedOut, ApiKeyIn, @@ -23,6 +24,9 @@ CliLoginStartOut, CliLoginTokenIn, CliLoginTokenOut, + EmailLoginConfirmIn, + EmailLoginIn, + EmailLoginTokenOut, McpAuthorizationDecisionIn, McpAuthorizationDecisionOut, McpAuthorizationOut, @@ -228,6 +232,55 @@ def deny_cli_login(request: HttpRequest, user_code: str): raise _cli_error(exc) from exc +# --- Signing in by email ---------------------------------------------------------- +# A code and a link in one message: see apps/accounts/email_login.py. + + +class EmailLoginRequestThrottle(AnonRateThrottle): + scope = "email_login_request" + + def __init__(self): + super().__init__(settings.EMAIL_LOGIN_REQUEST_RATE) + + +class EmailLoginConfirmThrottle(AnonRateThrottle): + scope = "email_login_confirm" + + def __init__(self): + super().__init__(settings.EMAIL_LOGIN_CONFIRM_RATE) + + +@router.post( + "/email-login", + response={204: None}, + throttle=[EmailLoginRequestThrottle()], + summary="Email a sign-in code and link", +) +def request_email_login(request: HttpRequest, payload: EmailLoginIn): + try: + email_login.request_login(payload.email, payload.next) + except email_login.InvalidAddress as exc: + raise HttpError(422, str(exc)) from exc + return Status(204, None) + + +@router.post( + "/email-login/confirm", + response=EmailLoginTokenOut, + throttle=[EmailLoginConfirmThrottle()], + summary="Sign in with an emailed code or link", +) +def confirm_email_login(request: HttpRequest, payload: EmailLoginConfirmIn): + try: + if payload.token: + user = email_login.confirm_with_link(payload.token) + else: + user = email_login.confirm_with_code(payload.email, payload.code) + except email_login.EmailLoginError as exc: + raise HttpError(400, str(exc)) from exc + return {**jwt.issue_token_pair(user), "user": user} + + # --- MCP connector consent --------------------------------------------------------- # The authorize step of the /mcp OAuth flow: see apps/accounts/mcp_oauth.py. Staff # only, because a code for anyone else mints a token the MCP server refuses anyway. diff --git a/backend/apps/accounts/email_login.py b/backend/apps/accounts/email_login.py new file mode 100644 index 0000000..24e25d2 --- /dev/null +++ b/backend/apps/accounts/email_login.py @@ -0,0 +1,151 @@ +"""Signing in by email: one message with a six-digit code and a link. + +Transport-free, like `cli_login.py`: the routes in `api.py` call these and map +`EmailLoginError` to HTTP. A successful sign-in ends in the same JWT pair Google's +does (`jwt.issue_token_pair`), so the frontend refreshes both the same way. + +The mail is sent in the request, never queued: django-q keeps task arguments in the +database, which would store the raw code and link. +""" + +import hashlib +import secrets +from datetime import timedelta +from urllib.parse import urlencode + +from allauth.account.models import EmailAddress +from allauth.account.signals import user_signed_up +from django.conf import settings +from django.contrib.auth import get_user_model +from django.core.exceptions import ValidationError +from django.core.validators import validate_email +from django.db import transaction +from django.utils import timezone + +from apps.accounts.models import EmailLogin +from apps.core.email import send_templated_email + +LIFETIME = timedelta(minutes=15) +# Wrong codes before the row is dead. Six digits against five guesses is 1 in 200,000. +MAX_ATTEMPTS = 5 +# Per address, between mails: what stops the form being used to flood one inbox. +COOLDOWN = timedelta(seconds=60) +DEFAULT_NEXT = "/account" + + +class InvalidAddress(Exception): + """The address is malformed - the one refusal requesting may give.""" + + +class EmailLoginError(Exception): + """Any failure to sign in. One message for all of them, so none leaks which.""" + + def __init__(self): + super().__init__("That code or link is wrong or has expired. Request a new one.") + + +def request_login(email, next_path=""): + """Mail a code and a link to `email`. Says nothing about whether it has an account.""" + email = email.strip().lower() + try: + validate_email(email) + except ValidationError as exc: + raise InvalidAddress("Enter a valid email address.") from exc + recent = timezone.now() - COOLDOWN + if EmailLogin.objects.filter(email=email, created_at__gt=recent).exists(): + return + if get_user_model().objects.filter(email__iexact=email, is_active=False).exists(): + return + code = f"{secrets.randbelow(1_000_000):06d}" + link = secrets.token_urlsafe(32) + # One transaction with the send: a mail that never left must not start the cooldown. + with transaction.atomic(): + # Only the newest mail works: an older one left in an inbox is one more to steal. + EmailLogin.objects.filter(email=email, used_at__isnull=True).update(used_at=timezone.now()) + EmailLogin.objects.create( + email=email, + code_hash=_hash(code), + link_hash=_hash(link), + expires_at=timezone.now() + LIFETIME, + ) + send_templated_email( + "login_link", + { + "code": code, + "link": _link_url(link, next_path), + "ttl_minutes": int(LIFETIME.total_seconds() // 60), + }, + to=[email], + ) + + +def confirm_with_code(email, code): + """Sign in with the code. Returns the user or raises `EmailLoginError`.""" + with transaction.atomic(): + login = _open().filter(email=email.strip().lower(), attempts__lt=MAX_ATTEMPTS).first() + if login is not None and secrets.compare_digest(login.code_hash, _hash(code.strip())): + return _sign_in(login) + if login is not None: + # Committed, not raised inside the transaction: a rollback would undo the count. + login.attempts += 1 + login.save(update_fields=["attempts"]) + raise EmailLoginError() + + +def confirm_with_link(token): + """Sign in with the link's token, on any device. Same return as `confirm_with_code`.""" + with transaction.atomic(): + login = _open().filter(link_hash=_hash(token.strip())).first() + if login is None: + raise EmailLoginError() + return _sign_in(login) + + +def _open(): + """Rows that can still sign someone in, locked for the rest of the transaction.""" + return EmailLogin.objects.select_for_update().filter( + used_at__isnull=True, expires_at__gt=timezone.now() + ) + + +def _sign_in(login): + user = get_user_model().objects.filter(email__iexact=login.email).first() + if user is not None and not user.is_active: + raise EmailLoginError() + login.used_at = timezone.now() + login.save(update_fields=["used_at"]) + if user is None: + user = get_user_model().objects.create_user(login.email) + # The signal a Google sign-up sends, so the welcome mail goes out the same way. + user_signed_up.send(sender=type(user), request=None, user=user) + _mark_verified(user) + return user + + +def _mark_verified(user): + """Record what the mail just proved. allauth connects a later Google sign-in with the + same address to this account only through a verified `EmailAddress`.""" + address = EmailAddress.objects.filter(user=user, email__iexact=user.email).first() + if address is None: + address = EmailAddress(user=user, email=user.email) + address.verified = True + if not EmailAddress.objects.filter(user=user, primary=True).exclude(pk=address.pk).exists(): + address.primary = True + address.save() + + +def _link_url(link, next_path): + query = urlencode({"token": link, "next": _safe_next(next_path)}) + return f"{settings.FRONTEND_URL}/auth/email/verify?{query}" + + +def _safe_next(path): + """A path on this site, or the account page: the link must not bounce anyone elsewhere.""" + path = (path or "").strip() + if not path.startswith("/") or path.startswith(("//", "/\\")): + return DEFAULT_NEXT + return path + + +def _hash(secret): + return hashlib.sha256(secret.encode()).hexdigest() diff --git a/backend/apps/accounts/migrations/0003_email_login.py b/backend/apps/accounts/migrations/0003_email_login.py new file mode 100644 index 0000000..b921077 --- /dev/null +++ b/backend/apps/accounts/migrations/0003_email_login.py @@ -0,0 +1,30 @@ +# Generated by Django 6.1.1 on 2026-10-08 19:59 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('accounts', '0002_cli_login'), + ] + + operations = [ + migrations.CreateModel( + name='EmailLogin', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('email', models.EmailField(db_index=True, max_length=254)), + ('code_hash', models.CharField(max_length=64)), + ('link_hash', models.CharField(max_length=64, unique=True)), + ('attempts', models.PositiveSmallIntegerField(default=0)), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('expires_at', models.DateTimeField()), + ('used_at', models.DateTimeField(blank=True, null=True)), + ], + options={ + 'db_table': 'accounts_email_login', + 'ordering': ['-created_at'], + }, + ), + ] diff --git a/backend/apps/accounts/models.py b/backend/apps/accounts/models.py index fd64a91..b398566 100644 --- a/backend/apps/accounts/models.py +++ b/backend/apps/accounts/models.py @@ -110,3 +110,27 @@ def __str__(self): @property def expired(self): return self.expires_at <= timezone.now() + + +class EmailLogin(models.Model): + """One emailed sign-in: a six-digit code and a link, either of which signs in once. + + Both secrets are stored only as SHA-256. They share the row, so using one spends the + other. Nothing ties the row to the browser that asked, which is what lets the link + finish on a phone a sign-in started on a laptop. + """ + + email = models.EmailField(db_index=True) + code_hash = models.CharField(max_length=64) + link_hash = models.CharField(max_length=64, unique=True) + attempts = models.PositiveSmallIntegerField(default=0) + created_at = models.DateTimeField(auto_now_add=True) + expires_at = models.DateTimeField() + used_at = models.DateTimeField(null=True, blank=True) + + class Meta: + db_table = "accounts_email_login" + ordering = ["-created_at"] + + def __str__(self): + return f"{self.email} ({self.created_at:%Y-%m-%d %H:%M})" diff --git a/backend/apps/accounts/schemas.py b/backend/apps/accounts/schemas.py index 5653cb6..10212af 100644 --- a/backend/apps/accounts/schemas.py +++ b/backend/apps/accounts/schemas.py @@ -107,3 +107,25 @@ class McpAuthorizationDecisionIn(Schema): class McpAuthorizationDecisionOut(Schema): #: The client's redirect URI with the code or the refusal on it. Send the browser there. redirect_url: str + + +class EmailLoginIn(Schema): + email: str = Field(min_length=3, max_length=254) + #: Where the link lands after signing in. A path on this site, or it is ignored. + next: str = Field(default="", max_length=500) + + +class EmailLoginConfirmIn(Schema): + """Either `email` and `code`, as typed on the sign-in page, or the link's `token`.""" + + email: str = "" + code: str = "" + token: str = "" + + +class EmailLoginTokenOut(Schema): + access_token: str + refresh_token: str + token_type: str + expires_in: int + user: UserSchema diff --git a/backend/apps/core/templates/email/claim_invite/body.html b/backend/apps/core/templates/email/claim_invite/body.html index 7dfa056..073d20d 100644 --- a/backend/apps/core/templates/email/claim_invite/body.html +++ b/backend/apps/core/templates/email/claim_invite/body.html @@ -147,7 +147,7 @@ -
Sign in with Google, then accept. As a maintainer you can propose corrections to the listing's pricing and details, and the listing shows that its vendor maintains it.
+
Sign in, then accept. As a maintainer you can propose corrections to the listing's pricing and details, and the listing shows that its vendor maintains it.
diff --git a/backend/apps/core/templates/email/claim_invite/body.mjml b/backend/apps/core/templates/email/claim_invite/body.mjml index 5ecce45..0c2af58 100644 --- a/backend/apps/core/templates/email/claim_invite/body.mjml +++ b/backend/apps/core/templates/email/claim_invite/body.mjml @@ -10,7 +10,7 @@ Our team has invited you to maintain the {{ tool_name }} listing on Redaction Tools. Accept the invitation - Sign in with Google, then accept. As a maintainer you can propose corrections to the listing's pricing and details, and the listing shows that its vendor maintains it. + Sign in, then accept. As a maintainer you can propose corrections to the listing's pricing and details, and the listing shows that its vendor maintains it. The link works once, for whoever accepts it first, so please do not forward it. If you were not expecting this, you can ignore it. diff --git a/backend/apps/core/templates/email/claim_invite/body.txt b/backend/apps/core/templates/email/claim_invite/body.txt index c24fdf3..ef155eb 100644 --- a/backend/apps/core/templates/email/claim_invite/body.txt +++ b/backend/apps/core/templates/email/claim_invite/body.txt @@ -3,7 +3,7 @@ Accept here: {{ url }} -Sign in with Google, then accept. As a maintainer you can propose corrections to the listing's pricing and details, and the listing shows that its vendor maintains it. +Sign in, then accept. As a maintainer you can propose corrections to the listing's pricing and details, and the listing shows that its vendor maintains it. The link works once, for whoever accepts it first, so please do not forward it. If you were not expecting this, you can ignore it. diff --git a/backend/apps/core/templates/email/login_link/body.html b/backend/apps/core/templates/email/login_link/body.html new file mode 100644 index 0000000..f2c1099 --- /dev/null +++ b/backend/apps/core/templates/email/login_link/body.html @@ -0,0 +1,212 @@ + + + + + Sign in to Redaction Tools + + + + + + + + + + + + + + + + + +
Your sign-in code is {{ code }}
+
+ +
+ + + + + + +
+ +
+ + + + + + +
+ + + + + + +
+ +
+
+
+ +
+
+ +
+ + + + + + +
+ +
+ + + + + + +
+ + + + + + + + + + + + + + + + + + +
+
Click the button to sign in to Redaction Tools:
+
+ + + + + + +
+ Sign in +
+
+
Or enter this code on the sign-in page:
+
+
{{ code }}
+
+
The link and the code expire in {{ ttl_minutes }} minutes, and either one works once. If you did not ask to sign in, you can ignore this email.
+
+
+
+ +
+
+ +
+ + + + + + +
+ +
+ + + + + + +
+ + + + + + +
+
You received this because of an action on Redaction Tools.
+
+
+
+ +
+
+ +
+ + + \ No newline at end of file diff --git a/backend/apps/core/templates/email/login_link/body.mjml b/backend/apps/core/templates/email/login_link/body.mjml new file mode 100644 index 0000000..3507c0e --- /dev/null +++ b/backend/apps/core/templates/email/login_link/body.mjml @@ -0,0 +1,20 @@ + + + Sign in to Redaction Tools + Your sign-in code is {{ code }} + + + + + + + Click the button to sign in to Redaction Tools: + Sign in + Or enter this code on the sign-in page: + {{ code }} + The link and the code expire in {{ ttl_minutes }} minutes, and either one works once. If you did not ask to sign in, you can ignore this email. + + + + + diff --git a/backend/apps/core/templates/email/login_link/body.txt b/backend/apps/core/templates/email/login_link/body.txt new file mode 100644 index 0000000..1232429 --- /dev/null +++ b/backend/apps/core/templates/email/login_link/body.txt @@ -0,0 +1,11 @@ +{% autoescape off %}Sign in to Redaction Tools with this link: +{{ link }} + +Or enter this code on the sign-in page: {{ code }} + +The link and the code expire in {{ ttl_minutes }} minutes, and either one works once. If you did not ask to sign in, you can ignore this email. + +-- +Redaction Tools +{{ site_url }} +{% endautoescape %} diff --git a/backend/apps/core/templates/email/login_link/preview.json b/backend/apps/core/templates/email/login_link/preview.json new file mode 100644 index 0000000..26bd572 --- /dev/null +++ b/backend/apps/core/templates/email/login_link/preview.json @@ -0,0 +1,5 @@ +{ + "code": "042917", + "link": "https://redaction-tools.com/auth/email/verify?token=preview-token", + "ttl_minutes": 15 +} diff --git a/backend/apps/core/templates/email/login_link/subject.txt b/backend/apps/core/templates/email/login_link/subject.txt new file mode 100644 index 0000000..f7e4901 --- /dev/null +++ b/backend/apps/core/templates/email/login_link/subject.txt @@ -0,0 +1 @@ +{% autoescape off %}Your Redaction Tools sign-in code: {{ code }}{% endautoescape %} diff --git a/backend/config/settings/base.py b/backend/config/settings/base.py index 3909c44..3b1ce06 100644 --- a/backend/config/settings/base.py +++ b/backend/config/settings/base.py @@ -139,14 +139,16 @@ LOGIN_URL = "/accounts/login/" LOGIN_REDIRECT_URL = "/admin/" -# allauth: email is the identifier, Google is the only provider for now. +# allauth: email is the identifier, Google its only provider. Signing in by email is +# ours, not allauth's (apps/accounts/email_login.py): allauth's login-by-code is bound to +# the browser that asked, so its link could not finish on another device. ACCOUNT_LOGIN_METHODS = {"email"} ACCOUNT_SIGNUP_FIELDS = ["email*"] ACCOUNT_USER_MODEL_USERNAME_FIELD = None ACCOUNT_EMAIL_VERIFICATION = "none" # Google hands us verified addresses ACCOUNT_UNIQUE_EMAIL = True ACCOUNT_PREVENT_ENUMERATION = True -SOCIALACCOUNT_ONLY = True # flip off when magic link / password login lands +SOCIALACCOUNT_ONLY = True # allauth's own local login stays off; see above SOCIALACCOUNT_STORE_TOKENS = False SOCIALACCOUNT_EMAIL_AUTHENTICATION = True SOCIALACCOUNT_EMAIL_AUTHENTICATION_AUTO_CONNECT = True @@ -300,6 +302,10 @@ MAX_API_KEYS_PER_USER = env.int("MAX_API_KEYS_PER_USER", default=10) # Per IP: how often `pdfredeval login` may open a sign-in. It needs no account. CLI_LOGIN_START_RATE = env("CLI_LOGIN_START_RATE", default="20/hour") +# Per IP. Signing in by email needs no account: requesting mails whatever address it +# names, and confirming is where a six-digit code would be guessed. +EMAIL_LOGIN_REQUEST_RATE = env("EMAIL_LOGIN_REQUEST_RATE", default="10/hour") +EMAIL_LOGIN_CONFIRM_RATE = env("EMAIL_LOGIN_CONFIRM_RATE", default="30/hour") # --- Comments -------------------------------------------------------------- # Per account. Login is the primary anti-spam control; this stops one account diff --git a/backend/openapi.json b/backend/openapi.json index a3f7dc8..8e320b3 100644 --- a/backend/openapi.json +++ b/backend/openapi.json @@ -349,6 +349,63 @@ ] } }, + "/api/v1/auth/email-login": { + "post": { + "operationId": "request_email_login", + "summary": "Email a sign-in code and link", + "parameters": [], + "responses": { + "204": { + "description": "No Content" + } + }, + "tags": [ + "auth" + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/EmailLoginIn" + } + } + }, + "required": true + } + } + }, + "/api/v1/auth/email-login/confirm": { + "post": { + "operationId": "confirm_email_login", + "summary": "Sign in with an emailed code or link", + "parameters": [], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/EmailLoginTokenOut" + } + } + } + } + }, + "tags": [ + "auth" + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/EmailLoginConfirmIn" + } + } + }, + "required": true + } + } + }, "/api/v1/auth/mcp-authorizations": { "get": { "operationId": "get_mcp_authorization", @@ -4018,6 +4075,81 @@ "title": "CliLoginOut", "type": "object" }, + "EmailLoginIn": { + "properties": { + "email": { + "maxLength": 254, + "minLength": 3, + "title": "Email", + "type": "string" + }, + "next": { + "default": "", + "maxLength": 500, + "title": "Next", + "type": "string" + } + }, + "required": [ + "email" + ], + "title": "EmailLoginIn", + "type": "object" + }, + "EmailLoginTokenOut": { + "properties": { + "access_token": { + "title": "Access Token", + "type": "string" + }, + "refresh_token": { + "title": "Refresh Token", + "type": "string" + }, + "token_type": { + "title": "Token Type", + "type": "string" + }, + "expires_in": { + "title": "Expires In", + "type": "integer" + }, + "user": { + "$ref": "#/components/schemas/UserSchema" + } + }, + "required": [ + "access_token", + "refresh_token", + "token_type", + "expires_in", + "user" + ], + "title": "EmailLoginTokenOut", + "type": "object" + }, + "EmailLoginConfirmIn": { + "description": "Either `email` and `code`, as typed on the sign-in page, or the link's `token`.", + "properties": { + "email": { + "default": "", + "title": "Email", + "type": "string" + }, + "code": { + "default": "", + "title": "Code", + "type": "string" + }, + "token": { + "default": "", + "title": "Token", + "type": "string" + } + }, + "title": "EmailLoginConfirmIn", + "type": "object" + }, "McpAuthorizationParams": { "description": "An OAuth authorization request, passed through from the consent page's URL.\n\nAll optional: checking them is the service's job, so a missing one is reported\nthe way OAuth says rather than as a 422.", "properties": { diff --git a/backend/tests/test_email_login.py b/backend/tests/test_email_login.py new file mode 100644 index 0000000..504980b --- /dev/null +++ b/backend/tests/test_email_login.py @@ -0,0 +1,247 @@ +"""Signing in by email: one message carries both a six-digit code and a link. + +Either one signs the reader in, on any device - the link is not bound to the browser +that asked for it. A first sign-in creates the account. Requesting answers 204 for +every address, so the form never says who has an account. +""" + +import re +from datetime import timedelta + +import pytest +from django.core import mail +from django.utils import timezone + +from apps.accounts.models import EmailLogin + +pytestmark = pytest.mark.django_db + +BASE = "/api/v1/auth/email-login" + + +@pytest.fixture(autouse=True) +def clear_throttle_cache(): + from django.core.cache import cache + + cache.clear() + + +def _request(client, email="reader@example.com", **extra): + return client.post(BASE, {"email": email, **extra}, content_type="application/json") + + +def _sent_code(message): + return re.search(r"\b(\d{6})\b", message.body).group(1) + + +def _sent_token(message): + return re.search(r"/auth/email/verify\?token=([\w-]+)", message.body).group(1) + + +def test_requesting_mails_both_a_code_and_a_link(client, settings): + settings.FRONTEND_URL = "https://redaction-tools.com" + + response = _request(client) + + assert response.status_code == 204 + assert len(mail.outbox) == 1 + message = mail.outbox[0] + assert message.to == ["reader@example.com"] + assert _sent_code(message) + assert "https://redaction-tools.com/auth/email/verify?token=" in message.body + + +def test_the_code_and_link_are_stored_only_as_hashes(client): + _request(client) + message = mail.outbox[0] + + login = EmailLogin.objects.get() + assert _sent_code(message) not in (login.code_hash, login.link_hash) + assert _sent_token(message) not in (login.code_hash, login.link_hash) + + +def _confirm(client, **payload): + return client.post(f"{BASE}/confirm", payload, content_type="application/json") + + +def test_the_code_signs_a_new_reader_in_and_creates_their_account(client): + _request(client, "New.Reader@Example.com") + code = _sent_code(mail.outbox[0]) + + response = _confirm(client, email="new.reader@example.com", code=code) + + assert response.status_code == 200 + body = response.json() + me = client.get("/api/v1/auth/me", headers={"Authorization": f"Bearer {body['access_token']}"}) + assert me.json()["email"] == "new.reader@example.com" + assert body["refresh_token"] + # For the session's own name and email: the frontend has no id_token to read them from. + assert body["user"]["email"] == "new.reader@example.com" + + +def test_the_link_signs_in_and_spends_the_code_with_it(client): + _request(client) + message = mail.outbox[0] + + first = _confirm(client, token=_sent_token(message)) + again = _confirm(client, email="reader@example.com", code=_sent_code(message)) + + assert first.status_code == 200 + assert again.status_code == 400 + + +def test_an_expired_code_and_link_are_refused(client): + _request(client) + message = mail.outbox[0] + EmailLogin.objects.update(expires_at=timezone.now() - timedelta(seconds=1)) + + assert _confirm(client, email="reader@example.com", code=_sent_code(message)).status_code == 400 + assert _confirm(client, token=_sent_token(message)).status_code == 400 + + +def test_five_wrong_codes_spend_the_sign_in(client): + _request(client) + code = _sent_code(mail.outbox[0]) + wrong = f"{(int(code) + 1) % 1_000_000:06d}" + + for _ in range(5): + assert _confirm(client, email="reader@example.com", code=wrong).status_code == 400 + + assert _confirm(client, email="reader@example.com", code=code).status_code == 400 + + +def test_a_new_request_spends_the_earlier_link(client): + _request(client) + EmailLogin.objects.update(created_at=timezone.now() - timedelta(minutes=5)) + _request(client) + + first, second = mail.outbox + assert _confirm(client, token=_sent_token(first)).status_code == 400 + assert _confirm(client, token=_sent_token(second)).status_code == 200 + + +def test_a_repeat_within_the_cooldown_sends_nothing_and_still_answers_204(client): + _request(client) + + response = _request(client) + + assert response.status_code == 204 + assert len(mail.outbox) == 1 + assert _confirm(client, token=_sent_token(mail.outbox[0])).status_code == 200 + + +def test_an_inactive_account_gets_no_mail_and_the_same_204(client, user): + user.is_active = False + user.save() + + response = _request(client, user.email) + + assert response.status_code == 204 + assert mail.outbox == [] + + +def test_an_existing_account_is_signed_in_whatever_the_case_of_its_address(client): + from apps.accounts.models import User + + existing = User.objects.create_user("Jane.Doe@example.com", name="Jane") + _request(client, "jane.doe@example.com") + + response = _confirm(client, token=_sent_token(mail.outbox[0])) + + me = client.get( + "/api/v1/auth/me", headers={"Authorization": f"Bearer {response.json()['access_token']}"} + ) + assert me.json()["id"] == str(existing.pk) + assert User.objects.filter(email__iexact="jane.doe@example.com").count() == 1 + + +def test_an_account_deactivated_after_the_mail_cannot_use_it(client, user): + _request(client, user.email) + user.is_active = False + user.save() + + assert _confirm(client, token=_sent_token(mail.outbox[0])).status_code == 400 + + +def test_a_first_sign_in_is_welcomed_and_a_returning_one_is_not( + client, user, django_capture_on_commit_callbacks +): + _request(client, "new@example.com") + _request(client, user.email) + new_mail, returning_mail = mail.outbox + + with django_capture_on_commit_callbacks(execute=True): + _confirm(client, token=_sent_token(new_mail)) + _confirm(client, token=_sent_token(returning_mail)) + + welcomes = [m for m in mail.outbox if m.subject == "Welcome to Redaction Tools"] + assert [m.to for m in welcomes] == [["new@example.com"]] + + +def test_signing_in_marks_the_address_verified_for_a_later_google_sign_in(client): + """allauth connects a Google login to an account only through a verified address.""" + from allauth.account.models import EmailAddress + + _request(client, "new@example.com") + _confirm(client, token=_sent_token(mail.outbox[0])) + + address = EmailAddress.objects.get(email="new@example.com") + assert address.verified and address.primary + assert address.user.email == "new@example.com" + + +@pytest.mark.parametrize( + ("asked", "landed"), + [ + ("/tools/acme?tab=pricing", "/tools/acme?tab=pricing"), + ("", "/account"), + ("https://evil.example/", "/account"), + ("//evil.example/", "/account"), + ("/\\evil.example/", "/account"), + ], +) +def test_the_link_lands_back_where_it_was_asked_from_but_only_on_this_site(client, asked, landed): + """Carried in the link, so the verify page knows where to go before it signs in.""" + from urllib.parse import parse_qs, urlparse + + _request(client, next=asked) + + link = re.search(r"https?://\S+/auth/email/verify\S+", mail.outbox[0].body).group(0) + assert parse_qs(urlparse(link).query)["next"] == [landed] + + +def test_requesting_is_throttled_per_ip(client): + """At the default 10/hour: the throttle reads its rate once, at import.""" + for n in range(10): + assert _request(client, f"r{n}@example.com").status_code == 204 + + assert _request(client, "one-more@example.com").status_code == 429 + + +def test_guessing_is_throttled_per_ip_across_addresses(client): + """At the default 30/hour: the per-row limit alone would let one IP spread guesses.""" + for n in range(30): + assert _confirm(client, email=f"r{n}@example.com", code="000000").status_code == 400 + + assert _confirm(client, email="r0@example.com", code="000000").status_code == 429 + + +def test_an_address_that_is_not_one_is_refused(client): + response = _request(client, "not-an-address") + + assert response.status_code == 422 + assert mail.outbox == [] + + +def test_a_mail_that_fails_to_send_leaves_nothing_to_wait_out(monkeypatch): + from apps.accounts import email_login + + def fail(*args, **kwargs): + raise ConnectionError("mail provider down") + + monkeypatch.setattr(email_login, "send_templated_email", fail) + + with pytest.raises(ConnectionError): + email_login.request_login("reader@example.com") + + assert not EmailLogin.objects.exists() diff --git a/frontend/src/app/auth/email/verify/page.tsx b/frontend/src/app/auth/email/verify/page.tsx new file mode 100644 index 0000000..3a201fa --- /dev/null +++ b/frontend/src/app/auth/email/verify/page.tsx @@ -0,0 +1,67 @@ +import type { Metadata } from 'next' +import { AuthError } from 'next-auth' +import Link from 'next/link' +import { redirect } from 'next/navigation' + +import { signIn } from '@/auth' +import { Button } from '@/components/ui/button' +import { Card, CardDescription, CardTitle } from '@/components/ui/card' +import { landingPath } from '@/lib/auth/landing' + +export const metadata: Metadata = { + title: 'Sign in', + robots: { index: false, follow: false }, + // The token in the query is the whole credential: a click out of this page + // must not hand it to the next site in a Referer header. + referrer: 'no-referrer', +} + +/* + * Signs in on the button, never on load: mail scanners open links before people + * do, and a link they could spend would leave the reader with a dead one. + */ +export default async function EmailLinkPage({ searchParams }: PageProps<'/auth/email/verify'>) { + const { token, next } = await searchParams + + if (typeof token !== 'string' || !token) { + return ( + + This link is incomplete + + Open it straight from the email, or ask for a new one. + + + + ) + } + + const redirectTo = landingPath(next) + + return ( + + Sign in to Redaction Tools + + Continue to finish signing in. The link works once. + +
{ + 'use server' + try { + await signIn('email', { token, redirectTo }) + } catch (error) { + // Success also throws - Next's redirect - and that must propagate. + if (error instanceof AuthError) redirect('/auth/error?error=EmailLinkInvalid') + throw error + } + }} + > + +
+ + ) +} diff --git a/frontend/src/app/auth/error/page.tsx b/frontend/src/app/auth/error/page.tsx index cee0582..f0400bc 100644 --- a/frontend/src/app/auth/error/page.tsx +++ b/frontend/src/app/auth/error/page.tsx @@ -9,6 +9,9 @@ const MESSAGES: Record = { MissingIdToken: 'Google did not return an identity token, so we could not sign you in.', TokenExchangeError: 'The backend rejected the Google sign-in. Please try again.', RefreshTokenError: 'Your session expired. Please sign in again.', + EmailLinkInvalid: + 'That sign-in link is wrong, already used or has expired. Only the newest email works.', + CredentialsSignin: 'That sign-in code is wrong or has expired.', Configuration: 'Sign-in is misconfigured. Check the Google client id and secret.', } diff --git a/frontend/src/app/auth/signin/page.tsx b/frontend/src/app/auth/signin/page.tsx index d71dfee..f8936e2 100644 --- a/frontend/src/app/auth/signin/page.tsx +++ b/frontend/src/app/auth/signin/page.tsx @@ -1,17 +1,22 @@ import { signIn } from '@/auth' import { Button } from '@/components/ui/button' import { Card, CardDescription, CardTitle } from '@/components/ui/card' +import { EmailSignIn } from '@/features/account/email-sign-in' +import { landingPath } from '@/lib/auth/landing' export const metadata = { title: 'Sign in' } export default async function SignInPage({ searchParams }: PageProps<'/auth/signin'>) { const { callbackUrl } = await searchParams - const redirectTo = typeof callbackUrl === 'string' ? callbackUrl : '/account' + const redirectTo = landingPath(callbackUrl) return ( Sign in - Google is the only sign-in method for now. + + With Google, or with a link and code we email you. New here? Either one creates your + account. +
{ @@ -23,6 +28,12 @@ export default async function SignInPage({ searchParams }: PageProps<'/auth/sign Continue with Google
+
+ + or + +
+ ) } diff --git a/frontend/src/auth.ts b/frontend/src/auth.ts index 362bd17..6314478 100644 --- a/frontend/src/auth.ts +++ b/frontend/src/auth.ts @@ -1,8 +1,21 @@ import NextAuth from 'next-auth' +import Credentials from 'next-auth/providers/credentials' import Google from 'next-auth/providers/google' +import { confirmEmailLogin, type EmailLoginProof } from '@/lib/auth/email-login' import { resolveAuthToken } from '@/lib/auth/resolve-token' +const field = (value: unknown) => (typeof value === 'string' ? value : '') + +/** The emailed code (with its address) or the link's token, whichever was sent. */ +function emailLoginProof(credentials: Partial>): EmailLoginProof | null { + const token = field(credentials.token) + if (token) return { token } + const email = field(credentials.email) + const code = field(credentials.code) + return email && code ? { email, code } : null +} + export const { handlers, auth, signIn, signOut } = NextAuth({ // The container hostname differs from AUTH_URL when running under Compose. trustHost: true, @@ -18,20 +31,39 @@ export const { handlers, auth, signIn, signOut } = NextAuth({ clientSecret: process.env.AUTH_GOOGLE_SECRET, authorization: { params: { scope: 'openid email profile', prompt: 'select_account' } }, }), + // One email carries a code and a link; either signs in. The backend mints + // the pair here, and resolveAuthToken keeps it - see src/lib/auth/email-login.ts. + Credentials({ + id: 'email', + credentials: { email: {}, code: {}, token: {} }, + async authorize(credentials) { + const proof = emailLoginProof(credentials) + if (!proof) return null + try { + const { tokens, user } = await confirmEmailLogin(proof) + // next-auth copies name and email onto the session; resolveAuthToken keeps tokens. + return { ...user, name: user.name || null, tokens } + } catch { + return null + } + }, + }), ], callbacks: { - // Cheap guard, no network. Without an id_token we cannot talk to allauth at + // Cheap guard, no network. An email sign-in got this far only by passing + // `authorize`, which already checked the code. Without an id_token we cannot talk to allauth at // all, so fail the sign-in rather than create a tokenless session. signIn({ account }) { + if (account?.provider === 'email') return true if (account?.provider !== 'google') return false return Boolean(account.id_token) || '/auth/error?error=MissingIdToken' }, // All branching lives in resolveAuthToken so it stays unit-testable without // booting NextAuth. See src/lib/auth/resolve-token.ts. - jwt({ token, account }) { - return resolveAuthToken({ token, account }) + jwt({ token, account, user }) { + return resolveAuthToken({ token, account, user }) }, session({ session, token }) { diff --git a/frontend/src/features/account/email-sign-in.test.tsx b/frontend/src/features/account/email-sign-in.test.tsx new file mode 100644 index 0000000..eac880c --- /dev/null +++ b/frontend/src/features/account/email-sign-in.test.tsx @@ -0,0 +1,95 @@ +import { screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { renderWithProviders } from '@/test/render' + +import { EmailSignIn } from './email-sign-in' + +const signIn = vi.fn() +const replace = vi.fn() +const refresh = vi.fn() + +vi.mock('next-auth/react', () => ({ + signIn: (...args: unknown[]) => signIn(...args), +})) + +vi.mock('next/navigation', () => ({ + useRouter: () => ({ replace, refresh }), +})) + +function stubFetch(status: number, body: unknown = null) { + return vi.spyOn(globalThis, 'fetch').mockImplementation( + async () => + new Response(body === null ? null : JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }), + ) +} + +afterEach(() => vi.restoreAllMocks()) + +async function requestCode(email = 'reader@example.com') { + await userEvent.type(screen.getByLabelText('Email'), email) + await userEvent.click(screen.getByRole('button', { name: 'Email me a sign-in link' })) +} + +describe('EmailSignIn', () => { + it('asks for a code and a link, carrying where to land afterwards', async () => { + const spy = stubFetch(204) + renderWithProviders() + + await requestCode() + + expect(await screen.findByText(/sent a sign-in link and a code/i)).toHaveTextContent( + 'reader@example.com', + ) + const [url, init] = spy.mock.calls[0] + expect(String(url)).toContain('/api/v1/auth/email-login') + expect(JSON.parse(String(init?.body))).toEqual({ + email: 'reader@example.com', + next: '/tools/acme', + }) + }) + + it('signs in with the typed code and lands where it was asked from', async () => { + stubFetch(204) + signIn.mockResolvedValue({ ok: true, error: undefined }) + renderWithProviders() + await requestCode() + + await userEvent.type(await screen.findByLabelText('Code'), '042917') + await userEvent.click(screen.getByRole('button', { name: 'Sign in' })) + + expect(signIn).toHaveBeenCalledWith('email', { + email: 'reader@example.com', + code: '042917', + redirect: false, + }) + expect(replace).toHaveBeenCalledWith('/tools/acme') + }) + + it('says so when the code is wrong, and stays put', async () => { + stubFetch(204) + signIn.mockResolvedValue({ ok: false, error: 'CredentialsSignin' }) + renderWithProviders() + await requestCode() + + await userEvent.type(await screen.findByLabelText('Code'), '000000') + await userEvent.click(screen.getByRole('button', { name: 'Sign in' })) + + expect(await screen.findByRole('alert')).toHaveTextContent(/wrong or has expired/i) + expect(replace).not.toHaveBeenCalled() + }) + + it('goes back to the address to fix a typo', async () => { + stubFetch(204) + renderWithProviders() + await requestCode('typo@example.con') + + await userEvent.click(await screen.findByRole('button', { name: 'Use a different email' })) + + expect(screen.getByLabelText('Email')).toHaveValue('typo@example.con') + }) +}) diff --git a/frontend/src/features/account/email-sign-in.tsx b/frontend/src/features/account/email-sign-in.tsx new file mode 100644 index 0000000..705fa9f --- /dev/null +++ b/frontend/src/features/account/email-sign-in.tsx @@ -0,0 +1,117 @@ +'use client' + +import { signIn } from 'next-auth/react' +import { useRouter } from 'next/navigation' +import { type FormEvent, useState } from 'react' + +import { Button } from '@/components/ui/button' +import { Input } from '@/components/ui/input' +import { errorMessage } from '@/lib/api/error-message' +import { useRequestEmailLogin } from '@/lib/api/generated/auth/auth' + +const WRONG_CODE = 'That code is wrong or has expired. Check the latest email, or send a new one.' + +/** + * Signing in by email. One message carries a link and a code: the link works on + * any device, and the code is for this tab - typed here, it signs in without + * leaving the page. + */ +export function EmailSignIn({ redirectTo }: { redirectTo: string }) { + const router = useRouter() + const request = useRequestEmailLogin() + const [email, setEmail] = useState('') + const [sentTo, setSentTo] = useState(null) + const [code, setCode] = useState('') + const [checking, setChecking] = useState(false) + const [codeError, setCodeError] = useState(null) + + const sendLink = (event: FormEvent) => { + event.preventDefault() + request.mutate( + { data: { email, next: redirectTo } }, + { onSuccess: () => setSentTo(email.trim()) }, + ) + } + + const submitCode = async (event: FormEvent) => { + event.preventDefault() + setChecking(true) + setCodeError(null) + const result = await signIn('email', { email: sentTo, code: code.trim(), redirect: false }) + if (result?.error) { + setCodeError(WRONG_CODE) + setChecking(false) + return + } + router.replace(redirectTo) + router.refresh() + } + + if (sentTo === null) { + return ( +
+ + {request.error ? ( +

+ {errorMessage(request.error, 'That did not work. Please try again.')} +

+ ) : null} + +
+ ) + } + + return ( +
+

+ We sent a sign-in link and a code to {sentTo}. + Open the link, or enter the code here. Both expire in 15 minutes. +

+ + {codeError ? ( +

+ {codeError} +

+ ) : null} + + +
+ ) +} diff --git a/frontend/src/features/account/user-menu.test.tsx b/frontend/src/features/account/user-menu.test.tsx index 58a9a2f..ce8b44d 100644 --- a/frontend/src/features/account/user-menu.test.tsx +++ b/frontend/src/features/account/user-menu.test.tsx @@ -67,7 +67,7 @@ describe('UserMenu when signed out', () => { await user.click(screen.getByRole('button', { name: /sign in/i })) - expect(signIn).toHaveBeenCalledWith('google') + expect(signIn).toHaveBeenCalledWith() }) it('treats a session carrying a token error as signed out', () => { diff --git a/frontend/src/features/account/user-menu.tsx b/frontend/src/features/account/user-menu.tsx index 8433c5c..be8362e 100644 --- a/frontend/src/features/account/user-menu.tsx +++ b/frontend/src/features/account/user-menu.tsx @@ -40,7 +40,7 @@ export function UserMenu() { // treat it as signed out rather than showing a menu that does not work. if (!session || session.error) { return ( - ) diff --git a/frontend/src/features/catalog/claim-invite.test.tsx b/frontend/src/features/catalog/claim-invite.test.tsx index 7cd5401..f57bf35 100644 --- a/frontend/src/features/catalog/claim-invite.test.tsx +++ b/frontend/src/features/catalog/claim-invite.test.tsx @@ -72,7 +72,7 @@ describe('ClaimInvite', () => { await user.click(screen.getByRole('button', { name: /sign in to accept/i })) - expect(signIn).toHaveBeenCalledWith('google', { redirectTo: `/claim/${TOKEN}` }) + expect(signIn).toHaveBeenCalledWith(undefined, { redirectTo: `/claim/${TOKEN}` }) expect(screen.queryByRole('button', { name: /^accept/i })).not.toBeInTheDocument() }) diff --git a/frontend/src/features/catalog/claim-invite.tsx b/frontend/src/features/catalog/claim-invite.tsx index 3e0d87e..64df9be 100644 --- a/frontend/src/features/catalog/claim-invite.tsx +++ b/frontend/src/features/catalog/claim-invite.tsx @@ -95,7 +95,7 @@ export function ClaimInvite({ token }: { token: string }) { {redeem.isPending ? 'Accepting…' : 'Accept and maintain this listing'} ) : ( - )} diff --git a/frontend/src/features/catalog/claim-listing.test.tsx b/frontend/src/features/catalog/claim-listing.test.tsx index 55cee77..6751b54 100644 --- a/frontend/src/features/catalog/claim-listing.test.tsx +++ b/frontend/src/features/catalog/claim-listing.test.tsx @@ -108,7 +108,7 @@ describe('ClaimListing', () => { await user.click(screen.getByRole('button', { name: /sign in to claim/i })) - expect(signIn).toHaveBeenCalledWith('google') + expect(signIn).toHaveBeenCalledWith() }) // Asking an owner whether they work for the vendor is the catalog forgetting diff --git a/frontend/src/features/catalog/claim-listing.tsx b/frontend/src/features/catalog/claim-listing.tsx index e4a75f2..7a4b1d8 100644 --- a/frontend/src/features/catalog/claim-listing.tsx +++ b/frontend/src/features/catalog/claim-listing.tsx @@ -74,7 +74,7 @@ export function ClaimListing({ tool }: { tool: ToolDetailOut }) { if (!signedIn) { return ( - + ) } diff --git a/frontend/src/features/comments/comment-section.test.tsx b/frontend/src/features/comments/comment-section.test.tsx index 0673f04..c247732 100644 --- a/frontend/src/features/comments/comment-section.test.tsx +++ b/frontend/src/features/comments/comment-section.test.tsx @@ -116,7 +116,7 @@ describe('CommentSection', () => { expect(screen.queryByRole('textbox')).not.toBeInTheDocument() await userEvent.click(screen.getByRole('button', { name: 'Sign in to comment' })) - expect(signIn).toHaveBeenCalledWith('google') + expect(signIn).toHaveBeenCalledWith() }) it('nests replies under the comment they answer', () => { diff --git a/frontend/src/features/comments/comment-section.tsx b/frontend/src/features/comments/comment-section.tsx index 7178053..6fdeff6 100644 --- a/frontend/src/features/comments/comment-section.tsx +++ b/frontend/src/features/comments/comment-section.tsx @@ -83,7 +83,7 @@ export function CommentSection({ ) : (

Sign in to join the discussion.

-
diff --git a/frontend/src/lib/api/generated/auth/auth.ts b/frontend/src/lib/api/generated/auth/auth.ts index 5a4272b..8e6c90c 100644 --- a/frontend/src/lib/api/generated/auth/auth.ts +++ b/frontend/src/lib/api/generated/auth/auth.ts @@ -33,6 +33,9 @@ import type { CliLoginStartOut, CliLoginTokenIn, CliLoginTokenOut, + EmailLoginConfirmIn, + EmailLoginIn, + EmailLoginTokenOut, GetMcpAuthorizationParams, McpAuthorizationDecisionIn, McpAuthorizationDecisionOut, @@ -918,6 +921,180 @@ export const useDenyCliLogin = => { return useMutation(getDenyCliLoginMutationOptions(options), queryClient); } + export const getRequestEmailLoginUrl = () => { + + + + + return `/api/v1/auth/email-login` +} + +/** + * @summary Email a sign-in code and link + */ +export const requestEmailLogin = async (emailLoginIn: EmailLoginIn, options?: Parameters[1]): Promise => { + + const getHeaders = (h?: NonNullable): Record => { + if (!h) return {}; + if (h instanceof Headers) return Object.fromEntries(h.entries()); + if (Symbol.iterator in h) { + return Object.fromEntries( + Array.from(h as Iterable>, (entry) => Array.from(entry) as [string, string]), + ); + } + const headers: Record = {}; + for (const [name, value] of Object.entries(h)) { + if (value !== undefined) headers[name] = value; + } + return headers; + }; +return customFetch(getRequestEmailLoginUrl(), + { + ...options, + method: 'POST', + headers: { 'Content-Type': 'application/json', ...getHeaders(options?.headers) }, + body: JSON.stringify(emailLoginIn) + } +);} + + + + + +export const getRequestEmailLoginMutationKey = () => ['requestEmailLogin'] as const; + +export const getRequestEmailLoginMutationOptions = (options?: { mutation?:UseMutationOptions>, TError,RequestEmailLoginMutationVariables, TContext>, request?: SecondParameter} +): UseMutationOptions>, TError,RequestEmailLoginMutationVariables, TContext> => { + +const mutationKey = getRequestEmailLoginMutationKey(); +const {mutation: mutationOptions, request: requestOptions} = options ? + options.mutation && 'mutationKey' in options.mutation && options.mutation.mutationKey ? + options + : {...options, mutation: {...options.mutation, mutationKey}} + : {mutation: { mutationKey, }, request: undefined}; + + + + + const mutationFn: MutationFunction>, RequestEmailLoginMutationVariables> = (props) => { + const {data} = props ?? {}; + + return requestEmailLogin(data,requestOptions) + } + + + + + + + return { mutationFn, ...mutationOptions }} + + export type RequestEmailLoginMutationResult = NonNullable>> + export type RequestEmailLoginMutationBody = EmailLoginIn + export type RequestEmailLoginMutationError = unknown + export type RequestEmailLoginMutationVariables = {data: EmailLoginIn} + + /** + * @summary Email a sign-in code and link + */ +export const useRequestEmailLogin = (options?: { mutation?:UseMutationOptions>, TError,RequestEmailLoginMutationVariables, TContext>, request?: SecondParameter} + , queryClient?: QueryClient): UseMutationResult< + Awaited>, + TError, + RequestEmailLoginMutationVariables, + TContext + > => { + return useMutation(getRequestEmailLoginMutationOptions(options), queryClient); + } + export const getConfirmEmailLoginUrl = () => { + + + + + return `/api/v1/auth/email-login/confirm` +} + +/** + * @summary Sign in with an emailed code or link + */ +export const confirmEmailLogin = async (emailLoginConfirmIn: EmailLoginConfirmIn, options?: Parameters[1]): Promise => { + + const getHeaders = (h?: NonNullable): Record => { + if (!h) return {}; + if (h instanceof Headers) return Object.fromEntries(h.entries()); + if (Symbol.iterator in h) { + return Object.fromEntries( + Array.from(h as Iterable>, (entry) => Array.from(entry) as [string, string]), + ); + } + const headers: Record = {}; + for (const [name, value] of Object.entries(h)) { + if (value !== undefined) headers[name] = value; + } + return headers; + }; +return customFetch(getConfirmEmailLoginUrl(), + { + ...options, + method: 'POST', + headers: { 'Content-Type': 'application/json', ...getHeaders(options?.headers) }, + body: JSON.stringify(emailLoginConfirmIn) + } +);} + + + + + +export const getConfirmEmailLoginMutationKey = () => ['confirmEmailLogin'] as const; + +export const getConfirmEmailLoginMutationOptions = (options?: { mutation?:UseMutationOptions>, TError,ConfirmEmailLoginMutationVariables, TContext>, request?: SecondParameter} +): UseMutationOptions>, TError,ConfirmEmailLoginMutationVariables, TContext> => { + +const mutationKey = getConfirmEmailLoginMutationKey(); +const {mutation: mutationOptions, request: requestOptions} = options ? + options.mutation && 'mutationKey' in options.mutation && options.mutation.mutationKey ? + options + : {...options, mutation: {...options.mutation, mutationKey}} + : {mutation: { mutationKey, }, request: undefined}; + + + + + const mutationFn: MutationFunction>, ConfirmEmailLoginMutationVariables> = (props) => { + const {data} = props ?? {}; + + return confirmEmailLogin(data,requestOptions) + } + + + + + + + return { mutationFn, ...mutationOptions }} + + export type ConfirmEmailLoginMutationResult = NonNullable>> + export type ConfirmEmailLoginMutationBody = EmailLoginConfirmIn + export type ConfirmEmailLoginMutationError = unknown + export type ConfirmEmailLoginMutationVariables = {data: EmailLoginConfirmIn} + + /** + * @summary Sign in with an emailed code or link + */ +export const useConfirmEmailLogin = (options?: { mutation?:UseMutationOptions>, TError,ConfirmEmailLoginMutationVariables, TContext>, request?: SecondParameter} + , queryClient?: QueryClient): UseMutationResult< + Awaited>, + TError, + ConfirmEmailLoginMutationVariables, + TContext + > => { + return useMutation(getConfirmEmailLoginMutationOptions(options), queryClient); + } export const getGetMcpAuthorizationUrl = (params?: GetMcpAuthorizationParams,) => { const normalizedParams = new URLSearchParams(); diff --git a/frontend/src/lib/api/generated/model/emailLoginConfirmIn.ts b/frontend/src/lib/api/generated/model/emailLoginConfirmIn.ts new file mode 100644 index 0000000..a872a7c --- /dev/null +++ b/frontend/src/lib/api/generated/model/emailLoginConfirmIn.ts @@ -0,0 +1,16 @@ +/** + * Generated by orval 🍺 + * Do not edit manually. + * redaction-tools API + * Redaction tools compared by price, media and method, with the source and date recorded for every figure. + * OpenAPI spec version: 1.0.0 + */ + +/** + * Either `email` and `code`, as typed on the sign-in page, or the link's `token`. + */ +export interface EmailLoginConfirmIn { + email?: string; + code?: string; + token?: string; +} diff --git a/frontend/src/lib/api/generated/model/emailLoginIn.ts b/frontend/src/lib/api/generated/model/emailLoginIn.ts new file mode 100644 index 0000000..54276ed --- /dev/null +++ b/frontend/src/lib/api/generated/model/emailLoginIn.ts @@ -0,0 +1,17 @@ +/** + * Generated by orval 🍺 + * Do not edit manually. + * redaction-tools API + * Redaction tools compared by price, media and method, with the source and date recorded for every figure. + * OpenAPI spec version: 1.0.0 + */ + +export interface EmailLoginIn { + /** + * @minLength 3 + * @maxLength 254 + */ + email: string; + /** @maxLength 500 */ + next?: string; +} diff --git a/frontend/src/lib/api/generated/model/emailLoginTokenOut.ts b/frontend/src/lib/api/generated/model/emailLoginTokenOut.ts new file mode 100644 index 0000000..337d417 --- /dev/null +++ b/frontend/src/lib/api/generated/model/emailLoginTokenOut.ts @@ -0,0 +1,16 @@ +/** + * Generated by orval 🍺 + * Do not edit manually. + * redaction-tools API + * Redaction tools compared by price, media and method, with the source and date recorded for every figure. + * OpenAPI spec version: 1.0.0 + */ +import type { UserSchema } from './userSchema'; + +export interface EmailLoginTokenOut { + access_token: string; + refresh_token: string; + token_type: string; + expires_in: number; + user: UserSchema; +} diff --git a/frontend/src/lib/api/generated/model/index.ts b/frontend/src/lib/api/generated/model/index.ts index 697c999..0645010 100644 --- a/frontend/src/lib/api/generated/model/index.ts +++ b/frontend/src/lib/api/generated/model/index.ts @@ -32,6 +32,9 @@ export * from './commentOut'; export * from './commentSettingsIn'; export * from './commentSettingsOut'; export * from './countsOut'; +export * from './emailLoginConfirmIn'; +export * from './emailLoginIn'; +export * from './emailLoginTokenOut'; export * from './errorSchema'; export * from './exchangeRatesOut'; export * from './exchangeRatesOutRates'; diff --git a/frontend/src/lib/auth/email-login.test.ts b/frontend/src/lib/auth/email-login.test.ts new file mode 100644 index 0000000..a1daa4f --- /dev/null +++ b/frontend/src/lib/auth/email-login.test.ts @@ -0,0 +1,77 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { ApiError } from '@/lib/api/api-error' +import { confirmEmailLogin } from '@/lib/auth/email-login' + +const NOW = 1_700_000_000_000 +const EXP_SECONDS = 1_700_000_900 + +function jwtWithExp(exp: number): string { + const encode = (value: object) => + btoa(JSON.stringify(value)).replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') + return `${encode({ alg: 'HS256' })}.${encode({ exp })}.signature` +} + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }) +} + +const tokenBody = () => ({ + access_token: jwtWithExp(EXP_SECONDS), + refresh_token: 'r1', + token_type: 'Bearer', + expires_in: 900, + user: { id: 'u1', email: 'reader@example.com', name: 'Reader', is_staff: false }, +}) + +let fetchSpy: ReturnType + +beforeEach(() => { + vi.setSystemTime(NOW) + fetchSpy = vi.spyOn(globalThis, 'fetch') +}) + +afterEach(() => { + vi.useRealTimers() +}) + +describe('confirmEmailLogin', () => { + it('trades the emailed code for our token pair', async () => { + fetchSpy.mockResolvedValue(jsonResponse(tokenBody())) + + const { tokens, user } = await confirmEmailLogin({ + email: 'reader@example.com', + code: '042917', + }) + + const [url, init] = fetchSpy.mock.calls[0] + expect(url).toBe('http://localhost:8007/api/v1/auth/email-login/confirm') + expect(JSON.parse(init?.body as string)).toEqual({ + email: 'reader@example.com', + code: '042917', + }) + expect(tokens).toEqual({ + accessToken: tokenBody().access_token, + refreshToken: 'r1', + accessTokenExpiresAt: EXP_SECONDS * 1000, + }) + expect(user).toEqual({ id: 'u1', email: 'reader@example.com', name: 'Reader' }) + }) + + it('trades the link token the same way', async () => { + fetchSpy.mockResolvedValue(jsonResponse(tokenBody())) + + await confirmEmailLogin({ token: 'link-token' }) + + expect(JSON.parse(fetchSpy.mock.calls[0][1]?.body as string)).toEqual({ token: 'link-token' }) + }) + + it('throws on a wrong or expired code', async () => { + fetchSpy.mockResolvedValue(jsonResponse({ detail: 'wrong' }, 400)) + + await expect(confirmEmailLogin({ token: 'stale' })).rejects.toBeInstanceOf(ApiError) + }) +}) diff --git a/frontend/src/lib/auth/email-login.ts b/frontend/src/lib/auth/email-login.ts new file mode 100644 index 0000000..3d66799 --- /dev/null +++ b/frontend/src/lib/auth/email-login.ts @@ -0,0 +1,32 @@ +/** + * Signing in by email: trading the emailed code, or the link's token, for our pair. + * + * Unlike the Google exchange in `allauth.ts`, this endpoint is ours and in the + * OpenAPI spec, so it goes through the generated client. The backend answers + * any failure with a 400, which surfaces here as an `ApiError`. + */ +import { confirmEmailLogin as postConfirm } from '@/lib/api/generated/auth/auth' +import type { EmailLoginConfirmIn } from '@/lib/api/generated/model' +import { readJwtExpiry } from '@/lib/auth/jwt' +import type { TokenPair } from '@/lib/auth/tokens' + +export type EmailLoginProof = { email: string; code: string } | { token: string } + +export interface EmailSignIn { + tokens: TokenPair + /** For the session's name and email: there is no id_token to read them from. */ + user: { id: string; email: string; name: string } +} + +export async function confirmEmailLogin(proof: EmailLoginProof): Promise { + const body: EmailLoginConfirmIn = proof + const out = await postConfirm(body) + return { + tokens: { + accessToken: out.access_token, + refreshToken: out.refresh_token, + accessTokenExpiresAt: readJwtExpiry(out.access_token) ?? Date.now() + out.expires_in * 1000, + }, + user: { id: out.user.id, email: out.user.email, name: out.user.name }, + } +} diff --git a/frontend/src/lib/auth/landing.test.ts b/frontend/src/lib/auth/landing.test.ts new file mode 100644 index 0000000..3085fee --- /dev/null +++ b/frontend/src/lib/auth/landing.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from 'vitest' + +import { landingPath } from '@/lib/auth/landing' + +describe('landingPath', () => { + it.each([ + ['/tools/acme?tab=pricing', '/tools/acme?tab=pricing'], + // next-auth's signIn() passes the current page as an absolute URL. + ['http://localhost:3007/tools/acme?tab=pricing#plans', '/tools/acme?tab=pricing#plans'], + // Another site's URL keeps only its path, so it can only land on this one. + ['https://evil.example/steal', '/steal'], + ['//evil.example/steal', '/account'], + ['https://x.example//evil.example/steal', '/account'], + ['/\\evil.example/steal', '/account'], + ['', '/account'], + [undefined, '/account'], + [['/a', '/b'], '/account'], + ])('%j lands on %j', (input, expected) => { + expect(landingPath(input)).toBe(expected) + }) +}) diff --git a/frontend/src/lib/auth/landing.ts b/frontend/src/lib/auth/landing.ts new file mode 100644 index 0000000..5ee6817 --- /dev/null +++ b/frontend/src/lib/auth/landing.ts @@ -0,0 +1,23 @@ +/** + * Where a sign-in lands: always a path on this site. + * + * `callbackUrl` and an email link's `next` are whatever the URL says. An + * absolute URL keeps only its path - next-auth's own `signIn()` sends the + * current page that way - so no input can send the reader to another site. + */ +const FALLBACK = '/account' + +export function landingPath(value: unknown): string { + if (typeof value !== 'string' || !value) return FALLBACK + let path = value + if (!value.startsWith('/')) { + try { + const url = new URL(value) + path = `${url.pathname}${url.search}${url.hash}` + } catch { + return FALLBACK + } + } + // `//host` and `/\host` are protocol-relative to a browser: another site. + return /^\/[/\\]/.test(path) ? FALLBACK : path +} diff --git a/frontend/src/lib/auth/resolve-token.test.ts b/frontend/src/lib/auth/resolve-token.test.ts index f738ba4..b3f5b2b 100644 --- a/frontend/src/lib/auth/resolve-token.test.ts +++ b/frontend/src/lib/auth/resolve-token.test.ts @@ -53,6 +53,24 @@ describe('resolveAuthToken on sign-in', () => { expect(result.error).toBe('TokenExchangeError') }) + + it('keeps the pair an email sign-in already traded for, with no network call', async () => { + const d = deps() + + const result = await resolveAuthToken( + { + token: { error: 'RefreshTokenError' }, + account: { provider: 'email' }, + user: { tokens: freshPair }, + }, + d, + ) + + expect(result).toMatchObject(freshPair) + expect(result.error).toBeUndefined() + expect(d.exchange).not.toHaveBeenCalled() + expect(d.refresh).not.toHaveBeenCalled() + }) }) describe('resolveAuthToken on subsequent calls', () => { diff --git a/frontend/src/lib/auth/resolve-token.ts b/frontend/src/lib/auth/resolve-token.ts index 108ed12..139cf22 100644 --- a/frontend/src/lib/auth/resolve-token.ts +++ b/frontend/src/lib/auth/resolve-token.ts @@ -5,7 +5,7 @@ * gets rewritten and this logic does not. */ import { exchangeGoogleIdToken, refreshTokenPair } from '@/lib/auth/allauth' -import { isAccessTokenExpired } from '@/lib/auth/tokens' +import { isAccessTokenExpired, type TokenPair } from '@/lib/auth/tokens' export type AuthTokenError = 'MissingIdToken' | 'TokenExchangeError' | 'RefreshTokenError' @@ -21,6 +21,11 @@ export interface ProviderAccount { id_token?: string } +/** What the email provider's `authorize` returns: the pair it already traded for. */ +export interface SignedInUser { + tokens?: TokenPair +} + export interface ResolveAuthTokenDeps { exchange: typeof exchangeGoogleIdToken refresh: typeof refreshTokenPair @@ -34,10 +39,10 @@ const defaultDeps: ResolveAuthTokenDeps = { } export async function resolveAuthToken( - args: { token: T; account?: ProviderAccount | null }, + args: { token: T; account?: ProviderAccount | null; user?: SignedInUser | null }, deps: ResolveAuthTokenDeps = defaultDeps, ): Promise { - const { token, account } = args + const { token, account, user } = args // 1. Fresh sign-in. `account` is only passed on the first call after a // successful provider login, which is why the exchange belongs here. @@ -53,6 +58,13 @@ export async function resolveAuthToken( } } + // 1b. Email sign-in. `authorize` has already traded the code or link for our + // pair (it is the only place that sees the credentials), so just keep it. + if (account?.provider === 'email') { + if (!user?.tokens) return { ...token, error: 'TokenExchangeError' } + return { ...token, ...user.tokens, error: undefined } + } + // 2. Nothing to refresh with - a previous exchange must have failed. if (!token.refreshToken) return token diff --git a/frontend/src/types/next-auth.d.ts b/frontend/src/types/next-auth.d.ts index e0223d2..aa80c21 100644 --- a/frontend/src/types/next-auth.d.ts +++ b/frontend/src/types/next-auth.d.ts @@ -1,10 +1,16 @@ import type { AuthTokenError } from '@/lib/auth/resolve-token' +import type { TokenPair } from '@/lib/auth/tokens' // `next-auth/jwt` is a bare `export * from "@auth/core/jwt"`, so augmenting it // would declare a separate interface rather than merging with the real one. // The JWT augmentation therefore targets @auth/core/jwt directly. declare module 'next-auth' { + interface User { + /** Set only by the email provider's `authorize`: the pair it traded the code for. */ + tokens?: TokenPair + } + interface Session { /** Our backend's access token, not Google's. Absent once `error` is set. */ accessToken?: string