Skip to content

Commit f4e769f

Browse files
auth: fix 2025-03-26 backcompat (#821)
1 parent 455fd93 commit f4e769f

2 files changed

Lines changed: 88 additions & 69 deletions

File tree

auth/authorization_code.go

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ func (h *AuthorizationCodeHandler) Authorize(ctx context.Context, req *http.Requ
230230
Scopes: scps,
231231
}
232232

233-
authRes, err := h.getAuthorizationCode(ctx, cfg, req.URL.String())
233+
authRes, err := h.getAuthorizationCode(ctx, cfg, prm.Resource)
234234
if err != nil {
235235
// Purposefully leaving the error unwrappable so it can be handled by the caller.
236236
return err
@@ -289,9 +289,24 @@ func (h *AuthorizationCodeHandler) getProtectedResourceMetadata(ctx context.Cont
289289
errs = append(errs, fmt.Errorf("protected resource metadata is nil"))
290290
continue
291291
}
292+
if len(prm.AuthorizationServers) == 0 {
293+
// If we found PRM, we enforce the 2025-11-25 spec and not search further.
294+
return nil, fmt.Errorf("protected resource metadata has no authorization servers specified")
295+
}
292296
return prm, nil
293297
}
294-
return nil, fmt.Errorf("failed to get protected resource metadata: %v", errors.Join(errs...))
298+
// Fallback to 2025-03-26 spec MCP server root is the Authorization Server:
299+
// https://modelcontextprotocol.io/specification/2025-03-26/basic/authorization#server-metadata-discovery
300+
u, err := url.Parse(mcpServerURL)
301+
if err != nil {
302+
return nil, fmt.Errorf("failed to parse MCP server URL: %v", err)
303+
}
304+
u.Path = ""
305+
prm := &oauthex.ProtectedResourceMetadata{
306+
AuthorizationServers: []string{u.String()},
307+
Resource: mcpServerURL,
308+
}
309+
return prm, nil
295310
}
296311

297312
type prmURL struct {
@@ -335,26 +350,14 @@ func protectedResourceMetadataURLs(metadataURL, resourceURL string) []prmURL {
335350
}
336351

337352
// getAuthServerMetadata returns the authorization server metadata.
338-
// The provided Protected Resource Metadata must not be nil.
353+
// The provided Protected Resource Metadata must not be nil and must contain
354+
// at least one authorization server.
339355
// It returns an error if the metadata request fails with non-4xx HTTP status code
340356
// or the fetched metadata fails security checks.
341357
// If no metadata was found, it returns a minimal set of endpoints
342358
// as a fallback to 2025-03-26 spec.
343359
func (h *AuthorizationCodeHandler) getAuthServerMetadata(ctx context.Context, prm *oauthex.ProtectedResourceMetadata) (*oauthex.AuthServerMeta, error) {
344-
var authServerURL string
345-
if len(prm.AuthorizationServers) > 0 {
346-
// Use the first authorization server, similarly to other SDKs.
347-
authServerURL = prm.AuthorizationServers[0]
348-
} else {
349-
// Fallback to 2025-03-26 spec: MCP server base URL acts as Authorization Server.
350-
authURL, err := url.Parse(prm.Resource)
351-
if err != nil {
352-
return nil, fmt.Errorf("failed to parse resource URL: %v", err)
353-
}
354-
authURL.Path = ""
355-
authServerURL = authURL.String()
356-
}
357-
360+
authServerURL := prm.AuthorizationServers[0]
358361
for _, u := range authorizationServerMetadataURLs(authServerURL) {
359362
asm, err := oauthex.GetAuthServerMeta(ctx, u, authServerURL, http.DefaultClient)
360363
if err != nil {

auth/authorization_code_test.go

Lines changed: 68 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"net/http"
1313
"net/http/httptest"
1414
"net/http/httputil"
15+
"strings"
1516
"testing"
1617

1718
"github.com/google/go-cmp/cmp"
@@ -322,43 +323,40 @@ func TestNewAuthorizationCodeHandler_Error(t *testing.T) {
322323
}
323324
}
324325

325-
func TestGetProtectedResourceMetadata(t *testing.T) {
326+
func TestGetProtectedResourceMetadata_Success(t *testing.T) {
326327
handler := &AuthorizationCodeHandler{} // No config needed for this method
327328
pathForChallenge := "/protected-resource"
328329

329330
tests := []struct {
330331
name string
331332
challengesProvided bool
332-
prmPath string
333-
resourcePath string
334-
wantError bool
333+
// Path of the PRM endpoint.
334+
prmPath string
335+
// Path of the MCP server that is accessed.
336+
mcpServerPath string
337+
// Path for the Resource expected in the returned PRM.
338+
resourcePath string
335339
}{
336340
{
337341
name: "FromChallenges",
338342
challengesProvided: true,
339343
prmPath: pathForChallenge,
344+
mcpServerPath: "/resource",
340345
resourcePath: "/resource",
341-
wantError: false,
342346
},
343347
{
344348
name: "FallbackToEndpoint",
345349
challengesProvided: false,
346350
prmPath: "/.well-known/oauth-protected-resource/resource",
351+
mcpServerPath: "/resource",
347352
resourcePath: "/resource",
348-
wantError: false,
349353
},
350354
{
351355
name: "FallbackToRoot",
352356
challengesProvided: false,
353357
prmPath: "/.well-known/oauth-protected-resource",
358+
mcpServerPath: "/resource",
354359
resourcePath: "",
355-
wantError: false,
356-
},
357-
{
358-
name: "NoMetadata",
359-
challengesProvided: false,
360-
prmPath: "/incorrect",
361-
wantError: true,
362360
},
363361
}
364362

@@ -367,10 +365,10 @@ func TestGetProtectedResourceMetadata(t *testing.T) {
367365
mux := http.NewServeMux()
368366
server := httptest.NewServer(mux)
369367
t.Cleanup(server.Close)
370-
resourceURL := server.URL + tt.resourcePath
371368
metadata := &oauthex.ProtectedResourceMetadata{
372-
Resource: resourceURL,
373-
ScopesSupported: []string{"read", "write"},
369+
Resource: server.URL + tt.resourcePath,
370+
AuthorizationServers: []string{"https://oauth.example.com"},
371+
ScopesSupported: []string{"read", "write"},
374372
}
375373
mux.Handle(tt.prmPath, ProtectedResourceMetadataHandler(metadata))
376374
var challenges []oauthex.Challenge
@@ -385,12 +383,9 @@ func TestGetProtectedResourceMetadata(t *testing.T) {
385383
}
386384
}
387385

388-
got, err := handler.getProtectedResourceMetadata(t.Context(), challenges, resourceURL)
386+
got, err := handler.getProtectedResourceMetadata(t.Context(), challenges, server.URL+tt.mcpServerPath)
389387
if err != nil {
390-
if !tt.wantError {
391-
t.Fatalf("getProtectedResourceMetadata() error = %v, want nil", err)
392-
}
393-
return
388+
t.Fatalf("getProtectedResourceMetadata() error = %v", err)
394389
}
395390
if got == nil {
396391
t.Fatal("getProtectedResourceMetadata() got nil, want metadata")
@@ -402,59 +397,86 @@ func TestGetProtectedResourceMetadata(t *testing.T) {
402397
}
403398
}
404399

400+
func TestGetProtectedResourceMetadata_Backcompat(t *testing.T) {
401+
var challenges []oauthex.Challenge
402+
handler := &AuthorizationCodeHandler{} // No config needed for this method
403+
got, err := handler.getProtectedResourceMetadata(t.Context(), challenges, "http://localhost:1234/resource")
404+
if err != nil {
405+
t.Fatalf("getProtectedResourceMetadata() error = %v", err)
406+
}
407+
wantPRM := &oauthex.ProtectedResourceMetadata{
408+
Resource: "http://localhost:1234/resource",
409+
AuthorizationServers: []string{"http://localhost:1234"},
410+
}
411+
if diff := cmp.Diff(wantPRM, got); diff != "" {
412+
t.Errorf("getProtectedResourceMetadata() metadata mismatch (-want +got):\n%s", diff)
413+
}
414+
}
415+
416+
func TestGetProtectedResourceMetadata_Error(t *testing.T) {
417+
mux := http.NewServeMux()
418+
server := httptest.NewServer(mux)
419+
t.Cleanup(server.Close)
420+
metadata := &oauthex.ProtectedResourceMetadata{
421+
Resource: server.URL + "/resource",
422+
AuthorizationServers: nil, // Empty list is invalid
423+
ScopesSupported: []string{"read", "write"},
424+
}
425+
mux.Handle("/.well-known/oauth-protected-resource/resource", ProtectedResourceMetadataHandler(metadata))
426+
var challenges []oauthex.Challenge
427+
handler := &AuthorizationCodeHandler{} // No config needed for this method
428+
got, err := handler.getProtectedResourceMetadata(t.Context(), challenges, server.URL+"/resource")
429+
if err == nil || !strings.Contains(err.Error(), "authorization servers") {
430+
t.Errorf("getProtectedResourceMetadata() = %v, want error containing \"authorization servers\"", err)
431+
}
432+
if got != nil {
433+
t.Errorf("getProtectedResourceMetadata() = %+v, want nil", got)
434+
}
435+
}
436+
405437
func TestGetAuthServerMetadata(t *testing.T) {
406438
handler := &AuthorizationCodeHandler{} // No config needed for this method
407439

408440
tests := []struct {
409-
name string
410-
authorizationAtMCPServer bool
411-
issuerPath string
412-
endpointConfig *oauthtest.MetadataEndpointConfig
441+
name string
442+
issuerPath string
443+
endpointConfig *oauthtest.MetadataEndpointConfig
413444
}{
414445
{
415-
name: "OAuthEndpoint_Root",
416-
authorizationAtMCPServer: false,
417-
issuerPath: "",
446+
name: "OAuthEndpoint_Root",
447+
issuerPath: "",
418448
endpointConfig: &oauthtest.MetadataEndpointConfig{
419449
ServeOAuthInsertedEndpoint: true,
420450
},
421451
},
422452
{
423-
name: "OpenIDEndpoint_Root",
424-
authorizationAtMCPServer: false,
425-
issuerPath: "",
453+
name: "OpenIDEndpoint_Root",
454+
issuerPath: "",
426455
endpointConfig: &oauthtest.MetadataEndpointConfig{
427456
ServeOpenIDInsertedEndpoint: true,
428457
},
429458
},
430459
{
431-
name: "OAuthEndpoint_Path",
432-
authorizationAtMCPServer: false,
433-
issuerPath: "/oauth",
460+
name: "OAuthEndpoint_Path",
461+
issuerPath: "/oauth",
434462
endpointConfig: &oauthtest.MetadataEndpointConfig{
435463
ServeOAuthInsertedEndpoint: true,
436464
},
437465
},
438466
{
439-
name: "OpenIDEndpoint_Path",
440-
authorizationAtMCPServer: false,
441-
issuerPath: "/openid",
467+
name: "OpenIDEndpoint_Path",
468+
issuerPath: "/openid",
442469
endpointConfig: &oauthtest.MetadataEndpointConfig{
443470
ServeOpenIDInsertedEndpoint: true,
444471
},
445472
},
446473
{
447-
name: "OpenIDAppendedEndpoint_Path",
448-
authorizationAtMCPServer: false,
449-
issuerPath: "/openid",
474+
name: "OpenIDAppendedEndpoint_Path",
475+
issuerPath: "/openid",
450476
endpointConfig: &oauthtest.MetadataEndpointConfig{
451477
ServeOpenIDAppendedEndpoint: true,
452478
},
453479
},
454-
{
455-
name: "FallbackToMCPServer",
456-
authorizationAtMCPServer: true,
457-
},
458480
{
459481
name: "NoMetadata",
460482
issuerPath: "",
@@ -474,15 +496,9 @@ func TestGetAuthServerMetadata(t *testing.T) {
474496
})
475497
s.Start(t)
476498
issuerURL := s.URL() + tt.issuerPath
477-
resourceURL := "https://example.com/resource"
478-
authServers := []string{issuerURL}
479-
if tt.authorizationAtMCPServer {
480-
resourceURL = issuerURL
481-
authServers = nil
482-
}
483499
prm := &oauthex.ProtectedResourceMetadata{
484-
Resource: resourceURL,
485-
AuthorizationServers: authServers,
500+
Resource: "https://example.com/resource",
501+
AuthorizationServers: []string{issuerURL},
486502
}
487503

488504
got, err := handler.getAuthServerMetadata(t.Context(), prm)

0 commit comments

Comments
 (0)