From b91f387a19b37971ed085c9d46ca45f0829a2e11 Mon Sep 17 00:00:00 2001 From: xianshujun Date: Thu, 5 Nov 2026 17:47:21 +0900 Subject: [PATCH] auth: include error="invalid_token" in 401 challenges --- auth/auth.go | 41 ++++++++++++++++++-------------- auth/auth_test.go | 60 ++++++++++++++++++++++++++++++----------------- 2 files changed, 61 insertions(+), 40 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index ccf071fbb..8733b5f9f 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -99,23 +99,23 @@ 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, challengeErr := verify(r, verifier, opts) if code != 0 { if code == http.StatusUnauthorized || code == http.StatusForbidden { + var params []string + if challengeErr != "" { + params = append(params, fmt.Sprintf("error=%q", challengeErr)) + } if opts != nil { - var params []string - if code == http.StatusForbidden { - params = append(params, `error="insufficient_scope"`) - } 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) @@ -127,27 +127,32 @@ func RequireBearerToken(verifier TokenVerifier, opts *RequireBearerTokenOptions) } } -func verify(req *http.Request, verifier TokenVerifier, opts *RequireBearerTokenOptions) (_ *TokenInfo, errmsg string, code int) { +// verify checks the request's bearer token. On failure it returns the HTTP +// status and, for 401 and 403 responses, the OAuth error code to include in +// the WWW-Authenticate challenge (RFC 6750 §3.1). +func verify(req *http.Request, verifier TokenVerifier, opts *RequireBearerTokenOptions) (_ *TokenInfo, errmsg string, code int, challengeErr 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 + // RFC 6750 §3.1: a request without authentication information should not + // be answered with an 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, "" } - 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, "" } if opts == nil { @@ -161,20 +166,20 @@ func verify(req *http.Request, verifier TokenVerifier, opts *RequireBearerTokenO // as lacking scope (403), prompting it to ask for more scopes. 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" } // Check scopes. All must be present. // 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" } } - 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 76760142a..af9eeda81 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -36,60 +36,62 @@ 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 + wantChallenge string }{ { "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, "", }, { "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", }, { "expired and missing scope", &RequireBearerTokenOptions{Scopes: []string{"s1"}}, "Bearer expired", - "token expired", 401, + "token expired", 401, "invalid_token", }, { "no expiration and missing scope", &RequireBearerTokenOptions{Scopes: []string{"s1"}}, "Bearer noexp", - "token missing expiration", 401, + "token missing expiration", 401, "invalid_token", }, } { t.Run(tt.name, func(t *testing.T) { - _, gotMsg, gotCode := verify(&http.Request{ + _, gotMsg, gotCode, gotChallenge := 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 || gotChallenge != tt.wantChallenge { + t.Errorf("got (%q, %d, %q), want (%q, %d, %q)", + gotMsg, gotCode, gotChallenge, tt.wantMsg, tt.wantCode, tt.wantChallenge) } }) } @@ -218,9 +220,23 @@ func TestRequireBearerToken(t *testing.T) { wantStatus int }{ { - name: "no middleware options", + name: "invalid token without middleware options", opts: nil, authHeader: "Bearer invalid", + wantHeader: "Bearer error=\"invalid_token\"", + wantStatus: http.StatusUnauthorized, + }, + { + name: "no bearer token", + opts: nil, + authHeader: "", + wantHeader: "", + wantStatus: http.StatusUnauthorized, + }, + { + name: "malformed authorization header", + opts: nil, + authHeader: "Basic invalid", wantHeader: "", wantStatus: http.StatusUnauthorized, }, @@ -230,7 +246,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, }, { @@ -239,7 +255,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, }, { @@ -249,7 +265,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, }, {