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 8f6860b034a2d4bf5bf1a82c4f215b39239fd044 Author: Sandor Molnar <[email protected]> AuthorDate: Tue Aug 11 23:22:23 2026 +0200 KNOX-3414: reject federated id_token missing sub with 4xx, not 500 (review finding M2) The federated callback derives the Knox subject and the federated-identity primary key from the OP id_token's sub claim, and the identity tables declare external_subject NOT NULL. A broken or hostile OP that returns a verified id_token with no sub therefore drove a NOT NULL INSERT failure -> HTTP 500 on every callback through that OP (a targeted DoS). sub is REQUIRED by OIDC Core 2. Enforce its presence on the (already signature/issuer/audience-verified) id_token via requireFederatedSubject and return invalid_request when it is absent/blank, before any persistence happens. The iss claim is already covered: the configured expectedIssuer is non-blank, so a blank issuer fails the existing issuer-mismatch check. Covered by AuthorizeResourceFederatedSubjectTest. Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../gateway/service/knoxidf/AuthorizeResource.java | 19 +++++- .../AuthorizeResourceFederatedSubjectTest.java | 68 ++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java index c2d063096..0ba9ac5bd 100644 --- a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java +++ b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java @@ -593,7 +593,7 @@ public class AuthorizeResource extends PasscodeTokenResourceBase { return error("invalid_request", "Federated id_token audience mismatch"); } - return null; + return requireFederatedSubject(idToken); } /** @@ -615,6 +615,23 @@ public class AuthorizeResource extends PasscodeTokenResourceBase { return null; } + /** + * Enforces that a verified federated id_token carries the {@code sub} claim, which OIDC Core 2 + * marks REQUIRED. Knox derives both the Knox subject and the federated-identity primary key from + * it, and the identity tables declare {@code external_subject NOT NULL}. A broken or hostile OP + * that omits {@code sub} would otherwise drive a NOT NULL insert failure -> HTTP 500 on every + * callback through that OP; reject it as a client/OP error instead. Call only after + * {@link #validateFederatedIdToken} has established the token's authenticity. + * + * @return an error {@link Response} when {@code sub} is absent/blank, or {@code null} otherwise. + */ + Response requireFederatedSubject(final JWT idToken) { + if (StringUtils.isBlank(idToken.getSubject())) { + return error("invalid_request", "Federated id_token is missing the required sub claim"); + } + return null; + } + private FederatedIdentity resolveFederatedIdentity(final JWT jwt, String opName) { final String issuer = jwt.getIssuer(); final String subject = jwt.getSubject(); diff --git a/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResourceFederatedSubjectTest.java b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResourceFederatedSubjectTest.java new file mode 100644 index 000000000..890abdc01 --- /dev/null +++ b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResourceFederatedSubjectTest.java @@ -0,0 +1,68 @@ +/* + * 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.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import javax.ws.rs.core.Response; + +import org.apache.knox.gateway.services.security.token.impl.JWT; +import org.easymock.EasyMock; +import org.junit.Test; + +/** + * Verifies that a federated id_token missing the required {@code sub} claim is rejected as a 4xx + * client/OP error rather than surfacing as an HTTP 500 (review finding M2). Knox derives the Knox + * subject and the federated-identity primary key from {@code sub}, and the identity tables declare + * {@code external_subject NOT NULL}; without this guard a broken or hostile OP omitting {@code sub} + * drives a NOT NULL insert failure and a 500 on every callback through that OP. + */ +public class AuthorizeResourceFederatedSubjectTest { + + private static JWT idTokenWithSubject(final String subject) { + final JWT idToken = EasyMock.createNiceMock(JWT.class); + EasyMock.expect(idToken.getSubject()).andReturn(subject).anyTimes(); + EasyMock.replay(idToken); + return idToken; + } + + @Test + public void testPresentSubjectPasses() { + final Response result = new AuthorizeResource().requireFederatedSubject(idTokenWithSubject("user-123")); + assertNull("An id_token carrying a sub claim must pass (null == no error).", result); + } + + @Test + public void testMissingSubjectIsRejectedWith4xx() { + final Response result = new AuthorizeResource().requireFederatedSubject(idTokenWithSubject(null)); + assertNotNull("An id_token without a sub claim must be rejected.", result); + assertEquals("A missing sub must be a client/OP error, not a 500.", + Response.Status.BAD_REQUEST.getStatusCode(), result.getStatus()); + assertTrue("The error body should identify the invalid_request condition.", + String.valueOf(result.getEntity()).contains("invalid_request")); + } + + @Test + public void testBlankSubjectIsRejectedWith4xx() { + final Response result = new AuthorizeResource().requireFederatedSubject(idTokenWithSubject(" ")); + assertNotNull("An id_token with a blank sub claim must be rejected.", result); + assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), result.getStatus()); + } +}
