feat: add ensure defaults to all migrated structs

Signed-off-by: Jan Larwig <jan@larwig.com>

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
 }
 

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),
 				},

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)
 		}
 

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

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
+}

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 = "/"

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))
 		})
 	})
 

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()
+}

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)
+	}
+}

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
+}

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)
+	}
+}

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
 	}
 

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 {

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
 	}