9c61c49 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

on master help sign in

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

9c61c49Stefan Markmann9 months ago

CHANGELOG.md+2

compare with master

unifiedsplit

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

9## Changes since v7.14.0

11+- [#3309](https://github.com/oauth2-proxy/oauth2-proxy/pull/3309) fix: Return 302 redirect from AuthOnly endpoint when skip-provider-button is true (@StefanMarkmann)

12+

11# V7.14.0

13# V7.14.0

13## Release Highlights

15## Release Highlights

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

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

215| 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 | |

216| 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 |

219| 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. | |

220| 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+9−1

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

987// and optional authorization).

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

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

990+ // If SkipProviderButton is enabled and user needs login, redirect directly

991+ // to OAuth provider instead of returning 401. This allows nginx auth_request

992+ // to pass through the 302 redirect to the browser, bypassing error_page

993+ // handling which can break redirect flows.

994+ // See: https://github.com/oauth2-proxy/oauth2-proxy/issues/334

995+ if p.SkipProviderButton && err == ErrNeedsLogin {

996+ p.doOAuthStart(rw, req, nil)

997+ return

990− if err != nil {

998+ } else if err != nil {

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

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

992 return

1000 return

993 }

1001 }

oauthproxy_test.go+43−2

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

834 return nil, err

835 }

836 testProvider := &TestProvider{

837− ProviderData: &providers.ProviderData{},

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+ },

838− ValidToken: opts.providerValidateCookieResponse,

850+ ValidToken: opts.providerValidateCookieResponse,

839 }

851 }

840

852

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

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

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

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

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

1113}

1125}

1114

1126

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+

1115func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) {

1156func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) {

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

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

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

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