From 9c61c49ec29dce1c6bddfe209940bb406f42f8fc Mon Sep 17 00:00:00 2001 From: Stefan Markmann <50301139+StefanMarkmann@users.noreply.github.com> Date: Sat, 17 Jan 2026 14:35:44 +0100 Subject: [PATCH] fix: skip provider button auth only redirect (#3309) * fix: Return 302 redirect from AuthOnly when skip-provider-button is true When SkipProviderButton is enabled and a user needs to login, the AuthOnly endpoint now returns a 302 redirect directly to the OAuth provider instead of returning 401. This fixes an issue with nginx auth_request architecture where 401 triggers error_page handling, which can break redirect flows because nginx overrides the status code (e.g., to 403), and browsers don't follow Location headers for non-3xx responses. Fixes: #334 Signed-off-by: Stefan Markmann * update docs and changelog Signed-off-by: Stefan Markmann * test: Add specific OAuth redirect assertions per code review feedback Improve TestAuthOnlyEndpointRedirectWithSkipProviderButton to verify that the Location header actually redirects to the OAuth provider's authorize endpoint with required parameters (client_id, redirect_uri, state), not just that a Location header exists. Signed-off-by: Stefan Markmann * refactor: Flatten AuthOnly error handling structure Move the SkipProviderButton check outside of the nested err != nil block using an if-else structure. This makes the special case more visible and reduces nesting depth without changing behavior. Signed-off-by: Stefan Markmann * doc: backport to v7.14.x Signed-off-by: Jan Larwig --------- Signed-off-by: Stefan Markmann Signed-off-by: Jan Larwig Co-authored-by: Jan Larwig --- CHANGELOG.md | 2 + docs/docs/configuration/overview.md | 2 +- .../version-7.14.x/configuration/overview.md | 2 +- oauthproxy.go | 10 ++++- oauthproxy_test.go | 45 ++++++++++++++++++- 5 files changed, 56 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f814426..00412234 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ## Changes since v7.14.0 +- [#3309](https://github.com/oauth2-proxy/oauth2-proxy/pull/3309) fix: Return 302 redirect from AuthOnly endpoint when skip-provider-button is true (@StefanMarkmann) + # V7.14.0 ## Release Highlights diff --git a/docs/docs/configuration/overview.md b/docs/docs/configuration/overview.md index 7bd7bf07..c5455336 100644 --- a/docs/docs/configuration/overview.md +++ b/docs/docs/configuration/overview.md @@ -214,7 +214,7 @@ Provider specific options can be found on their respective subpages. | flag: `--skip-auth-regex`
toml: `skip_auth_regex` | string \| list | (DEPRECATED for `--skip-auth-route`) bypass authentication for requests paths that match (may be given multiple times) | | | 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 | | | 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 | -| flag: `--skip-provider-button`
toml: `skip_provider_button` | bool | will skip sign-in-page to directly reach the next step: oauth/start | false | +| 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 | | flag: `--ssl-insecure-skip-verify`
toml: `ssl_insecure_skip_verify` | bool | skip validation of certificates presented when using HTTPS providers | false | | 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. | | | 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] | | diff --git a/docs/versioned_docs/version-7.14.x/configuration/overview.md b/docs/versioned_docs/version-7.14.x/configuration/overview.md index 7bd7bf07..c5455336 100644 --- a/docs/versioned_docs/version-7.14.x/configuration/overview.md +++ b/docs/versioned_docs/version-7.14.x/configuration/overview.md @@ -214,7 +214,7 @@ Provider specific options can be found on their respective subpages. | flag: `--skip-auth-regex`
toml: `skip_auth_regex` | string \| list | (DEPRECATED for `--skip-auth-route`) bypass authentication for requests paths that match (may be given multiple times) | | | 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 | | | 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 | -| flag: `--skip-provider-button`
toml: `skip_provider_button` | bool | will skip sign-in-page to directly reach the next step: oauth/start | false | +| 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 | | flag: `--ssl-insecure-skip-verify`
toml: `ssl_insecure_skip_verify` | bool | skip validation of certificates presented when using HTTPS providers | false | | 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. | | | 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] | | diff --git a/oauthproxy.go b/oauthproxy.go index d933d930..dda1b34f 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -987,7 +987,15 @@ func (p *OAuthProxy) enrichSessionState(ctx context.Context, s *sessionsapi.Sess // and optional authorization). func (p *OAuthProxy) AuthOnly(rw http.ResponseWriter, req *http.Request) { session, err := p.getAuthenticatedSession(rw, req) - if err != nil { + // If SkipProviderButton is enabled and user needs login, redirect directly + // to OAuth provider instead of returning 401. This allows nginx auth_request + // to pass through the 302 redirect to the browser, bypassing error_page + // handling which can break redirect flows. + // See: https://github.com/oauth2-proxy/oauth2-proxy/issues/334 + if p.SkipProviderButton && err == ErrNeedsLogin { + p.doOAuthStart(rw, req, nil) + return + } else if err != nil { http.Error(rw, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) return } diff --git a/oauthproxy_test.go b/oauthproxy_test.go index ccabdbbd..489a1b69 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -834,8 +834,20 @@ func NewProcessCookieTest(opts ProcessCookieTestOpts, modifiers ...OptionsModifi return nil, err } testProvider := &TestProvider{ - ProviderData: &providers.ProviderData{}, - ValidToken: opts.providerValidateCookieResponse, + ProviderData: &providers.ProviderData{ + ProviderName: "Test Provider", + LoginURL: &url.URL{ + Scheme: "http", + Host: "localhost", + Path: "/oauth/authorize", + }, + RedeemURL: &url.URL{ + Scheme: "http", + Host: "localhost", + Path: "/oauth/token", + }, + }, + ValidToken: opts.providerValidateCookieResponse, } groups := pcTest.opts.Providers[0].AllowedGroups @@ -1112,6 +1124,35 @@ func TestAuthOnlyEndpointUnauthorizedOnNoCookieSetError(t *testing.T) { assert.Equal(t, "Unauthorized\n", string(bodyBytes)) } +// TestAuthOnlyEndpointRedirectWithSkipProviderButton tests that when SkipProviderButton +// is true and no session exists, the /auth endpoint should return a 302 redirect +// to the OAuth provider instead of 401. This is important for nginx auth_request +// architecture where 401 triggers error_page handling which can break the redirect flow. +// See: https://github.com/oauth2-proxy/oauth2-proxy/issues/334 +func TestAuthOnlyEndpointRedirectWithSkipProviderButton(t *testing.T) { + test, err := NewAuthOnlyEndpointTest("", func(opts *options.Options) { + opts.SkipProviderButton = true + }) + if err != nil { + t.Fatal(err) + } + + test.proxy.ServeHTTP(test.rw, test.req) + + // With SkipProviderButton=true and no session, should return 302 redirect + // to OAuth provider, NOT 401 Unauthorized + assert.Equal(t, http.StatusFound, test.rw.Code) + location := test.rw.Header().Get("Location") + assert.NotEmpty(t, location, "Expected Location header for redirect") + + // Verify the redirect points to the OAuth provider's authorize endpoint + // and contains key OAuth parameters + assert.Contains(t, location, "/oauth/authorize", "Expected redirect to OAuth authorize endpoint") + assert.Contains(t, location, "client_id=", "Expected client_id in redirect URL") + assert.Contains(t, location, "redirect_uri=", "Expected redirect_uri in redirect URL") + assert.Contains(t, location, "state=", "Expected state parameter in redirect URL") +} + func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) { test, err := NewAuthOnlyEndpointTest("", func(opts *options.Options) { opts.Cookie.Expire = time.Duration(24) * time.Hour