Summary

ToolHive currently implements token persistence and Dynamic Client Registration (DCR) persistence, but lacks support for tracking and renewing expired client secrets. When an OAuth provider issues a client secret with an expiration time (per RFC 7591), ToolHive should:

1. Store the expiration time from the DCR response
2. Monitor for secret expiration
3. Automatically renew the client secret before it expires using the registration access token

Current Implementation Status

✅ Implemented

1. Token Persistence: Refresh tokens are persisted with expiry tracking (CachedTokenExpiry)
2. DCR Client Credentials Persistence: Client ID and client secret are stored
   • Client ID stored as plain text in CachedClientID
   • Client secret stored securely via secret manager reference in CachedClientSecretRef
3. Client Secret Expiry Storage: The CachedSecretExpiry field in pkg/auth/remote/config.go stores the client secret expiration time

❌ Missing Implementation

1. DCR Response Metadata Not Fully Stored:

   • RegistrationAccessToken is not stored (needed for secret renewal)
   • RegistrationClientURI is not stored (needed for secret renewal)

2. Expiry Checking Logic:

   • No logic to check if CachedSecretExpiry indicates an expired or soon-to-expire secret
   • No validation when restoring cached credentials
   • Expired secrets are not detected before use

3. Renewal Logic:

   • No implementation to renew client secret using RFC 7592 Update Client Registration
   • No use of RegistrationAccessToken and RegistrationClientURI for renewal
   • No automatic renewal when secret is expiring soon

Technical Details

Relevant Code Locations

1. DCR Response Structure: pkg/auth/oauth/dynamic_registration.go

   • DynamicClientRegistrationResponse includes ClientSecretExpiresAt, RegistrationAccessToken, RegistrationClientURI

2. OAuth Flow Result: pkg/auth/discovery/discovery.go

   • OAuthFlowResult struct needs to include DCR metadata fields

3. Persistence Callback: pkg/auth/remote/handler.go

   • ClientCredentialsPersister type signature needs to accept additional parameters
   • wrapWithPersistence() calls the persister but doesn't pass expiry/metadata

4. Runner Implementation: pkg/runner/runner.go

   • SetClientCredentialsPersister() callback implementation needs to store expiry and registration metadata

5. Config Storage: pkg/auth/remote/config.go

   • CachedSecretExpiry field exists and is populated with expiry time
   • CachedRegTokenRef field exists but is never populated (needed for renewal)

RFC Compliance

RFC 7591 (OAuth 2.0 Dynamic Client Registration Protocol):

• Section 3.2.1: Defines client_secret_expires_at in registration response - RFC 7591 Section 3.2.1
• Section 3.2.1: Defines registration_access_token and registration_client_uri in registration response

RFC 7592 (OAuth 2.0 Dynamic Client Registration Management Protocol):

• Section 2.1: Defines response format for read/update operations - RFC 7592 Section 2.1
• Section 2.2: Defines client update mechanism using PUT to registration_client_uri - RFC 7592 Section 2.2
• Section 3: Defines complete client information response structure - RFC 7592 Section 3

Proposed Implementation

Phase 1: Store Additional DCR Metadata

1. Extend OAuthFlowResult to include registration metadata:

   type OAuthFlowResult struct {
       // ... existing fields ...
       
       // DCR metadata for secret renewal
       RegistrationAccessToken string    // For updating registration
       RegistrationClientURI   string    // Endpoint for registration updates
   }

2. Update handleDynamicRegistration():

   • Store RegistrationAccessToken and RegistrationClientURI from DCR response
   • Pass these through to OAuthFlowResult

3. Extend ClientCredentialsPersister signature:

   type ClientCredentialsPersister func(
       clientID string,
       clientSecret string,
       secretExpiry time.Time,
       registrationAccessToken string,
       registrationClientURI string,
   ) error

4. Update persistence in runner.go:

   • Store registrationAccessToken securely via secret manager in CachedRegTokenRef
   • Store registrationClientURI (can be plain text, or derive from issuer)

Phase 2: Expiry Checking

1. Add expiry check in resolveClientCredentials() (pkg/auth/remote/handler.go):

   • Check if CachedSecretExpiry indicates secret is expired or expiring soon (e.g., within 24 hours)
   • If expired/expiring, trigger renewal before using credentials

2. Add validation in tryRestoreFromCachedTokens():

   • Verify secret hasn't expired before attempting token refresh
   • Return appropriate error if secret is expired

Phase 3: Secret Renewal

1. Implement RFC 7592 Update Client Registration:

   • Create function to update client registration using RegistrationAccessToken
   • Use HTTP PUT request to RegistrationClientURI per RFC 7592 Section 2.2
   • Authenticate using RegistrationAccessToken as Bearer token
   • Include all client metadata fields in the request (as required by RFC 7592 Section 2.2)
   • Handle response per RFC 7592 Section 2.1 with new client_secret and updated client_secret_expires_at
   • Response may include new registration_access_token which must replace the old one

2. Automatic Renewal Logic:

   • When secret is expiring soon, automatically renew it
   • Update stored secret and expiry in config
   • Handle renewal failures gracefully (log warning, allow manual intervention)

3. Error Handling:

   • If renewal fails, log warning and continue with existing secret (may still work)
   • If secret is already expired and renewal fails, return error requiring re-authentication

Testing Considerations

1. Unit Tests:

   • Test expiry checking logic with various expiry times
   • Test renewal flow with mock DCR server
   • Test error handling for expired secrets

2. Integration Tests:

   • Test with OAuth provider that issues expiring secrets
   • Test automatic renewal before expiry
   • Test behavior when renewal fails

3. Edge Cases:

   • Zero expiry (secret never expires)
   • Missing registration access token (some providers don't provide it)
   • Clock skew handling
   • Concurrent renewal attempts

Related Code References

• DCR Response: pkg/auth/oauth/dynamic_registration.go:135-153
• OAuth Flow Result: pkg/auth/discovery/discovery.go:505-517
• Persistence Callback: pkg/auth/remote/handler.go:22-47
• Runner Persistence: pkg/runner/runner.go:666-694
• Config Structure: pkg/auth/remote/config.go:58-68

RFC References

• RFC 7591 Section 3.2.1 - Client Information Response (defines client_secret_expires_at, registration_access_token, registration_client_uri)
• RFC 7592 Section 2.1 - Client Read Request (response format for read/update operations)
• RFC 7592 Section 2.2 - Client Update Request (PUT mechanism for updating registration)
• RFC 7592 Section 3 - Client Information Response (complete response structure)

Acceptance Criteria

• DCR response metadata (RegistrationAccessToken, RegistrationClientURI) is stored
• Expiry checking logic validates secret expiry before use
• Automatic renewal of expiring secrets using RFC 7592 Update Client Registration (Section 2.2)
• Graceful handling of renewal failures
• Comprehensive test coverage for expiry and renewal scenarios
• Documentation updated to explain secret expiry behavior

Additional Notes

• This feature is important for long-running workloads that rely on DCR
• Some OAuth providers (e.g., Keycloak) issue client secrets with expiration times
• The implementation should be backward compatible with providers that don't expire secrets (zero expiry value)