diff --git a/.golangci.yml b/.golangci.yml index edab12d0..62e1df8e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -37,7 +37,7 @@ linters: - linters: - revive path: _test\.go - text: 'dot-imports:' + text: "dot-imports:" # # If we have tests in shared test folders, these can be less strictly linted - linters: - bodyclose diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index 28645ceb..dcfb9648 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -204,16 +204,6 @@ ClaimSource allows loading a header value from a claim within the session | `prefix` | _string_ | Prefix is an optional prefix that will be prepended to the value of the
claim if it is non-empty. | | `basicAuthPassword` | _[SecretSource](#secretsource)_ | 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. | -### Duration -#### (`string` alias) - -(**Appears on:** [Upstream](#upstream)) - -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". - ### GitHubOptions (**Appears on:** [Provider](#provider)) @@ -275,7 +265,7 @@ make up the header value | Field | Type | Description | | ----- | ---- | ----------- | -| `value` | _[]byte_ | Value expects a base64 encoded string value. | +| `value` | _string_ | Value expects a base64 encoded string value. | | `fromEnv` | _string_ | FromEnv expects the name of an environment variable. | | `fromFile` | _string_ | FromFile expects a path to a file containing the secret value. | | `claim` | _string_ | Claim is the name of the claim in the session that the value should be
loaded from. Available claims: `access_token` `id_token` `created_at`
`expires_on` `refresh_token` `email` `user` `groups` `preferred_username`. | @@ -487,7 +477,7 @@ Only one source within the struct should be defined at any time. | Field | Type | Description | | ----- | ---- | ----------- | -| `value` | _[]byte_ | Value expects a base64 encoded string value. | +| `value` | _string_ | Value expects a base64 encoded string value. | | `fromEnv` | _string_ | FromEnv expects the name of an environment variable. | | `fromFile` | _string_ | FromFile expects a path to a file containing the secret value. | @@ -547,10 +537,10 @@ Requests will be proxied to this upstream if the path matches the request path. | `insecureSkipTLSVerify` | _bool_ | InsecureSkipTLSVerify will skip TLS verification of upstream HTTPS hosts.
This option is insecure and will allow potential Man-In-The-Middle attacks
between OAuth2 Proxy and the upstream server.
Defaults to false. | | `static` | _bool_ | 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. | | `staticCode` | _int_ | StaticCode determines the response code for the Static response.
This option can only be used with Static enabled. | -| `flushInterval` | _[Duration](#duration)_ | FlushInterval is the period between flushing the response buffer when
streaming response from the upstream.
Defaults to 1 second. | +| `flushInterval` | _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. | +| `timeout` | _duration_ | Timeout is the maximum duration the server will wait for a response from the upstream server.
Defaults to 30 seconds. | | `disableKeepAlives` | _bool_ | DisableKeepAlives disables HTTP keep-alive connections to the upstream server.
Defaults to false. | ### UpstreamConfig diff --git a/main.go b/main.go index cf7e964c..d9fc406b 100644 --- a/main.go +++ b/main.go @@ -67,12 +67,17 @@ func main() { // 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) +func loadConfiguration(config, yamlConfig string, extraFlags *pflag.FlagSet, args []string) (*options.Options, error) { + opts, err := loadLegacyOptions(config, extraFlags, args) + if err != nil { + return nil, err } - return loadLegacyOptions(config, extraFlags, args) + + if yamlConfig != "" { + 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 loadYamlOptions(yamlConfig, config, extraFlags, args) + } + return opts, err } // loadLegacyOptions loads the old toml options using the legacy flagset @@ -97,17 +102,17 @@ func loadLegacyOptions(config string, extraFlags *pflag.FlagSet, args []string) return opts, nil } -// loadAlphaOptions loads the old style config excluding options converted to +// loadYamlOptions 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) { +func loadYamlOptions(yamlConfig, config 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 { + alphaOpts := options.NewAlphaOptions(opts) + if err := options.LoadYAML(yamlConfig, alphaOpts); err != nil { return nil, fmt.Errorf("failed to load alpha options: %v", err) } @@ -137,10 +142,16 @@ func loadOptions(config string, extraFlags *pflag.FlagSet, args []string) (*opti // 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) + alphaConfig := options.NewAlphaOptions(opts) - data, err := yaml.Marshal(alphaConfig) + // Generic interface for loading arbitrary yaml structure + var buffer map[string]interface{} + + if err := options.Decode(alphaConfig, &buffer); err != nil { + return fmt.Errorf("unable to decode alpha config into interface: %w", err) + } + + data, err := yaml.Marshal(buffer) if err != nil { return fmt.Errorf("unable to marshal config: %v", err) } diff --git a/main_test.go b/main_test.go index 8e40fe7f..a6ea83c2 100644 --- a/main_test.go +++ b/main_test.go @@ -43,29 +43,35 @@ upstreamConfig: injectRequestHeaders: - name: Authorization values: - - claim: user - prefix: "Basic " - basicAuthPassword: - value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk + - claimSource: + claim: user + prefix: "Basic " + basicAuthPassword: + value: super-secret-password - name: X-Forwarded-Groups values: - - claim: groups + - claimSource: + claim: groups - name: X-Forwarded-User values: - - claim: user + - claimSource: + claim: user - name: X-Forwarded-Email values: - - claim: email + - claimSource: + claim: email - name: X-Forwarded-Preferred-Username values: - - claim: preferred_username + - claimSource: + claim: preferred_username injectResponseHeaders: - name: Authorization values: - - claim: user - prefix: "Basic " - basicAuthPassword: - value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk + - claimSource: + claim: user + prefix: "Basic " + basicAuthPassword: + value: super-secret-password server: bindAddress: "127.0.0.1:4180" providers: @@ -100,9 +106,8 @@ redirect_url="http://localhost:4180/oauth2/callback" return &b } - durationPtr := func(d time.Duration) *options.Duration { - du := options.Duration(d) - return &du + durationPtr := func(d time.Duration) *time.Duration { + return &d } testExpectedOptions := func() *options.Options { @@ -136,7 +141,7 @@ redirect_url="http://localhost:4180/oauth2/callback" Claim: "user", Prefix: "Basic ", BasicAuthPassword: &options.SecretSource{ - Value: []byte("super-secret-password"), + Value: "super-secret-password", }, }, }, @@ -226,7 +231,7 @@ redirect_url="http://localhost:4180/oauth2/callback" opts, err := loadConfiguration(configFileName, alphaConfigFileName, extraFlags, in.args) if in.expectedErr != nil { - Expect(err).To(MatchError(in.expectedErr.Error())) + Expect(err).To(MatchError(ContainSubstring(in.expectedErr.Error()))) } else { Expect(err).ToNot(HaveOccurred()) } diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 488b8cea..7b495bff 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -215,7 +215,7 @@ func TestBasicAuthPassword(t *testing.T) { ClaimSource: &options.ClaimSource{ Claim: "email", BasicAuthPassword: &options.SecretSource{ - Value: []byte(basicAuthPassword), + Value: basicAuthPassword, }, }, }, @@ -1282,7 +1282,7 @@ func TestAuthOnlyEndpointSetBasicAuthTrueRequestHeaders(t *testing.T) { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte("This is a secure password"), + Value: "This is a secure password", }, }, }, @@ -2044,7 +2044,7 @@ func baseTestOptions() *options.Options { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("This is a secure password"))), + Value: base64.StdEncoding.EncodeToString([]byte("This is a secure password")), }, }, }, diff --git a/pkg/apis/options/alpha_options.go b/pkg/apis/options/alpha_options.go index a438518c..278db401 100644 --- a/pkg/apis/options/alpha_options.go +++ b/pkg/apis/options/alpha_options.go @@ -47,15 +47,11 @@ type AlphaOptions struct { Providers Providers `json:"providers,omitempty"` } -// MergeInto replaces alpha options in the Options struct with the values -// from the AlphaOptions -func (a *AlphaOptions) MergeInto(opts *Options) { - opts.UpstreamServers = a.UpstreamConfig - opts.InjectRequestHeaders = a.InjectRequestHeaders - opts.InjectResponseHeaders = a.InjectResponseHeaders - opts.Server = a.Server - opts.MetricsServer = a.MetricsServer - opts.Providers = a.Providers +// Initialize alpha options with default values and settings of the core options +func NewAlphaOptions(opts *Options) *AlphaOptions { + aOpts := &AlphaOptions{} + aOpts.ExtractFrom(opts) + return aOpts } // ExtractFrom populates the fields in the AlphaOptions with the values from @@ -68,3 +64,14 @@ func (a *AlphaOptions) ExtractFrom(opts *Options) { a.MetricsServer = opts.MetricsServer a.Providers = opts.Providers } + +// MergeInto replaces alpha options in the Options struct with the values +// from the AlphaOptions +func (a *AlphaOptions) MergeInto(opts *Options) { + opts.UpstreamServers = a.UpstreamConfig + opts.InjectRequestHeaders = a.InjectRequestHeaders + opts.InjectResponseHeaders = a.InjectResponseHeaders + opts.Server = a.Server + opts.MetricsServer = a.MetricsServer + opts.Providers = a.Providers +} diff --git a/pkg/apis/options/common.go b/pkg/apis/options/common.go deleted file mode 100644 index 88d24d82..00000000 --- a/pkg/apis/options/common.go +++ /dev/null @@ -1,63 +0,0 @@ -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 `json:"value,omitempty"` - - // FromEnv expects the name of an environment variable. - FromEnv string `json:"fromEnv,omitempty"` - - // FromFile expects a path to a file containing the secret value. - 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". -// +reference-gen:alias-name=string -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/duration.go b/pkg/apis/options/duration.go new file mode 100644 index 00000000..da13d96e --- /dev/null +++ b/pkg/apis/options/duration.go @@ -0,0 +1,43 @@ +package options + +import ( + "reflect" + "time" + + "github.com/mitchellh/mapstructure" +) + +// 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". + +// Conversion from string or floating point to golang duration type +// This way floating points will be converted to seconds and strings +// of type 3s or 5m will be parsed with time.ParseDuration +func toDurationHookFunc() mapstructure.DecodeHookFunc { + return func( + f reflect.Type, + t reflect.Type, + data interface{}) (interface{}, error) { + if t != reflect.TypeOf(time.Duration(0)) { + return data, nil + } + + switch f.Kind() { + case reflect.String: + return time.ParseDuration(data.(string)) + case reflect.Float64: + return time.Duration(data.(float64) * float64(time.Second)), nil + case reflect.Int64: + return time.Duration(data.(int64)), nil + default: + return data, nil + } + } +} diff --git a/pkg/apis/options/header.go b/pkg/apis/options/header.go index 90e6445c..a7e10c44 100644 --- a/pkg/apis/options/header.go +++ b/pkg/apis/options/header.go @@ -21,10 +21,10 @@ type Header struct { // make up the header value type HeaderValue struct { // Allow users to load the value from a secret source - *SecretSource `json:",omitempty"` + *SecretSource `json:"secretSource,omitempty"` // Allow users to load the value from a session claim - *ClaimSource `json:",omitempty"` + *ClaimSource `json:"claimSource,omitempty"` } // ClaimSource allows loading a header value from a claim within the session diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index 12975225..db73f910 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -136,8 +136,8 @@ func (l *LegacyUpstreams) convert() (UpstreamConfig, error) { u.Path = "/" } - flushInterval := Duration(l.FlushInterval) - timeout := Duration(l.Timeout) + flushInterval := l.FlushInterval + timeout := l.Timeout upstream := Upstream{ ID: u.Path, Path: u.Path, @@ -294,7 +294,7 @@ func getBasicAuthHeader(preferEmailToUser bool, basicAuthPassword string) Header Claim: claim, Prefix: "Basic ", BasicAuthPassword: &SecretSource{ - Value: []byte(basicAuthPassword), + Value: basicAuthPassword, }, }, }, diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index 9481cf95..c5f0e4da 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -15,8 +15,8 @@ var _ = Describe("Legacy Options", func() { legacyOpts := NewLegacyOptions() // Set upstreams and related options to test their conversion - flushInterval := Duration(5 * time.Second) - timeout := Duration(5 * time.Second) + flushInterval := 5 * time.Second + timeout := 5 * time.Second legacyOpts.LegacyUpstreams.FlushInterval = time.Duration(flushInterval) legacyOpts.LegacyUpstreams.Timeout = time.Duration(timeout) legacyOpts.LegacyUpstreams.PassHostHeader = true @@ -147,8 +147,8 @@ var _ = Describe("Legacy Options", func() { skipVerify := true passHostHeader := false proxyWebSockets := true - flushInterval := Duration(5 * time.Second) - timeout := Duration(5 * time.Second) + flushInterval := 5 * time.Second + timeout := 5 * time.Second disableKeepAlives := true // Test cases and expected outcomes @@ -369,7 +369,7 @@ var _ = Describe("Legacy Options", func() { Claim: "user", Prefix: "Basic ", BasicAuthPassword: &SecretSource{ - Value: []byte(basicAuthSecret), + Value: basicAuthSecret, }, }, }, @@ -409,7 +409,7 @@ var _ = Describe("Legacy Options", func() { Claim: "email", Prefix: "Basic ", BasicAuthPassword: &SecretSource{ - Value: []byte(basicAuthSecret), + Value: basicAuthSecret, }, }, }, diff --git a/pkg/apis/options/load.go b/pkg/apis/options/load.go index c302e8e7..22e775a8 100644 --- a/pkg/apis/options/load.go +++ b/pkg/apis/options/load.go @@ -55,6 +55,76 @@ func Load(configFileName string, flagSet *pflag.FlagSet, into interface{}) error return nil } +// LoadYAML will load a YAML based configuration file into the options interface provided. +func LoadYAML(configFileName string, opts interface{}) error { + buffer, err := loadAndSubstituteEnvs(configFileName) + if err != nil { + return err + } + + // Generic interface for loading arbitrary yaml structure + var intermediate map[string]interface{} + + if err := yaml.Unmarshal(buffer, &intermediate); err != nil { + return fmt.Errorf("error unmarshalling config: %w", err) + } + + return Decode(intermediate, opts) +} + +func Decode(input interface{}, result interface{}) error { + // Using mapstructure to decode arbitrary yaml structure into options and + // merge with existing values instead of overwriting everything. This is especially + // important as we have a lot of default values for boolean which are supposed to be + // true by default. Normally by just parsing through yaml all booleans that aren't + // referenced in the config file would be parsed as false and we cannot identify after + // the fact if they have been explicitly set to false or have not been referenced. + decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: mapstructure.ComposeDecodeHookFunc(toDurationHookFunc()), + Metadata: nil, // Don't track any metadata + Result: result, // Decode the result into the prefilled options + TagName: "json", // Parse all fields that use the yaml tag + ZeroFields: false, // Don't clean the default values from the result map (options) + ErrorUnused: true, // Throw an error if keys have been used that aren't mapped to any struct fields + IgnoreUntaggedFields: true, // Ignore fields in structures that aren't tagged with yaml + }) + + if err != nil { + return fmt.Errorf("error creating decoder for config: %w", err) + } + + if err := decoder.Decode(input); err != nil { + return fmt.Errorf("error decoding config: %w", err) + } + + return nil +} + +// loadAndSubstituteEnvs reads the yaml config into a generic byte buffer and +// substitute env references +func loadAndSubstituteEnvs(configFileName string) ([]byte, error) { + if configFileName == "" { + return nil, errors.New("no configuration file provided") + } + + unparsedBuffer, err := os.ReadFile(configFileName) + if err != nil { + return nil, fmt.Errorf("unable to load config file: %w", err) + } + + modifiedBuffer, err := normalizeSubstitution(unparsedBuffer) + if err != nil { + return nil, fmt.Errorf("error normalizing substitution string : %w", err) + } + + buffer, err := envsubst.Bytes(modifiedBuffer) + if err != nil { + return nil, fmt.Errorf("error in substituting env variables : %w", err) + } + + return buffer, nil +} + // registerFlags uses `cfg` and `flag` tags to associate flags in the flagSet // to the fields in the options interface provided. // Each exported field in the options must have a `cfg` tag otherwise an error will occur. @@ -140,47 +210,6 @@ func isUnexported(name string) bool { 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 { - buffer, err := loadAndParseYaml(configFileName) - if err != nil { - return err - } - - // UnmarshalStrict will return an error if the config includes options that are - // not mapped to fields of the into struct - if err := yaml.UnmarshalStrict(buffer, into, yaml.DisallowUnknownFields); err != nil { - return fmt.Errorf("error unmarshalling config: %w", err) - } - - return nil -} - -// loadAndParseYaml reads the config from the filesystem and -// execute the environment variable substitution -func loadAndParseYaml(configFileName string) ([]byte, error) { - if configFileName == "" { - return nil, errors.New("no configuration file provided") - } - - unparsedBuffer, err := os.ReadFile(configFileName) - if err != nil { - return nil, fmt.Errorf("unable to load config file: %w", err) - } - - modifiedBuffer, err := normalizeSubstitution(unparsedBuffer) - if err != nil { - return nil, fmt.Errorf("error normalizing substitution string : %w", err) - } - - buffer, err := envsubst.Bytes(modifiedBuffer) - if err != nil { - return nil, fmt.Errorf("error in substituting env variables : %w", err) - } - - return buffer, nil -} - // normalizeSubstitution normalizes dollar signs ($) with numerals like // $1 or $2 properly by correctly escaping them func normalizeSubstitution(unparsedBuffer []byte) ([]byte, error) { diff --git a/pkg/apis/options/load_test.go b/pkg/apis/options/load_test.go index 06123c37..6f327a22 100644 --- a/pkg/apis/options/load_test.go +++ b/pkg/apis/options/load_test.go @@ -355,15 +355,15 @@ var _ = Describe("Load", func() { var _ = Describe("LoadYAML", func() { Context("with a testOptions structure", func() { type TestOptionSubStruct struct { - StringSliceOption []string `yaml:"stringSliceOption,omitempty"` + StringSliceOption []string `json:"stringSliceOption,omitempty"` } type TestOptions struct { - StringOption string `yaml:"stringOption,omitempty"` - Sub TestOptionSubStruct `yaml:"sub,omitempty"` + StringOption string `json:"stringOption,omitempty"` + Sub TestOptionSubStruct `json:"sub,omitempty"` // Check that embedded fields can be unmarshalled - TestOptionSubStruct `yaml:",inline,squash"` + TestOptionSubStruct `json:",inline,squash"` } var testOptionsConfigBytesFull = []byte(` @@ -416,7 +416,7 @@ sub: err := LoadYAML(configFileName, input) if in.expectedErr != nil { - Expect(err).To(MatchError(in.expectedErr.Error())) + Expect(err).To(MatchError(ContainSubstring(in.expectedErr.Error()))) } else { Expect(err).ToNot(HaveOccurred()) } @@ -459,13 +459,13 @@ sub: StringSliceOption: []string{"a", "b", "c"}, }, }, - expectedErr: errors.New("error unmarshalling config: error unmarshaling JSON: while decoding JSON: json: unknown field \"foo\""), + expectedErr: errors.New("has invalid keys: 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"), + expectedErr: errors.New("'stringOption' expected type 'string', got unconvertible type"), }), Entry("with an incorrect type for an array field", loadYAMLTableInput{ configFile: []byte(`stringSliceOption: "a"`), @@ -526,11 +526,13 @@ upstreamConfig: injectRequestHeaders: - name: X-Forwarded-User values: - - claim: user + - claimSource: + claim: user injectResponseHeaders: - name: X-Secret values: - - value: c2VjcmV0 + - secretSource: + value: secret `) By("Creating a config file") @@ -548,7 +550,7 @@ injectResponseHeaders: into := &AlphaOptions{} Expect(LoadYAML(configFileName, into)).To(Succeed()) - flushInterval := Duration(500 * time.Millisecond) + flushInterval := 500 * time.Millisecond Expect(into).To(Equal(&AlphaOptions{ UpstreamConfig: UpstreamConfig{ @@ -579,7 +581,7 @@ injectResponseHeaders: Values: []HeaderValue{ { SecretSource: &SecretSource{ - Value: []byte("secret"), + Value: "secret", }, }, }, diff --git a/pkg/apis/options/providers.go b/pkg/apis/options/providers.go index 0f254575..c94b6b92 100644 --- a/pkg/apis/options/providers.go +++ b/pkg/apis/options/providers.go @@ -238,16 +238,16 @@ type OIDCOptions struct { IssuerURL string `json:"issuerURL,omitempty"` // InsecureAllowUnverifiedEmail prevents failures if an email address in an id_token is not verified // default set to 'false' - InsecureAllowUnverifiedEmail bool `json:"insecureAllowUnverifiedEmail,omitempty"` + InsecureAllowUnverifiedEmail bool `json:"insecureAllowUnverifiedEmail"` // InsecureSkipIssuerVerification skips verification of ID token issuers. When false, ID Token Issuers must match the OIDC discovery URL // default set to 'false' - InsecureSkipIssuerVerification bool `json:"insecureSkipIssuerVerification,omitempty"` + InsecureSkipIssuerVerification bool `json:"insecureSkipIssuerVerification"` // InsecureSkipNonce skips verifying the ID Token's nonce claim that must match // the random nonce sent in the initial OAuth flow. Otherwise, the nonce is checked // after the initial OAuth redeem & subsequent token refreshes. // default set to 'true' // Warning: In a future release, this will change to 'false' by default for enhanced security. - InsecureSkipNonce bool `json:"insecureSkipNonce,omitempty"` + InsecureSkipNonce bool `json:"insecureSkipNonce"` // SkipDiscovery allows to skip OIDC discovery and use manually supplied Endpoints // default set to 'false' SkipDiscovery bool `json:"skipDiscovery,omitempty"` diff --git a/pkg/apis/options/secret_source.go b/pkg/apis/options/secret_source.go new file mode 100644 index 00000000..2be4d890 --- /dev/null +++ b/pkg/apis/options/secret_source.go @@ -0,0 +1,14 @@ +package options + +// 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 string `json:"value,omitempty"` + + // FromEnv expects the name of an environment variable. + FromEnv string `json:"fromEnv,omitempty"` + + // FromFile expects a path to a file containing the secret value. + FromFile string `json:"fromFile,omitempty"` +} diff --git a/pkg/apis/options/upstreams.go b/pkg/apis/options/upstreams.go index b3c7195f..1002ae07 100644 --- a/pkg/apis/options/upstreams.go +++ b/pkg/apis/options/upstreams.go @@ -79,7 +79,7 @@ type Upstream struct { // FlushInterval is the period between flushing the response buffer when // streaming response from the upstream. // Defaults to 1 second. - FlushInterval *Duration `json:"flushInterval,omitempty"` + FlushInterval *time.Duration `json:"flushInterval,omitempty"` // PassHostHeader determines whether the request host header should be proxied // to the upstream server. @@ -92,7 +92,7 @@ type Upstream struct { // 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"` + Timeout *time.Duration `json:"timeout,omitempty"` // DisableKeepAlives disables HTTP keep-alive connections to the upstream server. // Defaults to false. diff --git a/pkg/apis/options/util/util.go b/pkg/apis/options/util/util.go index 03f0a134..794a6e91 100644 --- a/pkg/apis/options/util/util.go +++ b/pkg/apis/options/util/util.go @@ -11,7 +11,7 @@ import ( func GetSecretValue(source *options.SecretSource) ([]byte, error) { switch { case len(source.Value) > 0 && source.FromEnv == "" && source.FromFile == "": - return source.Value, nil + return []byte(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 e84db1ec..5c4bfa6d 100644 --- a/pkg/apis/options/util/util_test.go +++ b/pkg/apis/options/util/util_test.go @@ -31,7 +31,7 @@ var _ = Describe("GetSecretValue", func() { It("returns the correct value from the string value", func() { value, err := GetSecretValue(&options.SecretSource{ - Value: []byte("secret-value-1"), + Value: "secret-value-1", }) Expect(err).ToNot(HaveOccurred()) Expect(string(value)).To(Equal("secret-value-1")) diff --git a/pkg/header/injector_test.go b/pkg/header/injector_test.go index 25c276dc..bb37261d 100644 --- a/pkg/header/injector_test.go +++ b/pkg/header/injector_test.go @@ -55,7 +55,7 @@ var _ = Describe("Injector Suite", func() { Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: []byte("super-secret"), + Value: "super-secret", }, }, }, @@ -199,7 +199,7 @@ var _ = Describe("Injector Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte("basic-password"), + Value: "basic-password", }, }, }, @@ -227,7 +227,7 @@ var _ = Describe("Injector Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), + Value: base64.StdEncoding.EncodeToString([]byte("basic-password")), }, }, }, @@ -322,7 +322,7 @@ var _ = Describe("Injector Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), + Value: base64.StdEncoding.EncodeToString([]byte("basic-password")), FromEnv: "SECRET_ENV", }, }, @@ -348,7 +348,7 @@ var _ = Describe("Injector Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte("basic-password"), + Value: "basic-password", }, }, }, @@ -379,17 +379,17 @@ var _ = Describe("Injector Suite", func() { Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: []byte("major=1"), + Value: "major=1", }, }, { SecretSource: &options.SecretSource{ - Value: []byte("minor=2"), + Value: "minor=2", }, }, { SecretSource: &options.SecretSource{ - Value: []byte("patch=3"), + Value: "patch=3", }, }, }, diff --git a/pkg/http/http_suite_test.go b/pkg/http/http_suite_test.go index 19d4d3ff..219f26ea 100644 --- a/pkg/http/http_suite_test.go +++ b/pkg/http/http_suite_test.go @@ -48,10 +48,10 @@ var _ = BeforeSuite(func() { certOut := new(bytes.Buffer) Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed()) - ipv4CertDataSource.Value = certOut.Bytes() + ipv4CertDataSource.Value = certOut.String() keyOut := new(bytes.Buffer) Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed()) - ipv4KeyDataSource.Value = keyOut.Bytes() + ipv4KeyDataSource.Value = keyOut.String() }) By("Generating a ipv6 self-signed cert for TLS tests", func() { @@ -61,16 +61,16 @@ var _ = BeforeSuite(func() { certOut := new(bytes.Buffer) Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed()) - ipv6CertDataSource.Value = certOut.Bytes() + ipv6CertDataSource.Value = certOut.String() keyOut := new(bytes.Buffer) Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed()) - ipv6KeyDataSource.Value = keyOut.Bytes() + ipv6KeyDataSource.Value = keyOut.String() }) By("Setting up a http client", func() { - ipv4cert, err := tls.X509KeyPair(ipv4CertDataSource.Value, ipv4KeyDataSource.Value) + ipv4cert, err := tls.X509KeyPair([]byte(ipv4CertDataSource.Value), []byte(ipv4KeyDataSource.Value)) Expect(err).ToNot(HaveOccurred()) - ipv6cert, err := tls.X509KeyPair(ipv6CertDataSource.Value, ipv6KeyDataSource.Value) + ipv6cert, err := tls.X509KeyPair([]byte(ipv6CertDataSource.Value), []byte(ipv6KeyDataSource.Value)) Expect(err).ToNot(HaveOccurred()) ipv4certificate, err := x509.ParseCertificate(ipv4cert.Certificate[0]) diff --git a/pkg/http/server_test.go b/pkg/http/server_test.go index 8dfa13af..6584b757 100644 --- a/pkg/http/server_test.go +++ b/pkg/http/server_test.go @@ -234,7 +234,7 @@ var _ = Describe("Server", func() { SecureBindAddress: "127.0.0.1:0", TLS: &options.TLS{ Key: &options.SecretSource{ - Value: []byte("invalid"), + Value: "invalid", }, Cert: &ipv4CertDataSource, }, @@ -250,7 +250,7 @@ var _ = Describe("Server", func() { TLS: &options.TLS{ Key: &ipv4KeyDataSource, Cert: &options.SecretSource{ - Value: []byte("invalid"), + Value: "invalid", }, }, }, @@ -506,7 +506,7 @@ var _ = Describe("Server", func() { SecureBindAddress: "[::1]:0", TLS: &options.TLS{ Key: &options.SecretSource{ - Value: []byte("invalid"), + Value: "invalid", }, Cert: &ipv6CertDataSource, }, @@ -523,7 +523,7 @@ var _ = Describe("Server", func() { TLS: &options.TLS{ Key: &ipv6KeyDataSource, Cert: &options.SecretSource{ - Value: []byte("invalid"), + Value: "invalid", }, }, }, diff --git a/pkg/middleware/headers_test.go b/pkg/middleware/headers_test.go index 06440eea..b1848ae0 100644 --- a/pkg/middleware/headers_test.go +++ b/pkg/middleware/headers_test.go @@ -188,7 +188,7 @@ var _ = Describe("Headers Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), + Value: base64.StdEncoding.EncodeToString([]byte("basic-password")), FromEnv: "SECRET_ENV", }, }, @@ -260,7 +260,7 @@ var _ = Describe("Headers Suite", func() { Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: []byte("_oauth2_proxy=ey123123123"), + Value: "_oauth2_proxy=ey123123123", }, }, }, @@ -270,7 +270,7 @@ var _ = Describe("Headers Suite", func() { Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: []byte("oauth_user"), + Value: "oauth_user", }, }, }, @@ -416,7 +416,7 @@ var _ = Describe("Headers Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), + Value: base64.StdEncoding.EncodeToString([]byte("basic-password")), FromEnv: "SECRET_ENV", }, }, diff --git a/pkg/upstream/http.go b/pkg/upstream/http.go index 7a0e6e84..d9d8f152 100644 --- a/pkg/upstream/http.go +++ b/pkg/upstream/http.go @@ -137,12 +137,12 @@ func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler Pr // Change default duration for waiting for an upstream response if upstream.Timeout != nil { - transport.ResponseHeaderTimeout = upstream.Timeout.Duration() + transport.ResponseHeaderTimeout = *upstream.Timeout } // Configure options on the SingleHostReverseProxy if upstream.FlushInterval != nil { - proxy.FlushInterval = upstream.FlushInterval.Duration() + proxy.FlushInterval = *upstream.FlushInterval } else { proxy.FlushInterval = options.DefaultUpstreamFlushInterval } diff --git a/pkg/upstream/http_test.go b/pkg/upstream/http_test.go index 31476df0..df264c33 100644 --- a/pkg/upstream/http_test.go +++ b/pkg/upstream/http_test.go @@ -21,8 +21,8 @@ import ( ) var _ = Describe("HTTP Upstream Suite", func() { - defaultFlushInterval := options.Duration(options.DefaultUpstreamFlushInterval) - defaultTimeout := options.Duration(options.DefaultUpstreamTimeout) + defaultFlushInterval := options.DefaultUpstreamFlushInterval + defaultTimeout := options.DefaultUpstreamTimeout truth := true falsum := false @@ -57,9 +57,9 @@ var _ = Describe("HTTP Upstream Suite", func() { req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{}) rw := httptest.NewRecorder() - flush := options.Duration(1 * time.Second) + flush := 1 * time.Second - timeout := options.Duration(options.DefaultUpstreamTimeout) + timeout := options.DefaultUpstreamTimeout upstream := options.Upstream{ ID: in.id, @@ -373,11 +373,11 @@ var _ = Describe("HTTP Upstream Suite", func() { type newUpstreamTableInput struct { proxyWebSockets bool - flushInterval options.Duration + flushInterval time.Duration skipVerify bool sigData *options.SignatureData errorHandler func(http.ResponseWriter, *http.Request, error) - timeout options.Duration + timeout time.Duration disableKeepAlives bool } @@ -406,10 +406,10 @@ var _ = Describe("HTTP Upstream Suite", func() { proxy, ok := upstreamProxy.handler.(*httputil.ReverseProxy) Expect(ok).To(BeTrue()) - Expect(proxy.FlushInterval).To(Equal(in.flushInterval.Duration())) + Expect(proxy.FlushInterval).To(Equal(in.flushInterval)) transport, ok := proxy.Transport.(*http.Transport) Expect(ok).To(BeTrue()) - Expect(transport.ResponseHeaderTimeout).To(Equal(in.timeout.Duration())) + Expect(transport.ResponseHeaderTimeout).To(Equal(in.timeout)) Expect(proxy.ErrorHandler != nil).To(Equal(in.errorHandler != nil)) if in.skipVerify { Expect(transport.TLSClientConfig.InsecureSkipVerify).To(Equal(true)) @@ -428,7 +428,7 @@ var _ = Describe("HTTP Upstream Suite", func() { }), Entry("with a non standard flush interval", &newUpstreamTableInput{ proxyWebSockets: false, - flushInterval: options.Duration(5 * time.Second), + flushInterval: 5 * time.Second, skipVerify: false, sigData: nil, errorHandler: nil, @@ -466,7 +466,7 @@ var _ = Describe("HTTP Upstream Suite", func() { skipVerify: false, sigData: nil, errorHandler: nil, - timeout: options.Duration(5 * time.Second), + timeout: 5 * time.Second, }), Entry("with a DisableKeepAlives", &newUpstreamTableInput{ proxyWebSockets: false, @@ -483,8 +483,8 @@ var _ = Describe("HTTP Upstream Suite", func() { var proxyServer *httptest.Server BeforeEach(func() { - flush := options.Duration(1 * time.Second) - timeout := options.Duration(options.DefaultUpstreamTimeout) + flush := 1 * time.Second + timeout := options.DefaultUpstreamTimeout upstream := options.Upstream{ ID: "websocketProxy", PassHostHeader: &truth, diff --git a/pkg/validation/common_test.go b/pkg/validation/common_test.go index 9e873c35..bb7c2dd6 100644 --- a/pkg/validation/common_test.go +++ b/pkg/validation/common_test.go @@ -9,12 +9,12 @@ import ( ) var _ = Describe("Common", func() { - var validSecretSourceValue []byte + var validSecretSourceValue string const validSecretSourceEnv = "OAUTH2_PROXY_TEST_SECRET_SOURCE_ENV" var validSecretSourceFile string BeforeEach(func() { - validSecretSourceValue = []byte("This is a secret source value") + validSecretSourceValue = "This is a secret source value" Expect(os.Setenv(validSecretSourceEnv, "This is a secret source env")).To(Succeed()) tmp, err := os.CreateTemp("", "oauth2-proxy-secret-source-test") Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/validation/header.go b/pkg/validation/header.go index b1258144..713113f4 100644 --- a/pkg/validation/header.go +++ b/pkg/validation/header.go @@ -51,11 +51,9 @@ func validateHeaderValue(_ string, value options.HeaderValue) []string { func validateHeaderValueClaimSource(claim options.ClaimSource) []string { msgs := []string{} - if claim.Claim == "" { msgs = append(msgs, "claim should not be empty") } - if claim.BasicAuthPassword != nil { msgs = append(msgs, prefixValues("invalid basicAuthPassword: ", validateSecretSource(*claim.BasicAuthPassword))...) } diff --git a/pkg/validation/header_test.go b/pkg/validation/header_test.go index 2d9ef6dd..88849ea1 100644 --- a/pkg/validation/header_test.go +++ b/pkg/validation/header_test.go @@ -30,7 +30,7 @@ var _ = Describe("Headers", func() { Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("secret"))), + Value: base64.StdEncoding.EncodeToString([]byte("secret")), }, }, }, @@ -43,7 +43,7 @@ var _ = Describe("Headers", func() { ClaimSource: &options.ClaimSource{ Claim: "email", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("secret"))), + Value: base64.StdEncoding.EncodeToString([]byte("secret")), }, }, }, diff --git a/pkg/validation/upstreams.go b/pkg/validation/upstreams.go index bafed628..52facb4d 100644 --- a/pkg/validation/upstreams.go +++ b/pkg/validation/upstreams.go @@ -69,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.Duration() != options.DefaultUpstreamFlushInterval { + if upstream.FlushInterval != nil && *upstream.FlushInterval != 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 fe431d27..67991b76 100644 --- a/pkg/validation/upstreams_test.go +++ b/pkg/validation/upstreams_test.go @@ -14,7 +14,7 @@ var _ = Describe("Upstreams", func() { errStrings []string } - flushInterval := options.Duration(5 * time.Second) + flushInterval := 5 * time.Second staticCode200 := 200 truth := true