From 96c5ab6e190c9d6bb25eebbfa45e2866fd4835bd Mon Sep 17 00:00:00 2001 From: Elaine Souza Date: Thu, 11 Jun 2026 13:59:11 -0300 Subject: [PATCH 1/2] fix: return 403 instead of 500 for malformed OAuth2 state on callback A missing or malformed 'state' parameter on /oauth2/callback (e.g. crawlers and scanners hitting the callback directly, without going through /oauth2/start) is an auth failure, not a server error. Previously this path returned HTTP 500, which surfaces a healthy proxy as a server failure and trips 5xx-based alerting. Return 403 Forbidden instead, restoring the pre-v7.7.1 behaviour and matching the surrounding auth-failure paths in OAuthCallback (CSRF cookie missing, CSRF token mismatch), which already return 403. Adds a regression test covering empty and non-colon-delimited state values. Fixes #2822 Refs #2833 Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Elaine Souza --- CHANGELOG.md | 2 ++ oauthproxy.go | 9 ++++++++- oauthproxy_test.go | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 788e82c2..4ad2bed4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ## Changes since v7.15.3 +- [#3453](https://github.com/oauth2-proxy/oauth2-proxy/pull/3453) fix: return 403 instead of 500 for malformed OAuth2 state on the callback (@Elaine-cls) + # V7.15.3 ## Release Highlights diff --git a/oauthproxy.go b/oauthproxy.go index f8dc5471..580bf8b7 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -905,8 +905,15 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { nonce, appRedirect, err := decodeState(req.Form.Get("state"), p.encodeState) if err != nil { + // A malformed or missing state parameter is a client/auth failure, not a + // server error: the request did not originate from a valid /oauth2/start + // flow (e.g. crawlers and scanners hitting the callback directly). Return + // 403 instead of 500 so these do not surface as server errors / trigger + // 5xx alerting, leaking minimal information to the caller. This restores + // the pre-v7.7.1 behaviour and is consistent with the other auth-failure + // paths below (CSRF cookie missing, CSRF mismatch), which also return 403. logger.Errorf("Error while parsing OAuth2 state: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) + p.ErrorPage(rw, req, http.StatusForbidden, err.Error()) return } diff --git a/oauthproxy_test.go b/oauthproxy_test.go index b3271e5b..35b17997 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -2087,6 +2087,43 @@ func Test_noCacheHeaders(t *testing.T) { }) } +func TestOAuthCallbackMalformedStateIsClientError(t *testing.T) { + // A crawler/scanner that hits /oauth2/callback directly sends a missing or + // malformed state parameter (no "nonce:redirect" form). This must be + // reported as a 4xx auth failure (403), not a 5xx server error, otherwise it + // pollutes 5xx-based alerting even though the proxy is healthy. + opts := baseTestOptions() + err := validation.Validate(opts) + require.NoError(t, err) + + proxy, err := NewOAuthProxy(opts, func(string) bool { return true }) + require.NoError(t, err) + + testCases := []struct { + name string + state string + }{ + {name: "EmptyState", state: ""}, + {name: "NoColonSeparator", state: "garbage"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + rec := httptest.NewRecorder() + req := httptest.NewRequest( + http.MethodGet, + fmt.Sprintf("/oauth2/callback?code=abc&state=%s", url.QueryEscape(tc.state)), + nil, + ) + proxy.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusForbidden, rec.Code) + assert.Less(t, rec.Code, http.StatusInternalServerError, + "malformed state must not be reported as a 5xx server error") + }) + } +} + func TestSignOutCallsBackendLogoutURL(t *testing.T) { const testIDToken = "test-id-token-12345" var receivedURL string From b8a981fea9cc5efc337be4a0d93e9f9763912b91 Mon Sep 17 00:00:00 2001 From: Elaine Souza Date: Tue, 4 Aug 2026 09:31:06 -0300 Subject: [PATCH 2/2] Use friendly login-failed message for malformed state error page Keeps err.Error() as the debug-only message and shows a user-facing message consistent with the other 403 auth-failure branches in OAuthCallback, per review feedback on PR #3453. --- oauthproxy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauthproxy.go b/oauthproxy.go index 580bf8b7..165c82ae 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -913,7 +913,7 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { // the pre-v7.7.1 behaviour and is consistent with the other auth-failure // paths below (CSRF cookie missing, CSRF mismatch), which also return 403. logger.Errorf("Error while parsing OAuth2 state: %v", err) - p.ErrorPage(rw, req, http.StatusForbidden, err.Error()) + p.ErrorPage(rw, req, http.StatusForbidden, err.Error(), "Login Failed: invalid or missing state parameter.") return }