diff --git a/auth/authorization_code.go b/auth/authorization_code.go index 1673a2e60..f349842f4 100644 --- a/auth/authorization_code.go +++ b/auth/authorization_code.go @@ -124,6 +124,11 @@ type AuthorizationCodeHandlerConfig struct { // See https://modelcontextprotocol.io/seps/2207-oidc-refresh-token-guidance. RequestRefreshToken bool + // AcceptUnadvertisedIss accepts a matching RFC 9207 iss even when the + // authorization server metadata omits authorization_response_iss_parameter_supported. + // The zero value (false) keeps the historical reject-unadvertised-iss behavior. + AcceptUnadvertisedIss bool + // Client is an optional HTTP client to use for HTTP requests. // It is used for the following requests: // - Fetching Protected Resource Metadata @@ -367,7 +372,7 @@ func (h *AuthorizationCodeHandler) Authorize(ctx context.Context, req *http.Requ // Purposefully leaving the error unwrappable so it can be handled by the caller. return err } - if err := validateIssuerResponse(authRes.Iss, asm.Issuer, asm.AuthorizationResponseIssParameterSupported); err != nil { + if err := validateIssuerResponse(authRes.Iss, asm.Issuer, asm.AuthorizationResponseIssParameterSupported, h.config.AcceptUnadvertisedIss); err != nil { return err } @@ -616,10 +621,14 @@ func (h *AuthorizationCodeHandler) getAuthorizationCode(ctx context.Context, cfg } // validateIssuerResponse validates the "iss" parameter in an authorization response -// per [RFC 9207]. +// per [RFC 9207]. When the server advertises authorization_response_iss_parameter_supported, +// iss is required and must match expectedIssuer. When it does not advertise support, +// an empty iss is accepted; a present iss is compared to expectedIssuer (RFC 9207 ยง2.4) +// and a mismatch is always rejected. A matching unadvertised iss is accepted only when +// acceptUnadvertisedIss is true (local policy); the default is the historical reject. // // [RFC 9207]: https://www.rfc-editor.org/rfc/rfc9207 -func validateIssuerResponse(iss, expectedIssuer string, issParameterSupported bool) error { +func validateIssuerResponse(iss, expectedIssuer string, issParameterSupported, acceptUnadvertisedIss bool) error { if issParameterSupported { if iss == "" { return fmt.Errorf("authorization server advertises RFC 9207 iss parameter support but none was received in the authorization response") @@ -627,12 +636,17 @@ func validateIssuerResponse(iss, expectedIssuer string, issParameterSupported bo if iss != expectedIssuer { return fmt.Errorf("authorization response issuer %q does not match expected issuer %q", iss, expectedIssuer) } - } else { - if iss != "" { - return fmt.Errorf("authorization server does not advertise RFC 9207 iss parameter support but iss was received in the authorization response") - } + return nil + } + if iss == "" { + return nil + } + if iss != expectedIssuer { + return fmt.Errorf("authorization response issuer %q does not match expected issuer %q", iss, expectedIssuer) + } + if !acceptUnadvertisedIss { + return fmt.Errorf("authorization server does not advertise RFC 9207 iss parameter support but iss was received in the authorization response") } - return nil } diff --git a/auth/authorization_code_test.go b/auth/authorization_code_test.go index c2f963f7a..69a7ca60a 100644 --- a/auth/authorization_code_test.go +++ b/auth/authorization_code_test.go @@ -919,11 +919,12 @@ func TestValidateIssuerResponse(t *testing.T) { const expectedIssuer = "https://auth.example.com" tests := []struct { - name string - iss string - issSupported bool - wantErr bool - wantErrContains string + name string + iss string + issSupported bool + acceptUnadvertisedIss bool + wantErr bool + wantErrContains string }{ { name: "ValidIss", @@ -949,11 +950,41 @@ func TestValidateIssuerResponse(t *testing.T) { iss: "", issSupported: false, }, + { + // Default / zero-value policy: reject a matching unadvertised iss + // (released v1.7.0 behavior). + name: "UnadvertisedIssCorrectRejectedByDefault", + iss: expectedIssuer, + issSupported: false, + wantErr: true, + wantErrContains: "does not advertise", + }, + { + name: "UnadvertisedIssCorrectAcceptedWhenOptedIn", + iss: expectedIssuer, + issSupported: false, + acceptUnadvertisedIss: true, + }, + { + name: "UnadvertisedIssWrong", + iss: "https://attacker.example.com", + issSupported: false, + wantErr: true, + wantErrContains: "does not match expected issuer", + }, + { + name: "UnadvertisedIssWrongStillRejectedWhenOptedIn", + iss: "https://attacker.example.com", + issSupported: false, + acceptUnadvertisedIss: true, + wantErr: true, + wantErrContains: "does not match expected issuer", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateIssuerResponse(tt.iss, expectedIssuer, tt.issSupported) + err := validateIssuerResponse(tt.iss, expectedIssuer, tt.issSupported, tt.acceptUnadvertisedIss) if tt.wantErr { if err == nil { t.Fatalf("validateIssuerResponse() = nil, want error containing %q", tt.wantErrContains) @@ -968,6 +999,112 @@ func TestValidateIssuerResponse(t *testing.T) { } } +// TestAuthorize_AcceptUnadvertisedIssPlumbing checks that Authorize passes +// AuthorizationCodeHandlerConfig.AcceptUnadvertisedIss through to +// validateIssuerResponse. The fake server returns iss without advertising +// authorization_response_iss_parameter_supported. +func TestAuthorize_AcceptUnadvertisedIssPlumbing(t *testing.T) { + tests := []struct { + name string + acceptUnadvertisedIss bool + wantErrContains string + }{ + { + name: "ZeroValueRejects", + wantErrContains: "does not advertise", + }, + { + name: "OptInAccepts", + acceptUnadvertisedIss: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authServer := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{ + UnadvertiseIssParameter: true, + RegistrationConfig: &oauthtest.RegistrationConfig{ + PreregisteredClients: map[string]oauthtest.ClientInfo{ + "test_client_id": { + Secret: "test_client_secret", + RedirectURIs: []string{"http://localhost:12345/callback"}, + }, + }, + }, + }) + authServer.Start(t) + + resourceMux := http.NewServeMux() + resourceServer := httptest.NewServer(resourceMux) + t.Cleanup(resourceServer.Close) + resourceURL := resourceServer.URL + "/resource" + resourceMux.Handle("/.well-known/oauth-protected-resource/resource", ProtectedResourceMetadataHandler(&oauthex.ProtectedResourceMetadata{ + Resource: resourceURL, + AuthorizationServers: []string{authServer.URL()}, + })) + + handler, err := NewAuthorizationCodeHandler(&AuthorizationCodeHandlerConfig{ + RedirectURL: "http://localhost:12345/callback", + AcceptUnadvertisedIss: tt.acceptUnadvertisedIss, + PreregisteredClient: &oauthex.ClientCredentials{ + ClientID: "test_client_id", + ClientSecretAuth: &oauthex.ClientSecretAuth{ + ClientSecret: "test_client_secret", + }, + }, + AuthorizationCodeFetcher: func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) { + client := &http.Client{ + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } + resp, err := client.Get(args.URL) + if err != nil { + return nil, fmt.Errorf("failed to visit auth URL: %v", err) + } + defer resp.Body.Close() + location, err := resp.Location() + if err != nil { + return nil, fmt.Errorf("failed to get location header: %v", err) + } + return &AuthorizationResult{ + Code: location.Query().Get("code"), + State: location.Query().Get("state"), + Iss: location.Query().Get("iss"), + }, nil + }, + }) + if err != nil { + t.Fatalf("NewAuthorizationCodeHandler failed: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, resourceURL, nil) + resp := &http.Response{ + StatusCode: http.StatusUnauthorized, + Header: make(http.Header), + Body: http.NoBody, + Request: req, + } + resp.Header.Set( + "WWW-Authenticate", + "Bearer resource_metadata="+resourceServer.URL+"/.well-known/oauth-protected-resource/resource", + ) + err = handler.Authorize(context.Background(), req, resp) + if tt.wantErrContains != "" { + if err == nil { + t.Fatalf("Authorize() = nil, want error containing %q", tt.wantErrContains) + } + if !strings.Contains(err.Error(), tt.wantErrContains) { + t.Fatalf("Authorize() error = %q, want it to contain %q", err.Error(), tt.wantErrContains) + } + return + } + if err != nil { + t.Fatalf("Authorize() unexpected error = %v", err) + } + }) + } +} + func TestInferApplicationType(t *testing.T) { tests := []struct { name string diff --git a/internal/oauthtest/fake_authorization_server.go b/internal/oauthtest/fake_authorization_server.go index e5134fb50..4cd68264a 100644 --- a/internal/oauthtest/fake_authorization_server.go +++ b/internal/oauthtest/fake_authorization_server.go @@ -97,6 +97,11 @@ type Config struct { // IssueRefreshToken, if true, includes a refresh_token in token responses and // enables grant_type=refresh_token at the /token endpoint. IssueRefreshToken bool + // UnadvertiseIssParameter, if true, omits + // authorization_response_iss_parameter_supported from metadata while the + // authorize endpoint still includes iss. Used to test local policy for + // unadvertised RFC 9207 iss. + UnadvertiseIssParameter bool } // testRefreshToken is the refresh token issued and accepted by the fake server @@ -199,8 +204,9 @@ func (s *FakeAuthorizationServer) handleMetadata(w http.ResponseWriter, r *http. CodeChallengeMethodsSupported: []string{"S256"}, ClientIDMetadataDocumentSupported: cimdSupported, TokenEndpointAuthMethodsSupported: []string{"client_secret_post", "client_secret_basic"}, - // Advertise RFC 9207 support: the authorize endpoint includes "iss" in responses. - AuthorizationResponseIssParameterSupported: true, + // Advertise RFC 9207 support unless UnadvertiseIssParameter is set: + // the authorize endpoint still includes "iss" in responses either way. + AuthorizationResponseIssParameterSupported: !s.config.UnadvertiseIssParameter, } // Set CORS headers for cross-origin client discovery. w.Header().Set("Access-Control-Allow-Origin", "*")