amadan
Tasks 9 and 10: admin that fails closed, and a tick the scheduler can reach
With no allowlist the admin surface is absent rather than forbidden — all seven routes 404, sign-in included, because a live /signin under an empty allowlist would let anyone make this app mail a link from our verified identity to any address they named. A test iterates the list rather than spot-checking, since a route added later without being listed is exactly the one that would stay live. POST /signin is ours, not auth.Begin. auth checks Authorize at admission, which is the right place for access and the wrong place for delivery: it sends first. A refused address gets the identical redirect and no mail, so the endpoint is not an oracle for who the admins are. Copied from sheets, which found this in production. The allowlist is re-checked per request and sessions last 8 hours, not the framework's 30 days. The removal test restarts the app rather than calling t.Setenv, because a process's environment is fixed at exec and the easy version of that test passes while proving nothing. The tick is mounted outside csrf.Protect. Moving it inside makes the gate fail with "refused by CSRF", which is the silent permanent breakage it exists to catch — 403 forever, and the platform reads 4xx as do-not-retry. Also pinned the rastrillo CLI as a tool dependency: go mod tidy pruned its toml import and broke migration-check twice. Plan 1 is complete. 61 tests plus 44 under -tags browser, make ci green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
12 files changed,
+833
−10
go.mod+5 −2internal/movement/admin.go+220 −0internal/movement/app.go+10 −0internal/movement/copy.go+13 −3internal/movement/handlers.go+5 −0internal/movement/migrations.go+6 −4internal/movement/render.go+1 −1internal/movement/sweep.go+74 −0internal/movement/templates/admin.html+48 −0internal/movement/templates/signin.html+19 −0internal/movementtest/admin_test.go+275 −0internal/movementtest/sweep_test.go+157 −0
diff --git a/go.mod b/go.mod| index edfab52..3613043 100644 |
| --- a/go.mod |
| +++ b/go.mod |
| @@ -4,13 +4,14 @@ go 1.25.0 |
| require ( |
| github.com/carlosframework/rastrillo v0.23.0 |
| + github.com/chromedp/cdproto v0.0.0-20250724212937-08a3db8b4327 |
| github.com/chromedp/chromedp v0.14.2 |
| github.com/go-chi/chi/v5 v5.3.2 |
| + gorm.io/gorm v1.31.2 |
| ) |
| require ( |
| github.com/BurntSushi/toml v1.6.0 // indirect |
| - github.com/chromedp/cdproto v0.0.0-20250724212937-08a3db8b4327 // indirect |
| github.com/chromedp/sysutil v1.1.0 // indirect |
| github.com/dustin/go-humanize v1.0.1 // indirect |
| github.com/go-json-experiment/json v0.0.0-20250725192818-e39067aee2d2 // indirect |
| @@ -20,15 +21,17 @@ require ( |
| github.com/google/uuid v1.6.0 // indirect |
| github.com/jinzhu/inflection v1.0.0 // indirect |
| github.com/jinzhu/now v1.1.5 // indirect |
| + github.com/keymaildev/signin v0.1.1 // indirect |
| github.com/mattn/go-isatty v0.0.20 // indirect |
| github.com/ncruces/go-strftime v1.0.0 // indirect |
| github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect |
| golang.org/x/sys v0.46.0 // indirect |
| golang.org/x/text v0.20.0 // indirect |
| - gorm.io/gorm v1.31.2 // indirect |
| gorm.io/plugin/dbresolver v1.6.2 // indirect |
| modernc.org/libc v1.74.1 // indirect |
| modernc.org/mathutil v1.7.1 // indirect |
| modernc.org/memory v1.11.0 // indirect |
| modernc.org/sqlite v1.55.0 // indirect |
| ) |
| + |
| +tool github.com/carlosframework/rastrillo/cmd/rastrillo |
diff --git a/internal/movement/admin.go b/internal/movement/admin.go| new file mode 100644 |
| index 0000000..601ef59 |
| --- /dev/null |
| +++ b/internal/movement/admin.go |
| @@ -0,0 +1,220 @@ |
| +package movement |
| + |
| +import ( |
| + "encoding/csv" |
| + "net/http" |
| + "time" |
| + |
| + "github.com/carlosframework/rastrillo/auth" |
| + "github.com/go-chi/chi/v5" |
| +) |
| + |
| +// adminSessionTTL caps how long an admin stays signed in. The framework's |
| +// default is thirty days, which is far too long for a surface that lists |
| +// named people's stated positions on AI. |
| +// |
| +// Not RequireFreshSession, which is a step-up mechanism for sensitive routes |
| +// layered on top of a longer session — that is a different tool for a |
| +// different job. |
| +const adminSessionTTL = 8 * time.Hour |
| + |
| +// mountAdmin registers every admin route, or none of them. |
| +// |
| +// With MOVEMENT_ADMINS empty the whole surface is absent: not 403, not an |
| +// open page, but genuinely not there, indistinguishable from an app that has |
| +// no admin at all. Sign-in is included in that. Leaving it live under an |
| +// empty allowlist would let anyone make this app email a link from our |
| +// verified sending identity to any address they named. |
| +func (a *app) mountAdmin(r chi.Router) error { |
| + if len(a.env.Admins) == 0 { |
| + Info("no MOVEMENT_ADMINS set; the admin surface is not mounted") |
| + return nil |
| + } |
| + |
| + au, err := auth.New(auth.Config{ |
| + DB: a.db.Writer(), |
| + Origin: a.env.Origin, |
| + InstanceKey: a.env.InstanceKey, |
| + // Explicit, always. A nil Mailer makes auth substitute mail.Logged, |
| + // which writes the magic link into carlos logs — the same leak |
| + // config.go refuses at boot, arriving through another door. |
| + Mailer: a.mailer, |
| + Authorize: a.env.IsAdmin, |
| + SigninPath: "/signin", |
| + SessionTTL: adminSessionTTL, |
| + Logger: slogFor(), |
| + }) |
| + if err != nil { |
| + return err |
| + } |
| + a.auth = au |
| + |
| + r.Get("/signin", a.signinPage) |
| + // Ours, not auth.Begin directly. See handleSignin. |
| + r.Post("/signin", a.handleSignin) |
| + r.Get("/auth/callback", au.Callback) |
| + r.Get("/auth/verify", au.Verify) |
| + r.Post("/signout", au.Signout) |
| + |
| + r.Group(func(r chi.Router) { |
| + r.Use(au.RequireSession) |
| + r.Use(a.requireAdmin) |
| + r.Get("/admin", a.adminIndex) |
| + r.Get("/admin/export.csv", a.adminExport) |
| + }) |
| + return nil |
| +} |
| + |
| +func (a *app) signinPage(w http.ResponseWriter, r *http.Request) { |
| + render(w, "signin", map[string]any{"Sent": r.URL.Query().Get("sent") == "1"}) |
| +} |
| + |
| +// handleSignin gates POST /signin on the allowlist before auth mints and |
| +// mails anything. |
| +// |
| +// auth checks Authorize at admission — the moment a verified address would |
| +// receive a session — which is the right place for the access decision, and |
| +// access is safe without this. What is not safe is delivery: without this |
| +// gate, anyone who can reach the instance can make it email a live sign-in |
| +// link to any address they name. The recipient gets unsolicited mail and our |
| +// sending identity wears it. |
| +// |
| +// The refusal is deliberately invisible. Saying "that address cannot sign in" |
| +// would turn this endpoint into a membership oracle for the admin list, so a |
| +// refused address gets exactly the response an accepted one gets, and no |
| +// mail. The cost is real and accepted: an admin who mistypes their own |
| +// address is told to check their inbox and nothing arrives. |
| +func (a *app) handleSignin(w http.ResponseWriter, r *http.Request) { |
| + if err := r.ParseForm(); err != nil { |
| + http.Error(w, "bad form", http.StatusBadRequest) |
| + return |
| + } |
| + if !a.env.IsAdmin(r.FormValue("address")) { |
| + http.Redirect(w, r, "/signin?sent=1", http.StatusSeeOther) |
| + return |
| + } |
| + a.auth.Begin(w, r) |
| +} |
| + |
| +// requireAdmin re-checks the allowlist on every request. |
| +// |
| +// auth.Config.Authorize runs once, when a magic link is redeemed; |
| +// RequireSession thereafter only checks that a session exists. Without this, |
| +// removing somebody from MOVEMENT_ADMINS would do nothing until their session |
| +// expired. The list itself is read at boot — a process's environment is fixed |
| +// at exec — so what makes a removal take effect is the platform bouncing the |
| +// instance when the config converges, and this middleware then refusing the |
| +// session that survived. |
| +func (a *app) requireAdmin(next http.Handler) http.Handler { |
| + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + id, ok := auth.From(r) |
| + if !ok || !a.env.IsAdmin(id.Address) { |
| + http.NotFound(w, r) |
| + return |
| + } |
| + next.ServeHTTP(w, r) |
| + }) |
| +} |
| + |
| +type adminRow struct { |
| + Email string |
| + Stance string |
| + Background string |
| + When time.Time |
| +} |
| + |
| +// adminIndex lists confirmed rows and counts the rest. |
| +// |
| +// Pending rows are not shown. One is an address nobody has proven owns it, |
| +// joined to a stated position on AI — the pair the logging rules exist to |
| +// keep apart — and showing it to satisfy curiosity about the funnel is the |
| +// wrong trade. The count is there so the funnel is not invisible. |
| +func (a *app) adminIndex(w http.ResponseWriter, r *http.Request) { |
| + var rows []Signup |
| + if err := a.db.G.Where("confirmed = ?", true). |
| + Order("confirmed_at desc").Find(&rows).Error; err != nil { |
| + http.Error(w, "something went wrong", http.StatusInternalServerError) |
| + return |
| + } |
| + |
| + var pending int64 |
| + if err := a.db.G.Model(&Signup{}).Where("confirmed = ?", false). |
| + Count(&pending).Error; err != nil { |
| + http.Error(w, "something went wrong", http.StatusInternalServerError) |
| + return |
| + } |
| + |
| + out := make([]adminRow, 0, len(rows)) |
| + for _, row := range rows { |
| + when := row.CreatedAt |
| + if row.ConfirmedAt != nil { |
| + when = *row.ConfirmedAt |
| + } |
| + out = append(out, adminRow{row.Email, row.Stance, row.Background, when}) |
| + } |
| + |
| + render(w, "admin", map[string]any{ |
| + "Rows": out, |
| + "Pending": pending, |
| + "Counts": a.stanceCounts(rows), |
| + }) |
| +} |
| + |
| +func (a *app) stanceCounts(rows []Signup) map[string]int { |
| + counts := map[string]int{} |
| + for _, s := range Stances { |
| + counts[string(s)] = 0 |
| + } |
| + for _, row := range rows { |
| + counts[row.Stance]++ |
| + } |
| + return counts |
| +} |
| + |
| +// adminExport writes confirmed rows only. That restriction is the one rule's |
| +// first half made mechanical, and it is a test rather than a convention. |
| +func (a *app) adminExport(w http.ResponseWriter, r *http.Request) { |
| + var rows []Signup |
| + if err := a.db.G.Where("confirmed = ?", true). |
| + Order("confirmed_at desc").Find(&rows).Error; err != nil { |
| + http.Error(w, "something went wrong", http.StatusInternalServerError) |
| + return |
| + } |
| + |
| + w.Header().Set("Content-Type", "text/csv; charset=utf-8") |
| + w.Header().Set("Content-Disposition", `attachment; filename="movement.csv"`) |
| + |
| + cw := csv.NewWriter(w) |
| + _ = cw.Write([]string{"email", "stance", "confirmed_at", "background"}) |
| + for _, row := range rows { |
| + when := "" |
| + if row.ConfirmedAt != nil { |
| + when = row.ConfirmedAt.UTC().Format(time.RFC3339) |
| + } |
| + _ = cw.Write([]string{ |
| + csvSafe(row.Email), |
| + csvSafe(row.Stance), |
| + when, |
| + csvSafe(row.Background), |
| + }) |
| + } |
| + cw.Flush() |
| +} |
| + |
| +// csvSafe defuses formula injection. |
| +// |
| +// This export exists to be opened in a spreadsheet, and a cell beginning |
| +// =, +, -, @, tab or carriage return is executed when it is. The background |
| +// field is free text somebody else wrote, which makes it the obvious carrier. |
| +// Quoting is the csv writer's job; this is about what the receiving |
| +// application does with the value afterwards. |
| +func csvSafe(s string) string { |
| + if s == "" { |
| + return s |
| + } |
| + switch s[0] { |
| + case '=', '+', '-', '@', '\t', '\r': |
| + return "'" + s |
| + } |
| + return s |
| +} |
diff --git a/internal/movement/app.go b/internal/movement/app.go| index f3c6b9e..f1c00b8 100644 |
| --- a/internal/movement/app.go |
| +++ b/internal/movement/app.go |
| @@ -83,12 +83,22 @@ func App(d *db.DB, env Env) (*http.ServeMux, error) { |
| r.Get("/confirm", a.handleConfirmPage) |
| r.Post("/confirm", a.handleConfirm) |
| + if err := a.mountAdmin(r); err != nil { |
| + return nil, err |
| + } |
| + |
| mux := http.NewServeMux() |
| // The app serves its own static files — the framework never does. |
| // They are embedded (render.go) and fingerprinted: templates link |
| // them via {{asset ...}}, which returns a content-hashed URL the |
| // handler serves cacheable-forever. |
| mux.Handle("GET /static/", assets.Handler()) |
| + // Outside the chi router, and therefore outside csrf.Protect. The |
| + // platform's scheduler sends no Origin, Referer or Sec-Fetch-Site, and |
| + // csrf.SameOrigin refuses a POST carrying none of them — so mounted |
| + // inside, this would 403 forever and the platform would stop retrying. |
| + // carlos.Tick's bearer check is what guards it instead. |
| + mux.Handle("POST /tick/sweep", http.HandlerFunc(a.handleSweep)) |
| mux.Handle("/", r) |
| return mux, nil |
| } |
diff --git a/internal/movement/copy.go b/internal/movement/copy.go| index 9e953ae..fb4d70d 100644 |
| --- a/internal/movement/copy.go |
| +++ b/internal/movement/copy.go |
| @@ -69,9 +69,19 @@ var table = map[string]entry{ |
| "mail.confirm.body": {"PLACEHOLDER confirmation body", true}, |
| // Admin. |
| - "signin.title": {"PLACEHOLDER sign in", true}, |
| - "signin.sent": {"PLACEHOLDER check your inbox for a link", true}, |
| - "admin.title": {"PLACEHOLDER admin", true}, |
| + "signin.title": {"PLACEHOLDER sign in", true}, |
| + "signin.sent": {"PLACEHOLDER check your inbox for a link", true}, |
| + "signin.submit": {"PLACEHOLDER send me a link", true}, |
| + "admin.title": {"PLACEHOLDER admin", true}, |
| + "admin.confirmed": {"PLACEHOLDER confirmed", true}, |
| + "admin.pending": {"PLACEHOLDER pending", true}, |
| + "admin.export": {"PLACEHOLDER download csv", true}, |
| + "admin.empty": {"PLACEHOLDER nobody yet", true}, |
| + "admin.signout": {"PLACEHOLDER sign out", true}, |
| + "admin.col.email": {"PLACEHOLDER email", true}, |
| + "admin.col.stance": {"PLACEHOLDER stance", true}, |
| + "admin.col.when": {"PLACEHOLDER confirmed on", true}, |
| + "admin.col.background": {"PLACEHOLDER background", true}, |
| // Not a placeholder: this one exists to say that the others are. |
| "banner.placeholder": {"This build carries unreviewed placeholder copy.", false}, |
diff --git a/internal/movement/handlers.go b/internal/movement/handlers.go| index 425bbd4..0a81897 100644 |
| --- a/internal/movement/handlers.go |
| +++ b/internal/movement/handlers.go |
| @@ -3,6 +3,7 @@ package movement |
| import ( |
| "net/http" |
| + "github.com/carlosframework/rastrillo/auth" |
| "github.com/carlosframework/rastrillo/db" |
| "github.com/carlosframework/rastrillo/mail" |
| ) |
| @@ -21,6 +22,10 @@ type app struct { |
| challengeKey []byte |
| mailer mail.Sender |
| + |
| + // auth is nil when MOVEMENT_ADMINS is empty: with no allowlist there is |
| + // no admin surface to sign in to, and none of its routes are mounted. |
| + auth *auth.Auth |
| } |
| func (a *app) index(w http.ResponseWriter, r *http.Request) { |
diff --git a/internal/movement/migrations.go b/internal/movement/migrations.go| index 82bde6c..7fb76a4 100644 |
| --- a/internal/movement/migrations.go |
| +++ b/internal/movement/migrations.go |
| @@ -3,7 +3,9 @@ package movement |
| import ( |
| "embed" |
| + "github.com/carlosframework/rastrillo/auth" |
| "github.com/carlosframework/rastrillo/migrate" |
| + "github.com/carlosframework/rastrillo/sessions" |
| ) |
| //go:embed migrations/*.sql |
| @@ -35,8 +37,8 @@ var Schema = migrate.MustFromFS(migrationFS, "movement") |
| // |
| // var BootSchema = migrate.Merge(sessions.Schema, Schema) |
| // |
| -// Merge's argument order is apply order, so a subsystem whose |
| -// migrations another one reads (auth after sessions) goes first. |
| -// Get this line wrong and either boot silently skips a subsystem's |
| +// Merge's argument order is apply order, and it matters here: auth's |
| +// backfill migration reads the sessions table, so sessions has to be created |
| +// first. Get this line wrong and either boot silently skips a subsystem's |
| // tables, or generate/check try to drop them. |
| -var BootSchema = migrate.Merge(Schema) |
| +var BootSchema = migrate.Merge(sessions.Schema, auth.Schema, Schema) |
diff --git a/internal/movement/render.go b/internal/movement/render.go| index f76a2c2..60abdf3 100644 |
| --- a/internal/movement/render.go |
| +++ b/internal/movement/render.go |
| @@ -26,7 +26,7 @@ var assets = rastrillo.NewAssets(appFS) |
| var pages = map[string]*template.Template{} |
| func init() { |
| - for _, name := range []string{"index", "stance", "sent", "confirm", "errors"} { |
| + for _, name := range []string{"index", "stance", "sent", "confirm", "signin", "admin", "errors"} { |
| pages[name] = template.Must( |
| // ui's partials and their helpers first, so every page can |
| // call one without registering anything. WithIcons points |
diff --git a/internal/movement/sweep.go b/internal/movement/sweep.go| new file mode 100644 |
| index 0000000..a8ee52f |
| --- /dev/null |
| +++ b/internal/movement/sweep.go |
| @@ -0,0 +1,74 @@ |
| +package movement |
| + |
| +import ( |
| + "net/http" |
| + "time" |
| + |
| + "github.com/carlosframework/rastrillo/carlos" |
| +) |
| + |
| +// pendingLifetime is how long an unconfirmed row is kept. |
| +// |
| +// Thirty days is longer than the token it holds (24 hours) and longer than |
| +// the per-address mail budget window (24 hours), and the second of those is |
| +// the reason. A sweep shorter than the budget window would be a way to reset |
| +// the budget by waiting: delete the row, and the counter goes with it. |
| +// |
| +// The cost is that a pending row outlives its own link by twenty-nine days, |
| +// purely to hold a counter. If that ever matters, moving the per-address |
| +// budget into its own table keyed by an address hash would let this drop back |
| +// to about forty-eight hours. |
| +const pendingLifetime = 30 * 24 * time.Hour |
| + |
| +// handleSweep is the scheduled tick. Declared with: |
| +// |
| +// carlos schedule set -app movement -name sweep -every 1h -path /tick/sweep |
| +// |
| +// It is mounted on the outer ServeMux, deliberately outside csrf.Protect. |
| +// csrf.SameOrigin refuses any POST carrying none of Sec-Fetch-Site, Origin or |
| +// Referer, and the platform's scheduler sends none of the three — so inside |
| +// the protected router this would answer 403 forever, and the platform reads |
| +// 4xx as "do not retry". It fails silently and permanently, which is why a |
| +// test stands in front of it. |
| +// |
| +// The work happens synchronously in the request. A 202 and a goroutine risks |
| +// the instance hibernating mid-job, and hibernating is this process's normal |
| +// state rather than an exception. |
| +func (a *app) handleSweep(w http.ResponseWriter, r *http.Request) { |
| + // carlos.Tick is false when no token is configured, so this fails closed |
| + // off-platform rather than exposing a public delete endpoint. |
| + if !carlos.Tick(r) { |
| + http.NotFound(w, r) |
| + return |
| + } |
| + |
| + now := time.Now() |
| + |
| + // Unconfirmed rows nobody came back for. Confirmed rows are never |
| + // touched: they are the list. |
| + if err := a.db.G.Where("confirmed = ? AND created_at < ?", false, now.Add(-pendingLifetime)). |
| + Delete(&Signup{}).Error; err != nil { |
| + http.Error(w, "sweep failed", http.StatusInternalServerError) |
| + return |
| + } |
| + |
| + // Spent nonces past the age at which a challenge would be refused anyway. |
| + if err := a.db.G.Where("expires_at < ?", now).Delete(&SpentNonce{}).Error; err != nil { |
| + http.Error(w, "sweep failed", http.StatusInternalServerError) |
| + return |
| + } |
| + |
| + // Expired magic links and sessions. Nothing else in the app calls this, |
| + // so without it auth's own tables grow forever. |
| + if a.auth != nil { |
| + if err := a.auth.Sweep(now); err != nil { |
| + http.Error(w, "sweep failed", http.StatusInternalServerError) |
| + return |
| + } |
| + } |
| + |
| + // All three deletions are idempotent, so at-least-once delivery needs no |
| + // occurrence-key dedupe and carlos.TickOccurrence is not used. |
| + Info("sweep complete") |
| + w.WriteHeader(http.StatusOK) |
| +} |
diff --git a/internal/movement/templates/admin.html b/internal/movement/templates/admin.html| new file mode 100644 |
| index 0000000..2add264 |
| --- /dev/null |
| +++ b/internal/movement/templates/admin.html |
| @@ -0,0 +1,48 @@ |
| +{{define "title"}}{{say "admin.title"}}{{end}} |
| +{{define "content"}} |
| +<h1 class="rst-page-title">{{say "admin.title"}}</h1> |
| + |
| +{{/* Confirmed rows only. A pending row is an address nobody has proven owns |
| + it, joined to a stated position on AI — the pair the logging rules keep |
| + apart — so the funnel shows as a count rather than a list. */}} |
| +<p> |
| + <span rst-badge>{{len .Rows}}</span> {{say "admin.confirmed"}} |
| + · |
| + <span rst-badge>{{.Pending}}</span> {{say "admin.pending"}} |
| +</p> |
| + |
| +<p> |
| + {{range $key, $count := .Counts}}<span rst-badge>{{$count}}</span> {{say (printf "stance.%s.label" $key)}} {{end}} |
| +</p> |
| + |
| +<p><a href="/admin/export.csv">{{say "admin.export"}}</a></p> |
| + |
| +<div class="rst-list"> |
| + <table> |
| + <thead> |
| + <tr> |
| + <th>{{say "admin.col.email"}}</th> |
| + <th>{{say "admin.col.stance"}}</th> |
| + <th>{{say "admin.col.when"}}</th> |
| + <th>{{say "admin.col.background"}}</th> |
| + </tr> |
| + </thead> |
| + <tbody> |
| + {{range .Rows}} |
| + <tr> |
| + <td>{{.Email}}</td> |
| + <td>{{say (printf "stance.%s.label" .Stance)}}</td> |
| + <td>{{.When.Format "2006-01-02"}}</td> |
| + <td>{{.Background}}</td> |
| + </tr> |
| + {{else}} |
| + <tr><td colspan="4">{{say "admin.empty"}}</td></tr> |
| + {{end}} |
| + </tbody> |
| + </table> |
| +</div> |
| + |
| +<form method="post" action="/signout"> |
| + <button type="submit">{{say "admin.signout"}}</button> |
| +</form> |
| +{{end}} |
diff --git a/internal/movement/templates/signin.html b/internal/movement/templates/signin.html| new file mode 100644 |
| index 0000000..0549ebd |
| --- /dev/null |
| +++ b/internal/movement/templates/signin.html |
| @@ -0,0 +1,19 @@ |
| +{{define "title"}}{{say "signin.title"}}{{end}} |
| +{{define "content"}} |
| +<h1 class="rst-page-title">{{say "signin.title"}}</h1> |
| + |
| +{{if .Sent}} |
| + {{/* Shown whether or not the address was on the allowlist. Saying "that |
| + address cannot sign in" would turn this page into a way to enumerate |
| + who the admins are. */}} |
| + <p rst-callout>{{say "signin.sent"}}</p> |
| +{{else}} |
| + <form class="rst-form" method="post" action="/signin"> |
| + <label class="rst-field" for="address"> |
| + <span>{{say "form.email"}}</span> |
| + <input type="email" id="address" name="address" required autocomplete="email"> |
| + </label> |
| + <button type="submit">{{say "signin.submit"}}</button> |
| + </form> |
| +{{end}} |
| +{{end}} |
diff --git a/internal/movementtest/admin_test.go b/internal/movementtest/admin_test.go| new file mode 100644 |
| index 0000000..ffb713b |
| --- /dev/null |
| +++ b/internal/movementtest/admin_test.go |
| @@ -0,0 +1,275 @@ |
| +package movementtest |
| + |
| +import ( |
| + "io" |
| + "log/slog" |
| + "net/http" |
| + "net/http/httptest" |
| + "net/url" |
| + "path/filepath" |
| + "regexp" |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/rastrillo/db" |
| + |
| + movement "amadan.net/carlos/movement/internal/movement" |
| +) |
| + |
| +const adminAddr = "paul@example.com" |
| + |
| +// newAdminApp is the app with an allowlist, so the admin surface exists at |
| +// all. newApp deliberately has none. |
| +func newAdminApp(t *testing.T) *testApp { |
| + t.Helper() |
| + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) |
| + d, err := db.Open(filepath.Join(t.TempDir(), "app.db"), logger) |
| + if err != nil { |
| + t.Fatalf("db.Open: %v", err) |
| + } |
| + t.Cleanup(func() { d.Close() }) |
| + movement.SetLogger(logger) |
| + |
| + box := &recorder{} |
| + env := testEnv() |
| + env.Admins = []string{adminAddr} |
| + env.Mailer = box |
| + |
| + mux, err := movement.App(d, env) |
| + if err != nil { |
| + t.Fatalf("App: %v", err) |
| + } |
| + return &testApp{Handler: mux, DB: d, Mail: box} |
| +} |
| + |
| +// adminRoutes is every route the admin surface adds. The fail-closed gate |
| +// iterates this rather than spot-checking, because a route added later |
| +// without being listed here is exactly the one that would stay live. |
| +var adminRoutes = []struct{ method, path string }{ |
| + {http.MethodGet, "/signin"}, |
| + {http.MethodPost, "/signin"}, |
| + {http.MethodGet, "/auth/callback"}, |
| + {http.MethodGet, "/auth/verify"}, |
| + {http.MethodPost, "/signout"}, |
| + {http.MethodGet, "/admin"}, |
| + {http.MethodGet, "/admin/export.csv"}, |
| +} |
| + |
| +// With no allowlist there is no admin surface: not 403, not an open page, |
| +// but genuinely absent — indistinguishable from an app that never had one. |
| +// |
| +// Sign-in has to be included. Left live under an empty allowlist it would let |
| +// anyone make this app email a link from our verified sending identity to any |
| +// address they named. |
| +func TestAdminSurfaceIsAbsentWithoutAnAllowlist(t *testing.T) { |
| + h := newApp(t) // testEnv sets no admins |
| + |
| + for _, rt := range adminRoutes { |
| + var rec *httptest.ResponseRecorder |
| + if rt.method == http.MethodGet { |
| + rec = get(t, h, rt.path) |
| + } else { |
| + rec = post(t, h, rt.path, url.Values{"address": {adminAddr}}) |
| + } |
| + if rec.Code != http.StatusNotFound { |
| + t.Errorf("%s %s: status %d, want 404", rt.method, rt.path, rec.Code) |
| + } |
| + } |
| + if h.Mail.count() != 0 { |
| + t.Error("a request to the unmounted admin surface still sent mail") |
| + } |
| +} |
| + |
| +// A refused address gets exactly the response an accepted one gets. Saying |
| +// "that address cannot sign in" would turn this endpoint into a way to |
| +// enumerate who the admins are. |
| +func TestSigninRefusesAStrangerWithoutSayingSo(t *testing.T) { |
| + h := newAdminApp(t) |
| + |
| + stranger := post(t, h, "/signin", url.Values{"address": {"nobody@example.com"}}) |
| + if h.Mail.countTo("nobody@example.com") != 0 { |
| + t.Error("a stranger's address was mailed a live sign-in link") |
| + } |
| + |
| + admin := post(t, h, "/signin", url.Values{"address": {adminAddr}}) |
| + |
| + if stranger.Code != admin.Code { |
| + t.Errorf("stranger got %d, admin got %d — the difference is an oracle", |
| + stranger.Code, admin.Code) |
| + } |
| + if stranger.Header().Get("Location") != admin.Header().Get("Location") { |
| + t.Errorf("stranger redirected to %q, admin to %q", |
| + stranger.Header().Get("Location"), admin.Header().Get("Location")) |
| + } |
| + if h.Mail.countTo(adminAddr) != 1 { |
| + t.Error("the admin was not sent a link") |
| + } |
| +} |
| + |
| +var anyLink = regexp.MustCompile(`https?://[^\s>"]+`) |
| + |
| +// signInAsAdmin drives the whole magic-link round trip and returns the |
| +// session cookies. |
| +func signInAsAdmin(t *testing.T, h *testApp) []*http.Cookie { |
| + t.Helper() |
| + post(t, h, "/signin", url.Values{"address": {adminAddr}}) |
| + body := h.Mail.waitForMailTo(t, adminAddr, 1) |
| + |
| + link := anyLink.FindString(body) |
| + if link == "" { |
| + t.Fatalf("no sign-in link in the message:\n%s", body) |
| + } |
| + u, err := url.Parse(link) |
| + if err != nil { |
| + t.Fatalf("parsing %q: %v", link, err) |
| + } |
| + |
| + req := httptest.NewRequest(http.MethodGet, u.RequestURI(), nil) |
| + rec := httptest.NewRecorder() |
| + h.ServeHTTP(rec, req) |
| + |
| + cookies := rec.Result().Cookies() |
| + if len(cookies) == 0 { |
| + t.Fatalf("redeeming the link minted no session (status %d)", rec.Code) |
| + } |
| + return cookies |
| +} |
| + |
| +func getAs(t *testing.T, h *testApp, target string, cookies []*http.Cookie) *httptest.ResponseRecorder { |
| + t.Helper() |
| + req := httptest.NewRequest(http.MethodGet, target, nil) |
| + for _, c := range cookies { |
| + req.AddCookie(c) |
| + } |
| + rec := httptest.NewRecorder() |
| + h.ServeHTTP(rec, req) |
| + return rec |
| +} |
| + |
| +// The export restriction is the one rule's first half made mechanical: a row |
| +// nobody has confirmed is not on the list, so it is not in the file. |
| +func TestExportCarriesOnlyConfirmedRows(t *testing.T) { |
| + h := newAdminApp(t) |
| + |
| + postSignup(t, h, submission(t, "in@example.com", "curious")) |
| + confirmSignup(t, h, "in@example.com") |
| + postSignup(t, h, submission(t, "waiting@example.com", "never")) |
| + |
| + cookies := signInAsAdmin(t, h) |
| + rec := getAs(t, h, "/admin/export.csv", cookies) |
| + |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("GET /admin/export.csv: %d", rec.Code) |
| + } |
| + body := rec.Body.String() |
| + if !strings.Contains(body, "in@example.com") { |
| + t.Error("a confirmed address is missing from the export") |
| + } |
| + if strings.Contains(body, "waiting@example.com") { |
| + t.Error("an unconfirmed address reached the export") |
| + } |
| +} |
| + |
| +// This file is opened in a spreadsheet, and a cell beginning =, +, -, @, tab |
| +// or carriage return executes when it is. The background field is free text |
| +// somebody else wrote, which makes it the obvious carrier. |
| +func TestExportDefusesFormulaInjection(t *testing.T) { |
| + h := newAdminApp(t) |
| + |
| + form := submission(t, "sneaky@example.com", "curious") |
| + form.Set("background", `=cmd|' /c calc'!A1`) |
| + postSignup(t, h, form) |
| + confirmSignup(t, h, "sneaky@example.com") |
| + |
| + cookies := signInAsAdmin(t, h) |
| + body := getAs(t, h, "/admin/export.csv", cookies).Body.String() |
| + |
| + if strings.Contains(body, ",=cmd") || strings.Contains(body, "\"=cmd") { |
| + t.Errorf("a formula reached the export unescaped:\n%s", body) |
| + } |
| + if !strings.Contains(body, "'=cmd") { |
| + t.Errorf("the escaping prefix is missing:\n%s", body) |
| + } |
| +} |
| + |
| +func TestAdminPageDoesNotListPendingRows(t *testing.T) { |
| + h := newAdminApp(t) |
| + |
| + postSignup(t, h, submission(t, "shown@example.com", "curious")) |
| + confirmSignup(t, h, "shown@example.com") |
| + postSignup(t, h, submission(t, "hidden@example.com", "never")) |
| + |
| + cookies := signInAsAdmin(t, h) |
| + body := getAs(t, h, "/admin", cookies).Body.String() |
| + |
| + if !strings.Contains(body, "shown@example.com") { |
| + t.Error("a confirmed address is missing from the admin list") |
| + } |
| + if strings.Contains(body, "hidden@example.com") { |
| + t.Error("a pending address — unproven, and joined to a stance — was listed") |
| + } |
| +} |
| + |
| +func TestAdminNeedsASession(t *testing.T) { |
| + h := newAdminApp(t) |
| + |
| + for _, path := range []string{"/admin", "/admin/export.csv"} { |
| + rec := get(t, h, path) |
| + if rec.Code == http.StatusOK { |
| + t.Errorf("GET %s answered 200 with no session", path) |
| + } |
| + } |
| +} |
| + |
| +// auth consults Authorize once, when a link is redeemed. Without a |
| +// per-request re-check, removing somebody from the allowlist would do nothing |
| +// until their session expired. |
| +// |
| +// The test restarts the app rather than changing an environment variable |
| +// mid-run: a process's environment is fixed at exec, so t.Setenv would pass |
| +// while proving nothing about how removal actually takes effect. |
| +func TestRemovedAdminLosesAccessOnTheNextRequest(t *testing.T) { |
| + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) |
| + dir := t.TempDir() |
| + |
| + box := &recorder{} |
| + env := testEnv() |
| + env.Admins = []string{adminAddr} |
| + env.Mailer = box |
| + |
| + d, err := db.Open(filepath.Join(dir, "app.db"), logger) |
| + if err != nil { |
| + t.Fatalf("db.Open: %v", err) |
| + } |
| + movement.SetLogger(logger) |
| + mux, err := movement.App(d, env) |
| + if err != nil { |
| + t.Fatalf("App: %v", err) |
| + } |
| + first := &testApp{Handler: mux, DB: d, Mail: box} |
| + |
| + cookies := signInAsAdmin(t, first) |
| + if rec := getAs(t, first, "/admin", cookies); rec.Code != http.StatusOK { |
| + t.Fatalf("setup: admin got %d before removal", rec.Code) |
| + } |
| + d.Close() |
| + |
| + // Same database, same session cookie, allowlist without them — which is |
| + // the state the platform leaves behind after `carlos env set` converges |
| + // and bounces the instance. |
| + d2, err := db.Open(filepath.Join(dir, "app.db"), logger) |
| + if err != nil { |
| + t.Fatalf("reopening: %v", err) |
| + } |
| + t.Cleanup(func() { d2.Close() }) |
| + env.Admins = []string{"someone.else@example.com"} |
| + mux2, err := movement.App(d2, env) |
| + if err != nil { |
| + t.Fatalf("App: %v", err) |
| + } |
| + second := &testApp{Handler: mux2, DB: d2, Mail: box} |
| + |
| + if rec := getAs(t, second, "/admin", cookies); rec.Code == http.StatusOK { |
| + t.Error("a removed admin still reached /admin with their old session") |
| + } |
| +} |
diff --git a/internal/movementtest/sweep_test.go b/internal/movementtest/sweep_test.go| new file mode 100644 |
| index 0000000..2361a33 |
| --- /dev/null |
| +++ b/internal/movementtest/sweep_test.go |
| @@ -0,0 +1,157 @@ |
| +package movementtest |
| + |
| +import ( |
| + "net/http" |
| + "net/http/httptest" |
| + "strings" |
| + "testing" |
| + "time" |
| + |
| + movement "amadan.net/carlos/movement/internal/movement" |
| +) |
| + |
| +const tickToken = "test-tick-token" |
| + |
| +// tick posts the way the platform's scheduler does: a bearer token, and none |
| +// of the three headers a browser would send. |
| +func tick(t *testing.T, h *testApp) *httptest.ResponseRecorder { |
| + t.Helper() |
| + req := httptest.NewRequest(http.MethodPost, "/tick/sweep", strings.NewReader("")) |
| + req.Header.Set("Authorization", "Bearer "+tickToken) |
| + rec := httptest.NewRecorder() |
| + h.ServeHTTP(rec, req) |
| + return rec |
| +} |
| + |
| +// The gate this whole route exists behind. |
| +// |
| +// csrf.SameOrigin refuses any POST carrying no Sec-Fetch-Site, Origin or |
| +// Referer, and the scheduler sends none of them. Mounted inside the protected |
| +// router the tick would answer 403 on every delivery, forever, and the |
| +// platform reads 4xx as "do not retry" — so the sweep would simply never run |
| +// and nothing would say so. |
| +func TestTickReachesTheSweepWithoutBrowserHeaders(t *testing.T) { |
| + t.Setenv("CARLOS_ADMIN_TOKEN", tickToken) |
| + h := newApp(t) |
| + |
| + rec := tick(t, h) |
| + |
| + if rec.Code == http.StatusForbidden { |
| + t.Fatal("the tick was refused by CSRF; it is mounted inside the protected router") |
| + } |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("POST /tick/sweep: %d, want 200", rec.Code) |
| + } |
| +} |
| + |
| +// Fails closed off-platform: with no token configured there is no public |
| +// delete endpoint sitting on the internet. |
| +func TestTickIsRefusedWithoutTheRightBearer(t *testing.T) { |
| + t.Setenv("CARLOS_ADMIN_TOKEN", tickToken) |
| + h := newApp(t) |
| + |
| + req := httptest.NewRequest(http.MethodPost, "/tick/sweep", nil) |
| + req.Header.Set("Authorization", "Bearer wrong") |
| + rec := httptest.NewRecorder() |
| + h.ServeHTTP(rec, req) |
| + |
| + if rec.Code == http.StatusOK { |
| + t.Error("a wrong bearer ran the sweep") |
| + } |
| +} |
| + |
| +func TestTickIsRefusedWhenNoTokenIsConfigured(t *testing.T) { |
| + t.Setenv("CARLOS_ADMIN_TOKEN", "") |
| + h := newApp(t) |
| + |
| + if rec := tick(t, h); rec.Code == http.StatusOK { |
| + t.Error("the sweep ran on an instance with no admin token; it must fail closed") |
| + } |
| +} |
| + |
| +// Confirmed rows are the list. Nothing sweeps them, ever. |
| +func TestSweepSparesConfirmedRowsAndFreshPendingOnes(t *testing.T) { |
| + t.Setenv("CARLOS_ADMIN_TOKEN", tickToken) |
| + h := newApp(t) |
| + |
| + postSignup(t, h, submission(t, "member@example.com", "curious")) |
| + confirmSignup(t, h, "member@example.com") |
| + postSignup(t, h, submission(t, "recent@example.com", "never")) |
| + |
| + // An old, unconfirmed row: exactly what the sweep is for. |
| + postSignup(t, h, submission(t, "abandoned@example.com", "never")) |
| + ageSignup(t, h, "abandoned@example.com", 40*24*time.Hour) |
| + |
| + if rec := tick(t, h); rec.Code != http.StatusOK { |
| + t.Fatalf("sweep: %d", rec.Code) |
| + } |
| + |
| + if got := countSignups(t, h, "member@example.com"); got != 1 { |
| + t.Error("the sweep deleted a confirmed row") |
| + } |
| + if got := countSignups(t, h, "recent@example.com"); got != 1 { |
| + t.Error("the sweep deleted a pending row that is still within its window") |
| + } |
| + if got := countSignups(t, h, "abandoned@example.com"); got != 0 { |
| + t.Error("an abandoned row nobody confirmed survived the sweep") |
| + } |
| +} |
| + |
| +func TestSweepClearsExpiredNoncesOnly(t *testing.T) { |
| + t.Setenv("CARLOS_ADMIN_TOKEN", tickToken) |
| + h := newApp(t) |
| + |
| + postSignup(t, h, submission(t, "fresh@example.com", "curious")) |
| + if countNonces(t, h) != 1 { |
| + t.Fatal("setup: expected one spent nonce") |
| + } |
| + |
| + if rec := tick(t, h); rec.Code != http.StatusOK { |
| + t.Fatalf("sweep: %d", rec.Code) |
| + } |
| + if got := countNonces(t, h); got != 1 { |
| + t.Errorf("the sweep dropped a nonce that can still be replayed (%d left)", got) |
| + } |
| + |
| + // Age it past the point where a challenge would be refused on age alone. |
| + if err := h.DB.G.Model(&movement.SpentNonce{}). |
| + Where("1 = 1"). |
| + Update("expires_at", time.Now().Add(-time.Hour)).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + tick(t, h) |
| + if got := countNonces(t, h); got != 0 { |
| + t.Errorf("%d expired nonces survived", got) |
| + } |
| +} |
| + |
| +// The sweep runs on every delivery and at-least-once delivery is the |
| +// platform's contract, so running it twice must be the same as running it |
| +// once. That is why it needs no occurrence-key dedupe. |
| +func TestSweepIsIdempotent(t *testing.T) { |
| + t.Setenv("CARLOS_ADMIN_TOKEN", tickToken) |
| + h := newApp(t) |
| + |
| + postSignup(t, h, submission(t, "kept@example.com", "curious")) |
| + confirmSignup(t, h, "kept@example.com") |
| + |
| + for i := 0; i < 3; i++ { |
| + if rec := tick(t, h); rec.Code != http.StatusOK { |
| + t.Fatalf("sweep %d: %d", i, rec.Code) |
| + } |
| + } |
| + if got := countSignups(t, h, "kept@example.com"); got != 1 { |
| + t.Error("repeated sweeps changed the result") |
| + } |
| +} |
| + |
| +func ageSignup(t *testing.T, h *testApp, email string, by time.Duration) { |
| + t.Helper() |
| + err := h.DB.G.Model(&movement.Signup{}). |
| + Where("email = ?", email). |
| + Update("created_at", time.Now().Add(-by)).Error |
| + if err != nil { |
| + t.Fatalf("ageing %s: %v", email, err) |
| + } |
| +} |