From 0f5cb77265b30c608c8e17d37252b16928400140 Mon Sep 17 00:00:00 2001 From: CallumWayve Date: Fri, 26 Sep 2025 17:21:03 +0100 Subject: [PATCH 1/4] Escape unencoded OAuth state redirects Signed-off-by: Callum Newton --- oauthproxy.go | 20 ++++++++++++++++++-- oauthproxy_test.go | 37 ++++++++++++++++++++++++++++--------- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index c6db18a7..6f7b3d20 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -1250,7 +1250,12 @@ func checkAllowedEmails(req *http.Request, s *sessionsapi.SessionState) bool { // encodeState builds the OAuth state param out of our nonce and // original application redirect func encodeState(nonce string, redirect string, encode bool) string { - rawString := fmt.Sprintf("%v:%v", nonce, redirect) + redirectPart := redirect + if !encode { + redirectPart = url.QueryEscape(redirectPart) + } + + rawString := fmt.Sprintf("%v:%v", nonce, redirectPart) if encode { return base64.RawURLEncoding.EncodeToString([]byte(rawString)) } @@ -1270,7 +1275,18 @@ func decodeState(state string, encode bool) (string, string, error) { if len(parsedState) != 2 { return "", "", errors.New("invalid length") } - return parsedState[0], parsedState[1], nil + nonce := parsedState[0] + redirect := parsedState[1] + + if !encode { + unescapedRedirect, err := url.QueryUnescape(redirect) + if err != nil { + return "", "", err + } + redirect = unescapedRedirect + } + + return nonce, redirect, nil } // addHeadersForProxying adds the appropriate headers the request / response for proxying diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 44569a35..954c5acc 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -3355,26 +3355,45 @@ func TestAuthOnlyAllowedEmailDomains(t *testing.T) { } func TestStateEncodesCorrectly(t *testing.T) { - state := "some_state_to_test" + state := "https://example.com/callback?foo=bar&baz=qux" nonce := "some_nonce_to_test" encodedResult := encodeState(nonce, state, true) - assert.Equal(t, "c29tZV9ub25jZV90b190ZXN0OnNvbWVfc3RhdGVfdG9fdGVzdA", encodedResult) + expectedEncoded := base64.RawURLEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", nonce, state))) + assert.Equal(t, expectedEncoded, encodedResult) notEncodedResult := encodeState(nonce, state, false) - assert.Equal(t, "some_nonce_to_test:some_state_to_test", notEncodedResult) + expectedUnencoded := fmt.Sprintf("%s:%s", nonce, url.QueryEscape(state)) + assert.Equal(t, expectedUnencoded, notEncodedResult) + assert.NotContains(t, notEncodedResult, "&") } func TestStateDecodesCorrectly(t *testing.T) { - nonce, redirect, _ := decodeState("c29tZV9ub25jZV90b190ZXN0OnNvbWVfc3RhdGVfdG9fdGVzdA", true) + state := "https://example.com/callback?foo=bar&baz=qux" + nonce := "some_nonce_to_test" - assert.Equal(t, "some_nonce_to_test", nonce) - assert.Equal(t, "some_state_to_test", redirect) + encodedState := base64.RawURLEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", nonce, state))) + decodedNonce, decodedRedirect, err := decodeState(encodedState, true) + assert.NoError(t, err) + assert.Equal(t, nonce, decodedNonce) + assert.Equal(t, state, decodedRedirect) - nonce2, redirect2, _ := decodeState("some_nonce_to_test:some_state_to_test", false) + rawState := fmt.Sprintf("%s:%s", nonce, url.QueryEscape(state)) + decodedNonce2, decodedRedirect2, err := decodeState(rawState, false) + assert.NoError(t, err) + assert.Equal(t, nonce, decodedNonce2) + assert.Equal(t, state, decodedRedirect2) +} - assert.Equal(t, "some_nonce_to_test", nonce2) - assert.Equal(t, "some_state_to_test", redirect2) +func TestStateRoundTripWithMultipleQueryParameters(t *testing.T) { + state := "https://example.com/callback?foo=bar&baz=qux&zap=zazzle" + nonce := "another_nonce" + + encoded := encodeState(nonce, state, false) + decodedNonce, decodedRedirect, err := decodeState(encoded, false) + assert.NoError(t, err) + assert.Equal(t, nonce, decodedNonce) + assert.Equal(t, state, decodedRedirect) } func TestAuthOnlyAllowedEmails(t *testing.T) { From 3b2f1d476113d4c658560b6b055298b92827f3f7 Mon Sep 17 00:00:00 2001 From: CallumWayve Date: Wed, 29 Oct 2025 20:23:10 +0000 Subject: [PATCH 2/4] Update oauthproxy.go Co-authored-by: Jan Larwig Signed-off-by: Callum Newton --- oauthproxy.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index 6f7b3d20..dd2dca10 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -1278,12 +1278,11 @@ func decodeState(state string, encode bool) (string, string, error) { nonce := parsedState[0] redirect := parsedState[1] - if !encode { - unescapedRedirect, err := url.QueryUnescape(redirect) + if encode { + redirect, err := url.QueryUnescape(redirect) if err != nil { - return "", "", err + return "", "", fmt.Errorf("couldn't parse redirect url: %w", err) } - redirect = unescapedRedirect } return nonce, redirect, nil From fb3fe419d705a22bf8cf356a5f67cf5162debf9e Mon Sep 17 00:00:00 2001 From: CallumWayve Date: Wed, 29 Oct 2025 20:23:17 +0000 Subject: [PATCH 3/4] Update oauthproxy.go Co-authored-by: Jan Larwig Signed-off-by: Callum Newton --- oauthproxy.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index dd2dca10..60524935 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -1250,12 +1250,11 @@ func checkAllowedEmails(req *http.Request, s *sessionsapi.SessionState) bool { // encodeState builds the OAuth state param out of our nonce and // original application redirect func encodeState(nonce string, redirect string, encode bool) string { - redirectPart := redirect if !encode { - redirectPart = url.QueryEscape(redirectPart) + redirect = url.QueryEscape(redirect) } - rawString := fmt.Sprintf("%v:%v", nonce, redirectPart) + rawString := fmt.Sprintf("%v:%v", nonce, redirect) if encode { return base64.RawURLEncoding.EncodeToString([]byte(rawString)) } From bb09f8b71ee6a8a3c5566e83040396ce00aa54fa Mon Sep 17 00:00:00 2001 From: Callum Newton Date: Fri, 21 Nov 2025 17:20:03 +0000 Subject: [PATCH 4/4] chore: add changelog entry for PR 3219 Signed-off-by: Callum Newton --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aa2ef46..9245bf12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ## Changes since v7.13.0 - [#3197](https://github.com/oauth2-proxy/oauth2-proxy/pull/3197) fix: NewRemoteKeySet is not using DefaultHTTPClient (@rsrdesarrollo / @tuunit) +- [#3219](https://github.com/oauth2-proxy/oauth2-proxy/pull/3219) fix: escape OAuth state redirect URLs to preserve query parameters (@CallumWayve) # V7.13.0