Skip to content

Commit 599a2ee

Browse files
authored
fix(oauth): parse resource_metadata per RFC 7235 auth-param rules (#1771)
## Bug `ExtractResourceMetadataURL` (`internal/oauth/discovery.go`) finds the PRM URL by splitting the header on the literal `resource_metadata="`. RFC 7235 section 2.1 allows a case-insensitive parameter name and whitespace around `=`, so a valid challenge can be missed and discovery falls back to a constructed URL. A plain substring split also misreads other parameters: - `x_resource_metadata="…"` is taken as `resource_metadata`. - An escaped mention inside another parameter's quoted value, such as `error_description`, is taken as the real one. ## Trigger All of these headers return `""` or the wrong URL today: - `Bearer Resource_Metadata="https://…/oauth-protected-resource"` - `Bearer error="invalid_token", resource_metadata = "https://…"` - `Bearer x_resource_metadata="https://wrong", resource_metadata="https://…"` ## Fix The function now runs a small auth-param scanner: - parameter names are case-insensitive tokens; - whitespace is allowed around `=`; - quoted strings are unescaped; - other parameters' quoted values are skipped, not searched. Token-form (unquoted) values are still rejected, matching the existing "missing opening quote" case. ## Verification - `go test -count=1 -run TestExtractResourceMetadataURL ./internal/oauth/` passes. That covers the new cases and the existing `TestExtractResourceMetadataURL` table. - With `discovery.go` reverted (`git stash push internal/oauth/discovery.go`), the new test fails on 4 of its 5 subtests: case-insensitive name, whitespace around `=`, similar-suffix parameter, and lookalike-only. - `gofmt -l` is clean on both files. - I haven't run the full `go test -count=1 ./internal/oauth/` locally. Work stopped because free disk fell below 1 GB.
1 parent 4c7d2b2 commit 599a2ee

2 files changed

Lines changed: 86 additions & 14 deletions

File tree

‎internal/oauth/discovery.go‎

Lines changed: 61 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -278,25 +278,72 @@ func discoverAuthServerURL(env Env, serverURL string, timeout time.Duration, fet
278278

279279
// ExtractResourceMetadataURL parses WWW-Authenticate header to extract resource_metadata URL
280280
// Format: Bearer error="invalid_request", resource_metadata="https://..."
281+
//
282+
// Parameters are scanned per RFC 7235 section 2.1: names are case-insensitive,
283+
// whitespace may surround "=", and quoted values of other parameters are
284+
// skipped rather than searched, so only a real resource_metadata parameter
285+
// with a quoted-string value is returned.
281286
func ExtractResourceMetadataURL(wwwAuthHeader string) string {
282-
// Look for resource_metadata parameter
283-
if !strings.Contains(wwwAuthHeader, "resource_metadata") {
284-
return ""
287+
h := wwwAuthHeader
288+
n := len(h)
289+
skipWS := func(i int) int {
290+
for i < n && (h[i] == ' ' || h[i] == '\t') {
291+
i++
292+
}
293+
return i
285294
}
286-
287-
// Split on resource_metadata=" to find the URL
288-
parts := strings.Split(wwwAuthHeader, "resource_metadata=\"")
289-
if len(parts) < 2 {
290-
return ""
295+
i := 0
296+
for i < n {
297+
for i < n && (h[i] == ' ' || h[i] == '\t' || h[i] == ',') {
298+
i++
299+
}
300+
start := i
301+
for i < n && isAuthTokenChar(h[i]) {
302+
i++
303+
}
304+
name := h[start:i]
305+
if name == "" {
306+
i++
307+
continue
308+
}
309+
j := skipWS(i)
310+
if j >= n || h[j] != '=' {
311+
continue
312+
}
313+
j = skipWS(j + 1)
314+
if j < n && h[j] == '"' {
315+
var b strings.Builder
316+
k := j + 1
317+
for ; k < n && h[k] != '"'; k++ {
318+
if h[k] == '\\' && k+1 < n {
319+
k++
320+
}
321+
b.WriteByte(h[k])
322+
}
323+
if k >= n {
324+
return ""
325+
}
326+
if strings.EqualFold(name, "resource_metadata") {
327+
return b.String()
328+
}
329+
i = k + 1
330+
continue
331+
}
332+
for j < n && h[j] != ',' && h[j] != ' ' && h[j] != '\t' {
333+
j++
334+
}
335+
i = j
291336
}
337+
return ""
338+
}
292339

293-
// Find the closing quote
294-
endIdx := strings.Index(parts[1], "\"")
295-
if endIdx == -1 {
296-
return ""
340+
// isAuthTokenChar reports whether c is an RFC 7230 tchar.
341+
func isAuthTokenChar(c byte) bool {
342+
switch {
343+
case c >= 'a' && c <= 'z', c >= 'A' && c <= 'Z', c >= '0' && c <= '9':
344+
return true
297345
}
298-
299-
return parts[1][:endIdx]
346+
return strings.IndexByte("!#$%&'*+-.^_`|~", c) >= 0
300347
}
301348

302349
// DiscoverProtectedResourceMetadata fetches RFC 9728 Protected Resource Metadata
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package oauth
2+
3+
import "testing"
4+
5+
func TestExtractResourceMetadataURL_RFC7235Params(t *testing.T) {
6+
const want = "https://api.example.com/.well-known/oauth-protected-resource"
7+
tests := []struct {
8+
name string
9+
header string
10+
want string
11+
}{
12+
{"case-insensitive name", `Bearer Resource_Metadata="` + want + `"`, want},
13+
{"whitespace around equals", `Bearer error="invalid_token", resource_metadata = "` + want + `"`, want},
14+
{"other param with similar suffix", `Bearer x_resource_metadata="https://wrong.example", resource_metadata="` + want + `"`, want},
15+
{"mention inside another quoted value", `Bearer error_description="see resource_metadata=\"https://wrong.example\"", resource_metadata="` + want + `"`, want},
16+
{"only a lookalike param", `Bearer x_resource_metadata="https://wrong.example"`, ""},
17+
}
18+
for _, tt := range tests {
19+
t.Run(tt.name, func(t *testing.T) {
20+
if got := ExtractResourceMetadataURL(tt.header); got != tt.want {
21+
t.Errorf("ExtractResourceMetadataURL(%q) = %q, want %q", tt.header, got, tt.want)
22+
}
23+
})
24+
}
25+
}

0 commit comments

Comments
 (0)