Skip to content

Commit 80ebee6

Browse files
authored
Issue 2680 (#2757)
* Improve error messaging when DCR is unsupported for remote MCP servers Add clear, actionable error messages when OAuth providers do not support Dynamic Client Registration (DCR), helping users understand why authentication fails and how to configure client credentials manually. Changes: - Detect missing registration_endpoint in OIDC discovery document - Recognize HTTP status codes indicating DCR is unavailable (404, 405, 501) - Provide actionable guidance directing users to --remote-auth-client-id and --remote-auth-client-secret flags - Add comprehensive test coverage for all DCR unsupported scenarios Error message improvements: - Before: "dynamic client registration failed with status 404: {...}" - After: "this provider does not support Dynamic Client Registration (DCR) - HTTP 404. Please configure OAuth client credentials using --remote-auth-client-id and --remote-auth-client-secret flags, or register a client manually with the provider" Fixes #2680 Signed-off-by: 4t8dd <[email protected]> * fix the lint errors Signed-off-by: 4t8dd <[email protected]> --------- Signed-off-by: 4t8dd <[email protected]>
1 parent c73c5ef commit 80ebee6

File tree

4 files changed

+92
-0
lines changed

4 files changed

+92
-0
lines changed

pkg/auth/discovery/discovery.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,13 @@ func registerDynamicClient(
677677
discoveredDoc *oauth.OIDCDiscoveryDocument,
678678
) (*oauth.DynamicClientRegistrationResponse, error) {
679679

680+
// Check if the provider supports Dynamic Client Registration
681+
if discoveredDoc.RegistrationEndpoint == "" {
682+
return nil, fmt.Errorf("this provider does not support Dynamic Client Registration (DCR). " +
683+
"Please configure OAuth client credentials using --remote-auth-client-id and --remote-auth-client-secret flags, " +
684+
"or register a client manually with the provider")
685+
}
686+
680687
// Use default client name if not provided
681688
registrationRequest := oauth.NewDynamicClientRegistrationRequest(config.Scopes, config.CallbackPort)
682689

pkg/auth/discovery/discovery_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/stretchr/testify/assert"
1515
"github.com/stretchr/testify/require"
1616

17+
"github.com/stacklok/toolhive/pkg/auth/oauth"
1718
"github.com/stacklok/toolhive/pkg/logger"
1819
"github.com/stacklok/toolhive/pkg/networking"
1920
)
@@ -1273,3 +1274,42 @@ func TestTryWellKnownDiscovery_ErrorPaths(t *testing.T) {
12731274
assert.Nil(t, result)
12741275
})
12751276
}
1277+
1278+
// TestRegisterDynamicClient_MissingRegistrationEndpoint tests that registerDynamicClient
1279+
// returns a clear error message when the OIDC discovery document doesn't include
1280+
// a registration_endpoint (provider doesn't support DCR).
1281+
func TestRegisterDynamicClient_MissingRegistrationEndpoint(t *testing.T) {
1282+
t.Parallel()
1283+
1284+
ctx := context.Background()
1285+
1286+
// Create a discovery document without registration_endpoint
1287+
discoveredDoc := &oauth.OIDCDiscoveryDocument{
1288+
Issuer: "https://auth.example.com",
1289+
AuthorizationEndpoint: "https://auth.example.com/oauth/authorize",
1290+
TokenEndpoint: "https://auth.example.com/oauth/token",
1291+
JWKSURI: "https://auth.example.com/oauth/jwks",
1292+
// Note: RegistrationEndpoint is intentionally omitted (empty string)
1293+
RegistrationEndpoint: "",
1294+
}
1295+
1296+
config := &OAuthFlowConfig{
1297+
Scopes: []string{"openid", "profile"},
1298+
CallbackPort: 8765,
1299+
}
1300+
1301+
// Call registerDynamicClient with a discovery document missing registration_endpoint
1302+
result, err := registerDynamicClient(ctx, config, discoveredDoc)
1303+
1304+
// Should return an error
1305+
require.Error(t, err)
1306+
assert.Nil(t, result)
1307+
1308+
// Error message should clearly indicate DCR is not supported
1309+
assert.Contains(t, err.Error(), "does not support Dynamic Client Registration")
1310+
assert.Contains(t, err.Error(), "DCR")
1311+
1312+
// Error message should provide actionable guidance
1313+
assert.Contains(t, err.Error(), "--remote-auth-client-id")
1314+
assert.Contains(t, err.Error(), "--remote-auth-client-secret")
1315+
}

pkg/auth/oauth/dynamic_registration.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,21 @@ func handleHTTPResponse(resp *http.Response) (*DynamicClientRegistrationResponse
264264
if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK {
265265
// Try to read error response
266266
errorBody, _ := io.ReadAll(resp.Body)
267+
268+
// Detect if DCR is not supported by the provider
269+
// Common HTTP status codes when DCR is unsupported:
270+
// - 404 Not Found: endpoint doesn't exist
271+
// - 405 Method Not Allowed: endpoint exists but POST not allowed
272+
// - 501 Not Implemented: DCR feature not implemented
273+
if resp.StatusCode == http.StatusNotFound ||
274+
resp.StatusCode == http.StatusMethodNotAllowed ||
275+
resp.StatusCode == http.StatusNotImplemented {
276+
return nil, fmt.Errorf("this provider does not support Dynamic Client Registration (DCR) - HTTP %d. "+
277+
"Please configure OAuth client credentials using --remote-auth-client-id and --remote-auth-client-secret flags, "+
278+
"or register a client manually with the provider. Error details: %s",
279+
resp.StatusCode, string(errorBody))
280+
}
281+
267282
return nil, fmt.Errorf("dynamic client registration failed with status %d: %s", resp.StatusCode, string(errorBody))
268283
}
269284

pkg/auth/oauth/dynamic_registration_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,36 @@ func TestRegisterClientDynamically(t *testing.T) {
347347
responseStatus: http.StatusBadRequest,
348348
expectedError: true,
349349
},
350+
{
351+
name: "DCR not supported - 404 Not Found",
352+
request: &DynamicClientRegistrationRequest{
353+
ClientName: "Test Client",
354+
RedirectURIs: []string{"http://localhost:8080/callback"},
355+
},
356+
response: `{"error": "not_found"}`,
357+
responseStatus: http.StatusNotFound,
358+
expectedError: true,
359+
},
360+
{
361+
name: "DCR not supported - 405 Method Not Allowed",
362+
request: &DynamicClientRegistrationRequest{
363+
ClientName: "Test Client",
364+
RedirectURIs: []string{"http://localhost:8080/callback"},
365+
},
366+
response: `{"error": "method_not_allowed"}`,
367+
responseStatus: http.StatusMethodNotAllowed,
368+
expectedError: true,
369+
},
370+
{
371+
name: "DCR not supported - 501 Not Implemented",
372+
request: &DynamicClientRegistrationRequest{
373+
ClientName: "Test Client",
374+
RedirectURIs: []string{"http://localhost:8080/callback"},
375+
},
376+
response: `{"error": "not_implemented", "error_description": "Dynamic Client Registration is not supported"}`,
377+
responseStatus: http.StatusNotImplemented,
378+
expectedError: true,
379+
},
350380
{
351381
name: "invalid request - no redirect URIs",
352382
request: &DynamicClientRegistrationRequest{

0 commit comments

Comments
 (0)