From 7f7afa1fd787e04f8b8edca7d1e65fb47477a373 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 03:03:52 +0000 Subject: [PATCH] Logging out ends every login session (closes #256) A login session was a cookie with nothing kept on the server, so /logout removed it from one browser and a copy kept working for 30 days. Every login is made with the one signing key, so /logout now ends them all: the session manager keeps the time of the latest logout, saves it in the new logout table so it outlasts a restart, and refuses a session made before it. A /logout without a valid session ends none and only removes the cookie, so another site cannot log everyone out. A logout that cannot be saved answers 500. session.NewManager, which existing tests use, keeps logouts in memory only; pixad uses NewManagerWithDatabase. Model: opus-5-5 --- README.md | 11 +++- TODO.md | 11 ++++ internal/db/migrations/001_schema.sql | 7 ++ internal/handlers/auth.go | 12 +++- internal/handlers/handlers.go | 4 +- internal/session/session.go | 94 +++++++++++++++++++++++++++ 6 files changed, 134 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 250463e..fd923d7 100644 --- a/README.md +++ b/README.md @@ -184,7 +184,10 @@ 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 `/`. +- `GET /logout` — end every login session, in every browser (see Encrypted + URLs), and remove the session cookie. Needs: a login session; without one it + only removes the cookie. Answers: 303 to `/`; 500 when the logout cannot be + saved in the database. - `GET` or `HEAD` `/v1/image///.`, or `/v1/image///` with no format — an image, fetched, resized and converted (below). Needs: a signature, unless the host is allowlisted (see @@ -352,7 +355,11 @@ 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 at `/logout`. Every login is made + with the one signing key, so logging out ends every login session, in every + browser: pixa saves the time of the latest logout in its database and refuses + a login session made before it, a copy of the session cookie included, also + after a restart. 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 diff --git a/TODO.md b/TODO.md index c046779..f1756b5 100644 --- a/TODO.md +++ b/TODO.md @@ -47,6 +47,17 @@ P2: security: per-origin rate limiting answered 500. Once the limit is used up, the fetcher now reads one more byte: if the body has ended it is accepted, and if there is more it is refused with `ErrResponseTooLarge`, as before. +- 2026-10-15 logging out ends every login session (closes #256): a login session + was a cookie with nothing kept on the server, so `/logout` removed it from one + browser and a copy of it kept working for its 30 days. Every login is made + with the one signing key, so `/logout` now ends them all: the session manager + keeps the time of the latest logout, saves it in the new `logout` table so + that it outlasts a restart, and refuses a session made before it. A `/logout` + without a valid login session, such as one another site makes a browser send + without the `SameSite=Strict` cookie, ends none and only removes the cookie. A + logout that cannot be saved answers 500. `session.NewManager`, which the tests + use, keeps logouts in memory only; pixad uses + `session.NewManagerWithDatabase`. - 2026-10-13 `fit=outside` no longer makes an output above 8192 on a side (closes #257): it scales the image to cover the requested size, and the side scaled past that size was held to no maximum, so `1x8192?fit=outside` of a diff --git a/internal/db/migrations/001_schema.sql b/internal/db/migrations/001_schema.sql index 68f3a32..d4788b9 100644 --- a/internal/db/migrations/001_schema.sql +++ b/internal/db/migrations/001_schema.sql @@ -176,3 +176,10 @@ CREATE TABLE IF NOT EXISTS cache_stats ( last_updated_at DATETIME DEFAULT CURRENT_TIMESTAMP ); INSERT OR IGNORE INTO cache_stats (id) VALUES (1); + +-- Time of the latest logout. A login session made before it is refused, +-- so logging out ends every login session. No row: nobody has logged out. +CREATE TABLE IF NOT EXISTS logout ( + id INTEGER PRIMARY KEY CHECK (id = 1), + logged_out_at DATETIME NOT NULL +); diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index 052ccd2..dff2763 100644 --- a/internal/handlers/auth.go +++ b/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) } } diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 297ba2d..be60019 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -163,7 +163,9 @@ 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) + // Logouts are saved in the database, so they outlast a restart. + sessMgr, err := session.NewManagerWithDatabase( + context.Background(), s.config.SigningKey, s.db.DB()) if err != nil { return err } diff --git a/internal/session/session.go b/internal/session/session.go index 7233d9e..c4ea3d2 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -2,8 +2,12 @@ package session import ( + "context" + "database/sql" "errors" + "fmt" "net/http" + "sync" "time" "github.com/gorilla/securecookie" @@ -35,11 +39,25 @@ type Data struct { } // 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 keeps the time of +// the latest logout 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 latestLogout, and is held while a logout is saved, so that + // the database and latestLogout agree. + mu sync.Mutex + latestLogout time.Time } // NewManager creates a session manager with keys derived from the signing key. +// It keeps logouts in memory only, so a restart forgets them; +// NewManagerWithDatabase keeps them 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 +85,29 @@ func NewManager(signingKey string) (*Manager, error) { }, nil } +// NewManagerWithDatabase creates a session manager like NewManager that saves +// each logout in db, which must have pixa's schema, and starts from the +// latest logout 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 logged_out_at FROM logout WHERE id = 1", + ).Scan(&m.latestLogout) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return nil, fmt.Errorf("failed to read the latest logout: %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() @@ -122,9 +163,31 @@ func (m *Manager) ValidateSession(r *http.Request) (*Data, error) { return nil, ErrInvalidSession } + // A session made before the latest logout has ended. + if !data.CreatedAt.After(m.latestLogoutTime()) { + 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: +// another site can make a browser request /logout, but the browser leaves +// the SameSite=Strict session cookie out of that request. +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 +208,34 @@ func (m *Manager) IsAuthenticated(r *http.Request) bool { return err == nil && data != nil && data.Authenticated } + +// saveLogout makes now the time of the latest logout, saving it in the +// database first when the Manager has one. +func (m *Manager) saveLogout(ctx context.Context) error { + m.mu.Lock() + defer m.mu.Unlock() + + now := time.Now().UTC() + + if m.db != nil { + _, err := m.db.ExecContext(ctx, + "INSERT OR REPLACE INTO logout (id, logged_out_at) VALUES (1, ?)", + now) + if err != nil { + return fmt.Errorf("failed to save the logout: %w", err) + } + } + + m.latestLogout = now + + return nil +} + +// latestLogoutTime returns the time of the latest logout, or the zero time +// when there has been none. +func (m *Manager) latestLogoutTime() time.Time { + m.mu.Lock() + defer m.mu.Unlock() + + return m.latestLogout +}