Implement Client Secret Expiry and Renewal for Dynamic Client Registration

pkg/auth/discovery/dcr_resolver_test.go

@@ -81,6 +81,11 @@ type dcrTestServerConfig struct {
 	// handler so concurrent goroutines can pile up at the singleflight
 	// before any can finish.
 	registrationDelay time.Duration
+
+	// Optional RFC 7591 response metadata used by renewal happy-path tests.
+	clientSecretExpiresAt   int64
+	registrationAccessToken string
+	registrationClientURI   string
 }
 
 // newDCRDiscoveryServer mounts /.well-known/openid-configuration and a
@@ -130,6 +135,9 @@ func newDCRDiscoveryServer(t *testing.T, cfg dcrTestServerConfig) *httptest.Serv
 		}
 		resp := oauthproto.DynamicClientRegistrationResponse{
 			ClientID:                "cli-registered-client",
+			ClientSecretExpiresAt:   cfg.clientSecretExpiresAt,
+			RegistrationAccessToken: cfg.registrationAccessToken,
+			RegistrationClientURI:   cfg.registrationClientURI,
 			TokenEndpointAuthMethod: "none",
 		}
 		w.Header().Set("Content-Type", "application/json")
@@ -529,20 +537,28 @@ func TestHandleDynamicRegistration_SynthesisesEndpointWhenMetadataOmitsIt(t *tes
 		"synthesised endpoint must be contacted exactly once")
 }
 
-// TestHandleDynamicRegistration_PopulatesEndpoints verifies the contract
-// the rest of the CLI flow depends on: handleDynamicRegistration writes
-// the resolved AuthorizeURL / TokenURL onto OAuthFlowConfig so
-// createOAuthConfig can construct the OAuth2 flow without re-discovery.
-func TestHandleDynamicRegistration_PopulatesEndpoints(t *testing.T) {
+// TestHandleDynamicRegistration_PopulatesEndpointsAndRenewalMetadata verifies
+// the contract the rest of the CLI flow depends on: handleDynamicRegistration
+// writes the resolved endpoints and DCR renewal metadata onto OAuthFlowConfig.
+func TestHandleDynamicRegistration_PopulatesEndpointsAndRenewalMetadata(t *testing.T) {
 	t.Parallel()
 
+	const (
+		callbackPort          = 8765
+		secretExpiryUnix      = int64(1_893_456_000)
+		registrationToken     = "registration-access-token"
+		registrationClientURI = "https://issuer.example/register/cli-registered-client"
+	)
 	server := newDCRDiscoveryServer(t, dcrTestServerConfig{
 		codeChallengeMethodsSupported: []string{"S256"},
+		clientSecretExpiresAt:         secretExpiryUnix,
+		registrationAccessToken:       registrationToken,
+		registrationClientURI:         registrationClientURI,
 	})
 
 	config := &OAuthFlowConfig{
 		Scopes:          []string{"openid", "profile"},
-		CallbackPort:    8765,
+		CallbackPort:    callbackPort,
 		AllowPrivateIPs: true, // loopback test server; guard would otherwise refuse to dial it
 	}
 
@@ -554,4 +570,9 @@ func TestHandleDynamicRegistration_PopulatesEndpoints(t *testing.T) {
 		"handleDynamicRegistration must populate OAuthFlowConfig.AuthorizeURL")
 	assert.Equal(t, server.URL+"/token", config.TokenURL,
 		"handleDynamicRegistration must populate OAuthFlowConfig.TokenURL")
+	assert.Equal(t, time.Unix(secretExpiryUnix, 0).UTC(), config.SecretExpiry)
+	assert.Equal(t, registrationToken, config.RegistrationAccessToken)
+	assert.Equal(t, registrationClientURI, config.RegistrationClientURI)
+	assert.Equal(t, "none", config.TokenEndpointAuthMethod)
+	assert.Equal(t, callbackPort, config.RegisteredCallbackPort)
 }

pkg/auth/discovery/discovery.go

@@ -529,6 +529,14 @@ type OAuthFlowConfig struct {
 	// blockPrivateIPs decision the flow already applies to its other discovery
 	// fetches. Defaults to false (guarded).
 	AllowPrivateIPs bool
+
+	// DCR renewal metadata — populated by handleDynamicRegistration and threaded
+	// into OAuthFlowResult so callers can persist the data for RFC 7592 operations.
+	SecretExpiry            time.Time // zero means the secret never expires
+	RegistrationAccessToken string    //nolint:gosec // G117: field legitimately holds sensitive data
+	RegistrationClientURI   string
+	TokenEndpointAuthMethod string
+	RegisteredCallbackPort  int
 }
 
 // OAuthFlowResult contains the result of an OAuth flow
@@ -544,6 +552,16 @@ type OAuthFlowResult struct {
 	// DCR client credentials for persistence (obtained during Dynamic Client Registration)
 	ClientID     string
 	ClientSecret string //nolint:gosec // G117: field legitimately holds sensitive data
+
+	// DCR renewal metadata (RFC 7591 §3.2.1 / RFC 7592).
+	// SecretExpiry is zero when the provider did not issue an expiring secret.
+	// RegistrationAccessToken and RegistrationClientURI are empty when the
+	// provider does not support RFC 7592 management operations.
+	SecretExpiry            time.Time
+	RegistrationAccessToken string //nolint:gosec // G117: field legitimately holds sensitive data
+	RegistrationClientURI   string
+	TokenEndpointAuthMethod string
+	RegisteredCallbackPort  int
 }
 
 func shouldDynamicallyRegisterClient(config *OAuthFlowConfig) bool {
@@ -616,20 +634,12 @@ func PerformOAuthFlow(ctx context.Context, issuer string, config *OAuthFlowConfi
 // exists.
 //
 // One consequence of option (b) is that the resolver's RFC 7591 §3.2.1
-// expiry-driven refetch does NOT participate in the CLI's cross-
-// invocation persistence loop: each PerformOAuthFlow call builds a fresh
-// in-memory store, so a "cached but expired" entry from the previous
-// invocation never reaches the resolver. Cross-invocation expiry is also
-// NOT enforced by the remote handler's gate today —
-// HasCachedClientCredentials only checks CachedClientID != "" and does
-// not consult CachedSecretExpiry, so an expired-but-still-cached client
-// gets reused on the next invocation and surfaces as a token-endpoint
-// failure rather than a clean DCR re-registration. Tightening the gate
-// to also check CachedSecretExpiry is open follow-up work; the
-// behaviour today is "cross-invocation expiry is unhandled". Within a
-// single invocation, the resolver's expiry check is still in the loop
-// and would fire if the same call site somehow registered, persisted,
-// and re-queried the in-memory store — but the CLI never does this today.
+// expiry-driven refetch does NOT participate in the CLI's cross-invocation
+// persistence loop: each PerformOAuthFlow call builds a fresh in-memory store,
+// so a cached entry from a previous invocation never reaches the resolver.
+// Cross-invocation client-secret expiry is handled instead by the remote
+// handler, which consults CachedSecretExpiry and renews through RFC 7592 before
+// cached credentials are used.
 //
 // Wrapping the remote handler's secretProvider into a dcr.CredentialStore
 // adapter (option (a)) would close that loop and is the natural follow-up;
@@ -677,6 +687,18 @@ func handleDynamicRegistration(ctx context.Context, issuer string, config *OAuth
 		config.TokenURL = resolution.TokenEndpoint
 	}
 
+	// Store DCR renewal metadata for RFC 7592 operations.
+	// A zero ClientSecretExpiresAt means the secret never expires (RFC 7591 §3.2.1).
+	config.SecretExpiry = resolution.ClientSecretExpiresAt
+	config.RegistrationAccessToken = resolution.RegistrationAccessToken
+	config.RegistrationClientURI = resolution.RegistrationClientURI
+	config.TokenEndpointAuthMethod = resolution.TokenEndpointAuthMethod
+	config.RegisteredCallbackPort = config.CallbackPort
+
+	if resolution.RegistrationAccessToken != "" {
+		slog.Debug("DCR response includes registration access token for RFC 7592 operations")
+	}
+
 	return nil
 }
 
@@ -929,15 +951,30 @@ func newOAuthFlow(ctx context.Context, oauthConfig *oauth.Config, config *OAuthF
 	}
 
 	source := flow.TokenSource()
+	return buildOAuthFlowResult(source, oauthConfig, tokenResult, config), nil
+}
+
+func buildOAuthFlowResult(
+	tokenSource oauth2.TokenSource,
+	oauthConfig *oauth.Config,
+	tokenResult *oauth.TokenResult,
+	config *OAuthFlowConfig,
+) *OAuthFlowResult {
 	return &OAuthFlowResult{
-		TokenSource:  source,
+		TokenSource:  tokenSource,
 		Config:       oauthConfig,
 		AccessToken:  tokenResult.AccessToken,
 		RefreshToken: tokenResult.RefreshToken,
 		Expiry:       tokenResult.Expiry,
 		ClientID:     oauthConfig.ClientID,
 		ClientSecret: oauthConfig.ClientSecret,
-	}, nil
+		// DCR renewal metadata — populated only when dynamic registration was performed.
+		SecretExpiry:            config.SecretExpiry,
+		RegistrationAccessToken: config.RegistrationAccessToken,
+		RegistrationClientURI:   config.RegistrationClientURI,
+		TokenEndpointAuthMethod: config.TokenEndpointAuthMethod,
+		RegisteredCallbackPort:  config.RegisteredCallbackPort,
+	}
 }
 
 // FetchResourceMetadata fetches RFC 9728 protected-resource metadata from a

pkg/auth/discovery/discovery_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/stacklok/toolhive/pkg/auth/oauth"
 	"github.com/stacklok/toolhive/pkg/networking"
 )
 
@@ -1340,3 +1341,24 @@ func TestHandleDynamicRegistration_MissingRegistrationEndpoint(t *testing.T) {
 	assert.Contains(t, err.Error(), "--remote-auth-client-id")
 	assert.Contains(t, err.Error(), "--remote-auth-client-secret")
 }
+
+func TestBuildOAuthFlowResult_CopiesDCRRenewalMetadata(t *testing.T) {
+	t.Parallel()
+
+	secretExpiry := time.Date(2030, time.January, 2, 3, 4, 5, 0, time.UTC)
+	config := &OAuthFlowConfig{
+		SecretExpiry:            secretExpiry,
+		RegistrationAccessToken: "registration-access-token",
+		RegistrationClientURI:   "https://issuer.example/register/client-id",
+		TokenEndpointAuthMethod: "client_secret_basic",
+		RegisteredCallbackPort:  49152,
+	}
+
+	result := buildOAuthFlowResult(nil, &oauth.Config{}, &oauth.TokenResult{}, config)
+
+	assert.Equal(t, config.SecretExpiry, result.SecretExpiry)
+	assert.Equal(t, config.RegistrationAccessToken, result.RegistrationAccessToken)
+	assert.Equal(t, config.RegistrationClientURI, result.RegistrationClientURI)
+	assert.Equal(t, config.TokenEndpointAuthMethod, result.TokenEndpointAuthMethod)
+	assert.Equal(t, config.RegisteredCallbackPort, result.RegisteredCallbackPort)
+}

pkg/auth/remote/config.go

@@ -11,7 +11,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/stacklok/toolhive-core/registry/types"
+	registry "github.com/stacklok/toolhive-core/registry/types"
 	httpval "github.com/stacklok/toolhive-core/validation/http"
 )
 
@@ -68,7 +68,8 @@ type Config struct {
 	// ClientSecretExpiresAt indicates when the client secret expires (if provided by the DCR server).
 	// A zero value means the secret does not expire.
 	CachedSecretExpiry time.Time `json:"cached_secret_expiry,omitempty" yaml:"cached_secret_expiry,omitempty"`
-	// RegistrationAccessToken is used to update/delete the client registration.
+	// CachedRegTokenRef is a secret manager reference to the registration_access_token
+	// returned in the DCR response. Used for RFC 7592 client update operations.
 	// Stored as a secret reference since it's sensitive.
 	CachedRegTokenRef string `json:"cached_reg_token_ref,omitempty" yaml:"cached_reg_token_ref,omitempty"`
 
@@ -78,6 +79,17 @@ type Config struct {
 	// rotation clears CachedClientID without touching the stable CIMD URL.
 	// Read by resolveClientCredentials to send the correct client_id on token refresh.
 	CachedCIMDClientID string `json:"cached_cimd_client_id,omitempty" yaml:"cached_cimd_client_id,omitempty"`
+	// CachedRegClientURI is the registration_client_uri from the DCR response.
+	// This is the endpoint used for RFC 7592 client read/update/delete operations.
+	// Stored as plain text since it is not sensitive.
+	CachedRegClientURI string `json:"cached_reg_client_uri,omitempty" yaml:"cached_reg_client_uri,omitempty"`
+	// CachedTokenEndpointAuthMethod is the auth method used for the token endpoint
+	// (e.g., "client_secret_basic", "none"). Persisted for RFC 7592 updates.
+	CachedTokenEndpointAuthMethod string `json:"cached_token_auth_method,omitempty" yaml:"cached_token_auth_method,omitempty"`
+	// CachedDCRCallbackPort is the callback port that was actually registered
+	// during DCR. It may differ from CallbackPort when the requested port was
+	// unavailable and a fallback port was selected.
+	CachedDCRCallbackPort int `json:"cached_dcr_callback_port,omitempty" yaml:"cached_dcr_callback_port,omitempty"`
 }
 
 // BearerTokenEnvVarName is the environment variable name used for bearer token authentication.
@@ -184,6 +196,9 @@ func (c *Config) ClearCachedClientCredentials() {
 	c.CachedClientSecretRef = ""
 	c.CachedSecretExpiry = time.Time{}
 	c.CachedRegTokenRef = ""
+	c.CachedRegClientURI = ""
+	c.CachedTokenEndpointAuthMethod = ""
+	c.CachedDCRCallbackPort = 0
 }
 
 // LogContext returns the upstream issuer and resolved client_id for use as