amadan
Keymail sign-in in Sheets: a continuation page, and no allowlist oracle
Standalone POST /signin answered a Keymail address with a 303 straight to keymail.dev/oauth/authorize. Safari applies form-action 'self' to a form's whole redirect chain and refused it (Paul, 2026-10-06). Both modes now wrap Begin in connect.OAuthContinuation. The admission gate in front of Begin is gone too: it answered a non-admitted Keymail address ?sent=1 while an admitted one went to Keymail, which told anyone who is on the team. Every address now reaches Begin; the ceremony depends on the domain alone, and admissionMailer mails a link only when App.Admits says yes. Admission stays auth's Authorize after verification, and its bare-text 403 becomes the door with ?err=refused (doorOnRefusal) until rastrillo has a hook. Tests stub classification and Keymail in-process (keymailtest), so no POST /signin in the suite reaches the real network any more.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
9 files changed,
+549
−60
internal/sheets/app.go+3 −3internal/sheets/auth.go+5 −3internal/sheets/begin.go+122 −39internal/sheets/begin_test.go+6 −1internal/sheets/home.go+3 −6internal/sheets/home_routes.go+6 −6internal/sheets/keymail_signin_test.go+207 −0internal/sheets/keymailtest/keymailtest.go+190 −0internal/sheets/signin.go+7 −2
CI passed — run details
CI log
amadan: .amadan/timeout: this job's budget is 15m0s (this runner allows up to 30m0s) === vet === CGO_ENABLED=0 go vet ./... CGO_ENABLED=0 go vet -tags browser ./internal/sheets/ === fmt === === build === CGO_ENABLED=0 go build ./... === migrations === go tool rastrillo migration check rastrillo migration check: models and migrations agree === go-test === CGO_ENABLED=0 go test -count=1 ./... ? amadan.net/oficina/sheets/cmd/sheets [no test files] ok amadan.net/oficina/sheets/internal/guardstest 0.090s ok amadan.net/oficina/sheets/internal/sheets 3.873s ok amadan.net/oficina/sheets/internal/sheets/calc 0.008s ok amadan.net/oficina/sheets/internal/sheets/format 0.004s ? amadan.net/oficina/sheets/internal/sheets/keymailtest [no test files] ok amadan.net/oficina/sheets/internal/sheets/live 0.054s ok amadan.net/oficina/sheets/internal/sheets/seal 0.003s ok amadan.net/oficina/sheets/internal/sheets/sealtest 0.004s ok amadan.net/oficina/sheets/internal/sheets/store 3.024s ok amadan.net/oficina/sheets/internal/sheets/xlsx 0.317s ? amadan.net/oficina/sheets/tools/gencorpus [no test files] === race === go test -race -count=1 ./internal/sheets/store/... ./internal/sheets/live/... ok amadan.net/oficina/sheets/internal/sheets/store 52.603s ok amadan.net/oficina/sheets/internal/sheets/live 1.063s
diff --git a/internal/sheets/app.go b/internal/sheets/app.go| index 0dc93c3..86eb225 100644 |
| --- a/internal/sheets/app.go |
| +++ b/internal/sheets/app.go |
| @@ -46,9 +46,9 @@ type Config struct { |
| // the CSRF same-origin check's reference value. |
| Origin string |
| - // Admit is the allowlist, consulted before a link is SENT. |
| - // auth.Config.Authorize covers the same list at redemption; this |
| - // one exists so we do not email strangers. See handleBegin. |
| + // Admit is the allowlist, the static half of App.Admits. Admits is |
| + // consulted before a link is SENT (admissionMailer) and at |
| + // redemption (auth.Config.Authorize). See handleBegin. |
| Admit func(string) bool |
| } |
diff --git a/internal/sheets/auth.go b/internal/sheets/auth.go| index a0f7a2a..752eeae 100644 |
| --- a/internal/sheets/auth.go |
| +++ b/internal/sheets/auth.go |
| @@ -159,9 +159,11 @@ func NewAuth(db *sql.DB, env AuthEnv, logger *slog.Logger, authorize func(string |
| DB: db, |
| Origin: env.Origin, |
| InstanceKey: env.InstanceKey, |
| - Mailer: env.Mailer, |
| - Authorize: authorize, |
| - SigninPath: "/signin", |
| + // Begin runs for every address (handleBegin); the mail goes |
| + // only to one authorize admits. |
| + Mailer: admissionMailer{next: env.Mailer, admits: authorize}, |
| + Authorize: authorize, |
| + SigninPath: "/signin", |
| // Where a fresh session lands. auth defaults this to "/", |
| // which is the anonymous entry page — so without this line |
| // every magic-link click arrives on a page that exists to |
diff --git a/internal/sheets/begin.go b/internal/sheets/begin.go| index a66d910..b551dcc 100644 |
| --- a/internal/sheets/begin.go |
| +++ b/internal/sheets/begin.go |
| @@ -1,49 +1,132 @@ |
| package sheets |
| -import "net/http" |
| +import ( |
| + "bytes" |
| + "context" |
| + "net/http" |
| + "strings" |
| -// handleBegin gates POST /signin on admission BEFORE auth mints and |
| -// emails a link. |
| + "amadan.net/oficina/home/connect" |
| + "amadan.net/rastrillo/rastrillo/mail" |
| +) |
| + |
| +// handleBegin is POST /signin, in both modes: auth's Begin for EVERY |
| +// address, under connect.OAuthContinuation. |
| +// |
| +// What the form answers depends on the address's domain, never on |
| +// admission. Answering "that address cannot sign in" — or answering an |
| +// admitted address differently from a refused one in any way — would |
| +// turn this endpoint into a membership oracle: anyone could enumerate |
| +// who is on the team by watching which addresses get a different |
| +// answer. So: |
| // |
| -// rastrillo/auth checks Config.Authorize at admission — the moment a |
| -// verified address would receive a session — not when the link is sent. |
| -// That is the right place for the security decision, and the security |
| -// property holds without this: an address that is not admitted cannot |
| -// get a session no matter how many links it redeems. |
| +// - A Keymail address (a domain that runs Keymail, or delegates to it |
| +// with a `_keymail` TXT record), admitted or not, goes to Keymail. |
| +// Begin answers with a 303 to Keymail's authorize URL, and |
| +// OAuthContinuation turns that into a page (renderHomeOAuth): CSP's |
| +// form-action 'self' covers a form submission's whole redirect |
| +// chain in Safari, so a 303 out of the POST was refused there |
| +// ("Refused to load https://keymail.dev/oauth/authorize… form-action", |
| +// Paul, 2026-10-06). Admission is asked after Keymail has verified |
| +// the address, by auth's Authorize (App.Admits), and a refusal is |
| +// the door saying so (doorOnRefusal), told only to someone who has |
| +// just proved they hold the address. |
| +// - Any other address is answered ?sent=1, and admissionMailer mails |
| +// the link only if App.Admits says yes. Without that, anyone who |
| +// can reach the instance could make it email a live sign-in link to |
| +// any address they name — abuse of our sending identity, found by |
| +// POSTing a stranger's address at the deployed instance. The cost |
| +// is real and accepted: a member who mistypes their own address is |
| +// told to check their email and nothing arrives. |
| // |
| -// What it leaves open is abuse, not access. Without this gate, anyone |
| -// who can reach the instance can make it email a live sign-in link to |
| -// any address they name, at whatever rate the IP limiter allows. The |
| -// recipient gets unsolicited mail from us and our sending identity wears |
| -// it. Found by POSTing a stranger's address at the deployed instance and |
| -// getting ?sent=1 back — the unit tests all passed, because they tested |
| -// admission rather than delivery. |
| +// Both kinds of address spend the same rate budgets and are parsed the |
| +// same way, so ?err=rate and ?err=address say nothing about admission |
| +// either. The one outcome only an admitted address can reach is ?err=1 |
| +// from a send that failed, an operational fault rather than an answer. |
| // |
| -// The refusal is deliberately INVISIBLE. Answering "that address cannot |
| -// sign in" would turn this endpoint into a membership oracle: anyone |
| -// could enumerate who is on the team by watching which addresses get a |
| -// different answer. So a non-admitted address gets exactly the response |
| -// an admitted one gets, and no email. The cost is real and accepted: a |
| -// member who mistypes their own address is told to check their email and |
| -// nothing arrives. Every mainstream provider makes the same trade. |
| +// It used to gate admission in front of Begin instead, which answered a |
| +// non-admitted Keymail address ?sent=1 and sent an admitted one to |
| +// Keymail: the oracle, one level narrower. |
| func (a *App) handleBegin(w http.ResponseWriter, r *http.Request) { |
| - // Parse before reading the field: Begin would do this itself, and |
| - // this handler has to read the address before deciding to call it. |
| - if err := r.ParseForm(); err != nil { |
| - a.badRequest(w, r, http.StatusBadRequest, "Malformed request.") |
| - return |
| + connect.OAuthContinuation(a.origin, a.auth.Begin, a.renderHomeOAuth)(w, r) |
| +} |
| + |
| +// admissionMailer is the mail half of admission: Begin runs for every |
| +// address (handleBegin), and this is where one App.Admits refuses is |
| +// quietly not mailed. App.Admits, NOT the static owners list: the two |
| +// must ask the same question as auth's own Authorize, or a guest |
| +// somebody shared a sheet with would be admitted at redemption and |
| +// never sent a link to redeem. auth has already stored an expiring |
| +// token by the time it calls Send; one never delivered is never |
| +// redeemed, and Authorize would refuse it if it were. |
| +type admissionMailer struct { |
| + next mail.Sender |
| + admits func(string) bool |
| +} |
| + |
| +func (m admissionMailer) Send(ctx context.Context, to, subject, body string) error { |
| + if !m.admits(to) { |
| + return nil |
| + } |
| + return m.next.Send(ctx, to, subject, body) |
| +} |
| + |
| +// 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 auth's Callback and Verify, the handlers that end |
| +// in admission, and turns its bare-text refusal into the sign-in 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 |
| } |
| - // App.Admits, NOT a.admit. The two must ask the same question as |
| - // auth's own Authorize, or the answers disagree: a.admit is the |
| - // static SHEETS_OWNERS list, so a guest somebody shared a sheet |
| - // with would be admitted at redemption and never sent a link to |
| - // redeem — a share that silently does nothing. Caught by the |
| - // two-user isolation suite the moment a guest tried to sign in. |
| - if !a.Admits(r.FormValue("address")) { |
| - // Identical to auth's own success redirect, so the two cases are |
| - // indistinguishable from outside. |
| - http.Redirect(w, r, "/signin?sent=1", http.StatusSeeOther) |
| - return |
| +} |
| +func (b *bufferedResponse) Write(p []byte) (int, error) { |
| + if b.status == 0 { |
| + b.status = http.StatusOK |
| } |
| - a.auth.Begin(w, r) |
| + return b.body.Write(p) |
| } |
diff --git a/internal/sheets/begin_test.go b/internal/sheets/begin_test.go| index f5c4bad..53a4d65 100644 |
| --- a/internal/sheets/begin_test.go |
| +++ b/internal/sheets/begin_test.go |
| @@ -67,6 +67,12 @@ func appWithRecorder(t *testing.T) (*App, *recorder) { |
| } |
| func appWithOrigin(t *testing.T, origin string) (*App, *recorder) { |
| + t.Helper() |
| + return appWithOwners(t, origin, signedInAddress) |
| +} |
| + |
| +// appWithOwners is appWithOrigin with the SHEETS_OWNERS list spelled out. |
| +func appWithOwners(t *testing.T, origin string, owners ...string) (*App, *recorder) { |
| t.Helper() |
| d, err := rdb.Open(filepath.Join(t.TempDir(), "test.db"), quietLogger()) |
| if err != nil { |
| @@ -77,7 +83,6 @@ func appWithOrigin(t *testing.T, origin string) (*App, *recorder) { |
| t.Fatalf("migrate: %v", err) |
| } |
| rec := &recorder{} |
| - owners := []string{signedInAddress} |
| sealer, err := seal.New(make([]byte, dekBytes)) |
| if err != nil { |
| t.Fatalf("seal.New: %v", err) |
diff --git a/internal/sheets/home.go b/internal/sheets/home.go| index a1261f2..e744ad8 100644 |
| --- a/internal/sheets/home.go |
| +++ b/internal/sheets/home.go |
| @@ -159,13 +159,10 @@ func (a *App) homeLocalComplete(w http.ResponseWriter, r *http.Request) { |
| } |
| a.homeRoutes.LocalComplete(w, r) |
| } |
| -func (a *App) homeLocalBegin(w http.ResponseWriter, r *http.Request) { |
| - connect.OAuthContinuation(a.origin, a.handleBegin, a.renderHomeOAuth)(w, r) |
| -} |
| -// renderHomeOAuth is the page between the door and Keymail, and it is |
| -// the same card: the refresh stays Home's (head), the body is the |
| -// door's notice. |
| +// renderHomeOAuth is the page between the door and Keymail, in both |
| +// modes (handleBegin), and it is the same card: the refresh stays |
| +// Home's (head), the body is the door's notice. |
| func (a *App) renderHomeOAuth(w http.ResponseWriter, r *http.Request, destination string) { |
| p := connect.OAuthPage(destination) |
| a.render(w, r, "home_oauth", frame{parts: []string{p.Title}, signinPage: true}, http.StatusOK, homeOAuthView{ |
diff --git a/internal/sheets/home_routes.go b/internal/sheets/home_routes.go| index 6aecf88..ce8c46a 100644 |
| --- a/internal/sheets/home_routes.go |
| +++ b/internal/sheets/home_routes.go |
| @@ -28,15 +28,15 @@ func (a *App) mountAuth(r chi.Router) { |
| // baked into every emailed link, so moving it later invalidates |
| // links already in people's inboxes. |
| if a.homeRoutes != nil { |
| - r.Post("/signin", a.homeRoutes.LocalGuard(a.homeLocalBegin)) |
| - r.Get("/auth/callback", a.homeRoutes.LocalGuard(a.auth.Callback)) |
| - r.Get("/auth/verify", a.homeRoutes.LocalGuard(a.auth.Verify)) |
| - r.Post("/auth/verify", a.homeRoutes.LocalGuard(a.auth.Verify)) |
| + r.Post("/signin", a.homeRoutes.LocalGuard(a.handleBegin)) |
| + r.Get("/auth/callback", a.homeRoutes.LocalGuard(doorOnRefusal(a.auth.Callback, "/signin/local"))) |
| + r.Get("/auth/verify", a.homeRoutes.LocalGuard(doorOnRefusal(a.auth.Verify, "/signin/local"))) |
| + r.Post("/auth/verify", a.homeRoutes.LocalGuard(doorOnRefusal(a.auth.Verify, "/signin/local"))) |
| r.Post("/signout", a.homeRoutes.Signout) |
| } else { |
| r.Post("/signin", a.handleBegin) |
| - r.Get("/auth/callback", a.auth.Callback) |
| - r.Get("/auth/verify", a.auth.Verify) |
| + r.Get("/auth/callback", doorOnRefusal(a.auth.Callback, "/signin")) |
| + r.Get("/auth/verify", doorOnRefusal(a.auth.Verify, "/signin")) |
| r.Post("/signout", a.signOutToPage) |
| } |
| } |
diff --git a/internal/sheets/keymail_signin_test.go b/internal/sheets/keymail_signin_test.go| new file mode 100644 |
| index 0000000..381ea8b |
| --- /dev/null |
| +++ b/internal/sheets/keymail_signin_test.go |
| @@ -0,0 +1,207 @@ |
| +package sheets |
| + |
| +import ( |
| + "fmt" |
| + "html" |
| + "net/http" |
| + "net/http/httptest" |
| + "net/url" |
| + "os" |
| + "regexp" |
| + "strings" |
| + "testing" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets/keymailtest" |
| +) |
| + |
| +// TestMain keeps Keymail classification inside the process: every DNS |
| +// query is answered NXDOMAIN and only the Keymail fixtures a test |
| +// registers answer HTTPS (keymailtest). Before it, every POST /signin |
| +// in this suite classified its address against the real network. |
| +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.sheets.test" |
| + keymailMember = "paul@" + keymailHost |
| + keymailStranger = "stranger@" + keymailHost |
| +) |
| + |
| +// keymailApp is a standalone app whose owners include keymailMember, |
| +// beside a Keymail deployment holding keymailMember and keymailStranger. |
| +func keymailApp(t *testing.T) (*App, *recorder, *keymailtest.Server) { |
| + t.Helper() |
| + km := keymailtest.Serve(t, keymailHost, keymailMember, keymailStranger) |
| + app, rec := appWithOwners(t, testOrigin, signedInAddress, keymailMember) |
| + return app, rec, km |
| +} |
| + |
| +func keymailPost(t *testing.T, h http.Handler, form url.Values, cookies ...*http.Cookie) *httptest.ResponseRecorder { |
| + t.Helper() |
| + r := httptest.NewRequest(http.MethodPost, testOrigin+"/signin", strings.NewReader(form.Encode())) |
| + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + r.Header.Set("Origin", testOrigin) |
| + r.Header.Set("Sec-Fetch-Site", "same-origin") |
| + for _, c := range cookies { |
| + r.AddCookie(c) |
| + } |
| + w := httptest.NewRecorder() |
| + h.ServeHTTP(w, r) |
| + return w |
| +} |
| + |
| +func keymailGet(t *testing.T, h http.Handler, path string, cookies ...*http.Cookie) *httptest.ResponseRecorder { |
| + t.Helper() |
| + r := httptest.NewRequest(http.MethodGet, testOrigin+path, nil) |
| + for _, c := range cookies { |
| + r.AddCookie(c) |
| + } |
| + w := httptest.NewRecorder() |
| + h.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 redirect from 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 — the hop to Keymail must not be a redirect from the form POST", 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 |
| +} |
| + |
| +// TestKeymailAddressGoesToKeymailWhetherAdmittedOrNot: whether an |
| +// address goes to Keymail depends on its domain alone. An admitted and |
| +// a non-admitted Keymail address get the same first answer — the |
| +// continuation page — so the answer says nothing about who is on the |
| +// team. Before, the admission gate answered the stranger ?sent=1 and |
| +// sent the member to Keymail with a 303 out of the form POST, which |
| +// Safari refused under form-action 'self'. |
| +func TestKeymailAddressGoesToKeymailWhetherAdmittedOrNot(t *testing.T) { |
| + app, mail, km := keymailApp(t) |
| + h := app.Router() |
| + member := keymailPost(t, h, url.Values{"address": {keymailMember}}) |
| + stranger := keymailPost(t, h, url.Values{"address": {keymailStranger}}) |
| + mu := continuation(t, "admitted Keymail address", member) |
| + su := continuation(t, "non-admitted 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:\nadmitted:\n%s\nnot admitted:\n%s", a, b) |
| + } |
| + if got := mail.addresses(); len(got) != 0 { |
| + t.Errorf("a Keymail address was mailed: %v", got) |
| + } |
| + if n := km.Lookups(); n != 2 { |
| + t.Errorf("Keymail saw %d lookups, want 2: both addresses classified", n) |
| + } |
| +} |
| + |
| +// TestHomeLocalKeymailIsTheSameAnswer is the same in Home mode, where the |
| +// local form posts to /signin under LocalGuard. |
| +func TestHomeLocalKeymailIsTheSameAnswer(t *testing.T) { |
| + km := keymailtest.Serve(t, keymailHost, keymailMember, keymailStranger) |
| + f := newHomeFixture(t) |
| + member := f.request(t, http.MethodPost, "/signin", url.Values{"address": {signedInAddress}}, false) |
| + if member.Code != http.StatusSeeOther || member.Header().Get("Location") != "/signin?sent=1" { |
| + t.Fatalf("Home-local plain member: %d %q", member.Code, member.Header().Get("Location")) |
| + } |
| + stranger := f.request(t, http.MethodPost, "/signin", url.Values{"address": {keymailStranger}}, false) |
| + continuation(t, "Home-local non-admitted Keymail address", stranger) |
| + if n := km.Lookups(); n != 1 { |
| + t.Errorf("Keymail saw %d lookups, want 1", n) |
| + } |
| +} |
| + |
| +// TestNonAdmittedPlainAddressIsAnsweredSentAndNotMailed: an address |
| +// with no Keymail still gets ?sent=1, admitted or not, and only the |
| +// admitted one is mailed. |
| +func TestNonAdmittedPlainAddressIsAnsweredSentAndNotMailed(t *testing.T) { |
| + app, mail, _ := keymailApp(t) |
| + h := app.Router() |
| + for _, address := range []string{"stranger@example.com", signedInAddress} { |
| + rec := keymailPost(t, h, 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 := mail.addresses(); len(got) != 1 || got[0] != signedInAddress { |
| + t.Fatalf("mail = %v, want one, to %s", got, signedInAddress) |
| + } |
| +} |
| + |
| +// TestKeymailFailureFallsBackToAnEmailLink: the form rastrillo lands a |
| +// failed Keymail exchange on carries force, and a forced POST mails a |
| +// link without classifying. |
| +func TestKeymailFailureFallsBackToAnEmailLink(t *testing.T) { |
| + app, mail, km := keymailApp(t) |
| + h := app.Router() |
| + if body := keymailGet(t, h, "/signin?force=1&err=keymail").Body.String(); !strings.Contains(body, `name="force"`) { |
| + t.Fatal("the err=keymail page does not carry force") |
| + } |
| + rec := keymailPost(t, h, 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 := mail.addresses(); len(got) != 1 || got[0] != keymailMember { |
| + t.Fatalf("mail = %v, want one link to %s", got, keymailMember) |
| + } |
| + if km.Lookups() != 0 { |
| + t.Error("a forced sign-in classified the address") |
| + } |
| +} |
| + |
| +// TestKeymailRefusalIsTheDoor: a non-admitted address Keymail verified |
| +// is refused after verification on the sign-in door, not in |
| +// rastrillo's bare text, and no session is minted. An admitted one is |
| +// signed in. |
| +func TestKeymailRefusalIsTheDoor(t *testing.T) { |
| + app, _, km := keymailApp(t) |
| + h := app.Router() |
| + roundTrip := func(address string) *httptest.ResponseRecorder { |
| + begin := keymailPost(t, h, url.Values{"address": {address}}) |
| + u := continuation(t, address, begin) |
| + return keymailGet(t, h, "/auth/callback?"+url.Values{"state": {u.Query().Get("state")}, "code": {km.Approve(address)}}.Encode(), begin.Result().Cookies()...) |
| + } |
| + if cb := roundTrip(keymailMember); cb.Code != http.StatusSeeOther || cb.Header().Get("Location") != "/sheets" { |
| + t.Fatalf("admitted callback: %d %q, want 303 /sheets", cb.Code, cb.Header().Get("Location")) |
| + } |
| + cb := roundTrip(keymailStranger) |
| + if cb.Code != http.StatusSeeOther || cb.Header().Get("Location") != "/signin?err=refused" { |
| + t.Fatalf("non-admitted callback: %d %q, want 303 /signin?err=refused (bare text is the bug)\n%s", cb.Code, cb.Header().Get("Location"), cb.Body.String()) |
| + } |
| + for _, c := range cb.Result().Cookies() { |
| + if strings.Contains(c.Name, "session") && c.MaxAge >= 0 && c.Value != "" { |
| + t.Fatalf("a refused sign-in set a session cookie %q", c.Name) |
| + } |
| + } |
| + door := keymailGet(t, h, "/signin?err=refused").Body.String() |
| + for _, want := range []string{"estilo-door", `name="address"`, "can’t sign in to Sheets"} { |
| + if !strings.Contains(door, want) { |
| + t.Errorf("refusal door lacks %q:\n%s", want, door) |
| + } |
| + } |
| +} |
diff --git a/internal/sheets/keymailtest/keymailtest.go b/internal/sheets/keymailtest/keymailtest.go| new file mode 100644 |
| index 0000000..92ad056 |
| --- /dev/null |
| +++ b/internal/sheets/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/sheets/signin.go b/internal/sheets/signin.go| index 199fe1c..eb71542 100644 |
| --- a/internal/sheets/signin.go |
| +++ b/internal/sheets/signin.go |
| @@ -42,9 +42,14 @@ func signinError(code string) (text string, field bool) { |
| return "Too many attempts. Wait a few minutes, then try again.", false |
| case "address": |
| // rastrillo's ?err=address is an address it could not parse. |
| - // A refused (unadmitted) address is never told so: handleBegin |
| - // answers it exactly as it answers an admitted one. |
| + // A refused (unadmitted) address is never told so at the form: |
| + // handleBegin answers it exactly as it answers an admitted one. |
| return "That doesn’t look like an email address.", true |
| + case "refused": |
| + // Keymail or a link verified the address and Admits said no |
| + // (doorOnRefusal). Told only to someone who just proved they |
| + // hold the address; the form below is the way on. |
| + return "That address can’t sign in to Sheets. Use a different address, or ask someone to share a sheet with it.", false |
| case "expired": |
| return "That link has expired or was already used. Enter your email to get a new one.", false |
| case "keymail": |