diff --git a/internal/utils/utils.go b/internal/utils/utils.go index f6c2a0a..f1f8cf0 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -142,7 +142,7 @@ func DetermineScheme(req *http.Request, forceHTTPS bool) string { // "X-Forwarded-Proto: ftp") would otherwise be emitted verbatim into // redirect_uri as a broken or weakened scheme. Invalid values fall // 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 } diff --git a/session.go b/session.go index 0fa3066..4ed64e3 100644 --- a/session.go +++ b/session.go @@ -2302,6 +2302,13 @@ func (sd *SessionData) SetAccessToken(token string) { 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 // before storing the new chunked one. sd.clearAccessTokenChunks() @@ -2937,6 +2944,9 @@ func (sd *SessionData) GetIncomingPath() string { 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. // This allows the user to be redirected to their originally requested resource // after completing the authentication flow. @@ -2945,6 +2955,11 @@ func (sd *SessionData) GetIncomingPath() string { func (sd *SessionData) SetIncomingPath(path string) { sd.sessionMutex.Lock() 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) if currentVal != path { sd.mainSession.Values["incoming_path"] = path diff --git a/session_access_token_nilrequest_test.go b/session_access_token_nilrequest_test.go new file mode 100644 index 0000000..e8f25f7 --- /dev/null +++ b/session_access_token_nilrequest_test.go @@ -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) + } +} diff --git a/session_incoming_scheme_test.go b/session_incoming_scheme_test.go new file mode 100644 index 0000000..f9c8070 --- /dev/null +++ b/session_incoming_scheme_test.go @@ -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) + } + } +}