diff --git a/main.go b/main.go index 12ca27393..7e18b95fc 100644 --- a/main.go +++ b/main.go @@ -20,7 +20,7 @@ func main() { // Because we parse early to determine alpha vs legacy config, we have to // ignore any unknown flags for now - configFlagSet.ParseErrorsWhitelist.UnknownFlags = true + configFlagSet.ParseErrorsAllowlist.UnknownFlags = true 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)") @@ -78,6 +78,8 @@ func loadConfiguration(config, yamlConfig string, extraFlags *pflag.FlagSet, arg return loadYamlOptions(yamlConfig, config, extraFlags, args) } + opts.EnsureDefaults() + return opts, nil } @@ -117,7 +119,7 @@ func loadYamlOptions(yamlConfig, config string, extraFlags *pflag.FlagSet, args return nil, fmt.Errorf("failed to load alpha options: %v", err) } - alphaOpts.MergeInto(opts) + alphaOpts.MergeOptionsWithDefaults(opts) return opts, nil } diff --git a/main_test.go b/main_test.go index b9ec39fb5..0323c838c 100644 --- a/main_test.go +++ b/main_test.go @@ -3,7 +3,6 @@ package main import ( "errors" "os" - "time" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" . "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options/testutil" @@ -114,10 +113,6 @@ cookie_secure="false" redirect_url="http://localhost:4180/oauth2/callback" ` - durationPtr := func(d time.Duration) *time.Duration { - return &d - } - testExpectedOptions := func() *options.Options { opts, err := options.NewLegacyOptions().ToOptions() Expect(err).ToNot(HaveOccurred()) @@ -133,10 +128,10 @@ redirect_url="http://localhost:4180/oauth2/callback" ID: "/", Path: "/", URI: "http://httpbin", - FlushInterval: durationPtr(options.DefaultUpstreamFlushInterval), + FlushInterval: ptr.Ptr(options.DefaultUpstreamFlushInterval), PassHostHeader: ptr.Ptr(true), ProxyWebSockets: ptr.Ptr(true), - Timeout: durationPtr(options.DefaultUpstreamTimeout), + Timeout: ptr.Ptr(options.DefaultUpstreamTimeout), InsecureSkipTLSVerify: ptr.Ptr(false), DisableKeepAlives: ptr.Ptr(false), }, diff --git a/pkg/apis/middleware/session.go b/pkg/apis/middleware/session.go index 9fcd974b8..e17c0249f 100644 --- a/pkg/apis/middleware/session.go +++ b/pkg/apis/middleware/session.go @@ -6,6 +6,7 @@ import ( "github.com/coreos/go-oidc/v3/oidc" sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" ) // TokenToSessionFunc takes a raw ID Token and converts it into a SessionState. @@ -40,7 +41,7 @@ func CreateTokenToSessionFunc(verify VerifyFunc) TokenToSessionFunc { claims.Email = claims.Subject } - if claims.Verified != nil && !*claims.Verified { + if !ptr.Deref(claims.Verified, false) { return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email) } diff --git a/pkg/apis/options/alpha_options.go b/pkg/apis/options/alpha_options.go index 0c78359a6..33daf17fe 100644 --- a/pkg/apis/options/alpha_options.go +++ b/pkg/apis/options/alpha_options.go @@ -65,9 +65,9 @@ func (a *AlphaOptions) ExtractFrom(opts *Options) { a.Providers = opts.Providers } -// MergeInto replaces alpha options in the Options struct with the values -// from the AlphaOptions -func (a *AlphaOptions) MergeInto(opts *Options) { +// MergeOptionsWithDefaults replaces alpha options in the Options struct +// with the values from the AlphaOptions and ensures the defaults +func (a *AlphaOptions) MergeOptionsWithDefaults(opts *Options) { opts.UpstreamServers = a.UpstreamConfig opts.InjectRequestHeaders = a.InjectRequestHeaders opts.InjectResponseHeaders = a.InjectResponseHeaders diff --git a/pkg/apis/options/header.go b/pkg/apis/options/header.go index 048b19741..a47e8a952 100644 --- a/pkg/apis/options/header.go +++ b/pkg/apis/options/header.go @@ -1,5 +1,7 @@ package options +import "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" + // Header represents an individual header that will be added to a request or // response header. type Header struct { @@ -53,3 +55,28 @@ type ClaimSource struct { // basicAuthPassword will be used as the password value. BasicAuthPassword *SecretSource `yaml:"basicAuthPassword,omitempty"` } + +// EnsureDefaults sets any default values for Header fields. +func (h *Header) EnsureDefaults() { + if h.PreserveRequestValue == nil { + h.PreserveRequestValue = ptr.Ptr(false) + } + for i := range h.Values { + h.Values[i].EnsureDefaults() + } +} + +// EnsureDefaults sets any default values for HeaderValue fields. +func (hv *HeaderValue) EnsureDefaults() { + if hv.ClaimSource != nil { + hv.ClaimSource.EnsureDefaults() + } + if hv.SecretSource != nil { + hv.SecretSource.EnsureDefaults() + } +} + +// EnsureDefaults sets any default values for ClaimSource fields. +func (hc *ClaimSource) EnsureDefaults() { + // No defaults to set currently +} diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index 672d6c63f..ffd98bd1b 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -177,10 +177,6 @@ func (l *LegacyUpstreams) convert() (UpstreamConfig, error) { // Force defaults compatible with static responses upstream.URI = "" upstream.InsecureSkipTLSVerify = ptr.Ptr(false) - upstream.PassHostHeader = nil - upstream.ProxyWebSockets = nil - upstream.FlushInterval = nil - upstream.Timeout = nil upstream.DisableKeepAlives = ptr.Ptr(false) case "unix": upstream.Path = "/" diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index 4348021b1..cc2a76e5b 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -3,6 +3,7 @@ package options import ( "time" + . "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options/testutil" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -132,7 +133,7 @@ var _ = Describe("Legacy Options", func() { converted, err := legacyOpts.ToOptions() Expect(err).ToNot(HaveOccurred()) - Expect(converted).To(Equal(opts)) + Expect(converted).To(EqualOpts(opts)) }) }) diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index 8fa72c7c5..b57d5aeda 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -168,3 +168,23 @@ func NewFlagSet() *pflag.FlagSet { return flagSet } + +// EnsureDefaults configures the defaults for all options +// to ensure no unexpected empty strings for enum types or nils for booleans +func (o *Options) EnsureDefaults() { + o.Providers.EnsureDefaults() + o.UpstreamServers.EnsureDefaults() + + for i := range o.InjectRequestHeaders { + o.InjectRequestHeaders[i].EnsureDefaults() + } + for i := range o.InjectResponseHeaders { + o.InjectResponseHeaders[i].EnsureDefaults() + } + + // TBD: Uncomment as we add EnsureDefaults methods + // o.Cookie.EnsureDefaults() + // o.Session.EnsureDefaults() + // o.Templates.EnsureDefaults() + // o.Logging.EnsureDefaults() +} diff --git a/pkg/apis/options/providers.go b/pkg/apis/options/providers.go index 027464de4..ed836656f 100644 --- a/pkg/apis/options/providers.go +++ b/pkg/apis/options/providers.go @@ -289,6 +289,7 @@ type LoginGovOptions struct { PubJWKURL string `yaml:"pubjwkURL,omitempty"` } +// Legacy default providers configuration func providerDefaults() Providers { providers := Providers{ { @@ -310,3 +311,72 @@ func providerDefaults() Providers { } return providers } + +// EnsureDefaults sets any default values for Providers fields. +func (p Providers) EnsureDefaults() { + for i := range p { + p[i].EnsureDefaults() + } +} + +// EnsureDefaults sets any default values for Provider fields. +func (p *Provider) EnsureDefaults() { + if p.SkipClaimsFromProfileURL == nil { + p.SkipClaimsFromProfileURL = ptr.Ptr(false) + } + if p.UseSystemTrustStore == nil { + p.UseSystemTrustStore = ptr.Ptr(true) + } + + p.OIDCConfig.EnsureDefaults() + p.MicrosoftEntraIDConfig.EnsureDefaults() + p.ADFSConfig.EnsureDefaults() + p.GoogleConfig.EnsureDefaults() +} + +// EnsureDefaults sets any default values for OIDCOptions fields. +func (o *OIDCOptions) EnsureDefaults() { + // Ensure OIDC defaults + if o.InsecureAllowUnverifiedEmail == nil { + o.InsecureAllowUnverifiedEmail = ptr.Ptr(false) + } + if o.InsecureSkipNonce == nil { + o.InsecureSkipNonce = ptr.Ptr(true) + } + if o.SkipDiscovery == nil { + o.SkipDiscovery = ptr.Ptr(false) + } + if o.UserIDClaim == "" { + o.UserIDClaim = OIDCEmailClaim + } + if o.EmailClaim == "" { + o.EmailClaim = OIDCEmailClaim + } + if o.GroupsClaim == "" { + o.GroupsClaim = OIDCGroupsClaim + } + if len(o.AudienceClaims) == 0 { + o.AudienceClaims = OIDCAudienceClaims + } +} + +// EnsureDefaults sets any default values for MicrosoftEntraIDOptions fields. +func (me *MicrosoftEntraIDOptions) EnsureDefaults() { + if me.FederatedTokenAuth == nil { + me.FederatedTokenAuth = ptr.Ptr(false) + } +} + +// EnsureDefaults sets any default values for ADFSOptions fields. +func (a *ADFSOptions) EnsureDefaults() { + if a.SkipScope == nil { + a.SkipScope = ptr.Ptr(false) + } +} + +// EnsureDefaults sets any default values for GoogleOptions fields. +func (g *GoogleOptions) EnsureDefaults() { + if g.UseApplicationDefaultCredentials == nil { + g.UseApplicationDefaultCredentials = ptr.Ptr(false) + } +} diff --git a/pkg/apis/options/secret_source.go b/pkg/apis/options/secret_source.go index 848f16351..8b7833426 100644 --- a/pkg/apis/options/secret_source.go +++ b/pkg/apis/options/secret_source.go @@ -12,3 +12,8 @@ type SecretSource struct { // FromFile expects a path to a file containing the secret value. FromFile string `yaml:"fromFile,omitempty"` } + +// EnsureDefaults sets any default values for SecretSource fields. +func (ss *SecretSource) EnsureDefaults() { + // No defaults to set currently +} diff --git a/pkg/apis/options/upstreams.go b/pkg/apis/options/upstreams.go index f12a4bf3f..05c12e033 100644 --- a/pkg/apis/options/upstreams.go +++ b/pkg/apis/options/upstreams.go @@ -1,6 +1,10 @@ package options -import "time" +import ( + "time" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" +) const ( // DefaultUpstreamFlushInterval is the default value for the Upstream FlushInterval. @@ -98,3 +102,38 @@ type Upstream struct { // Defaults to false. DisableKeepAlives *bool `yaml:"disableKeepAlives,omitempty"` } + +// EnsureDefaults sets any default values for UpstreamConfig fields. +func (uc *UpstreamConfig) EnsureDefaults() { + if uc.ProxyRawPath == nil { + uc.ProxyRawPath = ptr.Ptr(false) + } + for i := range uc.Upstreams { + uc.Upstreams[i].EnsureDefaults() + } +} + +// EnsureDefaults sets any default values for Upstream fields. +func (u *Upstream) EnsureDefaults() { + if u.InsecureSkipTLSVerify == nil { + u.InsecureSkipTLSVerify = ptr.Ptr(false) + } + if u.Static == nil { + u.Static = ptr.Ptr(false) + } + if u.FlushInterval == nil { + u.FlushInterval = ptr.Ptr(DefaultUpstreamFlushInterval) + } + if u.PassHostHeader == nil { + u.PassHostHeader = ptr.Ptr(true) + } + if u.ProxyWebSockets == nil { + u.ProxyWebSockets = ptr.Ptr(true) + } + if u.Timeout == nil { + u.Timeout = ptr.Ptr(DefaultUpstreamTimeout) + } + if u.DisableKeepAlives == nil { + u.DisableKeepAlives = ptr.Ptr(false) + } +} diff --git a/pkg/upstream/http.go b/pkg/upstream/http.go index 38e0e7a96..59580ee33 100644 --- a/pkg/upstream/http.go +++ b/pkg/upstream/http.go @@ -11,6 +11,7 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/authentication/hmacauth" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" ) const ( @@ -53,8 +54,8 @@ func newHTTPUpstreamProxy(upstream options.Upstream, u *url.URL, sigData *option // Set up a WebSocket proxy if required var wsProxy http.Handler - if upstream.ProxyWebSockets == nil || *upstream.ProxyWebSockets { - wsProxy = newWebSocketReverseProxy(u, *upstream.InsecureSkipTLSVerify) + if *upstream.ProxyWebSockets { + wsProxy = newWebSocketReverseProxy(u, upstream.InsecureSkipTLSVerify) } var auth hmacauth.HmacAuth @@ -156,7 +157,7 @@ func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler Pr // Ensure we always pass the original request path setProxyDirector(proxy) - if upstream.PassHostHeader != nil && !*upstream.PassHostHeader { + if upstream.PassHostHeader != nil && !(*upstream.PassHostHeader) { setProxyUpstreamHostHeader(proxy, target) } @@ -200,14 +201,14 @@ func setProxyDirector(proxy *httputil.ReverseProxy) { } // newWebSocketReverseProxy creates a new reverse proxy for proxying websocket connections. -func newWebSocketReverseProxy(u *url.URL, skipTLSVerify bool) http.Handler { +func newWebSocketReverseProxy(u *url.URL, skipTLSVerify *bool) http.Handler { wsProxy := httputil.NewSingleHostReverseProxy(u) // Inherit default transport options from Go's stdlib transport := http.DefaultTransport.(*http.Transport).Clone() /* #nosec G402 */ - if skipTLSVerify { + if ptr.Deref(skipTLSVerify, false) { transport.TLSClientConfig.InsecureSkipVerify = true } diff --git a/pkg/validation/providers.go b/pkg/validation/providers.go index 3abf69d40..345274d89 100644 --- a/pkg/validation/providers.go +++ b/pkg/validation/providers.go @@ -5,7 +5,6 @@ import ( "os" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" ) // validateProviders is the initial validation migration for multiple providrers @@ -97,9 +96,9 @@ func validateGoogleConfig(provider options.Provider) []string { hasAdminEmail := provider.GoogleConfig.AdminEmail != "" hasSAJSON := provider.GoogleConfig.ServiceAccountJSON != "" - useADC := ptr.Deref(provider.GoogleConfig.UseApplicationDefaultCredentials, false) + useADC := provider.GoogleConfig.UseApplicationDefaultCredentials - if !hasAdminEmail && !hasSAJSON && !useADC { + if !hasAdminEmail && !hasSAJSON && !(*useADC) { return msgs } @@ -108,7 +107,7 @@ func validateGoogleConfig(provider options.Provider) []string { } _, err := os.Stat(provider.GoogleConfig.ServiceAccountJSON) - if !useADC { + if !(*useADC) { if !hasSAJSON { msgs = append(msgs, "missing setting: google-service-account-json or google-use-application-default-credentials") } else if err != nil { diff --git a/pkg/validation/upstreams.go b/pkg/validation/upstreams.go index 05d441e3a..2750e22c9 100644 --- a/pkg/validation/upstreams.go +++ b/pkg/validation/upstreams.go @@ -54,12 +54,12 @@ func validateUpstream(upstream options.Upstream, ids, paths map[string]struct{}) func validateStaticUpstream(upstream options.Upstream) []string { msgs := []string{} - if !*upstream.Static && upstream.StaticCode != nil { + if !(*upstream.Static) && upstream.StaticCode != nil { msgs = append(msgs, fmt.Sprintf("upstream %q has staticCode (%d), but is not a static upstream, set 'static' for a static response", upstream.ID, *upstream.StaticCode)) } // Checks after this only make sense when the upstream is static - if !*upstream.Static { + if !(*upstream.Static) { return msgs } @@ -72,10 +72,10 @@ func validateStaticUpstream(upstream options.Upstream) []string { 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 { + if *upstream.PassHostHeader { msgs = append(msgs, fmt.Sprintf("upstream %q has passHostHeader, but is a static upstream, this will have no effect.", upstream.ID)) } - if upstream.ProxyWebSockets != nil { + if *upstream.ProxyWebSockets { msgs = append(msgs, fmt.Sprintf("upstream %q has proxyWebSockets, but is a static upstream, this will have no effect.", upstream.ID)) } @@ -85,7 +85,7 @@ func validateStaticUpstream(upstream options.Upstream) []string { func validateUpstreamURI(upstream options.Upstream) []string { msgs := []string{} - if !*upstream.Static && upstream.URI == "" { + if !(*upstream.Static) && upstream.URI == "" { msgs = append(msgs, fmt.Sprintf("upstream %q has empty uri: uris are required for all non-static upstreams", upstream.ID)) return msgs }