Send sign-in mail after the response, so its timing says nothing
Astra on #20 (P1): a permitted address waited for SMTP before its ?sent=1 and a denied one did not — 252 ms against 0.35 ms — so the allowlist was readable off a stopwatch, and force=1 exposed Keymail addresses the same way. rauth's Mailer is now mailQueue: Send queues the link and returns at once, and one background worker asks the allowlist and only then talks to SMTP. POST /signin does the same synchronous work for every address (rauth's budgets, its link row, the enqueue); the send, and any failure of it, happens where nobody can time it and is logged, never answered. The queue is bounded (a full queue drops with a log line) and app.Close drains it. TestSigninNeverWaitsForTheMailer holds the mailer blocked and requires every POST — permitted, denied, and both with force=1 — to answer while it is, then the permitted links to go out. It fails on a61f0d2.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
12 files changed,
+264
−35
AGENTS.md+1 −1internal/slides/app.go+11 −1internal/slides/auth.go+15 −32internal/slides/config.go+1 −1internal/slides/handlers.go+15 −0internal/slides/home_test.go+1 −0internal/slides/keymail_home_test.go+1 −0internal/slides/mailqueue.go+124 −0internal/slidestest/browser_test.go+1 −0internal/slidestest/capabilities_test.go+1 −0internal/slidestest/harness_test.go+10 −0internal/slidestest/signin_timing_test.go+83 −0
CI passed — run details
CI log
=== gate ===
=== rust: fmt
=== rust: clippy
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.16s
=== wasm: the artifact the host embeds
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.05s
Finished `release` profile [optimized] target(s) in 0.18s
core/target/wasm32-unknown-unknown/release/slides_core.wasm (1228229 bytes)
=== the embedded wasm must match a fresh build
=== rust: test
Finished `test` profile [unoptimized + debuginfo] target(s) in 0.05s
Running unittests src/lib.rs (core/target/debug/deps/slides_core-929bfceb2edd4a94)
running 30 tests
test check::tests::the_empty_document_passes ... ok
test check::tests::bare_text_at_fragment_level_is_reported ... ok
test check::tests::a_non_paragraph_child_is_reported ... ok
test layout::tests::default_region_by_kind ... ok
test layout::tests::heading_region_rect ... ok
test check::tests::y_prosemirror_shapes_pass ... ok
test check::tests::an_element_inside_a_paragraph_is_reported ... ok
test check::tests::an_unknown_formatting_attribute_is_reported ... ok
test check::tests::what_the_ops_write_passes ... ok
test layout::tests::to_canvas_narrows_body_when_an_aside_is_present ... ok
test layout::tests::to_flow_classifies_by_position_and_kind ... ok
test layout::tests::to_flow_is_a_no_op_when_already_in_order ... ok
test layout::tests::to_flow_orders_y_then_x ... ok
test layout::tests::to_flow_keeps_a_concurrent_text_edit ... ok
test markdown::tests::marks_nest_and_links_carry_marks ... ok
test markdown::tests::paragraphs_split_on_blank_lines_and_list_markers ... ok
test markdown::tests::inline_marks ... ok
test schema::tests::an_empty_map_reads_as_a_set_mark ... ok
test schema::tests::an_href_map_without_a_string_url_reads_as_no_link ... ok
test schema::tests::href_is_written_as_a_one_key_map_and_reads_back ... ok
test markdown::tests::empty_input_is_no_paragraphs_and_lines_keep_blanks ... ok
test schema::tests::href_written_as_a_plain_string_still_reads_back ... ok
test layout::tests::media_region_rect ... ok
test layout::tests::body_region_rect_narrows_for_an_aside ... ok
test view::tests::assets_are_typed_or_skipped ... ok
test view::tests::malformed_background_is_the_default ... ok
test view::tests::malformed_box_is_null_and_phone_keeps_only_typed_fields ... ok
test layout::tests::aside_region_rect ... ok
test layout::tests::to_canvas_splits_a_shared_region_equally_and_sets_z_by_index ... ok
test view::tests::malformed_fonts_are_null ... ok
test result: ok. 30 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s
Running tests/no_host_imports.rs (core/target/debug/deps/no_host_imports-184c0d8bf00e14f9)
running 1 test
test no_host_imports ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s
Running tests/ops.rs (core/target/debug/deps/ops-256addce27f366d1)
running 12 tests
test add_slide_then_view_lists_it ... ok
test add_block_with_markdown_produces_runs ... ok
test layout_switch_assigns_boxes ... ok
test remove_block_drops_it_from_columns_children ... ok
test duplicate_slide_remaps_columns_children ... ok
test columns_children_must_name_blocks_on_the_slide ... ok
test columns_children_have_one_parent_and_no_cycles ... ok
test move_slide_keeps_a_concurrent_text_edit ... ok
test update_roundtrip ... ok
test ops_are_atomic ... ok
test move_block_keeps_a_concurrent_text_edit ... ok
test remaining_ops_roundtrip ... ok
test result: ok. 12 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.03s
Doc-tests slides_core
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
=== rust: fmt (pdf)
=== rust: clippy (pdf)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.07s
=== wasm: the pdf artifact the host embeds
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.03s
Finished `release` profile [optimized] target(s) in 0.10s
core/pdf/target/wasm32-unknown-unknown/release/slides_pdf.wasm (5301002 bytes)
=== the embedded pdf wasm must match a fresh build
=== rust: test (pdf)
Finished `test` profile [unoptimized + debuginfo] target(s) in 0.03s
Running unittests src/lib.rs (core/pdf/target/debug/deps/slides_pdf-891fafaa44f9fd64)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
Running tests/no_host_imports.rs (core/pdf/target/debug/deps/no_host_imports-17ccbb1d62120093)
running 1 test
test no_host_imports ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s
Running tests/render.rs (core/pdf/target/debug/deps/render-2e1d7e8b19d78595)
running 6 tests
test a_null_zero_length_pair_is_no_frames_not_a_dereference ... ok
test text_with_no_fonts_at_all_still_renders_the_shapes ... ok
test raster_image_is_embedded ... ok
test renders_text_as_embedded_glyphs ... ok
test two_svgs_make_two_pages ... ok
test abi_frames_round_trip ... ok
test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s
Doc-tests slides_pdf
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
=== fixtures: node dependencies
v24.19.0
11.17.0
=== fixtures: npm ci (needs the registry — see .amadan/README.md)
added 19 packages, and audited 20 packages in 5s
5 packages are looking for funding
run `npm fund` for details
found 0 vulnerabilities
npm warn allow-scripts 1 package has install scripts not yet covered by allowScripts:
npm warn allow-scripts esbuild@0.28.2 (postinstall: node install.js)
npm warn allow-scripts
npm warn allow-scripts Run `npm approve-scripts --allow-scripts-pending` to review, or `npm approve-scripts <pkg>` to allow.
=== fixtures: the JS side's own tests
✔ one finished upload is register_asset plus the block that uses it, in one batch (11.204028ms)
✔ a record missing its numbers still makes an op the core accepts (1.081049ms)
✔ unusedAssets is what no slide refers to, and remove_asset drops one (3.483043ms)
✔ the Unused images list reconciles by id and names each Remove button (1.260042ms)
✔ an upload refusal is said in words the person can act on (0.251217ms)
✔ clampBox keeps the box inside the frame and no smaller than the minimum (1.993835ms)
✔ fixBox writes four decimals (0.337868ms)
✔ guides are the thirds plus every other block edge (0.306069ms)
✔ snapMove pulls the nearest edge onto a guide within one per cent, and leaves a far one (0.567555ms)
✔ snapResize moves only the edges the handle drags (0.279519ms)
✔ moveBox and resizeBox by handle (0.307171ms)
✔ nudge: arrows move by half a per cent, Shift by two, Alt resizes (0.269982ms)
✔ rectBox reads a block rectangle against its frame (0.44121ms)
✔ typing into a block is a text change on its slide only (6.944404ms)
✔ a block's props changing is structure on its slide (1.321896ms)
✔ adding, moving or removing a block is structure on its slide (5.315839ms)
✔ the notes are text on their slide (1.268437ms)
✔ a slide added, removed or moved is an order change (1.612747ms)
✔ one transaction touching several things reports each once (0.930499ms)
✔ what the ops write passes, and so does an empty document (10.13931ms)
✔ the shapes y-prosemirror and older documents write pass (1.994096ms)
✔ an unknown formatting attribute is reported with the core's message (2.029382ms)
✔ an element inside a paragraph, a non-paragraph child and bare text are reported (2.977814ms)
✔ the unknownmark poison shape is what the check refuses (1.921891ms)
✔ y-prosemirror 1.3.7 deletes the words under an unknown mark; sanitising first keeps them (5.283117ms)
✔ the vocabulary is the five marks and nothing else (0.169916ms)
✔ a reload reuses the id this tab stored for the deck (1.206031ms)
✔ two tabs, and two decks in one tab, get different ids (1.218555ms)
✔ a stored value that is not an id, or storage that throws, gives a fresh id (0.418588ms)
✔ a duplicated tab gives up the copied id; the original keeps it (4.664849ms)
✔ a reload finds no rival and keeps its id (2.709986ms)
✔ presence switches id: beats carry the new one and the old one is somebody else (1.322778ms)
✔ an empty body is no frames (1.660766ms)
✔ frames come back in order with their exact bytes (0.372813ms)
✔ a length prefix is big-endian (0.198008ms)
✔ a truncated frame throws rather than returning a short update (1.411933ms)
✔ deck controls are set_meta (15.057429ms)
✔ slide controls are set_slide, and background keeps its colour (4.157656ms)
✔ move up and down are move_slide with the right after (3.176093ms)
✔ block props: value kinds, region, a shape label (3.089631ms)
✔ box fields are set_block on the desktop and set_phone in the phone view (5.195584ms)
✔ move block and the layer buttons (1.676846ms)
✔ add block: region after the selection on flow, a default box on canvas, a heading level that fits (8.414195ms)
✔ a block is named by its type and its first words (1.774648ms)
✔ blockChange: a control element to its op (2.09901ms)
✔ the Blocks list reconciles by id and keeps its buttons (1.510076ms)
✔ open resolves to the button that closed it and the trimmed field (1.995177ms)
✔ a second open while the dialog is up joins the first (0.228414ms)
✔ a missing dialog mounts as nothing (0.13947ms)
✔ a tab advances its own cursor only when its POST is the very next row (2.901191ms)
✔ the cursor never moves backwards when a response is overtaken by this tab's own write (0.628619ms)
✔ a later response still carries the cursor forward (0.275832ms)
✔ a burst of notifications collapses into one fetch at a time (0.247671ms)
✔ an event naming a seq the tab already has fetches nothing (0.206714ms)
✔ a stopped subscriber drops a response that arrives late (0.415863ms)
✔ stop closes the stream, and start opens exactly one (1.7004ms)
✔ a hidden tab closes its stream on a shortened deadline, and activity reopens it (1.305136ms)
✔ switching away and back inside the grace never drops the stream, so nothing has to be caught up (0.274671ms)
✔ a visible tab that is only being READ closes when idle and comes back on any activity (1.790527ms)
✔ activity while streaming defers the idle close rather than reopening anything (0.213396ms)
✔ a stream failure is never reported as a save failure, and heals on activity (0.283898ms)
✔ an empty response advances the cursor without applying anything (0.185154ms)
✔ a failed fetch leaves the cursor alone so the next notification retries it (0.278137ms)
✔ a decode failure retries once through the raw log, then gives up (3.124116ms)
✔ updatesURL spells the query string exactly, in particular raw as the literal raw=1 (0.181537ms)
✔ empty_doc_views_as_defaults (7.090897ms)
✔ set_meta (3.613464ms)
✔ slides: add, move, remove, set_slide, set_notes (7.980028ms)
✔ duplicate_slide (3.666924ms)
✔ blocks: add, move, remove, set_block (4.477561ms)
✔ set_phone (2.054738ms)
✔ table cells follow rows and cols (2.52378ms)
✔ canvas blocks: box defaults from the region, z from the index (1.680723ms)
✔ assets: register and remove (1.41542ms)
✔ markdown: every mark, code blocks are plain lines (7.483495ms)
✔ batch_is_atomic (2.723341ms)
✔ a batch applies in one transaction with the given origin (1.147994ms)
✔ columns_children_validated (2.106454ms)
✔ layout_switch_round_trip (2.368653ms)
✔ view_matches_snapshot_fixtures (14.724862ms)
✔ columns children have one parent and no cycles (codex 7) (1.543027ms)
✔ canvas-to-flow keeps a concurrent text edit (codex 4) (4.540918ms)
✔ move_block keeps a concurrent text edit, and later inserts land where after says (codex 4) (3.197543ms)
✔ move_slide keeps a concurrent text edit, and later inserts land where after says (codex 4 at slide level) (2.580084ms)
✔ format: {name} and {n} interpolate; an unknown key is left as text (0.88803ms)
✔ a beat on connect, then one every twenty seconds (2.3196ms)
✔ a move posts once, 200ms after the last of a burst, and re-arms the beat (1.656317ms)
✔ a hidden tab stops beating; coming back posts at once (0.42477ms)
✔ applySet leaves this tab out and answers what changed (0.623338ms)
✔ a set with rubbish in it is read for what it does carry (0.273218ms)
✔ initials, and a client id per tab (4.210776ms)
✔ a resync that fails is retried with backoff until it succeeds, without ever running twice at once (2.384771ms)
✔ the browser's online event retries immediately, not just on the next backoff tick (0.374616ms)
✔ reportDivergence before any failure runs the first attempt without a caller ever calling online (1.561362ms)
✔ a permanent 4xx drops its batch instead of retrying it, and does not poison the next edit (87.771479ms)
✔ a network failure or a 5xx is retried, not dropped (60.902602ms)
✔ a 500 is retried the same way a network failure is, never dropped (60.411418ms)
✔ the core-written structure reads as ProseMirror marks (6.45793ms)
✔ a y-prosemirror round trip preserves bold and the link (2.820572ms)
✔ y-prosemirror writes href as the one-key map and marks as {} (0.861882ms)
✔ a plain-string href, written before the map shape, still reads (0.520437ms)
✔ textFragment writes the same shape as writeText (0.530536ms)
✔ safeHref is the server allowlist, and a disallowed href never draws an anchor (0.372141ms)
✔ parseHash splits the slide and the block, and survives a stray % (2.702643ms)
✔ destroy takes the listeners off, and a second mount leaves one of each (5.084087ms)
✔ formatHash is the inverse (1.477706ms)
✔ textBlocks finds the text-bearing blocks and skips the rest (1.445486ms)
✔ mountTarget is the element whose children are the paragraphs (0.184433ms)
✔ paragraphTag keeps the renderer's own markup per type (0.141002ms)
✔ blockFragment finds a block's text by slide and block id (5.697378ms)
✔ notesFragment finds the slide's notes (0.822138ms)
✔ blockIDs and slideIDs are the orders on the stage and the strip (0.779829ms)
✔ cellFragments are a table's cells, row-major (1.23238ms)
✔ a load that fails leaves the stage on the slide it was showing and says so (1.11914ms)
✔ slideAfter is the slide to show once one is gone (0.166109ms)
✔ the queue words map to savestatus states, failed staying failed (1.180374ms)
✔ a dropped batch outranks the queue, for the rest of the session (0.191245ms)
✔ the editor status: the save line and the load error are two lines (0.278879ms)
✔ mountSaveStatus loads estilo's module and mounts it on the line (0.927994ms)
✔ a save-status module that will not load says so and refuses to mount (0.472328ms)
✔ dropTarget: before puts it ahead of the item, after behind it, null means first (0.878363ms)
✔ a drop that leaves the order as it is is no move (0.151692ms)
✔ dropOp is the move_slide (0.609713ms)
✔ labelFor is the slide's first heading, else the numbered fallback (5.086212ms)
✔ flush re-points every dirty thumbnail and relabels it (1.669834ms)
✔ destroy takes the item-menu listener off; a second mount leaves one (0.245937ms)
✔ plan says what to add, drop and where each item goes (0.669795ms)
✔ sync moves only the items out of place and keeps focus (1.044671ms)
✔ desktop view: box and size are the block; hidden and order are always the phone map (1.532147ms)
✔ phone view: everything goes to set_phone, carrying the rest of the map (0.45203ms)
✔ the ops it makes are ones the document takes (8.376405ms)
✔ phoneMap reads only the four keys, and only well-typed (0.168343ms)
✔ the stored view is read defensively (0.272296ms)
✔ linkAt reads the link under the cursor and nothing elsewhere (2.202202ms)
✔ setLink marks the selection (2.095133ms)
✔ setLink on a collapsed cursor inside a link replaces that link (0.564159ms)
✔ setLink with nothing selected and no link does nothing (0.170297ms)
✔ removeLink clears the link under the cursor (0.357966ms)
✔ the keymap binds every documented shortcut (0.233875ms)
✔ Mod-b toggles bold through the keymap (0.500911ms)
✔ setLink refuses an address outside the allowlist and writes nothing (0.436701ms)
✔ the state machine runs choose → sign → put → register → op (3.420115ms)
✔ a file the deck will not take never reaches the network (0.706252ms)
✔ alt text is required before the bytes move, unless the image is decorative (0.404852ms)
✔ a step that fails stops the ones after it and reports its own code (0.638897ms)
✔ a failure that is not ours still becomes an UploadError (0.36577ms)
✔ a second pick while one is in flight is refused, not raced (0.459624ms)
✔ an unreadable size is zero rather than a failure (0.281543ms)
✔ a per-call applyOp wins, so an upload lands on the block it was chosen for (0.292223ms)
✔ checkFile takes exactly the four types (0.315977ms)
✔ sha256Hex is the hex digest of the bytes (11.643587ms)
✔ refusal reads this app's JSON error shape, and survives one that is not (0.294899ms)
✔ malformed fonts are null (3.143943ms)
✔ a malformed background is the theme default (1.359136ms)
✔ a malformed box is null (0.644217ms)
✔ the phone layer keeps only its typed fields, order as a whole number (0.532038ms)
✔ assets are typed or skipped (1.830431ms)
✔ every poison update is deterministic bytes that apply to any deck (7.480871ms)
ℹ tests 158
ℹ suites 0
ℹ pass 158
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 313.90088
=== the vectors must match a fresh generation
canvas: 22 ops, 3715 bytes
columns: 23 ops, 3134 bytes
every-block: 36 ops, 3180 bytes
table: 14 ops, 1509 bytes
title-only: 6 ops, 1136 bytes
=== the editor bundle must match a fresh build
wrote internal/slides/static/editor.0463180baf5aab9f.js (376416 bytes)
=== go: vet, gofmt, test, migration check
go vet ./...
go test ./...
? amadan.net/oficina/slides/cmd/slides [no test files]
ok amadan.net/oficina/slides/internal/slides 12.073s
ok amadan.net/oficina/slides/internal/slides/core 2.669s
ok amadan.net/oficina/slides/internal/slides/deck (cached)
ok amadan.net/oficina/slides/internal/slides/fonts (cached)
? amadan.net/oficina/slides/internal/slides/icons [no test files]
? amadan.net/oficina/slides/internal/slides/keymailtest [no test files]
ok amadan.net/oficina/slides/internal/slides/layout (cached)
ok amadan.net/oficina/slides/internal/slides/live (cached)
ok amadan.net/oficina/slides/internal/slides/pdfcore (cached)
ok amadan.net/oficina/slides/internal/slides/present (cached)
ok amadan.net/oficina/slides/internal/slides/render (cached)
ok amadan.net/oficina/slides/internal/slides/svg (cached)
ok amadan.net/oficina/slides/internal/slidestest 38.630s
? amadan.net/oficina/slides/scripts/writeestilo [no test files]
go run amadan.net/rastrillo/rastrillo/cmd/rastrillo migration check
rastrillo migration check: models and migrations agree
python3 scripts/credits.py --check
credits: checked
gate: ok
diff --git a/AGENTS.md b/AGENTS.md| index 05b7831..ce5858b 100644 |
| --- a/AGENTS.md |
| +++ b/AGENTS.md |
| @@ -51,7 +51,7 @@ seeded on first sign-in — and comes back out as a PDF. |
| oracle. A Keymail address, permitted or not, gets the continuation |
| page to Keymail (never a 303 out of the form POST: Safari applies |
| `form-action` to it); any other gets `303 ?sent=1`, and only a |
| - permitted one is mailed (`admissionMailer`). Admission is |
| + permitted one is mailed, by a worker after the response (`mailQueue`), so not even the response time differs. Admission is |
| rastrillo/auth's `Authorize`, after verification, and its refusal is |
| the door with `?err=refused` (`doorOnRefusal`), not rastrillo's bare |
| text. `?force=1&err=keymail` (a failed Keymail exchange) makes the |
diff --git a/internal/slides/app.go b/internal/slides/app.go| index c820921..55f269c 100644 |
| --- a/internal/slides/app.go |
| +++ b/internal/slides/app.go |
| @@ -302,12 +302,17 @@ func newApp(d *db.DB, cfg Config) (*app, *rauth.Auth, error) { |
| logger.Info("migrate", "applied", migrated.Applied, |
| "skipped", migrated.Skipped, "adopted", migrated.Adopted) |
| - authPlugin, err := newAuthPlugin(cfg, d.Writer(), list) |
| + // The sign-in mailer: links are queued and sent after the response, |
| + // to permitted addresses only (mailqueue.go). Closed by app.Close. |
| + mailer := newMailQueue(cfg.Mailer, list.allows, logger) |
| + authPlugin, err := newAuthPlugin(cfg, d.Writer(), list, mailer) |
| if err != nil { |
| + mailer.close() |
| return nil, nil, err |
| } |
| returnSeal, err := newReturnSealer(cfg.InstanceKey) |
| if err != nil { |
| + mailer.close() |
| return nil, nil, err |
| } |
| @@ -320,6 +325,7 @@ func newApp(d *db.DB, cfg Config) (*app, *rauth.Auth, error) { |
| // request happens to open the first deck. |
| rt, err := core.New(context.Background()) |
| if err != nil { |
| + mailer.close() |
| return nil, nil, err |
| } |
| @@ -332,6 +338,7 @@ func newApp(d *db.DB, cfg Config) (*app, *rauth.Auth, error) { |
| returnNow: time.Now, |
| core: rt, |
| auth: authPlugin, |
| + mail: mailer, |
| hub: present.New(), |
| live: live.New(), |
| jobs: jobs.New(logger), |
| @@ -345,12 +352,14 @@ func newApp(d *db.DB, cfg Config) (*app, *rauth.Auth, error) { |
| sess, err := sessions.New(sessions.Config{DB: d.Writer(), Origin: cfg.Origin, Logger: logger}) |
| if err != nil { |
| rt.Close(context.Background()) |
| + mailer.close() |
| return nil, nil, err |
| } |
| a.sessions = sess |
| if err := a.initHome(homeConfig); err != nil { |
| rt.Close(context.Background()) |
| + mailer.close() |
| return nil, nil, err |
| } |
| @@ -365,6 +374,7 @@ func newApp(d *db.DB, cfg Config) (*app, *rauth.Auth, error) { |
| }) |
| if err != nil { |
| rt.Close(context.Background()) |
| + mailer.close() |
| return nil, nil, err |
| } |
| a.jobPages = pages |
diff --git a/internal/slides/auth.go b/internal/slides/auth.go| index d8abe2e..551f5f8 100644 |
| --- a/internal/slides/auth.go |
| +++ b/internal/slides/auth.go |
| @@ -15,8 +15,8 @@ |
| // credential at Keymail's own origin, and a magic link enrols nothing. |
| // |
| // Admission is a separate question from the ceremony, asked at two |
| -// places that cannot be observed from the form: the mailer |
| -// (admissionMailer — a non-permitted address is never mailed) and |
| +// places that cannot be observed from the form: the mail worker |
| +// (mailQueue — a non-permitted address is never mailed) and |
| // rauth's Authorize, after Keymail or the link has verified the |
| // address. Neither changes what POST /signin answers (gateBegin). |
| // |
| @@ -168,7 +168,8 @@ func resolveAllowlist(cfg Config) (allowlist, error) { |
| // door saying so (doorOnRefusal) — told only to somebody who has |
| // just proved they hold the address. |
| // - Any other address is answered `303 /signin?sent=1`, and a link is |
| -// mailed only if it is permitted: admissionMailer drops the rest. |
| +// mailed only if it is permitted: mailQueue drops the rest, after |
| +// the response, so not even its timing differs (mailqueue.go). |
| // So this instance's sending identity is no open relay for whatever |
| // a stranger types, which is why a gate exists at all |
| // (rauth.Config.Authorize alone runs only after a link was mailed |
| @@ -181,37 +182,19 @@ func resolveAllowlist(cfg Config) (allowlist, error) { |
| // stuck. A stranger posting force for any address gets exactly the |
| // plain-email answer above. |
| // |
| -// What sameAnswer still has to hide is the one outcome only a |
| -// permitted address can reach: `?err=1`, a send that failed, since a |
| -// non-permitted address never gets as far as a real send. `?err=rate` |
| -// and `?err=address` no longer depend on admission — both kinds of |
| -// address spend the same budgets and parse the same way — but |
| -// sameAnswer still answers them as sent, as it always has. Giving those |
| -// two back their own sentences is a separate, safe change, not made |
| -// here. |
| +// No outcome of Begin depends on admission any more: both kinds of |
| +// address spend the same budgets and parse the same way, and the send |
| +// that only a permitted address reaches happens in mailQueue's worker, |
| +// after the response, where its failures are logged rather than |
| +// answered. sameAnswer still rewrites `?err=rate|address|1` to sent, as |
| +// it always has; giving rate and address back their own sentences is a |
| +// separate, safe change, not made here. |
| func gateBegin(next http.HandlerFunc, logger *slog.Logger) http.HandlerFunc { |
| return func(w http.ResponseWriter, r *http.Request) { |
| next(&sameAnswer{ResponseWriter: w, logger: logger}, r) |
| } |
| } |
| -// admissionMailer is the mail half of the allowlist: Begin runs for |
| -// every address (gateBegin), and this is where a non-permitted one is |
| -// quietly not mailed. rauth has already stored an expiring link token |
| -// by the time it calls Send; one that is never delivered is never |
| -// redeemed, and Authorize would refuse it if it were. |
| -type admissionMailer struct { |
| - next mail.Sender |
| - list allowlist |
| -} |
| - |
| -func (m admissionMailer) Send(ctx context.Context, to, subject, body string) error { |
| - if !m.list.allows(to) { |
| - return nil |
| - } |
| - return m.next.Send(ctx, to, subject, body) |
| -} |
| - |
| // newBegin is the POST /signin handler both modes mount: gateBegin |
| // under connect.OAuthContinuation, which ends the POST with a page |
| // (renderKeymailContinuation) when Begin's answer is Keymail's |
| @@ -345,8 +328,8 @@ func (s *sameAnswer) WriteHeader(status int) { |
| // both the emailed link's base and the cookie decision, InstanceKey and |
| // Mailer already resolved and fail-closed above. |
| // |
| -// Authorize is list.allows as well as admissionMailer — two checks |
| -// defending two different things: admissionMailer stops the mail, |
| +// Authorize is list.allows as well as mailQueue — two checks |
| +// defending two different things: mailQueue stops the mail, |
| // Authorize stops a session being minted for a non-permitted address, |
| // whether Keymail verified it or a link was somehow issued for it |
| // (mailed before the allowlist changed, say). SecondFactor stays nil, |
| @@ -361,7 +344,7 @@ func (s *sameAnswer) WriteHeader(status int) { |
| // copy of that literal and app.go mounts the page at ours; saying it |
| // once, out loud, is what keeps the two from drifting silently if |
| // either default ever changes. |
| -func newAuthPlugin(cfg Config, writer *sql.DB, list allowlist) (*rauth.Auth, error) { |
| +func newAuthPlugin(cfg Config, writer *sql.DB, list allowlist, mailer *mailQueue) (*rauth.Auth, error) { |
| signedInPath := landedPath |
| if cfg.Home != nil { |
| signedInPath = connect.LocalCompletePath |
| @@ -371,7 +354,7 @@ func newAuthPlugin(cfg Config, writer *sql.DB, list allowlist) (*rauth.Auth, err |
| DB: writer, |
| Origin: cfg.Origin, |
| InstanceKey: cfg.InstanceKey, |
| - Mailer: admissionMailer{next: cfg.Mailer, list: list}, |
| + Mailer: mailer, |
| Subject: chrome.EmailSubject("Slides"), |
| Authorize: list.allows, |
| SigninPath: signinPath, |
diff --git a/internal/slides/config.go b/internal/slides/config.go| index 3374f11..89bcff4 100644 |
| --- a/internal/slides/config.go |
| +++ b/internal/slides/config.go |
| @@ -92,7 +92,7 @@ type Config struct { |
| // |
| // It is wired to two places on purpose: rauth.Config.Authorize |
| // gates admission, after Keymail or a link has verified the |
| - // address, and admissionMailer (auth.go) drops the mail for an |
| + // address, and mailQueue (mailqueue.go) drops the mail for an |
| // address not on it. The second is what keeps this instance's |
| // sending identity from being an open relay for any address a |
| // stranger types into the form, since Authorize fires only after |
diff --git a/internal/slides/handlers.go b/internal/slides/handlers.go| index 83fa42d..2de7561 100644 |
| --- a/internal/slides/handlers.go |
| +++ b/internal/slides/handlers.go |
| @@ -42,6 +42,8 @@ type app struct { |
| logger *slog.Logger |
| cfg Config |
| allowed allowlist |
| + // mail is the sign-in mail queue rauth sends through (mailqueue.go). |
| + mail *mailQueue |
| // returnSeal authenticates the short-lived return_to cookie. It is |
| // derived from this instance's sign-in key, never from a browser |
| // value, and the clock is a seam for expiry tests. |
| @@ -113,6 +115,10 @@ func (a *app) Close() error { |
| if a.live != nil { |
| a.live.Close() |
| } |
| + // Queued sign-in links are sent before the process lets go. |
| + if a.mail != nil { |
| + a.mail.close() |
| + } |
| // The PDF module, if any export ever built one. Same argument as |
| // the deck core's below: a test binary builds an app per test, and |
| // a compiled wasm module nobody releases is a leak per app. |
| @@ -124,3 +130,12 @@ func (a *app) Close() error { |
| } |
| return a.core.Close(context.Background()) |
| } |
| + |
| +// FlushMail waits until every sign-in link queued so far has been sent |
| +// or dropped. The links go out after the response (mailqueue.go), so a |
| +// test reads the mailer only after this. |
| +func (a *app) FlushMail() { |
| + if a.mail != nil { |
| + a.mail.wait() |
| + } |
| +} |
diff --git a/internal/slides/home_test.go b/internal/slides/home_test.go| index 6400586..700de7b 100644 |
| --- a/internal/slides/home_test.go |
| +++ b/internal/slides/home_test.go |
| @@ -469,6 +469,7 @@ func TestHomeLocalPluginRecoveryRotatesThenReturnsToLink(t *testing.T) { |
| if sent.Code != 303 { |
| t.Fatal(sent.Code) |
| } |
| + a.FlushMail() |
| body := a.cfg.Mailer.(*homeTestMail).body |
| link := regexp.MustCompile(`https://[^\s<>]+/auth/verify\?token=[A-Za-z0-9_-]+`).FindString(body) |
| if link == "" { |
diff --git a/internal/slides/keymail_home_test.go b/internal/slides/keymail_home_test.go| index 66841cf..b0a8d0f 100644 |
| --- a/internal/slides/keymail_home_test.go |
| +++ b/internal/slides/keymail_home_test.go |
| @@ -92,6 +92,7 @@ func TestHomeLocalKeymailIsAContinuationAndRefusesAtTheDoor(t *testing.T) { |
| _, _ = begin(member) |
| rec, u := begin(stranger) |
| + a.FlushMail() |
| if mail.body != "" { |
| t.Fatal("a Keymail address was mailed a link") |
| } |
diff --git a/internal/slides/mailqueue.go b/internal/slides/mailqueue.go| new file mode 100644 |
| index 0000000..a0f9d55 |
| --- /dev/null |
| +++ b/internal/slides/mailqueue.go |
| @@ -0,0 +1,124 @@ |
| +package slides |
| + |
| +import ( |
| + "context" |
| + "log/slog" |
| + "sync" |
| + "time" |
| + |
| + "amadan.net/rastrillo/rastrillo/mail" |
| +) |
| + |
| +// mailQueue is the sign-in mailer rauth is given: Send queues the link |
| +// and returns at once, and one background worker asks the allowlist and |
| +// only then talks to SMTP. |
| +// |
| +// Both halves are the no-oracle guarantee (gateBegin), measured rather |
| +// than reasoned about. When the send ran inside the request, a permitted |
| +// address waited for the SMTP round trip before its `303 ?sent=1` and a |
| +// denied one did not: 252 ms against 0.35 ms (Astra on #20). A response |
| +// time is an answer like any other. So POST /signin does the same |
| +// synchronous work for every address — rauth's budgets, its link row, |
| +// this enqueue — and the one admission-dependent step, the send, happens |
| +// after the response, where nobody can time it. The admission check |
| +// moved into the worker for the same reason: a check before responding |
| +// would be a branch an attacker could time, however cheap it is today. |
| +// |
| +// Delivery failures are logged, never surfaced. A failure only a |
| +// permitted address can reach is the oracle again, and the person is |
| +// told what every address is told: check your email, or ask again. |
| +// |
| +// Bounded: a queue of mailQueueDepth links. A full queue drops the link |
| +// with a log line instead of blocking a request — the per-IP and |
| +// per-address budgets inside Begin keep that from being an ordinary |
| +// event. Close drains what is queued, within mailDrainTimeout. |
| +type mailQueue struct { |
| + next mail.Sender |
| + admits func(string) bool |
| + logger *slog.Logger |
| + |
| + mu sync.Mutex |
| + closed bool |
| + jobs chan mailJob |
| + pending sync.WaitGroup |
| + done chan struct{} |
| +} |
| + |
| +type mailJob struct{ to, subject, body string } |
| + |
| +const ( |
| + mailQueueDepth = 64 |
| + mailSendTimeout = time.Minute |
| + mailDrainTimeout = 30 * time.Second |
| +) |
| + |
| +func newMailQueue(next mail.Sender, admits func(string) bool, logger *slog.Logger) *mailQueue { |
| + if logger == nil { |
| + logger = slog.Default() |
| + } |
| + q := &mailQueue{next: next, admits: admits, logger: logger, |
| + jobs: make(chan mailJob, mailQueueDepth), done: make(chan struct{})} |
| + go q.run() |
| + return q |
| +} |
| + |
| +// Send queues one link. It never blocks and never fails: the request |
| +// that called it answers the same way whatever happens next. |
| +func (q *mailQueue) Send(_ context.Context, to, subject, body string) error { |
| + q.mu.Lock() |
| + defer q.mu.Unlock() |
| + if q.closed { |
| + q.logger.Error("slides: sign-in mail after shutdown; dropped") |
| + return nil |
| + } |
| + q.pending.Add(1) |
| + select { |
| + case q.jobs <- mailJob{to: to, subject: subject, body: body}: |
| + default: |
| + q.pending.Done() |
| + q.logger.Error("slides: sign-in mail queue full; link dropped") |
| + } |
| + return nil |
| +} |
| + |
| +func (q *mailQueue) run() { |
| + defer close(q.done) |
| + for job := range q.jobs { |
| + q.deliver(job) |
| + q.pending.Done() |
| + } |
| +} |
| + |
| +func (q *mailQueue) deliver(job mailJob) { |
| + if !q.admits(job.to) { |
| + // Not on the allowlist: the link rauth stored is never |
| + // delivered, so it is never redeemed. No address in the log. |
| + return |
| + } |
| + ctx, cancel := context.WithTimeout(context.Background(), mailSendTimeout) |
| + defer cancel() |
| + if err := q.next.Send(ctx, job.to, job.subject, job.body); err != nil { |
| + q.logger.Error("slides: send sign-in link", "err", err) |
| + } |
| +} |
| + |
| +// wait blocks until every link queued so far has been delivered or |
| +// dropped. Tests use it (App.FlushMail) to read the mailer afterwards. |
| +func (q *mailQueue) wait() { q.pending.Wait() } |
| + |
| +// close stops taking links and drains what is queued. |
| +func (q *mailQueue) close() { |
| + q.mu.Lock() |
| + if q.closed { |
| + q.mu.Unlock() |
| + return |
| + } |
| + q.closed = true |
| + close(q.jobs) |
| + q.mu.Unlock() |
| + select { |
| + case <-q.done: |
| + case <-time.After(mailDrainTimeout): |
| + q.logger.Error("slides: sign-in mail still sending at shutdown; abandoned") |
| + } |
| +} |
diff --git a/internal/slidestest/browser_test.go b/internal/slidestest/browser_test.go| index cec28e9..f5dad94 100644 |
| --- a/internal/slidestest/browser_test.go |
| +++ b/internal/slidestest/browser_test.go |
| @@ -82,6 +82,7 @@ func newRigWith(t *testing.T, store blobs.Store) (*harness.Rig, *fakeMailer) { |
| t.Fatalf("slides.New: %v", err) |
| } |
| t.Cleanup(func() { a.Close() }) |
| + mailer.flush = a.FlushMail |
| return localised(t, mux) |
| }) |
| return rig, mailer |
diff --git a/internal/slidestest/capabilities_test.go b/internal/slidestest/capabilities_test.go| index 76dd319..840b817 100644 |
| --- a/internal/slidestest/capabilities_test.go |
| +++ b/internal/slidestest/capabilities_test.go |
| @@ -395,5 +395,6 @@ func newAppOver(t *testing.T, d *db.DB, cfg slides.Config, mailer *fakeMailer) * |
| t.Fatalf("slides.New: %v", err) |
| } |
| t.Cleanup(func() { a.Close() }) |
| + mailer.flush = a.FlushMail |
| return &testApp{Handler: localised(t, mux), mailer: mailer, db: d, app: a} |
| } |
diff --git a/internal/slidestest/harness_test.go b/internal/slidestest/harness_test.go| index 01841e4..dde46af 100644 |
| --- a/internal/slidestest/harness_test.go |
| +++ b/internal/slidestest/harness_test.go |
| @@ -55,6 +55,9 @@ type sentMail struct{ To, Subject, Body string } |
| type fakeMailer struct { |
| mu sync.Mutex |
| sent []sentMail |
| + // flush waits for the app's mail queue: links are sent after the |
| + // response (mailqueue.go), so messages reads only after it. |
| + flush func() |
| } |
| func (m *fakeMailer) Send(_ context.Context, to, subject, body string) error { |
| @@ -66,6 +69,9 @@ func (m *fakeMailer) Send(_ context.Context, to, subject, body string) error { |
| // messages returns a copy of everything sent so far. |
| func (m *fakeMailer) messages() []sentMail { |
| + if m.flush != nil { |
| + m.flush() |
| + } |
| m.mu.Lock() |
| defer m.mu.Unlock() |
| return append([]sentMail(nil), m.sent...) |
| @@ -94,6 +100,7 @@ type testApp struct { |
| Close() error |
| SetExportCaps(perDeck, perInstance int64) |
| ForgetExportJobs() error |
| + FlushMail() |
| } |
| } |
| @@ -133,6 +140,9 @@ func newAppWith(t *testing.T, cfg slides.Config, mailer *fakeMailer) *testApp { |
| // Releases the compiled deck core's wazero runtime; every test app |
| // built over slides.New leaks one without this (task 3 fix round 1). |
| t.Cleanup(func() { a.Close() }) |
| + if mailer != nil { |
| + mailer.flush = a.FlushMail |
| + } |
| return &testApp{Handler: localised(t, mux), mailer: mailer, db: d, app: a} |
| } |
diff --git a/internal/slidestest/signin_timing_test.go b/internal/slidestest/signin_timing_test.go| new file mode 100644 |
| index 0000000..9fd99b0 |
| --- /dev/null |
| +++ b/internal/slidestest/signin_timing_test.go |
| @@ -0,0 +1,83 @@ |
| +package slidestest |
| + |
| +import ( |
| + "context" |
| + "net/http/httptest" |
| + "net/url" |
| + "sync" |
| + "testing" |
| + "time" |
| +) |
| + |
| +// blockingMailer holds every Send until release is closed — an SMTP |
| +// relay that takes as long as the test likes. |
| +type blockingMailer struct { |
| + release chan struct{} |
| + mu sync.Mutex |
| + to []string |
| +} |
| + |
| +func (m *blockingMailer) Send(_ context.Context, to, _, _ string) error { |
| + <-m.release |
| + m.mu.Lock() |
| + m.to = append(m.to, to) |
| + m.mu.Unlock() |
| + return nil |
| +} |
| + |
| +func (m *blockingMailer) sent() []string { |
| + m.mu.Lock() |
| + defer m.mu.Unlock() |
| + return append([]string(nil), m.to...) |
| +} |
| + |
| +// TestSigninNeverWaitsForTheMailer is the timing half of the no-oracle |
| +// guarantee (Astra on #20, P1). When the send ran inside the request, a |
| +// permitted address waited for SMTP before its ?sent=1 and a denied one |
| +// did not — 252 ms against 0.35 ms, the allowlist readable off a |
| +// stopwatch, and force=1 made Keymail addresses readable the same way. |
| +// |
| +// Here the mailer never answers until the test says so. Every POST — |
| +// permitted, denied, and both again with force=1 — must come back while |
| +// it is still blocked, and the permitted links must then go out. |
| +func TestSigninNeverWaitsForTheMailer(t *testing.T) { |
| + bm := &blockingMailer{release: make(chan struct{})} |
| + cfg, _ := newConfig(t) |
| + cfg.Mailer = bm |
| + app := newAppWith(t, cfg, nil) |
| + var once sync.Once |
| + free := func() { once.Do(func() { close(bm.release) }) } |
| + t.Cleanup(free) // before the app's own Close, which drains the queue |
| + |
| + posts := []url.Values{ |
| + {"address": {allowedAddress}}, |
| + {"address": {deniedAddress}}, |
| + {"address": {allowedAddress}, "force": {"1"}}, |
| + {"address": {deniedAddress}, "force": {"1"}}, |
| + } |
| + for _, form := range posts { |
| + done := make(chan *httptest.ResponseRecorder, 1) |
| + go func() { done <- post(t, app, "/signin", form) }() |
| + select { |
| + case rec := <-done: |
| + if rec.Code != 303 || rec.Header().Get("Location") != "/signin?sent=1" { |
| + t.Errorf("POST %v: %d %q, want 303 /signin?sent=1", form, rec.Code, rec.Header().Get("Location")) |
| + } |
| + case <-time.After(2 * time.Second): |
| + t.Errorf("POST %v waited on the mailer: the response time says the address is permitted", form) |
| + free() |
| + <-done |
| + } |
| + } |
| + |
| + free() |
| + deadline := time.Now().Add(5 * time.Second) |
| + for len(bm.sent()) < 2 && time.Now().Before(deadline) { |
| + time.Sleep(10 * time.Millisecond) |
| + } |
| + time.Sleep(50 * time.Millisecond) // and nothing more arrives |
| + got := bm.sent() |
| + if len(got) != 2 || got[0] != allowedAddress || got[1] != allowedAddress { |
| + t.Fatalf("mail sent = %v, want two links, both to %s", got, allowedAddress) |
| + } |
| +} |