(knox) 27/37: KNOX-3414: /userinfo returns 401 invalid_token, not 500, for a bad bearer token (review finding M6)

[sm...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

smolnar82 pushed a commit to branch knox_idf
in repository https://gitbox.apache.org/repos/asf/knox.git

commit 478c561810f00a02e08dc40ad878a2e1c3204f37
Author: Sandor Molnar <sm...@apache.org>
AuthorDate: Tue Aug 11 23:35:34 2026 +0200

    KNOX-3414: /userinfo returns 401 invalid_token, not 500, for a bad bearer token (review finding M6)
    
    getTokenMetadata throws UnknownTokenException for an expired/revoked/unknown
    token, and doGet rethrew it as an unmapped RuntimeException -> HTTP 500. RFC 6750
    requires a protected resource to answer such a token with 401 and a
    WWW-Authenticate: Bearer error="invalid_token" challenge.
    
    Catch UnknownTokenException in getUserInfo and return the RFC 6750 challenge via
    the new invalidToken() helper. Also handle a token that references a
    now-missing federated identity the same way (the token can no longer be
    honored) instead of throwing. doGet no longer wraps-and-rethrows.
    
    Covered by UserInfoResourceInvalidTokenTest.
    
    Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
---
 .../gateway/service/knoxidf/UserInfoResource.java  | 40 ++++++---
 .../knoxidf/UserInfoResourceInvalidTokenTest.java  | 94 ++++++++++++++++++++++
 2 files changed, 123 insertions(+), 11 deletions(-)

diff --git a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/UserInfoResource.java b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/UserInfoResource.java
index b4fccedd0..f500d6d57 100644
--- a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/UserInfoResource.java
+++ b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/UserInfoResource.java
@@ -25,7 +25,6 @@ import org.apache.knox.gateway.services.ServiceType;
 import org.apache.knox.gateway.services.knoxidf.federation.FederatedIdentity;
 import org.apache.knox.gateway.services.knoxidf.federation.FederatedIdentityService;
 import org.apache.knox.gateway.services.security.token.TokenMetadata;
-import org.apache.knox.gateway.services.security.token.TokenServiceException;
 import org.apache.knox.gateway.services.security.token.TokenStateService;
 import org.apache.knox.gateway.services.security.token.UnknownTokenException;
 import org.apache.knox.gateway.util.JsonUtils;
@@ -60,7 +59,7 @@ public class UserInfoResource {
     private ServletContext servletContext;
 
     @Context
-    private HttpServletRequest request;
+    HttpServletRequest request;
 
     private FederatedIdentityService federatedIdentityService;
 
@@ -72,11 +71,7 @@ public class UserInfoResource {
     }
 
     public Response doGet() {
-        try {
-            return getUserInfo();
-        } catch (UnknownTokenException | TokenServiceException e) {
-            throw new RuntimeException(e);
-        }
+        return getUserInfo();
     }
 
     public Response doPost() {
@@ -85,14 +80,22 @@ public class UserInfoResource {
 
     @GET
     @Produces(MediaType.APPLICATION_JSON)
-    public Response getUserInfo() throws UnknownTokenException, TokenServiceException {
+    public Response getUserInfo() {
         final String tokenId = request.getAttribute(TOKEN_ID_ATTRIBUTE) == null ? null : request.getAttribute(TOKEN_ID_ATTRIBUTE).toString();
         if (tokenId == null) {
             return error("invalid_request", "Cannot find tokenId");
         }
 
         final String scope = request.getAttribute(SCOPE_ATTRIBUTE) == null ? "" : request.getAttribute(SCOPE_ATTRIBUTE).toString();
-        final TokenMetadata tokenMetadata = getReadonlyTokenStateService().getTokenMetadata(tokenId);
+        final TokenMetadata tokenMetadata;
+        try {
+            tokenMetadata = getReadonlyTokenStateService().getTokenMetadata(tokenId);
+        } catch (UnknownTokenException e) {
+            // Expired, revoked, or otherwise unknown bearer token. Per RFC 6750 the protected
+            // resource must answer 401 with a WWW-Authenticate: Bearer error="invalid_token"
+            // challenge rather than leaking a 500 for what is a client authentication failure.
+            return invalidToken("The access token is expired, revoked, or unknown");
+        }
         final Map<String, Object> userInfo = new HashMap<>();
 
         // Check if this token has a federated identity
@@ -102,7 +105,12 @@ public class UserInfoResource {
             // Federated user
             final FederatedIdentity federatedIdentity = federatedIdentityService
                     .findById(federatedIdentityId)
-                    .orElseThrow(() -> new TokenServiceException("Federated identity not found"));
+                    .orElse(null);
+            if (federatedIdentity == null) {
+                // The token references a federated identity that no longer exists; the bearer token
+                // can no longer be honored, so answer with the RFC 6750 invalid_token challenge.
+                return invalidToken("The access token references an unknown federated identity");
+            }
 
             // Include only allowed claims
             Map<String, Object> claims = federatedIdentity.getAttributes().entrySet().stream()
@@ -129,10 +137,20 @@ public class UserInfoResource {
         return Response.ok(JsonUtils.renderAsJsonString(userInfo, true)).build();
     }
 
-    private TokenStateService getReadonlyTokenStateService() {
+    TokenStateService getReadonlyTokenStateService() {
         GatewayServices services = (GatewayServices) servletContext.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
         return services.getService(ServiceType.TOKEN_STATE_SERVICE);
     }
 
+    /**
+     * Builds the RFC 6750 §3 response for a bad bearer token: HTTP 401 with a
+     * {@code WWW-Authenticate: Bearer error="invalid_token"} challenge and a matching JSON body.
+     */
+    static Response invalidToken(final String description) {
+        final Response base = error("invalid_token", description, Response.Status.UNAUTHORIZED);
+        final String challenge = "Bearer error=\"invalid_token\", error_description=\"" + description + "\"";
+        return Response.fromResponse(base).header("WWW-Authenticate", challenge).build();
+    }
+
 }
 
diff --git a/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/UserInfoResourceInvalidTokenTest.java b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/UserInfoResourceInvalidTokenTest.java
new file mode 100644
index 000000000..a2c20bc10
--- /dev/null
+++ b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/UserInfoResourceInvalidTokenTest.java
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.knox.gateway.service.knoxidf;
+
+import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.SCOPE_ATTRIBUTE;
+import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.TOKEN_ID_ATTRIBUTE;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.core.Response;
+
+import org.apache.knox.gateway.services.security.token.TokenStateService;
+import org.apache.knox.gateway.services.security.token.UnknownTokenException;
+import org.easymock.EasyMock;
+import org.junit.Test;
+
+/**
+ * Verifies /userinfo answers a bad bearer token per RFC 6750 (review finding M6): an expired,
+ * revoked, or unknown token must yield HTTP 401 with a
+ * {@code WWW-Authenticate: Bearer error="invalid_token"} challenge, not a 500 from an unmapped
+ * RuntimeException.
+ */
+public class UserInfoResourceInvalidTokenTest {
+
+  private static final String TOKEN_ID = "11111111-2222-3333-[VISA_CARD_NUMBER_REDACTED]";
+
+  /** Injects the request and a token-state service that throws for an unknown token. */
+  static final class TestableUserInfoResource extends UserInfoResource {
+    private final TokenStateService tss;
+
+    TestableUserInfoResource(final HttpServletRequest req, final TokenStateService tss) {
+      this.request = req;
+      this.tss = tss;
+    }
+
+    @Override
+    TokenStateService getReadonlyTokenStateService() {
+      return tss;
+    }
+  }
+
+  private static HttpServletRequest requestWithToken(final String tokenId) {
+    final HttpServletRequest req = EasyMock.createNiceMock(HttpServletRequest.class);
+    EasyMock.expect(req.getAttribute(TOKEN_ID_ATTRIBUTE)).andReturn(tokenId).anyTimes();
+    EasyMock.expect(req.getAttribute(SCOPE_ATTRIBUTE)).andReturn(null).anyTimes();
+    EasyMock.replay(req);
+    return req;
+  }
+
+  @Test
+  public void testUnknownTokenYields401WithBearerChallenge() throws Exception {
+    final TokenStateService tss = EasyMock.createNiceMock(TokenStateService.class);
+    EasyMock.expect(tss.getTokenMetadata(TOKEN_ID)).andThrow(new UnknownTokenException(TOKEN_ID)).anyTimes();
+    EasyMock.replay(tss);
+
+    final Response response = new TestableUserInfoResource(requestWithToken(TOKEN_ID), tss).getUserInfo();
+
+    assertEquals("An unknown/expired token must be 401, not 500.",
+        Response.Status.UNAUTHORIZED.getStatusCode(), response.getStatus());
+    final Object challenge = response.getHeaderString("WWW-Authenticate");
+    assertNotNull("RFC 6750 requires a WWW-Authenticate challenge.", challenge);
+    assertTrue("The challenge must be a Bearer invalid_token challenge.",
+        challenge.toString().contains("Bearer") && challenge.toString().contains("invalid_token"));
+    assertTrue("The JSON body should carry the invalid_token error code.",
+        String.valueOf(response.getEntity()).contains("invalid_token"));
+  }
+
+  @Test
+  public void testMissingTokenIdYieldsInvalidRequest() {
+    final TokenStateService tss = EasyMock.createNiceMock(TokenStateService.class);
+    EasyMock.replay(tss);
+
+    final Response response = new TestableUserInfoResource(requestWithToken(null), tss).getUserInfo();
+
+    assertEquals("A missing token id is a client request error (400).",
+        Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus());
+  }
+}