From 3b190e36f39457c50f9da675c8f9688b450abddf Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 7 Oct 2025 12:28:34 +0100 Subject: [PATCH] internal/oauthex: fix GetProtectedResourceMetadataFromHeader validation GetProtectedResourceMetadataFromHeader was validating the resource field against the metadata endpoint URL instead of the original server URL. Per RFC 9728 section 3.3, the resource field must match the URL that the client used to make the request to the resource server. Fixes: #560 This fix adds a serverURL parameter and validates against the correct value. --- oauthex/oauth2_test.go | 58 ++++++++++++++++++++++++++++++++++++---- oauthex/resource_meta.go | 17 ++++++------ 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/oauthex/oauth2_test.go b/oauthex/oauth2_test.go index e4c71cbe..08d2d314 100644 --- a/oauthex/oauth2_test.go +++ b/oauthex/oauth2_test.go @@ -212,14 +212,15 @@ func TestGetProtectedResourceMetadata(t *testing.T) { server := httptest.NewTLSServer(h) h.installHandlers(server.URL) client := server.Client() - res, err := client.Get(server.URL + "/resource") + serverURL := server.URL + "/resource" + res, err := client.Get(serverURL) if err != nil { t.Fatal(err) } if res.StatusCode != http.StatusUnauthorized { t.Fatal("want unauth") } - prm, err := GetProtectedResourceMetadataFromHeader(ctx, res.Header, client) + prm, err := GetProtectedResourceMetadataFromHeader(ctx, serverURL, res.Header, client) if err != nil { t.Fatal(err) } @@ -240,11 +241,53 @@ func TestGetProtectedResourceMetadata(t *testing.T) { t.Fatal("nil prm") } }) + // Test that metadata URL and resource identifier are properly distinguished (issue #560). + t.Run("FromHeaderValidatesAgainstServerURL", func(t *testing.T) { + h := &fakeResourceHandler{serveWWWAuthenticate: true} + server := httptest.NewTLSServer(h) + h.installHandlers(server.URL) + client := server.Client() + serverURL := server.URL + "/resource" + res, err := client.Get(serverURL) + if err != nil { + t.Fatal(err) + } + // This should succeed because we validate against serverURL, not metadataURL + prm, err := GetProtectedResourceMetadataFromHeader(ctx, serverURL, res.Header, client) + if err != nil { + t.Fatalf("Expected validation to succeed, got error: %v", err) + } + if prm == nil { + t.Fatal("Expected non-nil prm") + } + if prm.Resource != serverURL { + t.Errorf("Expected resource %q, got %q", serverURL, prm.Resource) + } + }) + t.Run("FromHeaderRejectsImpersonation", func(t *testing.T) { + h := &fakeResourceHandler{serveWWWAuthenticate: true, resourceOverride: "https://attacker.com/evil"} + server := httptest.NewTLSServer(h) + h.installHandlers(server.URL) + client := server.Client() + serverURL := server.URL + "/resource" + res, err := client.Get(serverURL) + if err != nil { + t.Fatal(err) + } + prm, err := GetProtectedResourceMetadataFromHeader(ctx, serverURL, res.Header, client) + if err == nil { + t.Fatal("Expected validation error for mismatched resource, got nil") + } + if prm != nil { + t.Fatal("Expected nil prm on validation failure") + } + }) } type fakeResourceHandler struct { http.ServeMux serveWWWAuthenticate bool + resourceOverride string // If set, use this instead of correct resource (for testing validation) } func (h *fakeResourceHandler) installHandlers(serverURL string) { @@ -258,11 +301,16 @@ func (h *fakeResourceHandler) installHandlers(serverURL string) { })) h.Handle("GET "+path, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - // If there is a WWW-Authenticate header, the resource field is the value of that header. - // If not, it's the server URL without the "/.well-known/..." part. + // Per RFC 9728 section 3.3, the resource field should contain the actual resource identifier, + // which is the URL the client uses to access the resource (serverURL + "/resource" for WWW-Authenticate case). + // For the FromID test case, it's just the serverURL. resource := serverURL if h.serveWWWAuthenticate { - resource = url + resource = serverURL + "/resource" + } + // Allow testing with custom resource values (e.g., impersonation attacks). + if h.resourceOverride != "" { + resource = h.resourceOverride } prm := &ProtectedResourceMetadata{Resource: resource} if err := json.NewEncoder(w).Encode(prm); err != nil { diff --git a/oauthex/resource_meta.go b/oauthex/resource_meta.go index e584b068..bb61f797 100644 --- a/oauthex/resource_meta.go +++ b/oauthex/resource_meta.go @@ -53,10 +53,11 @@ func GetProtectedResourceMetadataFromID(ctx context.Context, resourceID string, // GetProtectedResourceMetadataFromHeader retrieves protected resource metadata // using information in the given header, using the given client (or the default // client if nil). -// It issues a GET request to a URL discovered by parsing the WWW-Authenticate headers in the given request, -// It then validates the resource field of the resulting metadata against the given URL. -// If there is no URL in the request, it returns nil, nil. -func GetProtectedResourceMetadataFromHeader(ctx context.Context, header http.Header, c *http.Client) (_ *ProtectedResourceMetadata, err error) { +// It issues a GET request to a URL discovered by parsing the WWW-Authenticate headers in the given request. +// Per RFC 9728 section 3.3, it validates that the resource field of the resulting metadata +// matches the serverURL (the URL that the client used to make the original request to the resource server). +// If there is no metadata URL in the header, it returns nil, nil. +func GetProtectedResourceMetadataFromHeader(ctx context.Context, serverURL string, header http.Header, c *http.Client) (_ *ProtectedResourceMetadata, err error) { defer util.Wrapf(&err, "GetProtectedResourceMetadataFromHeader") headers := header[http.CanonicalHeaderKey("WWW-Authenticate")] if len(headers) == 0 { @@ -66,11 +67,11 @@ func GetProtectedResourceMetadataFromHeader(ctx context.Context, header http.Hea if err != nil { return nil, err } - url := ResourceMetadataURL(cs) - if url == "" { + metadataURL := ResourceMetadataURL(cs) + if metadataURL == "" { return nil, nil } - return getPRM(ctx, url, c, url) + return getPRM(ctx, metadataURL, c, serverURL) } // getPRM makes a GET request to the given URL, and validates the response. @@ -83,7 +84,7 @@ func getPRM(ctx context.Context, purl string, c *http.Client, wantResource strin if err != nil { return nil, err } - // Validate the Resource field to thwart impersonation attacks (section 3.3). + // Validate the Resource field (see RFC 9728, section 3.3). if prm.Resource != wantResource { return nil, fmt.Errorf("got metadata resource %q, want %q", prm.Resource, wantResource) }