Commit `e3cd3c4236`

e3cd3c4236a453c3f7856efe9380aa48c1123211

parent: 650b235a4b

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-05 06:54 UTC

httpd: pin the login() disabled guard and the checkOrigin invariant

TestLoginRefusesTokenForDisabledAccount inserts a login token and disables
the account with a bare UPDATE, bypassing SetUserDisabled's login_tokens
delete so the login() re-read guard is pinned alone.

TestMutatingRoutesRequireCheckOrigin asserts every Mutating route rejects a
cross-site Origin with 403, with an explicit two-entry allowlist for the
routes that legitimately carry no cookie for checkOrigin to protect.

Closes #156
Closes #157

Layout: unified · split

internal/httpd/checkorigin_test.go added +54

@@ -0,0 +1,54 @@
	1	package httpd
	2
	3	import (
	4	"net/http"
	5	"net/http/httptest"
	6	"testing"
	7
	8	"gitbay.org/gitbay/internal/config"
	9	)
	10
	11	// TestMutatingRoutesRequireCheckOrigin pins the invariant checkOrigin's doc
	12	// comment rests on (#155, #157): the session cookie is SameSite=Lax, which
	13	// withholds it from a cross-site POST but not a cross-site top-level GET, so
	14	// checkOrigin has to be the thing that refuses every other mutating route.
	15	// checkOrigin is the outermost wrapper, so a cross-site Origin is refused
	16	// with 403 before any handler logic runs — no session or repository needed.
	17	//
	18	// Two routes are exempt on purpose, not by omission:
	19	var checkOriginAllowlist = map[string]string{
	20	// Authenticates only via "Authorization: Bearer ...". A cross-site
	21	// browser request cannot attach one, so there is no cookie for
	22	// checkOrigin to protect here.
	23	"POST /api/v1/cmd": "bearer-token auth, no cookie in play",
	24	// Consumes a single-use token from the query string and sets no
	25	// cookie on failure; browsers withhold Origin from a cross-site
	26	// top-level GET, which is the only way this route is ever reached
	27	// cross-site.
	28	"GET /login": "single-use token GET, no cookie read",
	29	}
	30
	31	func TestMutatingRoutesRequireCheckOrigin(t *testing.T) {
	32	cfg := config.Default()
	33	cfg.Web.Mode = "accounts" // superset of routes
	34	cfg.API.Enabled = true
	35	s := New(cfg, nil)
	36
	37	for _, r := range s.Routes() {
	38	if !r.Mutating {
	39	continue
	40	}
	41	key := r.Method + " " + r.Pattern
	42	if _, exempt := checkOriginAllowlist[key]; exempt {
	43	continue
	44	}
	45	req := httptest.NewRequest(r.Method, "http://example.com/", nil)
	46	req.Header.Set("Origin", "https://evil.example")
	47	rr := httptest.NewRecorder()
	48	r.Handler(rr, req)
	49	if rr.Code != http.StatusForbidden {
	50	t.Errorf("%s: cross-site Origin got status %d, want %d (missing checkOrigin?)",
	51	key, rr.Code, http.StatusForbidden)
	52	}
	53	}
	54	}

internal/httpd/logindisabled_test.go added +56

@@ -0,0 +1,56 @@
	1	package httpd
	2
	3	import (
	4	"net/http/httptest"
	5	"strings"
	6	"testing"
	7	"time"
	8
	9	"gitbay.org/gitbay/internal/config"
	10	"gitbay.org/gitbay/internal/store"
	11	)
	12
	13	// login() re-reads the user after ConsumeLoginToken and refuses a disabled
	14	// account with the same badLoginToken page a bad token gets (#155, #156).
	15	// SetUserDisabled also deletes login_tokens, which would mask this guard if
	16	// the test went through it, so the token is inserted directly and the row
	17	// is disabled with a bare UPDATE — bypassing SetUserDisabled entirely.
	18	func TestLoginRefusesTokenForDisabledAccount(t *testing.T) {
	19	st, err := store.Open(":memory:")
	20	if err != nil {
	21	t.Fatal(err)
	22	}
	23	defer st.Close()
	24	if err := st.MigrateUp(); err != nil {
	25	t.Fatal(err)
	26	}
	27
	28	uid, err := st.CreateUser("alice", false)
	29	if err != nil {
	30	t.Fatal(err)
	31	}
	32	tok, hash, err := store.NewToken()
	33	if err != nil {
	34	t.Fatal(err)
	35	}
	36	if err := st.CreateLoginToken(uid, hash, time.Hour); err != nil {
	37	t.Fatal(err)
	38	}
	39	if _, err := st.DB.Exec("UPDATE users SET disabled = 1 WHERE id = ?", uid); err != nil {
	40	t.Fatal(err)
	41	}
	42
	43	s := New(config.Default(), st)
	44	rr := httptest.NewRecorder()
	45	req := httptest.NewRequest("GET", "/login?token="+tok, nil)
	46	s.login(rr, req)
	47
	48	for _, c := range rr.Result().Cookies() {
	49	if c.Name == sessionCookie {
	50	t.Fatalf("login set a session cookie for a disabled account: %+v", c)
	51	}
	52	}
	53	if !strings.Contains(rr.Body.String(), badLoginToken) {
	54	t.Errorf("body = %q, want the bad-token page", rr.Body.String())
	55	}
	56	}

Commit e3cd3c4236

Commit `e3cd3c4236`