diff options
| author | grm <grm@eyesin.space> | 2026-09-18 13:48:27 +0300 |
|---|---|---|
| committer | grm <grm@eyesin.space> | 2026-09-18 13:48:27 +0300 |
| commit | 526a8742688ed58ae40ba1f254c2e7f1f7bbd866 (patch) | |
| tree | adbcd6e77ee36259e8df5fb53c8762b4cbe6fc81 | |
| parent | d71a01f4fcc4bb5ca040889694c94a07e52fa50e (diff) | |
| download | blogspace-526a8742688ed58ae40ba1f254c2e7f1f7bbd866.tar.gz blogspace-526a8742688ed58ae40ba1f254c2e7f1f7bbd866.tar.bz2 blogspace-526a8742688ed58ae40ba1f254c2e7f1f7bbd866.zip | |
Security: Check the CSRF token on logout
/logout was the one management POST without the token. A blog lives on
a subdomain of the root domain, which is same-site, so SameSite=Lax
does not keep the cookie off a form a blog page submits: any blogger's
custom HTML could log the superadmin out at will. The logout form
already carried _csrf; the handler now checks it through guardPOST.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sd8UPWrvyYCLj97JexNw3A
| -rw-r--r-- | AGENTS.md | 5 | ||||
| -rw-r--r-- | internal/web/handlers_auth.go | 6 | ||||
| -rw-r--r-- | internal/web/web_test.go | 12 |
3 files changed, 22 insertions, 1 deletions
@@ -133,7 +133,10 @@ internal/web/ server.go (host router, middleware, render helpers) `guardPOST(blog.UploadLimit(cfg))`, and `withBlogFiles(n, …)` allows n limits for the Files page's multi-upload (`maxUploadFiles` = 10). Errors from `guardPOST` go through `s.fail`, which answers JSON when the request - has `Accept: application/json` (the upload scripts). + has `Accept: application/json` (the upload scripts). `POST /logout` runs + `guardPOST` too when a user is logged in (blogs are same-site with the + root domain, so `SameSite=Lax` alone would let a blog page log a + superadmin out). - **Hardening** (`web/ratelimit.go`): a per-key token bucket throttles the anonymous endpoints worth abusing — `loginLimit` on `POST /webadmin`, keyed by client address *and* by lowercased username (10 at once, then 10 a diff --git a/internal/web/handlers_auth.go b/internal/web/handlers_auth.go index 708285d..5c57254 100644 --- a/internal/web/handlers_auth.go +++ b/internal/web/handlers_auth.go @@ -91,7 +91,13 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, s.landing(r, u, next), http.StatusSeeOther) } +// handleLogout ends the session. It needs the CSRF token like every other +// management POST, or any page could log the user out (a blog is same-site, +// so SameSite=Lax alone would not stop it); anonymous requests just bounce. func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) { + if currentUser(r) != nil && !s.guardPOST(w, r, 0) { + return + } auth.ClearSessionCookie(w, s.cfg.HTTPS) http.Redirect(w, r, "/webadmin", http.StatusSeeOther) } diff --git a/internal/web/web_test.go b/internal/web/web_test.go index 7f66085..74c0fb7 100644 --- a/internal/web/web_test.go +++ b/internal/web/web_test.go @@ -1014,3 +1014,15 @@ func TestSafeNext(t *testing.T) { } } } + +// Logging out while not logged in just goes to the login page (nothing to protect). +func TestLogoutAnonymous(t *testing.T) { + s := NewServer(&config.Config{BaseDomain: "example.com", JWTSecret: []byte("x")}, nil) + rec := httptest.NewRecorder() + req := httptest.NewRequest("POST", "/logout", nil) + req.Host = "example.com" + s.ServeHTTP(rec, req) + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/webadmin" { + t.Errorf("got %d → %q", rec.Code, rec.Header().Get("Location")) + } +} |
