From 1e5c8b5ab7c18f4d2885927ecd2d14198bdbc8a8 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 20 Apr 2026 22:32:12 -0400 Subject: [PATCH] fix(api): trim whitespace from bearer token in logout sessionTokenFromHTTP now matches extractBearerToken's trimming behavior. Without this, a bearer header with trailing whitespace (e.g. a buggy client or proxy) authenticates via RequireUser but hashes the padded value on logout, silently no-oping and leaving the server-side session alive. Co-Authored-By: Claude Opus 4.7 --- internal/api/auth.go | 8 ++++++-- internal/api/auth_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/internal/api/auth.go b/internal/api/auth.go index e29beb37..5a5bbf46 100644 --- a/internal/api/auth.go +++ b/internal/api/auth.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "net/http" + "strings" "time" "github.com/jackc/pgx/v5" @@ -41,7 +42,10 @@ func (h *handlers) handleLogout(w http.ResponseWriter, r *http.Request) { // sessionTokenFromHTTP duplicates the internal helper in internal/auth // because that one is unexported. Cheap to repeat here; keeping the auth -// package's internal helper package-private is worth more than DRY. +// package's internal helper package-private is worth more than DRY. Must +// match that helper's trimming behavior exactly — otherwise a bearer with +// trailing whitespace authenticates via RequireUser (which trims) but logout +// hashes the padded value and silently no-ops, leaving the session alive. func sessionTokenFromHTTP(r *http.Request) string { if c, err := r.Cookie(auth.SessionCookieName); err == nil && c.Value != "" { return c.Value @@ -49,7 +53,7 @@ func sessionTokenFromHTTP(r *http.Request) string { h := r.Header.Get("Authorization") const prefix = "bearer " if len(h) > len(prefix) && (h[:7] == "Bearer " || h[:7] == "bearer ") { - return h[len(prefix):] + return strings.TrimSpace(h[len(prefix):]) } return "" } diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go index 961601c7..f0b8cd39 100644 --- a/internal/api/auth_test.go +++ b/internal/api/auth_test.go @@ -205,3 +205,35 @@ func TestHandleLogout_DeletesSessionAndClearsCookie(t *testing.T) { t.Error("session row still present after logout") } } + +func TestHandleLogout_BearerHeaderWithTrailingWhitespaceDeletesSession(t *testing.T) { + h, pool := testHandlers(t) + user := seedUser(t, pool, "alice", "hunter2", false) + + token, err := auth.MintSessionToken() + if err != nil { + t.Fatalf("mint: %v", err) + } + if _, err := dbq.New(pool).InsertSession(context.Background(), dbq.InsertSessionParams{ + UserID: user.ID, + TokenHash: auth.HashSessionToken(token), + }); err != nil { + t.Fatalf("insert: %v", err) + } + + req := httptest.NewRequest(http.MethodPost, "/api/auth/logout", nil) + // Trailing whitespace — RequireUser trims this, so logout must too. + req.Header.Set("Authorization", "Bearer "+token+" ") + req = req.WithContext(context.WithValue(req.Context(), userCtxKeyForTest(), user)) + w := httptest.NewRecorder() + h.handleLogout(w, req) + + if w.Code != http.StatusNoContent { + t.Errorf("status = %d, want 204", w.Code) + } + + _, err = dbq.New(pool).GetSessionByTokenHash(context.Background(), auth.HashSessionToken(token)) + if err == nil { + t.Error("session row still present after bearer logout with trailing whitespace") + } +}