From 137decb1ec59efc2eaa44df49372567272ca6dcc Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Fri, 7 Nov 2025 23:26:00 +0100 Subject: [PATCH] adapting unit tests and fixing minor issues introduced with the derefing Signed-off-by: Jan Larwig --- pkg/apis/middleware/session.go | 5 +- pkg/apis/options/legacy_options.go | 44 +++++++++---- pkg/apis/options/legacy_options_test.go | 86 +++++++++++++++++-------- pkg/apis/options/providers.go | 2 +- pkg/middleware/jwt_session_test.go | 4 +- pkg/validation/sessions_test.go | 4 +- pkg/validation/upstreams.go | 8 +-- 7 files changed, 103 insertions(+), 50 deletions(-) diff --git a/pkg/apis/middleware/session.go b/pkg/apis/middleware/session.go index e17c0249f..afa56e9d6 100644 --- a/pkg/apis/middleware/session.go +++ b/pkg/apis/middleware/session.go @@ -41,7 +41,10 @@ func CreateTokenToSessionFunc(verify VerifyFunc) TokenToSessionFunc { claims.Email = claims.Subject } - if !ptr.Deref(claims.Verified, false) { + // Ensure email is verified + // If the email is not verified, return an error + // If the email_verified claim is missing, assume it is verified + if !ptr.Deref(claims.Verified, true) { return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email) } diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index ffd98bd1b..5f4efe578 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -96,6 +96,7 @@ func (l *LegacyOptions) ToOptions() (*Options, error) { return nil, fmt.Errorf("error converting provider: %v", err) } l.Options.Providers = providers + l.Options.EnsureDefaults() return &l.Options, nil } @@ -178,6 +179,10 @@ func (l *LegacyUpstreams) convert() (UpstreamConfig, error) { upstream.URI = "" upstream.InsecureSkipTLSVerify = ptr.Ptr(false) upstream.DisableKeepAlives = ptr.Ptr(false) + upstream.PassHostHeader = nil + upstream.ProxyWebSockets = nil + upstream.FlushInterval = nil + upstream.Timeout = nil case "unix": upstream.Path = "/" } @@ -284,7 +289,8 @@ func getBasicAuthHeader(preferEmailToUser bool, basicAuthPassword string) Header } return Header{ - Name: "Authorization", + Name: "Authorization", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ @@ -302,7 +308,8 @@ func getBasicAuthHeader(preferEmailToUser bool, basicAuthPassword string) Header func getPassUserHeaders(preferEmailToUser bool) []Header { headers := []Header{ { - Name: "X-Forwarded-Groups", + Name: "X-Forwarded-Groups", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ @@ -316,7 +323,8 @@ func getPassUserHeaders(preferEmailToUser bool) []Header { if preferEmailToUser { return append(headers, Header{ - Name: "X-Forwarded-User", + Name: "X-Forwarded-User", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ @@ -330,7 +338,8 @@ func getPassUserHeaders(preferEmailToUser bool) []Header { return append(headers, Header{ - Name: "X-Forwarded-User", + Name: "X-Forwarded-User", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ @@ -340,7 +349,8 @@ func getPassUserHeaders(preferEmailToUser bool) []Header { }, }, Header{ - Name: "X-Forwarded-Email", + Name: "X-Forwarded-Email", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ @@ -354,7 +364,8 @@ func getPassUserHeaders(preferEmailToUser bool) []Header { func getPassAccessTokenHeader() Header { return Header{ - Name: "X-Forwarded-Access-Token", + Name: "X-Forwarded-Access-Token", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ @@ -367,7 +378,8 @@ func getPassAccessTokenHeader() Header { func getAuthorizationHeader() Header { return Header{ - Name: "Authorization", + Name: "Authorization", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ @@ -381,7 +393,8 @@ func getAuthorizationHeader() Header { func getPreferredUsernameHeader() Header { return Header{ - Name: "X-Forwarded-Preferred-Username", + Name: "X-Forwarded-Preferred-Username", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ @@ -395,7 +408,8 @@ func getPreferredUsernameHeader() Header { func getXAuthRequestHeaders() []Header { headers := []Header{ { - Name: "X-Auth-Request-User", + Name: "X-Auth-Request-User", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ @@ -405,7 +419,8 @@ func getXAuthRequestHeaders() []Header { }, }, { - Name: "X-Auth-Request-Email", + Name: "X-Auth-Request-Email", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ @@ -415,7 +430,8 @@ func getXAuthRequestHeaders() []Header { }, }, { - Name: "X-Auth-Request-Preferred-Username", + Name: "X-Auth-Request-Preferred-Username", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ @@ -425,7 +441,8 @@ func getXAuthRequestHeaders() []Header { }, }, { - Name: "X-Auth-Request-Groups", + Name: "X-Auth-Request-Groups", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ @@ -441,7 +458,8 @@ func getXAuthRequestHeaders() []Header { func getXAuthRequestAccessTokenHeader() Header { return Header{ - Name: "X-Auth-Request-Access-Token", + Name: "X-Auth-Request-Access-Token", + PreserveRequestValue: ptr.Ptr(false), Values: []HeaderValue{ { ClaimSource: &ClaimSource{ diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index cc2a76e5b..65813c801 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -122,16 +122,19 @@ var _ = Describe("Legacy Options", func() { BindAddress: "127.0.0.1:4180", } - opts.Providers[0].ClientID = "oauth-proxy" opts.Providers[0].ID = "google=oauth-proxy" - opts.Providers[0].OIDCConfig.InsecureSkipNonce = ptr.Ptr(true) + opts.Providers[0].ClientID = "oauth-proxy" opts.Providers[0].OIDCConfig.AudienceClaims = []string{"aud"} opts.Providers[0].OIDCConfig.ExtraAudiences = []string{} + opts.Providers[0].OIDCConfig.InsecureSkipNonce = ptr.Ptr(true) + opts.Providers[0].OIDCConfig.InsecureSkipIssuerVerification = ptr.Ptr(false) opts.Providers[0].LoginURLParameters = []LoginURLParameter{ {Name: "approval_prompt", Default: []string{"force"}}, } converted, err := legacyOpts.ToOptions() + opts.EnsureDefaults() + Expect(err).ToNot(HaveOccurred()) Expect(converted).To(EqualOpts(opts)) }) @@ -944,37 +947,50 @@ var _ = Describe("Legacy Options", func() { {Name: "approval_prompt", Default: []string{"force"}}, } - defaultProvider := Provider{ - ID: "google=" + clientID, - ClientID: clientID, - Type: "google", - LoginURLParameters: defaultURLParams, + defaultOIDCOptions := OIDCOptions{ + SkipDiscovery: ptr.Ptr(false), + InsecureSkipNonce: ptr.Ptr(false), + InsecureAllowUnverifiedEmail: ptr.Ptr(false), + InsecureSkipIssuerVerification: ptr.Ptr(false), } + + defaultGoogleOptions := GoogleOptions{ + UseApplicationDefaultCredentials: ptr.Ptr(false), + } + defaultLegacyProvider := LegacyProvider{ ClientID: clientID, ProviderType: "google", } - defaultProviderWithPrompt := Provider{ - ID: "google=" + clientID, - ClientID: clientID, - Type: "google", - LoginURLParameters: []LoginURLParameter{ - {Name: "prompt", Default: []string{"switch_user"}}, - }, + defaultProvider := Provider{ + ID: "google=" + clientID, + ClientID: clientID, + Type: "google", + OIDCConfig: defaultOIDCOptions, + GoogleConfig: defaultGoogleOptions, + LoginURLParameters: defaultURLParams, + UseSystemTrustStore: ptr.Ptr(false), + SkipClaimsFromProfileURL: ptr.Ptr(false), } + defaultLegacyProviderWithPrompt := LegacyProvider{ ClientID: clientID, ProviderType: "google", Prompt: "switch_user", } - displayNameProvider := Provider{ - ID: "displayName", - Name: "displayName", - ClientID: clientID, - Type: "google", - LoginURLParameters: defaultURLParams, + defaultProviderWithPrompt := Provider{ + ID: "google=" + clientID, + ClientID: clientID, + Type: "google", + OIDCConfig: defaultOIDCOptions, + GoogleConfig: defaultGoogleOptions, + LoginURLParameters: []LoginURLParameter{ + {Name: "prompt", Default: []string{"switch_user"}}, + }, + UseSystemTrustStore: ptr.Ptr(false), + SkipClaimsFromProfileURL: ptr.Ptr(false), } displayNameLegacyProvider := LegacyProvider{ @@ -983,16 +999,32 @@ var _ = Describe("Legacy Options", func() { ProviderType: "google", } + displayNameProvider := Provider{ + ID: "displayName", + Name: "displayName", + ClientID: clientID, + Type: "google", + OIDCConfig: defaultOIDCOptions, + GoogleConfig: defaultGoogleOptions, + LoginURLParameters: defaultURLParams, + UseSystemTrustStore: ptr.Ptr(false), + SkipClaimsFromProfileURL: ptr.Ptr(false), + } + internalConfigProvider := Provider{ - ID: "google=" + clientID, - ClientID: clientID, - Type: "google", + ID: "google=" + clientID, + ClientID: clientID, + Type: "google", + OIDCConfig: defaultOIDCOptions, GoogleConfig: GoogleOptions{ - AdminEmail: "email@email.com", - ServiceAccountJSON: "test.json", - Groups: []string{"1", "2"}, + AdminEmail: "email@email.com", + ServiceAccountJSON: "test.json", + Groups: []string{"1", "2"}, + UseApplicationDefaultCredentials: ptr.Ptr(false), }, - LoginURLParameters: defaultURLParams, + LoginURLParameters: defaultURLParams, + UseSystemTrustStore: ptr.Ptr(false), + SkipClaimsFromProfileURL: ptr.Ptr(false), } internalConfigLegacyProvider := LegacyProvider{ diff --git a/pkg/apis/options/providers.go b/pkg/apis/options/providers.go index e8c392269..b93fb7adb 100644 --- a/pkg/apis/options/providers.go +++ b/pkg/apis/options/providers.go @@ -43,7 +43,7 @@ const ( // DefaultUseSystemTrustStore is the default value // for Provider.UseSystemTrustStore - DefaultUseSystemTrustStore bool = true + DefaultUseSystemTrustStore bool = false ) // OIDCAudienceClaims is the generic audience claim list used by the OIDC provider. diff --git a/pkg/middleware/jwt_session_test.go b/pkg/middleware/jwt_session_test.go index 12f30f5c9..7b724280d 100644 --- a/pkg/middleware/jwt_session_test.go +++ b/pkg/middleware/jwt_session_test.go @@ -304,7 +304,7 @@ Nnc3a3lGVWFCNUMxQnNJcnJMTWxka1dFaHluYmI4Ongtb2F1dGgtYmFzaWM=` authorizationHeader: fmt.Sprintf("Bearer %s", nonVerifiedToken), expectedErr: k8serrors.NewAggregate([]error{ errors.New("unable to verify bearer token"), - errors.New("oidc: malformed jwt: oidc: malformed jwt payload: illegal base64 data at input byte 8"), + errors.New("oidc: malformed jwt: illegal base64 data at input byte 8"), }), expectedSession: nil, }), @@ -317,7 +317,7 @@ Nnc3a3lGVWFCNUMxQnNJcnJMTWxka1dFaHluYmI4Ongtb2F1dGgtYmFzaWM=` authorizationHeader: "Basic ZXlKZm9vYmFyLmV5SmZvb2Jhci4xMjM0NWFzZGY6", expectedErr: k8serrors.NewAggregate([]error{ errors.New("unable to verify bearer token"), - errors.New("oidc: malformed jwt: oidc: malformed jwt payload: illegal base64 data at input byte 8"), + errors.New("oidc: malformed jwt: illegal base64 data at input byte 8"), }), expectedSession: nil, }), diff --git a/pkg/validation/sessions_test.go b/pkg/validation/sessions_test.go index 6f590ac5d..cb54c5714 100644 --- a/pkg/validation/sessions_test.go +++ b/pkg/validation/sessions_test.go @@ -193,8 +193,8 @@ var _ = Describe("Sessions", func() { unreachableRedisDelMsg = "unable to delete the redis initialization key: dial tcp 127.0.0.1:65535: connect: connection refused" unreachableSentinelSetMsg = "unable to set a redis initialization key: redis: all sentinels specified in configuration are unreachable: redis: nil" unrechableSentinelDelMsg = "unable to delete the redis initialization key: redis: all sentinels specified in configuration are unreachable: redis: nil" - refusedSentinelSetMsg = "unable to set a redis initialization key: redis: all sentinels specified in configuration are unreachable: dial tcp 127.0.0.1:65535: connect: connection refused" - refusedSentinelDelMsg = "unable to delete the redis initialization key: redis: all sentinels specified in configuration are unreachable: dial tcp 127.0.0.1:65535: connect: connection refused" + refusedSentinelSetMsg = "unable to set a redis initialization key: redis: all sentinels specified in configuration are unreachable: context deadline exceeded" + refusedSentinelDelMsg = "unable to delete the redis initialization key: redis: all sentinels specified in configuration are unreachable: context deadline exceeded" ) type redisStoreTableInput struct { diff --git a/pkg/validation/upstreams.go b/pkg/validation/upstreams.go index 4c566659d..9a3819180 100644 --- a/pkg/validation/upstreams.go +++ b/pkg/validation/upstreams.go @@ -70,13 +70,13 @@ func validateStaticUpstream(upstream options.Upstream) []string { if ptr.Deref(upstream.InsecureSkipTLSVerify, options.DefaultUpsteamInsecureSkipTLSVerify) { msgs = append(msgs, fmt.Sprintf("upstream %q has insecureSkipTLSVerify, but is a static upstream, this will have no effect.", upstream.ID)) } - if ptr.Deref(upstream.FlushInterval, options.DefaultUpstreamFlushInterval) != 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 ptr.Deref(upstream.PassHostHeader, options.DefaultUpstreamPassHostHeader) { + if upstream.PassHostHeader != nil { msgs = append(msgs, fmt.Sprintf("upstream %q has passHostHeader, but is a static upstream, this will have no effect.", upstream.ID)) } - if ptr.Deref(upstream.ProxyWebSockets, options.DefaultUpstreamProxyWebSockets) { + if upstream.ProxyWebSockets != nil { msgs = append(msgs, fmt.Sprintf("upstream %q has proxyWebSockets, but is a static upstream, this will have no effect.", upstream.ID)) } @@ -92,7 +92,7 @@ func validateUpstreamURI(upstream options.Upstream) []string { } // Checks after this only make sense the upstream is not static - if !ptr.Deref(upstream.Static, options.DefaultUpstreamStatic) { + if ptr.Deref(upstream.Static, options.DefaultUpstreamStatic) { return msgs }