Skip to content

Commit fb5450f

Browse files
committed
fix(session): redesign read-only extractor handling — opt-in trust + cached source
Replaces the implicit "preserve query/form/param/custom session IDs" behaviour with an explicit, validated opt-in and centralizes source tracking so chained extractors stay consistent across multiple Store.Get calls in the same request. Changes: - extractors: add Source.IsWritable() so callers no longer hand-roll the cookie/header check. - session.Store: cache the resolved session ID together with its originating Source in request locals (sessionIDInfo). resolveSessionID iterates chain sub-extractors and reports the source that actually produced the value; subsequent Store.Get calls in the same request reuse that decision instead of re-deriving it from the chain wrapper. - session.Config: add TrustClientSessionID (default false) and ClientSessionIDValidator. Read-only sources (query/form/param/custom) only preserve a client-supplied unknown ID when both flags are set and the validator accepts it; otherwise the ID is discarded and a fresh server ID is generated, matching cookie/header fixation protection. Cookie/header sources are unaffected and always discard unknown IDs. - docs/middleware/session.md: document the new flags, the security trade-offs, and the recommended HMAC/allow-list validator pattern. - store_test.go: update Test_Store_resolveSessionID to the new signature, cover trust-on/off, validator rejects, opt-in roundtrip, chain source resolution, two-Get-same-request consistency for both query and cookie sources, and empty client ID. Test_Store_DeleteSession now models the cross-request delete + re-Get path instead of relying on the previous regenerate-loop bug. Closes #4234
1 parent 52046e0 commit fb5450f

5 files changed

Lines changed: 357 additions & 102 deletions

File tree

docs/middleware/session.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,37 @@ extractors.Chain(
369369
)
370370
```
371371

372+
### Trusting Client-Supplied IDs from Read-Only Sources
373+
374+
By default, an unknown session ID from any source is discarded and a new one is generated via `KeyGenerator`. For cookie/header sources that is also the response channel for the new ID, so the next request continues with it. Read-only sources (query, form, URL param, custom extractors) cannot communicate a new ID back, so the same client request would otherwise create a new orphan session every time.
375+
376+
If your application needs read-only sources to drive a persistent session — for example a non-browser client that always sends the same `?SESSIONID=...` — opt in explicitly:
377+
378+
```go
379+
app.Use(session.New(session.Config{
380+
Extractor: extractors.FromQuery("SESSIONID"),
381+
TrustClientSessionID: true,
382+
ClientSessionIDValidator: func(id string) bool {
383+
// Verify the format/origin of the ID. Reject anything you did not issue.
384+
// Example: HMAC-signed IDs, length checks, allow-list lookups, ...
385+
return isValidSignedID(id)
386+
},
387+
}))
388+
```
389+
390+
**Security implications.** Trusting client-supplied IDs without validation enables:
391+
392+
- **Session fixation** — an attacker can craft a link such as `?SESSIONID=ATTACKER_KNOWN_VALUE`; once the victim follows it, the server creates a session under that ID and the attacker can hijack it.
393+
- **Storage poisoning** — any caller can populate your session storage with arbitrary keys.
394+
395+
Mitigations:
396+
397+
1. Always supply a `ClientSessionIDValidator` that rejects IDs you did not issue (HMAC signature, registered allow-list, signed JWT, etc.).
398+
2. Combine the read-only source with a server-issued token bootstrap step.
399+
3. Prefer cookie or header extractors whenever the client can store them.
400+
401+
Cookie and header sources are unaffected by this flag — their unknown IDs are always discarded to prevent fixation.
402+
372403
### Custom Extractors (Session-specific)
373404

374405
Prefer the helper constructors from the extractors module. See the Extractors Guide for the full API; below are session-specific examples and notes.
@@ -681,6 +712,8 @@ extractors.Chain(extractors ...extractors.Extractor) extractors.Extractor
681712
| `Store` | `*session.Store` | Pre-built session store (use when you need to share/register types) | `nil` (auto-created) |
682713
| `Storage` | `fiber.Storage` | Session storage backend (used when creating a store if `Store` is nil) | `memory.New()` |
683714
| `Extractor` | `extractors.Extractor` | Session ID extraction | `extractors.FromCookie("session_id")` |
715+
| `TrustClientSessionID` | `bool` | Accept client-supplied IDs from read-only sources (query/form/param/custom) when no data exists. Requires `ClientSessionIDValidator`. | `false` |
716+
| `ClientSessionIDValidator` | `func(string) bool` | Validates a client-supplied session ID before persisting it. Required when `TrustClientSessionID` is `true`; `nil` rejects all. | `nil` |
684717
| `KeyGenerator` | `func() string` | Session ID generator | `utils.SecureToken` |
685718
| `IdleTimeout` | `time.Duration` | Inactivity timeout | `30 * time.Minute` |
686719
| `AbsoluteTimeout` | `time.Duration` | Maximum session duration | `0` (unlimited) |

extractors/extractors.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,15 @@ const (
6060
SourceCustom
6161
)
6262

63+
// IsWritable reports whether the source allows the server to send a value back
64+
// to the client in the response (e.g. Set-Cookie, response header). Cookie and
65+
// header sources are writable; query, form, param, custom and auth-header
66+
// sources are read-only — the client controls the value on every request and
67+
// the server cannot rotate it in the response.
68+
func (s Source) IsWritable() bool {
69+
return s == SourceCookie || s == SourceHeader
70+
}
71+
6372
// ErrNotFound is returned when the requested value is missing or empty.
6473
var ErrNotFound = errors.New("value not found")
6574

middleware/session/config.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,36 @@ type Config struct {
5656
// Optional. Default: extractors.FromCookie("session_id")
5757
Extractor extractors.Extractor
5858

59+
// TrustClientSessionID controls whether client-supplied session IDs from
60+
// read-only sources (query, form, URL param, custom extractor) are accepted
61+
// as-is when no session data exists yet for that ID.
62+
//
63+
// When false (default), unknown IDs from read-only sources are discarded
64+
// and the server generates a new ID via KeyGenerator — the same behaviour
65+
// already applied to cookie/header sources to prevent session fixation.
66+
//
67+
// When true, the client-supplied ID is preserved and a fresh session is
68+
// stored under it, so subsequent requests carrying the same ID load the
69+
// same session. ClientSessionIDValidator must be set in this case;
70+
// otherwise the client ID is rejected and a new one is generated.
71+
//
72+
// SECURITY: Enabling this opens the door to session fixation and storage
73+
// poisoning by clients that can choose arbitrary IDs. Use only with a
74+
// strong validator (HMAC-signed, length/format-checked, monotonically
75+
// issued, etc.) and prefer cookie/header extractors when possible.
76+
//
77+
// Optional. Default: false
78+
TrustClientSessionID bool
79+
80+
// ClientSessionIDValidator is invoked for client-supplied session IDs from
81+
// read-only sources before they are persisted. Return true to accept the
82+
// ID, false to reject it (a fresh server-generated ID is used instead).
83+
//
84+
// Required when TrustClientSessionID is true; nil = reject all.
85+
//
86+
// Optional. Default: nil
87+
ClientSessionIDValidator func(id string) bool
88+
5989
// IdleTimeout defines the maximum duration of inactivity before the session expires.
6090
//
6191
// Note: The idle timeout is updated on each `Save()` call. If a middleware handler is used, `Save()` is called automatically.

middleware/session/store.go

Lines changed: 65 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,16 @@ const (
2828
sessionIDContextKey sessionIDKey = iota
2929
)
3030

31+
// sessionIDInfo bundles the resolved session ID with the extractor source that
32+
// produced it. Both pieces are cached together in the request locals so that a
33+
// second Store.Get within the same request returns a consistent answer — in
34+
// particular, chained extractors keep their original source decision instead of
35+
// being re-derived from the wrapper Extractor.Source.
36+
type sessionIDInfo struct {
37+
id string
38+
source extractors.Source
39+
}
40+
3141
// Store manages session data using the configured storage backend.
3242
type Store struct {
3343
Config
@@ -124,22 +134,16 @@ func (s *Store) getSession(c fiber.Ctx) (*Session, error) {
124134
var rawData []byte
125135
var err error
126136

127-
id, ok := c.Locals(sessionIDContextKey).(string)
128-
129-
// writableSource tracks whether the session ID came from a writable source
130-
// (cookie or header). For writable sources, an unknown ID is discarded and a new
131-
// one is generated to prevent session fixation. For read-only sources (query,
132-
// form, param, custom), the client-provided ID is preserved so that subsequent
133-
// requests using the same query parameter are associated with the same session.
134-
var writableSource bool
135-
if !ok {
136-
id, writableSource = s.getSessionID(c)
137-
} else {
138-
// ID was cached from a prior call within this request; derive writability
139-
// from the primary (first or only) extractor source.
140-
src := s.Extractor.Source
141-
writableSource = src == extractors.SourceCookie || src == extractors.SourceHeader
137+
// Resolve the session ID and the source that produced it. The pair is cached
138+
// in the request locals so a second call within the same request returns the
139+
// same answer — including for chained extractors where the source is decided
140+
// at extraction time and would otherwise be lost.
141+
info, alreadyResolved := c.Locals(sessionIDContextKey).(sessionIDInfo)
142+
if !alreadyResolved {
143+
info = s.resolveSessionID(c)
144+
c.Locals(sessionIDContextKey, info)
142145
}
146+
id := info.id
143147

144148
fresh := false // Session is not fresh initially; only set to true if we generate a new ID
145149

@@ -150,21 +154,22 @@ func (s *Store) getSession(c fiber.Ctx) (*Session, error) {
150154
return nil, err
151155
}
152156
if rawData == nil {
153-
if writableSource {
154-
// For writable sources (cookie, header), discard the client-provided
155-
// ID and generate a new one to prevent session fixation attacks.
156-
id = ""
157-
} else {
158-
// For read-only sources (query, form, param, custom), preserve the
159-
// client-provided ID and create a fresh session under it so that
160-
// subsequent requests carrying the same ID are served the same session.
161-
//
162-
// Security note: using a read-only source (e.g. FromQuery) means the
163-
// client controls the session ID on every request and the server cannot
164-
// rotate it in the response. Callers that require strong session
165-
// fixation protection should use a cookie or header extractor instead.
157+
switch {
158+
case alreadyResolved:
159+
// A prior call within this request already committed to this ID.
160+
// Keep it so multiple Store.Get calls in the same request observe
161+
// the same session.
162+
fresh = true
163+
case s.acceptClientID(info):
164+
// Read-only source with an opt-in trusted client ID — preserve so
165+
// that subsequent requests carrying the same ID load the same
166+
// session.
166167
fresh = true
167-
c.Locals(sessionIDContextKey, id)
168+
default:
169+
// Writable source (cookie/header) with an unknown ID, or
170+
// untrusted read-only ID — discard and generate a fresh one to
171+
// prevent session fixation and storage poisoning.
172+
id = ""
168173
}
169174
}
170175
}
@@ -173,7 +178,9 @@ func (s *Store) getSession(c fiber.Ctx) (*Session, error) {
173178
if id == "" {
174179
fresh = true // The session is fresh if a new ID is generated
175180
id = s.KeyGenerator()
176-
c.Locals(sessionIDContextKey, id)
181+
// Mark the cached source as cookie so the regenerated ID is treated as
182+
// server-issued (writable) on any subsequent call within this request.
183+
c.Locals(sessionIDContextKey, sessionIDInfo{id: id, source: extractors.SourceCookie})
177184
}
178185

179186
// Create session object
@@ -212,56 +219,54 @@ func (s *Store) getSession(c fiber.Ctx) (*Session, error) {
212219
return sess, nil
213220
}
214221

215-
// getSessionID returns the session ID using the configured extractor, and whether
216-
// the extraction source is writable (cookie or header). A writable source means
217-
// the middleware sets the session ID back in the response (e.g. Set-Cookie), so
218-
// an unrecognized client-supplied ID should be discarded to prevent session
219-
// fixation. A non-writable source (query, form, param, custom) is read-only; the
220-
// client controls the ID on every request, so an unrecognized ID is preserved and
221-
// a fresh session is stored under it.
222-
//
223-
// For chained extractors the function iterates the sub-extractors in order and
224-
// returns the source of the first one that provides a value.
222+
// resolveSessionID extracts the session ID from the request and reports the
223+
// source that produced it. For chained extractors the sub-extractors are tried
224+
// in order so the source of the first one that yields a value wins; for a
225+
// single extractor the source on the wrapper is used. When extraction fails the
226+
// returned ID is empty and the source falls back to the wrapper's source.
225227
//
226228
// Parameters:
227229
// - c: The Fiber context.
228230
//
229231
// Returns:
230-
// - string: The session ID.
231-
// - bool: true when the source is writable (cookie or header).
232-
//
233-
// Usage:
234-
//
235-
// id, writable := store.getSessionID(c)
236-
func (s *Store) getSessionID(c fiber.Ctx) (string, bool) {
237-
isWritable := func(src extractors.Source) bool {
238-
return src == extractors.SourceCookie || src == extractors.SourceHeader
239-
}
240-
232+
// - sessionIDInfo: The resolved ID together with its originating source.
233+
func (s *Store) resolveSessionID(c fiber.Ctx) sessionIDInfo {
241234
ext := s.Extractor
242235

243-
// For chained extractors, try each sub-extractor in order so we can identify
244-
// which source actually provided the value.
245236
if len(ext.Chain) > 0 {
246237
for _, chainExt := range ext.Chain {
247238
if chainExt.Extract == nil {
248239
continue
249240
}
250241
v, err := chainExt.Extract(c)
251242
if err == nil && v != "" {
252-
return v, isWritable(chainExt.Source)
243+
return sessionIDInfo{id: v, source: chainExt.Source}
253244
}
254245
}
255-
return "", false
246+
return sessionIDInfo{source: ext.Source}
256247
}
257248

258-
// Single extractor.
259-
sessionID, err := ext.Extract(c)
249+
v, err := ext.Extract(c)
260250
if err != nil {
261-
// If extraction fails, return empty string to generate a new session
262-
return "", false
251+
return sessionIDInfo{source: ext.Source}
252+
}
253+
return sessionIDInfo{id: v, source: ext.Source}
254+
}
255+
256+
// acceptClientID reports whether a client-supplied session ID from a read-only
257+
// source should be persisted as-is. Writable sources (cookie/header) are never
258+
// accepted here — they are subject to fixation protection. For read-only
259+
// sources the application must explicitly opt in via TrustClientSessionID and
260+
// supply a ClientSessionIDValidator that accepts the ID; otherwise the ID is
261+
// rejected and a server-generated one is used.
262+
func (s *Store) acceptClientID(info sessionIDInfo) bool {
263+
if info.id == "" || info.source.IsWritable() {
264+
return false
265+
}
266+
if !s.TrustClientSessionID || s.ClientSessionIDValidator == nil {
267+
return false
263268
}
264-
return sessionID, isWritable(ext.Source)
269+
return s.ClientSessionIDValidator(info.id)
265270
}
266271

267272
// Reset deletes all sessions from the storage.

0 commit comments

Comments
 (0)