Skip to content

Commit f985369

Browse files
committed
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.
1 parent 9348d33 commit f985369

File tree

2 files changed

+62
-13
lines changed

2 files changed

+62
-13
lines changed

oauthex/oauth2_test.go

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,14 +212,15 @@ func TestGetProtectedResourceMetadata(t *testing.T) {
212212
server := httptest.NewTLSServer(h)
213213
h.installHandlers(server.URL)
214214
client := server.Client()
215-
res, err := client.Get(server.URL + "/resource")
215+
serverURL := server.URL + "/resource"
216+
res, err := client.Get(serverURL)
216217
if err != nil {
217218
t.Fatal(err)
218219
}
219220
if res.StatusCode != http.StatusUnauthorized {
220221
t.Fatal("want unauth")
221222
}
222-
prm, err := GetProtectedResourceMetadataFromHeader(ctx, res.Header, client)
223+
prm, err := GetProtectedResourceMetadataFromHeader(ctx, serverURL, res.Header, client)
223224
if err != nil {
224225
t.Fatal(err)
225226
}
@@ -240,11 +241,53 @@ func TestGetProtectedResourceMetadata(t *testing.T) {
240241
t.Fatal("nil prm")
241242
}
242243
})
244+
// Test that metadata URL and resource identifier are properly distinguished (issue #560)
245+
t.Run("FromHeaderValidatesAgainstServerURL", func(t *testing.T) {
246+
h := &fakeResourceHandler{serveWWWAuthenticate: true}
247+
server := httptest.NewTLSServer(h)
248+
h.installHandlers(server.URL)
249+
client := server.Client()
250+
serverURL := server.URL + "/resource"
251+
res, err := client.Get(serverURL)
252+
if err != nil {
253+
t.Fatal(err)
254+
}
255+
// This should succeed because we validate against serverURL, not metadataURL
256+
prm, err := GetProtectedResourceMetadataFromHeader(ctx, serverURL, res.Header, client)
257+
if err != nil {
258+
t.Fatalf("Expected validation to succeed, got error: %v", err)
259+
}
260+
if prm == nil {
261+
t.Fatal("Expected non-nil prm")
262+
}
263+
if prm.Resource != serverURL {
264+
t.Errorf("Expected resource %q, got %q", serverURL, prm.Resource)
265+
}
266+
})
267+
t.Run("FromHeaderRejectsImpersonation", func(t *testing.T) {
268+
h := &fakeResourceHandler{serveWWWAuthenticate: true, resourceOverride: "https://attacker.com/evil"}
269+
server := httptest.NewTLSServer(h)
270+
h.installHandlers(server.URL)
271+
client := server.Client()
272+
serverURL := server.URL + "/resource"
273+
res, err := client.Get(serverURL)
274+
if err != nil {
275+
t.Fatal(err)
276+
}
277+
prm, err := GetProtectedResourceMetadataFromHeader(ctx, serverURL, res.Header, client)
278+
if err == nil {
279+
t.Fatal("Expected validation error for mismatched resource, got nil")
280+
}
281+
if prm != nil {
282+
t.Fatal("Expected nil prm on validation failure")
283+
}
284+
})
243285
}
244286

245287
type fakeResourceHandler struct {
246288
http.ServeMux
247289
serveWWWAuthenticate bool
290+
resourceOverride string // If set, use this instead of correct resource (for testing validation)
248291
}
249292

250293
func (h *fakeResourceHandler) installHandlers(serverURL string) {
@@ -258,11 +301,16 @@ func (h *fakeResourceHandler) installHandlers(serverURL string) {
258301
}))
259302
h.Handle("GET "+path, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
260303
w.Header().Set("Content-Type", "application/json")
261-
// If there is a WWW-Authenticate header, the resource field is the value of that header.
262-
// If not, it's the server URL without the "/.well-known/..." part.
304+
// Per RFC 9728 section 3.3, the resource field should contain the actual resource identifier,
305+
// which is the URL the client uses to access the resource (serverURL + "/resource" for WWW-Authenticate case).
306+
// For the FromID test case, it's just the serverURL.
263307
resource := serverURL
264308
if h.serveWWWAuthenticate {
265-
resource = url
309+
resource = serverURL + "/resource"
310+
}
311+
// Allow testing with custom resource values (e.g., impersonation attacks)
312+
if h.resourceOverride != "" {
313+
resource = h.resourceOverride
266314
}
267315
prm := &ProtectedResourceMetadata{Resource: resource}
268316
if err := json.NewEncoder(w).Encode(prm); err != nil {

oauthex/resource_meta.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,11 @@ func GetProtectedResourceMetadataFromID(ctx context.Context, resourceID string,
5353
// GetProtectedResourceMetadataFromHeader retrieves protected resource metadata
5454
// using information in the given header, using the given client (or the default
5555
// client if nil).
56-
// It issues a GET request to a URL discovered by parsing the WWW-Authenticate headers in the given request,
57-
// It then validates the resource field of the resulting metadata against the given URL.
58-
// If there is no URL in the request, it returns nil, nil.
59-
func GetProtectedResourceMetadataFromHeader(ctx context.Context, header http.Header, c *http.Client) (_ *ProtectedResourceMetadata, err error) {
56+
// It issues a GET request to a URL discovered by parsing the WWW-Authenticate headers in the given request.
57+
// Per RFC 9728 section 3.3, it validates that the resource field of the resulting metadata
58+
// matches the serverURL (the URL that the client used to make the original request to the resource server).
59+
// If there is no metadata URL in the header, it returns nil, nil.
60+
func GetProtectedResourceMetadataFromHeader(ctx context.Context, serverURL string, header http.Header, c *http.Client) (_ *ProtectedResourceMetadata, err error) {
6061
defer util.Wrapf(&err, "GetProtectedResourceMetadataFromHeader")
6162
headers := header[http.CanonicalHeaderKey("WWW-Authenticate")]
6263
if len(headers) == 0 {
@@ -66,11 +67,11 @@ func GetProtectedResourceMetadataFromHeader(ctx context.Context, header http.Hea
6667
if err != nil {
6768
return nil, err
6869
}
69-
url := ResourceMetadataURL(cs)
70-
if url == "" {
70+
metadataURL := ResourceMetadataURL(cs)
71+
if metadataURL == "" {
7172
return nil, nil
7273
}
73-
return getPRM(ctx, url, c, url)
74+
return getPRM(ctx, metadataURL, c, serverURL)
7475
}
7576

7677
// 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
8384
if err != nil {
8485
return nil, err
8586
}
86-
// Validate the Resource field to thwart impersonation attacks (section 3.3).
87+
// Validate the Resource field (see RFC 9728, section 3.3) as well as issue #560
8788
if prm.Resource != wantResource {
8889
return nil, fmt.Errorf("got metadata resource %q, want %q", prm.Resource, wantResource)
8990
}

0 commit comments

Comments
 (0)