Log out with a form and count logouts instead of timing them (part of #256)

Logging out is now a POST in the CSRF-protected route group, sent by a
form on the generator page, so a link or image on another page of the
same site cannot end every login session. The session manager keeps a
count of logouts instead of the time of the latest one: each session
carries the count at its login and is refused once it has changed. No
clock is compared, so a login after a logout always works.

Model: opus-5-5

README.md

@@ -71,9 +71,10 @@ The state directory (`state_dir`, `/var/lib/pixa` in the container) holds the
 database and the disk cache:
 
 - It needs a persistent volume: without one, every restart starts with an empty
-  cache. In the container, the startup script gives the directory to the user
+  cache and forgets the count of logouts, so login sessions that a logout ended
-  pixa runs as (uid 65532) and sets its mode to `750`; outside it, that user
+  can work again (see Encrypted URLs). In the container, the startup script
-  must be able to write the directory.
+  gives the directory to the user pixa runs as (uid 65532) and sets its mode to
+  `750`; outside it, that user must be able to write the directory.
 - `cache_max_bytes` limits the source and transformed images together. The
   database, the metadata files, the `.meta` file beside each transformed image
   and files still being written come on top, and eviction runs in the
@@ -126,27 +127,6 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
     - `PIXA_CACHE_MAX_BYTES`: disk cache limit in bytes; `0` disables it;
       default 75% of (free space + what the cache holds)
     - the rest are in the table under Configuration below
-- **Memory:** pixa can use up to `max_concurrent_processing` times 1.6 GiB for
-  the images it processes at once, plus `upstream_connections` times
-  `upstream_max_response_size` for the source images it has fetched that wait
-  for processing. 1.6 GiB is the most pixad's resident memory was measured to
-  grow while it processed one 8192x8192 source, the largest it accepts, into
-  each output format in turn, one at a time; it grew most for a 40 MB JPEG of
-  random pixels saved as AVIF. At the defaults that is half the CPUs pixa sees
-  times 1.6 GiB, plus 64 times 50 MiB: on a host where pixa sees 48 CPUs, 24
-  times 1.6 GiB plus 3.1 GiB, about 42 GiB. Set `PIXA_MAX_CONCURRENT_PROCESSING`
-  and `PIXA_UPSTREAM_CONNECTIONS` so that this fits the memory the container may
-  use.
-- **Temporary files:** while pixa processes a large source, libvips writes its
-  decoded pixels to a file in `/tmp` and deletes the file when the image is
-  done. For an 8192x8192 source the file was measured on pixad at 192 MiB for an
-  8-bit RGB JPEG, 512 MiB for a 16-bit RGBA PNG or JPEG XL, and 1 GiB for a JPEG
-  XL of 32-bit floating-point RGBA, the largest measured: up to 1 GiB per image
-  processed at once, 24 GiB at the defaults on a host where pixa sees 48 CPUs.
-  This `/tmp` is in the container's own filesystem, not on the volume at
-  `/var/lib/pixa`, and `cache_max_bytes` does not count it. The 1.6 GiB above
-  does not include these files; where `/tmp` is a tmpfs, they are memory on top
-  of it.
 - **Health check:** the image's `HEALTHCHECK` requests
   `/.well-known/healthcheck.json`. upaas reads the container's health 60 seconds
   after a deploy and marks the deploy failed unless it is `healthy`. The probe
@@ -205,7 +185,11 @@ path under `/v1/` answers 200, in maintenance mode too.
   session it answers 303 to `/`. Answers: 200 with the page showing the URL; 400
   with the page naming a field that is not valid; 500 when the URL cannot be
   made.
-- `GET /logout` — end the login session. Needs: nothing. Answers: 303 to `/`.
+- `POST /logout` — log out with the generator page's Logout button: end every
+  login session, in every browser (see Encrypted URLs), and remove the session
+  cookie. Needs: a login session and the generator page's form (below); without
+  a login session it only removes the cookie. Answers: 303 to `/`; 500 when the
+  logout cannot be saved in the database.
 - `GET` or `HEAD` `/v1/image/<host>/<path>/<size>.<format>`, or
   `/v1/image/<host>/<path>/<size>` with no format — an image, fetched, resized
   and converted (below). Needs: a signature, unless the host is allowlisted (see
@@ -253,7 +237,7 @@ the request carries the same ID as `request_id`, and so do the lines it logs
 when it fetches, converts and serves an image; the fetch sends it to the
 upstream host as `X-Request-ID`.
 
-Both `POST` routes accept only a form that pixa's own page served: the page puts
+The `POST` routes accept only a form that pixa's own page served: the page puts
 a token in the form and sets a cookie to match, and a request without both is
 refused with 403, so another site cannot submit the form from a visitor's
 browser. The login and generator pages are meant to be opened over HTTPS: while
@@ -374,7 +358,14 @@ nor change what it asks for.
 
 1. Open `/` in a browser over HTTPS (or over plain HTTP while `debug` is on, see
    Routes) and log in with the signing key (`signing_key`). The login session
-   lasts 30 days, or until `/logout`.
+   lasts 30 days, or until someone logs out with the generator page's Logout
+   button. Every login is made with the one signing key, so logging out ends
+   every login session, in every browser, a copy of the session cookie included.
+   pixa keeps a count of logouts in its database: each logout adds one, a login
+   session carries the count at its login, and a login session whose count is
+   not the current one is refused, also after a restart that keeps the state
+   directory (see Deployment). No time is compared, so a login after a logout
+   works whatever the host's clock says.
 2. On the generator page, give the source image's URL, the width and height, the
    format, quality and fit, and how long the URL lasts, then submit the form
    (`POST /generate`). The format is JPEG XL unless another is chosen; a form
@@ -545,7 +536,7 @@ tell. With no file, pixa uses the environment and the defaults.
 | `PIXA_ALLOW_HTTP`                    | `allow_http`                    | Allow plain-HTTP upstreams, for testing only; default `false`                |
 | `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20`                       |
 | `PIXA_UPSTREAM_CONNECTIONS`          | `upstream_connections`          | Concurrent connections to all upstream hosts together; default `64`          |
-| `PIXA_MAX_CONCURRENT_PROCESSING`     | `max_concurrent_processing`     | Images processed at once; default half the CPUs, at least 1                  |
+| `PIXA_MAX_CONCURRENT_PROCESSING`     | `max_concurrent_processing`     | Images processed at once; default the number of CPUs                         |
 | `PIXA_IMAGE_REQUESTS_PER_MINUTE`     | `image_requests_per_minute`     | Image requests per client per minute; `0` turns it off; default `600`        |
 | `PIXA_UPSTREAM_FETCH_TIMEOUT`        | `upstream_fetch_timeout`        | Time allowed for one fetch from an upstream host; default `30s`              |
 | `PIXA_UPSTREAM_MAX_RESPONSE_SIZE`    | `upstream_max_response_size`    | Largest upstream response accepted, in bytes; default 50 MiB                 |
@@ -631,12 +622,10 @@ Key settings in more detail:
   does, and `downstream_timeout` has not ended first, the request is answered
   503 with the error `server busy, try again later`
 - `max_concurrent_processing` — the most images decoded and encoded at once;
-  default half the CPUs pixa can use (`GOMAXPROCS`, which follows a container's
+  default the number of CPUs pixa can use (`GOMAXPROCS`), which follows a
-  CPU limit), rounded down, and at least 1. A value that is not a whole number
+  container's CPU limit. A request that finds all of them in use waits up to 10
-  of at least 1 aborts startup. A request that finds all of them in use waits up
+  seconds for one to free up; if none does, and `downstream_timeout` has not
-  to 10 seconds for one to free up; if none does, and `downstream_timeout` has
+  ended first, it is answered 503 the same way
-  not ended first, it is answered 503 the same way. Each one can take a lot of
-  memory: see "Running under upaas"
 - `image_requests_per_minute` — how many requests one client may send to the
   image routes per minute (see the limit on them under Routes); default `600`.
   `0` turns the limit off, and a negative or non-integer value aborts startup.

TODO.md

@@ -30,18 +30,22 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
-- 2026-10-15 `max_concurrent_processing` defaults to half the CPUs pixa sees,
+- 2026-10-15 logging out ends every login session (closes #256): a login session
-  rounded down, and at least 1 (closes #242): it defaulted to one image per CPU,
+  was a cookie with nothing kept on the server, so `/logout` removed it from one
-  and a container with no CPU limit sees every CPU of the host, so on a 48-CPU
+  browser and a copy of it kept working for its 30 days. Every login is made
-  host pixa processed 48 images at once, each of which can take about 1.6 GiB.
+  with the one signing key, so logging out now ends them all: the session
-  `config.DefaultMaxConcurrentProcessing` gives the default to the config and to
+  manager counts logouts, saves the count in the new `logout` table so that it
-  `imageprocessor.New`; `PIXA_MAX_CONCURRENT_PROCESSING` still overrides it.
+  outlasts a restart, puts the count at login in each session, and refuses a
-  "Running under upaas" in `README.md` gives the worst-case memory, measured,
+  session whose count is not the current one. No time is compared, so a login
-  and tells the operator to set `PIXA_MAX_CONCURRENT_PROCESSING` and
+  after a logout works whatever the host's clock does. A restart without a
-  `PIXA_UPSTREAM_CONNECTIONS` to fit the container's memory. It also gives the
+  persistent volume for the state directory forgets the count. Logging out is
-  file libvips writes to `/tmp` for each large source, up to 1 GiB, measured,
+  now `POST /logout` from a form on the generator page, behind the CSRF check
-  which is outside `/var/lib/pixa` and `cache_max_bytes` and is memory where
+  the other forms have, so a link or image elsewhere on the same site cannot end
-  `/tmp` is a tmpfs.
+  the sessions. A logout without a valid login session ends none and only
+  removes the cookie, as anyone can take a CSRF token from the login page. A
+  logout that cannot be saved answers 500. `session.NewManager`, which the tests
+  use, keeps the count in memory only; pixad uses
+  `session.NewManagerWithDatabase`.
 - 2026-10-15 expired negative cache rows are removed on a schedule (closes
   #244): the row of a failed fetch, which makes pixa answer 502 for that source
   URL for 5 minutes, was deleted only when the same URL was requested again

configs/config.example.yml

@@ -108,11 +108,10 @@ upstream_connections_per_host: 20
 # downstream_timeout has ended first.
 upstream_connections: 64
 
-# Maximum number of images decoded and encoded at once (default: half the
+# Maximum number of images decoded and encoded at once (default: the
-# number of CPUs pixa can use, at least 1; the number of CPUs follows a
+# number of CPUs pixa can use, which follows a container's CPU limit). A
-# container's CPU limit). A request that finds none free waits up to 10
+# request that finds none free waits up to 10 seconds for one, and if none
-# seconds for one, and if none frees up it is answered 503, unless
+# frees up it is answered 503, unless downstream_timeout has ended first.
-# downstream_timeout has ended first.
 # max_concurrent_processing: 4
 
 # How many requests to the image routes (/v1/image/ and /v1/e/) one client

internal/config/concurrency_limits_internal_test.go

@@ -1,7 +1,6 @@
 package config
 
 import (
-	"fmt"
 	"runtime"
 	"testing"
 )
@@ -13,8 +12,8 @@ const (
 )
 
 // TestOmittedConcurrencyLimitsUseDefaults checks that an omitted
-// max_concurrent_processing is half the CPUs Go uses, and at least 1, and
+// max_concurrent_processing is the number of CPUs Go uses and an omitted
-// an omitted upstream_connections is 64.
+// upstream_connections is 64.
 func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
 	t.Parallel()
 
@@ -23,9 +22,9 @@ func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
 		t.Fatalf("minimal config should be valid, got error: %v", err)
 	}
 
-	if c.MaxConcurrentProcessing != max(runtime.GOMAXPROCS(0)/2, 1) {
+	if c.MaxConcurrentProcessing != runtime.GOMAXPROCS(0) {
-		t.Errorf("MaxConcurrentProcessing = %d, want %d, half the CPUs",
+		t.Errorf("MaxConcurrentProcessing = %d, want %d, one per CPU",
-			c.MaxConcurrentProcessing, max(runtime.GOMAXPROCS(0)/2, 1))
+			c.MaxConcurrentProcessing, runtime.GOMAXPROCS(0))
 	}
 
 	if c.UpstreamConnections != 64 {
@@ -33,36 +32,6 @@ func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
 	}
 }
 
-// TestOmittedMaxConcurrentProcessingIsHalfTheCPUsAndAtLeastOne checks
-// that an omitted max_concurrent_processing is half the CPUs Go uses,
-// rounded down, and 1 when Go uses one CPU.
-//
-//nolint:paralleltest // it sets GOMAXPROCS, which other tests read
-func TestOmittedMaxConcurrentProcessingIsHalfTheCPUsAndAtLeastOne(t *testing.T) {
-	cpusBefore := runtime.GOMAXPROCS(0)
-
-	t.Cleanup(func() { runtime.GOMAXPROCS(cpusBefore) })
-
-	for _, tc := range []struct{ cpus, want int }{
-		{1, 1}, {2, 1}, {3, 1}, {8, 4},
-	} {
-		runtime.GOMAXPROCS(tc.cpus)
-
-		c, err := configFromYAML(t, signingKeyLine)
-		if err != nil {
-			t.Fatalf("minimal config should be valid, got error: %v", err)
-		}
-
-		t.Logf("GOMAXPROCS %d: MaxConcurrentProcessing %d",
-			tc.cpus, c.MaxConcurrentProcessing)
-
-		if c.MaxConcurrentProcessing != tc.want {
-			t.Errorf("GOMAXPROCS %d: MaxConcurrentProcessing = %d, want %d",
-				tc.cpus, c.MaxConcurrentProcessing, tc.want)
-		}
-	}
-}
-
 // TestExplicitConcurrencyLimitsAreUsed checks that valid values for the
 // two limits are used as given.
 func TestExplicitConcurrencyLimitsAreUsed(t *testing.T) {
@@ -191,19 +160,3 @@ func TestConcurrencyLimitsFromEnvironment(t *testing.T) {
 		})
 	}
 }
-
-// TestInvalidMaxConcurrentProcessingFromEnvironmentAbortsStartup checks
-// that a PIXA_MAX_CONCURRENT_PROCESSING that is not a whole number of at
-// least 1, an empty one included, aborts startup naming the variable and
-// the key instead of taking the default.
-func TestInvalidMaxConcurrentProcessingFromEnvironmentAbortsStartup(t *testing.T) {
-	for _, value := range []string{"", " ", "0.5", "half", "-1", "4 images"} {
-		t.Run(fmt.Sprintf("value %q", value), func(t *testing.T) {
-			t.Setenv(testMaxConcurrentProcessingVar, value)
-
-			_, err := configFromYAML(t, signingKeyLine)
-			wantStartupError(t, err,
-				testMaxConcurrentProcessingVar, keyMaxConcurrentProcessing)
-		})
-	}
-}

internal/config/config.go

@@ -37,16 +37,6 @@ const (
 	DefaultImageRequestsPerMinute     = 600
 )
 
-// DefaultMaxConcurrentProcessing returns the most images processed at once
-// when max_concurrent_processing is omitted: half the CPUs Go uses
-// (runtime.GOMAXPROCS(0), which follows a container's CPU limit), rounded
-// down, and at least 1.
-func DefaultMaxConcurrentProcessing() int {
-	const cpusPerImage = 2
-
-	return max(runtime.GOMAXPROCS(0)/cpusPerImage, 1)
-}
-
 // Configuration key names.
 const (
 	keyDebug                      = "debug"
@@ -307,8 +297,10 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 			keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
 		UpstreamConnections: loader.intVal(
 			keyUpstreamConnections, DefaultUpstreamConnections),
+		// Decoding and encoding are CPU-bound, so the default is one image
+		// per CPU Go uses, which follows a container's CPU limit.
 		MaxConcurrentProcessing: loader.intVal(
-			keyMaxConcurrentProcessing, DefaultMaxConcurrentProcessing()),
+			keyMaxConcurrentProcessing, runtime.GOMAXPROCS(0)),
 		UpstreamFetchTimeout: loader.durationVal(
 			keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout),
 		UpstreamMaxResponseSize: loader.int64Val(

internal/db/migrations/001_schema.sql

@@ -176,3 +176,12 @@ CREATE TABLE IF NOT EXISTS cache_stats (
     last_updated_at DATETIME DEFAULT CURRENT_TIMESTAMP
 );
 INSERT OR IGNORE INTO cache_stats (id) VALUES (1);
+
+-- Count of logouts. Each logout adds one; a login session carries the count
+-- at its login and is refused once the count has changed, so logging out ends
+-- every login session.
+CREATE TABLE IF NOT EXISTS logout (
+    id INTEGER PRIMARY KEY CHECK (id = 1),
+    logout_count INTEGER NOT NULL DEFAULT 0
+);
+INSERT OR IGNORE INTO logout (id) VALUES (1);

internal/handlers/auth.go

@@ -89,10 +89,18 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, "/", http.StatusSeeOther)
 }
 
-// HandleLogout clears the session and redirects to login.
+// HandleLogout logs out, which ends every login session (see
+// session.Manager.Logout), and redirects to login.
 func (s *Handlers) HandleLogout() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		s.sessMgr.ClearSession(w)
+		err := s.sessMgr.Logout(w, r)
+		if err != nil {
+			s.log.Error("failed to log out", "error", err)
+			http.Error(w, "Failed to log out", http.StatusInternalServerError)
+
+			return
+		}
+
 		http.Redirect(w, r, "/", http.StatusSeeOther)
 	}
 }

internal/handlers/auth_session_internal_test.go

@@ -185,6 +185,98 @@ func TestHandleLogout_ClearsSessionCookie(t *testing.T) {
 	}
 }
 
+// TestHandleLogout_EndsCopiedSession verifies that after POST /logout a copy
+// of the session cookie taken before it shows the login page, not the
+// generator page.
+func TestHandleLogout_EndsCopiedSession(t *testing.T) {
+	t.Parallel()
+
+	h, srv := newCSRFTestRouter(t)
+
+	copied := newSessionCookie(t, h)
+
+	req := httptest.NewRequestWithContext(
+		t.Context(), http.MethodPost, "/logout", nil)
+	req.AddCookie(copied)
+
+	rec := httptest.NewRecorder()
+	h.HandleLogout().ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusSeeOther {
+		t.Fatalf("POST /logout status = %d, want %d",
+			rec.Code, http.StatusSeeOther)
+	}
+
+	req = httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+	req.AddCookie(copied)
+
+	rec = httptest.NewRecorder()
+	srv.ServeHTTP(rec, req)
+
+	body := rec.Body.String()
+	t.Logf("GET / with the copied cookie: status %d, login page %v",
+		rec.Code, strings.Contains(body, loginForm))
+
+	if !strings.Contains(body, loginForm) || strings.Contains(body, generatorForm) {
+		t.Error("a cookie copied before /logout still opens the generator page")
+	}
+}
+
+// TestHandleLogout_SaveFails_Answers500 verifies that when the logout cannot
+// be saved in the database, POST /logout answers 500, leaves the session
+// cookie in place and ends no session.
+func TestHandleLogout_SaveFails_Answers500(t *testing.T) {
+	t.Parallel()
+
+	h, srv := newCSRFTestRouter(t)
+
+	db := setupTestDB(t)
+
+	sessMgr, err := session.NewManagerWithDatabase(
+		t.Context(), testSigningKey, db)
+	if err != nil {
+		t.Fatalf("session.NewManagerWithDatabase() error = %v", err)
+	}
+
+	h.sessMgr = sessMgr
+	cookie := newSessionCookie(t, h)
+
+	// Saving the logout fails on a closed database.
+	err = db.Close()
+	if err != nil {
+		t.Fatalf("closing the database: %v", err)
+	}
+
+	req := httptest.NewRequestWithContext(
+		t.Context(), http.MethodPost, "/logout", nil)
+	req.AddCookie(cookie)
+
+	rec := httptest.NewRecorder()
+	h.HandleLogout().ServeHTTP(rec, req)
+
+	t.Logf("POST /logout: status %d, Set-Cookie %q",
+		rec.Code, rec.Header().Get("Set-Cookie"))
+
+	if rec.Code != http.StatusInternalServerError {
+		t.Errorf("POST /logout status = %d, want %d",
+			rec.Code, http.StatusInternalServerError)
+	}
+
+	if c := findSessionCookie(rec); c != nil {
+		t.Errorf("a logout that failed set the session cookie: %s", c)
+	}
+
+	req = httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+	req.AddCookie(cookie)
+
+	rec = httptest.NewRecorder()
+	srv.ServeHTTP(rec, req)
+
+	if !strings.Contains(rec.Body.String(), generatorForm) {
+		t.Error("a logout that failed ended the session")
+	}
+}
+
 // TestGeneratePost_NoSession_RedirectsToLogin verifies that POST /generate
 // with a valid CSRF token but no login session answers 303 to / and makes no
 // URL.

internal/handlers/handlers.go

@@ -164,7 +164,10 @@ func (s *Handlers) initImageService() error {
 
 	// Initialize session manager (signing key is validated at config load
 	// time). Session cookies are always Secure/HttpOnly/SameSite=Strict.
-	sessMgr, err := session.NewManager(s.config.SigningKey)
+	// The count of logouts is saved in the database, so it outlasts a
+	// restart.
+	sessMgr, err := session.NewManagerWithDatabase(
+		context.Background(), s.config.SigningKey, s.db.DB())
 	if err != nil {
 		return err
 	}

internal/imageprocessor/imageprocessor.go

@@ -7,11 +7,11 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"runtime"
 	"sync"
 	"time"
 
 	"github.com/davidbyttow/govips/v2/vips"
-	"sneak.berlin/go/pixa/internal/config"
 )
 
 // vipsOnce ensures vips is initialized exactly once.
@@ -20,10 +20,11 @@ import (
 var vipsOnce sync.Once
 
 // initVips initializes libvips with quiet logging, one worker thread per
-// image and no operation cache. Each request decodes different source
+// image and no operation cache. Process already works on one image per CPU
-// bytes, so the operation cache would rarely be hit and would hold memory
+// by default, so more threads per image would only compete for the CPUs.
-// outside MaxConcurrentProcessing; repeated requests are served from
+// Each request decodes different source bytes, so the operation cache
-// pixa's disk cache instead.
+// would rarely be hit and would hold memory outside MaxConcurrentProcessing;
+// repeated requests are served from pixa's disk cache instead.
 func initVips() {
 	vipsOnce.Do(func() {
 		vips.LoggingSettings(nil, vips.LogLevelError)
@@ -178,7 +179,7 @@ type Params struct {
 	// If <= 0, DefaultMaxInputBytes is used.
 	MaxInputBytes int64
 	// MaxConcurrentProcessing is the most images processed at once.
-	// If <= 0, config.DefaultMaxConcurrentProcessing() is used.
+	// If <= 0, the number of CPUs Go uses (runtime.GOMAXPROCS(0)) is used.
 	MaxConcurrentProcessing int
 }
 
@@ -194,7 +195,7 @@ func New(params Params) *ImageProcessor {
 
 	maxConcurrentProcessing := params.MaxConcurrentProcessing
 	if maxConcurrentProcessing <= 0 {
-		maxConcurrentProcessing = config.DefaultMaxConcurrentProcessing()
+		maxConcurrentProcessing = runtime.GOMAXPROCS(0)
 	}
 
 	return &ImageProcessor{

internal/imageprocessor/max_concurrent_processing_internal_test.go

@@ -115,14 +115,14 @@ func waitForEntries(t *testing.T, entered <-chan struct{}, count int) {
 	}
 }
 
-func TestNewDefaultsMaxConcurrentProcessingToHalfTheCPUs(t *testing.T) {
+func TestNewDefaultsMaxConcurrentProcessingToCPUs(t *testing.T) {
 	t.Parallel()
 
 	for _, limit := range []int{0, -1} {
 		proc := New(Params{MaxConcurrentProcessing: limit})
-		if got := cap(proc.processingSemaphore); got != max(runtime.GOMAXPROCS(0)/2, 1) {
+		if got := cap(proc.processingSemaphore); got != runtime.GOMAXPROCS(0) {
-			t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, half the CPUs",
+			t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, one per CPU",
-				limit, got, max(runtime.GOMAXPROCS(0)/2, 1))
+				limit, got, runtime.GOMAXPROCS(0))
 		}
 	}
 
@@ -132,30 +132,6 @@ func TestNewDefaultsMaxConcurrentProcessingToHalfTheCPUs(t *testing.T) {
 	}
 }
 
-// TestNewDefaultIsHalfTheCPUsAndAtLeastOne checks that New, given no
-// MaxConcurrentProcessing, makes one slot for every two CPUs Go uses,
-// rounded down, and one slot when Go uses one CPU.
-//
-//nolint:paralleltest // it sets GOMAXPROCS, which other tests read
-func TestNewDefaultIsHalfTheCPUsAndAtLeastOne(t *testing.T) {
-	cpusBefore := runtime.GOMAXPROCS(0)
-
-	t.Cleanup(func() { runtime.GOMAXPROCS(cpusBefore) })
-
-	for _, tc := range []struct{ cpus, want int }{
-		{1, 1}, {2, 1}, {3, 1}, {8, 4},
-	} {
-		runtime.GOMAXPROCS(tc.cpus)
-
-		got := cap(New(Params{}).processingSemaphore)
-		t.Logf("GOMAXPROCS %d: %d slots", tc.cpus, got)
-
-		if got != tc.want {
-			t.Errorf("GOMAXPROCS %d: %d slots, want %d", tc.cpus, got, tc.want)
-		}
-	}
-}
-
 // TestProcessNeverExceedsMaxConcurrentProcessing starts more Process calls
 // than MaxConcurrentProcessing allows and holds the first ones inside
 // Process until the test lets them go. No more than the limit may be

internal/imgcache/service.go

@@ -53,7 +53,7 @@ type ServiceConfig struct {
 	// Allowlist is the list of hosts that don't require signatures
 	Allowlist []string
 	// MaxConcurrentProcessing is the most images processed at once; zero
-	// uses the image processor's default, half the CPUs
+	// uses the image processor's default, one per CPU
 	MaxConcurrentProcessing int
 	// Logger for logging
 	Logger *slog.Logger

internal/server/logout_internal_test.go

@@ -0,0 +1,232 @@
+package server
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"regexp"
+	"strings"
+	"testing"
+
+	"go.uber.org/fx"
+	"go.uber.org/fx/fxtest"
+
+	"sneak.berlin/go/pixa/internal/config"
+	"sneak.berlin/go/pixa/internal/database"
+	"sneak.berlin/go/pixa/internal/globals"
+	"sneak.berlin/go/pixa/internal/handlers"
+	"sneak.berlin/go/pixa/internal/healthcheck"
+	"sneak.berlin/go/pixa/internal/logger"
+	"sneak.berlin/go/pixa/internal/middleware"
+	"sneak.berlin/go/pixa/internal/session"
+)
+
+// logoutFormPattern extracts the CSRF token rendered into the generator
+// page's logout form.
+var logoutFormPattern = regexp.MustCompile(
+	`(?s)action="/logout".*?name="gorilla\.csrf\.Token" value="([^"]+)"`)
+
+// startPixa starts the components pixad's fx app builds, from cfg, and
+// returns the server with its routes, as newTestServer does.
+func startPixa(t *testing.T, cfg *config.Config) *Server {
+	t.Helper()
+
+	var (
+		h  *handlers.Handlers
+		mw *middleware.Middleware
+	)
+
+	app := fxtest.New(t,
+		fx.Supply(cfg),
+		fx.Provide(
+			globals.New,
+			logger.New,
+			database.New,
+			healthcheck.New,
+			handlers.New,
+			middleware.New,
+		),
+		fx.Populate(&h, &mw),
+	)
+	app.RequireStart()
+	t.Cleanup(app.RequireStop)
+
+	s := &Server{config: cfg, mw: mw, h: h}
+	s.SetupRoutes()
+
+	return s
+}
+
+// logIn logs in to s with the signing key, as postLogin does, and returns
+// the session cookie it sets.
+func logIn(t *testing.T, s *Server) *http.Cookie {
+	t.Helper()
+
+	rec := postLogin(t, s, firstClient, "", testSigningKey)
+
+	for _, c := range rec.Result().Cookies() {
+		if c.Name == session.CookieName {
+			return c
+		}
+	}
+
+	t.Fatal("logging in set no session cookie")
+
+	return nil
+}
+
+// opensGeneratorPage reports whether GET / with cookie shows s's generator
+// page, which only a valid login session opens.
+func opensGeneratorPage(t *testing.T, s *Server, cookie *http.Cookie) bool {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+	req.AddCookie(cookie)
+
+	rec := httptest.NewRecorder()
+	s.ServeHTTP(rec, req)
+
+	return rec.Code == http.StatusOK &&
+		strings.Contains(rec.Body.String(), `action="/generate"`)
+}
+
+// postLogout loads the generator page with GET / and sessionCookie and
+// submits its logout form with POST /logout, as a browser does.
+func postLogout(
+	t *testing.T, s *Server, sessionCookie *http.Cookie,
+) *httptest.ResponseRecorder {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+	req.AddCookie(sessionCookie)
+
+	page := httptest.NewRecorder()
+	s.ServeHTTP(page, req)
+
+	match := logoutFormPattern.FindStringSubmatch(page.Body.String())
+	if match == nil {
+		t.Fatal("the generator page has no logout form with a CSRF token")
+	}
+
+	form := url.Values{"gorilla.csrf.Token": {match[1]}}
+	req = httptest.NewRequestWithContext(t.Context(), http.MethodPost,
+		"/logout", strings.NewReader(form.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req.AddCookie(sessionCookie)
+
+	for _, c := range page.Result().Cookies() {
+		req.AddCookie(c)
+	}
+
+	rec := httptest.NewRecorder()
+	s.ServeHTTP(rec, req)
+
+	return rec
+}
+
+// TestLogoutWithoutCSRFTokenEndsNoSession verifies that a request to /logout
+// carrying the session cookie but not the logout form's CSRF token ends no
+// session: GET /logout, as an image on another page of the same site makes a
+// browser send, answers 405, and POST /logout without the token answers 403.
+func TestLogoutWithoutCSRFTokenEndsNoSession(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		method string
+		want   int
+	}{
+		{http.MethodGet, http.StatusMethodNotAllowed},
+		{http.MethodPost, http.StatusForbidden},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.method, func(t *testing.T) {
+			t.Parallel()
+
+			s := newTestServer(t)
+			cookie := logIn(t, s)
+
+			req := httptest.NewRequestWithContext(
+				t.Context(), tc.method, "/logout", nil)
+			req.AddCookie(cookie)
+
+			rec := httptest.NewRecorder()
+			s.ServeHTTP(rec, req)
+			t.Logf("%s /logout without the CSRF token: status %d",
+				tc.method, rec.Code)
+
+			if rec.Code != tc.want {
+				t.Errorf("%s /logout without the CSRF token status = %d, "+
+					"want %d", tc.method, rec.Code, tc.want)
+			}
+
+			if !opensGeneratorPage(t, s, cookie) {
+				t.Errorf("%s /logout without the CSRF token ended the "+
+					"login session", tc.method)
+			}
+		})
+	}
+}
+
+// TestLoginAfterLogoutOpensGeneratorPage verifies that after a logout from
+// the generator page a copy of the session cookie taken before it is
+// refused, and that a login after it opens the generator page.
+func TestLoginAfterLogoutOpensGeneratorPage(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+
+	copied := logIn(t, s)
+	if !opensGeneratorPage(t, s, copied) {
+		t.Fatal("logging in did not open the generator page")
+	}
+
+	logout := postLogout(t, s, copied)
+	if logout.Code != http.StatusSeeOther {
+		t.Fatalf("POST /logout status = %d, want %d",
+			logout.Code, http.StatusSeeOther)
+	}
+
+	if opensGeneratorPage(t, s, copied) {
+		t.Error("a cookie copied before the logout still opens the " +
+			"generator page")
+	}
+
+	opens := opensGeneratorPage(t, s, logIn(t, s))
+	t.Logf("a login after the logout opens the generator page: %v", opens)
+
+	if !opens {
+		t.Error("a login after the logout does not open the generator page")
+	}
+}
+
+// TestLogoutOutlastsRestart verifies that a copy of the session cookie taken
+// before a logout is still refused by a pixa started again on the same state
+// directory, as pixad is after a restart.
+func TestLogoutOutlastsRestart(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+
+	copied := logIn(t, s)
+	if !opensGeneratorPage(t, s, copied) {
+		t.Fatal("logging in did not open the generator page")
+	}
+
+	logout := postLogout(t, s, copied)
+	if logout.Code != http.StatusSeeOther {
+		t.Fatalf("POST /logout status = %d, want %d",
+			logout.Code, http.StatusSeeOther)
+	}
+
+	restarted := startPixa(t, s.config)
+
+	opens := opensGeneratorPage(t, restarted, copied)
+	t.Logf("after the restart, the copied cookie opens the generator page: %v",
+		opens)
+
+	if opens {
+		t.Error("after a restart, a cookie copied before the logout " +
+			"still opens the generator page")
+	}
+}

internal/server/routes.go

@@ -69,10 +69,9 @@ func (s *Server) SetupRoutes() {
 		r.With(s.mw.RateLimit(LoginAttemptsPerMinute, time.Minute, nil)).
 			Post("/", s.h.HandleRoot())
 		r.Post("/generate", s.h.HandleGenerateURL())
+		r.Post("/logout", s.h.HandleLogout())
 	})
 
-	s.router.Get("/logout", s.h.HandleLogout())
-
 	// Image routes, the only ones that send CORS headers, as pages on other
 	// sites read them. They are a subrouter rather than a group: a group's
 	// middleware runs only for a request that matches one of its routes,

internal/session/session.go

@@ -1,9 +1,15 @@
-// Package session provides encrypted session cookie management.
+// Package session provides encrypted session cookie management, and keeps a
+// count of logouts in the database so that a logout ends every session made
+// before it.
 package session
 
 import (
+	"context"
+	"database/sql"
 	"errors"
+	"fmt"
 	"net/http"
+	"sync"
 	"time"
 
 	"github.com/gorilla/securecookie"
@@ -32,14 +38,34 @@ type Data struct {
 	Authenticated bool      `json:"auth"`
 	CreatedAt     time.Time `json:"created"`
 	ExpiresAt     time.Time `json:"expires"`
+
+	// LogoutCount is the count of logouts when the session was made.
+	LogoutCount int64 `json:"logoutCount"`
 }
 
 // Manager handles session creation and validation using encrypted cookies.
+//
+// Every login is made with the one signing key, so a logout ends every
+// session made before it, in every browser. The Manager counts logouts: each
+// logout adds one, a session carries the count at its login, and a session
+// whose count is not the current one is refused. No clock is compared, so a
+// login after a logout works whatever the host's clock does. The Manager
+// keeps the count in memory, so checking a session reads no database; a
+// Manager from NewManagerWithDatabase also saves it in the database, so that
+// it outlasts a restart.
 type Manager struct {
 	sc *securecookie.SecureCookie
+	db *sql.DB
+
+	// mu guards logoutCount, and is held while a logout is saved, so that
+	// the database and logoutCount agree.
+	mu          sync.Mutex
+	logoutCount int64
 }
 
 // NewManager creates a session manager with keys derived from the signing key.
+// It keeps the count of logouts in memory only, so a restart forgets it;
+// NewManagerWithDatabase keeps it in the database.
 //
 // Session cookies always carry the Secure, HttpOnly, and SameSite=Strict
 // attributes; this cannot be configured. Browsers treat http://localhost as a
@@ -67,6 +93,29 @@ func NewManager(signingKey string) (*Manager, error) {
 	}, nil
 }
 
+// NewManagerWithDatabase creates a session manager like NewManager that saves
+// the count of logouts in db, which must have pixa's schema, and starts from
+// the count saved there.
+func NewManagerWithDatabase(
+	ctx context.Context, signingKey string, db *sql.DB,
+) (*Manager, error) {
+	m, err := NewManager(signingKey)
+	if err != nil {
+		return nil, err
+	}
+
+	err = db.QueryRowContext(ctx,
+		"SELECT logout_count FROM logout WHERE id = 1",
+	).Scan(&m.logoutCount)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read the count of logouts: %w", err)
+	}
+
+	m.db = db
+
+	return m, nil
+}
+
 // CreateSession creates a new authenticated session and sets the cookie.
 func (m *Manager) CreateSession(w http.ResponseWriter) error {
 	now := time.Now()
@@ -74,6 +123,7 @@ func (m *Manager) CreateSession(w http.ResponseWriter) error {
 		Authenticated: true,
 		CreatedAt:     now,
 		ExpiresAt:     now.Add(SessionTTL),
+		LogoutCount:   m.currentLogoutCount(),
 	}
 
 	encoded, err := m.sc.Encode(CookieName, data)
@@ -122,9 +172,30 @@ func (m *Manager) ValidateSession(r *http.Request) (*Data, error) {
 		return nil, ErrInvalidSession
 	}
 
+	// A logout since the session was made has ended it.
+	if data.LogoutCount != m.currentLogoutCount() {
+		return nil, ErrInvalidSession
+	}
+
 	return &data, nil
 }
 
+// Logout removes the session cookie and, when r has a valid session, ends
+// every session made until now. A request without one ends no session, so
+// someone who has not logged in cannot end the sessions of those who have.
+func (m *Manager) Logout(w http.ResponseWriter, r *http.Request) error {
+	if m.IsAuthenticated(r) {
+		err := m.saveLogout(r.Context())
+		if err != nil {
+			return err
+		}
+	}
+
+	m.ClearSession(w)
+
+	return nil
+}
+
 // ClearSession removes the session cookie.
 func (m *Manager) ClearSession(w http.ResponseWriter) {
 	http.SetCookie(w, &http.Cookie{
@@ -145,3 +216,31 @@ func (m *Manager) IsAuthenticated(r *http.Request) bool {
 
 	return err == nil && data != nil && data.Authenticated
 }
+
+// saveLogout adds one to the count of logouts, saving the new count in the
+// database first when the Manager has one.
+func (m *Manager) saveLogout(ctx context.Context) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+
+	if m.db != nil {
+		_, err := m.db.ExecContext(ctx,
+			"UPDATE logout SET logout_count = ? WHERE id = 1",
+			m.logoutCount+1)
+		if err != nil {
+			return fmt.Errorf("failed to save the logout: %w", err)
+		}
+	}
+
+	m.logoutCount++
+
+	return nil
+}
+
+// currentLogoutCount returns the count of logouts.
+func (m *Manager) currentLogoutCount() int64 {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+
+	return m.logoutCount
+}

internal/session/session_logout_test.go

@@ -0,0 +1,148 @@
+package session_test
+
+import (
+	"database/sql"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"sneak.berlin/go/pixa/internal/database"
+	"sneak.berlin/go/pixa/internal/session"
+)
+
+const logoutTestSigningKey = "test-signing-key-12345"
+
+// newTestDatabase opens an empty in-memory database with pixa's schema. It
+// has one connection, as pixad's has: each connection to ":memory:" would
+// open a database of its own.
+func newTestDatabase(t *testing.T) *sql.DB {
+	t.Helper()
+
+	db, err := sql.Open("sqlite", ":memory:")
+	if err != nil {
+		t.Fatalf("sql.Open() error = %v", err)
+	}
+
+	db.SetMaxOpenConns(1)
+	t.Cleanup(func() { _ = db.Close() })
+
+	err = database.ApplyMigrations(t.Context(), db, nil)
+	if err != nil {
+		t.Fatalf("ApplyMigrations() error = %v", err)
+	}
+
+	return db
+}
+
+// createSessionCookie logs in with mgr and returns the session cookie it
+// sets.
+func createSessionCookie(t *testing.T, mgr *session.Manager) *http.Cookie {
+	t.Helper()
+
+	rec := httptest.NewRecorder()
+
+	err := mgr.CreateSession(rec)
+	if err != nil {
+		t.Fatalf("CreateSession() error = %v", err)
+	}
+
+	for _, c := range rec.Result().Cookies() {
+		if c.Name == session.CookieName {
+			return c
+		}
+	}
+
+	t.Fatalf("CreateSession() did not set a %q cookie", session.CookieName)
+
+	return nil
+}
+
+// requestWithCookie returns a GET request carrying cookie, or no cookie when
+// cookie is nil.
+func requestWithCookie(t *testing.T, cookie *http.Cookie) *http.Request {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+	if cookie != nil {
+		req.AddCookie(cookie)
+	}
+
+	return req
+}
+
+// TestLogout_EndsEverySessionMadeBeforeIt verifies that after a logout a
+// copy of the session cookie taken before it is refused and a login after it
+// works, and that both still hold for a new Manager on the same database, as
+// after a restart.
+func TestLogout_EndsEverySessionMadeBeforeIt(t *testing.T) {
+	t.Parallel()
+
+	db := newTestDatabase(t)
+
+	mgr, err := session.NewManagerWithDatabase(
+		t.Context(), logoutTestSigningKey, db)
+	if err != nil {
+		t.Fatalf("NewManagerWithDatabase() error = %v", err)
+	}
+
+	copied := createSessionCookie(t, mgr)
+
+	err = mgr.Logout(httptest.NewRecorder(), requestWithCookie(t, copied))
+	if err != nil {
+		t.Fatalf("Logout() error = %v", err)
+	}
+
+	_, err = mgr.ValidateSession(requestWithCookie(t, copied))
+	t.Logf("cookie copied before the logout: ValidateSession() error = %v", err)
+
+	if !errors.Is(err, session.ErrInvalidSession) {
+		t.Errorf("cookie copied before the logout: error = %v, want %v",
+			err, session.ErrInvalidSession)
+	}
+
+	loginAfter := createSessionCookie(t, mgr)
+
+	_, err = mgr.ValidateSession(requestWithCookie(t, loginAfter))
+	if err != nil {
+		t.Errorf("login after the logout: ValidateSession() error = %v", err)
+	}
+
+	restarted, err := session.NewManagerWithDatabase(
+		t.Context(), logoutTestSigningKey, db)
+	if err != nil {
+		t.Fatalf("NewManagerWithDatabase() after restart error = %v", err)
+	}
+
+	if restarted.IsAuthenticated(requestWithCookie(t, copied)) {
+		t.Error("after a restart, a cookie copied before the logout is accepted")
+	}
+
+	if !restarted.IsAuthenticated(requestWithCookie(t, loginAfter)) {
+		t.Error("after a restart, a login made after the logout is refused")
+	}
+}
+
+// TestLogout_WithoutSession_EndsNoSession verifies that a logout from a
+// request without a session cookie ends no session, so someone who has not
+// logged in cannot end the sessions of those who have.
+func TestLogout_WithoutSession_EndsNoSession(t *testing.T) {
+	t.Parallel()
+
+	mgr, err := session.NewManagerWithDatabase(
+		t.Context(), logoutTestSigningKey, newTestDatabase(t))
+	if err != nil {
+		t.Fatalf("NewManagerWithDatabase() error = %v", err)
+	}
+
+	cookie := createSessionCookie(t, mgr)
+
+	err = mgr.Logout(httptest.NewRecorder(), requestWithCookie(t, nil))
+	if err != nil {
+		t.Fatalf("Logout() error = %v", err)
+	}
+
+	if !mgr.IsAuthenticated(requestWithCookie(t, cookie)) {
+		t.Error("a logout without a session cookie ended a session")
+	}
+}

internal/static/style.css

@@ -122,12 +122,20 @@ header {
     margin-bottom: 2rem;
 }
 
-header a {
+/* The logout button, drawn as a link. */
+
+header button {
+    width: auto;
+    margin: 0;
+    padding: 0;
+    background: none;
     font-size: 0.875rem;
     color: #4b5563;
+    text-decoration: underline;
 }
 
-header a:hover {
+header button:hover {
+    background: none;
     color: #1f2937;
 }
 

internal/templates/generator.html

@@ -10,9 +10,12 @@
     <div class="page">
         <header>
             <h1>Pixa URL Generator</h1>
-            <a href="/logout">
+            <form method="POST" action="/logout">
+                {{ .CSRFField }}
+                <button type="submit">
                     Logout
-            </a>
+                </button>
+            </form>
         </header>
 
         {{if .GeneratedURL}}