Commit `cfc0bc7785`

cfc0bc778541d776a5b6eb5d0dc2accc80332700

parent: 3f7a91e284

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-05 04:02 UTC

web: a suspended account cannot mail itself a session

RequestLoginLink resolved an identifier to a bare user id and minted
without consulting disabled or pending, and login() created a session
without checking either. Every other mint path runs through Dispatch,
which refuses both.

RequestLoginLink now resolves to a store.User and returns nil for
disabled and pending, keeping the response indistinguishable from a
miss. login() refuses a disabled account after consuming the token, with
the message a bad token gets. SetUserDisabled deletes login_tokens
alongside web_sessions and api_tokens.

Also: checkOrigin's comment says Lax and that the Origin check is the
primary CSRF defense; cookieclear_test uses sessionSameSite instead of a
Strict literal it passed in itself; login()'s cookie literal moves to
sessionCookieFor, which logincookie_test uses to assert Lax, HttpOnly and
Secure under both TLS settings; the enumeration test pins its reference
body; the SMTP failure log carries a user id, not an address; the design
doc's signature matches the code.

Ref #155

Layout: unified · split

docs/plans/2026-09-04-email-login.md deleted −782

	@@ -1,782 +0,0 @@
1	# Browser login without an SSH key — Implementation Plan
2
3	> For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
4
5	Goal: Let a person with no SSH key log into the web UI by requesting a
6	one-time link at their verified email address.
7
8	Architecture: An anonymous `POST /login` calls a plain exported function in
9	`internal/control`, which resolves the identifier to a user, mints the same
10	one-time token `web login` already mints, and mails it. The existing
11	`GET /login?token=` handler consumes it unchanged. No new control command,
12	because the caller has no authenticated user to run one as.
13
14	Tech Stack: Go, SQLite (hand-written SQL, no ORM), `html/template`,
15	`internal/mail` over SMTP.
16
17	Spec: `docs/specs/2026-09-04-email-login-design.md`
18
19	## Global Constraints
20
21	- Branch `email-login`. Never push to `main`. One MR, `Closes #155`.
22	- Never attribute anything to an assistant or model, anywhere.
23	- No ORM. Hand-written SQL. Migrations in `internal/store/migrations/`.
24	- Comments state facts, not before/after commentary. Git history holds the rest.
25	- `--json` output is the contract; human output is not.
26	- Locally run build, vet, and the unit tests of touched packages plus the one
27	e2e test being written. Full e2e belongs to CI on bay1.
28	- Adding a top-level route means adding the word to `internal/policy/names.go`.
29	`/login` already exists, so this plan adds no reserved name.
30
31	## Change from the spec
32
33	The spec proposed `RequestLoginLink` returning `(msg, errMsg string, code int)`
34	to mirror `control.RegisterAccount`. It returns only `error` instead.
35	`RegisterAccount` reports per-case failures because registration is allowed to
36	say what went wrong; here, reporting anything about the outcome is precisely the
37	enumeration leak the spec forbids. The returned error is for the server log
38	only — SMTP down, database failure — and is never rendered. The handler draws
39	the same page whatever it gets back.
40
41	## File Structure
42
43	\| Path \| Responsibility \|
44	\|---\|---\|
45	\| `internal/store/migrations/0040_login_token_index.{up,down}.sql` \| index behind the throttle count \|
46	\| `internal/store/sessions.go` \| `CountLoginTokensSince`, beside `CreateLoginToken` \|
47	\| `internal/control/loginlink.go` \| new — resolve identifier, throttle, mint, mail \|
48	\| `internal/httpd/accounts.go` \| `loginSubmit`; session cookie `SameSite` \|
49	\| `internal/httpd/routes.go` \| `POST /login` \|
50	\| `internal/web/templates/login.html` \| request form and the sent confirmation \|
51	\| `internal/store/sessions_test.go` \| new — counter unit test \|
52	\| `internal/httpd/logincookie_test.go` \| new — cookie attribute test \|
53	\| `e2e/emaillogin_test.go` \| new — the whole path against real SMTP \|
54
55	---
56
57	### Task 1: The throttle counter and its index
58
59	`login_tokens` has no index on `user_id`, and rows are never deleted. Counting
60	per user on an anonymous endpoint would be an unbounded table scan on every
61	request, which makes the throttle its own denial-of-service vector.
62
63	Files:
64	- Create: `internal/store/migrations/0040_login_token_index.up.sql`
65	- Create: `internal/store/migrations/0040_login_token_index.down.sql`
66	- Modify: `internal/store/sessions.go` (append after `CreateLoginToken`, line 36)
67	- Create: `internal/store/sessions_test.go`
68
69	Interfaces:
70	- Consumes: `Store.CreateLoginToken(userID int64, hash string, ttl time.Duration) error`, `store.NewToken() (token, hash string, err error)` — both exist.
71	- Produces: `func (s *Store) CountLoginTokensSince(userID int64, since time.Time) (int, error)`
72
73	- [ ] Step 1: Write the failing test
74
75	Create `internal/store/sessions_test.go`:
76
77	```go
78	package store
79
80	import (
81	"testing"
82	"time"
83	)
84
85	func TestCountLoginTokensSince(t *testing.T) {
86	s := open(t)
87	if err := s.MigrateUp(); err != nil {
88	t.Fatal(err)
89	}
90	uid, err := s.CreateUser("cmc", true)
91	if err != nil {
92	t.Fatal(err)
93	}
94	for i := 0; i < 3; i++ {
95	_, hash, err := NewToken()
96	if err != nil {
97	t.Fatal(err)
98	}
99	if err := s.CreateLoginToken(uid, hash, time.Minute); err != nil {
100	t.Fatal(err)
101	}
102	}
103
104	n, err := s.CountLoginTokensSince(uid, time.Now().Add(-time.Hour))
105	if err != nil \|\| n != 3 {
106	t.Fatalf("count in the last hour = %d, %v; want 3", n, err)
107	}
108
109	// A window that opens in the future sees none of them, which is what
110	// makes the hourly bound a window rather than a lifetime total.
111	if n, err := s.CountLoginTokensSince(uid, time.Now().Add(time.Hour)); err != nil \|\| n != 0 {
112	t.Fatalf("count in a future window = %d, %v; want 0", n, err)
113	}
114
115	// One account's requests must not spend another account's budget.
116	other, err := s.CreateUser("kim", false)
117	if err != nil {
118	t.Fatal(err)
119	}
120	if n, err := s.CountLoginTokensSince(other, time.Now().Add(-time.Hour)); err != nil \|\| n != 0 {
121	t.Fatalf("other account count = %d, %v; want 0", n, err)
122	}
123	}
124	```
125
126	- [ ] Step 2: Run it and watch it fail
127
128	Run: `go test ./internal/store/ -run TestCountLoginTokensSince -v`
129	Expected: FAIL — `s.CountLoginTokensSince undefined`.
130
131	- [ ] Step 3: Write the migration
132
133	`internal/store/migrations/0040_login_token_index.up.sql`:
134
135	```sql
136	CREATE INDEX login_tokens_user_created ON login_tokens(user_id, created_at);
137	```
138
139	`internal/store/migrations/0040_login_token_index.down.sql`:
140
141	```sql
142	DROP INDEX login_tokens_user_created;
143	```
144
145	- [ ] Step 4: Write the counter
146
147	Append to `internal/store/sessions.go`, directly after `CreateLoginToken`:
148
149	```go
150	// CountLoginTokensSince counts the login tokens minted for a user within a
151	// window. An unauthenticated request can ask for a login link, so the mint
152	// needs a durable per-account bound the way email verification does (#136).
153	func (s *Store) CountLoginTokensSince(userID int64, since time.Time) (int, error) {
154	var n int
155	err := s.DB.QueryRow(
156	"SELECT count(*) FROM login_tokens WHERE user_id = ? AND created_at > ?",
157	userID, fmtTime(since)).Scan(&n)
158	return n, err
159	}
160	```
161
162	- [ ] Step 5: Run the test and the package
163
164	Run: `go test ./internal/store/ -run TestCountLoginTokensSince -v`
165	Expected: PASS.
166
167	Run: `go test ./internal/store/`
168	Expected: PASS — the new migration must not break existing store tests.
169
170	- [ ] Step 6: Confirm the index is actually used
171
172	The count runs on every anonymous request, so a sequential scan here would
173	make the throttle its own denial-of-service vector. Verify the plan names the
174	index:
175
176	```bash
177	sqlite3 "$SCRATCH/plan.db" <<'EOF'
178	CREATE TABLE login_tokens (token_hash TEXT PRIMARY KEY, user_id INTEGER NOT NULL,
179	created_at TEXT NOT NULL, expires_at TEXT NOT NULL, used_at TEXT);
180	CREATE INDEX login_tokens_user_created ON login_tokens(user_id, created_at);
181	EXPLAIN QUERY PLAN SELECT count(*) FROM login_tokens WHERE user_id = 1 AND created_at > 'x';
182	EOF
183	```
184
185	Expected: the output names `login_tokens_user_created`. A line reading
186	`SCAN login_tokens` means the index is not being used and the migration is
187	wrong.
188
189	- [ ] Step 7: Commit
190
191	```bash
192	git add internal/store/sessions.go internal/store/sessions_test.go \
193	internal/store/migrations/0040_login_token_index.up.sql \
194	internal/store/migrations/0040_login_token_index.down.sql
195	git commit -m "store: count login tokens per account, indexed
196
197	Ref #155"
198	```
199
200	---
201
202	### Task 2: Session cookie SameSite
203
204	A link clicked in a webmail client is a cross-site top-level navigation. Under
205	`SameSite=Strict` the redirect chain to `/` can arrive without the cookie, so
206	the visitor lands logged out and is logged in only after a refresh. Pasting a
207	URL into the address bar does not hit this, which is why the SSH flow never
208	showed it.
209
210	`Lax` is safe here, and it was verified rather than assumed: every
211	cookie-authenticated mutating route carries `checkOrigin`. The four POST routes
212	without it — `git-upload-pack`, `git-receive-pack`, `lfs/objects/batch`,
213	`/api/v1/cmd` — do not accept the session cookie at all; the API takes only
214	`Authorization: Bearer` (`internal/httpd/api.go:126`). For POST, `Lax` is
215	strictly stronger than the Origin check, because it withholds the cookie
216	outright. The only `Mutating: true` GET is `/login` itself, whose token is
217	single-use.
218
219	Files:
220	- Modify: `internal/httpd/accounts.go:92` (set), and the `clearCookie` call in `logout`
221	- Create: `internal/httpd/logincookie_test.go`
222
223	Interfaces:
224	- Consumes: `Server.clearCookie(name string, sameSite http.SameSite) *http.Cookie` (`internal/httpd/flash.go:55`), `sessionCookie`
225	- Produces: nothing new; changes an attribute value.
226
227	- [ ] Step 1: Write the failing test
228
229	Create `internal/httpd/logincookie_test.go`:
230
231	```go
232	package httpd
233
234	import (
235	"net/http"
236	"testing"
237
238	"gitbay.org/gitbay/internal/config"
239	)
240
241	// The session cookie must be Lax, not Strict. A login link clicked in a mail
242	// client is a cross-site top-level navigation, and Strict can withhold the
243	// cookie through the redirect that follows, so the visitor lands logged out
244	// (#155). Cross-site POSTs stay protected: Lax withholds the cookie from them,
245	// and checkOrigin refuses them besides.
246	func TestSessionCookieIsLax(t *testing.T) {
247	s := &Server{cfg: config.Config{}}
248	s.cfg.HTTP.TLS = "acme"
249	if got := s.clearCookie(sessionCookie, sessionSameSite); got.SameSite != http.SameSiteLaxMode {
250	t.Errorf("clearing cookie SameSite = %v, want Lax", got.SameSite)
251	}
252	if sessionSameSite != http.SameSiteLaxMode {
253	t.Errorf("sessionSameSite = %v, want Lax", sessionSameSite)
254	}
255	}
256	```
257
258	- [ ] Step 2: Run it and watch it fail
259
260	Run: `go test ./internal/httpd/ -run TestSessionCookieIsLax -v`
261	Expected: FAIL — `undefined: sessionSameSite`.
262
263	- [ ] Step 3: Introduce the constant and use it in both places
264
265	In `internal/httpd/accounts.go`, near the `sessionCookie` declaration, add:
266
267	```go
268	// sessionSameSite is Lax so a login link followed from a mail client keeps
269	// its session through the redirect. Cross-site POSTs are refused by
270	// checkOrigin and carry no Lax cookie anyway.
271	const sessionSameSite = http.SameSiteLaxMode
272	```
273
274	In the `login` handler (`internal/httpd/accounts.go:92`), replace
275	`SameSite: http.SameSiteStrictMode,` with `SameSite: sessionSameSite,`.
276
277	In the `logout` handler, replace
278	`s.clearCookie(sessionCookie, http.SameSiteStrictMode)` with
279	`s.clearCookie(sessionCookie, sessionSameSite)` so the set and clear paths
280	match, which is the rule `TestClearCookieMirrorsTheSettingCall` exists to keep.
281
282	- [ ] Step 4: Run the package tests
283
284	Run: `go test ./internal/httpd/`
285	Expected: PASS, including the pre-existing
286	`TestClearCookieMirrorsTheSettingCall`, which passes its own `SameSite`
287	argument and is unaffected.
288
289	- [ ] Step 5: Commit
290
291	```bash
292	git add internal/httpd/accounts.go internal/httpd/logincookie_test.go
293	git commit -m "httpd: session cookie is SameSite=Lax
294
295	A link followed from a mail client is a cross-site navigation; Strict can
296	drop the cookie through the redirect after /login?token=. Every
297	cookie-authenticated mutating route carries checkOrigin, and Lax withholds
298	the cookie from cross-site POSTs regardless.
299
300	Ref #155"
301	```
302
303	---
304
305	### Task 3: The login link request
306
307	The deliverable. The function, the route, the handler, and the form land
308	together because none of them is testable without the others.
309
310	Files:
311	- Create: `internal/control/loginlink.go`
312	- Modify: `internal/httpd/accounts.go` (`renderLogin`, new `loginSubmit`)
313	- Modify: `internal/httpd/routes.go:100` (add `POST /login`)
314	- Modify: `internal/web/templates/login.html`
315	- Create: `e2e/emaillogin_test.go`
316
317	Interfaces:
318	- Consumes: `store.UserIDByVerifiedEmail(address string) (int64, bool)` (`internal/store/activity.go:11`); `store.PrimaryVerifiedEmail(userID int64) (string, error)` (`internal/store/mrs.go:440`); `store.UserByUsername`; `store.CountLoginTokensSince` (Task 1); `store.NewToken`; `store.CreateLoginToken`; `mail.Send(cfg config.Config, to, subject, body string) error`; `Server.apiLimit.allow(key string, write bool) (bool, time.Duration)`; `Server.clientIP(r)`.
319	- Produces: `func control.RequestLoginLink(cfg config.Config, st *store.Store, identifier string) error`
320
321	- [ ] Step 1: Write the failing e2e test
322
323	Create `e2e/emaillogin_test.go`:
324
325	```go
326	package e2e
327
328	import (
329	"fmt"
330	"net/url"
331	"strings"
332	"testing"
333	)
334
335	// A person with no SSH key can still get into the web UI: they ask for a
336	// link by username or verified address and it arrives by mail (#155).
337	func TestEmailLogin(t *testing.T) {
338	smtp := startFakeSMTP(t)
339	inst := startInstanceWith(t, fmt.Sprintf(
340	"[web]\nmode = \"accounts\"\n[mail]\nsmtp_host = %q\nfrom = \"noreply@gitbay.test\"\n",
341	smtp.addr))
342
343	// No --key: this account has no way to authenticate over SSH at all,
344	// which is the whole point.
345	inst.admin(t, "admin", "user", "create", "dana",
346	"--email", "dana@example.test", "--verified")
347
348	browser := newBrowser(t)
349	status, body := browserPost(t, browser, inst.base()+"/login",
350	url.Values{"identifier": {"dana@example.test"}})
351	if status != 200 {
352	t.Fatalf("POST /login: %d", status)
353	}
354	if !strings.Contains(body, "on its way") {
355	t.Fatalf("no confirmation in body: %s", body)
356	}
357
358	msg := smtp.waitFor(t, "dana@example.test", "/login?token=")
359	i := strings.Index(msg, "/login?token=")
360	link := msg[i:]
361	if j := strings.IndexAny(link, " \r\n"); j >= 0 {
362	link = link[:j]
363	}
364
365	if status, _ := browserGet(t, browser, inst.base()+link); status != 200 {
366	t.Fatalf("following the link: %d", status)
367	}
368	status, body = browserGet(t, browser, inst.base()+"/settings")
369	if status != 200 \|\| !strings.Contains(body, "dana@example.test") {
370	t.Fatalf("not logged in after the link: %d", status)
371	}
372
373	// The link is single use.
374	second := newBrowser(t)
375	browserGet(t, second, inst.base()+link)
376	if status, _ := browserGet(t, second, inst.base()+"/settings"); status == 200 {
377	t.Error("login link worked twice")
378	}
379	}
380
381	// The response must not say whether an account exists. A different status,
382	// body, or destination answers "is this person here?" to anyone who asks.
383	func TestEmailLoginDoesNotEnumerate(t *testing.T) {
384	smtp := startFakeSMTP(t)
385	inst := startInstanceWith(t, fmt.Sprintf(
386	"[web]\nmode = \"accounts\"\n[mail]\nsmtp_host = %q\nfrom = \"noreply@gitbay.test\"\n",
387	smtp.addr))
388	inst.admin(t, "admin", "user", "create", "dana",
389	"--email", "dana@example.test", "--verified")
390	// An account whose address was never verified must look like an absent
391	// one, or an unverified address becomes an oracle.
392	inst.admin(t, "admin", "user", "create", "eve", "--email", "eve@example.test")
393
394	browser := newBrowser(t)
395	real1, bodyReal := browserPost(t, browser, inst.base()+"/login",
396	url.Values{"identifier": {"dana@example.test"}})
397	absent, bodyAbsent := browserPost(t, browser, inst.base()+"/login",
398	url.Values{"identifier": {"nobody@example.test"}})
399	unver, bodyUnver := browserPost(t, browser, inst.base()+"/login",
400	url.Values{"identifier": {"eve@example.test"}})
401	empty, bodyEmpty := browserPost(t, browser, inst.base()+"/login",
402	url.Values{"identifier": {""}})
403
404	for _, c := range []struct {
405	name string
406	status int
407	body string
408	}{
409	{"absent", absent, bodyAbsent},
410	{"unverified", unver, bodyUnver},
411	{"empty", empty, bodyEmpty},
412	} {
413	if c.status != real1 \|\| c.body != bodyReal {
414	t.Errorf("%s differs from a real address: status %d vs %d", c.name, c.status, real1)
415	}
416	}
417	if len(smtp.mailTo("eve@example.test")) != 0 {
418	t.Error("mailed an unverified address")
419	}
420	if len(smtp.mailTo("nobody@example.test")) != 0 {
421	t.Error("mailed an address with no account")
422	}
423	}
424
425	// An anonymous endpoint that sends mail needs a durable per-account bound,
426	// the same one email verification has (#136).
427	func TestEmailLoginThrottled(t *testing.T) {
428	smtp := startFakeSMTP(t)
429	inst := startInstanceWith(t, fmt.Sprintf(
430	"[web]\nmode = \"accounts\"\n[mail]\nsmtp_host = %q\nfrom = \"noreply@gitbay.test\"\n",
431	smtp.addr))
432	inst.admin(t, "admin", "user", "create", "dana",
433	"--email", "dana@example.test", "--verified")
434
435	browser := newBrowser(t)
436	for i := 0; i < 6; i++ {
437	browserPost(t, browser, inst.base()+"/login",
438	url.Values{"identifier": {"dana@example.test"}})
439	}
440	if n := len(smtp.mailTo("dana@example.test")); n > 5 {
441	t.Fatalf("sent %d login mails in an hour, want at most 5", n)
442	}
443	}
444	```
445
446	- [ ] Step 2: Run it and watch it fail
447
448	Run: `go test ./e2e/ -run TestEmailLogin -v -timeout 20m`
449	Expected: FAIL — `POST /login: 405`, because no such route exists.
450
451	Note the explicit `-timeout`: `go test` defaults to 10 minutes and the e2e
452	suite has exceeded it before, which reads as a hang rather than a failure
453	(#143).
454
455	- [ ] Step 3: Write the control function
456
457	Create `internal/control/loginlink.go`:
458
459	```go
460	package control
461
462	import (
463	"fmt"
464	"strings"
465	"time"
466
467	"gitbay.org/gitbay/internal/config"
468	"gitbay.org/gitbay/internal/mail"
469	"gitbay.org/gitbay/internal/store"
470	)
471
472	// maxLoginLinksPerHour bounds what one account's address can be made to
473	// receive. It matches maxEmailAddsPerHour: enough for a person who mistypes
474	// and retries, nothing for a script.
475	const maxLoginLinksPerHour = 5
476
477	// loginLinkTTL is longer than the five minutes an SSH-minted link gets.
478	// That one is pasted from a terminal already open; this one has to survive
479	// delivery and someone noticing the mail.
480	const loginLinkTTL = 15 * time.Minute
481
482	// RequestLoginLink mails a one-time login link to the account named by
483	// identifier, which is a username or a verified email address.
484	//
485	// It is not a registered command: the caller is an unauthenticated web
486	// request, and commands run as c.User. RegisterAccount is exported for the
487	// same reason.
488	//
489	// The returned error is for the server log only. Nothing about the outcome
490	// may reach the caller — that a request found an account, found one without
491	// a verified address, or found nothing at all must be indistinguishable, or
492	// the endpoint answers "does this person have an account here?" to anyone
493	// who asks. Every miss returns nil.
494	func RequestLoginLink(cfg config.Config, st *store.Store, identifier string) error {
495	if cfg.Web.Mode != "accounts" \|\| cfg.Mail.SMTPHost == "" {
496	return nil
497	}
498	identifier = strings.TrimSpace(identifier)
499	if identifier == "" {
500	return nil
501	}
502
503	var userID int64
504	var address string
505	if strings.Contains(identifier, "@") {
506	id, ok := st.UserIDByVerifiedEmail(identifier)
507	if !ok {
508	return nil
509	}
510	userID, address = id, identifier
511	} else {
512	u, err := st.UserByUsername(identifier)
513	if err != nil {
514	return nil
515	}
516	addr, err := st.PrimaryVerifiedEmail(u.ID)
517	if err != nil \|\| addr == "" {
518	return nil
519	}
520	userID, address = u.ID, addr
521	}
522
523	n, err := st.CountLoginTokensSince(userID, time.Now().Add(-time.Hour))
524	if err != nil {
525	return err
526	}
527	if n >= maxLoginLinksPerHour {
528	return nil
529	}
530
531	token, hash, err := store.NewToken()
532	if err != nil {
533	return err
534	}
535	if err := st.CreateLoginToken(userID, hash, loginLinkTTL); err != nil {
536	return err
537	}
538	host := siteHost(cfg)
539	body := fmt.Sprintf(
540	"Someone (hopefully you) asked to log in to %s.\n\n"+
541	"Open this link within 15 minutes. It works once:\n\n %s/login?token=%s\n\n"+
542	"If this wasn't you, ignore this mail. Nothing has changed on the account.\n",
543	host, strings.TrimSuffix(cfg.Server.SiteURL, "/"), token)
544	return mail.Send(cfg, address, "log in to "+host, body)
545	}
546	```
547
548	`UserByUsername(name string) (User, error)` is at `internal/store/users.go:109`.
549
550	- [ ] Step 4: Write the handler
551
552	In `internal/httpd/accounts.go`, replace `renderLogin` and add `loginSubmit`:
553
554	```go
555	// renderLogin draws the login page. Mode carries the registration mode so
556	// the page can tell a brand-new visitor how to get an account. EmailLogin
557	// says whether this instance can mail a link; Sent switches the page to the
558	// confirmation that follows a request.
559	func (s *Server) renderLogin(w http.ResponseWriter, errMsg string, sent bool) {
560	s.render(w, "login.html", struct {
561	basePage
562	Mode string // closed \| invite \| open
563	Error string
564	EmailLogin bool
565	Sent bool
566	}{basePage{Site: s.siteName(), Host: s.cfg.SiteHost()},
567	s.cfg.Registration.Mode, errMsg, s.emailLoginEnabled(), sent})
568	}
569
570	// emailLoginEnabled reports whether a link can be mailed at all. There is no
571	// separate switch: the capability is exactly the SMTP the instance already
572	// configured for verification and notification mail.
573	func (s *Server) emailLoginEnabled() bool {
574	return s.cfg.Web.Mode == "accounts" && s.cfg.Mail.SMTPHost != ""
575	}
576
577	// loginSubmit mails a one-time login link. The response is the same page
578	// whatever happened, including when nothing happened.
579	func (s Server) loginSubmit(w http.ResponseWriter, r http.Request) {
580	if !s.emailLoginEnabled() {
581	s.notFound(w, r)
582	return
583	}
584	// The per-account bound lives in the store and survives a restart; this
585	// one stops a single source from spending every account's budget.
586	if allowed, wait := s.apiLimit.allow("login"+s.clientIP(r), true); !allowed {
587	w.Header().Set("Retry-After", strconv.Itoa(int(wait.Seconds())+1))
588	http.Error(w, "too many login requests; wait a moment", http.StatusTooManyRequests)
589	return
590	}
591	if err := control.RequestLoginLink(s.cfg, s.st, r.FormValue("identifier")); err != nil {
592	log.Printf("login link: %v", err)
593	}
594	s.renderLogin(w, "", true)
595	}
596	```
597
598	Update the two existing `renderLogin` calls in the `login` handler to pass
599	`false` as the new argument.
600
601	Add `"log"` and `"strconv"` to the file's imports if they are not already
602	there, and `"gitbay.org/gitbay/internal/control"` if absent.
603
604	- [ ] Step 5: Add the route
605
606	In `internal/httpd/routes.go`, directly after the `GET /login` line at 100:
607
608	```go
609	Route{Method: "POST", Pattern: "/login", Mutating: true,
610	Handler: s.checkOrigin(s.loginSubmit)},
611	```
612
613	- [ ] Step 6: Update the template
614
615	Replace the top of `internal/web/templates/login.html`, keeping the "New
616	here?" block below it exactly as it is:
617
618	```html
619	{{define "title"}}login · {{.Site}}{{end}}
620	{{define "content"}}
621	<h1>Log in</h1>
622	{{if .Error}}<p class="error" role="alert">{{.Error}}</p>{{end}}
623	{{if .Sent}}
624	<p>If that account exists, a login link is on its way. It works once and
625	expires in fifteen minutes.</p>
626	{{else}}
627	{{if .EmailLogin}}
628	<form method="post" action="/login">
629	<label for="identifier">Username or email address</label>
630	<input type="text" id="identifier" name="identifier" autocomplete="username" required>
631	<button type="submit">Email me a link</button>
632	</form>
633	<p>Or, from a machine with your registered key:</p>
634	{{else}}
635	<p>Browser sessions are minted over SSH — there is no password. From a machine
636	with your registered key:</p>
637	{{end}}
638	<pre class="message">ssh git@{{.Host}} web login</pre>
639	<p>then open the printed URL within five minutes.</p>
640	{{end}}
641	```
642
643	The `<label for>` is not decoration: `Web:InputWithoutLabelCheck` and #133
644	cover this, and a placeholder is not an accessible name.
645
646	- [ ] Step 7: Build, vet, and run the e2e tests
647
648	```bash
649	go build ./... && go vet ./...
650	go test ./internal/httpd/ ./internal/control/ ./internal/store/
651	go test ./e2e/ -run TestEmailLogin -v -timeout 20m
652	```
653	Expected: all PASS, including `TestEveryCommandIsReachable` and
654	`TestViewOnlyHasNoMutatingRoutes` in their packages.
655
656	- [ ] Step 8: Commit
657
658	```bash
659	git add internal/control/loginlink.go internal/httpd/accounts.go \
660	internal/httpd/routes.go internal/web/templates/login.html \
661	e2e/emaillogin_test.go
662	git commit -m "web: request a login link by email
663
664	An account with no SSH key had no way into the web UI at all: the only
665	caller of CreateWebSession consumed a token that only 'web login' over SSH
666	could mint. An unauthenticated POST /login now mails the same one-time
667	token, throttled per account and per source, with a response that does not
668	vary with whether the account exists.
669
670	Closes #155"
671	```
672
673	---
674
675	### Task 4: Parity row and the merge request
676
677	The Parity wiki page is a maintained matrix of capability by surface, and the
678	convention is to update the row in the change that moves it.
679
680	Files:
681	- Modify: `Parity.org` in the `krz/gitbay.wiki` clone
682
683	- [ ] Step 1: Clone or update the wiki
684
685	```bash
686	cd /tmp && git clone ssh://git@gitbay.org/krz/gitbay.wiki 2>/dev/null \|\| \
687	(cd /tmp/gitbay.wiki && git pull)
688	```
689
690	- [ ] Step 2: Add the row
691
692	Open `/tmp/gitbay.wiki/Parity.org`, find the table that carries the
693	authentication and account rows, and add a row for browser login in the same
694	format the neighbouring rows use: available on web, not applicable to CLI or
695	SSH (SSH has `web login`, which is the row above). Match the file's existing
696	markers rather than inventing new ones — read three neighbouring rows first.
697
698	- [ ] Step 3: Commit and push the wiki
699
700	```bash
701	cd /tmp/gitbay.wiki
702	git add Parity.org
703	git commit -m "Parity: browser login by emailed link"
704	git push
705	```
706
707	- [ ] Step 4: Push the branch and open the merge request
708
709	```bash
710	cd /Users/cmc/git/krz/gitbay
711	git push -u origin email-login
712	gitbay mr create --source email-login --target main \
713	--title "Browser login without an SSH key" --file - <<'EOF'
714	An account with no SSH key could not use the web UI at all. `CreateWebSession`
715	has one caller, the `/login?token=` handler, and only `web login` over SSH could
716	mint a token for it.
717
718	An unauthenticated `POST /login` now mails the same one-time token to a verified
719	address, resolved by username or address. Bounded at five an hour per account in
720	the store and by the existing token bucket per source. The response does not
721	vary with whether the account exists, whether its address is verified, or
722	whether it is over its budget.
723
724	The session cookie moves from `SameSite=Strict` to `Lax`, because a link clicked
725	in a mail client is a cross-site navigation and Strict can drop the cookie
726	through the redirect. Every cookie-authenticated mutating route carries
727	`checkOrigin`; the POST routes that do not take only bearer tokens or no auth at
728	all.
729
730	This does not widen what a browser session can do. The web dispatches with
731	`ViaAPI: true`, so no `SSHOnly` command is reachable from one however it was
732	obtained.
733
734	Design: `docs/specs/2026-09-04-email-login-design.md`.
735	Plan: `docs/plans/2026-09-04-email-login.md`.
736
737	Closes #155
738	EOF
739	```
740
741	- [ ] Step 5: Wait for CI, then merge
742
743	```bash
744	gitbay build list --json
745	```
746
747	Poll no more often than every 120 seconds: the CLI shares one SSH connection
748	per instance, and the auth limiter reads a burst of failures as an attack.
749
750	When green:
751
752	```bash
753	gitbay mr merge <n> --strategy squash
754	git checkout main && git pull
755	git branch -d email-login && git push origin --delete email-login
756	```
757
758	---
759
760	## Self-Review
761
762	Spec coverage. Every spec section maps to a task: the exported function,
763	resolution, and mail body to Task 3 Step 3; TTL to `loginLinkTTL`; the durable
764	per-account throttle to Task 1 and its use in Task 3; the per-IP throttle to
765	Task 3 Step 4; enumeration to `TestEmailLoginDoesNotEnumerate`; the empty
766	identifier to the same test; the cookie change to Task 2; the no-new-config
767	decision to `emailLoginEnabled`; out-of-scope signup untouched. The spec's
768	implementation gate on `checkOrigin` coverage was discharged before this plan
769	was written and its result is recorded in Task 2.
770
771	Deviation. The return type of `RequestLoginLink` changed from the spec's
772	triple to a single `error`, recorded above under "Change from the spec".
773
774	Types. `RequestLoginLink(config.Config, *store.Store, string) error`;
775	`CountLoginTokensSince(int64, time.Time) (int, error)`;
776	`emailLoginEnabled() bool`; `renderLogin(http.ResponseWriter, string, bool)`.
777	Each is used with that signature everywhere it appears. `sessionSameSite` is
778	declared in Task 2 and used in Task 2 only.
779
780	Unverified at plan time. `Parity.org`'s table format is read at execution
781	rather than guessed, which is why Task 4 Step 2 says to read neighbouring rows
782	first rather than giving the row verbatim.

docs/specs/2026-09-04-email-login-design.md +7 −5

	@@ -52,10 +52,13 @@ precedent is `control.RegisterAccount`, a plain exported function that
52	New file `internal/control/loginlink.go`:	52	New file `internal/control/loginlink.go`:
53		53
54	```go	54	```go
55	func RequestLoginLink(cfg config.Config, st *store.Store, identifier string) (msg, errMsg string, code int)	55	func RequestLoginLink(cfg config.Config, st *store.Store, identifier string) error
56	```	56	```
57		57
58	Same return triple as `RegisterAccount`. No registry entry, so	58	The error is for the server log only, and every miss — no such account, no
		59	verified address, disabled, pending, over the throttle — returns nil. A return
		60	triple like `RegisterAccount`'s would hand the caller the distinction the
		61	enumeration rule below forbids it from rendering. No registry entry, so
59	`TestEveryCommandIsReachable` is unaffected and no CLI passthrough is needed —	62	`TestEveryCommandIsReachable` is unaffected and no CLI passthrough is needed —
60	anyone at a terminal has SSH and already has `web login`.	63	anyone at a terminal has SSH and already has `web login`.
61		64
	@@ -89,9 +92,8 @@ Two layers.
89		92
90	The response is identical whether the account exists, exists without a verified	93	The response is identical whether the account exists, exists without a verified
91	address, or is over its throttle: "if that account exists, a link is on its	94	address, or is over its throttle: "if that account exists, a link is on its
92	way." `RequestLoginLink` returns a generic `msg` and keeps the distinction	95	way." `RequestLoginLink` reports nothing about which case it took.
93	internal. Differences in status code, body, or redirect target all count as a	96	Differences in status code, body, or redirect target all count as a leak.
94	leak.
95		97
96	### Cookie SameSite	98	### Cookie SameSite
97		99

e2e/emaillogin_test.go +40

	@@ -87,6 +87,11 @@ func TestEmailLoginDoesNotEnumerate(t *testing.T) {
87	browser := newBrowser(t)	87	browser := newBrowser(t)
88	real1, bodyReal := browserPost(t, browser, inst.base()+"/login",	88	real1, bodyReal := browserPost(t, browser, inst.base()+"/login",
89	url.Values{"identifier": {"dana@example.test"}})	89	url.Values{"identifier": {"dana@example.test"}})
		90	// Pin the reference. Without this the comparison below passes just as
		91	// well if renderLogin regressed and every case returned an error page.
		92	if !strings.Contains(bodyReal, "on its way") {
		93	t.Fatalf("a real address did not get the confirmation: %s", bodyReal)
		94	}
90	absent, bodyAbsent := browserPost(t, browser, inst.base()+"/login",	95	absent, bodyAbsent := browserPost(t, browser, inst.base()+"/login",
91	url.Values{"identifier": {"nobody@example.test"}})	96	url.Values{"identifier": {"nobody@example.test"}})
92	unver, bodyUnver := browserPost(t, browser, inst.base()+"/login",	97	unver, bodyUnver := browserPost(t, browser, inst.base()+"/login",
	@@ -161,3 +166,38 @@ func TestEmailLoginThrottled(t *testing.T) {
161	t.Fatalf("sent %d login mails in an hour, want exactly 5", n)	166	t.Fatalf("sent %d login mails in an hour, want exactly 5", n)
162	}	167	}
163	}	168	}
		169
		170	// A suspended account still controls its verified address, so it can mail
		171	// itself a link. It must not get a session out of it: read access to the
		172	// private repos it is a member of is what suspension takes away.
		173	func TestEmailLoginRefusesDisabledAccount(t *testing.T) {
		174	smtp := startFakeSMTP(t)
		175	inst := startInstanceWith(t, fmt.Sprintf(
		176	"[web]\nmode = \"accounts\"\n[mail]\nsmtp_host = %q\nfrom = \"noreply@gitbay.test\"\n",
		177	smtp.addr))
		178	inst.admin(t, "admin", "user", "create", "dana",
		179	"--email", "dana@example.test", "--verified")
		180	inst.admin(t, "admin", "user", "disable", "dana")
		181
		182	browser := newBrowser(t)
		183	status, body := browserPost(t, browser, inst.base()+"/login",
		184	url.Values{"identifier": {"dana@example.test"}})
		185	if status != 200 \|\| !strings.Contains(body, "on its way") {
		186	t.Fatalf("the response gave the suspension away: %d %s", status, body)
		187	}
		188	// Nothing should be mailed at all, but the assertion that matters is
		189	// that no link completes a session, so wait long enough to catch one.
		190	deadline := time.Now().Add(2 * time.Second)
		191	for time.Now().Before(deadline) && len(smtp.mailTo("dana@example.test")) == 0 {
		192	time.Sleep(25 * time.Millisecond)
		193	}
		194	if n := len(smtp.mailTo("dana@example.test")); n != 0 {
		195	t.Errorf("mailed a login link to a disabled account: %d mails", n)
		196	}
		197
		198	// Same check as the single-use one: the client follows the logged-out
		199	// redirect to /login, which is a 200 either way, so read the body.
		200	if _, body := browserGet(t, browser, inst.base()+"/settings"); strings.Contains(body, "dana@example.test") {
		201	t.Error("a disabled account got a browser session")
		202	}
		203	}

internal/control/loginlink.go +17 −6

 		return nil
 	}
-	var userID int64
+	var user store.User
 	var address string
 	if strings.Contains(identifier, "@") {
 		id, ok := st.UserIDByVerifiedEmail(identifier)
 		if !ok {
 			return nil
 		}
-		userID, address = id, identifier
+		u, err := st.UserByID(id)
+		if err != nil {
+			return nil
+		}
+		user, address = u, identifier
 	} else {
 		u, err := st.UserByUsername(identifier)
 		if err != nil {
 		if err != nil || addr == "" {
 			return nil
 		}
-		userID, address = u.ID, addr
+		user, address = u, addr
+	}
+	// Dispatch refuses both of these, so a session they reach only renders
+	// read paths — which is the whole of what suspension prevents, and more
+	// than pendingAllowed grants an unverified account. Returning nil rather
+	// than an error keeps the response identical to a miss.
+	if user.Disabled || user.Pending {
+		return nil
 	}
-	n, err := st.CountLoginTokensSince(userID, time.Now().Add(-time.Hour))
+	n, err := st.CountLoginTokensSince(user.ID, time.Now().Add(-time.Hour))
 	if err != nil {
 		return err
 	}
 	if err != nil {
 		return err
 	}
-	if err := st.CreateLoginToken(userID, hash, loginLinkTTL); err != nil {
+	if err := st.CreateLoginToken(user.ID, hash, loginLinkTTL); err != nil {
 		return err
 	}
 	host := siteHost(cfg)
 	// response time cannot answer what the response body is built not to.
 	go func() {
 		if err := mail.Send(cfg, address, subject, body); err != nil {
-			slog.Error("login link mail", "address", address, "err", err)
+			slog.Error("login link mail", "user", user.ID, "err", err)
 		}
 	}()
 	return nil

internal/httpd/accounts.go +26 −7

 // checkOrigin and carry no Lax cookie anyway.
 const sessionSameSite = http.SameSiteLaxMode
+// badLoginToken is what every refused /login?token= gets, whatever the
+// reason. The reasons differ in whether the account exists.
+const badLoginToken = "that login link is invalid, expired, or already used — mint a new one"
 // viewer returns the logged-in user, or a zero User for anonymous visitors.
 // Only meaningful in accounts mode; in view_only no session route exists so
 // every request is anonymous.
 	}
 }
-// checkOrigin rejects cross-site POSTs. Sessions also use SameSite=Strict;
+// checkOrigin rejects cross-site POSTs. It is the primary CSRF defense:
-// this is the second layer.
+// sessions use SameSite=Lax, which withholds the cookie from a cross-site
+// POST but not from a cross-site top-level GET.
 func (s *Server) checkOrigin(h http.HandlerFunc) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if origin := r.Header.Get("Origin"); origin != "" && origin != "null" {
 	}
 	userID, err := s.st.ConsumeLoginToken(store.HashToken(token))
 	if err != nil {
-		s.renderLogin(w, "that login link is invalid, expired, or already used — mint a new one", false)
+		s.renderLogin(w, badLoginToken, false)
+		return
+	}
+	// A token minted before the account was suspended is still consumable,
+	// and the session it would create renders every page the account can
+	// read. Checking here covers every mint path. The message is the one a
+	// bad token gets: a distinct one would confirm the account exists.
+	if u, err := s.st.UserByID(userID); err != nil || u.Disabled {
+		s.renderLogin(w, badLoginToken, false)
 		return
 	}
 	sessTok, sessHash, err := store.NewToken()
 		http.Error(w, "internal error", http.StatusInternalServerError)
 		return
 	}
-	http.SetCookie(w, &http.Cookie{
+	http.SetCookie(w, s.sessionCookieFor(sessTok))
-		Name: sessionCookie, Value: sessTok, Path: "/",
+	http.Redirect(w, r, "/", http.StatusSeeOther)
+}
+// sessionCookieFor is the cookie a new session ships in. Secure follows TLS
+// the way clearCookie does, so a plain-HTTP deployment still works.
+func (s *Server) sessionCookieFor(tok string) *http.Cookie {
+	return &http.Cookie{
+		Name: sessionCookie, Value: tok, Path: "/",
 		HttpOnly: true, SameSite: sessionSameSite,
 		Secure: s.cfg.HTTP.TLS != "off",
 		MaxAge: 7 * 24 * 3600,
-	})
+	}
-	http.Redirect(w, r, "/", http.StatusSeeOther)
 }
 func (s *Server) logout(w http.ResponseWriter, r *http.Request) {

internal/httpd/cookieclear_test.go +3 −4

 package httpd
 import (
-	"net/http"
 	"testing"
 	"gitbay.org/gitbay/internal/config"
 	for _, tls := range []string{"acme", "off"} {
 		s := &Server{cfg: config.Config{}}
 		s.cfg.HTTP.TLS = tls
-		c := s.clearCookie(sessionCookie, http.SameSiteStrictMode)
+		c := s.clearCookie(sessionCookie, sessionSameSite)
 		if c.Value != "" || c.MaxAge >= 0 {
 			t.Errorf("tls=%s: not an expiring cookie: value=%q maxage=%d", tls, c.Value, c.MaxAge)
 		if !c.HttpOnly {
 			t.Errorf("tls=%s: clearing cookie is not HttpOnly", tls)
 		}
-		if c.SameSite != http.SameSiteStrictMode {
+		if c.SameSite != sessionSameSite {
-			t.Errorf("tls=%s: SameSite = %v, want Strict", tls, c.SameSite)
+			t.Errorf("tls=%s: SameSite = %v, want %v", tls, c.SameSite, sessionSameSite)
 		}
 		if c.Path != "/" {
 			t.Errorf("tls=%s: Path = %q, want /", tls, c.Path)

internal/httpd/logincookie_test.go +20 −6

	@@ -12,13 +12,27 @@ import (
12	// cookie through the redirect that follows, so the visitor lands logged out	12	// cookie through the redirect that follows, so the visitor lands logged out
13	// (#155). Cross-site POSTs stay protected: Lax withholds the cookie from them,	13	// (#155). Cross-site POSTs stay protected: Lax withholds the cookie from them,
14	// and checkOrigin refuses them besides.	14	// and checkOrigin refuses them besides.
15	func TestSessionCookieIsLax(t *testing.T) {	15	//
16	s := &Server{cfg: config.Config{}}	16	// The other two attributes are what keep the token out of a script's reach
17	s.cfg.HTTP.TLS = "acme"	17	// and off the wire in clear, so they are asserted on the same literal login
18	if got := s.clearCookie(sessionCookie, sessionSameSite); got.SameSite != http.SameSiteLaxMode {	18	// hands to http.SetCookie.
19	t.Errorf("clearing cookie SameSite = %v, want Lax", got.SameSite)	19	func TestSessionCookieAttributes(t *testing.T) {
20	}
21	if sessionSameSite != http.SameSiteLaxMode {	20	if sessionSameSite != http.SameSiteLaxMode {
22	t.Errorf("sessionSameSite = %v, want Lax", sessionSameSite)	21	t.Errorf("sessionSameSite = %v, want Lax", sessionSameSite)
23	}	22	}
		23	for _, tls := range []string{"acme", "off"} {
		24	s := &Server{cfg: config.Config{}}
		25	s.cfg.HTTP.TLS = tls
		26	c := s.sessionCookieFor("tok")
		27
		28	if c.SameSite != http.SameSiteLaxMode {
		29	t.Errorf("tls=%s: SameSite = %v, want Lax", tls, c.SameSite)
		30	}
		31	if !c.HttpOnly {
		32	t.Errorf("tls=%s: session cookie is not HttpOnly", tls)
		33	}
		34	if want := tls != "off"; c.Secure != want {
		35	t.Errorf("tls=%s: Secure = %v, want %v", tls, c.Secure, want)
		36	}
		37	}
24	}	38	}

internal/store/users.go +6 −5

 	return out, rows.Err()
 }
-// SetUserDisabled suspends or restores an account. Disabling also drops
+// SetUserDisabled suspends or restores an account. Disabling drops every
-// the user's web sessions; their keys and tokens stay registered but are
+// credential that would grant a session on its own — web sessions, API
+// tokens, unclaimed login links — and leaves the SSH keys registered but
 // refused at every entry point until re-enabled.
 func (s *Store) SetUserDisabled(userID int64, disabled bool) error {
 	v := 0
 		return ErrNotFound
 	}
 	if disabled {
-		// Every credential the account holds goes with it: browser
+		// A pending login link is a session in waiting, so it goes with
-		// sessions and API tokens. Re-enabling means minting again.
+		// the sessions and API tokens. Re-enabling means minting again.
-		for _, table := range []string{"web_sessions", "api_tokens"} {
+		for _, table := range []string{"web_sessions", "api_tokens", "login_tokens"} {
 			if _, err = s.DB.Exec("DELETE FROM "+table+" WHERE user_id = ?", userID); err != nil {
 				return err
 			}

Commit cfc0bc7785

Commit `cfc0bc7785`