| 1 | package docs |
| 2 | |
| 3 | import ( |
| 4 | "bytes" |
| 5 | "context" |
| 6 | "net/http" |
| 7 | "net/http/httptest" |
| 8 | "net/url" |
| 9 | "path/filepath" |
| 10 | "strings" |
| 11 | "testing" |
| 12 | ) |
| 13 | |
| 14 | // loggingMailerStub is a minimal mail.Sender good enough to satisfy |
| 15 | // Config.Mailer's own "non-nil outside development" fail-closed |
| 16 | // check (buildMailer, auth.go), so |
| 17 | // TestNewRefusesMissingAllowlistOutsideDevelopment reaches the |
| 18 | // allowlist check it actually means to pin rather than failing |
| 19 | // earlier on an unrelated one. It never actually sends anything — |
| 20 | // nothing in that test signs in. |
| 21 | type loggingMailerStub struct{} |
| 22 | |
| 23 | func (loggingMailerStub) Send(ctx context.Context, to, subject, body string) error { return nil } |
| 24 | |
| 25 | // newAllowlistedTestApp builds a development App exactly like |
| 26 | // NewForTest (testing.go), except with AllowedAddresses set |
| 27 | // explicitly. NewForTest itself cannot exercise the allowlist at all: |
| 28 | // an empty AllowedAddresses in development means "admit everyone" |
| 29 | // (resolveAllowlist's own doc comment, auth.go) — precisely so |
| 30 | // TestSignIn and every other test built on NewForTest need no |
| 31 | // allowlist setup of their own. These tests are the ones that |
| 32 | // actually need a non-default allowlist configured, so they build |
| 33 | // their own App via New directly, package-internal (like |
| 34 | // origin_test.go and render_test.go) so they can also reach |
| 35 | // app.mailRecorder — unexported, and deliberately not something |
| 36 | // testing.go exposes outside this package: nothing outside auth.go's |
| 37 | // own tests should need to know whether mail was sent versus merely |
| 38 | // that a link came back, and TestSignIn already covers the latter. |
| 39 | func newAllowlistedTestApp(t *testing.T, allowed string) *App { |
| 40 | t.Helper() |
| 41 | app, err := New(Config{ |
| 42 | DBPath: filepath.Join(t.TempDir(), "docs.db"), |
| 43 | Env: "development", |
| 44 | AllowedAddresses: allowed, |
| 45 | }) |
| 46 | if err != nil { |
| 47 | t.Fatalf("New: %v", err) |
| 48 | } |
| 49 | t.Cleanup(func() { |
| 50 | if cerr := app.Close(); cerr != nil { |
| 51 | t.Errorf("App.Close: %v", cerr) |
| 52 | } |
| 53 | }) |
| 54 | return app |
| 55 | } |
| 56 | |
| 57 | // postSignin POSTs address to /signin as a same-origin browser form |
| 58 | // would, with force=1 — the same escape hatch TestSignIn (testing.go) |
| 59 | // uses to skip signin's own keymail classification (which would |
| 60 | // otherwise make a real outbound DNS/HTTPS call per address, per |
| 61 | // signin's own README, entirely beside the point of what these tests |
| 62 | // check). force=1 selects email but does not bypass the mail queue's admission check. |
| 63 | func postSignin(app *App, address string) *httptest.ResponseRecorder { |
| 64 | form := url.Values{"address": {address}, "force": {"1"}} |
| 65 | req := httptest.NewRequest(http.MethodPost, "/signin", strings.NewReader(form.Encode())) |
| 66 | req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| 67 | req.Header.Set("Sec-Fetch-Site", "same-origin") |
| 68 | rec := httptest.NewRecorder() |
| 69 | app.Handler().ServeHTTP(rec, req) |
| 70 | return rec |
| 71 | } |
| 72 | |
| 73 | // lastMailBody reads back app.mailRecorder's most recently recorded |
| 74 | // send, "" if nothing has been sent yet (or since the caller last |
| 75 | // cleared it) — the direct way to observe whether Begin actually ran, |
| 76 | // as opposed to TestSignIn's own success-shaped check (a link came |
| 77 | // back), which a test proving the ABSENCE of a send cannot use: there |
| 78 | // is no link to find in an empty body. |
| 79 | func lastMailBody(app *App) string { |
| 80 | app.signinMail.wait() // links are sent after the response |
| 81 | app.mailRecorder.mu.Lock() |
| 82 | defer app.mailRecorder.mu.Unlock() |
| 83 | return app.mailRecorder.lastBody |
| 84 | } |
| 85 | |
| 86 | // TestNonPermittedAddressSendsNoMail is this task's whole point, |
| 87 | // stated as literally as possible: the defect (Config.AllowedAddresses' |
| 88 | // own doc comment on Config has the full story — credited to the |
| 89 | // CARLOS Sheets session's finding on their own deployed instance) was |
| 90 | // never that a non-permitted address could sign in; rauth.Config. |
| 91 | // Authorize already stopped that, once wired. It was that Begin sends |
| 92 | // mail to any address at all, regardless of Authorize, because |
| 93 | // Authorize is only ever consulted afterward, at the callback. This |
| 94 | // is the test whose absence let that ship: every existing test in |
| 95 | // this package (before this task) proved something about admission, |
| 96 | // and none of them ever looked at whether a Send happened. Before |
| 97 | // the delivery gate existed, this test failed exactly this way — see this |
| 98 | // file's own commit message for how that was confirmed live, not |
| 99 | // assumed. |
| 100 | func TestNonPermittedAddressSendsNoMail(t *testing.T) { |
| 101 | app := newAllowlistedTestApp(t, "permitted@example.com") |
| 102 | |
| 103 | rec := postSignin(app, "stranger@evil.example") |
| 104 | |
| 105 | if rec.Code != http.StatusSeeOther { |
| 106 | t.Fatalf("POST /signin for a non-permitted address: status %d, want %d", rec.Code, http.StatusSeeOther) |
| 107 | } |
| 108 | if loc := rec.Header().Get("Location"); !strings.Contains(loc, "sent=1") { |
| 109 | t.Fatalf("POST /signin for a non-permitted address redirected to %q, want a %q outcome — the refusal must be invisible, not merely non-fatal", loc, "?sent=1") |
| 110 | } |
| 111 | if body := lastMailBody(app); body != "" { |
| 112 | t.Fatalf("POST /signin for a non-permitted address sent mail (recorded body: %q) — Begin must never run for an address the allowlist refuses", body) |
| 113 | } |
| 114 | } |
| 115 | |
| 116 | // TestSigninResponseIdenticalForNonPermittedAddress is fix |
| 117 | // requirement 3, checked the way its own reasoning demands: not |
| 118 | // "both responses are a 303" but the actual bytes — status, Location |
| 119 | // and body — compared directly. Any observable difference here would |
| 120 | // turn POST /signin into a membership oracle: an attacker who can |
| 121 | // tell "sent" from "not sent" has learned whether a given address is |
| 122 | // on this instance's allowlist, which is exactly the kind of leak an |
| 123 | // admission check alone would not create (it would 403 visibly) but |
| 124 | // a badly-built send-time gate could. |
| 125 | func TestSigninResponseIdenticalForNonPermittedAddress(t *testing.T) { |
| 126 | app := newAllowlistedTestApp(t, "permitted@example.com") |
| 127 | |
| 128 | permitted := postSignin(app, "permitted@example.com") |
| 129 | nonPermitted := postSignin(app, "stranger@evil.example") |
| 130 | |
| 131 | if permitted.Code != http.StatusSeeOther { |
| 132 | t.Fatalf("POST /signin for the permitted address: status %d, want %d", permitted.Code, http.StatusSeeOther) |
| 133 | } |
| 134 | if got, want := nonPermitted.Code, permitted.Code; got != want { |
| 135 | t.Errorf("status differs: non-permitted=%d permitted=%d", got, want) |
| 136 | } |
| 137 | if got, want := nonPermitted.Header().Get("Location"), permitted.Header().Get("Location"); got != want { |
| 138 | t.Errorf("Location differs: non-permitted=%q permitted=%q", got, want) |
| 139 | } |
| 140 | if got, want := nonPermitted.Header().Get("Content-Type"), permitted.Header().Get("Content-Type"); got != want { |
| 141 | t.Errorf("Content-Type differs: non-permitted=%q permitted=%q", got, want) |
| 142 | } |
| 143 | if got, want := nonPermitted.Body.Bytes(), permitted.Body.Bytes(); !bytes.Equal(got, want) { |
| 144 | t.Errorf("body differs: non-permitted=%q permitted=%q", got, want) |
| 145 | } |
| 146 | } |
| 147 | |
| 148 | // TestPermittedAddressStillSignsIn proves the fix does not break the |
| 149 | // working path: a permitted address must still complete the full |
| 150 | // magic-link ceremony end to end, mirroring |
| 151 | // TestMagicLinkThenSignedInRouteWorks (auth_test.go) but against an |
| 152 | // App with a real, non-empty allowlist instead of NewForTest's |
| 153 | // admit-everyone default. |
| 154 | func TestPermittedAddressStillSignsIn(t *testing.T) { |
| 155 | app := newAllowlistedTestApp(t, "permitted@example.com") |
| 156 | |
| 157 | link, err := app.TestSignIn(t, "permitted@example.com") |
| 158 | if err != nil { |
| 159 | t.Fatalf("TestSignIn: %v", err) |
| 160 | } |
| 161 | |
| 162 | verifyReq := confirmRequest(link) |
| 163 | verifyRec := httptest.NewRecorder() |
| 164 | app.Handler().ServeHTTP(verifyRec, verifyReq) |
| 165 | cookies := verifyRec.Result().Cookies() |
| 166 | if len(cookies) == 0 { |
| 167 | t.Fatal("following the magic link set no session cookie") |
| 168 | } |
| 169 | |
| 170 | req := httptest.NewRequest(http.MethodGet, "/", nil) |
| 171 | for _, c := range cookies { |
| 172 | req.AddCookie(c) |
| 173 | } |
| 174 | rec := httptest.NewRecorder() |
| 175 | app.Handler().ServeHTTP(rec, req) |
| 176 | if rec.Code != http.StatusOK { |
| 177 | t.Fatalf("GET / signed in as a permitted address: status %d, want 200", rec.Code) |
| 178 | } |
| 179 | } |
| 180 | |
| 181 | // TestAllowlistIsCaseAndWhitespaceInsensitive is fix requirement 1's |
| 182 | // own explicit demand: real config files have spaces after their |
| 183 | // commas, and people type capitals — $DOCS_ALLOWED_ADDRESSES= |
| 184 | // "Permitted@Example.com" must still admit |
| 185 | // "permitted@example.com" and vice versa, in both directions |
| 186 | // (the configured entry's own case/whitespace, and the submitted |
| 187 | // address's). |
| 188 | func TestAllowlistIsCaseAndWhitespaceInsensitive(t *testing.T) { |
| 189 | app := newAllowlistedTestApp(t, " Permitted@Example.com , other@example.com") |
| 190 | |
| 191 | for _, variant := range []string{ |
| 192 | "permitted@example.com", |
| 193 | "PERMITTED@EXAMPLE.COM", |
| 194 | " permitted@example.com ", |
| 195 | "Permitted@Example.com", |
| 196 | } { |
| 197 | if _, err := app.TestSignIn(t, variant); err != nil { |
| 198 | t.Errorf("TestSignIn(%q): %v, want the allowlisted address to be admitted regardless of case or surrounding whitespace", variant, err) |
| 199 | } |
| 200 | } |
| 201 | } |
| 202 | |
| 203 | // TestResolveAllowlistParsesCommaSeparatedList pins |
| 204 | // resolveAllowlist's own parsing directly: comma-separated, each |
| 205 | // entry trimmed and lowercased, blank entries (a trailing comma, or |
| 206 | // stray whitespace between two commas) dropped rather than becoming a |
| 207 | // spurious "" that could never match a real address anyway but would |
| 208 | // otherwise clutter the set. |
| 209 | func TestResolveAllowlistParsesCommaSeparatedList(t *testing.T) { |
| 210 | list, err := resolveAllowlist(Config{ |
| 211 | Env: "production", |
| 212 | AllowedAddresses: " Alice@Example.com, bob@example.com ,, carol@example.com ", |
| 213 | }) |
| 214 | if err != nil { |
| 215 | t.Fatalf("resolveAllowlist: %v", err) |
| 216 | } |
| 217 | for _, addr := range []string{"alice@example.com", "ALICE@EXAMPLE.COM", "bob@example.com", "carol@example.com"} { |
| 218 | if !list.admits(addr) { |
| 219 | t.Errorf("list.admits(%q) = false, want true", addr) |
| 220 | } |
| 221 | } |
| 222 | if list.admits("mallory@example.com") { |
| 223 | t.Error("list.admits(\"mallory@example.com\") = true, want false — not on the configured list") |
| 224 | } |
| 225 | } |
| 226 | |
| 227 | // TestResolveAllowlistEmptyOutsideDevelopmentRefuses is fix |
| 228 | // requirement 4's decision, pinned directly: an unset |
| 229 | // $DOCS_ALLOWED_ADDRESSES must never mean "allow everyone" outside |
| 230 | // development. resolveAllowlist's own doc comment has the full |
| 231 | // reasoning for refusing at startup rather than the alternative |
| 232 | // (silently admitting nobody) — this test only pins the outcome: |
| 233 | // New must refuse to start, the same fail-closed shape |
| 234 | // TestNewRefusesMissingInstanceKeyOutsideDevelopment and |
| 235 | // TestNewRefusesMissingMailOutsideDevelopment (auth_test.go) already |
| 236 | // pin for InstanceKey and Mailer. |
| 237 | func TestResolveAllowlistEmptyOutsideDevelopmentRefuses(t *testing.T) { |
| 238 | for _, env := range []string{"production", "staging", ""} { |
| 239 | if _, err := resolveAllowlist(Config{Env: env}); err == nil { |
| 240 | t.Errorf("resolveAllowlist with Env %q and no AllowedAddresses: err = nil, want a refusal", env) |
| 241 | } |
| 242 | } |
| 243 | } |
| 244 | |
| 245 | // TestResolveAllowlistEmptyInDevelopmentAdmitsEveryone pins the other |
| 246 | // half of the same decision: development's own convenience default |
| 247 | // (TestSignIn, scripts/run-dev.sh) needs no allowlist configured at |
| 248 | // all, exactly as it needed no InstanceKey or Mailer configured |
| 249 | // before this task. |
| 250 | func TestResolveAllowlistEmptyInDevelopmentAdmitsEveryone(t *testing.T) { |
| 251 | list, err := resolveAllowlist(Config{Env: "development"}) |
| 252 | if err != nil { |
| 253 | t.Fatalf("resolveAllowlist: %v", err) |
| 254 | } |
| 255 | if !list.admits("anyone@example.com") { |
| 256 | t.Error("an empty AllowedAddresses in development must admit everyone") |
| 257 | } |
| 258 | } |
| 259 | |
| 260 | // TestNewRefusesMissingAllowlistOutsideDevelopment is the New-level |
| 261 | // version of TestResolveAllowlistEmptyOutsideDevelopmentRefuses, |
| 262 | // exercised the way a real misconfigured deployment would actually |
| 263 | // hit it — through New itself, alongside its own InstanceKey/Mailer |
| 264 | // siblings in auth_test.go. |
| 265 | func TestNewRefusesMissingAllowlistOutsideDevelopment(t *testing.T) { |
| 266 | _, err := New(Config{ |
| 267 | Env: "production", |
| 268 | Origin: "https://docs.example.com", |
| 269 | InstanceKey: "some-real-key", |
| 270 | Mailer: loggingMailerStub{}, |
| 271 | }) |
| 272 | if err == nil { |
| 273 | t.Fatal("New must refuse to start outside development with no allowlist configured") |
| 274 | } |
| 275 | } |
| 276 | |
amadan