From a96d15188821e51f349fe480c1ab50c6df7e4da1 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 15:15:04 +0000 Subject: [PATCH] Test logging in, logging out, the URL generator and /v1/e/ tokens (closes #77) New handler tests in internal/handlers, with no network: GET / shows the login form without a session; a wrong key shows it again with an error and sets no session cookie; the right key answers 303 with a session cookie marked Secure, HttpOnly and SameSite=Strict, with which GET / shows the generator page; GET /logout empties the cookie with Max-Age=0; POST /generate without a session answers 303 to /; /v1/e/ serves a valid token's image, answers 410 for an expired token and 400 for one changed, cut short or made with another signing key; a URL made on the generator page is served by /v1/e/. No code changes. Model: opus-5-5 --- TODO.md | 11 + .../handlers/auth_session_internal_test.go | 243 ++++++++++++++++++ .../handlers/imageenc_token_internal_test.go | 126 +++++++++ 3 files changed, 380 insertions(+) create mode 100644 internal/handlers/auth_session_internal_test.go create mode 100644 internal/handlers/imageenc_token_internal_test.go diff --git a/TODO.md b/TODO.md index ec2ee54..4366348 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,17 @@ P2: security: referer blacklist # Completed Steps +- 2026-10-04 logging in, logging out, the URL generator and `/v1/e/` have + handler tests (closes #77): new tests in `internal/handlers`, with no + network, check that `GET /` without a login session shows the login form; a + wrong key shows it again with an error and sets no session cookie; the right + key answers 303 to `/` with a session cookie marked `Secure`, `HttpOnly` and + `SameSite=Strict`, with which `GET /` shows the generator page; `GET /logout` + answers 303 to `/` with an empty session cookie sent with `Max-Age=0`; + `POST /generate` without a login session answers 303 to `/`; `/v1/e/` serves + the image for a valid token, answers 410 for an expired one and 400 for one + with a character changed, cut short or made with another signing key; and a + URL made on the generator page is served by `/v1/e/`. No code changes. - 2026-10-04 `TestEvictionRunsOnPeriodicSchedule` no longer races the evictor (closes #183): it wrote each variant file and then inserted its accounting row by hand, and a reconciliation pass between the two adopted the file first, so diff --git a/internal/handlers/auth_session_internal_test.go b/internal/handlers/auth_session_internal_test.go new file mode 100644 index 0000000..3bc57f8 --- /dev/null +++ b/internal/handlers/auth_session_internal_test.go @@ -0,0 +1,243 @@ +package handlers + +import ( + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "regexp" + "strings" + "testing" + + "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/session" +) + +// formatField is the generator form's format field name. +const formatField = "format" + +// Markers telling the login page from the generator page. +const ( + loginForm = `action="/"` + loginKeyInput = `name="key"` + generatorForm = `action="/generate"` +) + +// generatedURLPattern extracts the path of the URL the generator page shows. +// The test router runs with debug on, so the URL starts with http, and its +// host is httptest's default request host. +var generatedURLPattern = regexp.MustCompile( + `value="http://example\.com(/v1/e/[^"]+)"`) + +// findSessionCookie returns the session cookie rec sets, or nil if it sets +// none. +func findSessionCookie(rec *httptest.ResponseRecorder) *http.Cookie { + for _, c := range rec.Result().Cookies() { + if c.Name == session.CookieName { + return c + } + } + + return nil +} + +// TestHandleRoot_NoSession_ShowsLoginForm verifies that GET / without a +// login session shows the login form. +func TestHandleRoot_NoSession_ShowsLoginForm(t *testing.T) { + t.Parallel() + + _, srv := newCSRFTestRouter(t) + + rec := httptest.NewRecorder() + srv.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, "/", nil)) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + body := rec.Body.String() + if !strings.Contains(body, loginForm) || !strings.Contains(body, loginKeyInput) { + t.Errorf("page is not the login form: %s", body) + } +} + +// TestLoginPost_WrongKey_ShowsErrorWithoutSession verifies that a wrong key +// shows the login form again with an error, and sets no session cookie. +func TestLoginPost_WrongKey_ShowsErrorWithoutSession(t *testing.T) { + t.Parallel() + + _, srv := newCSRFTestRouter(t) + + cookies, token := csrfCredentials(t, srv, nil) + + rec := postForm(srv, "/", cookies, url.Values{ + loginKeyField: {"wrong-signing-key-fedcba9876543210"}, + csrfTokenField: {token}, + }) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + body := rec.Body.String() + if !strings.Contains(body, loginForm) || !strings.Contains(body, loginKeyInput) { + t.Errorf("page is not the login form: %s", body) + } + + if !strings.Contains(body, "Invalid signing key") { + t.Error("login form does not show the error") + } + + if c := findSessionCookie(rec); c != nil { + t.Errorf("wrong key set a session cookie: %s", c) + } +} + +// TestLoginPost_RightKey_SetsSessionCookie verifies that the right key answers +// 303 to / with a session cookie marked Secure, HttpOnly and SameSite=Strict, +// and that GET / with that cookie shows the generator page. +func TestLoginPost_RightKey_SetsSessionCookie(t *testing.T) { + t.Parallel() + + _, srv := newCSRFTestRouter(t) + + cookies, token := csrfCredentials(t, srv, nil) + + rec := postForm(srv, "/", cookies, url.Values{ + loginKeyField: {testSigningKey}, + csrfTokenField: {token}, + }) + + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/" { + t.Fatalf("status = %d, Location = %q, want %d to /", + rec.Code, rec.Header().Get("Location"), http.StatusSeeOther) + } + + sessionCookie := findSessionCookie(rec) + if sessionCookie == nil { + t.Fatal("right key set no session cookie") + } + + t.Logf("Set-Cookie: %s", sessionCookie) + + if !sessionCookie.Secure { + t.Error("session cookie is not Secure") + } + + if !sessionCookie.HttpOnly { + t.Error("session cookie is not HttpOnly") + } + + if sessionCookie.SameSite != http.SameSiteStrictMode { + t.Errorf("session cookie SameSite = %v, want Strict", sessionCookie.SameSite) + } + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil) + req.AddCookie(sessionCookie) + + rec = httptest.NewRecorder() + srv.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK || + !strings.Contains(rec.Body.String(), generatorForm) { + t.Errorf("GET / with the session cookie: status = %d, "+ + "want %d and the generator page", rec.Code, http.StatusOK) + } +} + +// TestHandleLogout_ClearsSessionCookie verifies that GET /logout answers 303 +// to / and replaces the session cookie with an empty one sent with +// Max-Age=0, which makes the browser delete it. +func TestHandleLogout_ClearsSessionCookie(t *testing.T) { + t.Parallel() + + h, _ := newCSRFTestRouter(t) + + req := httptest.NewRequestWithContext( + t.Context(), http.MethodGet, "/logout", nil) + req.AddCookie(newSessionCookie(t, h)) + + rec := httptest.NewRecorder() + h.HandleLogout().ServeHTTP(rec, req) + + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/" { + t.Fatalf("status = %d, Location = %q, want %d to /", + rec.Code, rec.Header().Get("Location"), http.StatusSeeOther) + } + + t.Logf("Set-Cookie: %s", rec.Header().Get("Set-Cookie")) + + sessionCookie := findSessionCookie(rec) + if sessionCookie == nil { + t.Fatal("logout did not set the session cookie") + } + + if sessionCookie.Value != "" { + t.Errorf("session cookie value = %q, want empty", sessionCookie.Value) + } + + // net/http reads a Max-Age=0 attribute back as MaxAge -1. + if sessionCookie.MaxAge != -1 { + t.Errorf("session cookie MaxAge = %d, want -1 (Max-Age=0)", + sessionCookie.MaxAge) + } +} + +// TestGeneratePost_NoSession_RedirectsToLogin verifies that POST /generate +// with a valid CSRF token but no login session answers 303 to / and makes no +// URL. +func TestGeneratePost_NoSession_RedirectsToLogin(t *testing.T) { + t.Parallel() + + _, srv := newCSRFTestRouter(t) + + cookies, token := csrfCredentials(t, srv, nil) + + rec := postForm(srv, "/generate", cookies, url.Values{ + sourceURLField: {testSourceURL}, + csrfTokenField: {token}, + }) + + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/" { + t.Fatalf("status = %d, Location = %q, want %d to /", + rec.Code, rec.Header().Get("Location"), http.StatusSeeOther) + } + + if strings.Contains(rec.Body.String(), "/v1/e/") { + t.Error("a URL was made without a login session") + } +} + +// TestGeneratePost_URLServesImage verifies that the URL the generator page +// makes is served by /v1/e/. The image route runs on handlers of its own, +// made with the same signing key. +func TestGeneratePost_URLServesImage(t *testing.T) { + t.Parallel() + + _, imageSrv := newSignedHostServer(t, slog.New(slog.DiscardHandler)) + + rec := generatePost(t, url.Values{ + sourceURLField: {"https://" + signedHost + photoPath}, + widthField: {"50"}, + heightField: {"50"}, + formatField: {string(imgcache.FormatJPEG)}, + }) + + if rec.Code != http.StatusOK { + t.Fatalf("POST /generate status = %d, want %d", rec.Code, http.StatusOK) + } + + match := generatedURLPattern.FindStringSubmatch(rec.Body.String()) + if match == nil { + t.Fatalf("generator page shows no URL: %s", rec.Body.String()) + } + + t.Logf("generated URL path: %s", match[1]) + + imageRec := httptest.NewRecorder() + imageSrv.ServeHTTP(imageRec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, match[1], nil)) + + requireServedPhoto(t, imageRec) +} diff --git a/internal/handlers/imageenc_token_internal_test.go b/internal/handlers/imageenc_token_internal_test.go new file mode 100644 index 0000000..f2fcda2 --- /dev/null +++ b/internal/handlers/imageenc_token_internal_test.go @@ -0,0 +1,126 @@ +package handlers + +import ( + "image/jpeg" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + "time" + + "sneak.berlin/go/pixa/internal/encurl" +) + +// requireServedPhoto requires that rec answers 200 with the JPEG at photoPath +// on signedHost at the 50x50 that encPhotoURL and the generator tests ask for. +func requireServedPhoto(t *testing.T, rec *httptest.ResponseRecorder) { + t.Helper() + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d; body %q", + rec.Code, http.StatusOK, rec.Body.String()) + } + + contentType := rec.Header().Get("Content-Type") + if contentType != "image/jpeg" { + t.Errorf("Content-Type = %q, want image/jpeg", contentType) + } + + img, err := jpeg.DecodeConfig(rec.Body) + if err != nil { + t.Fatalf("body is not a JPEG: %v", err) + } + + if img.Width != 50 || img.Height != 50 { + t.Errorf("image is %dx%d, want 50x50", img.Width, img.Height) + } +} + +// TestHandleImageEnc_ValidToken_ServesImage verifies that a token made with +// the signing key serves the image it asks for. +func TestHandleImageEnc_ValidToken_ServesImage(t *testing.T) { + t.Parallel() + + h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler)) + + rec := httptest.NewRecorder() + srv.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, encPhotoURL(t, h), nil)) + + requireServedPhoto(t, rec) +} + +// TestHandleImageEnc_RejectedToken verifies that a token that has expired +// answers 410, and that a token with one character changed, a token cut +// short, and a token made with another signing key answer 400. The server +// would serve the photo for a token it accepted. +func TestHandleImageEnc_RejectedToken(t *testing.T) { + t.Parallel() + + h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler)) + + photo := encurl.Payload{ + SourceHost: signedHost, + SourcePath: photoPath, + Width: 50, + Height: 50, + } + + valid, err := h.encGen.Generate(&photo) + if err != nil { + t.Fatalf("Generate() error = %v", err) + } + + expiredPhoto := photo + expiredPhoto.ExpiresAt = time.Now().Add(-time.Minute).Unix() + + expired, err := h.encGen.Generate(&expiredPhoto) + if err != nil { + t.Fatalf("Generate() error = %v", err) + } + + otherGen, err := encurl.NewGenerator("another-signing-key-fedcba9876543210") + if err != nil { + t.Fatalf("encurl.NewGenerator() error = %v", err) + } + + otherKey, err := otherGen.Generate(&photo) + if err != nil { + t.Fatalf("Generate() error = %v", err) + } + + // Changing a character in the middle always changes the decoded bytes; + // the last character of unpadded base64 can carry unused bits. + middle := len(valid) / 2 + + replacement := "A" + if valid[middle] == 'A' { + replacement = "B" + } + + changed := valid[:middle] + replacement + valid[middle+1:] + + tests := []struct { + name string + token string + wantStatus int + }{ + {"expired", expired, http.StatusGone}, + {"one character changed", changed, http.StatusBadRequest}, + {"cut short", valid[:middle], http.StatusBadRequest}, + {"another signing key", otherKey, http.StatusBadRequest}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + rec := getEncToken(srv, tt.token) + t.Logf("GET /v1/e/%s/img.jpg: %d %s", tt.token, rec.Code, rec.Body) + + if rec.Code != tt.wantStatus { + t.Errorf("status = %d, want %d", rec.Code, tt.wantStatus) + } + }) + } +}