diff --git a/CHANGELOG.md b/CHANGELOG.md index 11b9e829b3..2bec1cf7f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ If you are using an architecture specific tag (ex: v7.2.1-arm64) you should move - [#1286](https://github.com/oauth2-proxy/oauth2-proxy/pull/1286) Add the `allowed_email_domains` and the `allowed_groups` on the `auth_request` + support standard wildcard char for validation with sub-domain and email-domain. (@w3st3ry @armandpicard) - [#1361](https://github.com/oauth2-proxy/oauth2-proxy/pull/1541) PKCE Code Challenge Support - RFC-7636 (@braunsonm) - [#1594](https://github.com/oauth2-proxy/oauth2-proxy/pull/1594) Release ARMv8 docker images (@braunsonm) +- [#1638](https://github.com/oauth2-proxy/oauth2-proxy/pull/1638) Implement configurable upstream timeout (@jacksgt) # V7.2.1 diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index cc9f5ae43f..f0e9d18213 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -512,6 +512,7 @@ Requests will be proxied to this upstream if the path matches the request path. | `flushInterval` | _[Duration](#duration)_ | FlushInterval is the period between flushing the response buffer when
streaming response from the upstream.
Defaults to 1 second. | | `passHostHeader` | _bool_ | PassHostHeader determines whether the request host header should be proxied
to the upstream server.
Defaults to true. | | `proxyWebSockets` | _bool_ | ProxyWebSockets enables proxying of websockets to upstream servers
Defaults to true. | +| `timeout` | _[Duration](#duration)_ | Timeout is the maximum duration the server will wait for a response from the upstream server.
Defaults to 30 seconds. | ### UpstreamConfig diff --git a/docs/docs/configuration/overview.md b/docs/docs/configuration/overview.md index 0cd01e5859..85d3cad539 100644 --- a/docs/docs/configuration/overview.md +++ b/docs/docs/configuration/overview.md @@ -196,6 +196,7 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/ | `--tls-key-file` | string | path to private key file | | | `--tls-min-version` | string | minimum TLS version that is acceptable, either `"TLS1.2"` or `"TLS1.3"` | `"TLS1.2"` | | `--upstream` | string \| list | the http url(s) of the upstream endpoint, file:// paths for static files or `static://` for static response. Routing is based on the path | | +| `--upstream-timeout` | duration | maximum amount of time the server will wait for a response from the upstream | 30s | | `--allowed-group` | string \| list | restrict logins to members of this group (may be given multiple times) | | | `--allowed-role` | string \| list | restrict logins to users with this role (may be given multiple times). Only works with the keycloak-oidc provider. | | | `--validate-url` | string | Access token validation endpoint | | diff --git a/main_test.go b/main_test.go index dd995a68f4..2ed44cb078 100644 --- a/main_test.go +++ b/main_test.go @@ -35,6 +35,7 @@ upstreamConfig: flushInterval: 1s passHostHeader: true proxyWebSockets: true + timeout: 30s injectRequestHeaders: - name: Authorization values: @@ -118,6 +119,7 @@ redirect_url="http://localhost:4180/oauth2/callback" FlushInterval: durationPtr(options.DefaultUpstreamFlushInterval), PassHostHeader: boolPtr(true), ProxyWebSockets: boolPtr(true), + Timeout: durationPtr(options.DefaultUpstreamTimeout), }, }, } diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index 89e3230a8d..16297e003c 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -33,6 +33,7 @@ func NewLegacyOptions() *LegacyOptions { PassHostHeader: true, ProxyWebSockets: true, FlushInterval: DefaultUpstreamFlushInterval, + Timeout: DefaultUpstreamTimeout, }, LegacyHeaders: LegacyHeaders{ @@ -101,6 +102,7 @@ type LegacyUpstreams struct { ProxyWebSockets bool `flag:"proxy-websockets" cfg:"proxy_websockets"` SSLUpstreamInsecureSkipVerify bool `flag:"ssl-upstream-insecure-skip-verify" cfg:"ssl_upstream_insecure_skip_verify"` Upstreams []string `flag:"upstream" cfg:"upstreams"` + Timeout time.Duration `flag:"upstream-timeout" cfg:"upstream_timeout"` } func legacyUpstreamsFlagSet() *pflag.FlagSet { @@ -111,6 +113,7 @@ func legacyUpstreamsFlagSet() *pflag.FlagSet { flagSet.Bool("proxy-websockets", true, "enables WebSocket proxying") flagSet.Bool("ssl-upstream-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS upstreams") flagSet.StringSlice("upstream", []string{}, "the http url(s) of the upstream endpoint, file:// paths for static files or static:// for static response. Routing is based on the path") + flagSet.Duration("upstream-timeout", DefaultUpstreamTimeout, "maximum amount of time the server will wait for a response from the upstream") return flagSet } @@ -129,6 +132,7 @@ func (l *LegacyUpstreams) convert() (UpstreamConfig, error) { } flushInterval := Duration(l.FlushInterval) + timeout := Duration(l.Timeout) upstream := Upstream{ ID: u.Path, Path: u.Path, @@ -137,6 +141,7 @@ func (l *LegacyUpstreams) convert() (UpstreamConfig, error) { PassHostHeader: &l.PassHostHeader, ProxyWebSockets: &l.ProxyWebSockets, FlushInterval: &flushInterval, + Timeout: &timeout, } switch u.Scheme { @@ -168,6 +173,7 @@ func (l *LegacyUpstreams) convert() (UpstreamConfig, error) { upstream.PassHostHeader = nil upstream.ProxyWebSockets = nil upstream.FlushInterval = nil + upstream.Timeout = nil } upstreams.Upstreams = append(upstreams.Upstreams, upstream) diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index 2e11e329c7..da5545d621 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -17,7 +17,9 @@ var _ = Describe("Legacy Options", func() { // Set upstreams and related options to test their conversion flushInterval := Duration(5 * time.Second) + timeout := Duration(5 * time.Second) legacyOpts.LegacyUpstreams.FlushInterval = time.Duration(flushInterval) + legacyOpts.LegacyUpstreams.Timeout = time.Duration(timeout) legacyOpts.LegacyUpstreams.PassHostHeader = true legacyOpts.LegacyUpstreams.ProxyWebSockets = true legacyOpts.LegacyUpstreams.SSLUpstreamInsecureSkipVerify = true @@ -36,6 +38,7 @@ var _ = Describe("Legacy Options", func() { InsecureSkipTLSVerify: true, PassHostHeader: &truth, ProxyWebSockets: &truth, + Timeout: &timeout, }, { ID: "/bar", @@ -45,6 +48,7 @@ var _ = Describe("Legacy Options", func() { InsecureSkipTLSVerify: true, PassHostHeader: &truth, ProxyWebSockets: &truth, + Timeout: &timeout, }, { ID: "static://204", @@ -56,6 +60,7 @@ var _ = Describe("Legacy Options", func() { InsecureSkipTLSVerify: false, PassHostHeader: nil, ProxyWebSockets: nil, + Timeout: nil, }, }, } @@ -140,6 +145,7 @@ var _ = Describe("Legacy Options", func() { passHostHeader := false proxyWebSockets := true flushInterval := Duration(5 * time.Second) + timeout := Duration(5 * time.Second) // Test cases and expected outcomes validHTTP := "http://foo.bar/baz" @@ -151,6 +157,7 @@ var _ = Describe("Legacy Options", func() { PassHostHeader: &passHostHeader, ProxyWebSockets: &proxyWebSockets, FlushInterval: &flushInterval, + Timeout: &timeout, } // Test cases and expected outcomes @@ -163,6 +170,7 @@ var _ = Describe("Legacy Options", func() { PassHostHeader: &passHostHeader, ProxyWebSockets: &proxyWebSockets, FlushInterval: &flushInterval, + Timeout: &timeout, } validFileWithFragment := "file:///var/lib/website#/bar" @@ -174,6 +182,7 @@ var _ = Describe("Legacy Options", func() { PassHostHeader: &passHostHeader, ProxyWebSockets: &proxyWebSockets, FlushInterval: &flushInterval, + Timeout: &timeout, } validStatic := "static://204" @@ -188,6 +197,7 @@ var _ = Describe("Legacy Options", func() { PassHostHeader: nil, ProxyWebSockets: nil, FlushInterval: nil, + Timeout: nil, } invalidStatic := "static://abc" @@ -202,6 +212,7 @@ var _ = Describe("Legacy Options", func() { PassHostHeader: nil, ProxyWebSockets: nil, FlushInterval: nil, + Timeout: nil, } invalidHTTP := ":foo" @@ -215,6 +226,7 @@ var _ = Describe("Legacy Options", func() { PassHostHeader: passHostHeader, ProxyWebSockets: proxyWebSockets, FlushInterval: time.Duration(flushInterval), + Timeout: time.Duration(timeout), } upstreams, err := legacyUpstreams.convert() diff --git a/pkg/apis/options/load_test.go b/pkg/apis/options/load_test.go index 6b529e7b1f..57efd9260e 100644 --- a/pkg/apis/options/load_test.go +++ b/pkg/apis/options/load_test.go @@ -22,6 +22,7 @@ var _ = Describe("Load", func() { PassHostHeader: true, ProxyWebSockets: true, FlushInterval: DefaultUpstreamFlushInterval, + Timeout: DefaultUpstreamTimeout, }, LegacyHeaders: LegacyHeaders{ diff --git a/pkg/apis/options/upstreams.go b/pkg/apis/options/upstreams.go index 9d735f1304..0042a61a5d 100644 --- a/pkg/apis/options/upstreams.go +++ b/pkg/apis/options/upstreams.go @@ -5,6 +5,9 @@ import "time" const ( // DefaultUpstreamFlushInterval is the default value for the Upstream FlushInterval. DefaultUpstreamFlushInterval = 1 * time.Second + + // DefaultUpstreamTimeout is the maximum duration a network dial to a upstream server for a response. + DefaultUpstreamTimeout = 30 * time.Second ) // UpstreamConfig is a collection of definitions for upstream servers. @@ -84,4 +87,8 @@ type Upstream struct { // ProxyWebSockets enables proxying of websockets to upstream servers // Defaults to true. ProxyWebSockets *bool `json:"proxyWebSockets,omitempty"` + + // Timeout is the maximum duration the server will wait for a response from the upstream server. + // Defaults to 30 seconds. + Timeout *Duration `json:"timeout,omitempty"` } diff --git a/pkg/upstream/http.go b/pkg/upstream/http.go index 1f9f51b8d4..031368e543 100644 --- a/pkg/upstream/http.go +++ b/pkg/upstream/http.go @@ -1,7 +1,6 @@ package upstream import ( - "crypto/tls" "net/http" "net/http/httputil" "net/url" @@ -100,6 +99,14 @@ func (h *httpUpstreamProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler ProxyErrorHandler) http.Handler { proxy := httputil.NewSingleHostReverseProxy(target) + // Inherit default transport options from Go's stdlib + transport := http.DefaultTransport.(*http.Transport).Clone() + + // Change default duration for waiting for an upstream response + if upstream.Timeout != nil { + transport.ResponseHeaderTimeout = upstream.Timeout.Duration() + } + // Configure options on the SingleHostReverseProxy if upstream.FlushInterval != nil { proxy.FlushInterval = upstream.FlushInterval.Duration() @@ -110,9 +117,7 @@ func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler Pr // InsecureSkipVerify is a configurable option we allow /* #nosec G402 */ if upstream.InsecureSkipTLSVerify { - proxy.Transport = &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - } + transport.TLSClientConfig.InsecureSkipVerify = true } // Ensure we always pass the original request path @@ -127,6 +132,10 @@ func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler Pr if errorHandler != nil { proxy.ErrorHandler = errorHandler } + + // Apply the customized transport to our proxy before returning it + proxy.Transport = transport + return proxy } @@ -156,11 +165,17 @@ func setProxyDirector(proxy *httputil.ReverseProxy) { // newWebSocketReverseProxy creates a new reverse proxy for proxying websocket connections. func newWebSocketReverseProxy(u *url.URL, skipTLSVerify bool) http.Handler { wsProxy := httputil.NewSingleHostReverseProxy(u) + + // Inherit default transport options from Go's stdlib + transport := http.DefaultTransport.(*http.Transport).Clone() + /* #nosec G402 */ if skipTLSVerify { - wsProxy.Transport = &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - } + transport.TLSClientConfig.InsecureSkipVerify = true } + + // Apply the customized transport to our proxy before returning it + wsProxy.Transport = transport + return wsProxy } diff --git a/pkg/upstream/http_test.go b/pkg/upstream/http_test.go index 7477ac3b09..23753086c3 100644 --- a/pkg/upstream/http_test.go +++ b/pkg/upstream/http_test.go @@ -3,7 +3,6 @@ package upstream import ( "bytes" "crypto" - "crypto/tls" "encoding/json" "fmt" "net/http" @@ -23,9 +22,8 @@ import ( ) var _ = Describe("HTTP Upstream Suite", func() { - - const flushInterval5s = options.Duration(5 * time.Second) - const flushInterval1s = options.Duration(1 * time.Second) + defaultFlushInterval := options.Duration(options.DefaultUpstreamFlushInterval) + defaultTimeout := options.Duration(options.DefaultUpstreamTimeout) truth := true falsum := false @@ -62,12 +60,15 @@ var _ = Describe("HTTP Upstream Suite", func() { flush := options.Duration(1 * time.Second) + timeout := options.Duration(options.DefaultUpstreamTimeout) + upstream := options.Upstream{ ID: in.id, PassHostHeader: &in.passUpstreamHostHeader, ProxyWebSockets: &falsum, InsecureSkipTLSVerify: false, FlushInterval: &flush, + Timeout: &timeout, } Expect(in.serverAddr).ToNot(BeNil()) @@ -318,13 +319,13 @@ var _ = Describe("HTTP Upstream Suite", func() { req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{}) rw := httptest.NewRecorder() - flush := options.Duration(1 * time.Second) upstream := options.Upstream{ ID: "noPassHost", PassHostHeader: &falsum, ProxyWebSockets: &falsum, InsecureSkipTLSVerify: false, - FlushInterval: &flush, + FlushInterval: &defaultFlushInterval, + Timeout: &defaultTimeout, } u, err := url.Parse(serverAddr) @@ -354,6 +355,7 @@ var _ = Describe("HTTP Upstream Suite", func() { skipVerify bool sigData *options.SignatureData errorHandler func(http.ResponseWriter, *http.Request, error) + timeout options.Duration } DescribeTable("newHTTPUpstreamProxy", @@ -366,6 +368,7 @@ var _ = Describe("HTTP Upstream Suite", func() { FlushInterval: &in.flushInterval, InsecureSkipTLSVerify: in.skipVerify, ProxyWebSockets: &in.proxyWebSockets, + Timeout: &in.timeout, } handler := newHTTPUpstreamProxy(upstream, u, in.sigData, in.errorHandler) @@ -380,49 +383,63 @@ var _ = Describe("HTTP Upstream Suite", func() { proxy, ok := upstreamProxy.handler.(*httputil.ReverseProxy) Expect(ok).To(BeTrue()) Expect(proxy.FlushInterval).To(Equal(in.flushInterval.Duration())) + transport, ok := proxy.Transport.(*http.Transport) + Expect(ok).To(BeTrue()) + Expect(transport.ResponseHeaderTimeout).To(Equal(in.timeout.Duration())) Expect(proxy.ErrorHandler != nil).To(Equal(in.errorHandler != nil)) if in.skipVerify { - Expect(proxy.Transport).To(Equal(&http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - })) + Expect(transport.TLSClientConfig.InsecureSkipVerify).To(Equal(true)) } }, Entry("with proxy websockets", &newUpstreamTableInput{ proxyWebSockets: true, - flushInterval: flushInterval1s, + flushInterval: defaultFlushInterval, skipVerify: false, sigData: nil, errorHandler: nil, + timeout: defaultTimeout, }), Entry("with a non standard flush interval", &newUpstreamTableInput{ proxyWebSockets: false, - flushInterval: flushInterval5s, + flushInterval: options.Duration(5 * time.Second), skipVerify: false, sigData: nil, errorHandler: nil, + timeout: defaultTimeout, }), Entry("with a InsecureSkipTLSVerify", &newUpstreamTableInput{ proxyWebSockets: false, - flushInterval: flushInterval1s, + flushInterval: defaultFlushInterval, skipVerify: true, sigData: nil, errorHandler: nil, + timeout: defaultTimeout, }), Entry("with a SignatureData", &newUpstreamTableInput{ proxyWebSockets: false, - flushInterval: flushInterval1s, + flushInterval: defaultFlushInterval, skipVerify: false, sigData: &options.SignatureData{Hash: crypto.SHA256, Key: "secret"}, errorHandler: nil, + timeout: defaultTimeout, }), Entry("with an error handler", &newUpstreamTableInput{ proxyWebSockets: false, - flushInterval: flushInterval1s, + flushInterval: defaultFlushInterval, skipVerify: false, sigData: nil, errorHandler: func(rw http.ResponseWriter, req *http.Request, arg3 error) { rw.WriteHeader(502) }, + timeout: defaultTimeout, + }), + Entry("with a non-default timeout", &newUpstreamTableInput{ + proxyWebSockets: false, + flushInterval: defaultFlushInterval, + skipVerify: false, + sigData: nil, + errorHandler: nil, + timeout: options.Duration(5 * time.Second), }), ) @@ -431,12 +448,14 @@ var _ = Describe("HTTP Upstream Suite", func() { BeforeEach(func() { flush := options.Duration(1 * time.Second) + timeout := options.Duration(options.DefaultUpstreamTimeout) upstream := options.Upstream{ ID: "websocketProxy", PassHostHeader: &truth, ProxyWebSockets: &truth, InsecureSkipTLSVerify: false, FlushInterval: &flush, + Timeout: &timeout, } u, err := url.Parse(serverAddr)