diff --git a/cmd/myday/main_test.go b/cmd/myday/main_test.go index 5823631..b70356a 100644 --- a/cmd/myday/main_test.go +++ b/cmd/myday/main_test.go @@ -23,7 +23,7 @@ import ( // stubEnroller satisfies setup.Enroller — this test never runs a ceremony. type stubEnroller struct{} -func (stubEnroller) TOTPBegin(http.ResponseWriter, *http.Request, string) error { return nil } +func (stubEnroller) IssueTOTP(context.Context, string) (string, error) { return "", nil } func (stubEnroller) TOTPVerify(http.ResponseWriter, *http.Request, string) error { return nil } // setupYAML is a fresh-instance config: setup mode, every secret a placeholder. diff --git a/internal/core/auth/flow/flow.go b/internal/core/auth/flow/flow.go index 72b4830..1a83c4f 100644 --- a/internal/core/auth/flow/flow.go +++ b/internal/core/auth/flow/flow.go @@ -41,6 +41,11 @@ import ( // finish (no server-side table needed). const CeremonyCookieName = "myday_webauthn" +// errNotPending reports that a GUID does not map to a pending identity inside +// its enrollment window — the ceremony's "not found" (A1). It is unexported: +// only this package renders a response for it (TOTPBegin's 404). +var errNotPending = errors.New("flow: no pending identity") + // Deps assembles the flow's dependencies. type Deps struct { // Identities resolves GUIDs, activates identities and checks the window. @@ -223,23 +228,41 @@ func (a *Actions) EnrollFinish(w http.ResponseWriter, r *http.Request, guid stri return writeJSON(w, http.StatusOK, []byte(`{"status":"credential-stored"}`)) } -// TOTPBegin issues (and encrypts) the TOTP secret and returns the otpauth URL. -func (a *Actions) TOTPBegin(w http.ResponseWriter, r *http.Request, guid string) error { - ctx := r.Context() +// IssueTOTP issues (and encrypts) a fresh TOTP secret for the identity behind +// guid and returns the otpauth URL. It WRITES NO RESPONSE — the caller owns the +// reply: the JSON ceremony below (the WebAuthn client's contract), or SPEC-005's +// wizard, which is server-rendered and no-JS and renders the secret itself. +// +// A guid that does not resolve to a pending identity inside its bounded +// enrollment window reports errNotPending (mapped to 404 by TOTPBegin). +func (a *Actions) IssueTOTP(ctx context.Context, guid string) (string, error) { idn, ok, err := a.pendingInWindow(ctx, guid) if err != nil { - return err + return "", err } if !ok { - writeError(w, http.StatusNotFound, "not found") - return nil + return "", errNotPending } issued, err := a.d.TOTP.Begin(ctx, idn.ID, idn.GUID) if err != nil { - return err + return "", err } a.audit(ctx, "enroll.totp.begin", idn, "secret issued") - payload, err := json.Marshal(map[string]string{"otpauth_url": issued.URL}) + return issued.URL, nil +} + +// TOTPBegin answers the WebAuthn client with the otpauth URL as JSON — +// webauthn.js's contract, kept intact. It is IssueTOTP plus the JSON write. +func (a *Actions) TOTPBegin(w http.ResponseWriter, r *http.Request, guid string) error { + url, err := a.IssueTOTP(r.Context(), guid) + if err != nil { + if errors.Is(err, errNotPending) { + writeError(w, http.StatusNotFound, "not found") + return nil + } + return err + } + payload, err := json.Marshal(map[string]string{"otpauth_url": url}) if err != nil { return err } diff --git a/internal/core/server/enroll_login_test.go b/internal/core/server/enroll_login_test.go index 56ced62..e049e5e 100644 --- a/internal/core/server/enroll_login_test.go +++ b/internal/core/server/enroll_login_test.go @@ -399,7 +399,15 @@ func TestSpec006AC6_TOTPSecretEncryptedAndNeverLogged(t *testing.T) { } // 2) The otpauth URL is shown to the user (the one display) — but the - // secret never reached the log (A9). + // secret never reached the log (A9). The enroll ceremony answers the + // WebAuthn client (`webauthn.js`) with JSON, not a page (SPEC-006 §4): the + // setup wizard is the server-rendered exception (SPEC-005 §4). + if ct := rec.Header().Get("Content-Type"); !strings.HasPrefix(ct, "application/json") { + t.Errorf("totp.begin Content-Type = %q, want application/json (webauthn.js's contract)", ct) + } + if !strings.Contains(rec.Body.String(), `"otpauth_url"`) { + t.Error("totp.begin did not return the otpauth_url JSON field (webauthn.js's contract)") + } if !strings.Contains(rec.Body.String(), "otpauth://") { t.Error("totp.begin did not return an otpauth URL") } diff --git a/internal/core/server/setup_wizard_test.go b/internal/core/server/setup_wizard_test.go index ac2ce2f..b31738f 100644 --- a/internal/core/server/setup_wizard_test.go +++ b/internal/core/server/setup_wizard_test.go @@ -241,17 +241,40 @@ func (h *setupHarness) beginAdmin(t *testing.T) { // enrollAdmin runs step 2b — the TOTP-only ceremony (D-041) — and returns the // session id the flow minted on completion (SPEC-006 reused). +// +// The `begin` response must be the wizard's server-rendered TOTP PAGE, not the +// enroll flow's JSON API (`{"otpauth_url":…}` — that is webauthn.js's contract): +// the browser must never be shown raw JSON. This is the regression the fix +// guards against, so it is asserted here (content type + secret + verify form). func (h *setupHarness) enrollAdmin(t *testing.T) string { t.Helper() - if rec := h.post("admin.totp.begin", nil); rec.Code != http.StatusOK { - t.Fatalf("admin.totp.begin = %d, want 200", rec.Code) + begin := h.post("admin.totp.begin", nil) + if begin.Code != http.StatusOK { + t.Fatalf("admin.totp.begin = %d, want 200", begin.Code) + } + if ct := begin.Header().Get("Content-Type"); !strings.HasPrefix(ct, "text/html") { + t.Errorf("admin.totp.begin Content-Type = %q, want text/html (the wizard renders a page, not JSON)", ct) + } + body := begin.Body.String() + if strings.Contains(body, `"otpauth_url"`) { + t.Error("admin.totp.begin answered with the enroll flow's JSON, not the wizard's TOTP page") + } + if !strings.Contains(body, "otpauth://") { + t.Error("the TOTP page did not show the otpauth URL") + } + if !strings.Contains(body, "admin.totp.verify") { + t.Error("the TOTP page did not render the verify form") } secret, err := h.ids.TOTPSecret(h.ctx, 1) if err != nil { t.Fatalf("TOTPSecret: %v", err) } + if !strings.Contains(body, string(secret)) { + t.Error("the TOTP page did not show the issued secret") + } + code, err := pqotp.GenerateCode(string(secret), time.Now()) if err != nil { t.Fatalf("GenerateCode: %v", err) diff --git a/internal/core/setup/render.go b/internal/core/setup/render.go index 21021ca..b88afc9 100644 --- a/internal/core/setup/render.go +++ b/internal/core/setup/render.go @@ -3,6 +3,7 @@ package setup import ( "html" "net/http" + "net/url" "git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/web" ) @@ -45,6 +46,47 @@ func wizardPage(st step, token, domain string) web.Page { } } +// totpPage renders the wizard's TOTP step (SPEC-005 §4) once the secret has +// been issued: the otpauth URL, its human-readable secret for manual entry, and +// the verify form. The wizard is server-rendered and works with no JavaScript, +// so the secret is shown as text rather than encoded in a QR image; the page +// renders inside the shell (no inline scripts) and the verify form carries the +// signed pre-session CSRF token (A15). +func totpPage(token, otpauthURL string) web.Page { + c := html.EscapeString(token) + u := html.EscapeString(otpauthURL) + secret := html.EscapeString(secretFromOtpauth(otpauthURL)) + return web.Page{ + Title: "MyDay setup", + Guest: true, + CSRF: token, + Content: web.Markup(`

MyDay setup

+

Add the authenticator, then enter a code from it to finish.

` + + `

2. Enroll the admin

` + + `

A passkey can be added later, at first sign-in.

` + + `

Authenticator app

` + + `

Open this link in your authenticator app:

` + + `

` + u + `

` + + `

Or enter this secret by hand:

` + + `

` + secret + `

` + + `
` + + `` + + `` + + `
`), + } +} + +// secretFromOtpauth pulls the shared secret out of an otpauth:// URL for the +// manual-entry fallback. It returns "" when the URL carries none — the page then +// shows the link alone. The secret is never logged. +func secretFromOtpauth(raw string) string { + u, err := url.Parse(raw) + if err != nil { + return "" + } + return u.Query().Get("secret") +} + // codeFormPage renders the D-044 first-run page — what `GET /setup` shows while // `setup: true` and a required `setup_code` has not yet been entered. It is // deliberately friendly: the point is that the domain must NOT look "down". diff --git a/internal/core/setup/setup.go b/internal/core/setup/setup.go index cf80869..fc7aa18 100644 --- a/internal/core/setup/setup.go +++ b/internal/core/setup/setup.go @@ -69,8 +69,17 @@ type Users interface { // Enroller is the SPEC-006 TOTP ceremony, reused so the wizard never // duplicates the work (SPEC-005 §4). Step 2 enrolls TOTP only (D-041) — the // wizard never calls the WebAuthn registration actions. +// +// The seam carries no JSON-writing method: the wizard is server-rendered and +// no-JS, so it renders the issued secret and the verify form itself rather than +// delegating to the enroll flow's JSON API (`webauthn.js`'s contract) — the +// exact coupling that stalled the wizard's TOTP step (SPEC-005 §11). type Enroller interface { - TOTPBegin(w http.ResponseWriter, r *http.Request, guid string) error + // IssueTOTP issues (and persists) the TOTP secret and returns the otpauth + // URL, writing NO response — the caller owns the reply. + IssueTOTP(ctx context.Context, guid string) (string, error) + // TOTPVerify verifies a code: it completes the admin's enrollment (the + // wizard's TOTP-only mode, D-041) and mints the session. TOTPVerify(w http.ResponseWriter, r *http.Request, guid string) error } @@ -330,9 +339,14 @@ func (a *Actions) ensureAdminIdentity(ctx context.Context) (repaired bool, err e return true, nil } -// TOTPBegin issues the admin's TOTP secret. +// TOTPBegin issues the admin's TOTP secret and renders the wizard's TOTP page: +// the otpauth URL with its human-readable secret and the verify form (SPEC-005 +// §4). The wizard is server-rendered and works with no JavaScript, so it does +// NOT answer with the enroll flow's JSON (`webauthn.js`'s contract) — it issues +// the secret through the write-free IssueTOTP seam and renders the page itself. func (a *Actions) TOTPBegin(w http.ResponseWriter, r *http.Request) error { - guid, ok, err := a.pendingGUID(r.Context()) + ctx := r.Context() + guid, ok, err := a.pendingGUID(ctx) if err != nil { return err } @@ -340,7 +354,15 @@ func (a *Actions) TOTPBegin(w http.ResponseWriter, r *http.Request) error { writeError(w, http.StatusConflict, "no admin is awaiting enrollment") return nil } - return a.d.Enroll.TOTPBegin(w, r, guid) + url, err := a.d.Enroll.IssueTOTP(ctx, guid) + if err != nil { + return err + } + token, err := a.d.CSRF.Ensure(w, r) + if err != nil { + return err + } + return renderHTML(w, http.StatusOK, totpPage(token, url)) } // TOTPVerify verifies the code, activating the identity and minting a session @@ -557,7 +579,9 @@ func (a *Actions) adminGUID(ctx context.Context) (string, error) { } // pendingGUID returns the admin's GUID when its identity still awaits -// enrollment; ok=false means there is nothing to enroll. +// enrollment — pending AND inside its bounded enrollment window (SPEC-004 §4, +// A1). ok=false means there is nothing to enroll: no identity, another status, +// or the window has closed. func (a *Actions) pendingGUID(ctx context.Context) (guid string, ok bool, err error) { idn, err := a.d.Identities.ByUserID(ctx, adminUserID) if err != nil { @@ -569,6 +593,13 @@ func (a *Actions) pendingGUID(ctx context.Context) (guid string, ok bool, err er if idn.Status != auth.StatusPendingEnrollment { return "", false, nil } + inWindow, err := a.d.Identities.WithinEnrollmentWindow(ctx, adminUserID) + if err != nil { + return "", false, err + } + if !inWindow { + return "", false, nil + } return idn.GUID, true, nil }