UPSTREAM: <carry>: use new access token inactivity timeout field.

openshift · Sep 8, 2021 · 3b09b50 · 3b09b50
1 parent f856f0f
commit 3b09b50
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 19 deletions.
diff --git a/openshift-kube-apiserver/admission/customresourcevalidation/oauth/validate_idp.go b/openshift-kube-apiserver/admission/customresourcevalidation/oauth/validate_idp.go
@@ -72,10 +72,10 @@ func validateOAuthSpec(spec configv1.OAuthSpec) field.ErrorList {
 	}
 
 	// TODO move to ValidateTokenConfig
-	timeout := spec.TokenConfig.AccessTokenInactivityTimeoutSeconds
-	if timeout > 0 && timeout < MinimumInactivityTimeoutSeconds {
+	timeout := spec.TokenConfig.AccessTokenInactivityTimeout
+	if timeout != nil && timeout.Seconds() < MinimumInactivityTimeoutSeconds {
 		errs = append(errs, field.Invalid(
-			specPath.Child("tokenConfig", "accessTokenInactivityTimeoutSeconds"), timeout,
+			specPath.Child("tokenConfig", "accessTokenInactivityTimeout"), timeout,
 			fmt.Sprintf("the minimum acceptable token timeout value is %d seconds",
 				MinimumInactivityTimeoutSeconds)))
 	}

diff --git a/openshift-kube-apiserver/admission/customresourcevalidation/oauth/validate_idp_test.go b/openshift-kube-apiserver/admission/customresourcevalidation/oauth/validate_idp_test.go
@@ -4,7 +4,9 @@ import (
 	"fmt"
 	"reflect"
 	"testing"
+	"time"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 
 	configv1 "github.com/openshift/api/config/v1"
@@ -143,17 +145,20 @@ func TestValidateOAuthSpec(t *testing.T) {
 			args: args{
 				spec: configv1.OAuthSpec{
 					TokenConfig: configv1.TokenConfig{
-						AccessTokenInactivityTimeoutSeconds: -50,
+						AccessTokenInactivityTimeout: &metav1.Duration{Duration: -50 * time.Second},
 					},
 				},
 			},
+			want: field.ErrorList{
+				field.Invalid(field.NewPath("spec", "tokenConfig", "accessTokenInactivityTimeout"), metav1.Duration{Duration: -50 * time.Second}, fmt.Sprintf("the minimum acceptable token timeout value is %d seconds", MinimumInactivityTimeoutSeconds)),
+			},
 		},
 		{
 			name: "positive token inactivity timeout",
 			args: args{
 				spec: configv1.OAuthSpec{
 					TokenConfig: configv1.TokenConfig{
-						AccessTokenInactivityTimeoutSeconds: 32578,
+						AccessTokenInactivityTimeout: &metav1.Duration{Duration: 32578 * time.Second},
 					},
 				},
 			},
@@ -163,22 +168,25 @@ func TestValidateOAuthSpec(t *testing.T) {
 			args: args{
 				spec: configv1.OAuthSpec{
 					TokenConfig: configv1.TokenConfig{
-						AccessTokenInactivityTimeoutSeconds: 0,
+						AccessTokenInactivityTimeout: &metav1.Duration{Duration: 0},
 					},
 				},
 			},
+			want: field.ErrorList{
+				field.Invalid(field.NewPath("spec", "tokenConfig", "accessTokenInactivityTimeout"), metav1.Duration{Duration: 0 * time.Second}, fmt.Sprintf("the minimum acceptable token timeout value is %d seconds", MinimumInactivityTimeoutSeconds)),
+			},
 		},
 		{
 			name: "token inactivity timeout lower than the api constant minimum",
 			args: args{
 				spec: configv1.OAuthSpec{
 					TokenConfig: configv1.TokenConfig{
-						AccessTokenInactivityTimeoutSeconds: 250,
+						AccessTokenInactivityTimeout: &metav1.Duration{Duration: 250 * time.Second},
 					},
 				},
 			},
 			want: field.ErrorList{
-				field.Invalid(field.NewPath("spec", "tokenConfig", "accessTokenInactivityTimeoutSeconds"), 250, fmt.Sprintf("the minimum acceptable token timeout value is %d seconds", MinimumInactivityTimeoutSeconds)),
+				field.Invalid(field.NewPath("spec", "tokenConfig", "accessTokenInactivityTimeout"), metav1.Duration{Duration: 250 * time.Second}, fmt.Sprintf("the minimum acceptable token timeout value is %d seconds", MinimumInactivityTimeoutSeconds)),
 			},
 		},
 		{
@@ -246,8 +254,8 @@ func TestValidateOAuthSpec(t *testing.T) {
 						},
 					},
 					TokenConfig: configv1.TokenConfig{
-						AccessTokenInactivityTimeoutSeconds: -1,
-						AccessTokenMaxAgeSeconds:            216000,
+						AccessTokenInactivityTimeout: &metav1.Duration{Duration: 300 * time.Second},
+						AccessTokenMaxAgeSeconds:     216000,
 					},
 					Templates: configv1.OAuthTemplates{
 						Login:             configv1.SecretNameReference{Name: "my-login-template"},

diff --git a/pkg/kubeapiserver/authenticator/patch_authenticator.go b/pkg/kubeapiserver/authenticator/patch_authenticator.go
@@ -54,18 +54,22 @@ func AddOAuthServerAuthenticatorIfNeeded(tokenAuthenticators []authenticator.Tok
 		panic(err)
 	}
 
-	// add our oauth token validator
-	validators := []oauth.OAuthTokenValidator{oauth.NewExpirationValidator(), oauth.NewUIDValidator()}
+	// Tokens are valid for their lifetime unless this value is overridden.
+	tokenTimeout := int32(0)
 	if enablement.OpenshiftConfig().OAuthConfig != nil {
-		if inactivityTimeout := enablement.OpenshiftConfig().OAuthConfig.TokenConfig.AccessTokenInactivityTimeoutSeconds; inactivityTimeout != nil {
-			timeoutValidator := oauth.NewTimeoutValidator(oauthClient.OauthV1().OAuthAccessTokens(), oauthInformer.Oauth().V1().OAuthClients().Lister(), *inactivityTimeout, oauthvalidation.MinimumInactivityTimeoutSeconds)
-			validators = append(validators, timeoutValidator)
-			enablement.AddPostStartHookOrDie("openshift.io-TokenTimeoutUpdater", func(context genericapiserver.PostStartHookContext) error {
-				go timeoutValidator.Run(context.StopCh)
-				return nil
-			})
+		if inactivityTimeout := enablement.OpenshiftConfig().OAuthConfig.TokenConfig.AccessTokenInactivityTimeout; inactivityTimeout != nil {
+			tokenTimeout = int32(inactivityTimeout.Seconds())
 		}
 	}
+
+	timeoutValidator := oauth.NewTimeoutValidator(oauthClient.OauthV1().OAuthAccessTokens(), oauthInformer.Oauth().V1().OAuthClients().Lister(), tokenTimeout, oauthvalidation.MinimumInactivityTimeoutSeconds)
+	// add our oauth token validator
+	validators := []oauth.OAuthTokenValidator{oauth.NewExpirationValidator(), oauth.NewUIDValidator(), timeoutValidator}
+	enablement.AddPostStartHookOrDie("openshift.io-TokenTimeoutUpdater", func(context genericapiserver.PostStartHookContext) error {
+		go timeoutValidator.Run(context.StopCh)
+		return nil
+	})
+
 	enablement.AddPostStartHookOrDie("openshift.io-StartOAuthInformers", func(context genericapiserver.PostStartHookContext) error {
 		go oauthInformer.Start(context.StopCh)
 		go userInformer.Start(context.StopCh)