diff --git a/README.md b/README.md index 61fc6c9..87d3838 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,8 @@ 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. Losing the directory also ends every login session (see Encrypted + URLs). In the container, the startup script 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 @@ -196,7 +197,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///.`, or `/v1/image///` with no format — an image, fetched, resized and converted (below). Needs: a signature, unless the host is allowlisted (see @@ -244,7 +249,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 @@ -382,7 +387,15 @@ 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 random value in its database, which it makes when the database + has none and replaces at each logout. A login session carries the value + current at its login and is refused once the value has changed, also after a + restart (see Deployment). The check for a logout compares no time, so a login + after a logout works whatever the host's clock says; the 30 days are still + measured by the host's clock. 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 a9f3843..724a404 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,23 @@ P2: security: per-origin rate limiting # Completed Steps +- 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 logging out now ends them all: the session + manager keeps a random value, made with `crypto/rand` when the new `logout` + table has none and replaced by each logout, saves it there so that it outlasts + a restart, puts the value at login in each session, and refuses a session + carrying another value. The check for a logout compares no time, so a login + after a logout works whatever the host's clock does; a session's 30 days are + still checked against the clock. Losing the state directory ends every login + session, and no logout can make an ended one work again. Logging out is now + `POST /logout` from a form on the generator page, behind the CSRF check the + other forms have, so a link or image elsewhere on the same site cannot end 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 value in memory only; pixad uses `session.NewManagerWithDatabase`. - 2026-10-15 a cached file left empty or partial is no longer served, or fetched again on every request (closes #252). Source and transformed image files, and each transformed image's `.meta` file, are synced to disk before they are diff --git a/internal/db/migrations/001_schema.sql b/internal/db/migrations/001_schema.sql index 68f3a32..234a141 100644 --- a/internal/db/migrations/001_schema.sql +++ b/internal/db/migrations/001_schema.sql @@ -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 +); 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/auth_session_internal_test.go b/internal/handlers/auth_session_internal_test.go index 883e434..250d627 100644 --- a/internal/handlers/auth_session_internal_test.go +++ b/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. diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index da9572f..9cfc3bb 100644 --- a/internal/handlers/handlers.go +++ b/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 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 } diff --git a/internal/server/logout_internal_test.go b/internal/server/logout_internal_test.go new file mode 100644 index 0000000..2d46d76 --- /dev/null +++ b/internal/server/logout_internal_test.go @@ -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") + } +} diff --git a/internal/server/routes.go b/internal/server/routes.go index 104f2e6..b7ed1a2 100644 --- a/internal/server/routes.go +++ b/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, diff --git a/internal/session/session.go b/internal/session/session.go index 7233d9e..788c114 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -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 +} diff --git a/internal/session/session_logout_test.go b/internal/session/session_logout_test.go new file mode 100644 index 0000000..b86dbb7 --- /dev/null +++ b/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") + } +} diff --git a/internal/static/style.css b/internal/static/style.css index da22318..407c46e 100644 --- a/internal/static/style.css +++ b/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; } diff --git a/internal/templates/generator.html b/internal/templates/generator.html index 2caaaac..6d679d2 100644 --- a/internal/templates/generator.html +++ b/internal/templates/generator.html @@ -10,9 +10,12 @@

Pixa URL Generator

- - Logout - +
+ {{ .CSRFField }} + +
{{if .GeneratedURL}}