Skip to content

fix(oauth): Honor resource_metadata in WWW-Authenticate fallback #794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Gujiassh wants to merge 2 commits into from
Closed

fix(oauth): Honor resource_metadata in WWW-Authenticate fallback #794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2471bcf
fix(oauth): honor resource_metadata from WWW-Authenticate
Gujiassh Apr 11, 2026
6c05322
fix(oauth): parse spaced resource_metadata params
Gujiassh Apr 15, 2026
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
81 changes: 81 additions & 0 deletions client/transport/oauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,36 @@ func (h *OAuthHandler) getServerMetadata(ctx context.Context) (*AuthServerMetada

// If we can't get the protected resource metadata, try OAuth Authorization Server discovery
if resp.StatusCode != http.StatusOK {
// RFC 9728 allows the server to advertise a protected resource metadata URL
// via the WWW-Authenticate header when direct discovery fails.
if resourceMetadataURL := extractResourceMetadataURL(resp.Header.Values("WWW-Authenticate")); resourceMetadataURL != "" {
protectedResource, err := h.fetchProtectedResourceFromURL(ctx, resourceMetadataURL)
if err == nil && len(protectedResource.AuthorizationServers) > 0 {
authServerURL := protectedResource.AuthorizationServers[0]
authMetadataURL, err := buildWellKnownURL(authServerURL, "oauth-authorization-server")
if err == nil {
h.fetchMetadataFromURL(ctx, authMetadataURL)
if h.serverMetadata != nil {
return
}
}

openidMetadataURL, err := buildWellKnownURL(authServerURL, "openid-configuration")
if err == nil {
h.fetchMetadataFromURL(ctx, openidMetadataURL)
if h.serverMetadata != nil {
Comment on lines +407 to +418

@coderabbitai coderabbitai Bot Apr 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't latch the first metadata error in this fallback chain.

Lines 409-417 call fetchMetadataFromURL, which writes h.metadataFetchErr immediately. If oauth-authorization-server fails with a transport/decode error but openid-configuration succeeds, h.serverMetadata gets populated but getServerMetadata() still returns the stale first error. That breaks this new RFC 9728 fallback whenever the first metadata document is malformed but a later fallback is usable. Please keep per-attempt errors local and only assign metadataFetchErr after all fallbacks fail.

return
}
}

metadata, err := h.getDefaultEndpoints(authServerURL)
if err == nil {
h.serverMetadata = metadata
return
}
}
}

authMetadataURL, err := buildWellKnownURL(baseURL, "oauth-authorization-server")
if err != nil {
h.metadataFetchErr = fmt.Errorf("failed to build authorization server metadata URL: %w", err)
Expand Down Expand Up @@ -495,6 +525,57 @@ func buildWellKnownURL(baseURL string, suffix string) (string, error) {
return root + "/.well-known/" + suffix + path, nil
}

func extractResourceMetadataURL(wwwAuthenticateHeaders []string) string {
for _, header := range wwwAuthenticateHeaders {
for _, param := range strings.Split(header, ",") {
param = strings.TrimSpace(param)
if param == "" {
continue
}

key, value, found := strings.Cut(param, "=")
if !found || !strings.EqualFold(strings.TrimSpace(key), "resource_metadata") {
continue
}

value = strings.TrimSpace(value)
value = strings.Trim(value, "\"")
if value != "" {
return value
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

return ""
}

func (h *OAuthHandler) fetchProtectedResourceFromURL(ctx context.Context, protectedResourceURL string) (*OAuthProtectedResource, error) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, protectedResourceURL, nil)
if err != nil {
return nil, fmt.Errorf("failed to create protected resource request: %w", err)
}

req.Header.Set("Accept", "application/json")
req.Header.Set("MCP-Protocol-Version", "2025-03-26")

resp, err := h.httpClient.Do(req)
if err != nil {
return nil, fmt.Errorf("failed to send protected resource request: %w", err)
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("protected resource request failed with status %d", resp.StatusCode)
}

var protectedResource OAuthProtectedResource
if err := json.NewDecoder(resp.Body).Decode(&protectedResource); err != nil {
return nil, fmt.Errorf("failed to decode protected resource response: %w", err)
}

return &protectedResource, nil
}

// fetchMetadataFromURL fetches and parses OAuth server metadata from a URL
func (h *OAuthHandler) fetchMetadataFromURL(ctx context.Context, metadataURL string) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, metadataURL, nil)
Expand Down
100 changes: 100 additions & 0 deletions client/transport/oauth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,106 @@ func TestOAuthHandler_GetServerMetadata_PathAwareDiscovery(t *testing.T) {
assert.Equal(t, server.URL+"/oauth/googledrive/token", metadata.TokenEndpoint)
}

func TestOAuthHandler_GetServerMetadata_UsesResourceMetadataHeader(t *testing.T) {
protectedResourceRequested := false
headerResourceMetadataRequested := false
authServerRequested := false

var server *httptest.Server
server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/.well-known/oauth-protected-resource":
protectedResourceRequested = true
w.Header().Set("WWW-Authenticate", `Bearer error="invalid_request", resource_metadata="`+server.URL+`/.well-known/oauth-protected-resource/googledrive"`)
w.WriteHeader(http.StatusUnauthorized)
case "/.well-known/oauth-protected-resource/googledrive":
headerResourceMetadataRequested = true
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(OAuthProtectedResource{
AuthorizationServers: []string{server.URL + "/oauth/googledrive"},
})
case "/.well-known/oauth-authorization-server/oauth/googledrive":
authServerRequested = true
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(AuthServerMetadata{
Issuer: server.URL + "/oauth/googledrive",
AuthorizationEndpoint: server.URL + "/oauth/googledrive/authorize",
TokenEndpoint: server.URL + "/oauth/googledrive/token",
RegistrationEndpoint: server.URL + "/oauth/googledrive/register",
})
default:
w.WriteHeader(http.StatusNotFound)
}
}))
defer server.Close()

handler := NewOAuthHandler(OAuthConfig{
ClientID: "test-client",
RedirectURI: "http://localhost/callback",
TokenStore: NewMemoryTokenStore(),
})
handler.SetBaseURL(server.URL)

metadata, err := handler.GetServerMetadata(context.Background())
require.NoError(t, err)
assert.True(t, protectedResourceRequested)
assert.True(t, headerResourceMetadataRequested)
assert.True(t, authServerRequested)
assert.Equal(t, server.URL+"/oauth/googledrive", metadata.Issuer)
assert.Equal(t, server.URL+"/oauth/googledrive/authorize", metadata.AuthorizationEndpoint)
assert.Equal(t, server.URL+"/oauth/googledrive/token", metadata.TokenEndpoint)
}

func TestOAuthHandler_GetServerMetadata_UsesResourceMetadataHeaderWithWhitespace(t *testing.T) {
protectedResourceRequested := false
headerResourceMetadataRequested := false
authServerRequested := false

var server *httptest.Server
server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/.well-known/oauth-protected-resource":
protectedResourceRequested = true
w.Header().Add("WWW-Authenticate", `Bearer error="invalid_request", resource_metadata = "`+server.URL+`/.well-known/oauth-protected-resource/googledrive"`)
w.WriteHeader(http.StatusUnauthorized)
case "/.well-known/oauth-protected-resource/googledrive":
headerResourceMetadataRequested = true
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(OAuthProtectedResource{
AuthorizationServers: []string{server.URL + "/oauth/googledrive"},
})
case "/.well-known/oauth-authorization-server/oauth/googledrive":
authServerRequested = true
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(AuthServerMetadata{
Issuer: server.URL + "/oauth/googledrive",
AuthorizationEndpoint: server.URL + "/oauth/googledrive/authorize",
TokenEndpoint: server.URL + "/oauth/googledrive/token",
RegistrationEndpoint: server.URL + "/oauth/googledrive/register",
})
default:
w.WriteHeader(http.StatusNotFound)
}
}))
defer server.Close()

handler := NewOAuthHandler(OAuthConfig{
ClientID: "test-client",
RedirectURI: "http://localhost/callback",
TokenStore: NewMemoryTokenStore(),
})
handler.SetBaseURL(server.URL)

metadata, err := handler.GetServerMetadata(context.Background())
require.NoError(t, err)
assert.True(t, protectedResourceRequested)
assert.True(t, headerResourceMetadataRequested)
assert.True(t, authServerRequested)
assert.Equal(t, server.URL+"/oauth/googledrive", metadata.Issuer)
assert.Equal(t, server.URL+"/oauth/googledrive/authorize", metadata.AuthorizationEndpoint)
assert.Equal(t, server.URL+"/oauth/googledrive/token", metadata.TokenEndpoint)
}
Comment on lines +990 to +1088

@coderabbitai coderabbitai Bot Apr 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Harden this regression by asserting exact discovery sequence.

Right now, the tests only check that key endpoints were hit. They don’t fail if default fallback endpoints are called unnecessarily. Capture requested paths and assert strict order to lock in precedence (protected-resource → header resource_metadata URL → derived auth-server metadata).

🔎 Suggested test hardening 
+    requestedPaths := make([]string, 0, 4)
     server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+        requestedPaths = append(requestedPaths, r.URL.Path)
         switch r.URL.Path {
         case "/.well-known/oauth-protected-resource":
             ...
         case "/.well-known/oauth-protected-resource/googledrive":
             ...
         case "/.well-known/oauth-authorization-server/oauth/googledrive":
             ...
         default:
             w.WriteHeader(http.StatusNotFound)
         }
     }))
 ...
+    assert.Equal(t, []string{
+        "/.well-known/oauth-protected-resource",
+        "/.well-known/oauth-protected-resource/googledrive",
+        "/.well-known/oauth-authorization-server/oauth/googledrive",
+    }, requestedPaths)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@client/transport/oauth_test.go` around lines 990 - 1088, The tests
TestOAuthHandler_GetServerMetadata_UsesResourceMetadataHeader and
TestOAuthHandler_GetServerMetadata_UsesResourceMetadataHeaderWithWhitespace
currently only assert that endpoints were hit; to harden them, record the
sequence of requested paths (e.g., append r.URL.Path to a requestOrder slice
inside the httptest.Server handler) and after calling
handler.GetServerMetadata(ctx) assert that requestOrder equals the exact
expected sequence ["/.well-known/oauth-protected-resource",
"/.well-known/oauth-protected-resource/googledrive",
"/.well-known/oauth-authorization-server/oauth/googledrive"] to enforce
precedence (protected-resource → header resource_metadata URL → derived
auth-server metadata); update both tests and keep existing boolean flags and
metadata assertions.


// TestOAuthHandler_RefreshToken_GitHubErrorIn200Response tests that we properly detect
// GitHub's non-spec-compliant behavior of returning HTTP 200 with error details in the JSON body
func TestOAuthHandler_RefreshToken_GitHubErrorIn200Response(t *testing.T) {
Expand Down
Loading