Skip to content

auth: advertise RFC 6750 error code in WWW-Authenticate challenge #1135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PratikDhanave wants to merge 1 commit into
base: main
Choose a base branch
from
Open

auth: advertise RFC 6750 error code in WWW-Authenticate challenge #1135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 29 additions & 15 deletions auth/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -124,35 +132,41 @@ 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.
if opts != nil {
// 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"
}
}
}
Expand All @@ -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
Expand Down
87 changes: 66 additions & 21 deletions auth/auth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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,
},
{
Expand All @@ -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,
},
{
Expand All @@ -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,
},
{
Expand All @@ -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,
},
{
Expand All @@ -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,
},
{
Expand Down