amadan

oficina / sheets Public

fix: the root page tells you whether you are signed in

auth's SignedInPath defaults to "/", so a magic link lands here. This
page told the first person to click one that the instance "answers
GET /healthz and GET /api/version" and that "there is nothing else to
reach at this address" — while /signin, /auth/verify and /signout were
all live and the click had just succeeded. A successful sign-in and a
silently failed one looked identical to the person who clicked, and the
page's claim about itself had gone stale the moment sign-in shipped.

It now has two states: anonymous gets a link to /signin, signed-in gets
their address and a sign-out button. The address arrives from a verified
session rather than from the request and is escaped anyway, because
"trusted source" is the reasoning behind most injection bugs.

Adds the test that would have caught it, which is an end-to-end one
rather than another unit: ask for a link, take the link out of the mail
that was actually sent, follow it, and check the root page names you.
Every existing test proved a piece — the allowlist gate, the four
mounted routes, the page rendering — and all of them passed while the
thing a person actually does was broken. The gap was never inside a
piece.

TestSignInIsSingleUse comes free from the same harness and confirms from
our side what Docs corrected me on: a second use of the same link mints
no session and redirects err=expired, not a generic failure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APtNStXDZ5H2fAc1ZuGqtS
Paul Campbell pushed by paul@keymail.dev b70b190a0244f8e21eb249f3b84b410fdedf9013 parent 359c37f
3 files changed, +176 −37
  • internal/sheets/begin_test.go +22 −3
  • internal/sheets/page.go +60 −24
  • internal/sheets/page_test.go +94 −10

CI passed — run details

Intent

full ledger: core

diff --git a/internal/sheets/begin_test.go b/internal/sheets/begin_test.go
index 3229bf8..49d5191 100644
--- a/internal/sheets/begin_test.go
+++ b/internal/sheets/begin_test.go
@@ -5,6 +5,7 @@ import (
"net/http"
"net/http/httptest"
"path/filepath"
+ "regexp"
"strings"
"sync"
"testing"
@@ -16,17 +17,35 @@ import (
// recorder is a mail.Sender that delivers nothing and remembers who it
// was asked to write to.
type recorder struct {
- mu sync.Mutex
- sent []string
+ mu sync.Mutex
+ sent []string
+ bodies []string
}
-func (m *recorder) Send(_ context.Context, to, _, _ string) error {
+func (m *recorder) Send(_ context.Context, to, _, body string) error {
m.mu.Lock()
defer m.mu.Unlock()
m.sent = append(m.sent, to)
+ m.bodies = append(m.bodies, body)
return nil
}
+// lastLink pulls the verify URL out of the most recent email.
+func (m *recorder) lastLink(t *testing.T) string {
+ t.Helper()
+ m.mu.Lock()
+ defer m.mu.Unlock()
+ if len(m.bodies) == 0 {
+ t.Fatal("no mail was sent")
+ }
+ link := regexp.MustCompile(`https?://\S*/auth/verify\?\S+`).
+ FindString(m.bodies[len(m.bodies)-1])
+ if link == "" {
+ t.Fatalf("no verify link in mail body: %q", m.bodies[len(m.bodies)-1])
+ }
+ return strings.TrimRight(link, ".,)\"'>")
+}
+
func (m *recorder) addresses() []string {
m.mu.Lock()
defer m.mu.Unlock()
diff --git a/internal/sheets/page.go b/internal/sheets/page.go
index bdc0aae..b5f1292 100644
--- a/internal/sheets/page.go
+++ b/internal/sheets/page.go
@@ -1,23 +1,23 @@
package sheets
-import "net/http"
+import (
+ "html"
+ "net/http"
+)
-// indexPage is the entire root page: bare fact, not marketing. It has
-// no dynamic content and no user input to escape, so a plain constant
-// string stands in for html/template here rather than pretending to
-// need one.
+// The root page has two states, and the signed-in one is the whole
+// point: a magic link lands here, so if this page cannot say "you are
+// signed in, as this address", then a successful sign-in and a silently
+// failed one look identical to the person who clicked. That is exactly
+// what happened the first time a link was clicked here — auth's
+// SignedInPath defaults to "/", and this page told the arriving user
+// there was nothing to reach at this address.
//
// No stylesheet is linked. oficina/estilo exists and is pinned to
-// rastrillo's day theme, but this app does not consume it and nobody
-// has decided that it should — the grid slice is where that decision
-// belongs. Semantic, unstyled HTML is the honest choice until then.
-//
-// The two routes named below are registered by rastrillo.Serve's outer
-// mux, not by this package (see cmd/sheets/main.go). They are named
-// here because someone who lands on / is usually an operator checking
-// whether the instance is alive, and both were verified answering on
-// the deployed instance.
-const indexPage = `<!doctype html>
+// rastrillo's day theme, but this app does not consume it and nobody has
+// decided that it should — the grid slice is where that decision belongs.
+const (
+ pageHead = `<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
@@ -26,13 +26,17 @@ const indexPage = `<!doctype html>
<body>
<main>
<h1>Oficina Sheets</h1>
-<p>This is the Oficina Sheets engine: sealed storage, formula
+`
+ pageWhat = `<p>This is the Oficina Sheets engine: sealed storage, formula
evaluation, and XLSX import and export.</p>
-<p>It has no user interface yet. That is deliberate: the grid is the
-next slice of work, not built here.</p>
-<p>In production this instance answers <code>GET /healthz</code> and
-<code>GET /api/version</code>. There is nothing else to reach at this
-address.</p>
+<p>It has no grid yet. That is deliberate: the spreadsheet interface is
+the next slice of work, not built here. Sign-in is real and working.</p>
+`
+ pageSignedOut = `<p>You are not signed in. <a href="/signin">Sign in</a> — we email you
+a link instead of asking for a password.</p>
+`
+ pageFoot = `<p>This instance also answers <code>GET /healthz</code> and
+<code>GET /api/version</code>.</p>
<p>The design is at
<code>docs/superpowers/specs/2026-08-30-sheets-design.md</code> in this
repository.</p>
@@ -40,9 +44,41 @@ repository.</p>
</body>
</html>
`
+)
-// handleIndex serves the one page this app has.
-func (a *App) handleIndex(w http.ResponseWriter, _ *http.Request) {
+// handleIndex serves the root page, reflecting whether the caller holds
+// a session.
+func (a *App) handleIndex(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/html; charset=utf-8")
- w.Write([]byte(indexPage))
+ w.Write([]byte(pageHead))
+ w.Write([]byte(pageWhat))
+
+ switch id, ok := a.identity(r); {
+ case ok:
+ // The address is the person's own, shown back to them, and it
+ // arrives from a verified session rather than from the request —
+ // but it is still escaped, because "trusted source" is the
+ // reasoning behind most injection bugs.
+ w.Write([]byte("<p>Signed in as <strong>" + html.EscapeString(id) +
+ "</strong>.</p>\n" +
+ `<form method="post" action="/signout"><button type="submit">Sign out</button></form>` +
+ "\n"))
+ default:
+ w.Write([]byte(pageSignedOut))
+ }
+
+ w.Write([]byte(pageFoot))
+}
+
+// identity returns the signed-in address, if any. A nil auth (tests)
+// simply has no session.
+func (a *App) identity(r *http.Request) (string, bool) {
+ if a.auth == nil {
+ return "", false
+ }
+ id, ok := a.auth.SessionFrom(r)
+ if !ok {
+ return "", false
+ }
+ return id.Address, true
}
diff --git a/internal/sheets/page_test.go b/internal/sheets/page_test.go
index beb8fd5..4a1e7cf 100644
--- a/internal/sheets/page_test.go
+++ b/internal/sheets/page_test.go
@@ -3,6 +3,7 @@ package sheets
import (
"net/http"
"net/http/httptest"
+ "net/url"
"strings"
"testing"
)
@@ -22,13 +23,12 @@ func TestIndexServesHTML(t *testing.T) {
if ct := rec.Header().Get("Content-Type"); ct != "text/html; charset=utf-8" {
t.Errorf("Content-Type = %q", ct)
}
- // The page's job is to say what this address is and what it is not.
for _, want := range []string{
"<h1>Oficina Sheets</h1>",
- "no user interface yet",
+ "no grid yet",
"/healthz",
- "/api/version",
"2026-08-30-sheets-design.md",
+ `<a href="/signin">`, // an anonymous visitor must be able to get in
} {
if !strings.Contains(rec.Body.String(), want) {
t.Errorf("body missing %q", want)
@@ -36,10 +36,16 @@ func TestIndexServesHTML(t *testing.T) {
}
}
-// TestIndexIsNotACatchAll is the point of the "/" route being exact.
-// A catch-all would serve the page at every URL, so a mistyped path
-// would look like a working page instead of a 404 — and nothing else
-// in the suite would notice.
+// TestIndexDoesNotClaimNothingElseExists: the page said "there is
+// nothing else to reach at this address" while /signin was live, which
+// it told the first person to click a magic link. A page that describes
+// the app has to be corrected when the app changes.
+func TestIndexDoesNotClaimNothingElseExists(t *testing.T) {
+ if body := get(t, "/").Body.String(); strings.Contains(body, "nothing else to reach") {
+ t.Error("root page still claims nothing else is reachable")
+ }
+}
+
func TestIndexIsNotACatchAll(t *testing.T) {
for _, path := range []string{"/nope", "/api/sheets", "/healthz/x"} {
if code := get(t, path).Code; code != http.StatusNotFound {
@@ -48,9 +54,7 @@ func TestIndexIsNotACatchAll(t *testing.T) {
}
}
-// TestIndexLeaksNothing: the one rule reaches even a static page. No
-// cell content, sheet title or tab name exists here, and no build,
-// path or host detail should either.
+// TestIndexLeaksNothing: the one rule reaches even a static page.
func TestIndexLeaksNothing(t *testing.T) {
body := get(t, "/").Body.String()
for _, forbidden := range []string{"/home/", "oncarlos.com", "sqlite", ".db"} {
@@ -59,3 +63,83 @@ func TestIndexLeaksNothing(t *testing.T) {
}
}
}
+
+// TestSignInEndToEnd drives the whole flow the way a person does:
+// request a link, take the link out of the email that was actually sent,
+// follow it, and check the root page then says who you are.
+//
+// It exists because every earlier test here proved a piece — the gate,
+// the routes, the page — and the first real click still landed on a page
+// that gave no sign anything had happened. The pieces were all correct.
+func TestSignInEndToEnd(t *testing.T) {
+ app, mail := appWithRecorder(t)
+
+ // 1. Anonymous: the page offers a way in and names nobody.
+ before := httptest.NewRecorder()
+ app.Router().ServeHTTP(before, httptest.NewRequest(http.MethodGet, "/", nil))
+ if strings.Contains(before.Body.String(), "Signed in as") {
+ t.Fatal("anonymous request already looks signed in")
+ }
+
+ // 2. Ask for a link as a member, and take the one that was emailed.
+ postSignin(t, app, "paul@example.com")
+ link := mail.lastLink(t)
+
+ u, err := url.Parse(link)
+ if err != nil {
+ t.Fatalf("parse emailed link %q: %v", link, err)
+ }
+ if u.Query().Get("token") == "" {
+ t.Fatalf("emailed link carries no token: %s", link)
+ }
+
+ // 3. Follow it.
+ verify := httptest.NewRecorder()
+ app.Router().ServeHTTP(verify, httptest.NewRequest(http.MethodGet, u.RequestURI(), nil))
+ cookies := verify.Result().Cookies()
+ if len(cookies) == 0 {
+ t.Fatalf("verify set no cookie (status %d, location %q)",
+ verify.Code, verify.Header().Get("Location"))
+ }
+
+ // 4. The root page now says who you are, and offers a way out.
+ after := httptest.NewRecorder()
+ req := httptest.NewRequest(http.MethodGet, "/", nil)
+ for _, c := range cookies {
+ req.AddCookie(c)
+ }
+ app.Router().ServeHTTP(after, req)
+
+ body := after.Body.String()
+ if !strings.Contains(body, "Signed in as") || !strings.Contains(body, "paul@example.com") {
+ t.Errorf("root page after sign-in does not name the user:\n%s", body)
+ }
+ if !strings.Contains(body, `action="/signout"`) {
+ t.Error("no way to sign out")
+ }
+ if strings.Contains(body, `<a href="/signin">`) {
+ t.Error("still offering sign-in to a signed-in user")
+ }
+}
+
+// TestSignInIsSingleUse: the same link must not work twice.
+func TestSignInIsSingleUse(t *testing.T) {
+ app, mail := appWithRecorder(t)
+ postSignin(t, app, "paul@example.com")
+ u, _ := url.Parse(mail.lastLink(t))
+
+ first := httptest.NewRecorder()
+ app.Router().ServeHTTP(first, httptest.NewRequest(http.MethodGet, u.RequestURI(), nil))
+ if len(first.Result().Cookies()) == 0 {
+ t.Fatal("first use did not sign in")
+ }
+
+ second := httptest.NewRecorder()
+ app.Router().ServeHTTP(second, httptest.NewRequest(http.MethodGet, u.RequestURI(), nil))
+ if len(second.Result().Cookies()) != 0 {
+ t.Error("second use of the same link minted a session")
+ }
+ if loc := second.Header().Get("Location"); !strings.Contains(loc, "err=expired") {
+ t.Errorf("second use redirected to %q, want err=expired", loc)
+ }
+}