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 <[email protected]> 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 <[email protected]> --- .../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()); + } +}
