auth: include error="invalid_token" in 401 challenges

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

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