diff --git a/docs/server/docs.go b/docs/server/docs.go index e741f3b3e0..d0eb38ec08 100644 --- a/docs/server/docs.go +++ b/docs/server/docs.go @@ -800,10 +800,25 @@ const docTemplate = `{ "bearer_token_file": { "type": "string" }, + "cached_client_id": { + "description": "Cached DCR client credentials for persistence across restarts.\nThese are obtained during Dynamic Client Registration and needed to refresh tokens.\nClientID is stored as plain text since it's public information.", + "type": "string" + }, + "cached_client_secret_ref": { + "type": "string" + }, "cached_refresh_token_ref": { "description": "Cached OAuth token reference for persistence across restarts.\nThe refresh token is stored securely in the secret manager, and this field\ncontains the reference to retrieve it (e.g., \"OAUTH_REFRESH_TOKEN_workload\").\nThis enables session restoration without requiring a new browser-based login.", "type": "string" }, + "cached_reg_token_ref": { + "description": "RegistrationAccessToken is used to update/delete the client registration.\nStored as a secret reference since it's sensitive.", + "type": "string" + }, + "cached_secret_expiry": { + "description": "ClientSecretExpiresAt indicates when the client secret expires (if provided by the DCR server).\nA zero value means the secret does not expire.", + "type": "string" + }, "cached_token_expiry": { "type": "string" }, diff --git a/docs/server/swagger.json b/docs/server/swagger.json index 562f16bba1..93f6ebd632 100644 --- a/docs/server/swagger.json +++ b/docs/server/swagger.json @@ -793,10 +793,25 @@ "bearer_token_file": { "type": "string" }, + "cached_client_id": { + "description": "Cached DCR client credentials for persistence across restarts.\nThese are obtained during Dynamic Client Registration and needed to refresh tokens.\nClientID is stored as plain text since it's public information.", + "type": "string" + }, + "cached_client_secret_ref": { + "type": "string" + }, "cached_refresh_token_ref": { "description": "Cached OAuth token reference for persistence across restarts.\nThe refresh token is stored securely in the secret manager, and this field\ncontains the reference to retrieve it (e.g., \"OAUTH_REFRESH_TOKEN_workload\").\nThis enables session restoration without requiring a new browser-based login.", "type": "string" }, + "cached_reg_token_ref": { + "description": "RegistrationAccessToken is used to update/delete the client registration.\nStored as a secret reference since it's sensitive.", + "type": "string" + }, + "cached_secret_expiry": { + "description": "ClientSecretExpiresAt indicates when the client secret expires (if provided by the DCR server).\nA zero value means the secret does not expire.", + "type": "string" + }, "cached_token_expiry": { "type": "string" }, diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml index 5eea6133ec..71c21efcd6 100644 --- a/docs/server/swagger.yaml +++ b/docs/server/swagger.yaml @@ -722,6 +722,14 @@ components: type: string bearer_token_file: type: string + cached_client_id: + description: |- + Cached DCR client credentials for persistence across restarts. + These are obtained during Dynamic Client Registration and needed to refresh tokens. + ClientID is stored as plain text since it's public information. + type: string + cached_client_secret_ref: + type: string cached_refresh_token_ref: description: |- Cached OAuth token reference for persistence across restarts. @@ -729,6 +737,16 @@ components: contains the reference to retrieve it (e.g., "OAUTH_REFRESH_TOKEN_workload"). This enables session restoration without requiring a new browser-based login. type: string + cached_reg_token_ref: + description: |- + RegistrationAccessToken is used to update/delete the client registration. + Stored as a secret reference since it's sensitive. + type: string + cached_secret_expiry: + description: |- + ClientSecretExpiresAt indicates when the client secret expires (if provided by the DCR server). + A zero value means the secret does not expire. + type: string cached_token_expiry: type: string callback_port: diff --git a/pkg/auth/discovery/discovery.go b/pkg/auth/discovery/discovery.go index 8376394399..d918f17d4b 100644 --- a/pkg/auth/discovery/discovery.go +++ b/pkg/auth/discovery/discovery.go @@ -510,6 +510,10 @@ type OAuthFlowResult struct { AccessToken string RefreshToken string Expiry time.Time + + // DCR client credentials for persistence (obtained during Dynamic Client Registration) + ClientID string + ClientSecret string } func shouldDynamicallyRegisterClient(config *OAuthFlowConfig) bool { @@ -691,6 +695,8 @@ func newOAuthFlow(ctx context.Context, oauthConfig *oauth.Config, config *OAuthF AccessToken: tokenResult.AccessToken, RefreshToken: tokenResult.RefreshToken, Expiry: tokenResult.Expiry, + ClientID: oauthConfig.ClientID, + ClientSecret: oauthConfig.ClientSecret, }, nil } diff --git a/pkg/auth/remote/config.go b/pkg/auth/remote/config.go index 258ce74725..1d885b31c8 100644 --- a/pkg/auth/remote/config.go +++ b/pkg/auth/remote/config.go @@ -54,6 +54,18 @@ type Config struct { // This enables session restoration without requiring a new browser-based login. CachedRefreshTokenRef string `json:"cached_refresh_token_ref,omitempty" yaml:"cached_refresh_token_ref,omitempty"` CachedTokenExpiry time.Time `json:"cached_token_expiry,omitempty" yaml:"cached_token_expiry,omitempty"` + + // Cached DCR client credentials for persistence across restarts. + // These are obtained during Dynamic Client Registration and needed to refresh tokens. + // ClientID is stored as plain text since it's public information. + CachedClientID string `json:"cached_client_id,omitempty" yaml:"cached_client_id,omitempty"` + CachedClientSecretRef string `json:"cached_client_secret_ref,omitempty" yaml:"cached_client_secret_ref,omitempty"` + // 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. + // Stored as a secret reference since it's sensitive. + CachedRegTokenRef string `json:"cached_reg_token_ref,omitempty" yaml:"cached_reg_token_ref,omitempty"` } // BearerTokenEnvVarName is the environment variable name used for bearer token authentication. @@ -142,6 +154,19 @@ func (c *Config) ClearCachedTokens() { c.CachedTokenExpiry = time.Time{} } +// HasCachedClientCredentials returns true if the config has cached DCR client credentials. +func (c *Config) HasCachedClientCredentials() bool { + return c.CachedClientID != "" +} + +// ClearCachedClientCredentials removes any cached DCR client credential references from the config. +func (c *Config) ClearCachedClientCredentials() { + c.CachedClientID = "" + c.CachedClientSecretRef = "" + c.CachedSecretExpiry = time.Time{} + c.CachedRegTokenRef = "" +} + // DefaultResourceIndicator derives the resource indicator (RFC 8707) from the remote server URL. // This function should only be called when the user has not explicitly provided a resource indicator. // If the resource indicator cannot be derived, it returns an empty string. diff --git a/pkg/auth/remote/config_test.go b/pkg/auth/remote/config_test.go index ea201478ae..bbefc78315 100644 --- a/pkg/auth/remote/config_test.go +++ b/pkg/auth/remote/config_test.go @@ -158,3 +158,69 @@ func TestConfig_UnmarshalJSON_BearerTokenFields(t *testing.T) { }) } } + +func TestConfig_HasCachedClientCredentials(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + config Config + expected bool + }{ + { + name: "no cached credentials", + config: Config{}, + expected: false, + }, + { + name: "has cached client ID only", + config: Config{ + CachedClientID: "test_client_id", + }, + expected: true, + }, + { + name: "has both cached credentials", + config: Config{ + CachedClientID: "test_client_id", + CachedClientSecretRef: "OAUTH_CLIENT_SECRET_test", + }, + expected: true, + }, + { + name: "has only cached client secret (invalid state)", + config: Config{ + CachedClientSecretRef: "OAUTH_CLIENT_SECRET_test", + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + result := tt.config.HasCachedClientCredentials() + if result != tt.expected { + t.Errorf("HasCachedClientCredentials() = %v, want %v", result, tt.expected) + } + }) + } +} + +func TestConfig_ClearCachedClientCredentials(t *testing.T) { + t.Parallel() + + config := Config{ + CachedClientID: "test_client_id", + CachedClientSecretRef: "OAUTH_CLIENT_SECRET_test", + } + + config.ClearCachedClientCredentials() + + if config.CachedClientID != "" { + t.Errorf("CachedClientID should be empty, got %s", config.CachedClientID) + } + if config.CachedClientSecretRef != "" { + t.Errorf("CachedClientSecretRef should be empty, got %s", config.CachedClientSecretRef) + } +} diff --git a/pkg/auth/remote/handler.go b/pkg/auth/remote/handler.go index bf09524510..83927d7ca9 100644 --- a/pkg/auth/remote/handler.go +++ b/pkg/auth/remote/handler.go @@ -17,9 +17,10 @@ import ( // Handler handles authentication for remote MCP servers. // Supports OAuth/OIDC-based authentication with automatic discovery. type Handler struct { - config *Config - tokenPersister TokenPersister - secretProvider secrets.Provider + config *Config + tokenPersister TokenPersister + clientCredentialsPersister ClientCredentialsPersister + secretProvider secrets.Provider } // NewHandler creates a new remote authentication handler @@ -40,6 +41,12 @@ func (h *Handler) SetSecretProvider(provider secrets.Provider) { h.secretProvider = provider } +// SetClientCredentialsPersister sets a callback function that will be called +// when DCR client credentials are obtained and need to be persisted. +func (h *Handler) SetClientCredentialsPersister(persister ClientCredentialsPersister) { + h.clientCredentialsPersister = persister +} + // Authenticate is the main entry point for remote MCP server authentication func (h *Handler) Authenticate(ctx context.Context, remoteURL string) (oauth2.TokenSource, error) { // Priority 1: Bearer token authentication (if configured) @@ -175,6 +182,16 @@ func (h *Handler) wrapWithPersistence(result *discovery.OAuthFlowResult) oauth2. } } + // Persist DCR client credentials if available (for servers that use Dynamic Client Registration) + // Only persist if client_id exists - client_secret may be empty for PKCE flows + if h.clientCredentialsPersister != nil && result.ClientID != "" { + if err := h.clientCredentialsPersister(result.ClientID, result.ClientSecret); err != nil { + logger.Warnf("Failed to persist DCR client credentials: %v", err) + } else { + logger.Infof("Successfully persisted DCR client credentials for future restarts") + } + } + // Wrap the token source to persist refreshed tokens tokenSource := result.TokenSource if h.tokenPersister != nil { @@ -184,6 +201,33 @@ func (h *Handler) wrapWithPersistence(result *discovery.OAuthFlowResult) oauth2. return tokenSource } +// resolveClientCredentials returns the client ID and secret to use, preferring +// cached DCR credentials over statically configured ones. +func (h *Handler) resolveClientCredentials(ctx context.Context) (clientID, clientSecret string) { + // First try to use statically configured credentials + clientID = h.config.ClientID + clientSecret = h.config.ClientSecret + + // If we have cached DCR client credentials, use those instead + if h.config.HasCachedClientCredentials() { + // ClientID is stored as plain text (it's public information) + clientID = h.config.CachedClientID + logger.Debugf("Using cached DCR client credentials (client_id: %s)", clientID) + + // Client secret is stored securely and may be empty for PKCE flows + if h.config.CachedClientSecretRef != "" && h.secretProvider != nil { + cachedClientSecret, err := h.secretProvider.GetSecret(ctx, h.config.CachedClientSecretRef) + if err != nil { + logger.Warnf("Failed to retrieve cached client secret: %v", err) + } else { + clientSecret = cachedClientSecret + } + } + } + + return clientID, clientSecret +} + // tryRestoreFromCachedTokens attempts to create a TokenSource from cached tokens func (h *Handler) tryRestoreFromCachedTokens( ctx context.Context, @@ -201,10 +245,13 @@ func (h *Handler) tryRestoreFromCachedTokens( return nil, fmt.Errorf("failed to retrieve cached refresh token: %w", err) } + // Resolve client credentials - prefer cached DCR credentials over config + clientID, clientSecret := h.resolveClientCredentials(ctx) + // Build OAuth2 config for token refresh oauth2Config := &oauth2.Config{ - ClientID: h.config.ClientID, - ClientSecret: h.config.ClientSecret, + ClientID: clientID, + ClientSecret: clientSecret, Scopes: scopes, Endpoint: oauth2.Endpoint{ AuthURL: h.config.AuthorizeURL, diff --git a/pkg/auth/remote/persisting_token_source.go b/pkg/auth/remote/persisting_token_source.go index 3d229da076..a57cba3355 100644 --- a/pkg/auth/remote/persisting_token_source.go +++ b/pkg/auth/remote/persisting_token_source.go @@ -18,6 +18,10 @@ import ( // since the access token can be regenerated from it. type TokenPersister func(refreshToken string, expiry time.Time) error +// ClientCredentialsPersister is called when DCR client credentials need to be persisted. +// This is used to store client_id and client_secret obtained during Dynamic Client Registration. +type ClientCredentialsPersister func(clientID, clientSecret string) error + // PersistingTokenSource wraps an oauth2.TokenSource and persists tokens // whenever they are refreshed. This enables session restoration across // workload restarts without requiring a new browser-based OAuth flow. diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index d34ba68315..066ace5605 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -605,6 +605,37 @@ func (r *Runner) handleRemoteAuthentication(ctx context.Context) (oauth2.TokenSo logger.Debugf("Stored OAuth refresh token in secret manager as %s", secretName) return nil }) + + // Set up client credentials persister for DCR (Dynamic Client Registration) + authHandler.SetClientCredentialsPersister(func(clientID, clientSecret string) error { + // Store client ID directly (it's public information) + r.Config.RemoteAuthConfig.CachedClientID = clientID + + // Only store client secret if it's non-empty (PKCE flows may not have one) + if clientSecret != "" { + clientSecretSecretName, err := authsecrets.GenerateUniqueSecretNameWithPrefix( + r.Config.Name, + "OAUTH_CLIENT_SECRET_", + secretManager, + ) + if err != nil { + return fmt.Errorf("failed to generate client secret secret name: %w", err) + } + + if err := authsecrets.StoreSecretInManagerWithProvider(ctx, clientSecretSecretName, clientSecret, secretManager); err != nil { + return fmt.Errorf("failed to store client secret: %w", err) + } + r.Config.RemoteAuthConfig.CachedClientSecretRef = clientSecretSecretName + } + + // Save the updated config to persist the credentials + if err := r.Config.SaveState(ctx); err != nil { + return fmt.Errorf("failed to save config with client credentials: %w", err) + } + + logger.Debugf("Stored DCR client credentials (client_id: %s)", clientID) + return nil + }) } // Perform authentication