oficina / docs Public

Merge branch 'fix/keymail-oauth' into main

claude pushed by paul@keymail.dev 1 month ago a8f5c0e1d67ad042e89793aa941135eacd98aa23 parent c703a41parent 3fffd1b

10 files changed, +524 −214

AGENTS.md +16 −9
internal/docs/admission_test.go +13 −41
internal/docs/app.go +4 −32
internal/docs/auth.go +29 −113
internal/docs/auth_allowlist_test.go +2 −4
internal/docs/auth_keymail_test.go +257 −0
internal/docs/auth_navigation.go +92 −0
internal/docs/auth_navigation_test.go +100 −0
internal/docs/render.go +3 −2
internal/docs/views/signin.html +8 −13

CI passed — run details

diff --git a/AGENTS.md b/AGENTS.md

index 2ce54d8..2fd2be7 100644

--- a/AGENTS.md

+++ b/AGENTS.md

@@ -24,8 +24,8 @@ nothing about either is in your training data.

Every document route — `GET /`, `POST /documents`, `GET /d/{sqid}` — sits

behind rastrillo/auth's `RequireSession` and is unreachable without a real

-session (`internal/docs/auth.go`). Sign-in is a magic link: `POST /signin`

-emails (or, in development, logs) a single-use link; following it

+session (`internal/docs/auth.go`). `POST /signin` uses Keymail OAuth when

+detected. Otherwise it emails (or, in development, logs) a single-use link; following it

(`GET /auth/verify`) sets a real session cookie; `POST /signout` (in the shell

bar's account menu on both signed-in screens) really revokes it,

not just clearing the cookie client-side. No passkey second factor is

@@ -77,13 +77,20 @@ the CARLOS Sheets session on their own deployed instance; every existing

test passed because they all tested admission, none tested delivery.

The fix is `$DOCS_ALLOWED_ADDRESSES` (`docs.Config.AllowedAddresses`,

-`internal/docs/auth.go`): a comma-separated, case/whitespace-insensitive

-allowlist, wired to *two* places — `rauth.Config.Authorize` (admission,

-belt-and-suspenders) and `gateBegin`, a wrapper mounted in front of

-`rauth.Auth.Begin` on `POST /signin` itself, which is the part that

-actually matters, since it is what stops the mail. A non-permitted

-address gets exactly the same `303 ?sent=1` response a permitted one

-gets — anything else would turn the endpoint into a membership oracle.

+`internal/docs/auth.go`), a normalized allowlist checked by both

+`rauth.Config.Authorize` and `admissionMailer`. The mailer suppresses delivery

+for refused addresses and failed grant lookups. Classification and rate limits

+run independently of admission, so an OAuth start does not disclose membership.

+Refused email starts still receive `303 ?sent=1`; their expiring tokens are never

+delivered. Callback authorization and per-request admission remain required.

+Detected Keymail addresses use OAuth, as §8.1 requires and Paul reaffirmed on

+2026-09-09. `force=1` remains an explicit email fallback. The OAuth start ends

+with a `200` continuation page (meta refresh plus link), preserving the pending

+cookie and the existing CSP: Safari enforces `form-action` across redirects.

+Never widen that directive to arbitrary Keymail hosts. Account linking still

+requires a fresh provider `AuthTime`; an ordinary OAuth session is not enough.

Empty is allowed only in development (admits everyone, so `TestSignIn`

and `run-dev.sh` need no setup); anywhere else it is a startup refusal,

the same fail-closed shape `Origin`/`InstanceKey`/`Mailer` already use,

diff --git a/internal/docs/admission_test.go b/internal/docs/admission_test.go

index 2c11100..0620ac5 100644

--- a/internal/docs/admission_test.go

+++ b/internal/docs/admission_test.go

@@ -5,8 +5,6 @@ import (

"errors"

"net/http"

"net/http/httptest"

- "net/url"

- "strings"

"testing"

)

@@ -147,47 +145,21 @@ func TestAdmitsFailsClosed(t *testing.T) {

}

-// gateBegin fails closed too, and nothing pinned it.

-//

-// A mutation flipping its condition to `err != nil || admitted` passed the

-// whole suite. Under it, a database stall turns POST /signin into an open

-// relay: every address a stranger types reaches Begin and gets a magic

-// link mailed. Config.AllowedAddresses' own comment calls gateBegin "the

-// wiring that actually matters" for exactly that, and admits() would still

-// refuse the session at callback — but the mail is already out, sent by us,

-// to an address we chose to accept. (Re-review 4, finding 1.)

-func TestGateBeginFailsClosed(t *testing.T) {

+// Delivery fails closed even when the grant store is unavailable.

+func TestAdmissionMailerFailsClosed(t *testing.T) {

list := allowlist{

- addrs: map[string]struct{}{"alice@example.com": {}},

- granted: func(string) (bool, error) {

- return false, errors.New("database is stalled")

- },

+ addrs: map[string]struct{}{"alice@example.com": {}},

+ granted: func(string) (bool, error) { return false, errors.New("database is stalled") },

}

- for _, c := range []struct {

- name string

- address string

- want bool

- }{

- {"a stranger during a failed check is refused", "mallory@example.com", false},

- {"a member is unaffected, the check never runs for them", "alice@example.com", true},

- } {

- t.Run(c.name, func(t *testing.T) {

- reached := false

- h := gateBegin(func(w http.ResponseWriter, r *http.Request) {

- reached = true

- }, list, "/signin")

- req := httptest.NewRequest(http.MethodPost, "/signin",

- strings.NewReader(url.Values{"address": {c.address}}.Encode()))

- req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

- h(httptest.NewRecorder(), req)

- if reached != c.want {

- t.Errorf("reached Begin = %v, want %v — a mail %s sent", reached, c.want,

- map[bool]string{true: "was not", false: "was"}[reached])

- }

- })

+ recorder := &recordingMailer{next: loggingMailerStub{}}

+ sender := admissionMailer{next: recorder, list: list}

+ for _, address := range []string{"mallory@example.com", "alice@example.com"} {

+ if err := sender.Send(t.Context(), address, "subject", "body"); err != nil {

+ t.Fatal(err)

+ }

+ if recorder.sends != 1 || recorder.lastTo != "alice@example.com" {

+ t.Fatalf("delivery count=%d recipient=%q", recorder.sends, recorder.lastTo)

}

diff --git a/internal/docs/app.go b/internal/docs/app.go

index 26378fe..1e2e5aa 100644

--- a/internal/docs/app.go

+++ b/internal/docs/app.go

@@ -144,20 +144,9 @@ type Config struct {

// normalizeAddress) — a real config file has spaces after its

// commas, and people type capitals into an email field.

- // This is wired to TWO places, deliberately, not one:

- // rauth.Config.Authorize (newAuthPlugin, auth.go) gates admission,

- // and gateBegin (auth.go) gates POST /signin itself, ahead of

- // rauth.Auth.Begin. The second wiring is the one that actually

- // matters: rastrillo/auth's Authorize hook fires only in its own

- // admit() (rastrillo/auth's handlers.go), which runs after Begin

- // has already mailed the link, so Authorize alone defends the

- // session but not the mail — an Authorize-only fix would leave

- // this app's own SES sending identity an open relay for any

- // address a stranger cares to type into the sign-in form (the

- // defect Task 4 exists to close; credited to the CARLOS Sheets

- // session's finding on their own deployed instance). gateBegin's

- // short-circuit is what actually stops that: a non-permitted

- // address never reaches Begin, so no mail is ever sent for it.

+ // admissionMailer restricts email delivery, and rauth.Config.Authorize

+ // checks verified identities before minting a session. Classification

+ // does not depend on admission, so an OAuth start cannot reveal membership.

// Empty is allowed only when Env is "development"

// (resolveAllowlist below defaults that to "admit everyone", so

@@ -462,13 +451,6 @@ func New(cfg Config) (*App, error) {

return nil, fmt.Errorf("docs: build renderer: %w", err)

}

- // auth.go: rastrillo's magic-link plugin, mounted per its own

- // SKILL.md contract (POST /signin, GET /auth/callback, GET

- // /auth/verify, POST /signout) — never a passkey second factor;

- // see spec §8.1 and auth.go's own doc comment for why. list also

- // becomes rauth.Config.Authorize here (admission), and — separately,

- // below — gateBegin's argument (delivery); see AllowedAddresses'

- // own doc comment on Config for why both wirings exist.

authPlugin, err := newAuthPlugin(cfg, d.Writer(), instanceKey, mailer, list, logger)

if err != nil {

rt.Close(context.Background())

@@ -514,17 +496,7 @@ func New(cfg Config) (*App, error) {

} else {

home.mountPublic(r)

}

- // gateBegin, not authPlugin.Begin directly: see AllowedAddresses'

- // own doc comment on Config, and gateBegin's own doc comment

- // (auth.go), for why the allowlist has to sit in front of Begin

- // itself rather than only behind it as rauth.Config.Authorize.

- // "/signin" here must keep matching rauth's own default

- // SigninPath (auth.New's own zero-value default) — newAuthPlugin

- // never overrides Config.SigninPath, and nothing else in this file

- // does either, so the two literals staying equal is not a

- // coincidence needing its own sync mechanism, only something a

- // future SigninPath override would have to touch in both places.

- r.Post("/signin", gateBegin(authPlugin.Begin, list, "/signin"))

+ r.Post("/signin", h.beginSignin(authPlugin.Begin))

r.Get("/auth/callback", localAuthGuard(home, authPlugin.Callback))

r.Get("/auth/verify", localAuthGuard(home, authPlugin.Verify))

// The published document, and the only route here no session guards.

diff --git a/internal/docs/auth.go b/internal/docs/auth.go

index a9bf757..ae36017 100644

--- a/internal/docs/auth.go

+++ b/internal/docs/auth.go

@@ -1,20 +1,5 @@

-// Per spec §8.1 (reframed the day this task started): a single

-// instance signs people in itself, permanently, not as a stopgap for

-// a shared home that does not exist yet — a lone instance has no

-// "one person, many teams" problem, which is the problem a shared

-// home would exist to solve. It does so with rastrillo/auth's

-// turnkey magic-link plugin (SKILL.md §5, docs/site/magic-links.md

-// in the rastrillo module): an emailed, single-use link, no password,

-// no credential enrolled.

-//

-// Deliberately magic link only, never a passkey second factor:

-// passkey.Config.Origin derives its relying party ID from this

-// instance's own host, so a person signed into three team instances

-// would enrol three separate passkeys under this app — exactly the

-// per-team credential sprawl §8.1's ruling forbids. A magic link

-// enrols no credential at all, so it creates no relying party and

-// violates nothing. Task 1's brief is explicit that this is a design

-// ruling, not an oversight to "improve" on later.

+// Instance-local sign-in uses Keymail OAuth when detected and email otherwise.

+// Credentials remain at Keymail; Docs never enrolls a per-instance passkey.

package docs

import (

@@ -111,7 +96,7 @@ type recordingMailer struct {

mu sync.Mutex

next mail.Sender

// lastTo is recorded alongside lastBody, and it is not decoration.

- // gateBegin answers a refused address with exactly the 303 ?sent=1 a

+ // admissionMailer leaves a refused address with exactly the 303 ?sent=1 a

// real one gets — deliberately, so the endpoint is not a membership

// oracle — so a caller that reads only lastBody cannot tell "a link

// was mailed to this address" from "somebody else's link is still

@@ -170,7 +155,7 @@ type allowlist struct {

// the allowlist alone, and any construction before the store exists).

// It exists because $DOCS_ALLOWED_ADDRESSES gates who may receive a

- // magic link at all, and gateBegin deliberately answers identically for

+ // magic link at all, and suppressed delivery answers identically for

// a refused address so this endpoint is not a membership oracle.

// Without this clause, sharing a document to somebody outside the list

// produces a grant that can never be redeemed: the sharer sees success,

@@ -256,9 +241,8 @@ func (a allowlist) admits(address string) bool {

// Refusal, not "admit nobody" — a real choice, made deliberately, not

// the shape everything else in this file happens to use. An instance

// that silently admitted nobody would boot looking entirely healthy:

-// /signin renders, POST /signin answers its ordinary 303 ?sent=1 on

-// every submission (gateBegin's whole point is that a refused

-// submission and a genuine one are byte-identical), and nothing would

+// /signin renders and email submissions answer 303 ?sent=1 whether

+// delivery was permitted or suppressed. Nothing would

// ever surface the mistake short of a person noticing that no sign-in

// link ever arrives — on an app whose entire premise is one

// instance, one team, where "nobody configured" is exactly the state

@@ -290,87 +274,23 @@ func resolveAllowlist(cfg Config) (allowlist, error) {

return allowlist{}, fmt.Errorf("docs: $DOCS_ALLOWED_ADDRESSES is empty and Env is %q, not %q — refusing to start with no allowlist configured: an instance-per-team app admitting nobody would look identical to one working correctly (see resolveAllowlist's own doc comment), so this refuses rather than boots quietly wrong; set $DOCS_ALLOWED_ADDRESSES to this team's address(es)", cfg.Env, "development")

}

-// gateBegin wraps rauth.Auth.Begin so a non-permitted address never

-// reaches it at all — the actual fix for the defect Config.

-// AllowedAddresses' own doc comment describes: Begin is what sends

-// the mail, and rauth.Config.Authorize (wired in newAuthPlugin below,

-// too) is checked only afterward, inside rauth's own admit(), which

-// runs at the callback once a link has already been mailed and

-// clicked. Gating only Authorize would leave this app's own mail

-// sending identity an open relay for any address a stranger types

-// into the sign-in form; gating here means a non-permitted address

-// causes rauth's Begin to never run, so no mail is ever sent for it.

-//

-// It reads the submitted address with r.FormValue exactly the way

-// rauth's own Begin does (rastrillo/auth's handlers.go), so by the

-// time next (Begin, for a permitted address) reads it again,

-// net/http's request.ParseForm has already cached the parse —

-// ParseForm is documented idempotent, so parsing twice costs nothing

-// and cannot disagree with itself.

-//

-// A non-permitted address gets exactly the response Begin's own

-// success path sends: the same 303, to the same signinPath+"?sent=1",

-// built the same way (http.Redirect). That is deliberate, not merely

-// convenient: any observable difference — a different status, a

-// different body, even a differently-cased query string — would turn

-// this endpoint into a membership oracle, letting an attacker learn

-// which addresses this instance's allowlist contains by watching how

-// its response to each one differs. See

-// TestSigninResponseIdenticalForNonPermittedAddress (auth_test.go),

-// which asserts the two responses byte-for-byte, not merely "both are

-// 303".

-//

-// That guarantee only held for a non-keymail address: on the

-// permitted branch, this used to hand the request to Begin exactly as

-// received, and Begin auto-upgrades a permitted address that resolves

-// to a claimed keymail inbox to the keymail ceremony — a 303 to the

-// keymail authorize URL, not signinPath+"?sent=1". A non-permitted

-// address never reaches Begin at all, so it always got ?sent=1. That

-// reopened the same oracle one level narrower: an attacker who knows

-// (or suspects) an address is keymail-hosted could tell "on the

-// allowlist" from "not" by whether the response redirected to keymail

-// or to ?sent=1.

-//

-// force=1 is what selects the plain-magic-link path over that

-// classification (rastrillo/auth's handlers.go), and this app never

-// wants the keymail ceremony in the first place — spec §8.1's ruling,

-// this file's own package comment, and signin.html's form, which

-// already hardcodes a hidden force=1 field so no request built by

-// this app's own UI ever takes the keymail branch. Setting it here

-// too, unconditionally, on every permitted request — regardless of

-// what the caller's own form said — makes that the actual guarantee

-// for any request this handler ever forwards, not just the ones sent

-// through this app's form: every permitted address takes the identical

-// plain-magic-link path Begin would take for force=1, so the keymail

-// branch is never reached and never has a distinct response to leak

-// through. This costs nothing a legitimate caller wanted: it matches

-// what the UI already always sends, and it is not a behavior change

-// this app would ever notice.

-func gateBegin(next http.HandlerFunc, list allowlist, signinPath string) http.HandlerFunc {

- return func(w http.ResponseWriter, r *http.Request) {

- // A malformed body (ParseForm's error) reads back as address

- // == "" below, which is never on a non-empty allowlist and is

- // on an admit-everyone one only in development — the same

- // answer Begin itself would eventually reach for the same

- // malformed request, just reached here instead, before any

- // mail could be sent over it.

- _ = r.ParseForm()

- // A failed check refuses here, unlike requireAdmission's 503. At

- // sign-in there is nothing to lose by refusing — no unsaved work,

- // and the person can try again — and gateBegin's whole contract is

- // that a refusal is indistinguishable from a success, which a 503

- // would break by telling a stranger the check happened.

- admitted, err := list.allows(r.FormValue("address"))

- if err == nil && admitted {

- if r.Form == nil {

- r.Form = url.Values{}

- }

- r.Form.Set("force", "1")

- next(w, r)

- return

- }

- http.Redirect(w, r, signinPath+"?sent=1", http.StatusSeeOther)

+// admissionMailer restricts delivery without exposing membership through the

+// choice of sign-in ceremony. All addresses reach classification and the same

+// rate limits; only admitted addresses receive mail. Callback admission remains

+// authoritative, including for tokens minted before an address was removed.

+// The underlying flow stores an expiring token before Send; a suppressed token

+// is never delivered. A failed grant lookup suppresses delivery too.

+type admissionMailer struct {

+ next mail.Sender

+ list allowlist

+func (m admissionMailer) Send(ctx context.Context, to, subject, body string) error {

+ admitted, err := m.list.allows(to)

+ if err != nil || !admitted {

+ return nil

}

+ return m.next.Send(ctx, to, subject, body)

}

// requireAdmission re-asks the front-door question on every request, and

@@ -481,16 +401,9 @@ func requireAdmissionOf(list allowlist, signinPath string, identity func(*http.R

// decision, InstanceKey and Mailer already resolved and fail-closed

// by resolveInstanceKey/buildMailer above. Authorize is list.admits —

// the fail-closed predicate form, because rauth's hook has no way to

-// say "could not tell" and refusing is the safe answer here —

-// belt-and-suspenders alongside gateBegin's own gate on POST /signin:

-// this defends admission itself (a link somehow minted for a

-// non-permitted address — a future code path, or a link mailed before

-// the allowlist changed — still cannot mint a session), even though

-// gateBegin's own gate is what actually stops the mail from going out

-// in the first place; see Config.AllowedAddresses' own doc comment

-// for why both wirings exist rather than either alone. SecondFactor

-// is left nil on purpose: no passkey step-up, ever, for the reason

-// this file's own package comment gives.

+// say "could not tell" and refusing is the safe answer here.

+// Delivery and callback admission are checked independently. No local passkey

+// second factor is enrolled; Keymail verifies credentials at its own origin.

func newAuthPlugin(cfg Config, writer *sql.DB, instanceKey string, mailer mail.Sender, list allowlist, logger *slog.Logger) (*rauth.Auth, error) {

signedInPath := "/"

if cfg.Home != nil {

@@ -501,7 +414,7 @@ func newAuthPlugin(cfg Config, writer *sql.DB, instanceKey string, mailer mail.S

DB: writer,

Origin: cfg.Origin,

InstanceKey: instanceKey,

- Mailer: mailer,

+ Mailer: admissionMailer{next: mailer, list: list},

Authorize: list.admits,

Logger: logger,

})

@@ -512,6 +425,8 @@ func newAuthPlugin(cfg Config, writer *sql.DB, instanceKey string, mailer mail.S

// §5/magic-links.md: "the sign-in page stays yours" — they report

// outcomes by redirecting back here with a query this handler reads.

type signinPage struct {

+ ForceEmail bool

+ ContinueURL string

Title string

Body string

AddressLabel string

@@ -537,6 +452,7 @@ type signinPage struct {

func (h *handlers) signin(w http.ResponseWriter, r *http.Request) {

q := r.URL.Query()

data := signinPage{

+ ForceEmail: q.Get("force") != "",

Title: rastrillo.T(r, "docs.signin.title"),

Body: rastrillo.T(r, "docs.signin.body"),

AddressLabel: rastrillo.T(r, "docs.signin.address.label"),

diff --git a/internal/docs/auth_allowlist_test.go b/internal/docs/auth_allowlist_test.go

index ddb20ef..6b6c943 100644

--- a/internal/docs/auth_allowlist_test.go

+++ b/internal/docs/auth_allowlist_test.go

@@ -59,9 +59,7 @@ func newAllowlistedTestApp(t *testing.T, allowed string) *App {

// uses to skip signin's own keymail classification (which would

// otherwise make a real outbound DNS/HTTPS call per address, per

// signin's own README, entirely beside the point of what these tests

-// check). force=1 does not, and must not, change gateBegin's own

-// decision: gateBegin looks only at the address field, not force —

-// see gateBegin's own doc comment (auth.go).

+// check). force=1 selects email but does not bypass admissionMailer.

func postSignin(app *App, address string) *httptest.ResponseRecorder {

form := url.Values{"address": {address}, "force": {"1"}}

req := httptest.NewRequest(http.MethodPost, "/signin", strings.NewReader(form.Encode()))

@@ -95,7 +93,7 @@ func lastMailBody(app *App) string {

// is the test whose absence let that ship: every existing test in

// this package (before this task) proved something about admission,

// and none of them ever looked at whether a Send happened. Before

-// gateBegin existed, this test failed exactly this way — see this

+// the delivery gate existed, this test failed exactly this way — see this

// file's own commit message for how that was confirmed live, not

// assumed.

func TestNonPermittedAddressSendsNoMail(t *testing.T) {

diff --git a/internal/docs/auth_keymail_test.go b/internal/docs/auth_keymail_test.go

new file mode 100644

index 0000000..e360963

--- /dev/null

+++ b/internal/docs/auth_keymail_test.go

@@ -0,0 +1,257 @@

+package docs

+import (

+ "context"

+ "crypto/sha256"

+ "encoding/base64"

+ "encoding/json"

+ "fmt"

+ "html"

+ "io"

+ "net"

+ "net/http"

+ "net/http/cookiejar"

+ "net/http/httptest"

+ "net/url"

+ "os"

+ "os/exec"

+ "regexp"

+ "strings"

+ "sync"

+ "testing"

+ "time"

+// Process isolation keeps the resolver and default transport overrides out of

+// concurrent tests. Every DNS packet and HTTPS request stays in this fixture.

+func TestKeymailOAuthActualIntegration(t *testing.T) {

+ if os.Getenv("DOCS_KEYMAIL_TEST_CHILD") == "1" {

+ keymailOAuthChild(t)

+ return

+ }

+ ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second)

+ defer cancel()

+ cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestKeymailOAuthActualIntegration$", "-test.count=1", "-test.v")

+ cmd.Dir = t.TempDir()

+ for _, e := range os.Environ() {

+ if strings.HasPrefix(e, "DOCS_") || strings.HasPrefix(e, "HOME_") || strings.HasPrefix(e, "CARLOS_") || strings.HasPrefix(e, "AWS_") {

+ continue

+ }

+ cmd.Env = append(cmd.Env, e)

+ }

+ cmd.Env = append(cmd.Env, "DOCS_KEYMAIL_TEST_CHILD=1")

+ out, err := cmd.CombinedOutput()

+ if err != nil {

+ t.Fatalf("isolated OAuth test: %v\n%s", err, out)

+ }

+type keymailTestTransport func(*http.Request) (*http.Response, error)

+func (f keymailTestTransport) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }

+func keymailOAuthChild(t *testing.T) {

+ dns, err := net.ListenPacket("udp", "127.0.0.1:0")

+ if err != nil {

+ t.Fatal(err)

+ }

+ defer dns.Close()

+ go func() {

+ b := make([]byte, 4096)

+ for {

+ n, a, e := dns.ReadFrom(b)

+ if e != nil {

+ return

+ }

+ if n < 12 {

+ continue

+ }

+ reply := append([]byte(nil), b[:n]...)

+ reply[2] = 0x81

+ reply[3] = 0x83

+ for i := 6; i < 12; i++ {

+ reply[i] = 0

+ }

+ _, _ = dns.WriteTo(reply, a)

+ }

+ }()

+ net.DefaultResolver = &net.Resolver{PreferGo: true, Dial: func(ctx context.Context, _, _ string) (net.Conn, error) {

+ return (&net.Dialer{}).DialContext(ctx, "udp", dns.LocalAddr().String())

+ }}

+ const origin = "https://docs.oauth.test"

+ const provider = "mail.oauth.test"

+ for _, tc := range []struct {

+ name, address, returned string

+ admitted, detected, badState, badPKCE bool

+ }{

+ {name: "allowed OAuth", address: "member@mail.oauth.test", admitted: true, detected: true},

+ {name: "denied OAuth", address: "outsider@mail.oauth.test", detected: true},

+ {name: "wrong address", address: "member@mail.oauth.test", returned: "outsider@mail.oauth.test", admitted: true, detected: true},

+ {name: "wrong state", address: "member@mail.oauth.test", admitted: true, detected: true, badState: true},

+ {name: "wrong PKCE", address: "member@mail.oauth.test", admitted: true, detected: true, badPKCE: true},

+ {name: "ordinary mail", address: "member@mail.oauth.test", admitted: true},

+ {name: "denied ordinary mail", address: "outsider@mail.oauth.test"},

+ } {

+ t.Run(tc.name, func(t *testing.T) {

+ var mu sync.Mutex

+ calls := map[string]int{}

+ challenge := ""

+ exchanges := 0

+ http.DefaultTransport = keymailTestTransport(func(r *http.Request) (*http.Response, error) {

+ mu.Lock()

+ defer mu.Unlock()

+ if r.URL.Scheme != "https" || r.URL.Host != provider {

+ t.Errorf("unexpected outbound request %s", r.URL)

+ return nil, fmt.Errorf("fixture refuses outbound request")

+ }

+ calls[r.URL.Path]++

+ status := http.StatusOK

+ var body any

+ switch r.URL.Path {

+ case "/.well-known/keymail":

+ body = map[string]any{"version": "1", "reauth": true}

+ case "/api/federation/lookup":

+ if r.URL.Query().Get("addr") != tc.address {

+ t.Errorf("wrong lookup address")

+ }

+ if !tc.detected {

+ status = http.StatusNotFound

+ }

+ body = map[string]bool{"ok": tc.detected}

+ case "/api/oauth/token":

+ exchanges++

+ var in map[string]string

+ if r.Method != "POST" || json.NewDecoder(io.LimitReader(r.Body, 8192)).Decode(&in) != nil {

+ t.Error("invalid token request")

+ }

+ sum := sha256.Sum256([]byte(in["code_verifier"]))

+ if in["client_id"] != origin || in["redirect_uri"] != origin+"/auth/callback" || in["code"] != "one-code" || base64.RawURLEncoding.EncodeToString(sum[:]) != challenge || exchanges != 1 {

+ status = http.StatusBadRequest

+ body = map[string]string{"error": "invalid grant"}

+ } else {

+ address := tc.address

+ if tc.returned != "" {

+ address = tc.returned

+ }

+ body = map[string]any{"access_token": "synthetic-token", "token_type": "bearer", "address": address, "auth_time": time.Now().Unix()}

+ }

+ default:

+ t.Errorf("unexpected endpoint %s", r.URL.Path)

+ status = http.StatusNotFound

+ }

+ raw, _ := json.Marshal(body)

+ return &http.Response{StatusCode: status, Header: make(http.Header), Body: io.NopCloser(strings.NewReader(string(raw))), Request: r}, nil

+ })

+ app, e := NewForTestWith(t, Config{Origin: origin, AllowedAddresses: "member@mail.oauth.test"})

+ if e != nil {

+ t.Fatal(e)

+ }

+ jar, _ := cookiejar.New(nil)

+ request := func(method, path, body string) *httptest.ResponseRecorder {

+ u, _ := url.Parse(origin + path)

+ req := httptest.NewRequest(method, u.String(), strings.NewReader(body))

+ req.Header.Set("Origin", origin)

+ req.Header.Set("Sec-Fetch-Site", "same-origin")

+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

+ for _, c := range jar.Cookies(u) {

+ req.AddCookie(c)

+ }

+ rec := httptest.NewRecorder()

+ app.Handler().ServeHTTP(rec, req)

+ jar.SetCookies(u, rec.Result().Cookies())

+ return rec

+ }

+ rec := request("POST", "/signin", url.Values{"address": {tc.address}}.Encode())

+ mu.Lock()

+ lookups := calls["/api/federation/lookup"]

+ mu.Unlock()

+ if lookups != 1 {

+ t.Fatalf("actual classifier lookup count=%d, status=%d", lookups, rec.Code)

+ }

+ if !tc.detected {

+ if rec.Code != 303 || rec.Header().Get("Location") != "/signin?sent=1" {

+ t.Fatalf("mail start: %d %s", rec.Code, rec.Header().Get("Location"))

+ }

+ app.mailRecorder.mu.Lock()

+ sent := app.mailRecorder.sends

+ app.mailRecorder.mu.Unlock()

+ want := 0

+ if tc.admitted {

+ want = 1

+ }

+ if sent != want {

+ t.Fatalf("mail deliveries=%d want%d", sent, want)

+ }

+ return

+ }

+ if rec.Code != 200 || rec.Header().Get("Location") != "" {

+ t.Fatalf("OAuth continuation: %d", rec.Code)

+ }

+ var target *url.URL

+ for _, m := range regexp.MustCompile(`href="([^"]+)"`).FindAllStringSubmatch(rec.Body.String(), -1) {

+ u, e := url.Parse(html.UnescapeString(m[1]))

+ if e == nil && u.Host == provider && u.Path == "/oauth/authorize" {

+ target = u

+ }

+ if target == nil {

+ t.Fatal("no generated OAuth authorize link")

+ }

+ q := target.Query()

+ if q.Get("client_id") != origin || q.Get("redirect_uri") != origin+"/auth/callback" || q.Get("code_challenge_method") != "S256" || q.Get("prompt") != "login" || q.Get("state") == "" {

+ t.Fatal("OAuth bindings missing")

+ }

+ mu.Lock()

+ challenge = q.Get("code_challenge")

+ if tc.badPKCE {

+ challenge = "wrong"

+ }

+ mu.Unlock()

+ state := q.Get("state")

+ if tc.badState {

+ state = "wrong"

+ }

+ callback := request("GET", "/auth/callback?"+url.Values{"state": {state}, "code": {"one-code"}}.Encode(), "")

+ allowed := tc.admitted && tc.returned == "" && !tc.badState && !tc.badPKCE

+ home := request("GET", "/", "")

+ if allowed {

+ if callback.Code != 303 || home.Code != 200 {

+ t.Fatalf("authorized callback=%d home=%d", callback.Code, home.Code)

+ }

+ } else if home.Code == 200 {

+ t.Fatal("refused OAuth created admitted session")

+ }

+ if !allowed {

+ u, _ := url.Parse(origin)

+ for _, c := range jar.Cookies(u) {

+ if strings.Contains(c.Name, "session") {

+ t.Fatal("refused OAuth minted session")

+ }

+ app.mailRecorder.mu.Lock()

+ sent := app.mailRecorder.sends

+ app.mailRecorder.mu.Unlock()

+ if sent != 0 {

+ t.Fatalf("OAuth sent %d emails", sent)

+ }

+ mu.Lock()

+ got := exchanges

+ mu.Unlock()

+ want := 1

+ if tc.badState {

+ want = 0

+ }

+ if got != want {

+ t.Fatalf("token exchanges=%d want%d", got, want)

+ }

+ // Pending state is cleared after callback: replay cannot reach exchange.

+ request("GET", "/auth/callback?"+url.Values{"state": {q.Get("state")}, "code": {"one-code"}}.Encode(), "")

+ mu.Lock()

+ defer mu.Unlock()

+ if exchanges != want {

+ t.Fatal("callback replay reached token endpoint")

+ }

+ })

+ }

diff --git a/internal/docs/auth_navigation.go b/internal/docs/auth_navigation.go

new file mode 100644

index 0000000..e8a83cb

--- /dev/null

+++ b/internal/docs/auth_navigation.go

@@ -0,0 +1,92 @@

+package docs

+import (

+ "bytes"

+ "net/http"

+ "net/url"

+ "strings"

+ "github.com/carlosframework/rastrillo"

+// End the POST with a document before navigating to an identity provider.

+// Safari applies form-action to a redirect chain, including intermediate GETs.

+// The continuation keeps the original CSP and works without JavaScript.

+func (h *handlers) beginSignin(next http.HandlerFunc) http.HandlerFunc {

+ return func(w http.ResponseWriter, r *http.Request) {

+ result := &signinResponse{header: make(http.Header)}

+ next(result, r)

+ destination := result.header.Get("Location")

+ external := destination != "" && !strings.HasPrefix(destination, "/")

+ if result.status == http.StatusSeeOther && external {

+ if !keymailDestination(destination, h.origin) {

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusBadGateway)

+ return

+ }

+ for _, cookie := range result.header.Values("Set-Cookie") {

+ w.Header().Add("Set-Cookie", cookie)

+ }

+ w.Header().Set("Cache-Control", "no-store")

+ w.Header().Set("Referrer-Policy", "no-referrer")

+ data := signinPage{

+ Title: rastrillo.T(r, "docs.signin.title"),

+ SubmitLabel: rastrillo.T(r, "docs.signin.keymail"),

+ ContinueURL: destination,

+ }

+ if err := h.render.page(w, r, "signin", data); err != nil {

+ h.logger.Error("docs: render Keymail continuation", "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ }

+ return

+ }

+ for name, values := range result.header {

+ for _, value := range values {

+ w.Header().Add(name, value)

+ }

+ if result.status == 0 {

+ result.status = http.StatusOK

+ }

+ w.WriteHeader(result.status)

+ _, _ = w.Write(result.body.Bytes())

+ }

+// The destination comes only from the identity plugin's server-generated

+// redirect, never from return_to or another submitted URL.

+func keymailDestination(destination, origin string) bool {

+ u, err := url.Parse(destination)

+ if err != nil || u.Scheme != "https" || u.Host == "" || u.User != nil || u.Fragment != "" || u.Path != "/oauth/authorize" || u.RawPath != "" {

+ return false

+ }

+ q, err := url.ParseQuery(u.RawQuery)

+ if err != nil {

+ return false

+ }

+ for _, key := range []string{"client_id", "redirect_uri", "scope", "code_challenge_method", "state", "code_challenge"} {

+ if len(q[key]) != 1 || q.Get(key) == "" {

+ return false

+ }

+ origin = strings.TrimSuffix(origin, "/")

+ return q.Get("client_id") == origin && q.Get("redirect_uri") == origin+"/auth/callback" && q.Get("scope") == "identify" && q.Get("code_challenge_method") == "S256"

+type signinResponse struct {

+ header http.Header

+ status int

+ body bytes.Buffer

+func (w *signinResponse) Header() http.Header { return w.header }

+func (w *signinResponse) WriteHeader(status int) {

+ if w.status == 0 {

+ w.status = status

+ }

+func (w *signinResponse) Write(p []byte) (int, error) {

+ if w.status == 0 {

+ w.WriteHeader(http.StatusOK)

+ }

+ return w.body.Write(p)

diff --git a/internal/docs/auth_navigation_test.go b/internal/docs/auth_navigation_test.go

new file mode 100644

index 0000000..a1f9584

--- /dev/null

+++ b/internal/docs/auth_navigation_test.go

@@ -0,0 +1,100 @@

+package docs

+import (

+ "encoding/json"

+ "html"

+ "net/http"

+ "net/http/httptest"

+ "net/url"

+ "os"

+ "path/filepath"

+ "strings"

+ "testing"

+ "github.com/keymaildev/signin"

+func TestKeymailContinuation(t *testing.T) {

+ app := newAllowlistedTestApp(t, "alice@example.test")

+ app.handlers.origin = "https://docs.example.test"

+ destination := (&signin.Keymail{Base: "https://keymail.example.test", Origin: app.handlers.origin}).AuthorizeURL("state", "challenge", true)

+ handler := app.handlers.beginSignin(func(w http.ResponseWriter, r *http.Request) {

+ http.SetCookie(w, &http.Cookie{Name: "__Host-pending", Value: "opaque", Path: "/", Secure: true, HttpOnly: true, SameSite: http.SameSiteLaxMode})

+ http.Redirect(w, r, destination, http.StatusSeeOther)

+ })

+ rec := httptest.NewRecorder()

+ policy := homeFormPolicy("https://home.example.test")

+ rec.Header().Set("Content-Security-Policy", policy)

+ handler(rec, httptest.NewRequest("POST", "/signin", nil))

+ if rec.Code != 200 || rec.Header().Get("Location") != "" {

+ t.Fatalf("status=%d location=%q", rec.Code, rec.Header().Get("Location"))

+ }

+ if rec.Header().Get("Content-Security-Policy") != policy {

+ t.Fatal("CSP changed")

+ }

+ if rec.Header().Get("Cache-Control") != "no-store" || rec.Header().Get("Referrer-Policy") != "no-referrer" {

+ t.Fatal("continuation must not cache or disclose the referring URL")

+ }

+ cookies := rec.Result().Cookies()

+ if len(cookies) != 1 || cookies[0].Name != "__Host-pending" || cookies[0].Value != "opaque" || !cookies[0].Secure || !cookies[0].HttpOnly || cookies[0].SameSite != http.SameSiteLaxMode {

+ t.Fatalf("lost pending cookie: %v", cookies)

+ }

+ body := html.UnescapeString(rec.Body.String())

+ for _, part := range []string{`http-equiv="refresh"`, `content="0;url=` + destination + `"`, `href="` + destination + `"`} {

+ if !strings.Contains(body, part) {

+ t.Fatalf("missing continuation component %q", part)

+ }

+ if strings.Contains(body, `action="/signin"`) {

+ t.Fatal("continuation rendered another sign-in form")

+ }

+ if dir := os.Getenv("DOCS_OAUTH_BROWSER_FIXTURE"); dir != "" {

+ if err := os.MkdirAll(dir, 0700); err != nil {

+ t.Fatal(err)

+ }

+ data, err := json.Marshal(map[string]any{"body": rec.Body.String(), "headers": rec.Header(), "destination": destination})

+ if err != nil {

+ t.Fatal(err)

+ }

+ if err := os.WriteFile(filepath.Join(dir, "continuation.json"), data, 0600); err != nil {

+ t.Fatal(err)

+ }

+func TestKeymailDestinationRefusals(t *testing.T) {

+ origin := "https://docs.example.test"

+ valid := (&signin.Keymail{Base: "https://keymail.example.test", Origin: origin}).AuthorizeURL("state", "challenge", false)

+ for _, target := range []string{

+ "javascript:alert(1)", "//keymail.example.test/oauth/authorize", strings.Replace(valid, "https:", "http:", 1),

+ strings.Replace(valid, "keymail.example.test", "user@keymail.example.test", 1), valid + "#fragment", valid + "&state=duplicate",

+ strings.Replace(valid, "/oauth/authorize", "/other", 1), strings.ReplaceAll(valid, url.QueryEscape(origin), url.QueryEscape("https://foreign.example.test")),

+ strings.Replace(valid, "S256", "plain", 1),

+ } {

+ if keymailDestination(target, origin) {

+ t.Fatalf("accepted %q", target)

+ }

+ app := newAllowlistedTestApp(t, "alice@example.test")

+ app.handlers.origin = origin

+ rec := httptest.NewRecorder()

+ app.handlers.beginSignin(func(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "https://foreign.example.test", 303) })(rec, httptest.NewRequest("POST", "/signin", nil))

+ if rec.Code != 502 || rec.Header().Get("Location") != "" {

+ t.Fatalf("invalid redirect status=%d location=%q", rec.Code, rec.Header().Get("Location"))

+ }

+func TestSigninEmailFallbackIsExplicit(t *testing.T) {

+ app := newAllowlistedTestApp(t, "alice@example.test")

+ for _, path := range []string{"/signin", "/signin?force=1&err=keymail"} {

+ rec := httptest.NewRecorder()

+ app.Handler().ServeHTTP(rec, httptest.NewRequest("GET", path, nil))

+ if rec.Code != 200 {

+ t.Fatal(rec.Code)

+ }

+ forced := strings.Contains(rec.Body.String(), `name="force" value="1"`)

+ if forced != strings.Contains(path, "force=1") {

+ t.Fatalf("email force for %s = %v", path, forced)

+ }

diff --git a/internal/docs/render.go b/internal/docs/render.go

index 8122c49..e7dea7a 100644

--- a/internal/docs/render.go

+++ b/internal/docs/render.go

@@ -244,9 +244,10 @@ var Catalog = rastrillo.Catalog{

// either (rastrillo/auth's own handlers already refuse to be an

// oracle between "wrong" and "unknown").

"docs.signin.title": "Sign in",

- "docs.signin.body": "Enter your email address and we'll send you a link to sign in.",

+ "docs.signin.body": "Enter your email address to sign in.",

"docs.signin.address.label": "Email address",

- "docs.signin.submit": "Send link",

+ "docs.signin.submit": "Continue",

+ "docs.signin.keymail": "Continue to Keymail",

"docs.signin.sent": "Check your email for a link to sign in.",

"docs.signin.error.rate": "Too many attempts. Wait a few minutes and try again.",

"docs.signin.error.address": "That doesn't look like a valid email address.",

diff --git a/internal/docs/views/signin.html b/internal/docs/views/signin.html

index 0c173fb..109ef5e 100644

--- a/internal/docs/views/signin.html

+++ b/internal/docs/views/signin.html

@@ -8,16 +8,6 @@

already-translated strings; this template only decides whether

to show them.

- The form itself POSTs straight to rastrillo/auth's own Begin

- (mounted at POST /signin, app.go) — there is no handler of this

- app's own in that path at all. force=1: this app never offers a

- passkey or keymail ceremony (spec §8.1's ruling — see auth.go's

- package comment), so there is nothing for a person to choose

- between; force always sends the plain magic link, skipping

- signin's classification network call outright rather than

- silently paying its latency for a decision this app never acts

- on.

Writes the bare rst-* attribute grammar throughout, same

convention as list.html/show.html and the same test

(render_test.go's TestOurTemplatesUseTheAttributeGrammar) holds

@@ -125,14 +115,19 @@

{{template "callout" dict "Body" .ErrorMessage "Tone" "negative" "Alert" true}}

- <div rst-box-head>

+ {{if .Body}}<div rst-box-head>

- </div>

+ </div>{{end}}

+ {{if .ContinueURL}}

+ <meta http-equiv="refresh" content="0;url={{.ContinueURL}}">

+ <a rst-btn="primary" href="{{.ContinueURL}}">{{.SubmitLabel}}</a>

+ {{else}}

- <input type="hidden" name="force" value="1">

+ {{if .ForceEmail}}<input type="hidden" name="force" value="1">{{end}}

{{template "field" dict "ID" "signin-address" "Name" "address" "Type" "email" "Label" .AddressLabel "Autocomplete" "email" "Required" true}}

<button rst-btn="primary" type="submit">{{.SubmitLabel}}</button>

</form>

+ {{end}}

</div>