From 5b003a5657c7fdc489d2a2b3e05e25424be22fc2 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Thu, 19 Nov 2020 19:58:50 +0000 Subject: [PATCH 1/5] SecretSource.Value should be plain text in memory --- oauthproxy_test.go | 4 ++-- pkg/apis/options/legacy_options.go | 3 +-- pkg/apis/options/legacy_options_test.go | 5 ++--- pkg/apis/options/util/util.go | 5 +---- pkg/apis/options/util/util_test.go | 15 +++------------ pkg/header/injector_test.go | 14 +++++++------- pkg/validation/common.go | 11 +---------- pkg/validation/common_test.go | 11 +---------- pkg/validation/header_test.go | 4 ++-- 9 files changed, 20 insertions(+), 52 deletions(-) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index fe68c90e..866109ca 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -515,7 +515,7 @@ func TestBasicAuthPassword(t *testing.T) { ClaimSource: &options.ClaimSource{ Claim: "email", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte(basicAuthPassword))), + Value: []byte(basicAuthPassword), }, }, }, @@ -1408,7 +1408,7 @@ func TestAuthOnlyEndpointSetBasicAuthTrueRequestHeaders(t *testing.T) { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("This is a secure password"))), + Value: []byte("This is a secure password"), }, }, }, diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index ae45bc7e..35edf680 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -1,7 +1,6 @@ package options import ( - "encoding/base64" "fmt" "net/url" "strconv" @@ -235,7 +234,7 @@ func getBasicAuthHeader(preferEmailToUser bool, basicAuthPassword string) Header Claim: claim, Prefix: "Basic ", BasicAuthPassword: &SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte(basicAuthPassword))), + Value: []byte(basicAuthPassword), }, }, }, diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index 684d7874..dbac5793 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -1,7 +1,6 @@ package options import ( - "encoding/base64" "time" . "github.com/onsi/ginkgo" @@ -332,7 +331,7 @@ var _ = Describe("Legacy Options", func() { Claim: "user", Prefix: "Basic ", BasicAuthPassword: &SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte(basicAuthSecret))), + Value: []byte(basicAuthSecret), }, }, }, @@ -372,7 +371,7 @@ var _ = Describe("Legacy Options", func() { Claim: "email", Prefix: "Basic ", BasicAuthPassword: &SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte(basicAuthSecret))), + Value: []byte(basicAuthSecret), }, }, }, diff --git a/pkg/apis/options/util/util.go b/pkg/apis/options/util/util.go index 918da13a..99988bdc 100644 --- a/pkg/apis/options/util/util.go +++ b/pkg/apis/options/util/util.go @@ -1,7 +1,6 @@ package util import ( - "encoding/base64" "errors" "io/ioutil" "os" @@ -13,9 +12,7 @@ import ( func GetSecretValue(source *options.SecretSource) ([]byte, error) { switch { case len(source.Value) > 0 && source.FromEnv == "" && source.FromFile == "": - value := make([]byte, base64.StdEncoding.DecodedLen(len(source.Value))) - decoded, err := base64.StdEncoding.Decode(value, source.Value) - return value[:decoded], err + return source.Value, nil case len(source.Value) == 0 && source.FromEnv != "" && source.FromFile == "": return []byte(os.Getenv(source.FromEnv)), nil case len(source.Value) == 0 && source.FromEnv == "" && source.FromFile != "": diff --git a/pkg/apis/options/util/util_test.go b/pkg/apis/options/util/util_test.go index 5ca76a04..8ee8cc30 100644 --- a/pkg/apis/options/util/util_test.go +++ b/pkg/apis/options/util/util_test.go @@ -1,7 +1,6 @@ package util import ( - "encoding/base64" "io/ioutil" "os" "path" @@ -31,20 +30,12 @@ var _ = Describe("GetSecretValue", func() { os.RemoveAll(fileDir) }) - It("returns the correct value from base64", func() { - originalValue := []byte("secret-value-1") - b64Value := base64.StdEncoding.EncodeToString((originalValue)) - - // Once encoded, the originalValue could have a decoded length longer than - // its actual length, ensure we trim this. - // This assertion ensures we are testing the triming - Expect(len(originalValue)).To(BeNumerically("<", base64.StdEncoding.DecodedLen(len(b64Value)))) - + It("returns the correct value from the string value", func() { value, err := GetSecretValue(&options.SecretSource{ - Value: []byte(b64Value), + Value: []byte("secret-value-1"), }) Expect(err).ToNot(HaveOccurred()) - Expect(value).To(Equal(originalValue)) + Expect(string(value)).To(Equal("secret-value-1")) }) It("returns the correct value from the environment", func() { diff --git a/pkg/header/injector_test.go b/pkg/header/injector_test.go index af034fd9..63f1e87a 100644 --- a/pkg/header/injector_test.go +++ b/pkg/header/injector_test.go @@ -49,14 +49,14 @@ var _ = Describe("Injector Suite", func() { }, expectedErr: nil, }), - Entry("with a static valued header from base64", newInjectorTableInput{ + Entry("with a static valued header from string", newInjectorTableInput{ headers: []options.Header{ { Name: "Secret", Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("super-secret"))), + Value: []byte("super-secret"), }, }, }, @@ -200,7 +200,7 @@ var _ = Describe("Injector Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), + Value: []byte("basic-password"), }, }, }, @@ -349,7 +349,7 @@ var _ = Describe("Injector Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), + Value: []byte("basic-password"), }, }, }, @@ -380,17 +380,17 @@ var _ = Describe("Injector Suite", func() { Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("major=1"))), + Value: []byte("major=1"), }, }, { SecretSource: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("minor=2"))), + Value: []byte("minor=2"), }, }, { SecretSource: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("patch=3"))), + Value: []byte("patch=3"), }, }, }, diff --git a/pkg/validation/common.go b/pkg/validation/common.go index bc9dba28..b9cb8e6f 100644 --- a/pkg/validation/common.go +++ b/pkg/validation/common.go @@ -1,7 +1,6 @@ package validation import ( - "encoding/base64" "fmt" "os" @@ -13,7 +12,7 @@ const multipleValuesForSecretSource = "multiple values specified for secret sour func validateSecretSource(source options.SecretSource) string { switch { case len(source.Value) > 0 && source.FromEnv == "" && source.FromFile == "": - return validateSecretSourceValue(source.Value) + return "" case len(source.Value) == 0 && source.FromEnv != "" && source.FromFile == "": return validateSecretSourceEnv(source.FromEnv) case len(source.Value) == 0 && source.FromEnv == "" && source.FromFile != "": @@ -23,14 +22,6 @@ func validateSecretSource(source options.SecretSource) string { } } -func validateSecretSourceValue(value []byte) string { - dst := make([]byte, len(value)) - if _, err := base64.StdEncoding.Decode(dst, value); err != nil { - return fmt.Sprintf("error decoding secret value: %v", err) - } - return "" -} - func validateSecretSourceEnv(key string) string { if value := os.Getenv(key); value == "" { return fmt.Sprintf("error loading secret from environent: no value for for key %q", key) diff --git a/pkg/validation/common_test.go b/pkg/validation/common_test.go index bdce5415..10c179b9 100644 --- a/pkg/validation/common_test.go +++ b/pkg/validation/common_test.go @@ -1,7 +1,6 @@ package validation import ( - "encoding/base64" "io/ioutil" "os" @@ -17,7 +16,7 @@ var _ = Describe("Common", func() { var validSecretSourceFile string BeforeEach(func() { - validSecretSourceValue = []byte(base64.StdEncoding.EncodeToString([]byte("This is a secret source value"))) + validSecretSourceValue = []byte("This is a secret source value") Expect(os.Setenv(validSecretSourceEnv, "This is a secret source env")).To(Succeed()) tmp, err := ioutil.TempFile("", "oauth2-proxy-secret-source-test") Expect(err).ToNot(HaveOccurred()) @@ -110,14 +109,6 @@ var _ = Describe("Common", func() { }, expectedMsg: "", }), - Entry("with an invalid Value", validateSecretSourceTableInput{ - source: func() options.SecretSource { - return options.SecretSource{ - Value: []byte("Invalid Base64 Value"), - } - }, - expectedMsg: "error decoding secret value: illegal base64 data at input byte 7", - }), Entry("with an invalid FromEnv", validateSecretSourceTableInput{ source: func() options.SecretSource { return options.SecretSource{ diff --git a/pkg/validation/header_test.go b/pkg/validation/header_test.go index fee4525d..ccc90857 100644 --- a/pkg/validation/header_test.go +++ b/pkg/validation/header_test.go @@ -148,7 +148,7 @@ var _ = Describe("Headers", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte("secret"), + FromEnv: "UNKNOWN_ENV", }, }, }, @@ -157,7 +157,7 @@ var _ = Describe("Headers", func() { validHeader1, }, expectedMsgs: []string{ - "invalid header \"With-Invalid-Basic-Auth\": invalid values: invalid basicAuthPassword: error decoding secret value: illegal base64 data at input byte 4", + "invalid header \"With-Invalid-Basic-Auth\": invalid values: invalid basicAuthPassword: error loading secret from environent: no value for for key \"UNKNOWN_ENV\"", }, }), ) From f36dfbb494e3e28021784a75247d479ad56af47d Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 9 Nov 2020 20:17:43 +0000 Subject: [PATCH 2/5] Introduce alpha configuration loading --- go.mod | 1 + go.sum | 3 + main.go | 96 ++++++++++--- main_suite_test.go | 16 +++ main_test.go | 215 +++++++++++++++++++++++++++++ pkg/apis/options/alpha_options.go | 8 ++ pkg/apis/options/legacy_options.go | 9 ++ pkg/apis/options/load.go | 28 ++++ pkg/apis/options/load_test.go | 193 +++++++++++++++++++++++++- pkg/apis/options/options.go | 2 - 10 files changed, 550 insertions(+), 21 deletions(-) create mode 100644 main_suite_test.go create mode 100644 main_test.go diff --git a/go.mod b/go.mod index ae5dadd5..9769492c 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/dgrijalva/jwt-go v3.2.0+incompatible github.com/frankban/quicktest v1.10.0 // indirect github.com/fsnotify/fsnotify v1.4.9 + github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 github.com/go-redis/redis/v8 v8.2.3 github.com/justinas/alice v1.2.0 github.com/mbland/hmacauth v0.0.0-20170912233209-44256dfd4bfa diff --git a/go.sum b/go.sum index ed64398e..32d59900 100644 --- a/go.sum +++ b/go.sum @@ -64,7 +64,10 @@ github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMo github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4= github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= +github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= +github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 h1:Mn26/9ZMNWSw9C9ERFA1PUxfmGpolnw2v0bKOREu5ew= +github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32/go.mod h1:GIjDIg/heH5DOkXY3YJ/wNhfHsQHoXGjl8G8amsYQ1I= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= diff --git a/main.go b/main.go index 151c8331..173540b3 100644 --- a/main.go +++ b/main.go @@ -12,36 +12,26 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/validation" + "github.com/spf13/pflag" ) func main() { logger.SetFlags(logger.Lshortfile) - flagSet := options.NewFlagSet() - config := flagSet.String("config", "", "path to config file") - showVersion := flagSet.Bool("version", false, "print version string") - - err := flagSet.Parse(os.Args[1:]) - if err != nil { - logger.Printf("ERROR: Failed to parse flags: %v", err) - os.Exit(1) - } + configFlagSet := pflag.NewFlagSet("oauth2-proxy", pflag.ContinueOnError) + config := configFlagSet.String("config", "", "path to config file") + alphaConfig := configFlagSet.String("alpha-config", "", "path to alpha config file (use at your own risk - the structure in this config file may change between minor releases)") + showVersion := configFlagSet.Bool("version", false, "print version string") + configFlagSet.Parse(os.Args[1:]) if *showVersion { fmt.Printf("oauth2-proxy %s (built with %s)\n", VERSION, runtime.Version()) return } - legacyOpts := options.NewLegacyOptions() - err = options.Load(*config, flagSet, legacyOpts) + opts, err := loadConfiguration(*config, *alphaConfig, configFlagSet, os.Args[1:]) if err != nil { - logger.Errorf("ERROR: Failed to load config: %v", err) - os.Exit(1) - } - - opts, err := legacyOpts.ToOptions() - if err != nil { - logger.Errorf("ERROR: Failed to convert config: %v", err) + logger.Printf("ERROR: %v", err) os.Exit(1) } @@ -74,3 +64,73 @@ func main() { }() s.ListenAndServe() } + +// loadConfiguration will load in the user's configuration. +// It will either load the alpha configuration (if alphaConfig is given) +// or the legacy configuration. +func loadConfiguration(config, alphaConfig string, extraFlags *pflag.FlagSet, args []string) (*options.Options, error) { + if alphaConfig != "" { + logger.Printf("WARNING: You are using alpha configuration. The structure in this configuration file may change without notice. You MUST remove conflicting options from your existing configuration.") + return loadAlphaOptions(config, alphaConfig, extraFlags, args) + } + return loadLegacyOptions(config, extraFlags, args) +} + +// loadLegacyOptions loads the old toml options using the legacy flagset +// and legacy options struct. +func loadLegacyOptions(config string, extraFlags *pflag.FlagSet, args []string) (*options.Options, error) { + optionsFlagSet := options.NewLegacyFlagSet() + optionsFlagSet.AddFlagSet(extraFlags) + if err := optionsFlagSet.Parse(args); err != nil { + return nil, fmt.Errorf("failed to parse flags: %v", err) + } + + legacyOpts := options.NewLegacyOptions() + if err := options.Load(config, optionsFlagSet, legacyOpts); err != nil { + return nil, fmt.Errorf("failed to load config: %v", err) + } + + opts, err := legacyOpts.ToOptions() + if err != nil { + return nil, fmt.Errorf("failed to convert config: %v", err) + } + + return opts, nil +} + +// loadAlphaOptions loads the old style config excluding options converted to +// the new alpha format, then merges the alpha options, loaded from YAML, +// into the core configuration. +func loadAlphaOptions(config, alphaConfig string, extraFlags *pflag.FlagSet, args []string) (*options.Options, error) { + opts, err := loadOptions(config, extraFlags, args) + if err != nil { + return nil, fmt.Errorf("failed to load core options: %v", err) + } + + alphaOpts := &options.AlphaOptions{} + if err := options.LoadYAML(alphaConfig, alphaOpts); err != nil { + return nil, fmt.Errorf("failed to load alpha options: %v", err) + } + + alphaOpts.MergeInto(opts) + return opts, nil +} + +// loadOptions loads the configuration using the old style format into the +// core options.Options struct. +// This means that none of the options that have been converted to alpha config +// will be loaded using this method. +func loadOptions(config string, extraFlags *pflag.FlagSet, args []string) (*options.Options, error) { + optionsFlagSet := options.NewFlagSet() + optionsFlagSet.AddFlagSet(extraFlags) + if err := optionsFlagSet.Parse(args); err != nil { + return nil, fmt.Errorf("failed to parse flags: %v", err) + } + + opts := options.NewOptions() + if err := options.Load(config, optionsFlagSet, opts); err != nil { + return nil, fmt.Errorf("failed to load config: %v", err) + } + + return opts, nil +} diff --git a/main_suite_test.go b/main_suite_test.go new file mode 100644 index 00000000..faa0383a --- /dev/null +++ b/main_suite_test.go @@ -0,0 +1,16 @@ +package main + +import ( + "testing" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestMainSuite(t *testing.T) { + logger.SetOutput(GinkgoWriter) + + RegisterFailHandler(Fail) + RunSpecs(t, "Main Suite") +} diff --git a/main_test.go b/main_test.go new file mode 100644 index 00000000..a91940e7 --- /dev/null +++ b/main_test.go @@ -0,0 +1,215 @@ +package main + +import ( + "errors" + "io/ioutil" + "os" + "time" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + "github.com/spf13/pflag" +) + +var _ = Describe("Configuration Loading Suite", func() { + const testLegacyConfig = ` +upstreams="http://httpbin" +set_basic_auth="true" +basic_auth_password="super-secret-password" +` + + const testAlphaConfig = ` +upstreams: + - id: / + path: / + uri: http://httpbin + flushInterval: 1s + passHostHeader: true + proxyWebSockets: true +injectRequestHeaders: +- name: Authorization + values: + - claim: user + prefix: "Basic " + basicAuthPassword: + value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk +- name: X-Forwarded-Groups + values: + - claim: groups +- name: X-Forwarded-User + values: + - claim: user +- name: X-Forwarded-Email + values: + - claim: email +- name: X-Forwarded-Preferred-Username + values: + - claim: preferred_username +injectResponseHeaders: +- name: Authorization + values: + - claim: user + prefix: "Basic " + basicAuthPassword: + value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk +` + + const testCoreConfig = ` +http_address="0.0.0.0:4180" +cookie_secret="OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=" +provider="oidc" +email_domains="example.com" +oidc_issuer_url="http://dex.localhost:4190/dex" +client_secret="b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK" +client_id="oauth2-proxy" +cookie_secure="false" + +redirect_url="http://localhost:4180/oauth2/callback" +` + + boolPtr := func(b bool) *bool { + return &b + } + + durationPtr := func(d time.Duration) *options.Duration { + du := options.Duration(d) + return &du + } + + testExpectedOptions := func() *options.Options { + opts, err := options.NewLegacyOptions().ToOptions() + Expect(err).ToNot(HaveOccurred()) + + opts.HTTPAddress = "0.0.0.0:4180" + opts.Cookie.Secret = "OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=" + opts.ProviderType = "oidc" + opts.EmailDomains = []string{"example.com"} + opts.OIDCIssuerURL = "http://dex.localhost:4190/dex" + opts.ClientSecret = "b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK" + opts.ClientID = "oauth2-proxy" + opts.Cookie.Secure = false + opts.RawRedirectURL = "http://localhost:4180/oauth2/callback" + + opts.UpstreamServers = options.Upstreams{ + { + ID: "/", + Path: "/", + URI: "http://httpbin", + FlushInterval: durationPtr(options.DefaultUpstreamFlushInterval), + PassHostHeader: boolPtr(true), + ProxyWebSockets: boolPtr(true), + }, + } + + authHeader := options.Header{ + Name: "Authorization", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "user", + Prefix: "Basic ", + BasicAuthPassword: &options.SecretSource{ + Value: []byte("super-secret-password"), + }, + }, + }, + }, + } + + opts.InjectRequestHeaders = append([]options.Header{authHeader}, opts.InjectRequestHeaders...) + opts.InjectResponseHeaders = append(opts.InjectResponseHeaders, authHeader) + return opts + } + + type loadConfigurationTableInput struct { + configContent string + alphaConfigContent string + args []string + extraFlags func() *pflag.FlagSet + expectedOptions func() *options.Options + expectedErr error + } + + DescribeTable("LoadConfiguration", + func(in loadConfigurationTableInput) { + var configFileName, alphaConfigFileName string + + defer func() { + if configFileName != "" { + Expect(os.Remove(configFileName)).To(Succeed()) + } + if alphaConfigFileName != "" { + Expect(os.Remove(alphaConfigFileName)).To(Succeed()) + } + }() + + if in.configContent != "" { + By("Writing the config to a temporary file", func() { + file, err := ioutil.TempFile("", "oauth2-proxy-test-config-XXXX.cfg") + Expect(err).ToNot(HaveOccurred()) + defer file.Close() + + configFileName = file.Name() + + _, err = file.WriteString(in.configContent) + Expect(err).ToNot(HaveOccurred()) + }) + } + + if in.alphaConfigContent != "" { + By("Writing the config to a temporary file", func() { + file, err := ioutil.TempFile("", "oauth2-proxy-test-alpha-config-XXXX.yaml") + Expect(err).ToNot(HaveOccurred()) + defer file.Close() + + alphaConfigFileName = file.Name() + + _, err = file.WriteString(in.alphaConfigContent) + Expect(err).ToNot(HaveOccurred()) + }) + } + + extraFlags := pflag.NewFlagSet("test-flagset", pflag.ExitOnError) + if in.extraFlags != nil { + extraFlags = in.extraFlags() + } + + opts, err := loadConfiguration(configFileName, alphaConfigFileName, extraFlags, in.args) + if in.expectedErr != nil { + Expect(err).To(MatchError(in.expectedErr.Error())) + } else { + Expect(err).ToNot(HaveOccurred()) + } + Expect(in.expectedOptions).ToNot(BeNil()) + Expect(opts).To(Equal(in.expectedOptions())) + }, + Entry("with legacy configuration", loadConfigurationTableInput{ + configContent: testCoreConfig + testLegacyConfig, + expectedOptions: testExpectedOptions, + }), + Entry("with alpha configuration", loadConfigurationTableInput{ + configContent: testCoreConfig, + alphaConfigContent: testAlphaConfig, + expectedOptions: testExpectedOptions, + }), + Entry("with bad legacy configuration", loadConfigurationTableInput{ + configContent: testCoreConfig + "unknown_field=\"something\"", + expectedOptions: func() *options.Options { return nil }, + expectedErr: errors.New("failed to load config: error unmarshalling config: 1 error(s) decoding:\n\n* '' has invalid keys: unknown_field"), + }), + Entry("with bad alpha configuration", loadConfigurationTableInput{ + configContent: testCoreConfig, + alphaConfigContent: testAlphaConfig + ":", + expectedOptions: func() *options.Options { return nil }, + expectedErr: errors.New("failed to load alpha options: error unmarshalling config: error converting YAML to JSON: yaml: line 34: did not find expected key"), + }), + Entry("with alpha configuration and bad core configuration", loadConfigurationTableInput{ + configContent: testCoreConfig + "unknown_field=\"something\"", + alphaConfigContent: testAlphaConfig, + expectedOptions: func() *options.Options { return nil }, + expectedErr: errors.New("failed to load core options: failed to load config: error unmarshalling config: 1 error(s) decoding:\n\n* '' has invalid keys: unknown_field"), + }), + ) +}) diff --git a/pkg/apis/options/alpha_options.go b/pkg/apis/options/alpha_options.go index 1086ee2a..4ff2b0e4 100644 --- a/pkg/apis/options/alpha_options.go +++ b/pkg/apis/options/alpha_options.go @@ -29,3 +29,11 @@ type AlphaOptions struct { // or from a static secret value. InjectResponseHeaders []Header `json:"injectResponseHeaders,omitempty"` } + +// MergeInto replaces alpha options in the Options struct with the values +// from the AlphaOptions +func (a *AlphaOptions) MergeInto(opts *Options) { + opts.UpstreamServers = a.Upstreams + opts.InjectRequestHeaders = a.InjectRequestHeaders + opts.InjectResponseHeaders = a.InjectResponseHeaders +} diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index 35edf680..d3fabd58 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -39,6 +39,15 @@ func NewLegacyOptions() *LegacyOptions { } } +func NewLegacyFlagSet() *pflag.FlagSet { + flagSet := NewFlagSet() + + flagSet.AddFlagSet(legacyUpstreamsFlagSet()) + flagSet.AddFlagSet(legacyHeadersFlagSet()) + + return flagSet +} + func (l *LegacyOptions) ToOptions() (*Options, error) { upstreams, err := l.LegacyUpstreams.convert() if err != nil { diff --git a/pkg/apis/options/load.go b/pkg/apis/options/load.go index aeb39b9a..9ead8a3b 100644 --- a/pkg/apis/options/load.go +++ b/pkg/apis/options/load.go @@ -1,10 +1,13 @@ package options import ( + "errors" "fmt" + "io/ioutil" "reflect" "strings" + "github.com/ghodss/yaml" "github.com/mitchellh/mapstructure" "github.com/spf13/pflag" "github.com/spf13/viper" @@ -132,3 +135,28 @@ func isUnexported(name string) bool { first := string(name[0]) return first == strings.ToLower(first) } + +// LoadYAML will load a YAML based configuration file into the options interface provided. +func LoadYAML(configFileName string, into interface{}) error { + v := viper.New() + v.SetConfigFile(configFileName) + v.SetConfigType("yaml") + v.SetTypeByDefaultValue(true) + + if configFileName == "" { + return errors.New("no configuration file provided") + } + + data, err := ioutil.ReadFile(configFileName) + if err != nil { + return fmt.Errorf("unable to load config file: %w", err) + } + + // UnmarshalStrict will return an error if the config includes options that are + // not mapped to felds of the into struct + if err := yaml.UnmarshalStrict(data, into, yaml.DisallowUnknownFields); err != nil { + return fmt.Errorf("error unmarshalling config: %w", err) + } + + return nil +} diff --git a/pkg/apis/options/load_test.go b/pkg/apis/options/load_test.go index 9f6f5d4f..28fd79e6 100644 --- a/pkg/apis/options/load_test.go +++ b/pkg/apis/options/load_test.go @@ -1,9 +1,11 @@ package options import ( + "errors" "fmt" "io/ioutil" "os" + "time" . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" @@ -295,10 +297,199 @@ var _ = Describe("Load", func() { expectedOutput: NewOptions(), }), Entry("with an empty LegacyOptions struct, should return default values", &testOptionsTableInput{ - flagSet: NewFlagSet, + flagSet: NewLegacyFlagSet, input: &LegacyOptions{}, expectedOutput: NewLegacyOptions(), }), ) }) }) + +var _ = Describe("LoadYAML", func() { + Context("with a testOptions structure", func() { + type TestOptionSubStruct struct { + StringSliceOption []string `yaml:"stringSliceOption,omitempty"` + } + + type TestOptions struct { + StringOption string `yaml:"stringOption,omitempty"` + Sub TestOptionSubStruct `yaml:"sub,omitempty"` + + // Check that embedded fields can be unmarshalled + TestOptionSubStruct `yaml:",inline,squash"` + } + + var testOptionsConfigBytesFull = []byte(` +stringOption: foo +stringSliceOption: +- a +- b +- c +sub: + stringSliceOption: + - d + - e +`) + + type loadYAMLTableInput struct { + configFile []byte + input interface{} + expectedErr error + expectedOutput interface{} + } + + DescribeTable("LoadYAML", + func(in loadYAMLTableInput) { + var configFileName string + + if in.configFile != nil { + By("Creating a config file") + configFile, err := ioutil.TempFile("", "oauth2-proxy-test-config-file") + Expect(err).ToNot(HaveOccurred()) + defer configFile.Close() + + _, err = configFile.Write(in.configFile) + Expect(err).ToNot(HaveOccurred()) + defer os.Remove(configFile.Name()) + + configFileName = configFile.Name() + } + + var input interface{} + if in.input != nil { + input = in.input + } else { + input = &TestOptions{} + } + err := LoadYAML(configFileName, input) + if in.expectedErr != nil { + Expect(err).To(MatchError(in.expectedErr.Error())) + } else { + Expect(err).ToNot(HaveOccurred()) + } + Expect(input).To(Equal(in.expectedOutput)) + }, + Entry("with a valid input", loadYAMLTableInput{ + configFile: testOptionsConfigBytesFull, + input: &TestOptions{}, + expectedOutput: &TestOptions{ + StringOption: "foo", + Sub: TestOptionSubStruct{ + StringSliceOption: []string{"d", "e"}, + }, + TestOptionSubStruct: TestOptionSubStruct{ + StringSliceOption: []string{"a", "b", "c"}, + }, + }, + }), + Entry("with no config file", loadYAMLTableInput{ + configFile: nil, + input: &TestOptions{}, + expectedOutput: &TestOptions{}, + expectedErr: errors.New("no configuration file provided"), + }), + Entry("with invalid YAML", loadYAMLTableInput{ + configFile: []byte("\tfoo: bar"), + input: &TestOptions{}, + expectedOutput: &TestOptions{}, + expectedErr: errors.New("error unmarshalling config: error converting YAML to JSON: yaml: found character that cannot start any token"), + }), + Entry("with extra fields in the YAML", loadYAMLTableInput{ + configFile: append(testOptionsConfigBytesFull, []byte("foo: bar\n")...), + input: &TestOptions{}, + expectedOutput: &TestOptions{ + StringOption: "foo", + Sub: TestOptionSubStruct{ + StringSliceOption: []string{"d", "e"}, + }, + TestOptionSubStruct: TestOptionSubStruct{ + StringSliceOption: []string{"a", "b", "c"}, + }, + }, + expectedErr: errors.New("error unmarshalling config: error unmarshaling JSON: while decoding JSON: json: unknown field \"foo\""), + }), + Entry("with an incorrect type for a string field", loadYAMLTableInput{ + configFile: []byte(`stringOption: ["a", "b"]`), + input: &TestOptions{}, + expectedOutput: &TestOptions{}, + expectedErr: errors.New("error unmarshalling config: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal array into Go struct field TestOptions.StringOption of type string"), + }), + Entry("with an incorrect type for an array field", loadYAMLTableInput{ + configFile: []byte(`stringSliceOption: "a"`), + input: &TestOptions{}, + expectedOutput: &TestOptions{}, + expectedErr: errors.New("error unmarshalling config: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go struct field TestOptions.StringSliceOption of type []string"), + }), + ) + }) + + It("should load a full example AlphaOptions", func() { + config := []byte(` +upstreams: +- id: httpbin + path: / + uri: http://httpbin + flushInterval: 500ms +injectRequestHeaders: +- name: X-Forwarded-User + values: + - claim: user +injectResponseHeaders: +- name: X-Secret + values: + - value: c2VjcmV0 +`) + + By("Creating a config file") + configFile, err := ioutil.TempFile("", "oauth2-proxy-test-alpha-config-file") + Expect(err).ToNot(HaveOccurred()) + defer configFile.Close() + + _, err = configFile.Write(config) + Expect(err).ToNot(HaveOccurred()) + defer os.Remove(configFile.Name()) + + configFileName := configFile.Name() + + By("Loading the example config") + into := &AlphaOptions{} + Expect(LoadYAML(configFileName, into)).To(Succeed()) + + flushInterval := Duration(500 * time.Millisecond) + + Expect(into).To(Equal(&AlphaOptions{ + Upstreams: []Upstream{ + { + ID: "httpbin", + Path: "/", + URI: "http://httpbin", + FlushInterval: &flushInterval, + }, + }, + InjectRequestHeaders: []Header{ + { + Name: "X-Forwarded-User", + Values: []HeaderValue{ + { + ClaimSource: &ClaimSource{ + Claim: "user", + }, + }, + }, + }, + }, + InjectResponseHeaders: []Header{ + { + Name: "X-Secret", + Values: []HeaderValue{ + { + SecretSource: &SecretSource{ + Value: []byte("secret"), + }, + }, + }, + }, + }, + })) + }) +}) diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index f6c13abb..6cb95f54 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -246,8 +246,6 @@ func NewFlagSet() *pflag.FlagSet { flagSet.AddFlagSet(cookieFlagSet()) flagSet.AddFlagSet(loggingFlagSet()) - flagSet.AddFlagSet(legacyUpstreamsFlagSet()) - flagSet.AddFlagSet(legacyHeadersFlagSet()) return flagSet } From 5b683a7631877f2f3df1e4c54c27f5de6c869b47 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 9 Nov 2020 20:19:03 +0000 Subject: [PATCH 3/5] Add local environment that uses alpha configuration --- contrib/local-environment/Makefile | 8 ++++++++ .../docker-compose-alpha-config.yaml | 19 +++++++++++++++++++ .../oauth2-proxy-alpha-config.cfg | 10 ++++++++++ .../oauth2-proxy-alpha-config.yaml | 17 +++++++++++++++++ 4 files changed, 54 insertions(+) create mode 100644 contrib/local-environment/docker-compose-alpha-config.yaml create mode 100644 contrib/local-environment/oauth2-proxy-alpha-config.cfg create mode 100644 contrib/local-environment/oauth2-proxy-alpha-config.yaml diff --git a/contrib/local-environment/Makefile b/contrib/local-environment/Makefile index 512e6fab..46d50c85 100644 --- a/contrib/local-environment/Makefile +++ b/contrib/local-environment/Makefile @@ -6,6 +6,14 @@ up: %: docker-compose $* +.PHONY: alpha-config-up +alpha-config-up: + docker-compose -f docker-compose.yaml -f docker-compose-alpha-config.yaml up -d + +.PHONY: alpha-config-% +alpha-config-%: + docker-compose -f docker-compose.yaml -f docker-compose-nginx.yaml $* + .PHONY: nginx-up nginx-up: docker-compose -f docker-compose.yaml -f docker-compose-nginx.yaml up -d diff --git a/contrib/local-environment/docker-compose-alpha-config.yaml b/contrib/local-environment/docker-compose-alpha-config.yaml new file mode 100644 index 00000000..275b6dd3 --- /dev/null +++ b/contrib/local-environment/docker-compose-alpha-config.yaml @@ -0,0 +1,19 @@ +# This docker-compose file can be used to bring up an example instance of oauth2-proxy +# for manual testing and exploration of features. +# Alongside OAuth2-Proxy, this file also starts Dex to act as the identity provider, +# etcd for storage for Dex and HTTPBin as an example upstream. +# This file also uses alpha configuration when configuring OAuth2 Proxy. +# +# This file is an extension of the main compose file and must be used with it +# docker-compose -f docker-compose.yaml -f docker-compose-alpha-config.yaml +# Alternatively: +# make alpha-config- (eg make nginx-up, make nginx-down) +# +# Access http://localhost:4180 to initiate a login cycle +version: '3.0' +services: + oauth2-proxy: + command: --config /oauth2-proxy.cfg --alpha-config /oauth2-proxy-alpha-config.yaml + volumes: + - "./oauth2-proxy-alpha-config.cfg:/oauth2-proxy.cfg" + - "./oauth2-proxy-alpha-config.yaml:/oauth2-proxy-alpha-config.yaml" diff --git a/contrib/local-environment/oauth2-proxy-alpha-config.cfg b/contrib/local-environment/oauth2-proxy-alpha-config.cfg new file mode 100644 index 00000000..1f1448fc --- /dev/null +++ b/contrib/local-environment/oauth2-proxy-alpha-config.cfg @@ -0,0 +1,10 @@ +http_address="0.0.0.0:4180" +cookie_secret="OQINaROshtE9TcZkNAm-5Zs2Pv3xaWytBmc5W7sPX7w=" +provider="oidc" +email_domains="example.com" +oidc_issuer_url="http://dex.localhost:4190/dex" +client_secret="b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK" +client_id="oauth2-proxy" +cookie_secure="false" + +redirect_url="http://localhost:4180/oauth2/callback" diff --git a/contrib/local-environment/oauth2-proxy-alpha-config.yaml b/contrib/local-environment/oauth2-proxy-alpha-config.yaml new file mode 100644 index 00000000..b88b386a --- /dev/null +++ b/contrib/local-environment/oauth2-proxy-alpha-config.yaml @@ -0,0 +1,17 @@ +upstreams: + - id: httpbin + path: / + uri: http://httpbin +injectRequestHeaders: +- name: X-Forwarded-Groups + values: + - claim: groups +- name: X-Forwarded-User + values: + - claim: user +- name: X-Forwarded-Email + values: + - claim: email +- name: X-Forwarded-Preferred-Username + values: + - claim: preferred_username From b201dbb2d3881ee75907d2449f2da38d039c2ce3 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 9 Nov 2020 20:34:55 +0000 Subject: [PATCH 4/5] Add convert-config-to-alpha flag to convert existing configuration to alpha structure --- main.go | 31 +++++++++++++++++++++++++++++++ pkg/apis/options/alpha_options.go | 8 ++++++++ 2 files changed, 39 insertions(+) diff --git a/main.go b/main.go index 173540b3..225fb9af 100644 --- a/main.go +++ b/main.go @@ -9,6 +9,7 @@ import ( "syscall" "time" + "github.com/ghodss/yaml" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/validation" @@ -21,6 +22,7 @@ func main() { configFlagSet := pflag.NewFlagSet("oauth2-proxy", pflag.ContinueOnError) config := configFlagSet.String("config", "", "path to config file") alphaConfig := configFlagSet.String("alpha-config", "", "path to alpha config file (use at your own risk - the structure in this config file may change between minor releases)") + convertConfig := configFlagSet.Bool("convert-config-to-alpha", false, "if true, the proxy will load configuration as normal and convert existing configuration to the alpha config structure, and print it to stdout") showVersion := configFlagSet.Bool("version", false, "print version string") configFlagSet.Parse(os.Args[1:]) @@ -29,12 +31,23 @@ func main() { return } + if *convertConfig && *alphaConfig != "" { + logger.Fatal("cannot use alpha-config and conver-config-to-alpha together") + } + opts, err := loadConfiguration(*config, *alphaConfig, configFlagSet, os.Args[1:]) if err != nil { logger.Printf("ERROR: %v", err) os.Exit(1) } + if *convertConfig { + if err := printConvertedConfig(opts); err != nil { + logger.Fatalf("ERROR: could not convert config: %v", err) + } + return + } + err = validation.Validate(opts) if err != nil { logger.Printf("%s", err) @@ -134,3 +147,21 @@ func loadOptions(config string, extraFlags *pflag.FlagSet, args []string) (*opti return opts, nil } + +// printConvertedConfig extracts alpha options from the loaded configuration +// and renders these to stdout in YAML format. +func printConvertedConfig(opts *options.Options) error { + alphaConfig := &options.AlphaOptions{} + alphaConfig.ExtractFrom(opts) + + data, err := yaml.Marshal(alphaConfig) + if err != nil { + return fmt.Errorf("unable to marshal config: %v", err) + } + + if _, err := os.Stdout.Write(data); err != nil { + return fmt.Errorf("unable to write output: %v", err) + } + + return nil +} diff --git a/pkg/apis/options/alpha_options.go b/pkg/apis/options/alpha_options.go index 4ff2b0e4..6016bac0 100644 --- a/pkg/apis/options/alpha_options.go +++ b/pkg/apis/options/alpha_options.go @@ -37,3 +37,11 @@ func (a *AlphaOptions) MergeInto(opts *Options) { opts.InjectRequestHeaders = a.InjectRequestHeaders opts.InjectResponseHeaders = a.InjectResponseHeaders } + +// ExtractFrom populates the fields in the AlphaOptions with the values from +// the Options +func (a *AlphaOptions) ExtractFrom(opts *Options) { + a.Upstreams = opts.UpstreamServers + a.InjectRequestHeaders = opts.InjectRequestHeaders + a.InjectResponseHeaders = opts.InjectResponseHeaders +} From d749c11e7374b172082d585b57d7431b6e073f3e Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sat, 28 Nov 2020 10:32:27 +0000 Subject: [PATCH 5/5] Add changelog entry for adding alpha configuration --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13b834db..330b49de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ ## Changes since v6.1.1 +- [#907](https://github.com/oauth2-proxy/oauth2-proxy/pull/907) Introduce alpha configuration option to enable testing of structured configuration (@JoelSpeed) - [#938](https://github.com/oauth2-proxy/oauth2-proxy/pull/938) Cleanup missed provider renaming refactor methods (@NickMeves) - [#925](https://github.com/oauth2-proxy/oauth2-proxy/pull/925) Fix basic auth legacy header conversion (@JoelSpeed) - [#916](https://github.com/oauth2-proxy/oauth2-proxy/pull/916) Add AlphaOptions struct to prepare for alpha config loading (@JoelSpeed)