cf5d34a · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

revert: "fix: skip provider button auth only redirect (#3309)" (#3314)

This reverts commit 9c61c49ec29dce1c6bddfe209940bb406f42f8fc. The original fix broke nginx deployments using `auth_request`. When `/oauth2/auth` returns 302, nginx's `auth_request` module treats this as an internal error: [error] auth request unexpected status: 302 while sending to client nginx then returns **500 Internal Server Error** to the browser. > If the subrequest returns a 2xx response code, the access is allowed. If it returns 401 or 403, > the access is denied with the corresponding error code. Any other response code returned by the > subrequest is considered an error. https://nginx.org/en/docs/http/ngx_http_auth_request_module.html The nginx `auth_request` module has strict semantics (non-negotiable): | Subrequest status | nginx behavior | |---|---| | 2xx | Allow request | | 401 / 403 | Deny → trigger `error_page` | | **Any other status** | **Internal error → 500** | The `/oauth2/auth` endpoint is used as a **policy oracle** (yes/no decision), not as a browser-facing endpoint. It cannot return redirects. Any nginx deployment with: - `skip-provider-button=true` - Using `auth_request` directive Will receive 500 errors instead of the expected authentication flow. The correct fix for #334 is a **documentation update**, not a code change: ```nginx error_page 401 = @oauth2_signin; location @oauth2_signin { return 302 /oauth2/sign_in?rd=$scheme://$host$request_uri; } ``` This keeps `/oauth2/auth` as a pure 401/2xx oracle and lets nginx perform the proper 302 redirect to the browser. - Original Issue: #334 - Regression introduced in PR: #3309 Signed-off-by: Stefan Markmann <stefan@markmann.net> Signed-off-by: Jan Larwig <jan@larwig.com> Co-authored-by: Jan Larwig <jan@larwig.com>

commitcf5d34acf60dc4e2a232e073bb906169be1dd32b authorStefan Markmann <50301139+StefanMarkmann@users.noreply.github.com> dateSun 18 Jan 2026, 00:36 (9 months ago) parent7bf586c

5 files changed+7−54

compare with master

unifiedsplit

CHANGELOG.md+2 docs/docs/configuration/overview.md+1−1 docs/versioned_docs/version-7.14.x/configuration/overview.md+1−1 oauthproxy.go+1−9 oauthproxy_test.go+2−43

CHANGELOG.md+2

@@ -8,6 +8,8 @@

99## Changes since v7.14.1

1010

11+- [#3314](https://github.com/oauth2-proxy/oauth2-proxy/pull/3314) revert: fix: skip provider button auth only redirect (#3309)

12+

1113# V7.14.1

1214

1315## Release Highlights

docs/docs/configuration/overview.md+1−1

@@ -214,7 +214,7 @@ Provider specific options can be found on their respective subpages.

215215| flag: `--skip-auth-route` toml: `skip_auth_routes` | string \| list | bypass authentication for requests that match the method & path. Format: method=path_regex OR method!=path_regex. For all methods: path_regex OR !=path_regex | |

216216| flag: `--skip-jwt-bearer-tokens` toml: `skip_jwt_bearer_tokens` | bool | will skip requests that have verified JWT bearer tokens (the token must have [`aud`](https://en.wikipedia.org/wiki/JSON_Web_Token#Standard_fields) that matches this client id or one of the extras from `extra-jwt-issuers`) | false |

217−| flag: `--skip-provider-button` toml: `skip_provider_button` | bool | will skip sign-in-page to directly reach the next step: oauth/start. When enabled, the `/oauth2/auth` endpoint returns 302 redirects instead of 401, ensuring compatibility with nginx `auth_request` and similar reverse proxy authentication architectures. | false |

219219| flag: `--trusted-ip` toml: `trusted_ips` | string \| list | list of IPs or CIDR ranges to allow to bypass authentication (may be given multiple times). When combined with `--reverse-proxy` and optionally `--real-client-ip-header` this will evaluate the trust of the IP stored in an HTTP header by a reverse proxy rather than the layer-3/4 remote address. WARNING: trusting IPs has inherent security flaws, especially when obtaining the IP address from an HTTP header (reverse-proxy mode). Use this option only if you understand the risks and how to manage them. | |

220220| flag: `--whitelist-domain` toml: `whitelist_domains` | string \| list | allowed domains for redirection after authentication. Prefix domain with a `.` or a `*.` to allow subdomains (e.g. `.example.com`, `*.example.com`) [^2] | |

docs/versioned_docs/version-7.14.x/configuration/overview.md+1−1

@@ -214,7 +214,7 @@ Provider specific options can be found on their respective subpages.

oauthproxy.go+1−9

@@ -988,15 +988,7 @@ func (p *OAuthProxy) enrichSessionState(ctx context.Context, s *sessionsapi.Sess

988988// and optional authorization).

989989func (p *OAuthProxy) AuthOnly(rw http.ResponseWriter, req *http.Request) {

990990 session, err := p.getAuthenticatedSession(rw, req)

991− // If SkipProviderButton is enabled and user needs login, redirect directly

992− // to OAuth provider instead of returning 401. This allows nginx auth_request

993− // to pass through the 302 redirect to the browser, bypassing error_page

994− // handling which can break redirect flows.

995− // See: https://github.com/oauth2-proxy/oauth2-proxy/issues/334

996− if p.SkipProviderButton && err == ErrNeedsLogin {

997− p.doOAuthStart(rw, req, nil)

998− return

999− } else if err != nil {

991+ if err != nil {

1000992 http.Error(rw, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)

1001993 return

1002994 }

oauthproxy_test.go+2−43

@@ -834,20 +834,8 @@ func NewProcessCookieTest(opts ProcessCookieTestOpts, modifiers ...OptionsModifi

834834 return nil, err

835835 }

836836 testProvider := &TestProvider{

837− ProviderData: &providers.ProviderData{

838− ProviderName: "Test Provider",

839− LoginURL: &url.URL{

840− Scheme: "http",

841− Host: "localhost",

842− Path: "/oauth/authorize",

843− },

844− RedeemURL: &url.URL{

845− Scheme: "http",

846− Host: "localhost",

847− Path: "/oauth/token",

848− },

849− },

850− ValidToken: opts.providerValidateCookieResponse,

837+ ProviderData: &providers.ProviderData{},

838+ ValidToken: opts.providerValidateCookieResponse,

851839 }

852840

853841 groups := pcTest.opts.Providers[0].AllowedGroups

@@ -1124,35 +1112,6 @@ func TestAuthOnlyEndpointUnauthorizedOnNoCookieSetError(t *testing.T) {

11241112 assert.Equal(t, "Unauthorized\n", string(bodyBytes))

11251113}

11261114

1127−// TestAuthOnlyEndpointRedirectWithSkipProviderButton tests that when SkipProviderButton

1128−// is true and no session exists, the /auth endpoint should return a 302 redirect

1129−// to the OAuth provider instead of 401. This is important for nginx auth_request

1130−// architecture where 401 triggers error_page handling which can break the redirect flow.

1131−// See: https://github.com/oauth2-proxy/oauth2-proxy/issues/334

1132−func TestAuthOnlyEndpointRedirectWithSkipProviderButton(t *testing.T) {

1133− test, err := NewAuthOnlyEndpointTest("", func(opts *options.Options) {

1134− opts.SkipProviderButton = true

1135− })

1136− if err != nil {

1137− t.Fatal(err)

1138− }

1139−

1140− test.proxy.ServeHTTP(test.rw, test.req)

1141−

1142− // With SkipProviderButton=true and no session, should return 302 redirect

1143− // to OAuth provider, NOT 401 Unauthorized

1144− assert.Equal(t, http.StatusFound, test.rw.Code)

1145− location := test.rw.Header().Get("Location")

1146− assert.NotEmpty(t, location, "Expected Location header for redirect")

1147−

1148− // Verify the redirect points to the OAuth provider's authorize endpoint

1149− // and contains key OAuth parameters

1150− assert.Contains(t, location, "/oauth/authorize", "Expected redirect to OAuth authorize endpoint")

1151− assert.Contains(t, location, "client_id=", "Expected client_id in redirect URL")

1152− assert.Contains(t, location, "redirect_uri=", "Expected redirect_uri in redirect URL")

1153− assert.Contains(t, location, "state=", "Expected state parameter in redirect URL")

1154−}

1155−

11561115func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) {

11571116 test, err := NewAuthOnlyEndpointTest("", func(opts *options.Options) {

11581117 opts.Cookie.Expire = time.Duration(24) * time.Hour