From 23d004e5e53397e7b81cfa8b22d66b2dd0478160 Mon Sep 17 00:00:00 2001 From: Otitodev <198799600+Otitodev@users.noreply.github.com> Date: Mon, 5 Oct 2026 08:43:49 +0100 Subject: [PATCH 01/10] fix(core): give the domain back when AWS refuses to set up receiving setup_receiving claims the domain before it calls AWS, so that the unique index can decide a race between two requests enabling the same domain. If AWS then refused - a missing permission is the likeliest - the claim stayed on the identity, and the identity said it received mail when nothing did. That is worse than it sounds because of how the routes that call this are written. They deliberately do not roll back on an APIError: a rollback expires every loaded object and the page then 500s instead of showing the message, which was seen on a real host for the event routes. So the same render would have shown "Receiving: on" beside the very reason it was not. On failure the identity's rule fields are now put back to what they were before the attempt, and flushed. "What they were" and not "blank": a repair run on a domain that already receives must not leave it blank because the second attempt failed - it was receiving, and it still is. The record of the bucket, topic and queue is kept when only the rule step fails, because they really are in the account and a row that forgot them is one teardown could never find. Tests cover a refusal before anything is built, a refusal after the plumbing exists, and a failed repair. They need the database, so they run in CI; the first would fail on the old code on `receives_mail is False`. --- .../core/src/seskit_core/services/inbound.py | 49 +++++++--- tests/test_inbound_service.py | 89 +++++++++++++++++++ 2 files changed, 127 insertions(+), 11 deletions(-) diff --git a/packages/core/src/seskit_core/services/inbound.py b/packages/core/src/seskit_core/services/inbound.py index 9e67057..af82ad9 100644 --- a/packages/core/src/seskit_core/services/inbound.py +++ b/packages/core/src/seskit_core/services/inbound.py @@ -210,20 +210,47 @@ async def setup_receiving( resource_prefix, account_id=connection.aws_account_id, region=connection.region ) + before = ( + identity.inbound_rule_name, + identity.inbound_rule_set, + identity.inbound_rule_set_created, + ) await _claim_domain(session, identity, rule_name) - provisioner = factory(connection.region, credentials) - infrastructure = await provisioner.provision_inbound( - bucket_name=bucket, - topic_name=topic_name_for(resource_prefix), - queue_name=queue_name_for(resource_prefix), - retention_days=retention_days, - ) - connection.record_inbound_infrastructure(infrastructure) + try: + provisioner = factory(connection.region, credentials) + infrastructure = await provisioner.provision_inbound( + bucket_name=bucket, + topic_name=topic_name_for(resource_prefix), + queue_name=queue_name_for(resource_prefix), + retention_days=retention_days, + ) + # Recorded as soon as it exists, and kept if the rule below then fails: + # the bucket, topic and queue are really in the account, and a row that + # forgot them is one teardown could never find. + connection.record_inbound_infrastructure(infrastructure) + + rule = await provisioner.add_inbound_rule( + infrastructure, domain=identity.value, rule_name=rule_name + ) + except Exception: + # Give the domain back. The claim above was written before AWS was + # called, so that the unique index could decide a race; if AWS then + # refuses, nothing is receiving, and the identity must not say it is. + # + # This matters beyond tidiness. The routes that call this deliberately + # do not roll back on an error - a rollback expires every loaded object + # and the page then 500s instead of showing the message - so without + # this the same render would show "Receiving: on" beside the reason it + # is not. A repair run puts back what was there, not blank. + ( + identity.inbound_rule_name, + identity.inbound_rule_set, + identity.inbound_rule_set_created, + ) = before + await session.flush() + raise - rule = await provisioner.add_inbound_rule( - infrastructure, domain=identity.value, rule_name=rule_name - ) identity.inbound_rule_name = rule.name identity.inbound_rule_set = rule.rule_set identity.inbound_rule_set_created = rule.created_rule_set diff --git a/tests/test_inbound_service.py b/tests/test_inbound_service.py index 4e39175..c083521 100644 --- a/tests/test_inbound_service.py +++ b/tests/test_inbound_service.py @@ -62,6 +62,11 @@ class FakeInbound: bucket_removed: ClassVar[bool] = True #: Raised by ``remove_inbound_rule`` when set. remove_rule_error: ClassVar[APIError | None] = None + #: Raised by ``provision_inbound`` when set, to stand in for AWS refusing - + #: a missing permission, say. + setup_error: ClassVar[APIError | None] = None + #: Raised by ``add_inbound_rule`` alone, after the plumbing was built. + rule_error: ClassVar[APIError | None] = None def __init__(self, region: str, credentials: AWSCredentials | None = None) -> None: self.region = region @@ -75,6 +80,8 @@ async def provision_inbound( queue_name: str, retention_days: int, ) -> InboundInfrastructure: + if FakeInbound.setup_error is not None: + raise FakeInbound.setup_error FakeInbound.calls.append( ("provision", {"bucket": bucket_name, "retention_days": retention_days}) ) @@ -89,6 +96,8 @@ async def provision_inbound( async def add_inbound_rule( self, infrastructure: InboundInfrastructure, *, domain: str, rule_name: str ) -> InboundRule: + if FakeInbound.rule_error is not None: + raise FakeInbound.rule_error FakeInbound.calls.append(("add_rule", domain)) return InboundRule(name=rule_name, rule_set="seskit-inbound", created_rule_set=True) @@ -107,6 +116,8 @@ def _reset() -> None: FakeInbound.calls = [] FakeInbound.bucket_removed = True FakeInbound.remove_rule_error = None + FakeInbound.setup_error = None + FakeInbound.rule_error = None def factory(region: str, credentials: AWSCredentials) -> InboundProvisioner: @@ -308,6 +319,84 @@ async def test_a_domain_already_receiving_elsewhere_is_refused_before_aws_is_tou assert mine.receives_mail is True +# --------------------------------------------------- when AWS refuses --- + + +async def test_a_refusal_from_aws_leaves_the_domain_not_receiving( + db_session: AsyncSession, +) -> None: + """The claim is written before AWS is called, so that the unique index can + decide a race. If AWS then refuses, the identity must not say it receives: the + routes that call this do not roll back, so the same render would show + "Receiving: on" beside the reason it is not. + """ + connection = await _project_connection(db_session, email="a@example.com") + identity = await _domain(db_session, connection) + FakeInbound.setup_error = APIError( + ErrorType.AUTHORIZATION_FAILED, "not permitted to call s3:CreateBucket" + ) + + with pytest.raises(APIError): + await _enable(db_session, connection, identity) + + assert identity.receives_mail is False + assert identity.inbound_rule_name is None + assert identity.inbound_rule_set is None + assert identity.inbound_rule_set_created is False + # And it is not left holding the domain against a second attempt. + FakeInbound.setup_error = None + await _enable(db_session, connection, identity) + assert identity.receives_mail is True + + +async def test_a_refusal_after_the_plumbing_exists_keeps_what_was_built( + db_session: AsyncSession, +) -> None: + """The bucket, topic and queue are really in the account. Forgetting them + would leave resources teardown can never find; the identity is what is given + back, not the record of what was created. + """ + connection = await _project_connection(db_session, email="a@example.com") + identity = await _domain(db_session, connection) + FakeInbound.rule_error = APIError( + ErrorType.AUTHORIZATION_FAILED, "not permitted to call ses:CreateReceiptRule" + ) + + with pytest.raises(APIError): + await _enable(db_session, connection, identity) + + assert identity.receives_mail is False + assert connection.inbound_enabled is True + + +async def test_a_failed_repair_puts_back_the_rule_that_was_there( + db_session: AsyncSession, +) -> None: + """Running setup again on a domain that already receives must not leave it + blank if the second attempt fails. It was receiving before; it still is. + """ + connection = await _project_connection(db_session, email="a@example.com") + identity = await _domain(db_session, connection) + await _enable(db_session, connection, identity) + before = ( + identity.inbound_rule_name, + identity.inbound_rule_set, + identity.inbound_rule_set_created, + ) + FakeInbound.setup_error = APIError(ErrorType.PROVIDER_ERROR, "AWS is down") + + with pytest.raises(APIError): + await _enable(db_session, connection, identity) + + after = ( + identity.inbound_rule_name, + identity.inbound_rule_set, + identity.inbound_rule_set_created, + ) + assert after == before + assert identity.receives_mail is True + + # ---------------------------------------------------------------- teardown --- From eccf6d1331d5678b68aa095428a81a9f8ccc8b0f Mon Sep 17 00:00:00 2001 From: Otitodev <198799600+Otitodev@users.noreply.github.com> Date: Mon, 5 Oct 2026 08:45:38 +0100 Subject: [PATCH 02/10] refactor(api): share the download code between /v1 and the dashboard The dashboard inbox needs the same downloads as /v1, with session auth in place of an API key. Both serve the same hostile thing from the same origin, so the part that makes that safe is written once: the header set, the filename handling, the order of refusal, and building the response. A header added to one surface and forgotten on the other is the failure this prevents, and it would look like nothing until somebody pointed a browser at the wrong route. The previous change in this series was that exact shape - a policy set on a route and silently discarded - so this is not a hypothetical. What stays in the routes is what genuinely differs: how a caller is authenticated and how a refusal is shown. An API key and a JSON envelope on one side, a session and a page on the other. No behaviour change. The regenerated OpenAPI document is identical, and the download tests are unchanged apart from where they import the shared names from. --- apps/api/src/seskit_api/downloads.py | 134 +++++++++++++++++++ apps/api/src/seskit_api/routes/v1/inbound.py | 110 +++------------ tests/test_v1_inbound_downloads.py | 2 +- 3 files changed, 150 insertions(+), 96 deletions(-) create mode 100644 apps/api/src/seskit_api/downloads.py diff --git a/apps/api/src/seskit_api/downloads.py b/apps/api/src/seskit_api/downloads.py new file mode 100644 index 0000000..7fedcad --- /dev/null +++ b/apps/api/src/seskit_api/downloads.py @@ -0,0 +1,134 @@ +"""Serving a stranger's bytes: shared by ``/v1`` and the dashboard. + +Both surfaces let a signed-in person fetch an attachment or the original of a +message somebody sent to them, and both are serving the same hostile thing from the +same origin. Written once so the two cannot drift - a header added to one and +forgotten on the other is exactly the failure this exists to prevent, and it would +look like nothing at all until somebody pointed a browser at the wrong route. + +Whatever the sender declared, the response is a download and not a document. A +browser pointed at one must not render it, sniff it, run script in it, or keep it. + +What is here is the part that is the same: the headers, the filename, the order of +refusal, and building the response. What is *not* here is how a caller is +authenticated or how a failure is shown - an API key and a JSON envelope on one +side, a session and a page on the other - because those are what differ. +""" + +from __future__ import annotations + +import re +from urllib.parse import quote + +from fastapi import Response +from seskit_core.config import Settings +from seskit_core.email.parse import read_attachment +from seskit_core.errors import APIError, ErrorType +from seskit_core.models import InboundEmail +from seskit_core.models.base import utcnow +from seskit_core.services import InboundStoreFactory, get_connection, stored_credentials +from seskit_provider_aws_ses import EXPIRED_MESSAGE +from sqlalchemy.ext.asyncio import AsyncSession + +#: Applied to every download. +#: +#: ``nosniff`` stops a browser deciding the bytes are HTML because they look like +#: it. The CSP ``sandbox`` is what is left if one renders it anyway: no script, no +#: forms, no same-origin access to anything. ``no-store`` because this is somebody's +#: private mail and should not sit in a shared cache. +DOWNLOAD_HEADERS = { + "X-Content-Type-Options": "nosniff", + "Content-Security-Policy": "sandbox; default-src 'none'", + "Cache-Control": "private, no-store", +} + +NO_SUCH_ATTACHMENT = "That message has no attachment with that index." + +_NOT_SAFE = re.compile(r"[^A-Za-z0-9._ -]") + + +def content_disposition(filename: str) -> str: + """A ``Content-Disposition`` that is always a download, for any filename. + + Two forms, because old clients read the plain one and current ones read the + encoded one. The plain form is reduced to characters that cannot end the + quoted string or start a new header; the encoded form carries the name with + everything outside the unreserved set percent-encoded. + + Both are built from the same basename, so neither can carry a path or a name + that is only dots, and an empty name becomes ``attachment`` in both. A name + that came through the parser already has all of that - this does not rely on + it, because it is the last thing between a sender's string and a header, and + a client saving the file should not have to sanitise what it was given. + """ + base = filename.replace("\\", "/").rsplit("/", 1)[-1].strip(" .") or "attachment" + fallback = _NOT_SAFE.sub("_", base) + return f"attachment; filename=\"{fallback}\"; filename*=UTF-8''{quote(base, safe='')}" + + +def require_attachment(row: InboundEmail, index: int) -> None: + """Refuse an attachment that does not exist, before anything is fetched. + + A request for an attachment the message does not have should not cost a + download of the message to find that out. + """ + if index < 0 or index >= len(row.attachments): + raise APIError(ErrorType.NOT_FOUND, NO_SUCH_ATTACHMENT) + + +async def fetch_original( + db: AsyncSession, + row: InboundEmail, + factory: InboundStoreFactory, + settings: Settings, +) -> bytes: + """The stored message, or the right error for why not. + + The retention date is checked before anything is fetched. After it the original + is gone or about to be, and an S3 call to confirm costs a round trip that can + only produce the same answer - so a request past it is refused at once, and one + with no known expiry is left to the fetch to settle. + """ + if row.raw_expires_at is not None and row.raw_expires_at <= utcnow(): + raise APIError(ErrorType.NOT_FOUND, EXPIRED_MESSAGE) + + connection = await get_connection(db, row.project_id) + if connection is None or not connection.has_credentials: + raise APIError( + ErrorType.INVALID_REQUEST, + "This project is not connected to AWS, so the original message cannot be " + "fetched. Connect it on the AWS page.", + ) + + store = factory( + connection.region, stored_credentials(connection, secret_key=settings.SECRET_KEY) + ) + return await store.fetch_message(bucket=row.storage_bucket, key=row.storage_key) + + +def attachment_response(raw: bytes, index: int) -> Response: + """One attachment out of a stored message, as a download. + + Always ``application/octet-stream`` however it was declared. The filename comes + from the stored message, not from anything the caller passed. + """ + content = read_attachment(raw, index) + if content is None: + raise APIError(ErrorType.NOT_FOUND, NO_SUCH_ATTACHMENT) + return Response( + content=content.data, + media_type="application/octet-stream", + headers={**DOWNLOAD_HEADERS, "Content-Disposition": content_disposition(content.filename)}, + ) + + +def raw_response(raw: bytes, inbound_id: str) -> Response: + """The original message as an ``.eml`` download.""" + return Response( + content=raw, + media_type="message/rfc822", + headers={ + **DOWNLOAD_HEADERS, + "Content-Disposition": content_disposition(f"{inbound_id}.eml"), + }, + ) diff --git a/apps/api/src/seskit_api/routes/v1/inbound.py b/apps/api/src/seskit_api/routes/v1/inbound.py index a406397..c202bb6 100644 --- a/apps/api/src/seskit_api/routes/v1/inbound.py +++ b/apps/api/src/seskit_api/routes/v1/inbound.py @@ -10,19 +10,14 @@ from __future__ import annotations -import re from typing import Annotated, Any -from urllib.parse import quote from fastapi import APIRouter, Depends, Path, Query, Response from seskit_core.config import Settings from seskit_core.db import get_session -from seskit_core.email.parse import read_attachment from seskit_core.errors import APIError, ErrorType from seskit_core.models import InboundEmail -from seskit_core.models.base import utcnow -from seskit_core.services import InboundStoreFactory, get_connection, stored_credentials -from seskit_provider_aws_ses import EXPIRED_MESSAGE +from seskit_core.services import InboundStoreFactory from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession @@ -32,6 +27,12 @@ get_inbound_store_factory, require_api_key, ) +from seskit_api.downloads import ( + attachment_response, + fetch_original, + raw_response, + require_attachment, +) from seskit_api.routes.v1.api_keys import API_RESPONSES, apply_rate_limit_headers from seskit_api.schemas.inbound import ( InboundList, @@ -164,22 +165,10 @@ async def list_inbound( # ------------------------------------------------------------- downloads --- # -# Everything below serves bytes a stranger wrote, from the API's own origin. That -# is the whole risk, and it is handled the same way for both routes: whatever the -# sender declared, the response is a download and not a document. A browser that -# is pointed at one must not render it, sniff it, run script in it, or keep it. - -#: Applied to every download. -#: -#: ``nosniff`` stops a browser deciding the bytes are HTML because they look like -#: it. The CSP ``sandbox`` is what is left if one renders it anyway: no script, no -#: forms, no same-origin access to anything. ``no-store`` because this is somebody's -#: private mail and should not sit in a shared cache. -DOWNLOAD_HEADERS = { - "X-Content-Type-Options": "nosniff", - "Content-Security-Policy": "sandbox; default-src 'none'", - "Cache-Control": "private, no-store", -} +# Everything below serves bytes a stranger wrote, from the API's own origin. How +# that is made safe lives in ``seskit_api.downloads``, shared with the dashboard so +# the two cannot drift; what is here is only what is specific to ``/v1``: the API +# key, the rate-limit headers, and how a refusal is reported. _BINARY_OK: dict[int | str, dict[str, Any]] = { 200: { @@ -188,57 +177,6 @@ async def list_inbound( } } -_NOT_SAFE = re.compile(r"[^A-Za-z0-9._ -]") - - -def content_disposition(filename: str) -> str: - """A ``Content-Disposition`` that is always a download, for any filename. - - Two forms, because old clients read the plain one and current ones read the - encoded one. The plain form is reduced to characters that cannot end the - quoted string or start a new header; the encoded form carries the name with - everything outside the unreserved set percent-encoded. - - Both are built from the same basename, so neither can carry a path or a name - that is only dots, and an empty name becomes ``attachment`` in both. A name - that came through the parser already has all of that - this does not rely on - it, because it is the last thing between a sender's string and a header, and - a client saving the file should not have to sanitise what it was given. - """ - base = filename.replace("\\", "/").rsplit("/", 1)[-1].strip(" .") or "attachment" - fallback = _NOT_SAFE.sub("_", base) - return f"attachment; filename=\"{fallback}\"; filename*=UTF-8''{quote(base, safe='')}" - - -async def _original( - db: AsyncSession, - row: InboundEmail, - factory: InboundStoreFactory, - settings: Settings, -) -> bytes: - """The stored message, or the right error for why not. - - The retention date is checked before anything is fetched. After it the original - is gone or about to be, and an S3 call to confirm costs a round trip that can - only produce the same answer - so a request past it is refused at once, and one - with no known expiry is left to the fetch to settle. - """ - if row.raw_expires_at is not None and row.raw_expires_at <= utcnow(): - raise APIError(ErrorType.NOT_FOUND, EXPIRED_MESSAGE) - - connection = await get_connection(db, row.project_id) - if connection is None or not connection.has_credentials: - raise APIError( - ErrorType.INVALID_REQUEST, - "This project is not connected to AWS, so the original message cannot be " - "fetched. Connect it on the AWS page.", - ) - - store = factory( - connection.region, stored_credentials(connection, secret_key=settings.SECRET_KEY) - ) - return await store.fetch_message(bucket=row.storage_bucket, key=row.storage_key) - @router.get( "/inbound/{inbound_id}/attachments/{index}", @@ -271,21 +209,9 @@ async def download_attachment( row = await _owned(db, inbound_id, context.project.id) if row is None: raise APIError(ErrorType.NOT_FOUND, "No received email with that id.") - if index >= len(row.attachments): - # Before the fetch: a request for an attachment that does not exist should - # not cost a download of the message to find that out. - raise APIError(ErrorType.NOT_FOUND, "That message has no attachment with that index.") - - raw = await _original(db, row, factory, settings) - content = read_attachment(raw, index) - if content is None: - raise APIError(ErrorType.NOT_FOUND, "That message has no attachment with that index.") - - out = Response( - content=content.data, - media_type="application/octet-stream", - headers={**DOWNLOAD_HEADERS, "Content-Disposition": content_disposition(content.filename)}, - ) + require_attachment(row, index) + + out = attachment_response(await fetch_original(db, row, factory, settings), index) apply_rate_limit_headers(out, context) return out @@ -322,12 +248,6 @@ async def download_raw( if row is None: raise APIError(ErrorType.NOT_FOUND, "No received email with that id.") - raw = await _original(db, row, factory, settings) - - out = Response( - content=raw, - media_type="message/rfc822", - headers={**DOWNLOAD_HEADERS, "Content-Disposition": content_disposition(f"{row.id}.eml")}, - ) + out = raw_response(await fetch_original(db, row, factory, settings), row.id) apply_rate_limit_headers(out, context) return out diff --git a/tests/test_v1_inbound_downloads.py b/tests/test_v1_inbound_downloads.py index 6274de5..8cac634 100644 --- a/tests/test_v1_inbound_downloads.py +++ b/tests/test_v1_inbound_downloads.py @@ -20,7 +20,7 @@ from fakes.inbound import EXPIRED, FakeInboundStore from fakes.ses import FAKE_CREDENTIALS, connect_project from httpx import AsyncClient -from seskit_api.routes.v1.inbound import DOWNLOAD_HEADERS, content_disposition +from seskit_api.downloads import DOWNLOAD_HEADERS, content_disposition from seskit_core.errors import APIError, ErrorType from seskit_core.models import InboundEmail from sqlalchemy.ext.asyncio import AsyncSession From 448bad13cd7dd452ee25e3ef72f29ac823f866c9 Mon Sep 17 00:00:00 2001 From: Otitodev <198799600+Otitodev@users.noreply.github.com> Date: Mon, 5 Oct 2026 08:51:04 +0100 Subject: [PATCH 03/10] feat(ui): turn receiving on and off from the Domains page A verified domain's card gains a Receiving row and a panel to start it. Starting creates the bucket, topic and queue in the user's AWS account and adds the domain's rule; the card then shows the MX record to add, with a Copy on each value like the DNS records, and a Stop button. Before it is pressed the card says exactly what it will create in the user's own account and that SESKit never replaces a rule set they already have - the same rule the event reporting panel follows, because both put resources somewhere that then appears on somebody's bill. The confirmation names them again. The warning that matters most is about the MX record, and it is on the card and not in the docs. An MX record sends all of a domain's mail to Amazon SES, so a domain that receives mail somewhere else today stops receiving it there. The card says so and points at a subdomain, which works because the rule is per identity: add mail.example.com as its own domain. Every reason receiving is unavailable is stated and none is hidden - an unverified domain, no AWS connection, a domain in a different region from the connection, a region SES cannot receive in, an instance reading events over HTTPS only, a domain another project already receives for. A button that is simply absent leaves somebody wondering whether the feature exists. The refusal for the last one does not say whose domain it is. The server refuses for every reason the page disables the button for. The button is a courtesy and a form can be posted by hand; the same check runs again in the route, and tests post to a blocked domain directly. The most important test is a regression guard from earlier in this series. The claim on a domain is written before AWS is called, and these routes do not roll back on an error, so an AWS refusal would have rendered "Receiving is on" beside the reason it was not. The test makes AWS refuse and asserts on the HTML the user is shown. A refused stop leaves it on and says why, for the converse reason: AWS first, the row second. The page is rendered offline against the real templates in all three states before the tests were written - on, available, blocked with the button actually disabled - which caught nothing, and is how it is known the card still renders for callers that never pass the new variable. Removing a receiving domain and disconnecting AWS are tested through their real routes, so a user pressing either cannot strand a rule in their account. --- apps/api/src/seskit_api/receiving.py | 136 ++++++ apps/api/src/seskit_api/routes/domains.py | 176 +++++++- .../templates/partials/identity_row.html | 16 + .../templates/partials/receiving.html | 86 ++++ tests/fakes/inbound.py | 15 + tests/test_domains_receiving.py | 398 ++++++++++++++++++ tests/test_receiving_view.py | 215 ++++++++++ 7 files changed, 1040 insertions(+), 2 deletions(-) create mode 100644 apps/api/src/seskit_api/receiving.py create mode 100644 apps/api/src/seskit_api/templates/partials/receiving.html create mode 100644 tests/test_domains_receiving.py create mode 100644 tests/test_receiving_view.py diff --git a/apps/api/src/seskit_api/receiving.py b/apps/api/src/seskit_api/receiving.py new file mode 100644 index 0000000..a90931c --- /dev/null +++ b/apps/api/src/seskit_api/receiving.py @@ -0,0 +1,136 @@ +"""What the Domains page shows about receiving mail for one domain. + +A view model and nothing more: it decides whether receiving can be turned on for a +domain *right now*, and if not, says why in words a person can act on. The page +renders it, and the route that turns receiving on asks it again before doing +anything - the disabled button is a courtesy, and this is the check, because a +form can be posted by hand. + +**Every reason is stated rather than hidden.** A button that is simply absent leaves +somebody wondering whether the feature exists. Each of these is a precondition they +can fix, and the page says which. + +The MX record is built here and not in a template so that a test can assert on it. +It is the one thing a user has to do by hand, outside SESKit, and a record with the +wrong host would send their mail nowhere without an error anywhere. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from seskit_core.config import Settings +from seskit_core.models import AWSConnection, Identity +from seskit_provider_aws_ses import supports_receiving + +#: SES takes mail at the lowest-preference host. One host, so one value; the number +#: only matters when a domain lists several, and nobody should be listing others +#: beside this one. +MX_PRIORITY = 10 + +#: Where Amazon SES takes mail in a region. Published by AWS in the General +#: Reference's "Email Receiving endpoints" table, one per receiving region. +MX_HOST = "inbound-smtp.{region}.amazonaws.com" + + +@dataclass(frozen=True, slots=True) +class MxRecord: + """The record to add at the user's DNS provider.""" + + name: str + priority: int + value: str + + @property + def zone_line(self) -> str: + """As one line, for a provider that takes a zone file.""" + return f"{self.name} MX {self.priority} {self.value}" + + +@dataclass(frozen=True, slots=True) +class ReceivingView: + """Receiving for one domain, as the page needs it.""" + + enabled: bool + #: Why it cannot be turned on right now, or ``None`` if it can. Always ``None`` + #: when it is already on - a running thing is not blocked, and the controls + #: that matter then are the ones to stop it. + blocked: str | None + mx: MxRecord + retention_days: int + + +def mx_record(identity: Identity) -> MxRecord: + return MxRecord( + name=identity.value, + priority=MX_PRIORITY, + value=MX_HOST.format(region=identity.region), + ) + + +def blocked_reason( + identity: Identity, + connection: AWSConnection | None, + settings: Settings, + *, + taken_elsewhere: bool, +) -> str | None: + """The first thing in the way of turning receiving on, in the order a person + would fix them, or ``None`` if nothing is. + """ + if not identity.is_verified: + return "Verify this domain first. Receiving needs a domain Amazon SES knows is yours." + + if connection is None or not connection.is_connected or not connection.has_credentials: + return "Connect AWS first, on the AWS page." + + if identity.region != connection.region: + return ( + f"This domain is verified in {identity.region}, but this project is connected to " + f"{connection.region}. Mail is received in the connection's region." + ) + + if not supports_receiving(identity.region): + return ( + f"Amazon SES cannot receive mail in {identity.region}. It can in regions such as " + "us-east-1, eu-west-1 and ap-southeast-2." + ) + + if not settings.polls_sqs: + return ( + "Receiving needs this instance to read from SQS, and EVENT_INGESTION is set to " + "https. Set it to sqs or both." + ) + + if taken_elsewhere: + return ( + "This domain already receives mail for another project or region. A domain's MX " + "record can only point at one place." + ) + + return None + + +def receiving_view( + identity: Identity, + connection: AWSConnection | None, + settings: Settings, + *, + taken_elsewhere: bool = False, +) -> ReceivingView | None: + """``None`` for something that cannot receive at all - an email address has no + MX record to point anywhere - so the page shows nothing rather than a control + that could never work. + """ + if not identity.is_domain: + return None + + enabled = identity.receives_mail + return ReceivingView( + enabled=enabled, + blocked=None + if enabled + else blocked_reason(identity, connection, settings, taken_elsewhere=taken_elsewhere), + mx=mx_record(identity), + retention_days=settings.INBOUND_RETENTION_DAYS, + ) diff --git a/apps/api/src/seskit_api/routes/domains.py b/apps/api/src/seskit_api/routes/domains.py index 931ffbe..ddb085b 100644 --- a/apps/api/src/seskit_api/routes/domains.py +++ b/apps/api/src/seskit_api/routes/domains.py @@ -21,7 +21,7 @@ from seskit_core.db import get_session from seskit_core.errors import APIError, ErrorType from seskit_core.logging import get_logger -from seskit_core.models import Project +from seskit_core.models import AWSConnection, Identity, Project from seskit_core.redis import get_redis from seskit_core.services import ( InboundProvisionerFactory, @@ -33,7 +33,10 @@ list_projects, refresh_identity, remove_identity, + setup_receiving, + teardown_receiving, ) +from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from seskit_api.dependencies import ( @@ -45,6 +48,7 @@ require_user, verify_csrf, ) +from seskit_api.receiving import ReceivingView, receiving_view from seskit_api.templating import render logger = get_logger(__name__) @@ -68,6 +72,7 @@ async def _page( status_code: int = 200, ) -> HTMLResponse: connection = await get_connection(db, project.id) + identities = await list_identities(db, project.id) return render( request, "pages/domains.html", @@ -78,11 +83,57 @@ async def _page( project=project, projects=await list_projects(db, current.user.id), connection=connection, - identities=await list_identities(db, project.id), + identities=identities, + receiving=await _receiving_views(request, db, project, connection, identities), error=error, ) +async def _receiving_views( + request: Request, + db: AsyncSession, + project: Project, + connection: AWSConnection | None, + identities: list[Identity], +) -> dict[str, ReceivingView]: + """Receiving for each domain, keyed by identity id. + + Computed here, once, so every handler that re-renders the page - add, refresh, + remove, and the two below - shows the same answer without each having to ask. + """ + settings: Settings = request.app.state.settings + taken = await _receiving_elsewhere(db, project.id, [i.value for i in identities if i.is_domain]) + views: dict[str, ReceivingView] = {} + for identity in identities: + view = receiving_view( + identity, + connection, + settings, + taken_elsewhere=identity.value in taken, + ) + if view is not None: + views[identity.id] = view + return views + + +async def _receiving_elsewhere(db: AsyncSession, project_id: str, values: list[str]) -> set[str]: + """Which of these domains already receive mail through another project. + + The database refuses a second receiver for a domain outright; this is so the + page can say so before somebody presses a button that cannot work. + """ + if not values: + return set() + rows = await db.scalars( + select(Identity.value).where( + Identity.value.in_(values), + Identity.inbound_rule_name.is_not(None), + Identity.project_id != project_id, + ) + ) + return set(rows) + + @router.get("/domains", response_class=HTMLResponse, summary="Sending identities") async def domains_page( request: Request, @@ -241,3 +292,124 @@ def _status_for(error: APIError) -> int: if error.error_type in {ErrorType.AUTHORIZATION_FAILED, ErrorType.AUTHENTICATION_FAILED}: return 400 return error.status_code + + +@router.post( + "/domains/{identity_id}/receiving", + response_class=HTMLResponse, + dependencies=[Depends(verify_csrf)], + summary="Start receiving mail for a domain", +) +async def start_receiving( + request: Request, + identity_id: str, + db: Annotated[AsyncSession, Depends(get_session)], + current: Annotated[CurrentUser, Depends(require_user)], + project: Annotated[Project, Depends(require_project)], + inbound: Annotated[InboundProvisionerFactory, Depends(get_inbound_provisioner_factory)], + settings: Annotated[Settings, Depends(get_app_settings)], +) -> HTMLResponse: + """Create the receiving plumbing in the user's AWS account and add this + domain's rule. + + Refuses on the server for every reason the page shows a disabled button for: + the button is a courtesy, and a form can be posted by hand. + + No rollback on an error, as on the AWS page. A rollback expires every loaded + object and the template then lazy-loads from inside sync Jinja, so the user + sees a 500 instead of the message. ``setup_receiving`` gives the domain back + itself when AWS refuses, which is what makes that safe. + """ + identity = await get_owned_identity(db, identity_id=identity_id, project_id=project.id) + if identity is None: + return await _page(request, db, current, project) + + connection = await get_connection(db, project.id) + taken = await _receiving_elsewhere(db, project.id, [identity.value]) + view = receiving_view(identity, connection, settings, taken_elsewhere=identity.value in taken) + if view is None: + return await _page( + request, + db, + current, + project, + error="Only a domain can receive mail.", + status_code=400, + ) + if view.blocked and connection is not None: + return await _page(request, db, current, project, error=view.blocked, status_code=400) + if connection is None: + return await _page( + request, db, current, project, error=NO_CONNECTION_MESSAGE, status_code=400 + ) + + try: + await setup_receiving( + db, + inbound, + connection, + identity, + resource_prefix=settings.EVENT_RESOURCE_PREFIX, + retention_days=settings.INBOUND_RETENTION_DAYS, + secret_key=settings.SECRET_KEY, + ) + except APIError as error: + return await _page( + request, db, current, project, error=error.message, status_code=_status_for(error) + ) + + await db.commit() + return await _page( + request, + db, + current, + project, + flash=( + f"Receiving is on for {identity.value}. SESKit created a bucket, a topic, a queue " + "and one receipt rule. Add the MX record below to start receiving." + ), + ) + + +@router.post( + "/domains/{identity_id}/receiving/stop", + response_class=HTMLResponse, + dependencies=[Depends(verify_csrf)], + summary="Stop receiving mail for a domain", +) +async def stop_receiving( + request: Request, + identity_id: str, + db: Annotated[AsyncSession, Depends(get_session)], + current: Annotated[CurrentUser, Depends(require_user)], + project: Annotated[Project, Depends(require_project)], + inbound: Annotated[InboundProvisionerFactory, Depends(get_inbound_provisioner_factory)], + settings: Annotated[Settings, Depends(get_app_settings)], +) -> HTMLResponse: + """Remove this domain's receipt rule, and the plumbing if nothing else uses it. + + Mail already received is kept. Stopping receiving is not deleting what arrived. + """ + identity = await get_owned_identity(db, identity_id=identity_id, project_id=project.id) + connection = await get_connection(db, project.id) + if identity is None or connection is None: + return await _page(request, db, current, project) + + try: + await teardown_receiving(db, inbound, connection, identity, secret_key=settings.SECRET_KEY) + except APIError as error: + return await _page( + request, db, current, project, error=error.message, status_code=_status_for(error) + ) + + await db.commit() + return await _page( + request, + db, + current, + project, + flash=( + f"Stopped receiving for {identity.value}. Mail already received is kept. " + "Remember to remove the MX record." + ), + ) diff --git a/apps/api/src/seskit_api/templates/partials/identity_row.html b/apps/api/src/seskit_api/templates/partials/identity_row.html index afd172e..6c56037 100644 --- a/apps/api/src/seskit_api/templates/partials/identity_row.html +++ b/apps/api/src/seskit_api/templates/partials/identity_row.html @@ -47,6 +47,18 @@
Region
{{ identity.region }}
+ {% if receiving is defined and receiving.get(identity.id) %} +
+
Receiving
+
+ {% if receiving[identity.id].enabled %} + {{ badge("On", tone="success") }} + {% else %} + {{ badge("Off", dot=False) }} + {% endif %} +
+
+ {% endif %}
Last checked
@@ -81,6 +93,10 @@ {% endif %} {% endif %} + {% if receiving is defined and receiving.get(identity.id) %} + {% include "partials/receiving.html" %} + {% endif %} +
diff --git a/apps/api/src/seskit_api/templates/partials/receiving.html b/apps/api/src/seskit_api/templates/partials/receiving.html new file mode 100644 index 0000000..127e223 --- /dev/null +++ b/apps/api/src/seskit_api/templates/partials/receiving.html @@ -0,0 +1,86 @@ +{# Receiving mail for one domain. + + Shown on a domain's card. Says what turning it on will create in the user's own + AWS account before the button is pressed - the same rule the event reporting + panel follows, because both put resources in somebody else's account that then + appear on their bill - and says plainly why it is unavailable when it is, rather + than hiding the control. #} +{% from "components/ui.html" import button, copy_value, code_block %} + +{% set view = receiving[identity.id] %} + +
+ {% if view.enabled %} +
+

Receiving is on

+

+ Mail sent to addresses at {{ identity.value }} is stored by + SESKit once the record below is in your DNS. The original message is kept + for {{ view.retention_days }} days; the parsed message stays after that, + but its attachments can no longer be downloaded. +

+
+ +

Add this MX record at your DNS provider.

+
+ + + + + + + + + + + + + + + + + +
TypeNamePriorityValue
MX{{ copy_value(view.mx.name) }}{{ view.mx.priority }}{{ copy_value(view.mx.value) }}
+
+ {{ code_block(view.mx.zone_line, language="DNS") }} + +
+

This replaces wherever the domain receives mail now

+

+ An MX record sends all of {{ identity.value }}'s + mail to Amazon SES. If it receives mail somewhere else today, that mail + stops arriving there. To keep that, receive on a subdomain such as + mail.{{ identity.value }} instead: add it above as its own + domain, verify it, and turn receiving on there. +

+
+ + + + {{ button("Stop receiving", variant="danger", type="submit") }} + + {% else %} +

+ Turn this on to have SESKit receive mail sent to + {{ identity.value }}, so your application can read it and be told + when it arrives. +

+

+ It creates, in your AWS account: an S3 bucket to hold the messages, an SNS + topic and an SQS queue, and one receipt rule for this domain added to your + active Amazon SES rule set. SESKit never replaces a rule set you already + have, and removes only what it created. AWS bills these to your account. +

+ + {% if view.blocked %} +
+

{{ view.blocked }}

+
+ {% endif %} + +
+ + {{ button("Start receiving", variant="primary", type="submit", disabled=view.blocked is not none) }} +
+ {% endif %} +
diff --git a/tests/fakes/inbound.py b/tests/fakes/inbound.py index 7c82236..6c20577 100644 --- a/tests/fakes/inbound.py +++ b/tests/fakes/inbound.py @@ -53,6 +53,17 @@ class FakeInboundProvisioner: """A provisioner that builds nothing and says it did.""" calls: ClassVar[list[str]] = [] + #: Raised by ``provision_inbound`` when set, to stand in for AWS refusing - a + #: missing permission, say. + error: ClassVar[APIError | None] = None + #: Raised by ``remove_inbound_rule`` when set. + remove_error: ClassVar[APIError | None] = None + + @classmethod + def reset(cls) -> None: + cls.calls = [] + cls.error = None + cls.remove_error = None def __init__(self, region: str, credentials: AWSCredentials | None = None) -> None: self.region = region @@ -61,6 +72,8 @@ def __init__(self, region: str, credentials: AWSCredentials | None = None) -> No async def provision_inbound( self, *, bucket_name: str, topic_name: str, queue_name: str, retention_days: int ) -> InboundInfrastructure: + if FakeInboundProvisioner.error is not None: + raise FakeInboundProvisioner.error FakeInboundProvisioner.calls.append("provision") return InboundInfrastructure( bucket=bucket_name, @@ -77,6 +90,8 @@ async def add_inbound_rule( return InboundRule(name=rule_name, rule_set="seskit-inbound", created_rule_set=True) async def remove_inbound_rule(self, rule: InboundRule) -> None: + if FakeInboundProvisioner.remove_error is not None: + raise FakeInboundProvisioner.remove_error FakeInboundProvisioner.calls.append("remove_rule") async def remove_inbound(self, infrastructure: InboundInfrastructure) -> bool: diff --git a/tests/test_domains_receiving.py b/tests/test_domains_receiving.py new file mode 100644 index 0000000..e756f76 --- /dev/null +++ b/tests/test_domains_receiving.py @@ -0,0 +1,398 @@ +"""Turning receiving on and off from the Domains page (inbound email, Phase D). + +What is checked is what a signed-in person can do, what they are told, and - the +reason most of these exist - what is *not* allowed to happen however the request +arrives. The disabled button is a courtesy; the server refuses for the same +reasons, because a form can be posted by hand. + +The most important test here is a regression guard from earlier in the series: an +AWS refusal must not leave the page saying "Receiving is on". The routes do not +roll back on an error, so that depends on the service giving the domain back, and +the only way to see it is in the HTML the user is shown. +""" + +from __future__ import annotations + +import pytest +from fakes.inbound import FakeInboundProvisioner +from httpx import AsyncClient +from seskit_core.errors import APIError, ErrorType +from seskit_core.models import AWSConnection, Identity, Project +from seskit_core.services import create_project, register_user +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession +from test_domains_page import _connect + +DOMAIN = "example.com" +MX_VALUE = "inbound-smtp.us-east-1.amazonaws.com" + + +@pytest.fixture(autouse=True) +def _reset() -> None: + FakeInboundProvisioner.reset() + + +async def _project(session: AsyncSession) -> Project: + project = await session.scalar(select(Project)) + assert project is not None + return project + + +async def _domain( + session: AsyncSession, + project: Project, + value: str = DOMAIN, + *, + verified: bool = True, + kind: str = "domain", + rule: str | None = None, + region: str = "us-east-1", +) -> Identity: + identity = Identity( + project_id=project.id, + identity_type=kind, + value=value, + region=region, + verification_status="success" if verified else "pending", + dkim_status="success" if verified else "pending", + dkim_tokens=[], + inbound_rule_name=rule, + inbound_rule_set="seskit-inbound" if rule else None, + ) + session.add(identity) + await session.flush() + return identity + + +async def _fresh(session: AsyncSession, identity: Identity) -> Identity: + await session.refresh(identity) + return identity + + +def _start(identity: Identity) -> str: + return f"/domains/{identity.id}/receiving" + + +def _stop(identity: Identity) -> str: + return f"/domains/{identity.id}/receiving/stop" + + +# ------------------------------------------------------------------- page --- + + +async def test_a_verified_domain_offers_to_start_receiving( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + await _connect(app_client) + await _domain(db_session, await _project(db_session)) + + page = await app_client.get("/domains") + + assert "Start receiving" in page.text + assert "Receiving" in page.text + assert "Receiving is on" not in page.text + # It says what it will create in the user's own account before it is pressed. + assert "an S3 bucket" in page.text + assert "never replaces a rule set you already have" in page.text + + +async def test_the_controls_are_forms_with_csrf_not_links( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """A link would let any page on the internet start creating resources in + somebody's AWS account with an tag. + """ + await _connect(app_client) + identity = await _domain(db_session, await _project(db_session)) + + page = await app_client.get("/domains") + + assert f'
' in page.text + assert f'href="/domains/{identity.id}/receiving' not in page.text + + +async def test_an_unverified_domain_has_a_disabled_button_and_a_reason( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + await _connect(app_client) + await _domain(db_session, await _project(db_session), verified=False) + + page = await app_client.get("/domains") + + assert "Verify this domain first" in page.text + button = page.text[ + page.text.index("Start receiving") - 120 : page.text.index("Start receiving") + ] + assert "disabled" in button + + +async def test_an_email_address_has_no_receiving_row( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + await _connect(app_client) + await _domain(db_session, await _project(db_session), "me@example.com", kind="email_address") + + page = await app_client.get("/domains") + + assert "Receiving" not in page.text + assert "Start receiving" not in page.text + + +# ----------------------------------------------------------------- starting --- + + +async def test_starting_shows_the_mx_record_to_add( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + token = await _connect(app_client) + identity = await _domain(db_session, await _project(db_session)) + + page = await app_client.post(_start(identity), data={"csrf_token": token}) + + assert page.status_code == 200 + assert "Receiving is on" in page.text + assert MX_VALUE in page.text + assert f'data-copy="{MX_VALUE}"' in page.text + assert "10" in page.text + # And the warning that matters most about an MX record. + assert "replaces wherever the domain receives mail now" in page.text + assert f"mail.{DOMAIN}" in page.text + assert FakeInboundProvisioner.calls == ["provision", "add_rule"] + assert (await _fresh(db_session, identity)).receives_mail is True + + +async def test_starting_names_what_was_created( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """Because it is in the user's own account and now on their bill.""" + token = await _connect(app_client) + identity = await _domain(db_session, await _project(db_session)) + + page = await app_client.post(_start(identity), data={"csrf_token": token}) + + assert "a bucket, a topic, a queue" in page.text + assert "one receipt rule" in page.text + + +async def test_an_aws_refusal_does_not_leave_the_page_saying_receiving_is_on( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """The regression guard. The claim on the domain is written before AWS is + called, and these routes do not roll back on an error - so without the service + giving the domain back, this page would render "Receiving is on" beside the + reason it is not. Checked in the HTML the user is shown, which is the only place + it can be seen. + """ + token = await _connect(app_client) + identity = await _domain(db_session, await _project(db_session)) + FakeInboundProvisioner.error = APIError( + ErrorType.AUTHORIZATION_FAILED, + "The AWS identity is not permitted to call s3:CreateBucket. Add it to its IAM policy.", + ) + + page = await app_client.post(_start(identity), data={"csrf_token": token}) + + assert page.status_code == 403 + assert "not permitted to call s3:CreateBucket" in page.text + assert "Receiving is on" not in page.text + assert "Start receiving" in page.text + assert (await _fresh(db_session, identity)).receives_mail is False + + +async def test_a_post_without_a_csrf_token_creates_nothing( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + await _connect(app_client) + identity = await _domain(db_session, await _project(db_session)) + + response = await app_client.post(_start(identity), data={}) + + assert response.status_code == 403 + assert FakeInboundProvisioner.calls == [] + assert (await _fresh(db_session, identity)).receives_mail is False + + +async def test_the_server_refuses_what_the_disabled_button_refuses( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """A form can be posted by hand. The button being disabled protects nobody.""" + token = await _connect(app_client) + identity = await _domain(db_session, await _project(db_session), verified=False) + + page = await app_client.post(_start(identity), data={"csrf_token": token}) + + assert page.status_code == 400 + assert "Verify this domain first" in page.text + assert FakeInboundProvisioner.calls == [] + assert (await _fresh(db_session, identity)).receives_mail is False + + +async def test_a_region_that_cannot_receive_is_refused_on_the_server( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + token = await _connect(app_client) + project = await _project(db_session) + connection = await db_session.scalar(select(AWSConnection)) + assert connection is not None + connection.region = "ap-south-2" + identity = await _domain(db_session, project, region="ap-south-2") + await db_session.flush() + + page = await app_client.post(_start(identity), data={"csrf_token": token}) + + assert page.status_code == 400 + assert "cannot receive mail in ap-south-2" in page.text + assert FakeInboundProvisioner.calls == [] + + +async def test_an_email_address_cannot_be_made_to_receive( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + token = await _connect(app_client) + identity = await _domain( + db_session, await _project(db_session), "me@example.com", kind="email_address" + ) + + page = await app_client.post(_start(identity), data={"csrf_token": token}) + + assert page.status_code == 400 + assert "Only a domain can receive mail" in page.text + assert FakeInboundProvisioner.calls == [] + + +async def test_a_domain_already_receiving_for_another_project_is_refused( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + token = await _connect(app_client) + mine = await _domain(db_session, await _project(db_session)) + other_user = await register_user( + db_session, email="other@example.com", password="correct-horse-battery", allow_signup=True + ) + other = await create_project(db_session, user_id=other_user.id, name="Other") + await _domain(db_session, other, rule="seskit-theirs") + + page = await app_client.post(_start(mine), data={"csrf_token": token}) + + assert page.status_code == 400 + assert "already receives mail for another project or region" in page.text + assert "other@example.com" not in page.text + assert "Other" not in page.text.replace("Otherwise", "") + assert FakeInboundProvisioner.calls == [] + assert (await _fresh(db_session, mine)).receives_mail is False + + +async def test_somebody_elses_domain_cannot_be_started( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """An id from another project resolves to nothing, not to somebody's domain.""" + token = await _connect(app_client) + await _domain(db_session, await _project(db_session), "mine.example") + other_user = await register_user( + db_session, email="other@example.com", password="correct-horse-battery", allow_signup=True + ) + other = await create_project(db_session, user_id=other_user.id, name="Other") + theirs = await _domain(db_session, other, "theirs.example") + + page = await app_client.post(_start(theirs), data={"csrf_token": token}) + + assert page.status_code == 200 + assert FakeInboundProvisioner.calls == [] + assert (await _fresh(db_session, theirs)).receives_mail is False + assert "theirs.example" not in page.text + + +# ----------------------------------------------------------------- stopping --- + + +async def test_stopping_removes_the_rule_and_says_what_is_kept( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + token = await _connect(app_client) + identity = await _domain(db_session, await _project(db_session)) + await app_client.post(_start(identity), data={"csrf_token": token}) + FakeInboundProvisioner.calls = [] + + page = await app_client.post(_stop(identity), data={"csrf_token": token}) + + assert page.status_code == 200 + assert FakeInboundProvisioner.calls == ["remove_rule", "remove"] + assert "Mail already received is kept" in page.text + assert "remove the MX record" in page.text + assert "Receiving is on" not in page.text + assert "Start receiving" in page.text + assert (await _fresh(db_session, identity)).receives_mail is False + + +async def test_a_refused_stop_leaves_it_on_and_says_why( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """AWS first, the row second. If the rule cannot be removed it is still there, + and a page that said it had stopped would be wrong about somebody's account. + """ + token = await _connect(app_client) + identity = await _domain(db_session, await _project(db_session)) + await app_client.post(_start(identity), data={"csrf_token": token}) + FakeInboundProvisioner.remove_error = APIError( + ErrorType.AUTHORIZATION_FAILED, "not permitted to call ses:DeleteReceiptRule" + ) + + page = await app_client.post(_stop(identity), data={"csrf_token": token}) + + assert page.status_code == 403 + assert "not permitted to call ses:DeleteReceiptRule" in page.text + assert "Receiving is on" in page.text + assert (await _fresh(db_session, identity)).receives_mail is True + + +async def test_stopping_somebody_elses_domain_does_nothing( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + token = await _connect(app_client) + await _domain(db_session, await _project(db_session), "mine.example") + other_user = await register_user( + db_session, email="other@example.com", password="correct-horse-battery", allow_signup=True + ) + other = await create_project(db_session, user_id=other_user.id, name="Other") + theirs = await _domain(db_session, other, "theirs.example", rule="seskit-theirs") + + await app_client.post(_stop(theirs), data={"csrf_token": token}) + + assert FakeInboundProvisioner.calls == [] + assert (await _fresh(db_session, theirs)).receives_mail is True + + +# --------------------------------------------- what else removes a receiver --- + + +async def test_removing_a_receiving_domain_removes_its_rule_first( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """Through the real route, not just the service: a user pressing Remove on a + domain that receives mail must not strand a rule in their AWS account. + """ + token = await _connect(app_client) + identity = await _domain(db_session, await _project(db_session)) + await app_client.post(_start(identity), data={"csrf_token": token}) + FakeInboundProvisioner.calls = [] + + page = await app_client.post(f"/domains/{identity.id}/delete", data={"csrf_token": token}) + + assert page.status_code == 200 + assert "remove_rule" in FakeInboundProvisioner.calls + assert await db_session.scalar(select(Identity).where(Identity.id == identity.id)) is None + + +async def test_disconnecting_aws_removes_the_receipt_rules_first( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """Once the connection is gone there is no key left to remove them with.""" + token = await _connect(app_client) + identity = await _domain(db_session, await _project(db_session)) + await app_client.post(_start(identity), data={"csrf_token": token}) + FakeInboundProvisioner.calls = [] + + page = await app_client.post("/aws/disconnect", data={"csrf_token": token}) + + assert page.status_code == 200 + assert "remove_rule" in FakeInboundProvisioner.calls + assert (await _fresh(db_session, identity)).receives_mail is False diff --git a/tests/test_receiving_view.py b/tests/test_receiving_view.py new file mode 100644 index 0000000..a0f0aee --- /dev/null +++ b/tests/test_receiving_view.py @@ -0,0 +1,215 @@ +"""What the Domains page says about receiving mail (inbound email, Phase D). + +The view model decides whether receiving can be turned on for a domain right now, +and says why in words a person can act on when it cannot. The route that turns it +on asks it again before doing anything - the disabled button is a courtesy and +this is the check, since a form can be posted by hand - so what is asserted here is +the contract both rely on. None of it needs a database. + +The MX record is worth its own tests. It is the one thing a user has to do by hand, +outside SESKit, and one with the wrong host would send their mail nowhere without +an error anywhere. +""" + +from __future__ import annotations + +import pytest +from seskit_api.receiving import MX_PRIORITY, mx_record, receiving_view +from seskit_core.config import Settings +from seskit_core.models import AWSConnection, ConnectionStatus, Identity +from seskit_provider_aws_ses import RECEIVING_REGION_CODES + +REGION = "us-east-1" + + +def _settings(**overrides: str) -> Settings: + fields = { + "SECRET_KEY": "a-real-secret", + "DATABASE_URL": "postgresql+asyncpg://u:p@localhost:5432/db", + "REDIS_URL": "redis://localhost:6379/0", + "SMTP_HOST": None, + "EMAILS_FROM_EMAIL": None, + } + return Settings(_env_file=None, **(fields | overrides)) # type: ignore[arg-type] + + +def _identity( + value: str = "example.com", + *, + kind: str = "domain", + verified: bool = True, + region: str = REGION, + rule: str | None = None, +) -> Identity: + return Identity( + id="dom_1", + project_id="proj_1", + identity_type=kind, + value=value, + region=region, + verification_status="success" if verified else "pending", + dkim_tokens=[], + inbound_rule_name=rule, + ) + + +def _connection( + *, region: str = REGION, connected: bool = True, keyed: bool = True +) -> AWSConnection: + return AWSConnection( + project_id="proj_1", + aws_account_id="123456789012", + region=region, + status=(ConnectionStatus.CONNECTED if connected else ConnectionStatus.ERROR).value, + aws_access_key_id="AKIAEXAMPLE" if keyed else None, + aws_secret_access_key_encrypted="enc" if keyed else None, + ) + + +def _blocked(identity: Identity, connection: AWSConnection | None, **kwargs: object) -> str | None: + settings = kwargs.pop("settings", None) or _settings() + view = receiving_view(identity, connection, settings, **kwargs) # type: ignore[arg-type] + assert view is not None + return view.blocked + + +# ----------------------------------------------------------- the MX record --- + + +def test_the_mx_record_points_at_the_ses_endpoint_for_the_domains_own_region() -> None: + """Mail is received in the region the domain is verified in. A record naming + any other would deliver nowhere, silently. + """ + record = mx_record(_identity(region="eu-west-1")) + + assert record.name == "example.com" + assert record.value == "inbound-smtp.eu-west-1.amazonaws.com" + assert record.priority == MX_PRIORITY == 10 + assert record.zone_line == "example.com MX 10 inbound-smtp.eu-west-1.amazonaws.com" + + +@pytest.mark.parametrize("region", sorted(RECEIVING_REGION_CODES)) +def test_every_receiving_region_has_the_endpoint_ses_publishes(region: str) -> None: + assert mx_record(_identity(region=region)).value == f"inbound-smtp.{region}.amazonaws.com" + + +# ---------------------------------------------------------- when it is on --- + + +def test_a_domain_that_can_receive_is_not_blocked() -> None: + view = receiving_view(_identity(), _connection(), _settings()) + + assert view is not None + assert view.enabled is False + assert view.blocked is None + assert view.retention_days == 30 + + +def test_a_domain_already_receiving_is_never_blocked() -> None: + """A running thing is not "blocked", whatever has changed since. Reporting an + unsupported region over a working rule would hide the control that matters, + which is the one to stop it. + """ + view = receiving_view( + _identity(rule="seskit-x", region="ap-south-2"), + _connection(connected=False), + _settings(), + taken_elsewhere=True, + ) + + assert view is not None + assert view.enabled is True + assert view.blocked is None + + +def test_the_retention_shown_is_the_configured_one() -> None: + view = receiving_view(_identity(), _connection(), _settings(INBOUND_RETENTION_DAYS="45")) + + assert view is not None + assert view.retention_days == 45 + + +def test_an_email_address_gets_no_receiving_panel_at_all() -> None: + """An address verifies a sender and has no MX record to point anywhere. Showing + a control that could never work would be a puzzle. + """ + assert ( + receiving_view(_identity("a@example.com", kind="email_address"), _connection(), _settings()) + is None + ) + + +# ----------------------------------------------------- why it cannot be on --- + + +def test_an_unverified_domain_is_told_to_verify_first() -> None: + reason = _blocked(_identity(verified=False), _connection()) + + assert reason is not None + assert "Verify this domain first" in reason + + +@pytest.mark.parametrize( + "connection", + [None, _connection(connected=False), _connection(keyed=False)], + ids=["no connection", "errored connection", "no stored key"], +) +def test_a_project_that_cannot_reach_aws_is_told_to_connect( + connection: AWSConnection | None, +) -> None: + reason = _blocked(_identity(), connection) + + assert reason is not None + assert "Connect AWS first" in reason + + +def test_a_domain_in_another_region_than_the_connection_says_both() -> None: + reason = _blocked(_identity(region="eu-west-1"), _connection(region="us-east-1")) + + assert reason is not None + assert "eu-west-1" in reason + assert "us-east-1" in reason + + +@pytest.mark.parametrize("region", ["ap-south-2", "ap-southeast-5", "eu-central-2", "ca-west-1"]) +def test_a_region_that_can_send_but_not_receive_says_so_and_offers_ones_that_can( + region: str, +) -> None: + reason = _blocked(_identity(region=region), _connection(region=region)) + + assert reason is not None + assert f"cannot receive mail in {region}" in reason + assert "us-east-1" in reason + + +def test_an_instance_that_only_takes_events_over_https_is_told_what_to_change() -> None: + settings = _settings(EVENT_INGESTION="https", PUBLIC_BASE_URL="https://seskit.example.com") + + reason = _blocked(_identity(), _connection(), settings=settings) + + assert reason is not None + assert "EVENT_INGESTION" in reason + assert "sqs" in reason + + +def test_a_domain_receiving_for_somebody_else_says_a_domain_can_receive_in_one_place() -> None: + reason = _blocked(_identity(), _connection(), taken_elsewhere=True) + + assert reason is not None + assert "another project or region" in reason + assert "MX" in reason + + +def test_the_first_thing_in_the_way_is_the_one_reported() -> None: + """In the order a person would fix them: verification before connection before + region. Reporting the last problem first would send them to fix something that + was never the obstacle. + """ + reason = _blocked( + _identity(verified=False, region="ap-south-2"), + None, + taken_elsewhere=True, + ) + + assert reason is not None + assert "Verify this domain first" in reason From 4150be8ae394c66f377cff56040d0732a6f66b9a Mon Sep 17 00:00:00 2001 From: Otitodev <198799600+Otitodev@users.noreply.github.com> Date: Mon, 5 Oct 2026 08:58:38 +0100 Subject: [PATCH 04/10] feat(ui): an Inbox for received mail A new Inbox page lists what the project received, newest first, with a domain filter that swaps the table over HTMX like the Emails page, and a detail page for one message: who it is from and to, who it was actually delivered to, what Amazon SES concluded, the text body, the HTML as source, the attachments, the headers, and the original. Everything on these pages was written by a stranger, and that is what most of the tests are about. Each sender-controlled field - subject, display name, both bodies, every address list, the message id, an attachment's filename, a header's name and value - is given a script tag and an onerror handler, and the assertion is that none reaches the response as markup. The page holds a signed-in person's session; a stranger's HTML does not belong beside it. The HTML body is shown as source here, and a test sends an iframe and a paragraph and asserts neither exists in the page while both survive as visible text. The templates were rendered offline with the same hostile input before any test was written. A verdict SES did not give reads "Not checked" and is deliberately not styled like a pass; a test asserts five of them on a message with none. The macro that does this lives in ui.html with the other components. The page says plainly that SESKit shows the verdicts and acts on none. An attachment's declared content type is labelled "declared", because the download is opaque bytes whatever it says. Downloads use the shared module, so they carry the same headers as /v1, and a test asserts the sandbox CSP on the response: the middleware keeps a route's own policy now, which is what an earlier commit in this series had to fix to make this true. A failed download lands on the message page and says why, instead of a bare JSON envelope in a browser tab - the person clicked a link, and an error object tells them nothing. A message past retention, a missing attachment index and a permission failure each render a page, and the first two fetch nothing. As on the other dashboard pages a permission problem answers 400, not 403: it is the user's AWS configuration and not their session. The id of another project's message and one that never existed give the same answer on every route, so a stranger cannot probe for real ids. Older mail is reached by cursor and not by offset, for the reason /v1/inbound is. The raw_available rule moves onto the model so the API and the dashboard cannot disagree about whether to offer a link. --- apps/api/src/seskit_api/main.py | 2 + apps/api/src/seskit_api/routes/inbox.py | 250 +++++++++ apps/api/src/seskit_api/schemas/inbound.py | 7 +- apps/api/src/seskit_api/templates/base.html | 1 + .../seskit_api/templates/components/ui.html | 20 + .../src/seskit_api/templates/pages/inbox.html | 28 + .../templates/pages/inbox_detail.html | 190 +++++++ .../templates/partials/inbox_table.html | 82 +++ .../src/seskit_core/models/inbound_email.py | 12 +- .../core/src/seskit_core/services/__init__.py | 4 + .../core/src/seskit_core/services/inbound.py | 51 +- tests/test_inbox.py | 490 ++++++++++++++++++ 12 files changed, 1129 insertions(+), 8 deletions(-) create mode 100644 apps/api/src/seskit_api/routes/inbox.py create mode 100644 apps/api/src/seskit_api/templates/pages/inbox.html create mode 100644 apps/api/src/seskit_api/templates/pages/inbox_detail.html create mode 100644 apps/api/src/seskit_api/templates/partials/inbox_table.html create mode 100644 tests/test_inbox.py diff --git a/apps/api/src/seskit_api/main.py b/apps/api/src/seskit_api/main.py index 488ffc9..c881154 100644 --- a/apps/api/src/seskit_api/main.py +++ b/apps/api/src/seskit_api/main.py @@ -34,6 +34,7 @@ domains, emails, health, + inbox, suppressions, unsubscribe, webhooks, @@ -258,6 +259,7 @@ async def _unhandled(request: Request, exc: Exception) -> JSONResponse | Respons app.include_router(aws.router) app.include_router(domains.router) app.include_router(emails.router) + app.include_router(inbox.router) app.include_router(webhooks.router) app.include_router(suppressions.router) # Public and unauthenticated, unlike everything above it. diff --git a/apps/api/src/seskit_api/routes/inbox.py b/apps/api/src/seskit_api/routes/inbox.py new file mode 100644 index 0000000..9ddc75a --- /dev/null +++ b/apps/api/src/seskit_api/routes/inbox.py @@ -0,0 +1,250 @@ +"""The Inbox: mail the project received, in the dashboard. + +Session-authenticated and out of the OpenAPI schema, like the rest of the +dashboard. The read side only - a message is recorded by the worker as it arrives, +and what can be done here is look at it, and fetch an attachment or the original. + +**Everything on these pages was written by somebody else.** Jinja escapes it, and +there is deliberately no ``|safe`` anywhere near a value from a message. The HTML +body is shown as source here; rendering it is a separate, sandboxed document so +that the page a signed-in person is looking at never contains a stranger's markup. + +Ownership is part of every query. An id from another project resolves to the same +answer as one that does not exist, so a stranger cannot probe for real ids. +""" + +from __future__ import annotations + +from typing import Annotated + +from fastapi import APIRouter, Depends, Request, Response +from fastapi.responses import HTMLResponse +from seskit_core.config import Settings +from seskit_core.db import get_session +from seskit_core.errors import APIError, ErrorType +from seskit_core.models import InboundEmail, Project +from seskit_core.services import ( + InboundStoreFactory, + get_received, + list_identities, + list_projects, + list_received, +) +from sqlalchemy.ext.asyncio import AsyncSession + +from seskit_api.dependencies import ( + AuthenticationRequired, + CurrentUser, + get_app_settings, + get_inbound_store_factory, + require_project, + require_user, +) +from seskit_api.downloads import ( + attachment_response, + fetch_original, + raw_response, + require_attachment, +) +from seskit_api.templating import render + +router = APIRouter(tags=["inbox"], include_in_schema=False) + +#: Rows on a page. A cap and not a suggestion, for the reason the Emails page has +#: one: without it a project with a year of mail renders all of it into a template. +PAGE_SIZE = 50 + + +def _status_for(error: APIError) -> int: + """HTTP status for a page that could not complete a download. + + As on the Domains and AWS pages: a credential or permission problem is the + user's AWS configuration, not their SESKit session, and answering 401 or 403 + would read as "you are not signed in". + """ + if error.error_type in {ErrorType.AUTHORIZATION_FAILED, ErrorType.AUTHENTICATION_FAILED}: + return 400 + return error.status_code + + +async def _receiving_domains(db: AsyncSession, project_id: str) -> list[str]: + """The domains this project receives for, for the filter and the empty state.""" + return [i.value for i in await list_identities(db, project_id) if i.receives_mail] + + +async def _list_page( + request: Request, + db: AsyncSession, + current: CurrentUser, + project: Project, + *, + domain: str | None, + before: str | None, + template: str, +) -> HTMLResponse: + domains = await _receiving_domains(db, project.id) + # An unrecognised domain narrows to nothing rather than raising: a stale or + # hand-edited URL should render the page it was clearly asking for. + selected = domain if domain in domains else None + rows, has_more = await list_received( + db, project.id, domain=selected, before=before, limit=PAGE_SIZE + ) + return render( + request, + template, + current=current, + nav_active="inbox", + project=project, + projects=await list_projects(db, current.user.id), + messages=rows, + has_more=has_more, + older=rows[-1].id if has_more and rows else None, + domains=domains, + selected_domain=selected, + ) + + +@router.get("/inbox", response_class=HTMLResponse, summary="Received email") +async def inbox_page( + request: Request, + db: Annotated[AsyncSession, Depends(get_session)], + current: Annotated[CurrentUser, Depends(require_user)], + project: Annotated[Project, Depends(require_project)], + domain: str | None = None, + before: str | None = None, +) -> HTMLResponse: + """The project's received mail, newest first.""" + return await _list_page( + request, db, current, project, domain=domain, before=before, template="pages/inbox.html" + ) + + +@router.get("/partials/inbox", response_class=HTMLResponse, summary="Inbox table fragment") +async def inbox_partial( + request: Request, + db: Annotated[AsyncSession, Depends(get_session)], + current: Annotated[CurrentUser, Depends(require_user)], + project: Annotated[Project, Depends(require_project)], + domain: str | None = None, + before: str | None = None, +) -> HTMLResponse: + """The table on its own, for the filter's HTMX swap. Authenticated like the + page it belongs to - this is a project's mail. + """ + return await _list_page( + request, + db, + current, + project, + domain=domain, + before=before, + template="partials/inbox_table.html", + ) + + +async def _detail_page( + request: Request, + db: AsyncSession, + current: CurrentUser, + project: Project, + message: InboundEmail, + *, + error: str | None = None, + status_code: int = 200, +) -> HTMLResponse: + """One renderer for the detail page, so a failed download lands on the page + the person was looking at and says why, instead of a bare JSON error in a tab. + """ + return render( + request, + "pages/inbox_detail.html", + status_code=status_code, + current=current, + nav_active="inbox", + project=project, + projects=await list_projects(db, current.user.id), + message=message, + error=error, + ) + + +async def _owned(db: AsyncSession, project: Project, inbound_id: str) -> InboundEmail: + message = await get_received(db, project.id, inbound_id) + if message is None: + # The dashboard's own convention for "not yours or not there" - the same + # answer either way, so a stranger cannot probe for real ids. + raise AuthenticationRequired("/inbox") + return message + + +@router.get("/inbox/{inbound_id}", response_class=HTMLResponse, summary="One received email") +async def inbox_detail( + request: Request, + inbound_id: str, + db: Annotated[AsyncSession, Depends(get_session)], + current: Annotated[CurrentUser, Depends(require_user)], + project: Annotated[Project, Depends(require_project)], +) -> HTMLResponse: + """One message in full.""" + message = await _owned(db, project, inbound_id) + return await _detail_page(request, db, current, project, message) + + +@router.get("/inbox/{inbound_id}/attachments/{index}", summary="Download an attachment") +async def inbox_attachment( + request: Request, + inbound_id: str, + index: int, + db: Annotated[AsyncSession, Depends(get_session)], + current: Annotated[CurrentUser, Depends(require_user)], + project: Annotated[Project, Depends(require_project)], + settings: Annotated[Settings, Depends(get_app_settings)], + factory: Annotated[InboundStoreFactory, Depends(get_inbound_store_factory)], +) -> Response: + """One attachment, as a download. + + A GET, because it changes nothing. A failure re-renders the message with the + reason rather than answering with a JSON envelope the browser would show raw. + """ + message = await _owned(db, project, inbound_id) + try: + require_attachment(message, index) + raw = await fetch_original(db, message, factory, settings) + return attachment_response(raw, index) + except APIError as error: + return await _detail_page( + request, + db, + current, + project, + message, + error=error.message, + status_code=_status_for(error), + ) + + +@router.get("/inbox/{inbound_id}/raw", summary="Download the original message") +async def inbox_raw( + request: Request, + inbound_id: str, + db: Annotated[AsyncSession, Depends(get_session)], + current: Annotated[CurrentUser, Depends(require_user)], + project: Annotated[Project, Depends(require_project)], + settings: Annotated[Settings, Depends(get_app_settings)], + factory: Annotated[InboundStoreFactory, Depends(get_inbound_store_factory)], +) -> Response: + """The original message as an ``.eml`` download.""" + message = await _owned(db, project, inbound_id) + try: + raw = await fetch_original(db, message, factory, settings) + except APIError as error: + return await _detail_page( + request, + db, + current, + project, + message, + error=error.message, + status_code=_status_for(error), + ) + return raw_response(raw, message.id) diff --git a/apps/api/src/seskit_api/schemas/inbound.py b/apps/api/src/seskit_api/schemas/inbound.py index 242bcf8..4833eef 100644 --- a/apps/api/src/seskit_api/schemas/inbound.py +++ b/apps/api/src/seskit_api/schemas/inbound.py @@ -22,7 +22,6 @@ from pydantic import BaseModel, Field from seskit_core.models import InboundEmail -from seskit_core.models.base import utcnow class InboundVerdicts(BaseModel): @@ -213,10 +212,6 @@ class InboundList(BaseModel): ) -def _raw_available(row: InboundEmail) -> bool: - return row.raw_expires_at is None or row.raw_expires_at > utcnow() - - def summary_of(row: InboundEmail) -> InboundSummary: return InboundSummary( id=row.id, @@ -239,7 +234,7 @@ def summary_of(row: InboundEmail) -> InboundSummary: ), truncated=row.truncated, parse_failed=row.parse_failed, - raw_available=_raw_available(row), + raw_available=row.raw_available, raw_expires_at=row.raw_expires_at, ) diff --git a/apps/api/src/seskit_api/templates/base.html b/apps/api/src/seskit_api/templates/base.html index 6ff7876..da9bc4c 100644 --- a/apps/api/src/seskit_api/templates/base.html +++ b/apps/api/src/seskit_api/templates/base.html @@ -70,6 +70,7 @@
{%- endmacro %} +{# What Amazon SES concluded about a received message. + + Verbatim states, never a judgement: SESKit shows these and acts on none. A verdict + that was not given is "Not checked" and is deliberately not styled like a pass - + reading an unchecked message as a clean one is the one mistake this exists to + prevent. #} +{% macro verdict(status) -%} + {%- if status == "PASS" -%} + {{ badge("Pass", tone="success") }} + {%- elif status == "FAIL" -%} + {{ badge("Fail", tone="danger") }} + {%- elif status == "GRAY" -%} + {{ badge("Inconclusive", tone="warning") }} + {%- elif status == "PROCESSING_FAILED" -%} + {{ badge("Could not check", tone="warning") }} + {%- else -%} + Not checked + {%- endif -%} +{%- endmacro %} + {# A single value with its own Copy. For a table of things that each go into a different field somewhere else - DNS records, mostly - where one Copy for the whole block still leaves the user picking the lines apart. #} diff --git a/apps/api/src/seskit_api/templates/pages/inbox.html b/apps/api/src/seskit_api/templates/pages/inbox.html new file mode 100644 index 0000000..7f07791 --- /dev/null +++ b/apps/api/src/seskit_api/templates/pages/inbox.html @@ -0,0 +1,28 @@ +{% extends "base.html" %} +{% from "components/ui.html" import empty %} + +{% block title %}Inbox · SESKit{% endblock %} + +{% block content %} +
+

Inbox

+

+ Mail sent to {{ project.name }}'s domains, newest first. +

+
+ + {% if domains or messages %} + {% include "partials/inbox_table.html" %} + {% else %} + {# Its own state. A project that has never turned receiving on has nothing to + wait for, and saying "no mail yet" would hide the one thing it has to do. #} + {{ empty( + "Not receiving mail yet", + "Turn receiving on for a verified domain and mail sent to it appears here, and + your application is told as it arrives.", + icon_name="inbox", + action_label="Go to Domains", + action_href="/domains", + ) }} + {% endif %} +{% endblock %} diff --git a/apps/api/src/seskit_api/templates/pages/inbox_detail.html b/apps/api/src/seskit_api/templates/pages/inbox_detail.html new file mode 100644 index 0000000..bc40a11 --- /dev/null +++ b/apps/api/src/seskit_api/templates/pages/inbox_detail.html @@ -0,0 +1,190 @@ +{% extends "base.html" %} +{% from "components/ui.html" import card_open, card_close, badge, button, code_block, verdict %} + +{% block title %}{{ message.subject or "(no subject)" }} · SESKit{% endblock %} + +{# Everything about this message was written by somebody else. It is escaped by + Jinja and there is no |safe anywhere in this file. The HTML body is shown as + source and never rendered into this page: a signed-in person's session is the + most valuable thing here, and a stranger's markup does not belong next to it. #} + +{% block content %} +
+

{{ message.subject or "(no subject)" }}

+

+ Back to inbox +

+
+ + {% if error %} + + {% endif %} + + {% if message.parse_failed %} +
+

SESKit could not read this message

+

+ Everything below is empty. The original is still stored + {% if message.raw_available %}and can be downloaded{% else %}but has passed its retention period{% endif %}. +

+
+ {% elif message.truncated %} +
+

Part of this message was left out

+

+ It hit a limit while being read - a very long body, or a very large number of + parts or headers. The original has all of it. +

+
+ {% endif %} + + {{ card_open("Message") }} +
+
+
From
+
+ {% if message.from_name %}{{ message.from_name }} {% endif %}{{ message.from_address or "unknown sender" }} +
+
+
+
To
+
{{ message.to_addresses | join(", ") or "—" }}
+
+ {% if message.cc_addresses %} +
+
Cc
+
{{ message.cc_addresses | join(", ") }}
+
+ {% endif %} + {% if message.reply_to %} +
+
Reply-To
+
{{ message.reply_to | join(", ") }}
+
+ {% endif %} +
+ {# Who it was addressed to on the wire - the address that received it. It + can be one the headers never mention, such as a blind copy. #} +
Delivered to
+
{{ message.envelope_to | join(", ") or "—" }}
+
+
+
Domain
+
{{ message.domain }}
+
+
+
Received
+
{{ message.received_at.strftime("%Y-%m-%d %H:%M UTC") }}
+
+
+
Size
+
{{ (message.size_bytes / 1024) | round(1) }} KB
+
+ {% if message.message_id_header %} +
+
Message-ID
+
{{ message.message_id_header }}
+
+ {% endif %} + {% if message.in_reply_to %} +
+
In reply to
+
{{ message.in_reply_to }}
+
+ {% endif %} +
+ {{ card_close() }} + + {{ card_open("Authentication") }} +
+
SPF
{{ verdict(message.spf_verdict) }}
+
DKIM
{{ verdict(message.dkim_verdict) }}
+
+
DMARC
+
+ {{ verdict(message.dmarc_verdict) }} + {% if message.dmarc_policy %}sender asks receivers to {{ message.dmarc_policy }}{% endif %} +
+
+
Spam
{{ verdict(message.spam_verdict) }}
+
Virus
{{ verdict(message.virus_verdict) }}
+
+

+ Amazon SES checked these. SESKit shows them and acts on none: whether to trust a + message is your decision. Not checked means SES did not say, which is + not the same as a pass. +

+ {{ card_close() }} + + {% if message.attachments %} + {{ card_open("Attachments") }} + + {% if not message.raw_available %} +

+ The original has passed its retention period, so attachments can no longer be + downloaded. +

+ {% endif %} + {{ card_close() }} + {% endif %} + + {% if message.text_body %} + {{ card_open("Text") }} + {{ code_block(message.text_body, language="text") }} + {{ card_close() }} + {% endif %} + + {% if message.html_body %} + {{ card_open("HTML source") }} + {{ code_block(message.html_body, language="html") }} + {{ card_close() }} + {% endif %} + + {{ card_open("Original") }} + {% if message.raw_available %} +

+ The message exactly as Amazon SES received it, every header and every part. + {% if message.raw_expires_at %} + It is removed after {{ message.raw_expires_at.strftime("%Y-%m-%d") }}; the parsed + message above stays. + {% endif %} +

+
+ {{ button("Download .eml", href="/inbox/" ~ message.id ~ "/raw", icon_name="arrow-right") }} +
+ {% else %} +

+ The original was removed when its retention period passed. The parsed message + above stays readable. +

+ {% endif %} + {{ card_close() }} + + {% if message.headers %} + {{ card_open("Headers", collapsible=True, open=False, meta=message.headers | length ~ " headers") }} + {%- set lines = [] %} + {%- for header in message.headers %} + {%- set _ = lines.append(header[0] ~ ": " ~ header[1]) %} + {%- endfor %} + {{ code_block(lines | join(" +"), language="headers") }} + {{ card_close(collapsible=True) }} + {% endif %} +{% endblock %} diff --git a/apps/api/src/seskit_api/templates/partials/inbox_table.html b/apps/api/src/seskit_api/templates/partials/inbox_table.html new file mode 100644 index 0000000..8a70d6f --- /dev/null +++ b/apps/api/src/seskit_api/templates/partials/inbox_table.html @@ -0,0 +1,82 @@ +{# The Inbox table and its domain filter. + + A partial so the filter can swap just this fragment over HTMX rather than + reloading, the pattern the Emails page uses. The filter is links, not buttons: + a filtered list is a view, and a view should be linkable and survive a refresh. + Without JavaScript they are ordinary navigation that still works. + + Every value from a message below is somebody else's. It is escaped, and there + is no |safe anywhere in this file. #} + +
+ {% if domains | length > 1 %} + + {% endif %} + + {% if messages %} +
+ + + + + + + + + + + {% for message in messages %} + + + {# The subject is the link, so the row does not carry two links to the + same page. A message with none still needs something to click. #} + + + + + {% endfor %} + +
FromSubjectReceivedFiles
+ {% if message.from_name %}{{ message.from_name }}{% endif %} + {{ message.from_address or "unknown sender" }} + + {{ message.subject or "(no subject)" }} + {{ message.received_at.strftime("%Y-%m-%d %H:%M UTC") }}{{ message.attachments | length or "—" }}
+
+ + {% if older %} +

+ Older messages +

+ {% endif %} + {% elif selected_domain %} +

No mail for {{ selected_domain }}. Show all.

+ {% else %} + {# Receiving is on and nothing has come in. The likeliest reason is the one + thing SESKit cannot check, so it says what it can: where mail would go. #} +
+

Waiting for the first message

+

+ Mail sent to {{ domains | join(", ") }} appears here once the MX record is in + your DNS. The Domains page shows the record to add. +

+
+ {% endif %} +
diff --git a/packages/core/src/seskit_core/models/inbound_email.py b/packages/core/src/seskit_core/models/inbound_email.py index 13fce1b..0b09505 100644 --- a/packages/core/src/seskit_core/models/inbound_email.py +++ b/packages/core/src/seskit_core/models/inbound_email.py @@ -46,7 +46,7 @@ from seskit_core.db import Base from seskit_core.ids import IDPrefix, generate_id -from seskit_core.models.base import TimestampMixin +from seskit_core.models.base import TimestampMixin, utcnow if TYPE_CHECKING: from seskit_core.models.project import Project @@ -184,6 +184,16 @@ class InboundEmail(Base, TimestampMixin): project: Mapped[Project] = relationship(back_populates="inbound_emails") + @property + def raw_available(self) -> bool: + """Whether the original is expected to still be in storage. + + True when the expiry is unknown, because unknown is not gone: the download + settles it. The one definition, so the API and the dashboard cannot + disagree about whether a link should be offered. + """ + return self.raw_expires_at is None or self.raw_expires_at > utcnow() + @property def has_attachments(self) -> bool: return bool(self.attachments) diff --git a/packages/core/src/seskit_core/services/__init__.py b/packages/core/src/seskit_core/services/__init__.py index f3557ef..49e4e90 100644 --- a/packages/core/src/seskit_core/services/__init__.py +++ b/packages/core/src/seskit_core/services/__init__.py @@ -63,6 +63,8 @@ bucket_name_for, count_other_receivers, distinct_inbound_queues, + get_received, + list_received, rule_name_for, setup_receiving, teardown_all_receiving, @@ -186,6 +188,7 @@ "get_owned_endpoint", "get_owned_identity", "get_owned_project", + "get_received", "get_user_by_email", "get_user_by_id", "identities_due", @@ -198,6 +201,7 @@ "list_events", "list_identities", "list_projects", + "list_received", "list_suppressions", "payload_bytes", "pending_delivery_ids", diff --git a/packages/core/src/seskit_core/services/inbound.py b/packages/core/src/seskit_core/services/inbound.py index af82ad9..19b0e87 100644 --- a/packages/core/src/seskit_core/services/inbound.py +++ b/packages/core/src/seskit_core/services/inbound.py @@ -41,7 +41,7 @@ from seskit_core.errors import APIError, ErrorType from seskit_core.logging import get_logger -from seskit_core.models import AWSConnection, Identity +from seskit_core.models import AWSConnection, Identity, InboundEmail from seskit_core.providers import ( AWSCredentials, InboundInfrastructure, @@ -495,3 +495,52 @@ async def distinct_inbound_queues(session: AsyncSession, *, secret_key: str) -> ) for (region, url), creds in credentials.items() ] + + +# --------------------------------------------------------------------- reading --- + + +async def get_received( + session: AsyncSession, project_id: str, inbound_id: str +) -> InboundEmail | None: + """One received message, if it belongs to this project. + + Ownership is part of the query, so an id from another project is + indistinguishable from one that never existed. + """ + found: InboundEmail | None = await session.scalar( + select(InboundEmail).where( + InboundEmail.id == inbound_id, InboundEmail.project_id == project_id + ) + ) + return found + + +async def list_received( + session: AsyncSession, + project_id: str, + *, + domain: str | None = None, + before: str | None = None, + limit: int = 50, +) -> tuple[list[InboundEmail], bool]: + """A page of received mail, newest first, and whether there is another page. + + By cursor and not by offset, for the reason ``/v1/inbound`` is: a message + arriving between two pages shifts every row of an offset down one and the + reader skips the one that moved across the boundary. Ids sort in the order they + were created, so ``before`` names a fixed point. + + One more than asked for is read, so "is there another page?" is answered by the + query that fetched this one. A ``before`` that is not an id of this project + simply matches by comparison and returns what is older than it - the comparison + is lexical, but everything returned is still scoped to the project. + """ + query = select(InboundEmail).where(InboundEmail.project_id == project_id) + if domain is not None: + query = query.where(InboundEmail.domain == domain) + if before is not None: + query = query.where(InboundEmail.id < before) + + rows = list(await session.scalars(query.order_by(InboundEmail.id.desc()).limit(limit + 1))) + return rows[:limit], len(rows) > limit diff --git a/tests/test_inbox.py b/tests/test_inbox.py new file mode 100644 index 0000000..168a9bf --- /dev/null +++ b/tests/test_inbox.py @@ -0,0 +1,490 @@ +"""The Inbox, over HTTP (inbound email, Phase D). + +What a signed-in person sees of mail somebody sent to them. The thing worth +asserting over and over is the same: **everything on these pages was written by a +stranger.** Every sender-controlled field is fed a script tag and the assertion is +that it is never in the response as markup - the page a signed-in person looks at +holds their session, and a stranger's HTML does not belong next to it. + +The second thing is that a failed download lands on the page the person was looking +at and says why, instead of a bare JSON envelope in a browser tab. +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from typing import Any + +import pytest +from fakes.inbound import FakeInboundStore +from httpx import AsyncClient +from seskit_core.models import InboundEmail, Project +from seskit_core.services import create_project, register_user +from sqlalchemy.ext.asyncio import AsyncSession +from test_domains_page import _connect, _sign_in +from test_domains_receiving import _domain, _project +from test_v1_inbound import _message +from test_v1_inbound_downloads import ATTACHMENTS, HTML_ATTACHMENT, PDF, _raw + +HOSTILE = "" + + +async def _seed(session: AsyncSession, project: Project, **overrides: Any) -> InboundEmail: + return await _message(session, project_id=project.id, **overrides) + + +async def _signed_in_with_project( + client: AsyncClient, session: AsyncSession +) -> tuple[str, Project]: + token = await _connect(client) + return token, await _project(session) + + +def _never_as_markup(text: str) -> None: + assert "' + message = await _seed(db_session, project, html_body=hostile) + + response = await app_client.get(f"/inbox/{message.id}/html") + + assert response.status_code == 200 + assert response.text == hostile + assert response.headers["content-type"] == "text/html; charset=utf-8" + + +async def test_the_policy_on_the_html_blocks_script_network_forms_and_navigation( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + _, project = await _signed_in_with_project(app_client, db_session) + message = await _seed(db_session, project) + + response = await app_client.get(f"/inbox/{message.id}/html") + policy = response.headers["content-security-policy"] + directives = {part.strip().split(" ", 1)[0]: part.strip() for part in policy.split(";")} + + # `sandbox` with no tokens: opaque origin, no script, no forms, no navigation. + assert directives["sandbox"] == "sandbox" + # Nothing loads from the network, which blocks tracking pixels and remote images. + assert directives["default-src"] == "default-src 'none'" + assert directives["img-src"] == "img-src data:" + # Inline styles are the one thing allowed; without them most mail is unreadable. + assert directives["style-src"] == "style-src 'unsafe-inline'" + assert "script-src" not in directives + assert "unsafe-eval" not in policy + assert "'self'" not in policy.replace("frame-ancestors 'self'", "") + + +async def test_only_the_message_page_may_frame_the_html( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """The middleware used to force every response to DENY. This is the one that has + to be framed, by the same origin and nobody else. + """ + _, project = await _signed_in_with_project(app_client, db_session) + message = await _seed(db_session, project) + + response = await app_client.get(f"/inbox/{message.id}/html") + + assert response.headers["x-frame-options"] == "SAMEORIGIN" + assert "frame-ancestors 'self'" in response.headers["content-security-policy"] + assert response.headers["x-content-type-options"] == "nosniff" + assert response.headers["referrer-policy"] == "no-referrer" + assert response.headers["cache-control"] == "private, no-store" + + +async def test_a_message_with_no_html_says_so_on_its_own_page( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + _, project = await _signed_in_with_project(app_client, db_session) + message = await _seed(db_session, project, html_body="") + + response = await app_client.get(f"/inbox/{message.id}/html") + + assert response.status_code == 404 + assert response.headers["content-type"].startswith("text/html") + assert "no HTML body" in response.text + + +async def test_the_html_route_gives_a_stranger_nothing( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """Another project's id and one that never existed answer the same, and without + a session there is nothing at all. + """ + await _signed_in_with_project(app_client, db_session) + other_user = await register_user( + db_session, email="other@example.com", password="correct-horse-battery", allow_signup=True + ) + theirs = await create_project(db_session, user_id=other_user.id, name="Other") + foreign = await _seed(db_session, theirs, html_body="

secret

") + + a = await app_client.get(f"/inbox/{foreign.id}/html", follow_redirects=False) + b = await app_client.get("/inbox/inbound_01NEVEREXISTED/html", follow_redirects=False) + + assert (a.status_code, a.headers.get("location")) == (b.status_code, b.headers.get("location")) + assert a.status_code == 303 + assert "secret" not in a.text + + async def test_another_projects_message_and_an_unknown_one_look_identical( app_client: AsyncClient, db_session: AsyncSession ) -> None: From b9b809208b97fbad8f60abef872932370ea5b7f1 Mon Sep 17 00:00:00 2001 From: Otitodev <198799600+Otitodev@users.noreply.github.com> Date: Mon, 5 Oct 2026 09:09:57 +0100 Subject: [PATCH 08/10] fix(ui): give the inbox filter a loading state CI's polish test failed on the inbox filter: every HTMX swap has to say it is working, and mine did not. That test reads the templates and not a rendered page, so that the third swap added later by someone who has not read it is the one that fails - and this was that swap, added by me. The two filter links now carry hx-indicator and the table gets the same hidden spinner the Emails page's filter has, so a click shows something while the fragment loads. Also fixes a verdict test that counted the phrase "Not checked" and found six, because the page's own explanation uses it once. It counts the verdict rows, which is what it meant to. --- .../api/src/seskit_api/templates/partials/inbox_table.html | 7 +++++++ tests/test_inbox.py | 4 +++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/api/src/seskit_api/templates/partials/inbox_table.html b/apps/api/src/seskit_api/templates/partials/inbox_table.html index 8a70d6f..1da7d73 100644 --- a/apps/api/src/seskit_api/templates/partials/inbox_table.html +++ b/apps/api/src/seskit_api/templates/partials/inbox_table.html @@ -15,6 +15,7 @@ href="/inbox" hx-get="/partials/inbox" hx-target="#inbox-table" + hx-indicator="#inbox-filter-busy" hx-swap="outerHTML" hx-push-url="/inbox" {% if not selected_domain %}aria-current="page"{% endif %}>All @@ -23,11 +24,17 @@ href="/inbox?domain={{ domain | urlencode }}" hx-get="/partials/inbox?domain={{ domain | urlencode }}" hx-target="#inbox-table" + hx-indicator="#inbox-filter-busy" hx-swap="outerHTML" hx-push-url="/inbox?domain={{ domain | urlencode }}" {% if domain == selected_domain %}aria-current="page"{% endif %}>{{ domain }} {% endfor %} + {# aria-hidden: the swapped table is the announcement, and a live region for the + wait as well would say two things about one click. #} + {% endif %} {% if messages %} diff --git a/tests/test_inbox.py b/tests/test_inbox.py index 91bbfb9..180d2f8 100644 --- a/tests/test_inbox.py +++ b/tests/test_inbox.py @@ -254,7 +254,9 @@ async def test_a_verdict_ses_did_not_give_reads_not_checked_never_pass( page = await app_client.get(f"/inbox/{message.id}") - assert page.text.count("Not checked") == 5 + # Five verdict rows. Counted as rows and not as the phrase, which the page's own + # explanation also uses once. + assert page.text.count('Not checked') == 5 assert "Pass" not in page.text From 5f1beb836b248b044be392d7cbad4c4ae89df229 Mon Sep 17 00:00:00 2001 From: Otitodev <198799600+Otitodev@users.noreply.github.com> Date: Mon, 5 Oct 2026 09:11:40 +0100 Subject: [PATCH 09/10] feat(api): say whether a domain receives mail on /v1/domains Each domain gains a read-only `receiving` object: whether a receipt rule exists for it, and the MX record to publish. An application managing its own sending infrastructure can then see which of its domains receive mail and what DNS they need, without scraping the dashboard. Read-only because domains are managed in the dashboard - they cannot be created through the API either - so there is nothing here to switch; it reports what was set up. The MX record is null until receiving is on. Published earlier, it would send senders' mail to Amazon SES with no rule to take it, and the mail would be refused. The description says so, and that publishing it replaces wherever the domain receives mail now, since that is the mistake most worth stopping. It names the SES inbound endpoint for the domain's own region, and a test uses one outside us-east-1: mail is received where the domain is verified, and a record naming any other would deliver nowhere without an error. The rule name, the rule set and the bucket are not in the response - a test greps for them. A customer cannot act on them and putting them in a response would make them a contract. The attachment is done in the route and not read off the row: the MX host is Amazon SES's and the row is core's, which knows nothing about a provider. Additive, so a client written before the field existed still reads the rest, and a test asserts the existing fields are unchanged. Another project's receiving domain is never listed. The OpenAPI document is regenerated, and the docs tests that require every field to be described pass. --- apps/api/src/seskit_api/routes/v1/domains.py | 33 +++- apps/api/src/seskit_api/schemas/domains.py | 49 ++++++ docs/reference/openapi.json | 82 ++++++++++ tests/test_v1_domains_receiving.py | 152 +++++++++++++++++++ 4 files changed, 314 insertions(+), 2 deletions(-) create mode 100644 tests/test_v1_domains_receiving.py diff --git a/apps/api/src/seskit_api/routes/v1/domains.py b/apps/api/src/seskit_api/routes/v1/domains.py index 533444e..ec8a5a0 100644 --- a/apps/api/src/seskit_api/routes/v1/domains.py +++ b/apps/api/src/seskit_api/routes/v1/domains.py @@ -14,13 +14,20 @@ from fastapi import APIRouter, Depends, Response from seskit_core.db import get_session +from seskit_core.models import Identity from seskit_core.providers import IdentityType from seskit_core.services import list_identities from sqlalchemy.ext.asyncio import AsyncSession from seskit_api.dependencies import APIContext, require_api_key +from seskit_api.receiving import mx_record from seskit_api.routes.v1.api_keys import API_RESPONSES, apply_rate_limit_headers -from seskit_api.schemas.domains import DomainList, DomainResponse +from seskit_api.schemas.domains import ( + DomainList, + DomainResponse, + MxRecordResponse, + ReceivingResponse, +) router = APIRouter(tags=["domains"]) @@ -42,4 +49,26 @@ async def list_domains( identities = await list_identities(db, context.project.id) domains = [identity for identity in identities if identity.type is IdentityType.DOMAIN] - return DomainList(data=[DomainResponse.model_validate(domain) for domain in domains]) + return DomainList(data=[_response(domain) for domain in domains]) + + +def _response(identity: Identity) -> DomainResponse: + """A domain as a customer sees it, with whether it receives mail. + + ``receiving`` is attached here and not read off the row, because the MX host is + Amazon SES's and the row is core's, which knows nothing about a provider. The + record is withheld until receiving is on, so nobody publishes it early. + """ + response = DomainResponse.model_validate(identity) + record = mx_record(identity) + response.receiving = ReceivingResponse( + enabled=identity.receives_mail, + mx=( + MxRecordResponse( + record_type="MX", name=record.name, priority=record.priority, value=record.value + ) + if identity.receives_mail + else None + ), + ) + return response diff --git a/apps/api/src/seskit_api/schemas/domains.py b/apps/api/src/seskit_api/schemas/domains.py index 6ef2e2c..a78199f 100644 --- a/apps/api/src/seskit_api/schemas/domains.py +++ b/apps/api/src/seskit_api/schemas/domains.py @@ -39,6 +39,48 @@ class DnsRecordResponse(BaseModel): ) +class MxRecordResponse(BaseModel): + """The MX record that sends a domain's mail to Amazon SES.""" + + record_type: str = Field(description="`MX`.", examples=["MX"]) + name: str = Field( + description=( + "The record name: the domain itself. Some DNS providers want only the part " + "before your domain, so check whether yours appends it for you." + ), + examples=["example.com"], + ) + priority: int = Field( + description="The preference value. SES takes mail at one host, so this only matters " + "if the domain lists others.", + examples=[10], + ) + value: str = Field( + description="The Amazon SES inbound endpoint for the domain's region.", + examples=["inbound-smtp.us-east-1.amazonaws.com"], + ) + + +class ReceivingResponse(BaseModel): + """Whether SESKit receives mail for a domain, and what to publish for it.""" + + enabled: bool = Field( + description=( + "Whether a receipt rule exists for this domain. Turned on from the Domains page " + "in the dashboard; there is no endpoint for it yet." + ), + ) + mx: MxRecordResponse | None = Field( + default=None, + description=( + "The record to publish at your DNS provider. **Null until receiving is on**: " + "publishing it earlier would send senders' mail to Amazon SES with no rule to " + "take it, and it would be refused. Publishing it replaces wherever the domain " + "receives mail now - consider a subdomain." + ), + ) + + class DomainResponse(BaseModel): """A sending domain and where its verification has got to.""" @@ -89,6 +131,13 @@ class DomainResponse(BaseModel): default=None, description="When SESKit last asked SES about this domain. Null until first checked. UTC.", ) + receiving: ReceivingResponse | None = Field( + default=None, + description=( + "Whether the project receives mail for this domain. Read-only: it reports what " + "the dashboard set up. Mail that arrives is read with `GET /v1/inbound`." + ), + ) created_at: datetime = Field(description="UTC.") diff --git a/docs/reference/openapi.json b/docs/reference/openapi.json index 52d912b..aadf0c4 100644 --- a/docs/reference/openapi.json +++ b/docs/reference/openapi.json @@ -262,6 +262,17 @@ ], "title": "Mail From Status" }, + "receiving": { + "anyOf": [ + { + "$ref": "#/components/schemas/ReceivingResponse" + }, + { + "type": "null" + } + ], + "description": "Whether the project receives mail for this domain. Read-only: it reports what the dashboard set up. Mail that arrives is read with `GET /v1/inbound`." + }, "region": { "description": "The AWS region the identity lives in. SES identities are per-region, so a domain verified in one region is not verified in another.", "examples": [ @@ -1102,6 +1113,51 @@ "title": "InboundVerdicts", "type": "object" }, + "MxRecordResponse": { + "description": "The MX record that sends a domain's mail to Amazon SES.", + "properties": { + "name": { + "description": "The record name: the domain itself. Some DNS providers want only the part before your domain, so check whether yours appends it for you.", + "examples": [ + "example.com" + ], + "title": "Name", + "type": "string" + }, + "priority": { + "description": "The preference value. SES takes mail at one host, so this only matters if the domain lists others.", + "examples": [ + 10 + ], + "title": "Priority", + "type": "integer" + }, + "record_type": { + "description": "`MX`.", + "examples": [ + "MX" + ], + "title": "Record Type", + "type": "string" + }, + "value": { + "description": "The Amazon SES inbound endpoint for the domain's region.", + "examples": [ + "inbound-smtp.us-east-1.amazonaws.com" + ], + "title": "Value", + "type": "string" + } + }, + "required": [ + "record_type", + "name", + "priority", + "value" + ], + "title": "MxRecordResponse", + "type": "object" + }, "ReadinessResponse": { "properties": { "dependencies": { @@ -1125,6 +1181,32 @@ "title": "ReadinessResponse", "type": "object" }, + "ReceivingResponse": { + "description": "Whether SESKit receives mail for a domain, and what to publish for it.", + "properties": { + "enabled": { + "description": "Whether a receipt rule exists for this domain. Turned on from the Domains page in the dashboard; there is no endpoint for it yet.", + "title": "Enabled", + "type": "boolean" + }, + "mx": { + "anyOf": [ + { + "$ref": "#/components/schemas/MxRecordResponse" + }, + { + "type": "null" + } + ], + "description": "The record to publish at your DNS provider. **Null until receiving is on**: publishing it earlier would send senders' mail to Amazon SES with no rule to take it, and it would be refused. Publishing it replaces wherever the domain receives mail now - consider a subdomain." + } + }, + "required": [ + "enabled" + ], + "title": "ReceivingResponse", + "type": "object" + }, "SendEmailRequest": { "description": "\u00a711's request body.", "properties": { diff --git a/tests/test_v1_domains_receiving.py b/tests/test_v1_domains_receiving.py new file mode 100644 index 0000000..5042ba2 --- /dev/null +++ b/tests/test_v1_domains_receiving.py @@ -0,0 +1,152 @@ +"""``receiving`` on ``GET /v1/domains`` (inbound email, Phase D). + +Read-only: it reports what the dashboard set up, so an application can see which +domains receive mail and what record to publish. There is no endpoint to turn +receiving on - domains themselves are managed in the dashboard - so this only ever +describes. + +The MX record is the one thing in the response somebody acts on outside SESKit, and +it is withheld until receiving is on. Published earlier, it would send senders' mail +to Amazon SES with no rule to take it, and the mail would be refused. +""" + +from __future__ import annotations + +from httpx import AsyncClient +from seskit_core.models import Identity +from sqlalchemy.ext.asyncio import AsyncSession +from test_v1_inbound import _auth, _project_key + +URL = "/v1/domains" + + +async def _identity( + session: AsyncSession, + project_id: str, + value: str = "example.com", + *, + kind: str = "domain", + region: str = "us-east-1", + rule: str | None = None, +) -> Identity: + identity = Identity( + project_id=project_id, + identity_type=kind, + value=value, + region=region, + verification_status="success", + dkim_status="success" if kind == "domain" else None, + dkim_tokens=[], + inbound_rule_name=rule, + inbound_rule_set="seskit-inbound" if rule else None, + inbound_rule_set_created=bool(rule), + ) + session.add(identity) + await session.flush() + return identity + + +async def test_a_domain_that_does_not_receive_says_so_and_withholds_the_record( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + project_id, key = await _project_key(db_session, owner="a@example.com") + await _identity(db_session, project_id) + await db_session.commit() + + (domain,) = (await app_client.get(URL, headers=_auth(key))).json()["data"] + + assert domain["receiving"] == {"enabled": False, "mx": None} + + +async def test_a_domain_that_receives_gives_the_record_to_publish( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + project_id, key = await _project_key(db_session, owner="a@example.com") + await _identity(db_session, project_id, rule="seskit-example-com") + await db_session.commit() + + (domain,) = (await app_client.get(URL, headers=_auth(key))).json()["data"] + + assert domain["receiving"] == { + "enabled": True, + "mx": { + "record_type": "MX", + "name": "example.com", + "priority": 10, + "value": "inbound-smtp.us-east-1.amazonaws.com", + }, + } + + +async def test_the_record_names_the_endpoint_for_the_domains_own_region( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """Mail is received in the region the domain is verified in. A record naming any + other would deliver nowhere, without an error. + """ + project_id, key = await _project_key(db_session, owner="a@example.com") + await _identity(db_session, project_id, "eu.example", region="eu-west-1", rule="seskit-eu") + await db_session.commit() + + (domain,) = (await app_client.get(URL, headers=_auth(key))).json()["data"] + + assert domain["receiving"]["mx"]["value"] == "inbound-smtp.eu-west-1.amazonaws.com" + + +async def test_what_the_response_adds_is_only_what_a_caller_can_act_on( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """The rule name, the rule set and the bucket are internal. A customer cannot use + them and putting them in a response would make them a contract. + """ + project_id, key = await _project_key(db_session, owner="a@example.com") + await _identity(db_session, project_id, rule="seskit-example-com") + await db_session.commit() + + text = (await app_client.get(URL, headers=_auth(key))).text + + assert "seskit-example-com" not in text + assert "seskit-inbound" not in text + assert "inbound_rule" not in text + assert "bucket" not in text + + +async def test_the_existing_fields_are_unchanged( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + """Additive: a client written before this field existed still reads the rest.""" + project_id, key = await _project_key(db_session, owner="a@example.com") + await _identity(db_session, project_id) + await db_session.commit() + + (domain,) = (await app_client.get(URL, headers=_auth(key))).json()["data"] + + for field in ("id", "value", "region", "verification_status", "dkim_status", "dns_records"): + assert field in domain + assert domain["value"] == "example.com" + assert domain["verification_status"] == "success" + + +async def test_another_projects_receiving_domain_is_never_listed( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + mine, key = await _project_key(db_session, owner="a@example.com") + theirs, _ = await _project_key(db_session, owner="b@example.com") + await _identity(db_session, mine, "mine.example") + await _identity(db_session, theirs, "theirs.example", rule="seskit-theirs") + await db_session.commit() + + domains = (await app_client.get(URL, headers=_auth(key))).json()["data"] + + assert [d["value"] for d in domains] == ["mine.example"] + assert "theirs.example" not in (await app_client.get(URL, headers=_auth(key))).text + + +async def test_an_email_address_identity_is_still_not_a_domain( + app_client: AsyncClient, db_session: AsyncSession +) -> None: + project_id, key = await _project_key(db_session, owner="a@example.com") + await _identity(db_session, project_id, "me@example.com", kind="email_address") + await db_session.commit() + + assert (await app_client.get(URL, headers=_auth(key))).json()["data"] == [] From f2a78ee85db12e061048040f10fb03508ffda2c1 Mon Sep 17 00:00:00 2001 From: Otitodev <198799600+Otitodev@users.noreply.github.com> Date: Mon, 5 Oct 2026 09:13:59 +0100 Subject: [PATCH 10/10] docs: a guide to receiving mail, and the IAM policy it needs The guide is the first thing a user reads for a feature that creates storage in their own AWS account and edits how the account receives mail, so it leads with what that means and not with how to switch it on: what is created and where, that SESKit never replaces an existing receipt rule set, and that AWS bills it. It gives no prices. Whether new accounts are billed for classic receipt rules under the newer SES plans was never established, and a figure that turned out wrong would be worse than none; it points at AWS's pricing pages and says SESKit does not estimate. The MX record gets the warning that matters most, at length and in an admonition: it sends all of a domain's mail to SES and replaces wherever it is received now. The guide recommends a subdomain and explains why that works - the rule is per domain, so the subdomain is added as its own - and says plainly that neither SESKit nor the page can check the record is live, with a dig command to do it. A section on treating everything as hostile is addressed to the reader's own code, not only to SESKit's: the API returns HTML exactly as sent, so the guide says not to render it where it could reach their credentials. Retention says what expires and what does not, and turning it off says what is kept - mail already received, and a bucket that still holds mail - and that removing a domain or disconnecting does the same first, so neither can leave a rule behind. The third IAM policy is sixteen actions: seven bucket-level S3 actions scoped to seskit-*, GetObject on its objects, and eight SES receipt-rule actions. It says why s3:ListBucket is worth keeping - without it an ordinary expiry reads as a permission failure, because S3 answers a missing key with access denied - and that the receipt-rule actions are granted on `*` because whether they can be narrowed was never checked, and a policy claiming a scope AWS does not enforce is worse than one that says it has none. The SNS and SQS statements of the second policy already cover the topic and queue, since they share the prefix. The test that counts the IAM guide's policy sections now expects three. Both the guide and the SDK, events and IAM descriptions are added to the agent index, which a test requires for every page in the nav. --- README.md | 1 + docs/guides/iam-policies.md | 75 ++++++++++++++- docs/guides/receive-email.md | 176 +++++++++++++++++++++++++++++++++++ mkdocs.yml | 8 +- tests/test_docs.py | 2 +- 5 files changed, 256 insertions(+), 6 deletions(-) create mode 100644 docs/guides/receive-email.md diff --git a/README.md b/README.md index 215cec1..139add4 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,7 @@ and dashboard. - **Verify senders** — email addresses and domains, with the DNS records to add - **Delivery events** — see whether each message was delivered, bounced, opened or clicked - **Webhooks** — get notified when something happens to a message +- **Receive email** — mail sent to a domain you choose is stored, shown in an Inbox, readable through the API, and announced by webhook - **Suppression list** — bounced and unsubscribed addresses are never sent to again - **Dashboard** — send a test message, watch deliveries, manage everything - **Request SES production access** from the dashboard diff --git a/docs/guides/iam-policies.md b/docs/guides/iam-policies.md index ae2493a..2b91574 100644 --- a/docs/guides/iam-policies.md +++ b/docs/guides/iam-policies.md @@ -1,8 +1,8 @@ # IAM policies -SESKit must never ask for `AdministratorAccess`. There are two policies because +SESKit must never ask for `AdministratorAccess`. There are three policies because they grant genuinely different things, and **you should be able to run SESKit -without the second**. +without the second and the third**. ## Sending — seven actions (required) @@ -114,6 +114,76 @@ them away. SNS does, under a queue policy SESKit sets during setup that admits that one topic and nothing else. Adding it here would grant a permission nothing uses. +## Receiving mail — sixteen more (optional) + +!!! caution "Read this one before granting it" + This creates **S3 buckets** and edits **SES receipt rules** in your account — + the first is storage that holds other people's mail, and the second changes how + your account receives it. It is a third policy and a separate button for the same + reason the second is. You only need it to [receive mail](receive-email.md). + +```json +{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:CreateBucket", + "s3:PutBucketPublicAccessBlock", + "s3:PutEncryptionConfiguration", + "s3:PutLifecycleConfiguration", + "s3:PutBucketPolicy", + "s3:DeleteBucket", + "s3:ListBucket" + ], + "Resource": "arn:aws:s3:::seskit-*" + }, + { + "Effect": "Allow", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::seskit-*/*" + }, + { + "Effect": "Allow", + "Action": [ + "ses:CreateReceiptRuleSet", + "ses:CreateReceiptRule", + "ses:UpdateReceiptRule", + "ses:DeleteReceiptRule", + "ses:DescribeActiveReceiptRuleSet", + "ses:DescribeReceiptRuleSet", + "ses:SetActiveReceiptRuleSet", + "ses:DeleteReceiptRuleSet" + ], + "Resource": "*" + } + ] +} +``` + +The topic and queue SESKit creates for received mail are named `seskit-inbound`, so +the SNS and SQS statements in the [second policy](#delivery-events-fifteen-more-optional) +already cover them. If you did not grant that policy, receiving needs those two +statements as well. + +The bucket statements are **scoped by resource** to names beginning `seskit-`, so +this policy cannot touch a bucket you made yourself. The same prefix as the second +policy applies: if you set `EVENT_RESOURCE_PREFIX` to something that does not begin +with `seskit-`, change those ARNs to match. + +`s3:ListBucket` is easy to leave out and worth keeping. Without it S3 answers a +request for a message that has expired with *access denied* instead of *not found*, +so an ordinary expiry would look like a permissions failure. + +The SES receipt-rule actions are granted on `*`. Whether they can be narrowed to a +single rule set has not been checked, and a policy that claimed a scope AWS does not +enforce would be worse than one that says it has none. + +`s3:DeleteBucket` is there so that turning receiving off, or disconnecting, can +tidy up. A bucket that still holds mail is never emptied: it is left in place and +removed by retention in time. + ## Starting smaller A reasonable order, if you would rather not grant everything at once: @@ -125,6 +195,7 @@ A reasonable order, if you would rather not grant everything at once: | Send for real | `+ ses:SendEmail` | Deliver through SES | | Leave the sandbox | `+ ses:PutAccountDetails` | Request production access from the AWS page | | See what happened | the second policy | Delivery events and webhooks | +| Receive mail | the third policy | [Receiving on a domain](receive-email.md), the Inbox, and `email.received` | Each stage is usable on its own, and the dashboard says what is missing rather than failing obscurely. diff --git a/docs/guides/receive-email.md b/docs/guides/receive-email.md new file mode 100644 index 0000000..20f7b95 --- /dev/null +++ b/docs/guides/receive-email.md @@ -0,0 +1,176 @@ +# Receive email + +SESKit can receive mail as well as send it. Mail sent to a domain you choose is +stored, shown in the dashboard's **Inbox**, readable through the +[API](../reference/api.md#get-v1inbound) and the [Python +SDK](../reference/sdk.md#received-mail), and announced to your application as an +[`email.received` webhook](../reference/events.md#emailreceived). + +It is built on Amazon SES's own receiving. SES accepts the message, stores the +original in an S3 bucket in **your** account, and announces it; SESKit reads the +announcement, parses the message, and keeps what it finds. There is no mail +server to run and nothing for you to monitor beyond the worker SESKit already +needs. + +!!! note "What this is not" + It does not give you a mailbox. There is no IMAP, no POP and no webmail: + an application reads the mail through the API or a webhook, and a person + reads it in the Inbox. To reply, send a message with the + [send API](../reference/api.md) and set `reply_to`. + +## Before you start + +- A **verified domain** — see [verify a sender](verify-a-sender.md). Receiving is + per domain; an email address verifies a sender and has no MX record to point + anywhere. +- An **AWS connection** in a region where Amazon SES can receive mail. That is + most regions but not all; the Domains page tells you if yours is one that + cannot, and which can. +- The third [IAM policy](iam-policies.md#receiving-mail-sixteen-more-optional). + Receiving creates a bucket and edits SES receipt rules, which nothing else in + SESKit does, so it is a separate grant you can leave out. +- The worker running, reading from SQS. That is the default + (`EVENT_INGESTION=sqs`); an instance set to `https` only cannot receive and the + page says so. + +## Turn it on + +Open the **Domains** page, find the domain, and press **Start receiving**. + +Before you press it the card names what it will create, in your AWS account: + +| Resource | Purpose | +|---|---| +| S3 bucket `seskit-inbound--` | holds the original messages; private, encrypted, reachable over TLS only | +| SNS topic and SQS queue `seskit-inbound` | SES announces a message here; the worker reads it | +| One SES receipt rule for the domain | stores the message and announces it | + +**SESKit never replaces your SES receipt rule set.** An account has one active +rule set, and replacing it would switch off whatever you receive now. The rule +is added to the set that is active, at the front, and it has no *stop* action, so +it does not change what any of your other rules do — a message that matches both +simply gets both. If your account has no active rule set, SESKit creates one +called `seskit-inbound` and activates it. Turning receiving off removes only what +SESKit created. + +AWS bills these to your account. SESKit does not estimate the amount; see the +Amazon SES, S3, SNS and SQS pricing pages. + +## Add the MX record + +Turning it on is not enough — mail only arrives once the domain's DNS points at +SES. The card shows the record, with a Copy on each value: + +| Type | Name | Priority | Value | +|---|---|---|---| +| `MX` | your domain | `10` | `inbound-smtp..amazonaws.com` | + +The region is the one the domain is verified in. A record naming a different +one would deliver nowhere, and nothing would say so. + +!!! warning "This replaces wherever the domain receives mail now" + An MX record sends **all** of a domain's mail to Amazon SES. If + `example.com` receives mail somewhere else today — Google Workspace, + Microsoft 365, a forwarding service — that mail stops arriving there the + moment this record is live. + + To keep that, receive on a **subdomain** such as `mail.example.com`. Add it on + the Domains page as its own domain, verify it, and turn receiving on there. + Mail to `you@example.com` carries on as before; mail to + `anything@mail.example.com` comes to SESKit. + +SESKit cannot check that the record is in place, and neither can the page: DNS is +yours. `dig MX example.com` from any terminal shows what the world sees. + +```bash +dig +short MX example.com +``` + +## Check it works + +Send a message to any address at the domain from an ordinary mailbox. It should +appear in the **Inbox** within about a minute — that is how often the worker +looks. + +If it does not: + +| Symptom | Usually | +|---|---| +| Nothing arrives | The MX record is not live yet (DNS can take a while), or names the wrong region. Check with `dig`. | +| "Start receiving" is greyed out | The card says why: not verified, no AWS connection, a region that cannot receive, an instance set to `https` ingestion, or the domain already receives for another project. | +| Starting fails with a permission error | The message names the AWS action that is missing. Add it to the [policy](iam-policies.md#receiving-mail-sixteen-more-optional). | +| A message shows but its attachments will not download | The original has passed [retention](#retention), or the access key lacks `s3:GetObject` or `s3:ListBucket`. The page says which. | +| "SESKit could not read this message" | The message is stored and counted but nothing could be parsed from it. The original is still downloadable until retention removes it. | + +## Reading it + +| Where | How | +|---|---| +| **Dashboard** | The Inbox page. Each message shows who it is from and to, who it was actually delivered to, what SES concluded about it, its text and HTML, its attachments, its headers and the original. | +| **API** | [`GET /v1/inbound`](../reference/api.md#get-v1inbound) lists, [`GET /v1/inbound/{id}`](../reference/api.md#get-v1inboundinbound_id) reads one, and attachments and the original download from the same path. | +| **SDK** | `client.inbound.list()`, `.get()`, `.attachment()` and `.raw()`. | +| **Webhook** | An [`email.received`](../reference/events.md#emailreceived) event for every message. It describes the message and does not carry it; fetch the rest with the API. | + +```python +page = client.inbound.list(domain="mail.example.com") +for summary in page: + message = client.inbound.get(summary.id) + print(message.from_, message.subject, message.verdicts.spf) +``` + +## Treat all of it as hostile + +Everything in a received message was written by somebody who may mean you harm. +SESKit is built accordingly, and your own code should be too. + +- **The HTML is shown in a sandbox.** The Inbox renders it as a separate document + with no script, no forms and no network access, so remote images and tracking + pixels do not load. A link you click opens inside that frame, still without + script. The source is one click away in a collapsed card. +- **The API returns HTML exactly as sent.** If your own code renders it, do not + render it in a page that carries your own credentials. +- **Attachments only ever download.** They are served as opaque bytes whatever + type the sender declared, with headers that stop a browser from displaying them. +- **The sender's name and address prove nothing.** Anybody can write anything in a + `From` header. SES checks SPF, DKIM, DMARC, spam and viruses and SESKit shows + the result, but acts on none of it: whether to trust a message is your + decision. A check shown as **Not checked** means SES did not say, which is not + the same as a pass. + +See the [security model](../design/security-model.md#received-mail-is-hostile-input) +for the reasoning. + +## Retention + +The original message and its attachments stay in your bucket for +`INBOUND_RETENTION_DAYS` days — **30** by default, set in +[configuration](../reference/configuration.md#received-mail) — and are then +removed by S3. + +The parsed message is not removed. After retention it is still in the Inbox and +the API with its bodies and headers; only the attachments and the downloadable +original are gone. Changing the setting applies the next time receiving is set up, +and messages already stored keep the clock they had. + +## Turn it off + +Press **Stop receiving** on the domain, then remove the MX record from your DNS. + +SESKit removes the receipt rule and, if no other domain uses them, the topic and +queue. **Mail already received is kept.** A bucket that still holds messages is +left in place for the same reason, and removed by retention in time; the card +does not delete somebody's mail as a side effect of switching receiving off. + +Removing a domain, or disconnecting AWS, does the same first, so neither can +leave a rule behind in your account. + +## Limits + +- **One project receives for a domain.** A domain's MX record names a single + endpoint in a single region, so two projects both receiving for it could not both + be right. A second is refused, and the page says so without saying whose it is. +- **Messages up to 40 MB**, which is Amazon SES's own limit for storing a message. +- **No quarantine.** SES scans for spam and viruses and SESKit records the verdicts, + but a flagged message is stored and shown like any other. +- **No threading yet.** A reply carries `In-Reply-To` and `References` and SESKit + keeps them, but it does not link a reply to the message you sent. diff --git a/mkdocs.yml b/mkdocs.yml index ac22f51..97f6089 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -119,18 +119,19 @@ plugins: - guides/no-aws-account-yet.md: The path from no AWS account to a verified sender, and what SES costs. - guides/get-an-access-key.md: The IAM console click-path to an access key SESKit can use, and how to rotate one. - guides/connect-aws.md: Pointing SESKit at an AWS account by pasting an access key, which is stored per project and encrypted. - - guides/iam-policies.md: The exact IAM permissions, why each is needed, and how to grant them in stages. + - guides/iam-policies.md: The exact IAM permissions for sending, delivery events and receiving mail, why each is needed, and how to grant them in stages. - guides/verify-a-sender.md: Verifying a domain or a single address; DKIM, SPF and DMARC. - guides/ses-sandbox.md: What the SES sandbox restricts and how to leave it. - guides/delivery-events.md: How SES reports deliveries and bounces, SQS versus HTTPS ingestion, and open/click tracking. - guides/webhooks.md: Registering endpoints, verifying the HMAC signature, retries, and the SSRF rules. + - guides/receive-email.md: Receiving mail for a domain - what it creates in your AWS account, the MX record and why to use a subdomain, the Inbox, retention, and why everything in a received message is untrusted. - guides/metrics.md: The six counts and five rates, and what each rate divides by. - guides/suppression.md: What lands on the suppression list, what a suppressed send answers, and one-click unsubscribe. Reference: - reference/api.md: Every HTTP endpoint, generated from the OpenAPI schema. - - reference/sdk.md: The Python SDK - send, get and list, sync and async, errors as classes, retries and idempotency. + - reference/sdk.md: The Python SDK - send, get and list, reading received mail, sync and async, errors as classes, retries and idempotency. - reference/configuration.md: Every environment variable, its default, and its effect. - - reference/events.md: The event vocabulary, webhook payload shape, and bounce types. + - reference/events.md: The event vocabulary, webhook payload shape, bounce types, and the email.received event for mail you receive. - reference/errors.md: Every error type, which are worth retrying, and which are not. Operating SESKit: - operating/deploying.md: What has to run, the required environment, and health checks. @@ -178,6 +179,7 @@ nav: - Leave the SES sandbox: guides/ses-sandbox.md - Delivery events: guides/delivery-events.md - Webhooks: guides/webhooks.md + - Receive email: guides/receive-email.md - Suppression: guides/suppression.md - Reading your metrics: guides/metrics.md - Reference: diff --git a/tests/test_docs.py b/tests/test_docs.py index 83a3237..b7ebf47 100644 --- a/tests/test_docs.py +++ b/tests/test_docs.py @@ -124,7 +124,7 @@ def test_the_iam_guide_counts_its_own_actions() -> None: ] assert WORDS[said.group(1)] == len(actions), heading checked += 1 - assert checked == 2, "both policy sections should have been counted" + assert checked == 3, "all three policy sections should have been counted" def test_the_agent_index_describes_every_page() -> None: