feat(backend): Discord OAuth login with DB-backed sessions (#23) (#31)

Implements #23 per ADR-0002. - Discord authorization code grant (identify + guilds.members.read), form-encoded token exchange - Guild membership gate via the single-guild endpoint; optional DISCORD_REQUIRED_ROLE (empty default) - Owner Discord ID is the only identity allowed to sign in - Sessions are DB rows with opaque random ids; cookie carries only the id; expiry enforced; delete = revoke - HMAC session signing, derived key, and WEB_PASSWORD removed; no replacement signing secret - Login rate limiting preserved on the callback - Full flow tested through the real router against a local Discord stub (DISCORD_API_BASE) - Env: DISCORD_CLIENT_ID/_CLIENT_SECRET/_GUILD_ID/_REQUIRED_ROLE/_API_BASE/_REDIRECT_URI; docs updated go test ./... passes. Reviewed-on: #31 Co-authored-by: Sulthan Zaki <sultankiki05@gmail.com> Co-committed-by: Sulthan Zaki <sultankiki05@gmail.com>
2026-08-08 08:51:22 +07:00
parent 8cebb94b92
commit bcc6b45515
20 changed files with 1280 additions and 363 deletions
@@ -0,0 +1,308 @@
+package web
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"log"
+	"net/http"
+	"net/url"
+	"slices"
+	"strconv"
+	"strings"
+	"sync"
+	"time"
+
+	"bookmarkmanager/backend/internal/session"
+)
+
+const (
+	// oauthStateTTL bounds how long a started sign-in stays valid. Ten
+	// minutes is generous for Discord's round trip and short enough that a
+	// captured state is stale before it is worth replaying.
+	oauthStateTTL = 10 * time.Minute
+	// maxStates caps the state map so a flood of /auth/discord hits cannot
+	// grow memory; past the cap the oldest state is evicted, which at worst
+	// invalidates an in-flight sign-in.
+	maxStates = 256
+	// maxResponseBytes caps Discord API bodies; they are small, and an
+	// unbounded read is an OOM handed to Discord's CDN.
+	maxResponseBytes = 1 << 20
+	// discordTimeout keeps a hung Discord request from hanging the login
+	// callback forever.
+	discordTimeout = 15 * time.Second
+)
+
+// DiscordConfig is the OAuth application this service registers as, plus the
+// guild that gates access.
+type DiscordConfig struct {
+	ClientID     string
+	ClientSecret string
+	GuildID      string
+	// RequiredRole, when non-empty, is a role ID a member must hold on top of
+	// guild membership. Empty by default: membership alone suffices.
+	RequiredRole string
+	// APIBase is the Discord API root; configurable so tests run the whole
+	// flow against a local stub.
+	APIBase string
+	// RedirectURI is the full public URL of the callback — Discord requires
+	// the exact string, so it is configured, never derived from headers.
+	RedirectURI string
+	// OwnerDiscordID is the only Discord identity allowed to sign in until
+	// registration exists (issue #23).
+	OwnerDiscordID string
+}
+
+// oauthStates stores one-time sign-in states. A state is generated at
+// /auth/discord, echoed back by Discord at the callback, and consumed there.
+type oauthStates struct {
+	mu     sync.Mutex
+	expiry map[string]time.Time
+}
+
+func newOAuthStates() *oauthStates {
+	return &oauthStates{expiry: make(map[string]time.Time)}
+}
+
+func (s *oauthStates) put(state string, expires time.Time) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	now := time.Now()
+	for k, at := range s.expiry {
+		if !at.After(now) {
+			delete(s.expiry, k)
+		}
+	}
+	// Evict the state closest to expiring when full, so a flood of starts
+	// cannot grow memory; at worst it invalidates an in-flight sign-in.
+	if len(s.expiry) >= maxStates {
+		var oldest string
+		var oldestAt time.Time
+		for k, at := range s.expiry {
+			if oldest == "" || at.Before(oldestAt) {
+				oldest, oldestAt = k, at
+			}
+		}
+		delete(s.expiry, oldest)
+	}
+	s.expiry[state] = expires
+}
+
+// take validates and consumes a state in one step: a state works exactly
+// once, which is what makes a replayed callback useless.
+func (s *oauthStates) take(state string) bool {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	expires, ok := s.expiry[state]
+	if !ok || !expires.After(time.Now()) {
+		return false
+	}
+	delete(s.expiry, state)
+	return true
+}
+
+// discordStart begins the authorization code grant: a fresh state, then a
+// redirect to Discord's authorize page.
+func (h *Handler) discordStart(w http.ResponseWriter, r *http.Request) {
+	state := session.NewID()
+	h.states.put(state, time.Now().Add(oauthStateTTL))
+	u := h.discord.APIBase + "/oauth2/authorize?" + url.Values{
+		"client_id":     {h.discord.ClientID},
+		"redirect_uri":  {h.discord.RedirectURI},
+		"response_type": {"code"},
+		"scope":         {"identify guilds.members.read"},
+		"state":         {state},
+	}.Encode()
+	http.Redirect(w, r, u, http.StatusSeeOther)
+}
+
+// discordCallback completes the grant: exchange the code, verify identity,
+// membership and role, then mint a session. Every failure path renders the
+// login page with an author-written message — nothing Discord supplied is
+// ever interpolated into a page, and no secret reaches a log line.
+func (h *Handler) discordCallback(w http.ResponseWriter, r *http.Request) {
+	ip := session.ClientIP(r)
+	if wait := h.limiter.RetryAfter(ip, time.Now()); wait > 0 {
+		secs := int(wait.Seconds()) + 1
+		w.Header().Set("Retry-After", strconv.Itoa(secs))
+		h.renderLogin(w, http.StatusTooManyRequests,
+			"Too many attempts. Try again in "+strconv.Itoa((secs+59)/60)+" min.")
+		return
+	}
+
+	// Discord refuses the grant (the reader hit cancel, or the application
+	// was misconfigured). The state is consumed so the flow is cleanly over;
+	// this makes no Discord calls, so it is not a failure the limiter counts.
+	if oerr := r.URL.Query().Get("error"); oerr != "" {
+		h.states.take(r.URL.Query().Get("state"))
+		h.renderLogin(w, http.StatusBadRequest, "Sign-in was cancelled.")
+		return
+	}
+
+	code := r.URL.Query().Get("code")
+	if code == "" || !h.states.take(r.URL.Query().Get("state")) {
+		h.limiter.Fail(ip, time.Now())
+		h.renderLogin(w, http.StatusBadRequest,
+			"This sign-in link was invalid or already used. Start again.")
+		return
+	}
+
+	tok, err := h.exchangeToken(r.Context(), code)
+	if err != nil {
+		h.limiter.Fail(ip, time.Now())
+		log.Printf("discord token exchange: %v", err)
+		h.renderLogin(w, http.StatusBadGateway,
+			"Discord sign-in is unavailable right now. Try again in a moment.")
+		return
+	}
+
+	userID, err := h.discordUserID(r.Context(), tok.AccessToken)
+	if err != nil {
+		h.limiter.Fail(ip, time.Now())
+		log.Printf("discord users/@me: %v", err)
+		h.renderLogin(w, http.StatusBadGateway,
+			"Discord sign-in is unavailable right now. Try again in a moment.")
+		return
+	}
+
+	if userID != h.discord.OwnerDiscordID {
+		h.limiter.Fail(ip, time.Now())
+		h.renderLogin(w, http.StatusForbidden,
+			"This Discord account is not the library owner.")
+		return
+	}
+
+	member, isMember, err := h.discordMember(r.Context(), tok.AccessToken, userID)
+	if err != nil {
+		h.limiter.Fail(ip, time.Now())
+		log.Printf("discord member check: %v", err)
+		h.renderLogin(w, http.StatusBadGateway,
+			"Discord sign-in is unavailable right now. Try again in a moment.")
+		return
+	}
+	// The refusal is the same for a non-member and a member without the
+	// required role, and it names neither the guild nor its id: an outsider
+	// cannot tell whether the guild exists, let alone which one gates.
+	if !isMember || (h.discord.RequiredRole != "" && !slices.Contains(member.Roles, h.discord.RequiredRole)) {
+		h.limiter.Fail(ip, time.Now())
+		h.renderLogin(w, http.StatusForbidden,
+			"This Discord account is not a member of this community.")
+		return
+	}
+
+	h.limiter.Reset(ip)
+	sess, err := h.store.CreateSession(session.NewID(), h.readerID, session.SessionTTL)
+	if err != nil {
+		log.Printf("create session: %v", err)
+		http.Error(w, "internal error", http.StatusInternalServerError)
+		return
+	}
+	session.SetCookie(w, r, sess.ID)
+	http.Redirect(w, r, "/", http.StatusSeeOther)
+}
+
+// exchangeToken trades an authorization code for an access token. The body is
+// form-encoded because that is what Discord accepts — it rejects a JSON
+// payload — so the wire format is fixed here, not in a client library.
+func (h *Handler) exchangeToken(ctx context.Context, code string) (discordToken, error) {
+	form := url.Values{
+		"client_id":     {h.discord.ClientID},
+		"client_secret": {h.discord.ClientSecret},
+		"grant_type":    {"authorization_code"},
+		"code":          {code},
+		"redirect_uri":  {h.discord.RedirectURI},
+	}
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost,
+		h.discord.APIBase+"/oauth2/token", strings.NewReader(form.Encode()))
+	if err != nil {
+		return discordToken{}, err
+	}
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req.Header.Set("Accept", "application/json")
+	resp, err := h.httpClient.Do(req)
+	if err != nil {
+		return discordToken{}, err
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		return discordToken{}, fmt.Errorf("status %d", resp.StatusCode)
+	}
+	var tok discordToken
+	if err := json.NewDecoder(io.LimitReader(resp.Body, maxResponseBytes)).Decode(&tok); err != nil {
+		return discordToken{}, err
+	}
+	if tok.AccessToken == "" {
+		return discordToken{}, errors.New("empty access token")
+	}
+	return tok, nil
+}
+
+// discordUserID fetches the signed-in user's id via the identify scope.
+func (h *Handler) discordUserID(ctx context.Context, accessToken string) (string, error) {
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet,
+		h.discord.APIBase+"/users/@me", nil)
+	if err != nil {
+		return "", err
+	}
+	req.Header.Set("Authorization", "Bearer "+accessToken)
+	req.Header.Set("Accept", "application/json")
+	resp, err := h.httpClient.Do(req)
+	if err != nil {
+		return "", err
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		return "", fmt.Errorf("status %d", resp.StatusCode)
+	}
+	var u struct {
+		ID string `json:"id"`
+	}
+	if err := json.NewDecoder(io.LimitReader(resp.Body, maxResponseBytes)).Decode(&u); err != nil {
+		return "", err
+	}
+	if u.ID == "" {
+		return "", errors.New("empty user id")
+	}
+	return u.ID, nil
+}
+
+type discordMember struct {
+	Roles []string `json:"roles"`
+}
+
+// discordMember fetches the user's membership in the configured guild — the
+// single-guild endpoint, not the list of every guild the user is in, so the
+// gate asks exactly the question it names. A 404 or 403 (not in the guild, or
+// the token lacks the scope) is a non-member, not an error.
+func (h *Handler) discordMember(ctx context.Context, accessToken, userID string) (discordMember, bool, error) {
+	u := h.discord.APIBase + "/guilds/" + url.PathEscape(h.discord.GuildID) +
+		"/members/" + url.PathEscape(userID)
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
+	if err != nil {
+		return discordMember{}, false, err
+	}
+	req.Header.Set("Authorization", "Bearer "+accessToken)
+	req.Header.Set("Accept", "application/json")
+	resp, err := h.httpClient.Do(req)
+	if err != nil {
+		return discordMember{}, false, err
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusForbidden {
+		return discordMember{}, false, nil
+	}
+	if resp.StatusCode != http.StatusOK {
+		return discordMember{}, false, fmt.Errorf("status %d", resp.StatusCode)
+	}
+	var m discordMember
+	if err := json.NewDecoder(io.LimitReader(resp.Body, maxResponseBytes)).Decode(&m); err != nil {
+		return discordMember{}, false, err
+	}
+	return m, true, nil
+}
+
+type discordToken struct {
+	AccessToken string `json:"access_token"`
+}
@@ -0,0 +1,55 @@
+package web
+
+import (
+	"testing"
+	"time"
+)
+
+func TestOAuthStateSingleUse(t *testing.T) {
+	s := newOAuthStates()
+	s.put("st", time.Now().Add(time.Minute))
+	if !s.take("st") {
+		t.Fatal("take of a fresh state = false, want true")
+	}
+	if s.take("st") {
+		t.Fatal("take of a consumed state = true, want false")
+	}
+}
+
+func TestOAuthStateUnknownOrExpired(t *testing.T) {
+	s := newOAuthStates()
+	if s.take("never-seen") {
+		t.Fatal("take of an unknown state = true, want false")
+	}
+	s.put("stale", time.Now().Add(-time.Minute))
+	if s.take("stale") {
+		t.Fatal("take of an expired state = true, want false")
+	}
+}
+
+// The map is capped: a flood of starts evicts older states instead of growing,
+// and consumed states must not change that.
+func TestOAuthStateEviction(t *testing.T) {
+	s := newOAuthStates()
+	key := func(i, salt int) string {
+		return string(rune('a'+i%26)) + string(rune('0'+i/26+salt*16))
+	}
+	now := time.Now().Add(time.Hour)
+	for i := 0; i < maxStates*2; i++ {
+		s.put(key(i, 0), now)
+	}
+	if got := len(s.expiry); got != maxStates {
+		t.Fatalf("states after a flood = %d, want %d", got, maxStates)
+	}
+
+	// Consume everything, then flood again: the map stays bounded.
+	for state := range s.expiry {
+		s.take(state)
+	}
+	for i := 0; i < maxStates; i++ {
+		s.put(key(i, 1), now)
+	}
+	if got := len(s.expiry); got != maxStates {
+		t.Fatalf("states after consume+flood = %d, want %d", got, maxStates)
+	}
+}
@@ -774,26 +774,6 @@ button { cursor: pointer; }
  filter: drop-shadow(0 0 34px var(--ember-wash)) drop-shadow(0 18px 24px rgba(0,0,0,.5));
 }
 .login-card form { display: flex; flex-direction: column; gap: 18px; }
-.login-card label {
-  font: 500 10px/1 var(--font-mono);
-  letter-spacing: .16em;
-  text-transform: uppercase;
-  color: var(--mute);
-}
-.login-card input {
-  width: 100%;
-  height: 54px;
-  margin-top: 9px;
-  padding: 0 2px;
-  border: none;
-  border-bottom: 1px solid var(--field-line);
-  background: transparent;
-  color: var(--paper);
-  font: 500 20px var(--font-mono);
-  letter-spacing: .16em;
-  outline: none;
-}
-.login-card input:focus { border-bottom-color: var(--paper); }
 .login-card .error {
  margin: 0;
  min-height: 20px;
@@ -810,7 +790,13 @@ button { cursor: pointer; }
 .login-card button:hover {
  background: var(--ember);
  border-color: var(--ember);
-  color: #fff;
+  color: var(--ember-ink);
+}
+.login-card .login-note {
+  margin: 14px 0 0;
+  text-align: center;
+  font: 400 12px/1.4 var(--font-body);
+  color: var(--mute);
 }

 /* ---- laptop and up: the whole sheet is drawn 20% larger, which is what
@@ -19,17 +19,13 @@
    <figure class="login-art" aria-hidden="true">
      <img src="/static/login-art.png" alt="">
    </figure>
-    <form method="post" action="/login">
-      <div>
-        <label for="password">Password</label>
-        <input id="password" name="password" type="password"
-               autocomplete="current-password" autofocus required>
-      </div>
+    <form method="get" action="/auth/discord">
      {{/* The page reloads on a failed sign-in, so the message is present from
           the start; role=alert is what gets it announced anyway. */}}
      <p class="error" role="alert">{{.Error}}</p>
-      <button type="submit">Sign in</button>
+      <button type="submit">Continue with Discord</button>
    </form>
+    <p class="login-note">Guild membership is required to sign in.</p>
  </main>
 </body>
 </html>
@@ -1,7 +1,7 @@
 package web

 import (
-	"crypto/subtle"
+	"context"
 	"embed"
 	"html/template"
 	"io/fs"
@@ -31,14 +31,18 @@ const RecentCount = 5
 // It is a separate handler from api.Handler because the two speak different
 // representations (HTML versus JSON) to different clients under different auth.
 type Handler struct {
-	store    *store.Store
-	// readerID is the Reader this UI acts as — the seeded owner, while the web
-	// password is still the only credential (issue #22).
+	store *store.Store
+	// readerID is the owner Reader's id, the only Reader that can exist
+	// while registration is closed (issue #23). Every session row points at
+	// it, so it is also the Reader the UI acts as.
 	readerID int64
 	tmpl     *template.Template
-	key      []byte
-	password string
+	discord  DiscordConfig
+	states   *oauthStates
 	limiter  *session.LoginLimiter
+	// httpClient is the plain stdlib client that talks to Discord. It is not
+	// an injected interface: tests point APIBase at a stub server instead.
+	httpClient *http.Client
 }

 // listView is what every list-rendering template receives.
@@ -46,7 +50,7 @@ type listView struct {
 	// Lib is the library this view renders: store.KindManga or store.KindNovel.
 	// Manga is the default and carries no query parameter, so every pre-novel
 	// URL keeps meaning exactly what it did.
-	Lib string
+	Lib    string
 	Tab    string // "all", "fav", or "new"
 	Recent []store.Bookmark
 	Items  []store.Bookmark
@@ -84,24 +88,26 @@ type loginView struct {

 // New parses every template up front so a broken one kills the process at
 // startup rather than the first request that touches it.
-func New(s *store.Store, readerID int64, apiToken, webPassword string) (*Handler, error) {
+func New(s *store.Store, readerID int64, discord DiscordConfig) (*Handler, error) {
 	tmpl, err := template.ParseFS(templateFS, "templates/*.html")
 	if err != nil {
 		return nil, err
 	}
 	return &Handler{
-		store:    s,
-		readerID: readerID,
-		tmpl:     tmpl,
-		key:      session.Key(apiToken, webPassword),
-		password: webPassword,
-		limiter:  session.NewLoginLimiter(),
+		store:      s,
+		readerID:   readerID,
+		tmpl:       tmpl,
+		discord:    discord,
+		states:     newOAuthStates(),
+		limiter:    session.NewLoginLimiter(),
+		httpClient: &http.Client{Timeout: discordTimeout},
 	}, nil
 }

 func (h *Handler) Register(mux *http.ServeMux) {
 	mux.HandleFunc("GET /{$}", h.index)
-	mux.HandleFunc("POST /login", h.login)
+	mux.HandleFunc("GET /auth/discord", h.discordStart)
+	mux.HandleFunc("GET /auth/discord/callback", h.discordCallback)
 	mux.HandleFunc("POST /logout", h.logout)
 	mux.Handle("GET /static/", staticHandler())

@@ -135,10 +141,26 @@ func staticHandler() http.Handler {
 		}))
 }

-// authed reports whether the request carries a valid session cookie.
-func (h *Handler) authed(r *http.Request) bool {
+type ctxKey int
+
+// readerCtxKey is where requireSession stashes the authenticated Reader id.
+const readerCtxKey ctxKey = iota
+
+// sessionReader reports whether the request carries a live session, and for
+// whom. The cookie holds only the session id; the row behind it is looked up
+// on every request, so deleting a session takes effect immediately. Expiry is
+// enforced here, in the store, which also removes rows that have lapsed.
+func (h *Handler) sessionReader(r *http.Request) (int64, bool) {
 	c, err := r.Cookie(session.CookieName)
-	return err == nil && session.Verify(h.key, c.Value, time.Now().UnixMilli())
+	if err != nil {
+		return 0, false
+	}
+	sess, ok, err := h.store.GetSession(c.Value, time.Now())
+	if err != nil {
+		log.Printf("session lookup: %v", err)
+		return 0, false
+	}
+	return sess.ReaderID, ok
 }

 // requireSession guards the fragment endpoints. It answers 401 rather than
@@ -146,14 +168,18 @@ func (h *Handler) authed(r *http.Request) bool {
 // redirected login page would be spliced into the card list.
 func (h *Handler) requireSession(next http.HandlerFunc) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		if !h.authed(r) {
+		readerID, ok := h.sessionReader(r)
+		if !ok {
 			http.Error(w, "unauthorized", http.StatusUnauthorized)
 			return
 		}
-		next(w, r)
+		next(w, r.WithContext(context.WithValue(r.Context(), readerCtxKey, readerID)))
 	}
 }

+// readerOf returns the authenticated Reader id requireSession stashed.
+func readerOf(r *http.Request) int64 { return r.Context().Value(readerCtxKey).(int64) }
+
 func (h *Handler) render(w http.ResponseWriter, status int, name string, data any) {
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
 	w.WriteHeader(status)
@@ -167,11 +193,12 @@ func (h *Handler) render(w http.ResponseWriter, status int, name string, data an
 // page is served at / with status 200 rather than as a redirect to a separate
 // URL: one page, no redirect loop to reason about.
 func (h *Handler) index(w http.ResponseWriter, r *http.Request) {
-	if !h.authed(r) {
+	readerID, ok := h.sessionReader(r)
+	if !ok {
 		h.render(w, http.StatusOK, "login", loginView{})
 		return
 	}
-	view, err := h.buildListView(libOf(r.URL.Query().Get("lib")), r.URL.Query().Get("tab"))
+	view, err := h.buildListView(readerID, libOf(r.URL.Query().Get("lib")), r.URL.Query().Get("tab"))
 	if err != nil {
 		log.Printf("index: %v", err)
 		http.Error(w, "internal error", http.StatusInternalServerError)
@@ -212,15 +239,15 @@ func libOf(q string) string {
 	return store.KindManga
 }

-// buildListView loads the list once and derives both the tab-filtered items and
-// the recent strip from it.
+// buildListView loads one reader's list once and derives both the tab-filtered
+// items and the recent strip from it.
 //
 // Archived and finished series appear in their own tab and nowhere else — not
 // in All, not in Updated, not in Favourites, and not in the recent strip. An
 // archived favourite therefore shows only under Archived: Favourites means
 // "favourites I am currently reading".
-func (h *Handler) buildListView(lib, tab string) (listView, error) {
-	all, err := h.store.List(h.readerID) // already ordered updated_at DESC
+func (h *Handler) buildListView(readerID int64, lib, tab string) (listView, error) {
+	all, err := h.store.List(readerID) // already ordered updated_at DESC
 	if err != nil {
 		return listView{}, err
 	}
@@ -271,7 +298,7 @@ func (h *Handler) buildListView(lib, tab string) (listView, error) {
 }

 func (h *Handler) uiList(w http.ResponseWriter, r *http.Request) {
-	view, err := h.buildListView(libOf(r.URL.Query().Get("lib")), r.URL.Query().Get("tab"))
+	view, err := h.buildListView(readerOf(r), libOf(r.URL.Query().Get("lib")), r.URL.Query().Get("tab"))
 	if err != nil {
 		log.Printf("ui list: %v", err)
 		http.Error(w, "internal error", http.StatusInternalServerError)
@@ -329,7 +356,7 @@ func (h *Handler) writeChromeOOB(w http.ResponseWriter, view listView) {
 // refreshChrome rebuilds the chrome for the reader's current tab after a
 // mutation and appends it to the response.
 func (h *Handler) refreshChrome(w http.ResponseWriter, r *http.Request) {
-	view, err := h.buildListView(currentLib(r), currentTab(r))
+	view, err := h.buildListView(readerOf(r), currentLib(r), currentTab(r))
 	if err != nil {
 		log.Printf("ui chrome: %v", err)
 		return
@@ -337,35 +364,21 @@ func (h *Handler) refreshChrome(w http.ResponseWriter, r *http.Request) {
 	h.writeChromeOOB(w, view)
 }

-func (h *Handler) login(w http.ResponseWriter, r *http.Request) {
-	ip := session.ClientIP(r)
-	if wait := h.limiter.RetryAfter(ip, time.Now()); wait > 0 {
-		secs := int(wait.Seconds()) + 1
-		w.Header().Set("Retry-After", strconv.Itoa(secs))
-		h.render(w, http.StatusTooManyRequests, "login", loginView{
-			Error: "Too many attempts. Try again in " +
-				strconv.Itoa((secs+59)/60) + " min.",
-		})
-		return
-	}
-
-	if err := r.ParseForm(); err != nil {
-		http.Error(w, "invalid form", http.StatusBadRequest)
-		return
-	}
-	got := r.PostFormValue("password")
-	if subtle.ConstantTimeCompare([]byte(got), []byte(h.password)) != 1 {
-		h.limiter.Fail(ip, time.Now())
-		h.render(w, http.StatusUnauthorized, "login", loginView{Error: "Wrong password."})
-		return
-	}
-
-	h.limiter.Reset(ip)
-	session.SetCookie(w, r, h.key)
-	http.Redirect(w, r, "/", http.StatusSeeOther)
+// renderLogin renders the login page with an error message, for refused or
+// failed sign-ins. Every message is author-written text — nothing Discord
+// supplied is ever interpolated into a page.
+func (h *Handler) renderLogin(w http.ResponseWriter, status int, msg string) {
+	h.render(w, status, "login", loginView{Error: msg})
 }

+// logout revokes the session row and clears the cookie in one step: the next
+// request finds no row and is rejected.
 func (h *Handler) logout(w http.ResponseWriter, r *http.Request) {
+	if c, err := r.Cookie(session.CookieName); err == nil {
+		if err := h.store.DeleteSession(c.Value); err != nil {
+			log.Printf("delete session: %v", err)
+		}
+	}
 	session.ClearCookie(w, r)
 	http.Redirect(w, r, "/", http.StatusSeeOther)
 }
@@ -378,7 +391,7 @@ func (h *Handler) loadForMutation(w http.ResponseWriter, r *http.Request) (store
 		http.Error(w, "missing key", http.StatusBadRequest)
 		return store.Bookmark{}, false
 	}
-	b, ok, err := h.store.Get(h.readerID, key)
+	b, ok, err := h.store.Get(readerOf(r), key)
 	if err != nil {
 		log.Printf("ui get %q: %v", key, err)
 		http.Error(w, "internal error", http.StatusInternalServerError)
@@ -401,7 +414,7 @@ func (h *Handler) loadForMutation(w http.ResponseWriter, r *http.Request) (store
 // describe the whole library, so they are rebuilt out of band on every
 // mutation, at the cost of one extra list read per toggle.
 func (h *Handler) saveAndRenderCard(w http.ResponseWriter, r *http.Request, b store.Bookmark) {
-	stored, err := h.store.Upsert(h.readerID, b)
+	stored, err := h.store.Upsert(readerOf(r), b)
 	if err != nil {
 		log.Printf("ui upsert %q: %v", b.Key, err)
 		http.Error(w, "internal error", http.StatusInternalServerError)
@@ -493,7 +506,7 @@ func (h *Handler) uiDelete(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "missing key", http.StatusBadRequest)
 		return
 	}
-	if err := h.store.Delete(h.readerID, key); err != nil {
+	if err := h.store.Delete(readerOf(r), key); err != nil {
 		log.Printf("ui delete %q: %v", key, err)
 		http.Error(w, "internal error", http.StatusInternalServerError)
 		return