From 5706ec9a042d7ec5e51e75062f964958674ac730 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 02:51:01 +0000 Subject: [PATCH] Test that logging out ends a copied login session (part of #256) A copy of the session cookie taken before /logout must be refused after it, a login after it must work, and both must still hold for a session manager made again on the same database, as after a restart. A logout from a request without a session cookie must end no session. These fail until the fix: the session tests do not compile yet. Model: opus-5-5 --- .../handlers/auth_session_internal_test.go | 37 +++++ internal/session/session_logout_test.go | 149 ++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100644 internal/session/session_logout_test.go diff --git a/internal/handlers/auth_session_internal_test.go b/internal/handlers/auth_session_internal_test.go index 883e434..bf95d9d 100644 --- a/internal/handlers/auth_session_internal_test.go +++ b/internal/handlers/auth_session_internal_test.go @@ -185,6 +185,43 @@ func TestHandleLogout_ClearsSessionCookie(t *testing.T) { } } +// TestHandleLogout_EndsCopiedSession verifies that after GET /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.MethodGet, "/logout", nil) + req.AddCookie(copied) + + rec := httptest.NewRecorder() + h.HandleLogout().ServeHTTP(rec, req) + + if rec.Code != http.StatusSeeOther { + t.Fatalf("GET /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") + } +} + // 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/session/session_logout_test.go b/internal/session/session_logout_test.go new file mode 100644 index 0000000..668513a --- /dev/null +++ b/internal/session/session_logout_test.go @@ -0,0 +1,149 @@ +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. Another site can make a +// browser request /logout, but the browser leaves the SameSite=Strict +// session cookie out of that request. +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") + } +}