From 48b02dc4c268dba8e5aad517d8a318b5d287ea8d Mon Sep 17 00:00:00 2001 From: Pratik Dhanave Date: Sun, 2 Aug 2026 12:24:51 +0530 Subject: [PATCH] auth: advertise RFC 6750 error code in WWW-Authenticate challenge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RequireBearerToken never added an error= auth-param to the WWW-Authenticate header, so on insufficient scope it returned 403 with resource_metadata/scope but no error="insufficient_scope". The SDK's own client gates step-up re-authorization on exactly that value (AuthorizationCodeHandler.Authorize), so errorFromChallenges returned "" and step-up never fired — a client hitting an SDK server that needs more scopes just kept failing with 403 instead of upgrading scopes. Thread the RFC 6750 error code out of verify and add the error= param: insufficient_scope on the 403 scope check, invalid_token on the token-invalid 401 paths, invalid_request on the 400. A missing token (no credentials presented) and 5xx failures advertise no code. The change is additive to the header, non-breaking. Fixes #1134 --- auth/auth.go | 44 ++++++++++++++++-------- auth/auth_test.go | 87 +++++++++++++++++++++++++++++++++++------------ 2 files changed, 95 insertions(+), 36 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index dc02d7d6..2ee1d9af 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -99,20 +99,28 @@ func RequireBearerToken(verifier TokenVerifier, opts *RequireBearerTokenOptions) return func(handler http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - tokenInfo, errmsg, code := verify(r, verifier, opts) + tokenInfo, errmsg, code, authError := verify(r, verifier, opts) if code != 0 { if code == http.StatusUnauthorized || code == http.StatusForbidden { + var params []string + // Emit the RFC 6750 error code so clients can react to it. In + // particular, the SDK's own step-up flow only re-authorizes + // when it sees error="insufficient_scope" on a 403 + // (see AuthorizationCodeHandler.Authorize); without it, a + // scope shortfall is never upgraded. + if authError != "" { + params = append(params, fmt.Sprintf("error=%q", authError)) + } if opts != nil { - var params []string if opts.ResourceMetadataURL != "" { params = append(params, fmt.Sprintf("resource_metadata=%q", opts.ResourceMetadataURL)) } if len(opts.Scopes) > 0 { params = append(params, fmt.Sprintf("scope=%q", strings.Join(opts.Scopes, " "))) } - if len(params) > 0 { - w.Header().Add("WWW-Authenticate", "Bearer "+strings.Join(params, ", ")) - } + } + if len(params) > 0 { + w.Header().Add("WWW-Authenticate", "Bearer "+strings.Join(params, ", ")) } } http.Error(w, errmsg, code) @@ -124,27 +132,33 @@ func RequireBearerToken(verifier TokenVerifier, opts *RequireBearerTokenOptions) } } -func verify(req *http.Request, verifier TokenVerifier, opts *RequireBearerTokenOptions) (_ *TokenInfo, errmsg string, code int) { +// verify authenticates and authorizes req. On failure it returns a non-zero +// HTTP status code and, where applicable, the RFC 6750 §3.1 error code +// (authError) to advertise in the WWW-Authenticate challenge. authError is +// empty when no error code applies — for a missing token (no credentials +// presented) and for server-side 5xx failures. +func verify(req *http.Request, verifier TokenVerifier, opts *RequireBearerTokenOptions) (_ *TokenInfo, errmsg string, code int, authError string) { // Extract bearer token. authHeader := req.Header.Get("Authorization") fields := strings.Fields(authHeader) if len(fields) != 2 || strings.ToLower(fields[0]) != "bearer" { - return nil, "no bearer token", http.StatusUnauthorized + // No credentials presented: RFC 6750 §3.1 advertises no error code. + return nil, "no bearer token", http.StatusUnauthorized, "" } // Verify the token and get information from it. tokenInfo, err := verifier(req.Context(), fields[1], req) if err != nil { if errors.Is(err, ErrInvalidToken) { - return nil, err.Error(), http.StatusUnauthorized + return nil, err.Error(), http.StatusUnauthorized, "invalid_token" } if errors.Is(err, ErrOAuth) { - return nil, err.Error(), http.StatusBadRequest + return nil, err.Error(), http.StatusBadRequest, "invalid_request" } - return nil, err.Error(), http.StatusInternalServerError + return nil, err.Error(), http.StatusInternalServerError, "" } if tokenInfo == nil { - return nil, "token validation failed", http.StatusInternalServerError + return nil, "token validation failed", http.StatusInternalServerError, "" } // Check scopes. All must be present. @@ -152,7 +166,7 @@ func verify(req *http.Request, verifier TokenVerifier, opts *RequireBearerTokenO // Note: quadratic, but N is small. for _, s := range opts.Scopes { if !slices.Contains(tokenInfo.Scopes, s) { - return nil, "insufficient scope", http.StatusForbidden + return nil, "insufficient scope", http.StatusForbidden, "insufficient_scope" } } } @@ -165,12 +179,12 @@ func verify(req *http.Request, verifier TokenVerifier, opts *RequireBearerTokenO // AllowMissingExpiration. if tokenInfo.Expiration.IsZero() { if !opts.AllowMissingExpiration { - return nil, "token missing expiration", http.StatusUnauthorized + return nil, "token missing expiration", http.StatusUnauthorized, "invalid_token" } } else if tokenInfo.Expiration.Add(opts.ClockSkew).Before(time.Now()) { - return nil, "token expired", http.StatusUnauthorized + return nil, "token expired", http.StatusUnauthorized, "invalid_token" } - return tokenInfo, "", 0 + return tokenInfo, "", 0, "" } // ProtectedResourceMetadataHandler returns an http.Handler that serves OAuth 2.0 diff --git a/auth/auth_test.go b/auth/auth_test.go index 654ca26b..31239a26 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -36,57 +36,102 @@ func TestVerify(t *testing.T) { } for _, tt := range []struct { - name string - opts *RequireBearerTokenOptions - header string - wantMsg string - wantCode int + name string + opts *RequireBearerTokenOptions + header string + wantMsg string + wantCode int + wantAuthError string // RFC 6750 error code to advertise, "" if none }{ { "valid", nil, "Bearer valid", - "", 0, + "", 0, "", }, { "bad header", nil, "Barer valid", - "no bearer token", 401, + "no bearer token", 401, "", }, { "invalid", nil, "bearer invalid", - "invalid token", 401, + "invalid token", 401, "invalid_token", }, { "oauth error", nil, "Bearer oauth", - "oauth error", 400, + "oauth error", 400, "invalid_request", }, { "no expiration", nil, "Bearer noexp", - "token missing expiration", 401, + "token missing expiration", 401, "invalid_token", }, { "no expiration with AllowMissingExpiration accepts", &RequireBearerTokenOptions{AllowMissingExpiration: true}, "Bearer noexp", - "", 0, + "", 0, "", }, { "expired", nil, "Bearer expired", - "token expired", 401, + "token expired", 401, "invalid_token", }, { "missing scope", &RequireBearerTokenOptions{Scopes: []string{"s1"}}, "Bearer valid", - "insufficient scope", 403, + "insufficient scope", 403, "insufficient_scope", }, } { t.Run(tt.name, func(t *testing.T) { - _, gotMsg, gotCode := verify(&http.Request{ + _, gotMsg, gotCode, gotAuthError := verify(&http.Request{ Header: http.Header{"Authorization": {tt.header}}, }, verifier, tt.opts) - if gotMsg != tt.wantMsg || gotCode != tt.wantCode { - t.Errorf("got (%q, %d), want (%q, %d)", gotMsg, gotCode, tt.wantMsg, tt.wantCode) + if gotMsg != tt.wantMsg || gotCode != tt.wantCode || gotAuthError != tt.wantAuthError { + t.Errorf("got (%q, %d, %q), want (%q, %d, %q)", + gotMsg, gotCode, gotAuthError, tt.wantMsg, tt.wantCode, tt.wantAuthError) } }) } } +func TestRequireBearerTokenAdvertisesInsufficientScope(t *testing.T) { + // A valid token lacking a required scope must yield a 403 whose + // WWW-Authenticate challenge carries error="insufficient_scope". The SDK's + // own client step-up flow (AuthorizationCodeHandler.Authorize) re-authorizes + // only when it sees exactly that value, so without it step-up never fires. + verifier := func(_ context.Context, _ string, _ *http.Request) (*TokenInfo, error) { + return &TokenInfo{Expiration: time.Now().Add(time.Hour), Scopes: []string{"read"}}, nil + } + mw := RequireBearerToken(verifier, &RequireBearerTokenOptions{ + Scopes: []string{"admin"}, + ResourceMetadataURL: "https://example.com/.well-known/oauth-protected-resource", + }) + handler := mw(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Error("handler should not be reached on insufficient scope") + })) + + req := httptest.NewRequest(http.MethodGet, "https://example.com/mcp", nil) + req.Header.Set("Authorization", "Bearer tok") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusForbidden) + } + + // Parse the challenge exactly as the client does (oauthex.ParseWWWAuthenticate + // + a "bearer" scheme match) and confirm the error code is visible. + challenges, err := oauthex.ParseWWWAuthenticate(rec.Result().Header[http.CanonicalHeaderKey("WWW-Authenticate")]) + if err != nil { + t.Fatalf("ParseWWWAuthenticate: %v", err) + } + var gotError string + for _, c := range challenges { + if c.Scheme == "bearer" && c.Params["error"] != "" { + gotError = c.Params["error"] + } + } + if gotError != "insufficient_scope" { + t.Errorf("WWW-Authenticate error param = %q, want %q (header: %q)", + gotError, "insufficient_scope", rec.Result().Header.Get("WWW-Authenticate")) + } +} + func TestProtectedResourceMetadataHandler(t *testing.T) { metadata := &oauthex.ProtectedResourceMetadata{ Resource: "https://example.com/mcp", @@ -213,7 +258,7 @@ func TestRequireBearerToken(t *testing.T) { name: "no middleware options", opts: nil, authHeader: "Bearer invalid", - wantHeader: "", + wantHeader: "Bearer error=\"invalid_token\"", wantStatus: http.StatusUnauthorized, }, { @@ -222,7 +267,7 @@ func TestRequireBearerToken(t *testing.T) { ResourceMetadataURL: "https://example.com/resource-metadata", }, authHeader: "Bearer invalid", - wantHeader: "Bearer resource_metadata=\"https://example.com/resource-metadata\"", + wantHeader: "Bearer error=\"invalid_token\", resource_metadata=\"https://example.com/resource-metadata\"", wantStatus: http.StatusUnauthorized, }, { @@ -231,7 +276,7 @@ func TestRequireBearerToken(t *testing.T) { Scopes: []string{"read", "write"}, }, authHeader: "Bearer invalid", - wantHeader: "Bearer scope=\"read write\"", + wantHeader: "Bearer error=\"invalid_token\", scope=\"read write\"", wantStatus: http.StatusUnauthorized, }, { @@ -241,7 +286,7 @@ func TestRequireBearerToken(t *testing.T) { Scopes: []string{"read", "write"}, }, authHeader: "Bearer invalid", - wantHeader: "Bearer resource_metadata=\"https://example.com/resource-metadata\", scope=\"read write\"", + wantHeader: "Bearer error=\"invalid_token\", resource_metadata=\"https://example.com/resource-metadata\", scope=\"read write\"", wantStatus: http.StatusUnauthorized, }, { @@ -250,7 +295,7 @@ func TestRequireBearerToken(t *testing.T) { Scopes: []string{"admin"}, }, authHeader: "Bearer valid", // Has "read", needs "admin" -> 403 - wantHeader: "Bearer scope=\"admin\"", + wantHeader: "Bearer error=\"insufficient_scope\", scope=\"admin\"", wantStatus: http.StatusForbidden, }, {