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 <noreply@anthropic.com>
This commit is contained in:
@@ -4,6 +4,7 @@ import (
|
|||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/jackc/pgx/v5"
|
"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
|
// sessionTokenFromHTTP duplicates the internal helper in internal/auth
|
||||||
// because that one is unexported. Cheap to repeat here; keeping the 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 {
|
func sessionTokenFromHTTP(r *http.Request) string {
|
||||||
if c, err := r.Cookie(auth.SessionCookieName); err == nil && c.Value != "" {
|
if c, err := r.Cookie(auth.SessionCookieName); err == nil && c.Value != "" {
|
||||||
return c.Value
|
return c.Value
|
||||||
@@ -49,7 +53,7 @@ func sessionTokenFromHTTP(r *http.Request) string {
|
|||||||
h := r.Header.Get("Authorization")
|
h := r.Header.Get("Authorization")
|
||||||
const prefix = "bearer "
|
const prefix = "bearer "
|
||||||
if len(h) > len(prefix) && (h[:7] == "Bearer " || h[:7] == "bearer ") {
|
if len(h) > len(prefix) && (h[:7] == "Bearer " || h[:7] == "bearer ") {
|
||||||
return h[len(prefix):]
|
return strings.TrimSpace(h[len(prefix):])
|
||||||
}
|
}
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -205,3 +205,35 @@ func TestHandleLogout_DeletesSessionAndClearsCookie(t *testing.T) {
|
|||||||
t.Error("session row still present after logout")
|
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")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user