diff --git a/CHANGELOG.md b/CHANGELOG.md index 72d570f3..c9b7911b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ ## Changes since v6.1.1 +- [#916](https://github.com/oauth2-proxy/oauth2-rpoxy/pull/916) Add AlphaOptions struct to prepare for alpha config loading (@JoelSpeed) - [#923](https://github.com/oauth2-proxy/oauth2-proxy/pull/923) Support TLS 1.3 (@aajisaka) - [#918](https://github.com/oauth2-proxy/oauth2-proxy/pull/918) Fix log header output (@JoelSpeed) - [#911](https://github.com/oauth2-proxy/oauth2-proxy/pull/911) Validate provider type on startup. diff --git a/pkg/apis/options/alpha_options.go b/pkg/apis/options/alpha_options.go new file mode 100644 index 00000000..1086ee2a --- /dev/null +++ b/pkg/apis/options/alpha_options.go @@ -0,0 +1,31 @@ +package options + +// AlphaOptions contains alpha structured configuration options. +// Usage of these options allows users to access alpha features that are not +// available as part of the primary configuration structure for OAuth2 Proxy. +// +// :::warning +// The options within this structure are considered alpha. +// They may change between releases without notice. +// ::: +type AlphaOptions struct { + // Upstreams is used to configure upstream servers. + // Once a user is authenticated, requests to the server will be proxied to + // these upstream servers based on the path mappings defined in this list. + Upstreams Upstreams `json:"upstreams,omitempty"` + + // InjectRequestHeaders is used to configure headers that should be added + // to requests to upstream servers. + // Headers may source values from either the authenticated user's session + // or from a static secret value. + InjectRequestHeaders []Header `json:"injectRequestHeaders,omitempty"` + + // InjectResponseHeaders is used to configure headers that should be added + // to responses from the proxy. + // This is typically used when using the proxy as an external authentication + // provider in conjunction with another proxy such as NGINX and its + // auth_request module. + // Headers may source values from either the authenticated user's session + // or from a static secret value. + InjectResponseHeaders []Header `json:"injectResponseHeaders,omitempty"` +} diff --git a/pkg/apis/options/common.go b/pkg/apis/options/common.go index 60d352a5..b08bfa6d 100644 --- a/pkg/apis/options/common.go +++ b/pkg/apis/options/common.go @@ -1,14 +1,62 @@ package options +import ( + "fmt" + "strconv" + "time" +) + // SecretSource references an individual secret value. // Only one source within the struct should be defined at any time. type SecretSource struct { // Value expects a base64 encoded string value. - Value []byte + Value []byte `json:"value,omitempty"` // FromEnv expects the name of an environment variable. - FromEnv string + FromEnv string `json:"fromEnv,omitempty"` // FromFile expects a path to a file containing the secret value. - FromFile string + FromFile string `json:"fromFile,omitempty"` +} + +// Duration is an alias for time.Duration so that we can ensure the marshalling +// and unmarshalling of string durations is done as users expect. +// Intentional blank line below to keep this first part of the comment out of +// any generated references. + +// Duration is as string representation of a period of time. +// A duration string is a is a possibly signed sequence of decimal numbers, +// each with optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". +// Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". +type Duration time.Duration + +// UnmarshalJSON parses the duration string and sets the value of duration +// to the value of the duration string. +func (d *Duration) UnmarshalJSON(data []byte) error { + input := string(data) + if unquoted, err := strconv.Unquote(input); err == nil { + input = unquoted + } + + du, err := time.ParseDuration(input) + if err != nil { + return err + } + *d = Duration(du) + return nil +} + +// MarshalJSON ensures that when the string is marshalled to JSON as a human +// readable string. +func (d *Duration) MarshalJSON() ([]byte, error) { + dStr := fmt.Sprintf("%q", d.Duration().String()) + return []byte(dStr), nil +} + +// Duration returns the time.Duration version of this Duration +func (d *Duration) Duration() time.Duration { + if d == nil { + return time.Duration(0) + } + return time.Duration(*d) } diff --git a/pkg/apis/options/common_test.go b/pkg/apis/options/common_test.go new file mode 100644 index 00000000..8fc4176b --- /dev/null +++ b/pkg/apis/options/common_test.go @@ -0,0 +1,88 @@ +package options + +import ( + "encoding/json" + "errors" + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" +) + +var _ = Describe("Common", func() { + Context("Duration", func() { + type marshalJSONTableInput struct { + duration Duration + expectedJSON string + } + + DescribeTable("MarshalJSON", + func(in marshalJSONTableInput) { + data, err := in.duration.MarshalJSON() + Expect(err).ToNot(HaveOccurred()) + Expect(string(data)).To(Equal(in.expectedJSON)) + + var d Duration + Expect(json.Unmarshal(data, &d)).To(Succeed()) + Expect(d).To(Equal(in.duration)) + }, + Entry("30 seconds", marshalJSONTableInput{ + duration: Duration(30 * time.Second), + expectedJSON: "\"30s\"", + }), + Entry("1 minute", marshalJSONTableInput{ + duration: Duration(1 * time.Minute), + expectedJSON: "\"1m0s\"", + }), + Entry("1 hour 15 minutes", marshalJSONTableInput{ + duration: Duration(75 * time.Minute), + expectedJSON: "\"1h15m0s\"", + }), + Entry("A zero Duration", marshalJSONTableInput{ + duration: Duration(0), + expectedJSON: "\"0s\"", + }), + ) + + type unmarshalJSONTableInput struct { + json string + expectedErr error + expectedDuration Duration + } + + DescribeTable("UnmarshalJSON", + func(in unmarshalJSONTableInput) { + // A duration must be initialised pointer before UnmarshalJSON will work. + zero := Duration(0) + d := &zero + + err := d.UnmarshalJSON([]byte(in.json)) + if in.expectedErr != nil { + Expect(err).To(MatchError(in.expectedErr.Error())) + } else { + Expect(err).ToNot(HaveOccurred()) + } + Expect(d).ToNot(BeNil()) + Expect(*d).To(Equal(in.expectedDuration)) + }, + Entry("1m", unmarshalJSONTableInput{ + json: "\"1m\"", + expectedDuration: Duration(1 * time.Minute), + }), + Entry("30s", unmarshalJSONTableInput{ + json: "\"30s\"", + expectedDuration: Duration(30 * time.Second), + }), + Entry("1h15m", unmarshalJSONTableInput{ + json: "\"1h15m\"", + expectedDuration: Duration(75 * time.Minute), + }), + Entry("am", unmarshalJSONTableInput{ + json: "\"am\"", + expectedErr: errors.New("time: invalid duration \"am\""), + expectedDuration: Duration(0), + }), + ) + }) +}) diff --git a/pkg/apis/options/header.go b/pkg/apis/options/header.go index 0b2e1b69..4b41eff9 100644 --- a/pkg/apis/options/header.go +++ b/pkg/apis/options/header.go @@ -5,26 +5,26 @@ package options type Header struct { // Name is the header name to be used for this set of values. // Names should be unique within a list of Headers. - Name string `json:"name"` + Name string `json:"name,omitempty"` // PreserveRequestValue determines whether any values for this header // should be preserved for the request to the upstream server. // This option only takes effet on injected request headers. // Defaults to false (headers that match this header will be stripped). - PreserveRequestValue bool `json:"preserveRequestValue"` + PreserveRequestValue bool `json:"preserveRequestValue,omitempty"` // Values contains the desired values for this header - Values []HeaderValue `json:"values"` + Values []HeaderValue `json:"values,omitempty"` } // HeaderValue represents a single header value and the sources that can // make up the header value type HeaderValue struct { // Allow users to load the value from a secret source - *SecretSource + *SecretSource `json:",omitempty"` // Allow users to load the value from a session claim - *ClaimSource + *ClaimSource `json:",omitempty"` } // ClaimSource allows loading a header value from a claim within the session @@ -40,5 +40,5 @@ type ClaimSource struct { // BasicAuthPassword converts this claim into a basic auth header. // Note the value of claim will become the basic auth username and the // basicAuthPassword will be used as the password value. - BasicAuthPassword *SecretSource + BasicAuthPassword *SecretSource `json:"basicAuthPassword,omitempty"` } diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index 88784e9b..6fb2596b 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -27,7 +27,7 @@ func NewLegacyOptions() *LegacyOptions { LegacyUpstreams: LegacyUpstreams{ PassHostHeader: true, ProxyWebSockets: true, - FlushInterval: time.Duration(1) * time.Second, + FlushInterval: DefaultUpstreamFlushInterval, }, LegacyHeaders: LegacyHeaders{ @@ -62,7 +62,7 @@ type LegacyUpstreams struct { func legacyUpstreamsFlagSet() *pflag.FlagSet { flagSet := pflag.NewFlagSet("upstreams", pflag.ExitOnError) - flagSet.Duration("flush-interval", time.Duration(1)*time.Second, "period between response flushing when streaming responses") + flagSet.Duration("flush-interval", DefaultUpstreamFlushInterval, "period between response flushing when streaming responses") flagSet.Bool("pass-host-header", true, "pass the request Host Header to upstream") 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") @@ -84,6 +84,7 @@ func (l *LegacyUpstreams) convert() (Upstreams, error) { u.Path = "/" } + flushInterval := Duration(l.FlushInterval) upstream := Upstream{ ID: u.Path, Path: u.Path, @@ -91,7 +92,7 @@ func (l *LegacyUpstreams) convert() (Upstreams, error) { InsecureSkipTLSVerify: l.SSLUpstreamInsecureSkipVerify, PassHostHeader: &l.PassHostHeader, ProxyWebSockets: &l.ProxyWebSockets, - FlushInterval: &l.FlushInterval, + FlushInterval: &flushInterval, } switch u.Scheme { diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index 2e50edcc..44c8c728 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -17,8 +17,8 @@ var _ = Describe("Legacy Options", func() { legacyOpts := NewLegacyOptions() // Set upstreams and related options to test their conversion - flushInterval := 5 * time.Second - legacyOpts.LegacyUpstreams.FlushInterval = flushInterval + flushInterval := Duration(5 * time.Second) + legacyOpts.LegacyUpstreams.FlushInterval = time.Duration(flushInterval) legacyOpts.LegacyUpstreams.PassHostHeader = true legacyOpts.LegacyUpstreams.ProxyWebSockets = true legacyOpts.LegacyUpstreams.SSLUpstreamInsecureSkipVerify = true @@ -124,7 +124,7 @@ var _ = Describe("Legacy Options", func() { skipVerify := true passHostHeader := false proxyWebSockets := true - flushInterval := 5 * time.Second + flushInterval := Duration(5 * time.Second) // Test cases and expected outcomes validHTTP := "http://foo.bar/baz" @@ -199,7 +199,7 @@ var _ = Describe("Legacy Options", func() { SSLUpstreamInsecureSkipVerify: skipVerify, PassHostHeader: passHostHeader, ProxyWebSockets: proxyWebSockets, - FlushInterval: flushInterval, + FlushInterval: time.Duration(flushInterval), } upstreams, err := legacyUpstreams.convert() diff --git a/pkg/apis/options/upstreams.go b/pkg/apis/options/upstreams.go index e879a107..6ae87487 100644 --- a/pkg/apis/options/upstreams.go +++ b/pkg/apis/options/upstreams.go @@ -2,6 +2,11 @@ package options import "time" +const ( + // DefaultUpstreamFlushInterval is the default value for the Upstream FlushInterval. + DefaultUpstreamFlushInterval = 1 * time.Second +) + // Upstreams is a collection of definitions for upstream servers. type Upstreams []Upstream @@ -10,11 +15,11 @@ type Upstreams []Upstream type Upstream struct { // ID should be a unique identifier for the upstream. // This value is required for all upstreams. - ID string `json:"id"` + ID string `json:"id,omitempty"` // Path is used to map requests to the upstream server. // The closest match will take precedence and all Paths must be unique. - Path string `json:"path"` + Path string `json:"path,omitempty"` // The URI of the upstream server. This may be an HTTP(S) server of a File // based URL. It may include a path, in which case all requests will be served @@ -26,19 +31,19 @@ type Upstream struct { // - file://host/path // If the URI's path is "/base" and the incoming request was for "/dir", // the upstream request will be for "/base/dir". - URI string `json:"uri"` + URI string `json:"uri,omitempty"` // InsecureSkipTLSVerify will skip TLS verification of upstream HTTPS hosts. // This option is insecure and will allow potential Man-In-The-Middle attacks // betweem OAuth2 Proxy and the usptream server. // Defaults to false. - InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify"` + InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty"` // Static will make all requests to this upstream have a static response. // The response will have a body of "Authenticated" and a response code // matching StaticCode. // If StaticCode is not set, the response will return a 200 response. - Static bool `json:"static"` + Static bool `json:"static,omitempty"` // StaticCode determines the response code for the Static response. // This option can only be used with Static enabled. @@ -47,14 +52,14 @@ type Upstream struct { // FlushInterval is the period between flushing the response buffer when // streaming response from the upstream. // Defaults to 1 second. - FlushInterval *time.Duration `json:"flushInterval,omitempty"` + FlushInterval *Duration `json:"flushInterval,omitempty"` // PassHostHeader determines whether the request host header should be proxied // to the upstream server. // Defaults to true. - PassHostHeader *bool `json:"passHostHeader"` + PassHostHeader *bool `json:"passHostHeader,omitempty"` // ProxyWebSockets enables proxying of websockets to upstream servers // Defaults to true. - ProxyWebSockets *bool `json:"proxyWebSockets"` + ProxyWebSockets *bool `json:"proxyWebSockets,omitempty"` } diff --git a/pkg/upstream/http.go b/pkg/upstream/http.go index 88c0afcd..a6e948c3 100644 --- a/pkg/upstream/http.go +++ b/pkg/upstream/http.go @@ -6,7 +6,6 @@ import ( "net/http/httputil" "net/url" "strings" - "time" "github.com/mbland/hmacauth" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" @@ -98,9 +97,9 @@ func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler Pr // Configure options on the SingleHostReverseProxy if upstream.FlushInterval != nil { - proxy.FlushInterval = *upstream.FlushInterval + proxy.FlushInterval = upstream.FlushInterval.Duration() } else { - proxy.FlushInterval = 1 * time.Second + proxy.FlushInterval = options.DefaultUpstreamFlushInterval } // InsecureSkipVerify is a configurable option we allow diff --git a/pkg/upstream/http_test.go b/pkg/upstream/http_test.go index 8bfe9087..3ce5bd19 100644 --- a/pkg/upstream/http_test.go +++ b/pkg/upstream/http_test.go @@ -22,8 +22,8 @@ import ( var _ = Describe("HTTP Upstream Suite", func() { - const flushInterval5s = 5 * time.Second - const flushInterval1s = 1 * time.Second + const flushInterval5s = options.Duration(5 * time.Second) + const flushInterval1s = options.Duration(1 * time.Second) truth := true falsum := false @@ -52,7 +52,7 @@ var _ = Describe("HTTP Upstream Suite", func() { rw := httptest.NewRecorder() - flush := 1 * time.Second + flush := options.Duration(1 * time.Second) upstream := options.Upstream{ ID: in.id, @@ -258,7 +258,7 @@ var _ = Describe("HTTP Upstream Suite", func() { req := httptest.NewRequest("", "http://example.localhost/foo", nil) rw := httptest.NewRecorder() - flush := 1 * time.Second + flush := options.Duration(1 * time.Second) upstream := options.Upstream{ ID: "noPassHost", PassHostHeader: &falsum, @@ -290,7 +290,7 @@ var _ = Describe("HTTP Upstream Suite", func() { type newUpstreamTableInput struct { proxyWebSockets bool - flushInterval time.Duration + flushInterval options.Duration skipVerify bool sigData *options.SignatureData errorHandler func(http.ResponseWriter, *http.Request, error) @@ -319,7 +319,7 @@ var _ = Describe("HTTP Upstream Suite", func() { proxy, ok := upstreamProxy.handler.(*httputil.ReverseProxy) Expect(ok).To(BeTrue()) - Expect(proxy.FlushInterval).To(Equal(in.flushInterval)) + Expect(proxy.FlushInterval).To(Equal(in.flushInterval.Duration())) Expect(proxy.ErrorHandler != nil).To(Equal(in.errorHandler != nil)) if in.skipVerify { Expect(proxy.Transport).To(Equal(&http.Transport{ @@ -370,7 +370,7 @@ var _ = Describe("HTTP Upstream Suite", func() { var proxyServer *httptest.Server BeforeEach(func() { - flush := 1 * time.Second + flush := options.Duration(1 * time.Second) upstream := options.Upstream{ ID: "websocketProxy", PassHostHeader: &truth, diff --git a/pkg/validation/upstreams.go b/pkg/validation/upstreams.go index 5cfe0b1e..7bd6b2d5 100644 --- a/pkg/validation/upstreams.go +++ b/pkg/validation/upstreams.go @@ -3,7 +3,6 @@ package validation import ( "fmt" "net/url" - "time" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" ) @@ -70,7 +69,7 @@ func validateStaticUpstream(upstream options.Upstream) []string { if upstream.InsecureSkipTLSVerify { msgs = append(msgs, fmt.Sprintf("upstream %q has insecureSkipTLSVerify, but is a static upstream, this will have no effect.", upstream.ID)) } - if upstream.FlushInterval != nil && *upstream.FlushInterval != time.Second { + if upstream.FlushInterval != nil && upstream.FlushInterval.Duration() != options.DefaultUpstreamFlushInterval { msgs = append(msgs, fmt.Sprintf("upstream %q has flushInterval, but is a static upstream, this will have no effect.", upstream.ID)) } if upstream.PassHostHeader != nil { diff --git a/pkg/validation/upstreams_test.go b/pkg/validation/upstreams_test.go index 6b8f9829..122286ad 100644 --- a/pkg/validation/upstreams_test.go +++ b/pkg/validation/upstreams_test.go @@ -15,7 +15,7 @@ var _ = Describe("Upstreams", func() { errStrings []string } - flushInterval := 5 * time.Second + flushInterval := options.Duration(5 * time.Second) staticCode200 := 200 truth := true