Test that logging out needs the form's CSRF token (part of #256)

A GET /logout, or a POST /logout without the CSRF token, must end no
login session; the generator page must carry a logout form with the
token; and after a logout from that form, a login opens the generator
page while a cookie copied before it is refused, also after a restart.

Model: opus-5-5

internal/handlers/auth_session_internal_test.go

@@ -185,7 +185,7 @@ func TestHandleLogout_ClearsSessionCookie(t *testing.T) {
 	}
 }
 
-// TestHandleLogout_EndsCopiedSession verifies that after GET /logout a copy
+// 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) {
@@ -196,14 +196,14 @@ func TestHandleLogout_EndsCopiedSession(t *testing.T) {
 	copied := newSessionCookie(t, h)
 
 	req := httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, "/logout", nil)
+		t.Context(), http.MethodPost, "/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",
+		t.Fatalf("POST /logout status = %d, want %d",
 			rec.Code, http.StatusSeeOther)
 	}
 
@@ -223,7 +223,7 @@ func TestHandleLogout_EndsCopiedSession(t *testing.T) {
 }
 
 // TestHandleLogout_SaveFails_Answers500 verifies that when the logout cannot
-// be saved in the database, GET /logout answers 500, leaves the session
+// 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()
@@ -248,17 +248,17 @@ func TestHandleLogout_SaveFails_Answers500(t *testing.T) {
 	}
 
 	req := httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, "/logout", nil)
+		t.Context(), http.MethodPost, "/logout", nil)
 	req.AddCookie(cookie)
 
 	rec := httptest.NewRecorder()
 	h.HandleLogout().ServeHTTP(rec, req)
 
-	t.Logf("GET /logout: status %d, Set-Cookie %q",
+	t.Logf("POST /logout: status %d, Set-Cookie %q",
 		rec.Code, rec.Header().Get("Set-Cookie"))
 
 	if rec.Code != http.StatusInternalServerError {
-		t.Errorf("GET /logout status = %d, want %d",
+		t.Errorf("POST /logout status = %d, want %d",
 			rec.Code, http.StatusInternalServerError)
 	}
 

internal/server/logout_internal_test.go

@@ -3,6 +3,8 @@ package server
 import (
 	"net/http"
 	"net/http/httptest"
+	"net/url"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -19,6 +21,11 @@ import (
 	"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 {
@@ -50,6 +57,24 @@ func startPixa(t *testing.T, cfg *config.Config) *Server {
 	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 {
@@ -65,37 +90,132 @@ func opensGeneratorPage(t *testing.T, s *Server, cookie *http.Cookie) bool {
 		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 /logout is still refused by a pixa started again on the same state
+// 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)
 
-	login := postLogin(t, s, firstClient, "", testSigningKey)
+	copied := logIn(t, s)
-
+	if !opensGeneratorPage(t, s, copied) {
-	var copied *http.Cookie
+		t.Fatal("logging in did not open the generator page")
-
-	for _, c := range login.Result().Cookies() {
-		if c.Name == session.CookieName {
-			copied = c
-		}
 	}
 
-	if copied == nil || !opensGeneratorPage(t, s, copied) {
+	logout := postLogout(t, s, copied)
-		t.Fatal("logging in gave no session cookie that opens the generator page")
-	}
-
-	req := httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, "/logout", nil)
-	req.AddCookie(copied)
-
-	logout := httptest.NewRecorder()
-	s.ServeHTTP(logout, req)
-
 	if logout.Code != http.StatusSeeOther {
-		t.Fatalf("GET /logout status = %d, want %d",
+		t.Fatalf("POST /logout status = %d, want %d",
 			logout.Code, http.StatusSeeOther)
 	}
 
@@ -106,7 +226,7 @@ func TestLogoutOutlastsRestart(t *testing.T) {
 		opens)
 
 	if opens {
-		t.Error("after a restart, a cookie copied before /logout " +
+		t.Error("after a restart, a cookie copied before the logout " +
 			"still opens the generator page")
 	}
 }

internal/session/session_logout_test.go

@@ -124,9 +124,8 @@ func TestLogout_EndsEverySessionMadeBeforeIt(t *testing.T) {
 }
 
 // TestLogout_WithoutSession_EndsNoSession verifies that a logout from a
-// request without a session cookie ends no session. Another site can make a
+// request without a session cookie ends no session, so someone who has not
-// browser request /logout, but the browser leaves the SameSite=Strict
+// logged in cannot end the sessions of those who have.
-// session cookie out of that request.
 func TestLogout_WithoutSession_EndsNoSession(t *testing.T) {
 	t.Parallel()