Make ID token lifetimes configurable on OIDCClient resources

internal/federationdomain/clientregistry/clientregistry.go

@@ -1,4 +1,4 @@
-// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 // Package clientregistry defines Pinniped's OAuth2/OIDC clients.
@@ -27,6 +27,12 @@ import (
 // or a dynamic client defined by an OIDCClient CR.
 type Client struct {
 	fosite.DefaultOpenIDConnectClient
+
+	// Optionally provide a lifetime for ID tokens that result from authcode exchanges (initial logins)
+	// and refresh grants for this specific client. This will not impact the lifetime of ID tokens created
+	// via RFC8693 token exchange. When zero, the ID token lifetime will be determined by the defaults
+	// for the FederationDomain.
+	IDTokenLifetimeConfiguration time.Duration
 }
 
 // Client implements the base, OIDC, and response_mode client interfaces of Fosite.
@@ -165,10 +171,19 @@ func PinnipedCLI() *Client {
 			TokenEndpointAuthSigningAlgorithm: coreosoidc.RS256,
 			TokenEndpointAuthMethod:           "none",
 		},
+		IDTokenLifetimeConfiguration: 0, // never override the default timeouts for this client
 	}
 }
 
 func oidcClientCRToFositeClient(oidcClient *configv1alpha1.OIDCClient, clientSecrets []string) *Client {
+	// Allow the user to optionally override the default timeouts for these clients.
+	idTokenLifetimeOverrideInSeconds := oidcClient.Spec.TokenLifetimes.IDTokenSeconds
+	var idTokenLifetime time.Duration
+	if idTokenLifetimeOverrideInSeconds != nil {
+		// It should be safe to cast this int32 to time.Duration, because time.Duration is an int64.
+		idTokenLifetime = time.Duration(*(oidcClient.Spec.TokenLifetimes.IDTokenSeconds)) * time.Second
+	}
+
 	return &Client{
 		DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{
 			DefaultClient: &fosite.DefaultClient{
@@ -192,6 +207,7 @@ func oidcClientCRToFositeClient(oidcClient *configv1alpha1.OIDCClient, clientSec
 			TokenEndpointAuthSigningAlgorithm: coreosoidc.RS256,
 			TokenEndpointAuthMethod:           "client_secret_basic",
 		},
+		IDTokenLifetimeConfiguration: idTokenLifetime,
 	}
 }
 

internal/federationdomain/endpoints/token/token_handler.go

@@ -9,6 +9,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"time"
 
 	"github.com/ory/fosite"
 	errorsx "github.com/pkg/errors"
@@ -19,8 +20,10 @@ import (
 
 	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	"go.pinniped.dev/internal/federationdomain/federationdomainproviders"
+	"go.pinniped.dev/internal/federationdomain/idtokenlifespan"
 	"go.pinniped.dev/internal/federationdomain/oidc"
 	"go.pinniped.dev/internal/federationdomain/resolvedprovider"
+	"go.pinniped.dev/internal/federationdomain/timeouts"
 	"go.pinniped.dev/internal/httputil/httperr"
 	"go.pinniped.dev/internal/idtransform"
 	"go.pinniped.dev/internal/plog"
@@ -30,6 +33,8 @@ import (
 func NewHandler(
 	idpLister federationdomainproviders.FederationDomainIdentityProvidersListerI,
 	oauthHelper fosite.OAuth2Provider,
+	overrideAccessTokenLifespan timeouts.OverrideLifespan,
+	overrideIDTokenLifespan timeouts.OverrideLifespan,
 ) http.Handler {
 	return httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
 		session := psession.NewPinnipedSession()
@@ -66,7 +71,17 @@ func NewHandler(
 			}
 		}
 
-		accessResponse, err := oauthHelper.NewAccessResponse(r.Context(), accessRequest)
+		// Lifetimes of the access and refresh tokens are determined by the above call to NewAccessRequest.
+		// Depending on the request, sometimes override the default access token lifespan.
+		maybeOverrideDefaultAccessTokenLifetime(overrideAccessTokenLifespan, accessRequest)
+
+		// Create the token response.
+		// The lifetime of the ID token will be determined inside the call NewAccessResponse.
+		// Depending on the request, sometimes override the default ID token lifespan by putting
+		// the override value onto the context.
+		accessResponse, err := oauthHelper.NewAccessResponse(
+			maybeOverrideDefaultIDTokenLifetime(r.Context(), overrideIDTokenLifespan, accessRequest),
+			accessRequest)
 		if err != nil {
 			plog.Info("token response error", oidc.FositeErrorForLog(err)...)
 			oauthHelper.WriteAccessError(r.Context(), w, accessRequest, err)
@@ -79,6 +94,19 @@ func NewHandler(
 	})
 }
 
+func maybeOverrideDefaultAccessTokenLifetime(overrideAccessTokenLifespan timeouts.OverrideLifespan, accessRequest fosite.AccessRequester) {
+	if doOverride, newLifespan := overrideAccessTokenLifespan(accessRequest); doOverride {
+		accessRequest.GetSession().SetExpiresAt(fosite.AccessToken, time.Now().UTC().Add(newLifespan).Round(time.Second))
+	}
+}
+
+func maybeOverrideDefaultIDTokenLifetime(baseCtx context.Context, overrideIDTokenLifespan timeouts.OverrideLifespan, accessRequest fosite.AccessRequester) context.Context {
+	if doOverride, newLifespan := overrideIDTokenLifespan(accessRequest); doOverride {
+		return idtokenlifespan.OverrideIDTokenLifespanInContext(baseCtx, newLifespan)
+	}
+	return baseCtx
+}
+
 func errMissingUpstreamSessionInternalError() *fosite.RFC6749Error {
 	return &fosite.RFC6749Error{
 		ErrorField:       "error",

internal/federationdomain/endpointsmanager/manager.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package endpointsmanager
@@ -161,6 +161,8 @@ func (m *Manager) SetFederationDomains(federationDomains ...*federationdomainpro
 		m.providerHandlers[(issuerHostWithPath + oidc.TokenEndpointPath)] = token.NewHandler(
 			idpLister,
 			oauthHelperWithKubeStorage,
+			timeoutsConfiguration.OverrideDefaultAccessTokenLifespan,
+			timeoutsConfiguration.OverrideDefaultIDTokenLifespan,
 		)
 
 		m.providerHandlers[(issuerHostWithPath + oidc.PinnipedLoginPath)] = login.NewHandler(

internal/federationdomain/idtokenlifespan/idtoken_lifespan.go

@@ -0,0 +1,55 @@
+// Copyright 2024 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package idtokenlifespan
+
+import (
+	"context"
+	"time"
+
+	"github.com/ory/fosite"
+	"github.com/ory/fosite/compose"
+	"github.com/ory/fosite/handler/openid"
+)
+
+// contextKey type is unexported to prevent collisions.
+type contextKey int
+
+const idTokenLifetimeOverrideKey contextKey = iota
+
+// OpenIDConnectExplicitFactory is similar to the function of the same name in the fosite compose package,
+// except it allows wrapping the IDTokenLifespanProvider.
+func OpenIDConnectExplicitFactory(config fosite.Configurator, storage interface{}, strategy interface{}) interface{} {
+	openIDConnectExplicitHandler := compose.OpenIDConnectExplicitFactory(config, storage, strategy).(*openid.OpenIDConnectExplicitHandler)
+	// Overwrite the config with a wrapper around the fosite.IDTokenLifespanProvider.
+	openIDConnectExplicitHandler.Config = &contextAwareIDTokenLifespanProvider{DelegateConfig: config}
+	return openIDConnectExplicitHandler
+}
+
+// OpenIDConnectRefreshFactory is similar to the function of the same name in the fosite compose package,
+// except it allows wrapping the IDTokenLifespanProvider.
+func OpenIDConnectRefreshFactory(config fosite.Configurator, _ interface{}, strategy interface{}) interface{} {
+	openIDConnectRefreshHandler := compose.OpenIDConnectRefreshFactory(config, nil, strategy).(*openid.OpenIDConnectRefreshHandler)
+	// Overwrite the config with a wrapper around the fosite.IDTokenLifespanProvider.
+	openIDConnectRefreshHandler.Config = &contextAwareIDTokenLifespanProvider{DelegateConfig: config}
+	return openIDConnectRefreshHandler
+}
+
+var _ fosite.IDTokenLifespanProvider = (*contextAwareIDTokenLifespanProvider)(nil)
+
+type contextAwareIDTokenLifespanProvider struct {
+	DelegateConfig fosite.IDTokenLifespanProvider
+}
+
+func (c *contextAwareIDTokenLifespanProvider) GetIDTokenLifespan(ctx context.Context) time.Duration {
+	idTokenLifespanOverride, ok := ctx.Value(idTokenLifetimeOverrideKey).(time.Duration)
+	if ok {
+		return idTokenLifespanOverride
+	}
+	// When there is no override on the context, just return the default by calling the delegate.
+	return c.DelegateConfig.GetIDTokenLifespan(ctx)
+}
+
+func OverrideIDTokenLifespanInContext(ctx context.Context, newLifespan time.Duration) context.Context {
+	return context.WithValue(ctx, idTokenLifetimeOverrideKey, newLifespan)
+}

internal/federationdomain/oidc/oidc.go

@@ -7,8 +7,10 @@ package oidc
 
 import (
 	"crypto/subtle"
+	"errors"
 	"fmt"
 	"net/http"
+	"reflect"
 	"time"
 
 	"github.com/felixge/httpsnoop"
@@ -17,10 +19,12 @@ import (
 	errorsx "github.com/pkg/errors"
 
 	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
+	"go.pinniped.dev/internal/federationdomain/clientregistry"
 	"go.pinniped.dev/internal/federationdomain/csrftoken"
 	"go.pinniped.dev/internal/federationdomain/endpoints/jwks"
 	"go.pinniped.dev/internal/federationdomain/endpoints/tokenexchange"
 	"go.pinniped.dev/internal/federationdomain/formposthtml"
+	"go.pinniped.dev/internal/federationdomain/idtokenlifespan"
 	"go.pinniped.dev/internal/federationdomain/strategy"
 	"go.pinniped.dev/internal/federationdomain/timeouts"
 	"go.pinniped.dev/internal/httputil/httperr"
@@ -102,23 +106,121 @@ type UpstreamStateParamData struct {
 	FormatVersion string              `json:"v"`
 }
 
-// Get the defaults for the Supervisor server.
+// DefaultOIDCTimeoutsConfiguration returns the default timeouts for the Supervisor server.
 func DefaultOIDCTimeoutsConfiguration() timeouts.Configuration {
-	accessTokenLifespan := 2 * time.Minute
+	// Note: The maximum time that users can access Kubernetes clusters without
+	// needing to do a Supervisor refresh is the sum of the access token lifetime,
+	// the ID token lifetime, and the Concierge's mTLS client cert lifetime.
+	// This is because a client can exchange the access token just before it expires
+	// for a new cluster-scoped ID token, and use that just before it expires to get
+	// a new mTLS client cert, which grants access to the cluster until it expires.
+	//
+	// Note that the Concierge's mTLS client cert lifetime is 5 minutes, which can
+	// be seen in its source at credentialrequest/rest.go.
+	//
+	// This maximum total time is important because it represents the longest possible
+	// time that a user could continue to use a cluster based on their original login
+	// (or most recent refresh) after an administrator of an external identity provider
+	// removes the user, revokes their session, changes their group membership,
+	// or otherwise makes any type of change to the user's account in the external
+	// identity provider that should be noticed by the Supervisor during an upstream
+	// refresh.
+	//
+	// Given the timeouts specified below, this is: 2 + 2 + 5 = 9 minutes.
+	// Note that this may be different if an OIDCClient's configuration has changed
+	// the lifetime of the ID tokens issued to that client, but usually will not be
+	// different because that configuration does not change the lifetime of the
+	// cluster-scoped ID tokens. The only case where that configuration would change
+	// it is if the admin configured a cluster to accept the initial ID token's
+	// audience instead of the cluster-scoped ID token's audience.
+	//
+	// The CLI will use a cached mTLS client cert until it expires.
+	// Because of the default timeouts, when the first mTLS client cert expires after
+	// five minutes, the CLI will need to perform a refresh before it can get a second
+	// client cert, due to the original access token and cluster-scoped ID token having
+	// already expired by that time (after two minutes).
+
+	// Give a generous amount of time for an authorized client to be able to exchange
+	// its authcode for tokens.
 	authorizationCodeLifespan := 10 * time.Minute
+
+	// This is intended to give a very short amount of time to allow the client to
+	// use the access token to exchange for cluster-scoped ID token(s). After this
+	// time runs out, they will need to perform a refresh to get a new tokens,
+	// ensuring the Supervisor has a chance to revalidate their session often.
+	accessTokenLifespan := 2 * time.Minute
+
+	// The ID token will have the same default lifespan as the access token for a
+	// similar reason. This is the default lifespan for ID tokens issued by the
+	// authcode flow, the refresh flow, and the cluster-scoped token exchange.
+	// The cluster-scoped ID token can be exchanged for an mTLS client cert, so
+	// limit the window of opportunity to make that exchange to be small.
+	idTokenLifespan := accessTokenLifespan
+
+	// This is just long enough to cover a typical work day, giving the end user an
+	// experience of logging in once per day to access all their Kubernetes clusters.
 	refreshTokenLifespan := 9 * time.Hour
 
+	// Give a little extra time for some storage lifetimes, to avoid the possibility
+	// that the storage be garbage collected in the middle of trying to look up the token.
+	storageExtraLifetime := time.Minute
+
 	return timeouts.Configuration{
-		UpstreamStateParamLifespan:              90 * time.Minute,
-		AuthorizeCodeLifespan:                   authorizationCodeLifespan,
-		AccessTokenLifespan:                     accessTokenLifespan,
-		IDTokenLifespan:                         accessTokenLifespan,
-		RefreshTokenLifespan:                    refreshTokenLifespan,
-		AuthorizationCodeSessionStorageLifetime: authorizationCodeLifespan + refreshTokenLifespan,
-		PKCESessionStorageLifetime:              authorizationCodeLifespan + (1 * time.Minute),
-		OIDCSessionStorageLifetime:              authorizationCodeLifespan + (1 * time.Minute),
-		AccessTokenSessionStorageLifetime:       refreshTokenLifespan + accessTokenLifespan,
-		RefreshTokenSessionStorageLifetime:      refreshTokenLifespan + accessTokenLifespan,
+		// Give enough time for someone to start an interactive authorization flow, go eat lunch,
+		// and then finish the authorization afterward.
+		UpstreamStateParamLifespan: 90 * time.Minute,
+
+		AuthorizeCodeLifespan: authorizationCodeLifespan,
+
+		AccessTokenLifespan: accessTokenLifespan,
+		OverrideDefaultAccessTokenLifespan: func(accessRequest fosite.AccessRequester) (bool, time.Duration) {
+			// Not currently overriding the defaults.
+			return false, 0
+		},
+
+		IDTokenLifespan: idTokenLifespan,
+		OverrideDefaultIDTokenLifespan: func(accessRequest fosite.AccessRequester) (bool, time.Duration) {
+			client := accessRequest.GetClient()
+			// Don't allow OIDCClients to override the default lifetime for ID tokens returned
+			// by RFC8693 token exchange. This is not user configurable for now.
+			if !accessRequest.GetGrantTypes().ExactOne(oidcapi.GrantTypeTokenExchange) {
+				if castClient, ok := client.(*clientregistry.Client); !ok {
+					// All clients returned by our client registry implement clientregistry.Client,
+					// so this should be a safe cast in practice.
+					plog.Error("could not check if client overrides token lifetimes",
+						errors.New("could not cast client to *clientregistry.Client"),
+						"clientID", client.GetID(), "clientType", reflect.TypeOf(client))
+				} else if castClient.IDTokenLifetimeConfiguration > 0 {
+					// An OIDCClient resource has provided an override, so use it.
+					// Note that the pinniped-cli client never overrides this value.
+					return true, castClient.IDTokenLifetimeConfiguration
+				}
+			}
+			// Otherwise, do not override the defaults.
+			return false, 0
+		},
+
+		RefreshTokenLifespan: refreshTokenLifespan,
+
+		AuthorizationCodeSessionStorageLifetime: func(requester fosite.Requester) time.Duration {
+			return authorizationCodeLifespan + refreshTokenLifespan
+		},
+
+		PKCESessionStorageLifetime: func(_requester fosite.Requester) time.Duration {
+			return authorizationCodeLifespan + storageExtraLifetime
+		},
+
+		OIDCSessionStorageLifetime: func(_requester fosite.Requester) time.Duration {
+			return authorizationCodeLifespan + storageExtraLifetime
+		},
+
+		AccessTokenSessionStorageLifetime: func(requester fosite.Requester) time.Duration {
+			return refreshTokenLifespan + accessTokenLifespan
+		},
+
+		RefreshTokenSessionStorageLifetime: func(requester fosite.Requester) time.Duration {
+			return refreshTokenLifespan + accessTokenLifespan
+		},
 	}
 }
 
@@ -170,8 +272,10 @@ func FositeOauth2Helper(
 		},
 		compose.OAuth2AuthorizeExplicitFactory,
 		compose.OAuth2RefreshTokenGrantFactory,
-		compose.OpenIDConnectExplicitFactory,
-		compose.OpenIDConnectRefreshFactory,
+		// Use a custom factory to allow selective overrides of the ID token lifespan during authcode exchange.
+		idtokenlifespan.OpenIDConnectExplicitFactory,
+		// Use a custom factory to allow selective overrides of the ID token lifespan during refresh.
+		idtokenlifespan.OpenIDConnectRefreshFactory,
 		compose.OAuth2PKCEFactory,
 		tokenexchange.HandlerFactory, // handle the "urn:ietf:params:oauth:grant-type:token-exchange" grant type
 	)
@@ -341,8 +445,8 @@ func rewriteStatusSeeOtherToStatusFoundForBrowserless(w http.ResponseWriter) htt
 	// https://tools.ietf.org/id/draft-ietf-oauth-security-topics-18.html#section-4.11
 	// Safari has the bad behavior in the case of http.StatusFound and not just http.StatusTemporaryRedirect.
 	//
-	// in the browserless flows, the OAuth client is the pinniped CLI and it already has access to the user's
-	// password.  Thus there is no security issue with using http.StatusFound vs. http.StatusSeeOther.
+	// In the browserless flows, the OAuth client is the pinniped CLI, and it already has access to the user's
+	// password.  Thus, there is no security issue with using http.StatusFound vs. http.StatusSeeOther.
 	return httpsnoop.Wrap(w, httpsnoop.Hooks{
 		WriteHeader: func(delegate httpsnoop.WriteHeaderFunc) httpsnoop.WriteHeaderFunc {
 			return func(code int) {

internal/federationdomain/timeouts/timeouts_configuration.go

@@ -3,7 +3,18 @@
 
 package timeouts
 
-import "time"
+import (
+	"time"
+
+	"github.com/ory/fosite"
+)
+
+// StorageLifetime is a function that can, given a request, decide how long it should live in session storage.
+type StorageLifetime func(requester fosite.Requester) time.Duration
+
+// OverrideLifespan is a function that, given a request, can suggest to override the default lifespan
+// by returning true along with a new lifespan. When false is returned, the returned duration should be ignored.
+type OverrideLifespan func(accessRequest fosite.AccessRequester) (bool, time.Duration)
 
 type Configuration struct {
 	// The length of time that our state param that we encrypt and pass to the upstream OIDC IDP should be considered
@@ -22,11 +33,28 @@ type Configuration struct {
 	// be fairly short-lived.
 	AccessTokenLifespan time.Duration
 
+	// Optionally override the default AccessTokenLifespan depending on the specific request.
+	// Note that access tokens can be issued by authcode exchanges and refreshes (with different grant types on the
+	// request), so implementations of this method should handle choosing lifespans for both cases as desired.
+	// Note that fosite offers the fosite.ClientWithCustomTokenLifespans interface, but that interface does not
+	// pass the full request details to the GetEffectiveLifespan() function, so it does not suit our needs,
+	// and we use this technique instead.
+	OverrideDefaultAccessTokenLifespan OverrideLifespan
+
 	// The lifetime of an downstream ID token issued by the token endpoint. This should generally be the same
 	// as the AccessTokenLifespan, or longer if it would be useful for the user's proof of identity to be valid
 	// for longer than their proof of authorization.
 	IDTokenLifespan time.Duration
 
+	// Optionally override the default IDTokenLifespan depending on the specific request.
+	// Note that ID tokens can be issued by authcode exchanges, refreshes, and RFC8693 token exchanges
+	// (with different grant types on the request), so implementations of this method should handle choosing
+	// lifespans for all three cases as desired.
+	// Note that fosite offers the fosite.ClientWithCustomTokenLifespans interface, but that interface does not
+	// pass the full request details to the GetEffectiveLifespan() function, so it does not suit our needs,
+	// and we use this technique instead.
+	OverrideDefaultIDTokenLifespan OverrideLifespan
+
 	// The lifetime of an downstream refresh token issued by the token endpoint. This should generally be
 	// significantly longer than the access token lifetime, so it can be used to refresh the access token
 	// multiple times. Once the refresh token expires, the user's session is over and they will need
@@ -40,7 +68,7 @@ type Configuration struct {
 	// include revoking the access and refresh tokens associated with the session. Therefore, this should be
 	// significantly longer than the AuthorizeCodeLifespan, and there is probably no reason to make it longer than
 	// the sum of the AuthorizeCodeLifespan and the RefreshTokenLifespan.
-	AuthorizationCodeSessionStorageLifetime time.Duration
+	AuthorizationCodeSessionStorageLifetime StorageLifetime
 
 	// PKCESessionStorageLifetime is the length of time after which PKCE data is allowed to be garbage collected from
 	// storage. PKCE sessions are closely related to authorization code sessions. After the authcode is successfully
@@ -48,19 +76,19 @@ type Configuration struct {
 	// but it is not explicitly deleted. Therefore, this can be just slightly longer than the AuthorizeCodeLifespan. We'll
 	// avoid making it exactly the same as AuthorizeCodeLifespan to avoid any chance of the garbage collector deleting it
 	// while it is being used.
-	PKCESessionStorageLifetime time.Duration
+	PKCESessionStorageLifetime StorageLifetime
 
 	// OIDCSessionStorageLifetime is the length of time after which the OIDC session data related to an authcode
 	// is allowed to be garbage collected from storage. After the authcode is successfully redeemed, the OIDC session is
 	// explicitly deleted. Similar to the PKCE session, they are not needed anymore after the corresponding authcode has expired.
 	// Therefore, this can be just slightly longer than the AuthorizeCodeLifespan. We'll avoid making it exactly the same
 	// as AuthorizeCodeLifespan to avoid any chance of the garbage collector deleting it while it is being used.
-	OIDCSessionStorageLifetime time.Duration
+	OIDCSessionStorageLifetime StorageLifetime
 
 	// AccessTokenSessionStorageLifetime is the length of time after which an access token's session data is allowed
 	// to be garbage collected from storage.  These must exist in storage for as long as the refresh token is valid
 	// or else the refresh flow will not work properly. So this must be longer than RefreshTokenLifespan.
-	AccessTokenSessionStorageLifetime time.Duration
+	AccessTokenSessionStorageLifetime StorageLifetime
 
 	// RefreshTokenSessionStorageLifetime is the length of time after which a refresh token's session data is allowed
 	// to be garbage collected from storage. These must exist in storage for as long as the refresh token is valid.
@@ -70,5 +98,5 @@ type Configuration struct {
 	// error message telling them that the token is expired, rather than a more generic error that is returned
 	// when the token does not exist. If this is desirable, then the RefreshTokenSessionStorageLifetime can be made
 	// to be significantly larger than RefreshTokenLifespan, at the cost of slower cleanup.
-	RefreshTokenSessionStorageLifetime time.Duration
+	RefreshTokenSessionStorageLifetime StorageLifetime
 }