Logging out ends every login session (closes #256)
check / check (push) Waiting to run

A login session was a cookie with nothing kept on the server, so
logging out removed it from one browser and a copy kept working for 30
days. The session manager now keeps a random value in the database,
made when it has none and replaced by each logout; a session carries
the value current at its login and is refused once it has changed. The
logout check compares no time, so the host's clock cannot block a
login, and losing the state directory ends every login session.
Logging out is now a POST with the CSRF token, sent by a form on the
generator page.

Model: opus-5-5
This commit was merged in pull request #265.
This commit is contained in:
2026-10-15 20:44:44 +02:00
parent 2ab96155ba
commit 742db9ceb1
12 changed files with 698 additions and 16 deletions
+8
View File
@@ -176,3 +176,11 @@ CREATE TABLE IF NOT EXISTS cache_stats (
last_updated_at DATETIME DEFAULT CURRENT_TIMESTAMP
);
INSERT OR IGNORE INTO cache_stats (id) VALUES (1);
-- A random value that a login session carries from its login. pixa makes it
-- when the table has no row, and each logout replaces it; a login session
-- carrying another value is refused, so logging out ends every login session.
CREATE TABLE IF NOT EXISTS logout (
id INTEGER PRIMARY KEY CHECK (id = 1),
logout_value TEXT NOT NULL
);
+10 -2
View File
@@ -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)
}
}
@@ -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.
+4 -1
View File
@@ -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 random value that each logout replaces 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
}
+269
View File
@@ -0,0 +1,269 @@
package server
import (
"net/http"
"net/http/httptest"
"net/url"
"path/filepath"
"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")
}
}
// TestLostStateDirectoryEndsEveryLoginSession verifies that a pixa started on
// an empty state directory, as pixad is after a restart that lost its state
// directory, refuses a session cookie from before, and that a login on it
// opens the generator page.
func TestLostStateDirectoryEndsEveryLoginSession(t *testing.T) {
t.Parallel()
s := newTestServer(t)
before := logIn(t, s)
if !opensGeneratorPage(t, s, before) {
t.Fatal("logging in did not open the generator page")
}
emptyStateDir := *s.config
emptyStateDir.StateDir = t.TempDir()
emptyStateDir.DBURL = "file:" +
filepath.Join(emptyStateDir.StateDir, "state.sqlite3")
restarted := startPixa(t, &emptyStateDir)
opens := opensGeneratorPage(t, restarted, before)
t.Logf("on an empty state directory, the cookie from before opens the "+
"generator page: %v", opens)
if opens {
t.Error("on an empty state directory, a cookie from before still " +
"opens the generator page")
}
if !opensGeneratorPage(t, restarted, logIn(t, restarted)) {
t.Error("a login on an empty state directory does not open the " +
"generator page")
}
}
+1 -2
View File
@@ -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,
+116 -2
View File
@@ -1,9 +1,16 @@
// Package session provides encrypted session cookie management.
// Package session provides encrypted session cookie management, and keeps in
// the database a random value that each logout replaces, so that a logout
// ends every session made before it.
package session
import (
"context"
"crypto/rand"
"database/sql"
"errors"
"fmt"
"net/http"
"sync"
"time"
"github.com/gorilla/securecookie"
@@ -32,14 +39,36 @@ type Data struct {
Authenticated bool `json:"auth"`
CreatedAt time.Time `json:"created"`
ExpiresAt time.Time `json:"expires"`
// LogoutValue is the Manager's random value, which each logout
// replaces, when the session was made.
LogoutValue string `json:"logoutValue"`
}
// 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 a random
// value, which each logout replaces: a session carries the value current at
// its login, and a session carrying another value is refused. The check for
// a logout compares no time, so a login after a logout works whatever the
// host's clock does; the session's expiry is still checked against the
// clock. The Manager keeps the value 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 logoutValue, and is held while a logout is saved, so that
// the database and logoutValue agree.
mu sync.Mutex
logoutValue string
}
// NewManager creates a session manager with keys derived from the signing key.
// It makes a new random value and keeps it in memory only, so a restart ends
// every session; NewManagerWithDatabase keeps the value in the database.
//
// Session cookies always carry the Secure, HttpOnly, and SameSite=Strict
// attributes; this cannot be configured. Browsers treat http://localhost as a
@@ -63,10 +92,44 @@ func NewManager(signingKey string) (*Manager, error) {
sc.MaxAge(int(SessionTTL.Seconds()))
return &Manager{
sc: sc,
sc: sc,
logoutValue: rand.Text(),
}, nil
}
// NewManagerWithDatabase creates a session manager like NewManager that keeps
// its random value in db, which must have pixa's schema: it saves the value
// NewManager made when db has none, and otherwise starts from the one 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.ExecContext(ctx,
"INSERT OR IGNORE INTO logout (id, logout_value) VALUES (1, ?)",
m.logoutValue)
if err != nil {
return nil, fmt.Errorf(
"failed to save the random value in the logout table: %w", err)
}
err = db.QueryRowContext(ctx,
"SELECT logout_value FROM logout WHERE id = 1",
).Scan(&m.logoutValue)
if err != nil {
return nil, fmt.Errorf(
"failed to read the random value in the logout table: %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 +137,7 @@ func (m *Manager) CreateSession(w http.ResponseWriter) error {
Authenticated: true,
CreatedAt: now,
ExpiresAt: now.Add(SessionTTL),
LogoutValue: m.currentLogoutValue(),
}
encoded, err := m.sc.Encode(CookieName, data)
@@ -122,9 +186,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.LogoutValue != m.currentLogoutValue() {
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 +230,32 @@ func (m *Manager) IsAuthenticated(r *http.Request) bool {
return err == nil && data != nil && data.Authenticated
}
// saveLogout replaces the random value with a new one, saving it in the
// database first when the Manager has one.
func (m *Manager) saveLogout(ctx context.Context) error {
value := rand.Text()
m.mu.Lock()
defer m.mu.Unlock()
if m.db != nil {
_, err := m.db.ExecContext(ctx,
"UPDATE logout SET logout_value = ? WHERE id = 1", value)
if err != nil {
return fmt.Errorf("failed to save the logout: %w", err)
}
}
m.logoutValue = value
return nil
}
// currentLogoutValue returns the Manager's random value.
func (m *Manager) currentLogoutValue() string {
m.mu.Lock()
defer m.mu.Unlock()
return m.logoutValue
}
+148
View File
@@ -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")
}
}
+10 -2
View File
@@ -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;
}
+6 -3
View File
@@ -10,9 +10,12 @@
<div class="page">
<header>
<h1>Pixa URL Generator</h1>
<a href="/logout">
Logout
</a>
<form method="POST" action="/logout">
{{ .CSRFField }}
<button type="submit">
Logout
</button>
</form>
</header>
{{if .GeneratedURL}}