diff --git a/api/.env.local.example b/api/.env.local.example index b1fd621..3c29275 100644 --- a/api/.env.local.example +++ b/api/.env.local.example @@ -24,6 +24,33 @@ DJANGO_SUPERUSER_PASSWORD= APP_URL= APP_MAGIC_LINK_PATH= +# Sending mail. Fastmail over SMTP, authenticated as an app password +# rather than the account password: SMTP refuses the account password +# once two-factor is on. +# +# Make one in Fastmail: Settings, Privacy & Security, Integrations, New +# App Password, scoped to SMTP. +# +# EMAIL_HOST_USER is the account that signs in. EMAIL_FROM is the +# address a reader sees. They may differ, and Fastmail refuses a From +# address the account does not own. +# +# Port 465 is implicit TLS. `settings.py` reads the port and sets +# EMAIL_USE_SSL or EMAIL_USE_TLS from it, so there is nothing else here. +EMAIL_HOST=smtp.fastmail.com +EMAIL_PORT=465 +EMAIL_HOST_USER= +EMAIL_HOST_PASSWORD= +EMAIL_FROM=Statusboard + +# Print the mail instead of sending it. Set this when a sign-in link +# should reach the terminal rather than a real inbox: +# +# EMAIL_USE_CONSOLE=1 just dev +# +# It beats EMAIL_HOST, so the credentials above can stay in place. +# EMAIL_USE_CONSOLE= + # A deployment sets these. Development does not need them: SECRET_KEY # falls back to a key in the repository, and DEBUG follows ENVIRONMENT. # Outside development a missing SECRET_KEY stops the process. diff --git a/api/api/defaults.py b/api/api/defaults.py index 7b452dd..41f6973 100644 --- a/api/api/defaults.py +++ b/api/api/defaults.py @@ -104,6 +104,34 @@ class Throttle(StrEnum): } +# Print mail instead of sending it, even when a host is configured. The +# local file carries real credentials, so this is how a sign-in link +# reaches a terminal rather than an inbox. +# +# It is ours, not Django's, and it reads like Django's. That is the +# price of sitting in the EMAIL_ family the rest of the file uses. +EMAIL_USE_CONSOLE = os.environ.get("EMAIL_USE_CONSOLE", "").lower() not in ( + "", + "0", + "false", + "no", +) + + +def email_backend(host, use_console): + """Which backend carries the mail. + + A magic link is the only way to sign in, so a deployment that cannot + send cannot be used. A host is what says it can. + + Here rather than inline in `settings.py`. A settings module computes + its values on import, and a test cannot call that twice. + """ + if host and not use_console: + return "django.core.mail.backends.smtp.EmailBackend" + return "django.core.mail.backends.console.EmailBackend" + + # Cursor pagination. `/meta` publishes both, so a client can size a page # without guessing which values the server will accept. DEFAULT_PAGE_SIZE = 50 diff --git a/api/api/settings.py b/api/api/settings.py index 1ad31cd..95bb61b 100644 --- a/api/api/settings.py +++ b/api/api/settings.py @@ -12,6 +12,7 @@ APP_MAGIC_LINK_PATH, APP_URL, DEFAULT_PAGE_SIZE, + EMAIL_USE_CONSOLE, ENVIRONMENT, EVENT_CLAIM_WINDOW, MAGIC_LINK_TTL, @@ -23,6 +24,7 @@ SYSTEM_EMAIL, THROTTLE_RATES, debug, + email_backend, secret_key, ) @@ -422,22 +424,20 @@ ], } -# A magic link is the only way to sign in, so a deployment that cannot -# send mail cannot be used. EMAIL_HOST decides: set, the mail is sent; -# unset, it is printed, which is what development wants. +# EMAIL_HOST says a deployment can send. EMAIL_USE_CONSOLE overrides it +# and prints instead. That one is ours, and `defaults.py` defines it and +# `email_backend` beside it. Every other name here is Django's. EMAIL_HOST = os.environ.get("EMAIL_HOST", "") -EMAIL_BACKEND = ( - "django.core.mail.backends.smtp.EmailBackend" - if EMAIL_HOST - else "django.core.mail.backends.console.EmailBackend" -) +EMAIL_BACKEND = email_backend(EMAIL_HOST, EMAIL_USE_CONSOLE) EMAIL_PORT = int(os.environ.get("EMAIL_PORT", "587")) EMAIL_HOST_USER = os.environ.get("EMAIL_HOST_USER", "") EMAIL_HOST_PASSWORD = os.environ.get("EMAIL_HOST_PASSWORD", "") EMAIL_USE_TLS = EMAIL_PORT != 465 EMAIL_USE_SSL = EMAIL_PORT == 465 +# Django's name, and Django reads it for every message. The variable +# behind it is EMAIL_FROM, so the file it comes from has one prefix. DEFAULT_FROM_EMAIL = os.environ.get( - "DEFAULT_FROM_EMAIL", "Statusboard " + "EMAIL_FROM", "Statusboard " ) # The client app is on another origin, so the browser asks first. A diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 7bd36c2..9f91460 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -73,3 +73,18 @@ def forget_throttles(): from django.core.cache import cache cache.clear() + + +@pytest.fixture(autouse=True) +def mail_takes_the_console_path(settings): + """Tests pick the backend the way a machine with no credentials does. + + `.env.local` now carries a real Fastmail password, and `defaults.py` + loads that file on import. Without this, `just test` on the host + reads the SMTP branch while CI, which has no file, reads the other. + + The inputs are pinned, not `EMAIL_BACKEND`. pytest-django swaps in + locmem on top, so `mail.outbox` still holds what was sent. + """ + settings.EMAIL_HOST = "" + settings.EMAIL_USE_CONSOLE = True diff --git a/api/tests/test_auth.py b/api/tests/test_auth.py index 2abdf51..6965217 100644 --- a/api/tests/test_auth.py +++ b/api/tests/test_auth.py @@ -6,6 +6,7 @@ from django.utils import timezone from rest_framework.test import APIClient +from api.defaults import email_backend from authentication.models import MagicLinkToken, User @@ -213,3 +214,19 @@ def test_logging_out_answers_with_an_empty_object(): assert response.status_code == 200 assert response.json() == {} + + +@pytest.mark.parametrize( + ("host", "console", "expected"), + [ + ("smtp.fastmail.com", False, "smtp"), + ("smtp.fastmail.com", True, "console"), + ("", False, "console"), + ("", True, "console"), + ], +) +def test_the_console_toggle_beats_a_configured_host(host, console, expected): + # A developer with real credentials in .env.local has to be able to + # stop a test sign-in reaching a real inbox. Were the host to win, + # the only way to print a link would be to delete the password. + assert expected in email_backend(host, console) diff --git a/bin/make-env.py b/bin/make-env.py index 21a4357..5e94798 100755 --- a/bin/make-env.py +++ b/bin/make-env.py @@ -21,8 +21,16 @@ PROMPTS = { "DJANGO_SUPERUSER_EMAIL": ("admin email: ", input), "DJANGO_SUPERUSER_PASSWORD": ("admin password: ", getpass), + "EMAIL_HOST_USER": ("fastmail address: ", input), + "EMAIL_HOST_PASSWORD": ("fastmail app password: ", getpass), } +# What makes a file filled in, which is not the same list. Mail is +# optional: EMAIL_USE_CONSOLE prints a sign-in link instead. So it is +# worth asking for once. Requiring it would re-ask on every init, and +# copy the shared file over a worktree's own values. +REQUIRED = ("DJANGO_SUPERUSER_EMAIL", "DJANGO_SUPERUSER_PASSWORD") + # Anchored to the checkout, not the cwd: `just` recipes cd into api/. ROOT = pathlib.Path( subprocess.run( @@ -56,16 +64,24 @@ def values(text: str) -> dict[str, str]: return out -def blanks(text: str) -> list[str]: +def blanks(text: str, keys=None) -> list[str]: found = values(text) - return [key for key in PROMPTS if not found.get(key)] + return [key for key in (keys or PROMPTS) if not found.get(key)] def fill(text: str, key: str, value: str) -> str: + """Set a key, appending it when the file predates it. + + A file written before the template grew a key does not hold it. The + rewrite went line by line. So an answer to a prompt for a missing + key went nowhere. + """ lines = [ f"{key}={value}" if line.startswith(f"{key}=") else line for line in text.splitlines() ] + if not any(line.startswith(f"{key}=") for line in lines): + lines.append(f"{key}={value}") return "\n".join(lines) + "\n" @@ -88,7 +104,7 @@ def ensure(path: pathlib.Path) -> str: text = starting_text(path) missing = blanks(text) if missing and sys.stdin.isatty(): - print(f"Setting the local admin in {path}. Press Enter to skip.") + print(f"Filling in {path}. Press Enter to skip any of them.") for key in missing: prompt, reader = PROMPTS[key] answer = reader(prompt).strip() @@ -109,7 +125,7 @@ def main() -> None: return # A worktree. Copy the shared file unless this one already has values. - if local.exists() and not blanks(local.read_text()): + if local.exists() and not blanks(local.read_text(), REQUIRED): return local.write_text(text) print(f"copied api/.env.local from {shared.parent.parent}") diff --git a/bin/tests/test_make_env.py b/bin/tests/test_make_env.py new file mode 100644 index 0000000..09e0e70 --- /dev/null +++ b/bin/tests/test_make_env.py @@ -0,0 +1,54 @@ +"""`bin/make-env.py` writes the answers it is given. + +`bin/` has no dependency manager and no Django. These use only the +standard library, so they run after nothing but `just init`. +""" + +import importlib.util +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +SPEC = importlib.util.spec_from_file_location("make_env", ROOT / "bin" / "make-env.py") +make_env = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(make_env) + + +class FillTests(unittest.TestCase): + def test_an_existing_key_is_replaced_in_place(self): + # The file keeps its comments and its order. A rewrite that + # reordered it would make every diff unreadable. + text = "A=1\nEMAIL_HOST_USER=\nB=2\n" + self.assertEqual( + make_env.fill(text, "EMAIL_HOST_USER", "me@example.com"), + "A=1\nEMAIL_HOST_USER=me@example.com\nB=2\n", + ) + + def test_a_key_the_file_predates_is_appended(self): + # Were it dropped, a developer would be asked for a password + # every init and it would go nowhere each time. + out = make_env.fill("A=1\n", "EMAIL_HOST_PASSWORD", "secret") + self.assertIn("EMAIL_HOST_PASSWORD=secret", out) + + def test_a_prefix_of_another_key_is_not_mistaken_for_it(self): + # EMAIL_HOST and EMAIL_HOST_USER share a prefix. Matching on + # the name alone would set the wrong one. + out = make_env.fill("EMAIL_HOST_USER=\n", "EMAIL_HOST", "smtp.example.com") + self.assertIn("EMAIL_HOST_USER=", out) + self.assertIn("EMAIL_HOST=smtp.example.com", out) + + +class RequiredTests(unittest.TestCase): + def test_mail_is_asked_for_but_not_required(self): + # Requiring it would re-ask on every init, and copy the shared + # file over a worktree's own values. + self.assertIn("EMAIL_HOST_PASSWORD", make_env.PROMPTS) + self.assertNotIn("EMAIL_HOST_PASSWORD", make_env.REQUIRED) + + def test_a_file_with_the_admin_in_it_counts_as_filled(self): + text = "DJANGO_SUPERUSER_EMAIL=a@b.c\nDJANGO_SUPERUSER_PASSWORD=x\n" + self.assertEqual(make_env.blanks(text, make_env.REQUIRED), []) + + +if __name__ == "__main__": + unittest.main()