fix(session): keep old access token on failed store, cap incoming path, fold proto case

SetAccessToken cleared the old token before it found that no request
was available for the chunk cookies. The old token was lost.
The stored post-login path had no length bound and could bloat every
later Cookie header. It is now capped at 2048 bytes.
DetermineScheme compared X-Forwarded-Proto case-sensitively while the
cookie code did not, so HTTPS gave an http redirect_uri.
This commit is contained in:
2026-11-03 01:10:36 +00:00
parent dc4da7b4c9
commit 3ce5f428cf
4 changed files with 93 additions and 1 deletions
+1 -1
View File
@@ -142,7 +142,7 @@ func DetermineScheme(req *http.Request, forceHTTPS bool) string {
// "X-Forwarded-Proto: ftp") would otherwise be emitted verbatim into // "X-Forwarded-Proto: ftp") would otherwise be emitted verbatim into
// redirect_uri as a broken or weakened scheme. Invalid values fall // redirect_uri as a broken or weakened scheme. Invalid values fall
// through to the TLS check (R103). // through to the TLS check (R103).
if scheme := req.Header.Get("X-Forwarded-Proto"); scheme == "https" || scheme == "http" { if scheme := strings.ToLower(req.Header.Get("X-Forwarded-Proto")); scheme == "https" || scheme == "http" {
return scheme return scheme
} }
+15
View File
@@ -2302,6 +2302,13 @@ func (sd *SessionData) SetAccessToken(token string) {
return return
} }
// Chunk cookies bind to the request; without one the store cannot
// complete, so keep the previous token.
if sd.request == nil {
sd.manager.logger.Error("SetAccessToken: sd.request is nil, cannot create chunk sessions")
return
}
// Validation passed: now it is safe to clear the existing token // Validation passed: now it is safe to clear the existing token
// before storing the new chunked one. // before storing the new chunked one.
sd.clearAccessTokenChunks() sd.clearAccessTokenChunks()
@@ -2937,6 +2944,9 @@ func (sd *SessionData) GetIncomingPath() string {
return path return path
} }
// maxIncomingPathLen bounds the post-login redirect path kept in the session.
const maxIncomingPathLen = 2048
// SetIncomingPath stores the original request URI for post-authentication redirect. // SetIncomingPath stores the original request URI for post-authentication redirect.
// This allows the user to be redirected to their originally requested resource // This allows the user to be redirected to their originally requested resource
// after completing the authentication flow. // after completing the authentication flow.
@@ -2945,6 +2955,11 @@ func (sd *SessionData) GetIncomingPath() string {
func (sd *SessionData) SetIncomingPath(path string) { func (sd *SessionData) SetIncomingPath(path string) {
sd.sessionMutex.Lock() sd.sessionMutex.Lock()
defer sd.sessionMutex.Unlock() defer sd.sessionMutex.Unlock()
// The path becomes cookie bytes on every later request; an oversized one
// would bloat the Cookie header or fail the save. Fall back to "/".
if len(path) > maxIncomingPathLen {
path = ""
}
currentVal, _ := sd.mainSession.Values["incoming_path"].(string) currentVal, _ := sd.mainSession.Values["incoming_path"].(string)
if currentVal != path { if currentVal != path {
sd.mainSession.Values["incoming_path"] = path sd.mainSession.Values["incoming_path"] = path
+36
View File
@@ -0,0 +1,36 @@
package traefikoidc
import (
"crypto/rand"
"encoding/base64"
"net/http/httptest"
"testing"
)
// A chunked store that cannot run (no request to bind chunk cookies to) must
// leave the previous access token intact.
func TestSetAccessTokenNilRequestKeepsOldToken(t *testing.T) {
sm, err := NewSessionManager("test-encryption-key-32-characters", false, "", "", 0, NewLogger("error"))
if err != nil {
t.Fatal(err)
}
sess, err := sm.GetSession(httptest.NewRequest("GET", "/", nil))
if err != nil {
t.Fatal(err)
}
t.Cleanup(sess.returnToPoolSafely)
const old = "smallaccesstokenvaluevaluevalue"
sess.SetAccessToken(old)
blob := make([]byte, 3000)
if _, err := rand.Read(blob); err != nil {
t.Fatal(err)
}
sess.request = nil
sess.SetAccessToken(base64.StdEncoding.EncodeToString(blob))
if got := sess.GetAccessToken(); got != old {
t.Fatalf("old access token lost after a failed chunked store: %q", got)
}
}
+41
View File
@@ -0,0 +1,41 @@
package traefikoidc
import (
"net/http/httptest"
"strings"
"testing"
"github.com/lukaszraczylo/traefikoidc/internal/utils"
)
// A deep link longer than the cap must not be persisted into the cookies.
func TestSetIncomingPathCapsLength(t *testing.T) {
sm, err := NewSessionManager("test-encryption-key-32-characters", false, "", "", 0, NewLogger("error"))
if err != nil {
t.Fatal(err)
}
sess, err := sm.GetSession(httptest.NewRequest("GET", "/", nil))
if err != nil {
t.Fatal(err)
}
t.Cleanup(sess.returnToPoolSafely)
sess.SetIncomingPath("/ok?a=1")
if got := sess.GetIncomingPath(); got != "/ok?a=1" {
t.Fatalf("short path changed: %q", got)
}
sess.SetIncomingPath("/x?" + strings.Repeat("q", maxIncomingPathLen+1))
if got := sess.GetIncomingPath(); got != "" {
t.Fatalf("oversized path stored (%d bytes)", len(got))
}
}
func TestDetermineSchemeCaseInsensitive(t *testing.T) {
for in, want := range map[string]string{"HTTPS": "https", "Https": "https", "HTTP": "http", "https": "https"} {
r := httptest.NewRequest("GET", "/", nil)
r.Header.Set("X-Forwarded-Proto", in)
if got := utils.DetermineScheme(r, false); got != want {
t.Errorf("X-Forwarded-Proto %q -> %q, want %q", in, got, want)
}
}
}