diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go new file mode 100644 index 00000000..b0198c9c --- /dev/null +++ b/pkg/apis/options/legacy_options.go @@ -0,0 +1,98 @@ +package options + +import ( + "fmt" + "net/url" + "strconv" + "time" + + "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" +) + +type LegacyOptions struct { + // Legacy options related to upstream servers + LegacyFlushInterval time.Duration `flag:"flush-interval" cfg:"flush_interval"` + LegacyPassHostHeader bool `flag:"pass-host-header" cfg:"pass_host_header"` + LegacyProxyWebSockets bool `flag:"proxy-websockets" cfg:"proxy_websockets"` + LegacySSLUpstreamInsecureSkipVerify bool `flag:"ssl-upstream-insecure-skip-verify" cfg:"ssl_upstream_insecure_skip_verify"` + LegacyUpstreams []string `flag:"upstream" cfg:"upstreams"` + + Options Options `cfg:",squash"` +} + +func NewLegacyOptions() *LegacyOptions { + return &LegacyOptions{ + LegacyPassHostHeader: true, + LegacyProxyWebSockets: true, + LegacyFlushInterval: time.Duration(1) * time.Second, + + Options: *NewOptions(), + } +} + +func (l *LegacyOptions) ToOptions() (*Options, error) { + upstreams, err := convertLegacyUpstreams(l.LegacyUpstreams, l.LegacySSLUpstreamInsecureSkipVerify, l.LegacyPassHostHeader, l.LegacyProxyWebSockets, l.LegacyFlushInterval) + if err != nil { + return nil, fmt.Errorf("error converting upstreams: %v", err) + } + l.Options.UpstreamServers = upstreams + + return &l.Options, nil +} + +func convertLegacyUpstreams(upstreamStrings []string, skipVerify, passHostHeader, proxyWebSockets bool, flushInterval time.Duration) (Upstreams, error) { + upstreams := Upstreams{} + + for _, upstreamString := range upstreamStrings { + u, err := url.Parse(upstreamString) + if err != nil { + return nil, fmt.Errorf("could not parse upstream %q: %v", upstreamString, err) + } + + if u.Path == "" { + u.Path = "/" + } + + upstream := Upstream{ + ID: u.Path, + Path: u.Path, + URI: upstreamString, + InsecureSkipTLSVerify: skipVerify, + PassHostHeader: passHostHeader, + ProxyWebSockets: proxyWebSockets, + FlushInterval: &flushInterval, + } + + switch u.Scheme { + case "file": + if u.Fragment != "" { + upstream.ID = u.Fragment + upstream.Path = u.Fragment + } + case "static": + responseCode, err := strconv.Atoi(u.Host) + if err != nil { + logger.Printf("unable to convert %q to int, use default \"200\"", u.Host) + responseCode = 200 + } + upstream.Static = true + upstream.StaticCode = &responseCode + + // These are not allowed to be empty and must be unique + upstream.ID = upstreamString + upstream.Path = upstreamString + + // Force defaults compatible with static responses + upstream.URI = "" + upstream.InsecureSkipTLSVerify = false + upstream.PassHostHeader = true + upstream.ProxyWebSockets = false + flush := 1 * time.Second + upstream.FlushInterval = &flush + } + + upstreams = append(upstreams, upstream) + } + + return upstreams, nil +} diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go new file mode 100644 index 00000000..0e221749 --- /dev/null +++ b/pkg/apis/options/legacy_options_test.go @@ -0,0 +1,194 @@ +package options + +import ( + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" +) + +var _ = Describe("Legacy Options", func() { + Context("ToOptions", func() { + It("converts the options as expected", func() { + opts := NewOptions() + + legacyOpts := NewLegacyOptions() + + // Set upstreams and related options to test their conversion + flushInterval := 5 * time.Second + legacyOpts.LegacyFlushInterval = flushInterval + legacyOpts.LegacyPassHostHeader = true + legacyOpts.LegacyProxyWebSockets = true + legacyOpts.LegacySSLUpstreamInsecureSkipVerify = true + legacyOpts.LegacyUpstreams = []string{"http://foo.bar/baz", "file://var/lib/website#/bar"} + + opts.UpstreamServers = Upstreams{ + { + ID: "/baz", + Path: "/baz", + URI: "http://foo.bar/baz", + FlushInterval: &flushInterval, + InsecureSkipTLSVerify: true, + PassHostHeader: true, + ProxyWebSockets: true, + }, + { + ID: "/bar", + Path: "/bar", + URI: "file://var/lib/website#/bar", + FlushInterval: &flushInterval, + InsecureSkipTLSVerify: true, + PassHostHeader: true, + ProxyWebSockets: true, + }, + } + + converted, err := legacyOpts.ToOptions() + Expect(err).ToNot(HaveOccurred()) + Expect(converted).To(Equal(opts)) + }) + }) + + Context("Legacy Upstreams", func() { + type convertUpstreamsTableInput struct { + upstreamStrings []string + expectedUpstreams Upstreams + errMsg string + } + + defaultFlushInterval := 1 * time.Second + + // Non defaults for these options + skipVerify := true + passHostHeader := false + proxyWebSockets := true + flushInterval := 5 * time.Second + + // Test cases and expected outcomes + validHTTP := "http://foo.bar/baz" + validHTTPUpstream := Upstream{ + ID: "/baz", + Path: "/baz", + URI: validHTTP, + InsecureSkipTLSVerify: skipVerify, + PassHostHeader: passHostHeader, + ProxyWebSockets: proxyWebSockets, + FlushInterval: &flushInterval, + } + + // Test cases and expected outcomes + emptyPathHTTP := "http://foo.bar" + emptyPathHTTPUpstream := Upstream{ + ID: "/", + Path: "/", + URI: emptyPathHTTP, + InsecureSkipTLSVerify: skipVerify, + PassHostHeader: passHostHeader, + ProxyWebSockets: proxyWebSockets, + FlushInterval: &flushInterval, + } + + validFileWithFragment := "file://var/lib/website#/bar" + validFileWithFragmentUpstream := Upstream{ + ID: "/bar", + Path: "/bar", + URI: validFileWithFragment, + InsecureSkipTLSVerify: skipVerify, + PassHostHeader: passHostHeader, + ProxyWebSockets: proxyWebSockets, + FlushInterval: &flushInterval, + } + + validStatic := "static://204" + validStaticCode := 204 + validStaticUpstream := Upstream{ + ID: validStatic, + Path: validStatic, + URI: "", + Static: true, + StaticCode: &validStaticCode, + InsecureSkipTLSVerify: false, + PassHostHeader: true, + ProxyWebSockets: false, + FlushInterval: &defaultFlushInterval, + } + + invalidStatic := "static://abc" + invalidStaticCode := 200 + invalidStaticUpstream := Upstream{ + ID: invalidStatic, + Path: invalidStatic, + URI: "", + Static: true, + StaticCode: &invalidStaticCode, + InsecureSkipTLSVerify: false, + PassHostHeader: true, + ProxyWebSockets: false, + FlushInterval: &defaultFlushInterval, + } + + invalidHTTP := ":foo" + invalidHTTPErrMsg := "could not parse upstream \":foo\": parse \":foo\": missing protocol scheme" + + DescribeTable("convertLegacyUpstreams", + func(o *convertUpstreamsTableInput) { + upstreams, err := convertLegacyUpstreams(o.upstreamStrings, skipVerify, passHostHeader, proxyWebSockets, flushInterval) + + if o.errMsg != "" { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(o.errMsg)) + } else { + Expect(err).ToNot(HaveOccurred()) + } + + Expect(upstreams).To(ConsistOf(o.expectedUpstreams)) + }, + Entry("with no upstreams", &convertUpstreamsTableInput{ + upstreamStrings: []string{}, + expectedUpstreams: Upstreams{}, + errMsg: "", + }), + Entry("with a valid HTTP upstream", &convertUpstreamsTableInput{ + upstreamStrings: []string{validHTTP}, + expectedUpstreams: Upstreams{validHTTPUpstream}, + errMsg: "", + }), + Entry("with a HTTP upstream with an empty path", &convertUpstreamsTableInput{ + upstreamStrings: []string{emptyPathHTTP}, + expectedUpstreams: Upstreams{emptyPathHTTPUpstream}, + errMsg: "", + }), + Entry("with a valid File upstream with a fragment", &convertUpstreamsTableInput{ + upstreamStrings: []string{validFileWithFragment}, + expectedUpstreams: Upstreams{validFileWithFragmentUpstream}, + errMsg: "", + }), + Entry("with a valid static upstream", &convertUpstreamsTableInput{ + upstreamStrings: []string{validStatic}, + expectedUpstreams: Upstreams{validStaticUpstream}, + errMsg: "", + }), + Entry("with an invalid static upstream, code is 200", &convertUpstreamsTableInput{ + upstreamStrings: []string{invalidStatic}, + expectedUpstreams: Upstreams{invalidStaticUpstream}, + errMsg: "", + }), + Entry("with an invalid HTTP upstream", &convertUpstreamsTableInput{ + upstreamStrings: []string{invalidHTTP}, + expectedUpstreams: Upstreams{}, + errMsg: invalidHTTPErrMsg, + }), + Entry("with an invalid HTTP upstream and other upstreams", &convertUpstreamsTableInput{ + upstreamStrings: []string{validHTTP, invalidHTTP}, + expectedUpstreams: Upstreams{}, + errMsg: invalidHTTPErrMsg, + }), + Entry("with multiple valid upstreams", &convertUpstreamsTableInput{ + upstreamStrings: []string{validHTTP, validFileWithFragment, validStatic}, + expectedUpstreams: Upstreams{validHTTPUpstream, validFileWithFragmentUpstream, validStaticUpstream}, + errMsg: "", + }), + ) + }) +}) diff --git a/pkg/apis/options/load_test.go b/pkg/apis/options/load_test.go index 0e61d707..9f6f5d4f 100644 --- a/pkg/apis/options/load_test.go +++ b/pkg/apis/options/load_test.go @@ -4,7 +4,6 @@ import ( "fmt" "io/ioutil" "os" - "testing" . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" @@ -12,11 +11,6 @@ import ( "github.com/spf13/pflag" ) -func TestOptionsSuite(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Options Suite") -} - var _ = Describe("Load", func() { Context("with a testOptions structure", func() { type TestOptionSubStruct struct { @@ -300,6 +294,11 @@ var _ = Describe("Load", func() { input: &Options{}, expectedOutput: NewOptions(), }), + Entry("with an empty LegacyOptions struct, should return default values", &testOptionsTableInput{ + flagSet: NewFlagSet, + input: &LegacyOptions{}, + expectedOutput: NewLegacyOptions(), + }), ) }) }) diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index 5e26ea1e..325c263c 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -24,7 +24,6 @@ type Options struct { ProxyPrefix string `flag:"proxy-prefix" cfg:"proxy_prefix"` PingPath string `flag:"ping-path" cfg:"ping_path"` PingUserAgent string `flag:"ping-user-agent" cfg:"ping_user_agent"` - ProxyWebSockets bool `flag:"proxy-websockets" cfg:"proxy_websockets"` HTTPAddress string `flag:"http-address" cfg:"http_address"` HTTPSAddress string `flag:"https-address" cfg:"https_address"` ReverseProxy bool `flag:"reverse-proxy" cfg:"reverse_proxy"` @@ -64,26 +63,26 @@ type Options struct { Session SessionOptions `cfg:",squash"` Logging Logging `cfg:",squash"` - Upstreams []string `flag:"upstream" cfg:"upstreams"` - SkipAuthRegex []string `flag:"skip-auth-regex" cfg:"skip_auth_regex"` - SkipAuthStripHeaders bool `flag:"skip-auth-strip-headers" cfg:"skip_auth_strip_headers"` - SkipJwtBearerTokens bool `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens"` - ExtraJwtIssuers []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers"` - PassBasicAuth bool `flag:"pass-basic-auth" cfg:"pass_basic_auth"` - SetBasicAuth bool `flag:"set-basic-auth" cfg:"set_basic_auth"` - PreferEmailToUser bool `flag:"prefer-email-to-user" cfg:"prefer_email_to_user"` - BasicAuthPassword string `flag:"basic-auth-password" cfg:"basic_auth_password"` - PassAccessToken bool `flag:"pass-access-token" cfg:"pass_access_token"` - PassHostHeader bool `flag:"pass-host-header" cfg:"pass_host_header"` - SkipProviderButton bool `flag:"skip-provider-button" cfg:"skip_provider_button"` - PassUserHeaders bool `flag:"pass-user-headers" cfg:"pass_user_headers"` - SSLInsecureSkipVerify bool `flag:"ssl-insecure-skip-verify" cfg:"ssl_insecure_skip_verify"` - SSLUpstreamInsecureSkipVerify bool `flag:"ssl-upstream-insecure-skip-verify" cfg:"ssl_upstream_insecure_skip_verify"` - SetXAuthRequest bool `flag:"set-xauthrequest" cfg:"set_xauthrequest"` - SetAuthorization bool `flag:"set-authorization-header" cfg:"set_authorization_header"` - PassAuthorization bool `flag:"pass-authorization-header" cfg:"pass_authorization_header"` - SkipAuthPreflight bool `flag:"skip-auth-preflight" cfg:"skip_auth_preflight"` - FlushInterval time.Duration `flag:"flush-interval" cfg:"flush_interval"` + // Not used in the legacy config, name not allowed to match an external key (upstreams) + // TODO(JoelSpeed): Rename when legacy config is removed + UpstreamServers Upstreams `cfg:",internal"` + + SkipAuthRegex []string `flag:"skip-auth-regex" cfg:"skip_auth_regex"` + SkipAuthStripHeaders bool `flag:"skip-auth-strip-headers" cfg:"skip_auth_strip_headers"` + SkipJwtBearerTokens bool `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens"` + ExtraJwtIssuers []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers"` + PassBasicAuth bool `flag:"pass-basic-auth" cfg:"pass_basic_auth"` + SetBasicAuth bool `flag:"set-basic-auth" cfg:"set_basic_auth"` + PreferEmailToUser bool `flag:"prefer-email-to-user" cfg:"prefer_email_to_user"` + BasicAuthPassword string `flag:"basic-auth-password" cfg:"basic_auth_password"` + PassAccessToken bool `flag:"pass-access-token" cfg:"pass_access_token"` + SkipProviderButton bool `flag:"skip-provider-button" cfg:"skip_provider_button"` + PassUserHeaders bool `flag:"pass-user-headers" cfg:"pass_user_headers"` + SSLInsecureSkipVerify bool `flag:"ssl-insecure-skip-verify" cfg:"ssl_insecure_skip_verify"` + SetXAuthRequest bool `flag:"set-xauthrequest" cfg:"set_xauthrequest"` + SetAuthorization bool `flag:"set-authorization-header" cfg:"set_authorization_header"` + PassAuthorization bool `flag:"pass-authorization-header" cfg:"pass_authorization_header"` + SkipAuthPreflight bool `flag:"skip-auth-preflight" cfg:"skip_auth_preflight"` // These options allow for other providers besides Google, with // potential overrides. @@ -114,7 +113,6 @@ type Options struct { // internal values that are set after config validation redirectURL *url.URL - proxyURLs []*url.URL compiledRegex []*regexp.Regexp provider providers.Provider signatureData *SignatureData @@ -125,7 +123,6 @@ type Options struct { // Options for Getting internal values func (o *Options) GetRedirectURL() *url.URL { return o.redirectURL } -func (o *Options) GetProxyURLs() []*url.URL { return o.proxyURLs } func (o *Options) GetCompiledRegex() []*regexp.Regexp { return o.compiledRegex } func (o *Options) GetProvider() providers.Provider { return o.provider } func (o *Options) GetSignatureData() *SignatureData { return o.signatureData } @@ -135,7 +132,6 @@ func (o *Options) GetRealClientIPParser() ipapi.RealClientIPParser { return o.re // Options for Setting internal values func (o *Options) SetRedirectURL(s *url.URL) { o.redirectURL = s } -func (o *Options) SetProxyURLs(s []*url.URL) { o.proxyURLs = s } func (o *Options) SetCompiledRegex(s []*regexp.Regexp) { o.compiledRegex = s } func (o *Options) SetProvider(s providers.Provider) { o.provider = s } func (o *Options) SetSignatureData(s *SignatureData) { o.signatureData = s } @@ -149,7 +145,6 @@ func NewOptions() *Options { ProxyPrefix: "/oauth2", ProviderType: "google", PingPath: "/ping", - ProxyWebSockets: true, HTTPAddress: "127.0.0.1:4180", HTTPSAddress: ":443", RealClientIPHeader: "X-Real-IP", @@ -160,13 +155,10 @@ func NewOptions() *Options { AzureTenant: "common", SetXAuthRequest: false, SkipAuthPreflight: false, - SkipAuthStripHeaders: false, - FlushInterval: time.Duration(1) * time.Second, PassBasicAuth: true, SetBasicAuth: false, PassUserHeaders: true, PassAccessToken: false, - PassHostHeader: true, SetAuthorization: false, PassAuthorization: false, PreferEmailToUser: false, diff --git a/pkg/apis/options/options_suite_test.go b/pkg/apis/options/options_suite_test.go new file mode 100644 index 00000000..a25cbe42 --- /dev/null +++ b/pkg/apis/options/options_suite_test.go @@ -0,0 +1,16 @@ +package options + +import ( + "testing" + + "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestOptionsSuite(t *testing.T) { + logger.SetOutput(GinkgoWriter) + + RegisterFailHandler(Fail) + RunSpecs(t, "Options Suite") +} diff --git a/pkg/validation/options.go b/pkg/validation/options.go index d0f4ba06..21601d5a 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -176,17 +176,7 @@ func Validate(o *options.Options) error { redirectURL, msgs = parseURL(o.RawRedirectURL, "redirect", msgs) o.SetRedirectURL(redirectURL) - for _, u := range o.Upstreams { - upstreamURL, err := url.Parse(u) - if err != nil { - msgs = append(msgs, fmt.Sprintf("error parsing upstream: %s", err)) - } else { - if upstreamURL.Path == "" { - upstreamURL.Path = "/" - } - o.SetProxyURLs(append(o.GetProxyURLs(), upstreamURL)) - } - } + msgs = append(msgs, validateUpstreams(o.UpstreamServers)...) for _, u := range o.SkipAuthRegex { compiledRegex, err := regexp.Compile(u) diff --git a/pkg/validation/options_test.go b/pkg/validation/options_test.go index 15761a28..8c9a892f 100644 --- a/pkg/validation/options_test.go +++ b/pkg/validation/options_test.go @@ -22,7 +22,11 @@ const ( func testOptions() *options.Options { o := options.NewOptions() - o.Upstreams = append(o.Upstreams, "http://127.0.0.1:8080/") + o.UpstreamServers = append(o.UpstreamServers, options.Upstream{ + ID: "upstream", + Path: "/", + URI: "http://127.0.0.1:8080/", + }) o.Cookie.Secret = cookieSecret o.ClientID = clientID o.ClientSecret = clientSecret @@ -140,26 +144,6 @@ func TestRedirectURL(t *testing.T) { assert.Equal(t, expected, o.GetRedirectURL()) } -func TestProxyURLs(t *testing.T) { - o := testOptions() - o.Upstreams = append(o.Upstreams, "http://127.0.0.1:8081") - assert.Equal(t, nil, Validate(o)) - expected := []*url.URL{ - {Scheme: "http", Host: "127.0.0.1:8080", Path: "/"}, - // note the '/' was added - {Scheme: "http", Host: "127.0.0.1:8081", Path: "/"}, - } - assert.Equal(t, expected, o.GetProxyURLs()) -} - -func TestProxyURLsError(t *testing.T) { - o := testOptions() - o.Upstreams = append(o.Upstreams, "127.0.0.1:8081") - err := Validate(o) - assert.NotEqual(t, nil, err) - assert.Contains(t, err.Error(), "error parsing upstream") -} - func TestCompiledRegex(t *testing.T) { o := testOptions() regexps := []string{"/foo/.*", "/ba[rz]/quux"}