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, }, {