Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pkg/auth/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,13 @@ func registerDynamicClient(
discoveredDoc *oauth.OIDCDiscoveryDocument,
) (*oauth.DynamicClientRegistrationResponse, error) {

// Check if the provider supports Dynamic Client Registration
if discoveredDoc.RegistrationEndpoint == "" {
return nil, fmt.Errorf("this provider does not support Dynamic Client Registration (DCR). " +
"Please configure OAuth client credentials using --remote-auth-client-id and --remote-auth-client-secret flags, " +
"or register a client manually with the provider")
}

// Use default client name if not provided
registrationRequest := oauth.NewDynamicClientRegistrationRequest(config.Scopes, config.CallbackPort)

Expand Down
40 changes: 40 additions & 0 deletions pkg/auth/discovery/discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/stacklok/toolhive/pkg/auth/oauth"
"github.com/stacklok/toolhive/pkg/logger"
"github.com/stacklok/toolhive/pkg/networking"
)
Expand Down Expand Up @@ -1273,3 +1274,42 @@ func TestTryWellKnownDiscovery_ErrorPaths(t *testing.T) {
assert.Nil(t, result)
})
}

// TestRegisterDynamicClient_MissingRegistrationEndpoint tests that registerDynamicClient
// returns a clear error message when the OIDC discovery document doesn't include
// a registration_endpoint (provider doesn't support DCR).
func TestRegisterDynamicClient_MissingRegistrationEndpoint(t *testing.T) {
t.Parallel()

ctx := context.Background()

// Create a discovery document without registration_endpoint
discoveredDoc := &oauth.OIDCDiscoveryDocument{
Issuer: "https://auth.example.com",
AuthorizationEndpoint: "https://auth.example.com/oauth/authorize",
TokenEndpoint: "https://auth.example.com/oauth/token",
JWKSURI: "https://auth.example.com/oauth/jwks",
// Note: RegistrationEndpoint is intentionally omitted (empty string)
RegistrationEndpoint: "",
}

config := &OAuthFlowConfig{
Scopes: []string{"openid", "profile"},
CallbackPort: 8765,
}

// Call registerDynamicClient with a discovery document missing registration_endpoint
result, err := registerDynamicClient(ctx, config, discoveredDoc)

// Should return an error
require.Error(t, err)
assert.Nil(t, result)

// Error message should clearly indicate DCR is not supported
assert.Contains(t, err.Error(), "does not support Dynamic Client Registration")
assert.Contains(t, err.Error(), "DCR")

// Error message should provide actionable guidance
assert.Contains(t, err.Error(), "--remote-auth-client-id")
assert.Contains(t, err.Error(), "--remote-auth-client-secret")
}
15 changes: 15 additions & 0 deletions pkg/auth/oauth/dynamic_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,21 @@ func handleHTTPResponse(resp *http.Response) (*DynamicClientRegistrationResponse
if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK {
// Try to read error response
errorBody, _ := io.ReadAll(resp.Body)

// Detect if DCR is not supported by the provider
// Common HTTP status codes when DCR is unsupported:
// - 404 Not Found: endpoint doesn't exist
// - 405 Method Not Allowed: endpoint exists but POST not allowed
// - 501 Not Implemented: DCR feature not implemented
if resp.StatusCode == http.StatusNotFound ||
resp.StatusCode == http.StatusMethodNotAllowed ||
resp.StatusCode == http.StatusNotImplemented {
return nil, fmt.Errorf("this provider does not support Dynamic Client Registration (DCR) - HTTP %d. "+
"Please configure OAuth client credentials using --remote-auth-client-id and --remote-auth-client-secret flags, "+
"or register a client manually with the provider. Error details: %s",
resp.StatusCode, string(errorBody))
}

return nil, fmt.Errorf("dynamic client registration failed with status %d: %s", resp.StatusCode, string(errorBody))
}

Expand Down
30 changes: 30 additions & 0 deletions pkg/auth/oauth/dynamic_registration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,36 @@ func TestRegisterClientDynamically(t *testing.T) {
responseStatus: http.StatusBadRequest,
expectedError: true,
},
{
name: "DCR not supported - 404 Not Found",
request: &DynamicClientRegistrationRequest{
ClientName: "Test Client",
RedirectURIs: []string{"http://localhost:8080/callback"},
},
response: `{"error": "not_found"}`,
responseStatus: http.StatusNotFound,
expectedError: true,
},
{
name: "DCR not supported - 405 Method Not Allowed",
request: &DynamicClientRegistrationRequest{
ClientName: "Test Client",
RedirectURIs: []string{"http://localhost:8080/callback"},
},
response: `{"error": "method_not_allowed"}`,
responseStatus: http.StatusMethodNotAllowed,
expectedError: true,
},
{
name: "DCR not supported - 501 Not Implemented",
request: &DynamicClientRegistrationRequest{
ClientName: "Test Client",
RedirectURIs: []string{"http://localhost:8080/callback"},
},
response: `{"error": "not_implemented", "error_description": "Dynamic Client Registration is not supported"}`,
responseStatus: http.StatusNotImplemented,
expectedError: true,
},
{
name: "invalid request - no redirect URIs",
request: &DynamicClientRegistrationRequest{
Expand Down
Loading