amadan
Keymail sign-in in Calls, without a membership oracle in the answer or its timing
gateBegin set force=1 on every member's POST /signin, so a Keymail address never reached Keymail (Paul, 2026-10-06: "I do want it to use keymail"); a non-member got ?sent=1 instead. - Every well-formed address reaches Begin, and the ceremony depends on the domain alone. A Keymail address, member or not, gets the same continuation page (connect.OAuthContinuation, both modes; Safari applies form-action to a 303 out of the form POST). - Any other address gets ?sent=1. The link is queued and sent after the response by signin_mail.go's worker, only if mayRequestLink allowed it for this request (ruling 12's budgets, carried on the context) and authorize admits the address — so neither the answer nor its timing says who is a member (Astra's finding on oficina/slides #20). The queue does no I/O on the request path, survives a panicking mailer, and stops when the app's context ends, within a deadline. - Authorize's bare-text 403 becomes the door with ?err=refused (doorOnRefusal). The form carries force on ?force=1&err=keymail, so a failed Keymail sign-in falls back to a link. Tests: keymailtest stubs classification and Keymail in-process (TestMain in both packages, so callstest no longer asks the real network about example.com). TestKeymailAddressGoesToKeymailWhetherPermittedOrNot, TestKeymailFailureFallsBackToAnEmailLink, TestKeymailRefusalIsTheDoor and TestSigninNeverWaitsForTheMailer fail on origin/main; the mail-queue tests fail with each defect put back. The two join tests that asserted Authorize's bare 403 now assert the door.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
17 files changed,
+1205
−39
AGENTS.md+9 −2internal/calls/app.go+15 −6internal/calls/auth.go+67 −21internal/calls/auth_refusal.go+67 −0internal/calls/home_router_test.go+9 −2internal/calls/keymail_signin_test.go+263 −0internal/calls/keymailtest/keymailtest.go+190 −0internal/calls/render.go+4 −0internal/calls/signin_mail.go+204 −0internal/calls/signin_mail_test.go+207 −0internal/calls/signin_timing_test.go+94 −0internal/calls/templates/signin.html+4 −1internal/callstest/browser_test.go+1 −0internal/callstest/harness_test.go+25 −0internal/callstest/join_test.go+6 −4internal/callstest/main_test.go+25 −0internal/callstest/signin_test.go+15 −3
CI passed — run details
CI log
=== vet === go vet ./... === fmt === === test === go test ./... ok amadan.net/oficina/calls/callscore 7.302s ? amadan.net/oficina/calls/cmd/calls [no test files] ok amadan.net/oficina/calls/cmd/quality-summary 0.006s ok amadan.net/oficina/calls/internal/calls 3.009s ok amadan.net/oficina/calls/internal/calls/apns 0.028s ok amadan.net/oficina/calls/internal/calls/hub 2.255s ? amadan.net/oficina/calls/internal/calls/icons [no test files] ? amadan.net/oficina/calls/internal/calls/keymailtest [no test files] ok amadan.net/oficina/calls/internal/calls/relay 0.012s ok amadan.net/oficina/calls/internal/calls/seal 0.005s ok amadan.net/oficina/calls/internal/callstest 10.852s ? amadan.net/oficina/calls/protocol [no test files] ? amadan.net/oficina/calls/scripts/writeestilo [no test files] === migration-check === go run amadan.net/rastrillo/rastrillo/cmd/rastrillo migration check rastrillo migration check: models and migrations agree === race === CGO_ENABLED=1 go test -race ./internal/... ok amadan.net/oficina/calls/internal/calls 12.656s ok amadan.net/oficina/calls/internal/calls/apns 1.022s ok amadan.net/oficina/calls/internal/calls/hub 2.970s ? amadan.net/oficina/calls/internal/calls/icons [no test files] ? amadan.net/oficina/calls/internal/calls/keymailtest [no test files] ok amadan.net/oficina/calls/internal/calls/relay 1.017s ok amadan.net/oficina/calls/internal/calls/seal 1.012s ok amadan.net/oficina/calls/internal/callstest 31.180s
diff --git a/AGENTS.md b/AGENTS.md| index 5420d63..3049b48 100644 |
| --- a/AGENTS.md |
| +++ b/AGENTS.md |
| @@ -32,8 +32,15 @@ mechanically. |
| - `internal/calls/store.go` — every database access. `standing.go` is |
| the entry authority (spec §2.3), computed per request, never stored. |
| - Sign-in: `auth.go` is Docs' shape with the pending-join exception of |
| - spec §3.2 and Slides' same-answer wrapper. Every answer from |
| - `POST /signin` is `303 /signin?sent=1`. |
| + spec §3.2 and Slides' same-answer wrapper. Keymail or an emailed link |
| + is chosen by the address's domain alone, never by membership (Paul, |
| + 2026-10-06). A Keymail address, member or not, gets the continuation |
| + page to Keymail (never a 303 out of the form POST: Safari applies |
| + `form-action` to it); every other answer from `POST /signin` is |
| + `303 /signin?sent=1`, and the link is mailed after the response by |
| + `signin_mail.go`'s worker, only if this request may have one, so |
| + neither the answer nor its timing says who is a member. A refusal |
| + after verification is the door with `?err=refused`. |
| ## Rules the family holds hard |
diff --git a/internal/calls/app.go b/internal/calls/app.go| index 0f1de31..80a12b8 100644 |
| --- a/internal/calls/app.go |
| +++ b/internal/calls/app.go |
| @@ -29,6 +29,8 @@ import ( |
| // app holds what every handler needs. |
| type app struct { |
| + // signinMail is the queue sign-in links go through (signin_mail.go). |
| + signinMail *mailQueue |
| db *db.DB |
| cfg resolved |
| sealer *seal.Sealer |
| @@ -430,6 +432,13 @@ func build(ctx context.Context, d *db.DB, cfg Config) (*http.ServeMux, *app, err |
| if err != nil { |
| return nil, nil, err |
| } |
| + // Sign-in links go out after the response, from a worker that asks |
| + // authorize first (signin_mail.go); it drains when ctx ends. |
| + a.signinMail = newMailQueue(a.cfg.mailer, a.authorize, mayMail, a.logger) |
| + if w, ok := cfg.Mailer.(queueAttacher); ok { |
| + w.AttachSigninQueue(a.signinMail.wait) |
| + } |
| + go func() { <-ctx.Done(); a.signinMail.close() }() |
| authPlugin, err := a.newAuthPlugin(writer) |
| if err != nil { |
| return nil, nil, err |
| @@ -484,15 +493,15 @@ func build(ctx context.Context, d *db.DB, cfg Config) (*http.ServeMux, *app, err |
| r.Post("/appearance", chrome.HandleAppearance) |
| r.Get(signinPath, a.signin) |
| if a.home == nil { |
| - r.Post(signinPath, a.gateBegin(authPlugin.Begin)) |
| - r.Get("/auth/callback", authPlugin.Callback) |
| - r.Get("/auth/verify", authPlugin.Verify) |
| + r.Post(signinPath, a.newBegin(authPlugin.Begin)) |
| + r.Get("/auth/callback", doorOnRefusal(authPlugin.Callback, signinPath)) |
| + r.Get("/auth/verify", doorOnRefusal(authPlugin.Verify, signinPath)) |
| r.Post("/signout", signedOutAfter(authPlugin.Signout)) |
| r.Get("/signed-out", a.signedOut) |
| } else { |
| - r.Post(signinPath, a.home.routes.LocalGuard(a.gateBegin(authPlugin.Begin))) |
| - r.Get("/auth/callback", a.home.routes.LocalGuard(authPlugin.Callback)) |
| - r.Get("/auth/verify", a.home.routes.LocalGuard(authPlugin.Verify)) |
| + r.Post(signinPath, a.home.routes.LocalGuard(a.newBegin(authPlugin.Begin))) |
| + r.Get("/auth/callback", a.home.routes.LocalGuard(doorOnRefusal(authPlugin.Callback, "/signin/local"))) |
| + r.Get("/auth/verify", a.home.routes.LocalGuard(doorOnRefusal(authPlugin.Verify, "/signin/local"))) |
| r.Post("/signout", a.home.routes.Signout) |
| r.Get("/signin/local", a.localSignin) |
| r.Get("/signed-out", a.home.routes.SignedOut) |
diff --git a/internal/calls/auth.go b/internal/calls/auth.go| index 8a29ba1..78bd7ac 100644 |
| --- a/internal/calls/auth.go |
| +++ b/internal/calls/auth.go |
| @@ -10,6 +10,7 @@ import ( |
| "amadan.net/oficina/estilo.git/chrome" |
| "amadan.net/oficina/estilo.git/sky" |
| + "amadan.net/oficina/home/connect" |
| rauth "amadan.net/rastrillo/rastrillo/auth" |
| "amadan.net/rastrillo/rastrillo/sessions" |
| "github.com/keymaildev/signin" |
| @@ -32,7 +33,7 @@ func (a *app) newAuthPlugin(writer *sql.DB) (*rauth.Auth, error) { |
| DB: writer, |
| Origin: a.cfg.Origin, |
| InstanceKey: a.cfg.instanceKey, |
| - Mailer: a.cfg.mailer, |
| + Mailer: a.signinMail, |
| Subject: chrome.EmailSubject("Calls"), |
| Authorize: a.authorize, |
| SigninPath: signinPath, |
| @@ -53,12 +54,32 @@ func (a *app) authorize(address string) bool { |
| return live |
| } |
| -// gateBegin sits in front of rastrillo's Begin. Calls' own refusals are |
| -// byte-identical to success; rastrillo's address and rate outcomes are |
| -// shown; membership of a specific address is not a secret Calls keeps at |
| -// this form, per spec §3.2, and the timing of a sent mail is a further |
| -// channel the response cannot hide. Task 6 adds the pending-join |
| -// exception in front of the allowlist check. |
| +// gateBegin sits in front of rastrillo's Begin, which it reaches for |
| +// EVERY well-formed address, so whether an address signs in at Keymail |
| +// or by emailed link depends on its domain alone, never on membership. |
| +// |
| +// - A Keymail address (keymail.dev itself, or a domain delegating to |
| +// Keymail with a `_keymail` TXT record) goes to Keymail, member or |
| +// not. Begin answers with a 303 to Keymail's authorize URL, which |
| +// newBegin's continuation turns into a page: Safari applies |
| +// form-action 'self' to a form's whole redirect chain. Admission is |
| +// rastrillo's Authorize after Keymail has verified the address (a |
| +// member, or a guest holding a live pending-join row), and its |
| +// refusal is the door saying so (doorOnRefusal). |
| +// - Any other address is answered ?sent=1, and a link is mailed only |
| +// when mayRequestLink allowed it for this request (ruling 12's |
| +// budgets included) and authorize admits the address. mailQueue |
| +// decides that after the response, in a worker, so not even the |
| +// response time differs (signin_mail.go). |
| +// |
| +// Calls' own refusals are byte-identical to success; rastrillo's |
| +// address and rate outcomes are shown. Membership of a specific address |
| +// is not a secret Calls keeps at this form (spec §3.2), but the choice |
| +// of ceremony must still not depend on it: it used to set force=1 for a |
| +// permitted address only, which kept Keymail out of Calls altogether |
| +// (Paul, 2026-10-06: "I do want it to use keymail"). force now arrives |
| +// only from the form rastrillo lands a failed Keymail exchange on |
| +// (?force=1&err=keymail), and asks for an emailed link instead. |
| func (a *app) gateBegin(next http.HandlerFunc) http.HandlerFunc { |
| return func(w http.ResponseWriter, r *http.Request) { |
| _ = r.ParseForm() |
| @@ -73,24 +94,37 @@ func (a *app) gateBegin(next http.HandlerFunc) http.HandlerFunc { |
| http.Redirect(w, r, a.signinAgain(r, "err=address"), http.StatusSeeOther) |
| return |
| } |
| - if !a.mayRequestLink(w, r, address) { |
| - http.Redirect(w, r, sentLocation, http.StatusSeeOther) |
| - return |
| - } |
| - if r.Form == nil { |
| - r.Form = url.Values{} |
| - } |
| - // force makes rastrillo skip its address classifier, so Begin |
| - // never takes the keymail branch (a 303 to an external host |
| - // chosen by the address's DNS, which would be a distinguishable |
| - // outcome and an oracle); it also means keymail sign-in is |
| - // disabled app-wide, deliberately. |
| - r.Form.Set("force", "1") |
| + may := a.mayRequestLink(w, r, address) |
| + r = r.WithContext(context.WithValue(r.Context(), mayMailKey{}, may)) |
| next(&sameAnswer{ResponseWriter: w, app: a}, r) |
| } |
| } |
| -// mayRequestLink decides whether Begin is reached (ruling 12). A member |
| +// newBegin is POST /signin in both modes: gateBegin under |
| +// connect.OAuthContinuation, which ends the POST with a page |
| +// (renderKeymail) when Begin's answer is Keymail's authorize URL. |
| +func (a *app) newBegin(begin http.HandlerFunc) http.HandlerFunc { |
| + return connect.OAuthContinuation(a.cfg.Origin, a.gateBegin(begin), a.renderKeymail) |
| +} |
| + |
| +// renderKeymail is the page between the form and Keymail: the door's |
| +// card saying where the person is going, with the meta refresh that |
| +// goes there (home-oauth-head) and a link for when it is blocked. |
| +func (a *app) renderKeymail(w http.ResponseWriter, r *http.Request, destination string) { |
| + p := connect.OAuthPage(destination) |
| + notice := chrome.SignInNotice{ |
| + Icon: assets.Path("static/brand/icon.svg"), App: appName, Sky: sky.For(r.Host), |
| + Heading: p.Title, Body: "Taking you to Keymail to finish signing in.", |
| + Link: &chrome.Link{Href: p.URL, Label: p.ActionLabel}, |
| + } |
| + render(w, "signin", a.page(r, frame{parts: []string{p.Title}, signinPage: true}, map[string]any{"Sent": true, "Notice": notice, "Continue": p})) |
| +} |
| + |
| +// mayMailKey carries gateBegin's mayRequestLink answer to mailQueue, |
| +// through Begin, on the request context. |
| +type mayMailKey struct{} |
| + |
| +// mayRequestLink decides whether a link may be mailed (ruling 12). A member |
| // address always may. Any other address may only when the browser |
| // carries a valid pending-join cookie for a live link, within two |
| // budgets counted before Begin: five attempts per link and ten across |
| @@ -264,6 +298,12 @@ func (a *app) localSignin(w http.ResponseWriter, r *http.Request) { |
| if hasReturn { |
| door.Return = ret |
| } |
| + if q.Get("force") != "" { |
| + // A failed Keymail exchange lands here as ?force=1&err=keymail. |
| + // Posting force back asks for an emailed link instead, so the |
| + // retry does not return to the Keymail that just failed. |
| + door.Hidden = []chrome.Hidden{{Name: "force", Value: "1"}} |
| + } |
| switch q.Get("err") { |
| case "address": |
| // An address error belongs to the field, not a banner. |
| @@ -274,6 +314,12 @@ func (a *app) localSignin(w http.ResponseWriter, r *http.Request) { |
| door.Error = "That link has expired or was already used. Enter your email to get a new one." |
| case "removed": |
| door.Error = "Your access has been removed, so you’ve been signed out. Ask whoever runs this team if that’s wrong." |
| + case "keymail": |
| + door.Error = "Your Keymail sign-in couldn’t be completed. Enter your email to get a link instead." |
| + case "refused": |
| + // Keymail or a link verified the address and Calls does not |
| + // admit it (doorOnRefusal). The form is the way on. |
| + door.Error = "That address can’t sign in to Calls. Use a different address, or ask whoever runs this team to add it." |
| case "": |
| default: |
| door.Error = "We couldn’t sign you in. Enter your email to get a new link." |
diff --git a/internal/calls/auth_refusal.go b/internal/calls/auth_refusal.go| new file mode 100644 |
| index 0000000..88e3ff6 |
| --- /dev/null |
| +++ b/internal/calls/auth_refusal.go |
| @@ -0,0 +1,67 @@ |
| +package calls |
| + |
| +import ( |
| + "bytes" |
| + "net/http" |
| + "strings" |
| +) |
| + |
| +// authRefusal is the bare-text body rastrillo/auth's admit() writes, |
| +// with a 403, when a verified address is not admitted. rastrillo at the |
| +// version pinned here has no hook to render anything else, so |
| +// doorOnRefusal recognises this exact answer; TestKeymailRefusalIsTheDoor |
| +// pins it, so a bump that changes the sentence fails loudly. |
| +const authRefusal = "This address is verified but not admitted here." |
| + |
| +// doorOnRefusal wraps rastrillo's Callback and Verify, the handlers that |
| +// end in admission, and turns its bare-text refusal into the door: a 303 |
| +// to the local form with ?err=refused, which says the address cannot |
| +// sign in here and offers the form for a different one. Every other |
| +// answer, cookies included, passes through untouched. Remove it when |
| +// rastrillo has a refusal hook and the pin is bumped. |
| +func doorOnRefusal(next http.HandlerFunc, form string) http.HandlerFunc { |
| + return func(w http.ResponseWriter, r *http.Request) { |
| + rec := &bufferedResponse{header: make(http.Header)} |
| + next(rec, r) |
| + if rec.status == http.StatusForbidden && strings.TrimSpace(rec.body.String()) == authRefusal { |
| + for _, c := range rec.header.Values("Set-Cookie") { |
| + w.Header().Add("Set-Cookie", c) |
| + } |
| + w.Header().Set("Cache-Control", "no-store") |
| + http.Redirect(w, r, form+"?err=refused", http.StatusSeeOther) |
| + return |
| + } |
| + for name, values := range rec.header { |
| + for _, v := range values { |
| + w.Header().Add(name, v) |
| + } |
| + } |
| + if rec.status == 0 { |
| + rec.status = http.StatusOK |
| + } |
| + w.WriteHeader(rec.status) |
| + _, _ = w.Write(rec.body.Bytes()) |
| + } |
| +} |
| + |
| +// bufferedResponse holds a whole answer so doorOnRefusal can decide what |
| +// to send after seeing it. Callback and Verify answer with a redirect |
| +// or a line of text, never anything large. |
| +type bufferedResponse struct { |
| + header http.Header |
| + status int |
| + body bytes.Buffer |
| +} |
| + |
| +func (b *bufferedResponse) Header() http.Header { return b.header } |
| +func (b *bufferedResponse) WriteHeader(status int) { |
| + if b.status == 0 { |
| + b.status = status |
| + } |
| +} |
| +func (b *bufferedResponse) Write(p []byte) (int, error) { |
| + if b.status == 0 { |
| + b.status = http.StatusOK |
| + } |
| + return b.body.Write(p) |
| +} |
diff --git a/internal/calls/home_router_test.go b/internal/calls/home_router_test.go| index c1eb08f..47e976e 100644 |
| --- a/internal/calls/home_router_test.go |
| +++ b/internal/calls/home_router_test.go |
| @@ -73,6 +73,13 @@ func newHomeRouterFixtureWith(t *testing.T, d *db.DB, logger *slog.Logger, key * |
| return &homeRouterFixture{a, mux, key, mail, map[string]*http.Cookie{}} |
| } |
| +// mailBody is the last sign-in mail, once the queue has sent it |
| +// (links go out after the response: signin_mail.go). |
| +func (f *homeRouterFixture) mailBody() string { |
| + f.app.signinMail.wait() |
| + return f.mail.body |
| +} |
| + |
| func homeKeyID(key *assertioncrypto.Keypair) string { |
| sum := sha256.Sum256(key.SignPub()) |
| return hex.EncodeToString(sum[:]) |
| @@ -111,7 +118,7 @@ func homeRedirect(t *testing.T, w *httptest.ResponseRecorder) string { |
| func (f *homeRouterFixture) local(t *testing.T) { |
| t.Helper() |
| homeRedirect(t, f.do("POST", "/signin", "address=alice%40example.test&force=1")) |
| - link := regexp.MustCompile(`https://[^\s<>\"]+/auth/verify\?[^\s<>\"]+`).FindString(f.mail.body) |
| + link := regexp.MustCompile(`https://[^\s<>\"]+/auth/verify\?[^\s<>\"]+`).FindString(f.mailBody()) |
| u, err := url.Parse(link) |
| if err != nil || link == "" { |
| t.Fatal("no captured local verification link") |
| @@ -240,7 +247,7 @@ func TestHomeEnabledPreservesInvitedGuestFlow(t *testing.T) { |
| t.Fatal("guest redirected to Home", w.Code) |
| } |
| homeRedirect(t, f.do("POST", "/signin", "address=guest%40example.test")) |
| - link := regexp.MustCompile(`https://[^\s<>\"]+/auth/verify\?[^\s<>\"]+`).FindString(f.mail.body) |
| + link := regexp.MustCompile(`https://[^\s<>\"]+/auth/verify\?[^\s<>\"]+`).FindString(f.mailBody()) |
| u, _ := url.Parse(link) |
| landing := homeRedirect(t, f.do("GET", u.RequestURI(), "")) |
| if landing != "/auth/local/complete" { |
diff --git a/internal/calls/keymail_signin_test.go b/internal/calls/keymail_signin_test.go| new file mode 100644 |
| index 0000000..72da2bf |
| --- /dev/null |
| +++ b/internal/calls/keymail_signin_test.go |
| @@ -0,0 +1,263 @@ |
| +package calls |
| + |
| +import ( |
| + "bytes" |
| + "context" |
| + "encoding/base64" |
| + "fmt" |
| + "html" |
| + "io" |
| + "log/slog" |
| + "net/http" |
| + "net/http/httptest" |
| + "net/url" |
| + "os" |
| + "path/filepath" |
| + "regexp" |
| + "strings" |
| + "sync" |
| + "testing" |
| + |
| + "amadan.net/oficina/calls/internal/calls/keymailtest" |
| + "amadan.net/oficina/home/connect" |
| + assertioncrypto "amadan.net/rastrillo/rastrillo/crypto" |
| + "amadan.net/rastrillo/rastrillo/db" |
| +) |
| + |
| +// TestMain keeps Keymail classification inside the process: every DNS |
| +// query is answered NXDOMAIN and only the Keymail fixtures a test |
| +// registers answer HTTPS (keymailtest). |
| +func TestMain(m *testing.M) { |
| + restore, err := keymailtest.Install() |
| + if err != nil { |
| + fmt.Fprintln(os.Stderr, "keymailtest.Install:", err) |
| + os.Exit(1) |
| + } |
| + code := m.Run() |
| + restore() |
| + os.Exit(code) |
| +} |
| + |
| +const ( |
| + keymailHost = "keymail.calls.test" |
| + keymailMember = "paul@" + keymailHost |
| + keymailStranger = "stranger@" + keymailHost |
| + plainMember = "paul@example.test" |
| + keymailOrigin = "https://calls.example.test" |
| +) |
| + |
| +type countingMailer struct { |
| + mu sync.Mutex |
| + to []string |
| + waits []func() // the sign-in mail queues in front of it |
| +} |
| + |
| +func (m *countingMailer) AttachSigninQueue(wait func()) { |
| + m.mu.Lock() |
| + m.waits = append(m.waits, wait) |
| + m.mu.Unlock() |
| +} |
| + |
| +func (m *countingMailer) Send(_ context.Context, to, _, _ string) error { |
| + m.mu.Lock() |
| + defer m.mu.Unlock() |
| + m.to = append(m.to, to) |
| + return nil |
| +} |
| + |
| +func (m *countingMailer) sent() []string { |
| + m.mu.Lock() |
| + waits := append([]func(){}, m.waits...) |
| + m.mu.Unlock() |
| + for _, w := range waits { |
| + w() |
| + } |
| + m.mu.Lock() |
| + defer m.mu.Unlock() |
| + return append([]string(nil), m.to...) |
| +} |
| + |
| +type keymailFixture struct { |
| + handler http.Handler |
| + mail *countingMailer |
| + km *keymailtest.Server |
| +} |
| + |
| +// newKeymailFixture builds Calls with plainMember and keymailMember on |
| +// the allowlist, beside a Keymail deployment holding keymailMember and |
| +// keymailStranger; home configures Home sign-in. |
| +func newKeymailFixture(t *testing.T, home bool) *keymailFixture { |
| + t.Helper() |
| + km := keymailtest.Serve(t, keymailHost, keymailMember, keymailStranger) |
| + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) |
| + d, err := db.Open(filepath.Join(t.TempDir(), "calls.db"), logger) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + t.Cleanup(func() { d.Close() }) |
| + mail := &countingMailer{} |
| + cfg := Config{Origin: keymailOrigin, InstanceKey: "test-only-instance", DEK: base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{3}, 32)), |
| + AllowedAddresses: []string{plainMember, keymailMember}, Mailer: mail, Logger: logger} |
| + if home { |
| + key, err := assertioncrypto.Generate() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + cfg.Home = &connect.Credentials{Issuer: homeTestIssuer, ConsumerID: "calls-test", KeysBase64: homeKeysBase64(key), DEKBase64: base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{7}, 32))} |
| + } |
| + mux, _, err := build(t.Context(), d, cfg) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + return &keymailFixture{handler: mux, mail: mail, km: km} |
| +} |
| + |
| +func (f *keymailFixture) post(form url.Values, cookies ...*http.Cookie) *httptest.ResponseRecorder { |
| + r := httptest.NewRequest(http.MethodPost, keymailOrigin+"/signin", strings.NewReader(form.Encode())) |
| + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + r.Header.Set("Origin", keymailOrigin) |
| + r.Header.Set("Sec-Fetch-Site", "same-origin") |
| + for _, c := range cookies { |
| + r.AddCookie(c) |
| + } |
| + w := httptest.NewRecorder() |
| + f.handler.ServeHTTP(w, r) |
| + return w |
| +} |
| + |
| +func (f *keymailFixture) get(path string, cookies ...*http.Cookie) *httptest.ResponseRecorder { |
| + r := httptest.NewRequest(http.MethodGet, keymailOrigin+path, nil) |
| + for _, c := range cookies { |
| + r.AddCookie(c) |
| + } |
| + w := httptest.NewRecorder() |
| + f.handler.ServeHTTP(w, r) |
| + return w |
| +} |
| + |
| +var authorizeHref = regexp.MustCompile(`href="(https://` + regexp.QuoteMeta(keymailHost) + `/oauth/authorize\?[^"]+)"`) |
| + |
| +// continuation asserts rec is the page between the form and Keymail — |
| +// a 200 document with a meta refresh, never a 303 out of the form POST |
| +// (Safari applies form-action to that redirect) — and returns the |
| +// authorize URL it carries. |
| +func continuation(t *testing.T, label string, rec *httptest.ResponseRecorder) *url.URL { |
| + t.Helper() |
| + if rec.Code != http.StatusOK || rec.Header().Get("Location") != "" { |
| + t.Fatalf("%s: %d %q, want 200 and the continuation page to Keymail", label, rec.Code, rec.Header().Get("Location")) |
| + } |
| + body := rec.Body.String() |
| + m := authorizeHref.FindStringSubmatch(body) |
| + if m == nil || !strings.Contains(body, `http-equiv="refresh"`) { |
| + t.Fatalf("%s: no Keymail continuation on the page:\n%s", label, body) |
| + } |
| + u, err := url.Parse(html.UnescapeString(m[1])) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + return u |
| +} |
| + |
| +// TestKeymailAddressGoesToKeymailWhetherPermittedOrNot: in both modes, |
| +// whether an address goes to Keymail depends on its domain alone. A |
| +// member and a stranger at a Keymail domain get the same first answer, |
| +// the continuation page, and neither is mailed. Before, gateBegin set |
| +// force=1 on every member's POST, so Keymail was never used. |
| +func TestKeymailAddressGoesToKeymailWhetherPermittedOrNot(t *testing.T) { |
| + for _, home := range []bool{false, true} { |
| + t.Run(fmt.Sprintf("home=%v", home), func(t *testing.T) { |
| + f := newKeymailFixture(t, home) |
| + member := f.post(url.Values{"address": {keymailMember}}) |
| + stranger := f.post(url.Values{"address": {keymailStranger}}) |
| + mu := continuation(t, "member Keymail address", member) |
| + su := continuation(t, "stranger Keymail address", stranger) |
| + mask := func(rec *httptest.ResponseRecorder, u *url.URL) string { |
| + return strings.ReplaceAll(rec.Body.String(), html.EscapeString(u.String()), "AUTHORIZE") |
| + } |
| + if a, b := mask(member, mu), mask(stranger, su); a != b { |
| + t.Errorf("the two Keymail answers differ beyond the authorize URL:\n%s\n----\n%s", a, b) |
| + } |
| + if got := f.mail.sent(); len(got) != 0 { |
| + t.Errorf("a Keymail address was mailed: %v", got) |
| + } |
| + if n := f.km.Lookups(); n != 2 { |
| + t.Errorf("Keymail saw %d lookups, want 2", n) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// TestNonPermittedPlainAddressIsAnsweredSentAndNotMailed: an address |
| +// with no Keymail gets ?sent=1, member or not; only the member is |
| +// mailed. |
| +func TestNonPermittedPlainAddressIsAnsweredSentAndNotMailed(t *testing.T) { |
| + f := newKeymailFixture(t, false) |
| + for _, address := range []string{"someone@else.test", plainMember} { |
| + rec := f.post(url.Values{"address": {address}}) |
| + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/signin?sent=1" { |
| + t.Fatalf("POST /signin %q: %d %q, want 303 /signin?sent=1", address, rec.Code, rec.Header().Get("Location")) |
| + } |
| + } |
| + if got := f.mail.sent(); len(got) != 1 || got[0] != plainMember { |
| + t.Fatalf("mail = %v, want one, to %s", got, plainMember) |
| + } |
| +} |
| + |
| +// TestKeymailFailureFallsBackToAnEmailLink: the page rastrillo lands a |
| +// failed Keymail exchange on (?force=1&err=keymail) carries force, so |
| +// the retry mails a link instead of returning to Keymail; the ordinary |
| +// form carries none. |
| +func TestKeymailFailureFallsBackToAnEmailLink(t *testing.T) { |
| + f := newKeymailFixture(t, false) |
| + forced := `name="force" value="1"` |
| + if strings.Contains(f.get("/signin").Body.String(), forced) { |
| + t.Fatal("the ordinary form forces an email link") |
| + } |
| + if !strings.Contains(f.get("/signin?force=1&err=keymail").Body.String(), forced) { |
| + t.Fatal("the err=keymail form does not carry force; a failed Keymail sign-in would loop back to Keymail") |
| + } |
| + rec := f.post(url.Values{"address": {keymailMember}, "force": {"1"}}) |
| + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/signin?sent=1" { |
| + t.Fatalf("forced POST: %d %q", rec.Code, rec.Header().Get("Location")) |
| + } |
| + if got := f.mail.sent(); len(got) != 1 || got[0] != keymailMember { |
| + t.Fatalf("mail = %v, want one link to %s", got, keymailMember) |
| + } |
| + if f.km.Lookups() != 0 { |
| + t.Error("a forced sign-in classified the address") |
| + } |
| +} |
| + |
| +// TestKeymailRefusalIsTheDoor: a stranger Keymail verified is refused |
| +// after verification on the door, with the form for a different |
| +// address, not in rastrillo's bare text; a member is signed in. Both |
| +// modes. |
| +func TestKeymailRefusalIsTheDoor(t *testing.T) { |
| + for _, home := range []bool{false, true} { |
| + t.Run(fmt.Sprintf("home=%v", home), func(t *testing.T) { |
| + f := newKeymailFixture(t, home) |
| + form := "/signin" |
| + if home { |
| + form = "/signin/local" |
| + } |
| + roundTrip := func(address string) *httptest.ResponseRecorder { |
| + begin := f.post(url.Values{"address": {address}}) |
| + u := continuation(t, address, begin) |
| + return f.get("/auth/callback?"+url.Values{"state": {u.Query().Get("state")}, "code": {f.km.Approve(address)}}.Encode(), begin.Result().Cookies()...) |
| + } |
| + if cb := roundTrip(keymailMember); cb.Code != http.StatusSeeOther || strings.Contains(cb.Header().Get("Location"), "err=") { |
| + t.Fatalf("member callback: %d %q", cb.Code, cb.Header().Get("Location")) |
| + } |
| + cb := roundTrip(keymailStranger) |
| + if cb.Code != http.StatusSeeOther || cb.Header().Get("Location") != form+"?err=refused" { |
| + t.Fatalf("stranger callback: %d %q, want 303 %s?err=refused (bare text is the bug)\n%s", cb.Code, cb.Header().Get("Location"), form, cb.Body.String()) |
| + } |
| + door := f.get(form + "?err=refused").Body.String() |
| + for _, want := range []string{"estilo-door", `name="address"`, "can’t sign in to Calls"} { |
| + if !strings.Contains(door, want) { |
| + t.Errorf("refusal door lacks %q:\n%s", want, door) |
| + } |
| + } |
| + }) |
| + } |
| +} |
diff --git a/internal/calls/keymailtest/keymailtest.go b/internal/calls/keymailtest/keymailtest.go| new file mode 100644 |
| index 0000000..92ad056 |
| --- /dev/null |
| +++ b/internal/calls/keymailtest/keymailtest.go |
| @@ -0,0 +1,190 @@ |
| +// Package keymailtest keeps sign-in's Keymail classification inside the |
| +// test process. It is imported by tests only. |
| +// |
| +// rastrillo/auth (at the version this app pins) builds its classifier |
| +// with no seams: TXT lookups go through net.DefaultResolver and the |
| +// well-known probe, the federation lookup and the token exchange go |
| +// through http.DefaultTransport. So those two globals are the hooks, |
| +// exactly as oficina/docs' auth_keymail_test.go uses them. Install |
| +// replaces both for the life of the test binary: |
| +// |
| +// - every DNS query is answered NXDOMAIN by a resolver on loopback, |
| +// so no DNS packet leaves the process, and a domain with no |
| +// `_keymail` record is probed directly — keymail.dev's own shape; |
| +// - an https request to a host registered with Serve is answered |
| +// in-process by that fixture; anything else falls through to the |
| +// real transport, where the resolver above makes a name |
| +// unreachable (IP literals, which httptest servers use, still work). |
| +// |
| +// Every address at a domain nobody registered therefore classifies as |
| +// "not Keymail", which is what every pre-existing sign-in test assumes. |
| +package keymailtest |
| + |
| +import ( |
| + "context" |
| + "crypto/rand" |
| + "encoding/hex" |
| + "encoding/json" |
| + "net" |
| + "net/http" |
| + "net/http/httptest" |
| + "strings" |
| + "sync" |
| + "time" |
| +) |
| + |
| +var ( |
| + mu sync.Mutex |
| + servers = map[string]*Server{} |
| +) |
| + |
| +// Install swaps net.DefaultResolver and http.DefaultTransport for the |
| +// stubs described in the package comment and returns a function that |
| +// puts them back. Call it once, from TestMain. |
| +func Install() (restore func(), err error) { |
| + dns, err := net.ListenPacket("udp", "127.0.0.1:0") |
| + if err != nil { |
| + return nil, err |
| + } |
| + go answerNXDOMAIN(dns) |
| + oldResolver, oldTransport := net.DefaultResolver, http.DefaultTransport |
| + net.DefaultResolver = &net.Resolver{PreferGo: true, Dial: func(ctx context.Context, _, _ string) (net.Conn, error) { |
| + return (&net.Dialer{}).DialContext(ctx, "udp", dns.LocalAddr().String()) |
| + }} |
| + http.DefaultTransport = transport{next: oldTransport} |
| + return func() { |
| + net.DefaultResolver, http.DefaultTransport = oldResolver, oldTransport |
| + _ = dns.Close() |
| + }, nil |
| +} |
| + |
| +// answerNXDOMAIN echoes each query back as a "no such name" answer. |
| +func answerNXDOMAIN(conn net.PacketConn) { |
| + b := make([]byte, 4096) |
| + for { |
| + n, addr, err := conn.ReadFrom(b) |
| + if err != nil { |
| + return |
| + } |
| + if n < 12 { |
| + continue |
| + } |
| + reply := append([]byte(nil), b[:n]...) |
| + reply[2], reply[3] = 0x81, 0x83 // response, recursion, NXDOMAIN |
| + for i := 6; i < 12; i++ { |
| + reply[i] = 0 // no answer, authority or additional records |
| + } |
| + _, _ = conn.WriteTo(reply, addr) |
| + } |
| +} |
| + |
| +type transport struct{ next http.RoundTripper } |
| + |
| +func (t transport) RoundTrip(r *http.Request) (*http.Response, error) { |
| + mu.Lock() |
| + s := servers[r.URL.Host] |
| + mu.Unlock() |
| + if s == nil || r.URL.Scheme != "https" { |
| + return t.next.RoundTrip(r) |
| + } |
| + rec := httptest.NewRecorder() |
| + s.ServeHTTP(rec, r) |
| + res := rec.Result() |
| + res.Request = r |
| + return res, nil |
| +} |
| + |
| +// Server is one Keymail deployment at one host: the well-known |
| +// document, the federation lookup for the accounts it holds, and the |
| +// token exchange for the codes Approve hands out. |
| +type Server struct { |
| + host string |
| + |
| + mu sync.Mutex |
| + accounts map[string]bool |
| + codes map[string]string |
| + lookups int |
| + tokens int |
| +} |
| + |
| +// Serve registers a Keymail deployment at host (which is also the |
| +// address domain it answers for, since nothing publishes a `_keymail` |
| +// record) holding the given accounts. It is unregistered when the test |
| +// ends. |
| +func Serve(t interface{ Cleanup(func()) }, host string, accounts ...string) *Server { |
| + s := &Server{host: host, accounts: map[string]bool{}, codes: map[string]string{}} |
| + for _, a := range accounts { |
| + s.accounts[strings.ToLower(a)] = true |
| + } |
| + mu.Lock() |
| + servers[host] = s |
| + mu.Unlock() |
| + t.Cleanup(func() { |
| + mu.Lock() |
| + delete(servers, host) |
| + mu.Unlock() |
| + }) |
| + return s |
| +} |
| + |
| +// Approve is Keymail's consent screen saying yes for address: it |
| +// returns the code the browser would carry back to /auth/callback. |
| +func (s *Server) Approve(address string) string { |
| + var b [12]byte |
| + _, _ = rand.Read(b[:]) |
| + code := hex.EncodeToString(b[:]) |
| + s.mu.Lock() |
| + s.codes[code] = address |
| + s.mu.Unlock() |
| + return code |
| +} |
| + |
| +// Lookups is how many federation lookups reached this deployment. |
| +func (s *Server) Lookups() int { |
| + s.mu.Lock() |
| + defer s.mu.Unlock() |
| + return s.lookups |
| +} |
| + |
| +// Tokens is how many token exchanges reached this deployment. |
| +func (s *Server) Tokens() int { |
| + s.mu.Lock() |
| + defer s.mu.Unlock() |
| + return s.tokens |
| +} |
| + |
| +func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { |
| + w.Header().Set("Content-Type", "application/json") |
| + switch r.URL.Path { |
| + case "/.well-known/keymail": |
| + _ = json.NewEncoder(w).Encode(map[string]any{"version": "1", "reauth": true}) |
| + case "/api/federation/lookup": |
| + s.mu.Lock() |
| + s.lookups++ |
| + ok := s.accounts[strings.ToLower(r.URL.Query().Get("addr"))] |
| + s.mu.Unlock() |
| + if !ok { |
| + w.WriteHeader(http.StatusNotFound) |
| + } |
| + _ = json.NewEncoder(w).Encode(map[string]bool{"ok": ok}) |
| + case "/api/oauth/token": |
| + var in map[string]string |
| + _ = json.NewDecoder(r.Body).Decode(&in) |
| + s.mu.Lock() |
| + s.tokens++ |
| + address, ok := s.codes[in["code"]] |
| + delete(s.codes, in["code"]) |
| + s.mu.Unlock() |
| + if r.Method != http.MethodPost || !ok { |
| + w.WriteHeader(http.StatusBadRequest) |
| + _ = json.NewEncoder(w).Encode(map[string]string{"error": "invalid_grant"}) |
| + return |
| + } |
| + _ = json.NewEncoder(w).Encode(map[string]any{ |
| + "access_token": "synthetic", "token_type": "bearer", |
| + "address": address, "auth_time": time.Now().Unix(), |
| + }) |
| + default: |
| + w.WriteHeader(http.StatusNotFound) |
| + } |
| +} |
diff --git a/internal/calls/render.go b/internal/calls/render.go| index 2c625a2..9c294b1 100644 |
| --- a/internal/calls/render.go |
| +++ b/internal/calls/render.go |
| @@ -67,6 +67,10 @@ var pages = map[string]*template.Template{} |
| func init() { |
| for _, name := range pageNames { |
| pages[name] = template.Must(buildPage(name).ParseFS(credits.Templates(), "templates/credits.html")) |
| + if name == "signin" { |
| + // The Keymail continuation's meta refresh (home-oauth-head). |
| + pages[name] = template.Must(pages[name].ParseFS(connect.Templates(), "templates/oauth.html")) |
| + } |
| if name == "home" { |
| pages[name] = template.Must(pages[name].ParseFS(connect.Templates(), "templates/home_link.html")) |
| } |
diff --git a/internal/calls/signin_mail.go b/internal/calls/signin_mail.go| new file mode 100644 |
| index 0000000..717eaac |
| --- /dev/null |
| +++ b/internal/calls/signin_mail.go |
| @@ -0,0 +1,204 @@ |
| +package calls |
| + |
| +import ( |
| + "context" |
| + "fmt" |
| + "log/slog" |
| + "sync" |
| + "sync/atomic" |
| + "time" |
| + |
| + "amadan.net/rastrillo/rastrillo/mail" |
| +) |
| + |
| +// mailQueue is the sign-in mailer the auth plugin is given: Send queues |
| +// the link and returns at once, and one background worker decides |
| +// whether the address may have it and only then talks to SMTP. |
| +// |
| +// Both halves are the no-oracle guarantee, 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 oficina/slides |
| +// #20). A response time is an answer like any other. So POST /signin |
| +// does the same synchronous work for every address — the auth plugin'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 request path is Send, and Send does no I/O and never blocks: not |
| +// a send, not an admission check, not even a log line. Every address's |
| +// link is queued, permitted or not, so how full the queue is never |
| +// depends on admission either; the worker decides afterwards. A full |
| +// queue drops the link and counts it in an atomic; the worker logs the |
| +// count when it next runs. The person is answered exactly as when the |
| +// queue is empty, because a different answer for a full queue would be |
| +// one more thing to read. (Astra on #20, round 2.) |
| +// |
| +// The worker survives a panic in the mailer: it recovers per message, |
| +// logs, and keeps draining. Delivery failures are logged, never |
| +// surfaced: a failure only a permitted address can reach is the oracle |
| +// again. |
| +// |
| +// close drains what is queued within drainTimeout. Past it, the send |
| +// in flight is cancelled through its context, everything still queued |
| +// is abandoned with a log line, and close waits a short grace for the |
| +// worker to stop. A mailer that ignores its context is the only way a |
| +// worker outlives close, and that is logged too. |
| +type mailQueue struct { |
| + next mail.Sender |
| + admits func(to string) bool |
| + // gate, when set, is read off the request's context at enqueue time |
| + // and travels with the link; the worker sends only if it said yes. |
| + gate func(context.Context) bool |
| + logger *slog.Logger |
| + |
| + drainTimeout time.Duration |
| + sendTimeout time.Duration |
| + |
| + mu sync.RWMutex // held for reading by Send, for writing by close |
| + closed bool |
| + jobs chan mailJob |
| + dropped atomic.Int64 |
| + pending sync.WaitGroup |
| + |
| + ctx context.Context // cancelled when close gives up waiting |
| + cancel context.CancelFunc |
| + done chan struct{} |
| +} |
| + |
| +type mailJob struct { |
| + to, subject, body string |
| + gated bool |
| +} |
| + |
| +const ( |
| + mailQueueDepth = 64 |
| + mailSendTimeout = time.Minute |
| + mailDrainTimeout = 30 * time.Second |
| + mailAbandonGrace = 5 * time.Second |
| +) |
| + |
| +func newMailQueue(next mail.Sender, admits func(string) bool, gate func(context.Context) bool, logger *slog.Logger) *mailQueue { |
| + if logger == nil { |
| + logger = slog.Default() |
| + } |
| + ctx, cancel := context.WithCancel(context.Background()) |
| + q := &mailQueue{next: next, admits: admits, gate: gate, logger: logger, |
| + drainTimeout: mailDrainTimeout, sendTimeout: mailSendTimeout, |
| + jobs: make(chan mailJob, mailQueueDepth), ctx: ctx, cancel: cancel, |
| + done: make(chan struct{})} |
| + go q.run() |
| + return q |
| +} |
| + |
| +// Send queues one link. It never blocks, never does I/O and never |
| +// fails: the request that called it answers the same way whatever |
| +// happens next. |
| +func (q *mailQueue) Send(ctx context.Context, to, subject, body string) error { |
| + job := mailJob{to: to, subject: subject, body: body, gated: true} |
| + if q.gate != nil { |
| + job.gated = q.gate(ctx) |
| + } |
| + q.mu.RLock() |
| + defer q.mu.RUnlock() |
| + if q.closed { |
| + q.dropped.Add(1) |
| + return nil |
| + } |
| + q.pending.Add(1) |
| + select { |
| + case q.jobs <- job: |
| + default: |
| + q.pending.Done() |
| + q.dropped.Add(1) |
| + } |
| + return nil |
| +} |
| + |
| +func (q *mailQueue) run() { |
| + defer close(q.done) |
| + abandoned := 0 |
| + for job := range q.jobs { |
| + q.reportDrops() |
| + if q.ctx.Err() != nil { |
| + abandoned++ |
| + } else { |
| + q.deliver(job) |
| + } |
| + q.pending.Done() |
| + } |
| + q.reportDrops() |
| + if abandoned > 0 { |
| + q.logger.Error("calls: sign-in links abandoned at shutdown", "count", abandoned) |
| + } |
| +} |
| + |
| +// reportDrops logs the links Send had to drop since the last report. |
| +// Here, on the worker, so the request path never writes a log line. |
| +func (q *mailQueue) reportDrops() { |
| + if n := q.dropped.Swap(0); n > 0 { |
| + q.logger.Error("calls: sign-in mail queue full; links dropped", "count", n) |
| + } |
| +} |
| + |
| +// deliver sends one link if it may go. A panic in the admission check |
| +// or the mailer is this message's failure, not the process's. |
| +func (q *mailQueue) deliver(job mailJob) { |
| + defer func() { |
| + if p := recover(); p != nil { |
| + q.logger.Error("calls: sign-in mail worker recovered from a panic", "panic", fmt.Sprint(p)) |
| + } |
| + }() |
| + if !job.gated || !q.admits(job.to) { |
| + // Not admitted: the link the auth plugin stored is never |
| + // delivered, so it is never redeemed. No address in the log. |
| + return |
| + } |
| + ctx, cancel := context.WithTimeout(q.ctx, q.sendTimeout) |
| + defer cancel() |
| + if err := q.next.Send(ctx, job.to, job.subject, job.body); err != nil { |
| + q.logger.Error("calls: send sign-in link", "err", err) |
| + } |
| +} |
| + |
| +// wait blocks until every link queued so far has been sent or refused. |
| +// Tests use it to read the mailer afterwards. |
| +func (q *mailQueue) wait() { q.pending.Wait() } |
| + |
| +// close stops taking links and drains what is queued, within |
| +// drainTimeout; then it cancels, abandons the rest, and stops the |
| +// worker (see the type's comment). |
| +func (q *mailQueue) close() { |
| + q.mu.Lock() |
| + if q.closed { |
| + q.mu.Unlock() |
| + return |
| + } |
| + q.closed = true |
| + close(q.jobs) |
| + q.mu.Unlock() |
| + defer q.cancel() |
| + select { |
| + case <-q.done: |
| + return |
| + case <-time.After(q.drainTimeout): |
| + } |
| + q.cancel() |
| + select { |
| + case <-q.done: |
| + case <-time.After(mailAbandonGrace): |
| + q.logger.Error("calls: sign-in mail worker ignored cancellation; left running") |
| + } |
| +} |
| + |
| +// mayMail is the queue's gate: gateBegin's mayRequestLink answer for |
| +// the request that asked for the link (ruling 12's budgets), carried |
| +// through Begin on the request context. A context lookup, no I/O. |
| +func mayMail(ctx context.Context) bool { |
| + may, _ := ctx.Value(mayMailKey{}).(bool) |
| + return may |
| +} |
| + |
| +// queueAttacher is a test mailer (another package's, so the method is |
| +// exported) that wants to wait for the queue in front of it before it |
| +// is read (callstest's recordingMailer). |
| +type queueAttacher interface{ AttachSigninQueue(wait func()) } |
diff --git a/internal/calls/signin_mail_test.go b/internal/calls/signin_mail_test.go| new file mode 100644 |
| index 0000000..0502e96 |
| --- /dev/null |
| +++ b/internal/calls/signin_mail_test.go |
| @@ -0,0 +1,207 @@ |
| +package calls |
| + |
| +import ( |
| + "context" |
| + "fmt" |
| + "log/slog" |
| + "path/filepath" |
| + "runtime" |
| + "strings" |
| + "sync" |
| + "testing" |
| + "time" |
| + |
| + "amadan.net/rastrillo/rastrillo/db" |
| +) |
| + |
| +// stuckHandler is a slog handler whose every write blocks until release |
| +// is closed: a log sink that has stalled, as a full pipe does. It |
| +// remembers what it was finally allowed to write. |
| +type stuckHandler struct { |
| + release chan struct{} |
| + mu *sync.Mutex |
| + msgs *[]string |
| +} |
| + |
| +func (h stuckHandler) Enabled(context.Context, slog.Level) bool { return true } |
| +func (h stuckHandler) Handle(_ context.Context, r slog.Record) error { |
| + <-h.release |
| + h.mu.Lock() |
| + *h.msgs = append(*h.msgs, r.Message) |
| + h.mu.Unlock() |
| + return nil |
| +} |
| +func (h stuckHandler) WithAttrs([]slog.Attr) slog.Handler { return h } |
| +func (h stuckHandler) WithGroup(string) slog.Handler { return h } |
| + |
| +// funcSender adapts a function to mail.Sender. |
| +type funcSender func(ctx context.Context, to string) error |
| + |
| +func (f funcSender) Send(ctx context.Context, to, _, _ string) error { return f(ctx, to) } |
| + |
| +func everyone(string) bool { return true } |
| + |
| +// TestMailQueueSendNeverBlocksWhenFull: Send is the request path, so it |
| +// may not block or do I/O even when the queue is full — not even to log |
| +// the drop (Astra on oficina/slides #20, round 2, P1: a "queue full" log |
| +// line on a stalled sink held the request, and saturation is |
| +// admission-dependent). The worker is stuck on its first link and the |
| +// log sink is stuck too; twice the queue's depth of Sends must still |
| +// return at once, and the drops are logged by the worker afterwards. |
| +func TestMailQueueSendNeverBlocksWhenFull(t *testing.T) { |
| + release := make(chan struct{}) |
| + var once sync.Once |
| + free := func() { once.Do(func() { close(release) }) } |
| + stuck := funcSender(func(ctx context.Context, _ string) error { |
| + select { |
| + case <-release: |
| + case <-ctx.Done(): |
| + } |
| + return nil |
| + }) |
| + var mu sync.Mutex |
| + var msgs []string |
| + q := newMailQueue(stuck, everyone, nil, slog.New(stuckHandler{release, &mu, &msgs})) |
| + t.Cleanup(func() { free(); q.close() }) |
| + |
| + done := make(chan struct{}) |
| + go func() { |
| + for i := 0; i < 2*mailQueueDepth; i++ { |
| + _ = q.Send(context.Background(), fmt.Sprintf("p%d@example.test", i), "s", "b") |
| + } |
| + close(done) |
| + }() |
| + select { |
| + case <-done: |
| + case <-time.After(2 * time.Second): |
| + free() |
| + <-done |
| + t.Fatal("Send blocked with the queue full: the request path waited on the queue or the log") |
| + } |
| + // The drops are the worker's to log, once it is free to. |
| + free() |
| + q.close() |
| + mu.Lock() |
| + defer mu.Unlock() |
| + for _, m := range msgs { |
| + if strings.Contains(m, "links dropped") { |
| + return |
| + } |
| + } |
| + t.Fatalf("no drop was logged with twice the queue's depth sent; the log has %q", msgs) |
| +} |
| + |
| +// TestMailQueueQueuesEveryAddress: the queue's fill level must not |
| +// depend on admission, so Send queues a refused address's link exactly |
| +// as it queues a permitted one; only the worker asks. |
| +func TestMailQueueQueuesEveryAddress(t *testing.T) { |
| + release := make(chan struct{}) |
| + stuck := funcSender(func(ctx context.Context, _ string) error { |
| + select { |
| + case <-release: |
| + case <-ctx.Done(): |
| + } |
| + return nil |
| + }) |
| + var asked sync.Map |
| + admits := func(to string) bool { asked.Store(to, true); return to == "first@example.test" } |
| + q := newMailQueue(stuck, admits, nil, slog.New(slog.DiscardHandler)) |
| + t.Cleanup(func() { close(release); q.close() }) |
| + _ = q.Send(context.Background(), "first@example.test", "s", "b") |
| + deadline := time.Now().Add(2 * time.Second) |
| + for _, ok := asked.Load("first@example.test"); !ok && time.Now().Before(deadline); _, ok = asked.Load("first@example.test") { |
| + time.Sleep(5 * time.Millisecond) |
| + } |
| + for i := 0; i < 5; i++ { |
| + _ = q.Send(context.Background(), fmt.Sprintf("refused%d@example.test", i), "s", "b") |
| + } |
| + if got := len(q.jobs); got != 5 { |
| + t.Fatalf("queued %d refused links, want all 5: fill level must not depend on admission", got) |
| + } |
| +} |
| + |
| +// TestMailQueueSurvivesAPanic: a panic in the mailer is that message's |
| +// failure; the worker recovers, logs, and sends the next one (Astra, |
| +// round 2, P2). |
| +func TestMailQueueSurvivesAPanic(t *testing.T) { |
| + var mu sync.Mutex |
| + var sent []string |
| + calls := 0 |
| + s := funcSender(func(_ context.Context, to string) error { |
| + mu.Lock() |
| + defer mu.Unlock() |
| + calls++ |
| + if calls == 1 { |
| + panic("mailer exploded") |
| + } |
| + sent = append(sent, to) |
| + return nil |
| + }) |
| + q := newMailQueue(s, everyone, nil, slog.New(slog.DiscardHandler)) |
| + t.Cleanup(q.close) |
| + _ = q.Send(context.Background(), "first@example.test", "s", "b") |
| + _ = q.Send(context.Background(), "second@example.test", "s", "b") |
| + q.wait() |
| + mu.Lock() |
| + defer mu.Unlock() |
| + if len(sent) != 1 || sent[0] != "second@example.test" { |
| + t.Fatalf("sent = %v, want the second link after the first panicked", sent) |
| + } |
| +} |
| + |
| +// TestMailQueueCloseIsBoundedAndStopsTheWorker: close drains within its |
| +// deadline, then cancels the send in flight, abandons the rest, and the |
| +// worker is gone — no goroutine outlives close (Astra, round 2, P2). |
| +func TestMailQueueCloseIsBoundedAndStopsTheWorker(t *testing.T) { |
| + baseline := runtime.NumGoroutine() |
| + honours := funcSender(func(ctx context.Context, _ string) error { |
| + <-ctx.Done() // a relay that never answers, but obeys cancellation |
| + return ctx.Err() |
| + }) |
| + q := newMailQueue(honours, everyone, nil, slog.New(slog.DiscardHandler)) |
| + q.drainTimeout = 50 * time.Millisecond |
| + for i := 0; i < 3; i++ { |
| + _ = q.Send(context.Background(), fmt.Sprintf("p%d@example.test", i), "s", "b") |
| + } |
| + start := time.Now() |
| + q.close() |
| + if took := time.Since(start); took > 2*time.Second { |
| + t.Fatalf("close took %v, want about its 50ms deadline", took) |
| + } |
| + select { |
| + case <-q.done: |
| + default: |
| + t.Fatal("the worker was still running after close") |
| + } |
| + deadline := time.Now().Add(time.Second) |
| + for runtime.NumGoroutine() > baseline && time.Now().Before(deadline) { |
| + time.Sleep(5 * time.Millisecond) |
| + } |
| + if n := runtime.NumGoroutine(); n > baseline { |
| + t.Fatalf("%d goroutines after close, %d before: the queue leaked", n, baseline) |
| + } |
| +} |
| + |
| +// TestShutdownStopsTheMailWorker: the queue lives as long as the app's |
| +// context, and ending it stops the worker, so no test that cancels its |
| +// app leaves one running. |
| +func TestShutdownStopsTheMailWorker(t *testing.T) { |
| + logger := slog.New(slog.DiscardHandler) |
| + d, err := db.Open(filepath.Join(t.TempDir(), "calls.db"), logger) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + t.Cleanup(func() { d.Close() }) |
| + ctx, cancel := context.WithCancel(context.Background()) |
| + _, a, err := build(ctx, d, Config{Env: EnvDevelopment, Origin: "http://app.test", Logger: logger}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + q := a.signinMail |
| + cancel() |
| + select { |
| + case <-q.done: |
| + case <-time.After(2 * time.Second): |
| + t.Fatal("the sign-in mail worker was still running after the app's context ended") |
| + } |
| +} |
diff --git a/internal/calls/signin_timing_test.go b/internal/calls/signin_timing_test.go| new file mode 100644 |
| index 0000000..e17a88f |
| --- /dev/null |
| +++ b/internal/calls/signin_timing_test.go |
| @@ -0,0 +1,94 @@ |
| +package calls |
| + |
| +import ( |
| + "bytes" |
| + "context" |
| + "encoding/base64" |
| + "io" |
| + "log/slog" |
| + "net/http/httptest" |
| + "net/url" |
| + "path/filepath" |
| + "sync" |
| + "testing" |
| + "time" |
| + |
| + "amadan.net/rastrillo/rastrillo/db" |
| +) |
| + |
| +// 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: when the send ran inside the |
| +// request, a member waited for SMTP before its ?sent=1 and a stranger |
| +// did not (Astra measured 252 ms against 0.35 ms on Slides, which had |
| +// the same shape). With the mailer blocked, every POST — member, |
| +// stranger, and both with force=1 — must answer, and the member's |
| +// links must then go out. |
| +func TestSigninNeverWaitsForTheMailer(t *testing.T) { |
| + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) |
| + d, err := db.Open(filepath.Join(t.TempDir(), "calls.db"), logger) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + t.Cleanup(func() { d.Close() }) |
| + bm := &blockingMailer{release: make(chan struct{})} |
| + var once sync.Once |
| + free := func() { once.Do(func() { close(bm.release) }) } |
| + t.Cleanup(free) |
| + mux, _, err := build(t.Context(), d, Config{Origin: keymailOrigin, InstanceKey: "test-only-instance", |
| + DEK: base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{3}, 32)), AllowedAddresses: []string{plainMember}, Mailer: bm, Logger: logger}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + f := &keymailFixture{handler: mux} |
| + |
| + for _, form := range []url.Values{ |
| + {"address": {plainMember}}, |
| + {"address": {"someone@else.test"}}, |
| + {"address": {plainMember}, "force": {"1"}}, |
| + {"address": {"someone@else.test"}, "force": {"1"}}, |
| + } { |
| + done := make(chan *httptest.ResponseRecorder, 1) |
| + go func() { done <- f.post(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 a member", 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) |
| + if got := bm.sent(); len(got) != 2 || got[0] != plainMember || got[1] != plainMember { |
| + t.Fatalf("mail sent = %v, want two links, both to %s", got, plainMember) |
| + } |
| +} |
diff --git a/internal/calls/templates/signin.html b/internal/calls/templates/signin.html| index c5f7a3b..06fca6b 100644 |
| --- a/internal/calls/templates/signin.html |
| +++ b/internal/calls/templates/signin.html |
| @@ -1,6 +1,9 @@ |
| {{/* The Oficina door (estilo chrome): one card on the suite's sky, Calls' |
| mark and name at its head. The form, or once a link has been asked |
| - for, the check-your-email notice. auth.go's localSignin builds both. */}} |
| + for, the check-your-email notice — or, on the way to Keymail, the |
| + door's notice with .Continue's meta refresh in head (renderKeymail). |
| + auth.go's localSignin builds the first two. */}} |
| +{{define "head"}}{{with .Continue}}{{template "home-oauth-head" .}}{{end}}{{end}} |
| {{define "content"}} |
| {{if .Sent}}{{template "estilo-signin-sent" .Notice}}{{else}}{{template "estilo-signin" .Door}}{{end}} |
| {{end}} |
diff --git a/internal/callstest/browser_test.go b/internal/callstest/browser_test.go| index 974da52..20acda9 100644 |
| --- a/internal/callstest/browser_test.go |
| +++ b/internal/callstest/browser_test.go |
| @@ -419,6 +419,7 @@ func (rig *browserRig) serve(t *testing.T, ln net.Listener) { |
| if err != nil { |
| t.Fatalf("calls.NewForTest: %v", err) |
| } |
| + t.Cleanup(ctl.ShutdownForTest) // stops the sign-in mail worker too |
| srv := &http.Server{Handler: mux} |
| go func() { _ = srv.Serve(ln) }() |
diff --git a/internal/callstest/harness_test.go b/internal/callstest/harness_test.go| index a72510f..85e9734 100644 |
| --- a/internal/callstest/harness_test.go |
| +++ b/internal/callstest/harness_test.go |
| @@ -163,6 +163,25 @@ type recordingMailer struct { |
| to []string |
| subjects []string |
| links []string |
| + // waits are the sign-in mail queues in front of this recorder: |
| + // links go out after the response (signin_mail.go), so every read |
| + // waits for them first. |
| + waits []func() |
| +} |
| + |
| +func (m *recordingMailer) AttachSigninQueue(wait func()) { |
| + m.mu.Lock() |
| + m.waits = append(m.waits, wait) |
| + m.mu.Unlock() |
| +} |
| + |
| +func (m *recordingMailer) flush() { |
| + m.mu.Lock() |
| + waits := append([]func(){}, m.waits...) |
| + m.mu.Unlock() |
| + for _, w := range waits { |
| + w() |
| + } |
| } |
| var magicLink = regexp.MustCompile(`https?://\S+/auth/verify\?token=\S+`) |
| @@ -177,6 +196,7 @@ func (m *recordingMailer) Send(ctx context.Context, to, subject, body string) er |
| } |
| func (m *recordingMailer) last() (string, string) { |
| + m.flush() |
| m.mu.Lock() |
| defer m.mu.Unlock() |
| if len(m.to) == 0 { |
| @@ -187,6 +207,7 @@ func (m *recordingMailer) last() (string, string) { |
| func (m *recordingMailer) lastSubject(t *testing.T) string { |
| t.Helper() |
| + m.flush() |
| m.mu.Lock() |
| defer m.mu.Unlock() |
| if len(m.subjects) == 0 { |
| @@ -196,6 +217,7 @@ func (m *recordingMailer) lastSubject(t *testing.T) string { |
| } |
| func (m *recordingMailer) count() int { |
| + m.flush() |
| m.mu.Lock() |
| defer m.mu.Unlock() |
| return len(m.to) |
| @@ -248,6 +270,9 @@ func newHarnessWith(t *testing.T, env string, logger *slog.Logger, adjust func(* |
| if err != nil { |
| t.Fatal(err) |
| } |
| + // Ends the app's context, which stops its sign-in mail worker |
| + // (signin_mail.go): no test leaves one running. |
| + t.Cleanup(ctl.ShutdownForTest) |
| // The base is now, not a fixed date. Calls' clock is the only one a |
| // test can move; rastrillo stamps its auth_links rows from the real |
| // one. A fixed base put the two most of a day apart, so nothing that |
diff --git a/internal/callstest/join_test.go b/internal/callstest/join_test.go| index a129127..347e28e 100644 |
| --- a/internal/callstest/join_test.go |
| +++ b/internal/callstest/join_test.go |
| @@ -103,9 +103,11 @@ func TestLinkReplacedBetweenMailAndClickAdmitsNobody(t *testing.T) { |
| member.signIn("paul@example.com") |
| member.post("/meetings/"+sqid+"/links", nil) |
| u, _ := url.Parse(link) |
| + // Authorize refuses it, and the refusal is the door (doorOnRefusal), |
| + // not rastrillo's bare-text 403. |
| rec := g.get(u.RequestURI()) |
| - if rec.Code != http.StatusForbidden { |
| - t.Fatalf("outsider's stale click: %d, want 403 from Authorize", rec.Code) |
| + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/signin?err=refused" { |
| + t.Fatalf("outsider's stale click: %d %q, want Authorize's refusal as 303 /signin?err=refused", rec.Code, rec.Header().Get("Location")) |
| } |
| if rec := g.get("/m/" + sqid + "/join"); rec.Code == http.StatusOK { |
| t.Fatal("a grant was minted through a replaced link") |
| @@ -395,8 +397,8 @@ func TestExpiredPendingRowAdmitsNobody(t *testing.T) { |
| if err != nil { |
| t.Fatal(err) |
| } |
| - if rec := g.get(u.RequestURI()); rec.Code != http.StatusForbidden { |
| - t.Fatalf("expired pending row click: %d, want 403 from Authorize", rec.Code) |
| + if rec := g.get(u.RequestURI()); rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/signin?err=refused" { |
| + t.Fatalf("expired pending row click: %d %q, want Authorize's refusal as 303 /signin?err=refused", rec.Code, rec.Header().Get("Location")) |
| } |
| if got := x.mail.count(); got != before { |
| t.Fatalf("mail count changed after the expired click: %d, want %d", got, before) |
diff --git a/internal/callstest/main_test.go b/internal/callstest/main_test.go| new file mode 100644 |
| index 0000000..cce6da6 |
| --- /dev/null |
| +++ b/internal/callstest/main_test.go |
| @@ -0,0 +1,25 @@ |
| +package callstest |
| + |
| +import ( |
| + "fmt" |
| + "os" |
| + "testing" |
| + |
| + "amadan.net/oficina/calls/internal/calls/keymailtest" |
| +) |
| + |
| +// TestMain keeps Keymail classification inside the process: every POST |
| +// /signin now classifies its address (auth.go's gateBegin), and without |
| +// this each one would ask the real network whether example.com runs |
| +// Keymail. Every DNS query is answered NXDOMAIN; no fixture is |
| +// registered here, so every address is plain email. |
| +func TestMain(m *testing.M) { |
| + restore, err := keymailtest.Install() |
| + if err != nil { |
| + fmt.Fprintln(os.Stderr, "keymailtest.Install:", err) |
| + os.Exit(1) |
| + } |
| + code := m.Run() |
| + restore() |
| + os.Exit(code) |
| +} |
diff --git a/internal/callstest/signin_test.go b/internal/callstest/signin_test.go| index 9d67806..b3caa4e 100644 |
| --- a/internal/callstest/signin_test.go |
| +++ b/internal/callstest/signin_test.go |
| @@ -88,7 +88,7 @@ func TestSigninOverAFailingMailerLeavesNoAddressInTheLog(t *testing.T) { |
| } |
| t.Cleanup(func() { d.Close() }) |
| - failing := failingMailer{err: fmt.Errorf("rastrillo/mail: send to %s: %w", address, errFakeSMTPFailure)} |
| + failing := &failingMailer{err: fmt.Errorf("rastrillo/mail: send to %s: %w", address, errFakeSMTPFailure)} |
| mux, _, err := calls.App(testCtx(t), d, calls.Config{ |
| Env: calls.EnvDevelopment, Origin: testOrigin, AllowedAddresses: []string{address}, |
| Mailer: failing, Logger: logger, |
| @@ -103,6 +103,7 @@ func TestSigninOverAFailingMailerLeavesNoAddressInTheLog(t *testing.T) { |
| rec.Code, rec.Header().Get("Location")) |
| } |
| + failing.flush() // the send, and its failure, happen after the response |
| log := buf.String() |
| if strings.Contains(strings.ToLower(log), strings.ToLower(address)) { |
| t.Fatalf("the log carries the address after a failing send:\n%s", log) |
| @@ -114,9 +115,20 @@ func TestSigninOverAFailingMailerLeavesNoAddressInTheLog(t *testing.T) { |
| var errFakeSMTPFailure = errors.New("550 mailbox unavailable") |
| -type failingMailer struct{ err error } |
| +type failingMailer struct { |
| + err error |
| + waits []func() // the sign-in mail queue in front of it |
| +} |
| + |
| +func (f *failingMailer) Send(ctx context.Context, to, subject, body string) error { return f.err } |
| + |
| +func (f *failingMailer) AttachSigninQueue(wait func()) { f.waits = append(f.waits, wait) } |
| -func (f failingMailer) Send(ctx context.Context, to, subject, body string) error { return f.err } |
| +func (f *failingMailer) flush() { |
| + for _, w := range f.waits { |
| + w() |
| + } |
| +} |
| // Re-review of 9e067e4: a plain member sign-in — no meeting-link cookie, |
| // so no pending_joins row at all — still leaves rastrillo's own |