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
The following commit(s) were added to refs/heads/knox_idf by this push:
new 7b9730b18 KNOX-3396: Return 4xx for type-mismatched register bodies
and issuer-limit-reached instead of 500 (#1328)
7b9730b18 is described below
commit 7b9730b18bb896a806955da32d607d4360438393
Author: Sandor Molnar <[email protected]>
AuthorDate: Fri Jul 24 20:32:00 2026 +0200
KNOX-3396: Return 4xx for type-mismatched register bodies and
issuer-limit-reached instead of 500 (#1328)
---
gateway-service-knoxidf/pom.xml | 2 +-
.../service/knoxidf/RegisterIssuerRequest.java | 54 ++++++++++++++++++++++
.../knoxidf/TrustedOidcIssuersResource.java | 24 ++++++----
.../knoxidf/TrustedOidcIssuersResourceTest.java | 40 +++++++++++++++-
4 files changed, 107 insertions(+), 13 deletions(-)
diff --git a/gateway-service-knoxidf/pom.xml b/gateway-service-knoxidf/pom.xml
index 48296b435..3c5fbce58 100644
--- a/gateway-service-knoxidf/pom.xml
+++ b/gateway-service-knoxidf/pom.xml
@@ -73,7 +73,7 @@
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
- <artifactId>jackson-core</artifactId>
+ <artifactId>jackson-annotations</artifactId>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
diff --git
a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/RegisterIssuerRequest.java
b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/RegisterIssuerRequest.java
new file mode 100644
index 000000000..eeb83c74f
--- /dev/null
+++
b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/RegisterIssuerRequest.java
@@ -0,0 +1,54 @@
+/*
+ * 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 com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+
+/**
+ * Request body for registering a trusted OIDC issuer via {@link
TrustedOidcIssuersResource#registerIssuer(String)}.
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class RegisterIssuerRequest {
+
+ private String issuerUrl;
+ private boolean dynamicJwks;
+ private String clusterName;
+
+ public String getIssuerUrl() {
+ return issuerUrl;
+ }
+
+ public void setIssuerUrl(String issuerUrl) {
+ this.issuerUrl = issuerUrl;
+ }
+
+ public boolean isDynamicJwks() {
+ return dynamicJwks;
+ }
+
+ public void setDynamicJwks(boolean dynamicJwks) {
+ this.dynamicJwks = dynamicJwks;
+ }
+
+ public String getClusterName() {
+ return clusterName;
+ }
+
+ public void setClusterName(String clusterName) {
+ this.clusterName = clusterName;
+ }
+}
diff --git
a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/TrustedOidcIssuersResource.java
b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/TrustedOidcIssuersResource.java
index b8e8b393e..4bced34ff 100644
---
a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/TrustedOidcIssuersResource.java
+++
b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/TrustedOidcIssuersResource.java
@@ -16,7 +16,6 @@
*/
package org.apache.knox.gateway.service.knoxidf;
-import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.knox.gateway.audit.api.Action;
import org.apache.knox.gateway.audit.api.ActionOutcome;
@@ -90,14 +89,15 @@ public class TrustedOidcIssuersResource {
String outcome = ActionOutcome.FAILURE;
try {
- final Map<String, Object> parsed;
+ final RegisterIssuerRequest parsed;
try {
- parsed = MAPPER.readValue(body, new TypeReference<Map<String,
Object>>() {});
+ parsed = MAPPER.readValue(body, RegisterIssuerRequest.class);
} catch (IOException e) {
- return errorResponse(Response.Status.BAD_REQUEST, "invalid_request",
"Malformed JSON body");
+ return errorResponse(Response.Status.BAD_REQUEST, "invalid_request",
+ "Malformed or invalid JSON body");
}
- final String rawUrl = (String) parsed.get("issuerUrl");
+ final String rawUrl = parsed.getIssuerUrl();
issuerUrl = (rawUrl != null && !rawUrl.isEmpty()) ? rawUrl :
"UNKNOWN_ISSUER";
if (rawUrl == null || rawUrl.isEmpty()) {
@@ -112,13 +112,17 @@ public class TrustedOidcIssuersResource {
"Issuer already registered: " + rawUrl);
}
- final boolean dynamicJwks =
Boolean.TRUE.equals(parsed.get("dynamicJwks"));
- final String clusterName = (String) parsed.get("clusterName");
-
- trustedIssuers.register(new TrustedOidcIssuer(rawUrl, dynamicJwks,
clusterName,
- Instant.now(), operatorId));
+ trustedIssuers.register(new TrustedOidcIssuer(rawUrl,
parsed.isDynamicJwks(),
+ parsed.getClusterName(), Instant.now(), operatorId));
outcome = ActionOutcome.SUCCESS;
return Response.status(Response.Status.CREATED).build();
+ } catch (IllegalStateException e) {
+ // The service throws IllegalStateException when the configured maximum
number of
+ // registered issuers (MAX_TRUSTED_ISSUERS) is reached. This is an
operator-facing
+ // capacity condition, distinct from an internal storage failure, so
report it as a
+ // 409 rather than lumping it into the generic 500 storage_error path
below.
+ return errorResponse(Response.Status.CONFLICT, "issuer_limit_reached",
+ "Maximum number of registered trusted issuers reached");
} catch (RuntimeException e) {
return errorResponse(Response.Status.INTERNAL_SERVER_ERROR,
"storage_error",
"Failed to register issuer");
diff --git
a/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/TrustedOidcIssuersResourceTest.java
b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/TrustedOidcIssuersResourceTest.java
index e082d3255..b5cea534e 100644
---
a/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/TrustedOidcIssuersResourceTest.java
+++
b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/TrustedOidcIssuersResourceTest.java
@@ -16,7 +16,6 @@
*/
package org.apache.knox.gateway.service.knoxidf;
-import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.knox.gateway.audit.api.Action;
import org.apache.knox.gateway.audit.api.ActionOutcome;
@@ -251,6 +250,42 @@ public class TrustedOidcIssuersResourceTest {
EasyMock.verify(mockService, mockAuditor);
}
+ @Test
+ public void testRegisterWrongTypeFieldReturnsBadRequest() {
+ // A syntactically valid JSON body with a type-mismatched field
(clusterName as an
+ // array instead of a string). Binding to the typed RegisterIssuerRequest
bean makes
+ // Jackson reject this during deserialization, so it is a 400
invalid_request rather
+ // than a ClassCastException surfacing as a 500. No service calls are
expected; audit
+ // fires with the INVALID_REQUEST sentinel because parsing failed before
the URL was read.
+ expectAudit("INVALID_REQUEST", ActionOutcome.FAILURE, "issuer_registered");
+ EasyMock.replay(mockService, mockAuditor);
+
+ final Response response = resource.registerIssuer(
+ "{\"issuerUrl\":\"" + ISSUER_A + "\",\"clusterName\":[1,2,3]}");
+
+ assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
response.getStatus());
+ assertErrorField(response, "invalid_request");
+ EasyMock.verify(mockService, mockAuditor);
+ }
+
+ @Test
+ public void testRegisterIssuerLimitReached() {
+ // The service throws IllegalStateException when MAX_TRUSTED_ISSUERS is
reached. This is
+ // an operator-facing capacity condition and must map to 409
issuer_limit_reached, not
+ // the generic 500 storage_error used for genuine storage failures.
+ EasyMock.expect(mockService.isTrusted(ISSUER_A)).andReturn(false).once();
+ mockService.register(EasyMock.anyObject(TrustedOidcIssuer.class));
+ EasyMock.expectLastCall().andThrow(new
IllegalStateException("MAX_TRUSTED_ISSUERS (100) reached")).once();
+ expectAudit(ISSUER_A, ActionOutcome.FAILURE, "issuer_registered");
+ EasyMock.replay(mockService, mockAuditor);
+
+ final Response response =
resource.registerIssuer(buildRegisterBody(ISSUER_A, false, null));
+
+ assertEquals(Response.Status.CONFLICT.getStatusCode(),
response.getStatus());
+ assertErrorField(response, "issuer_limit_reached");
+ EasyMock.verify(mockService, mockAuditor);
+ }
+
//
---------------------------------------------------------------------------
// DELETE /
//
---------------------------------------------------------------------------
@@ -531,8 +566,9 @@ public class TrustedOidcIssuersResourceTest {
body.contains(expectedError));
}
+ @SuppressWarnings("unchecked")
private static List<Map<String, Object>> parseJsonList(String json) throws
Exception {
- return new ObjectMapper().readValue(json, new
TypeReference<List<Map<String, Object>>>() {});
+ return new ObjectMapper().readValue(json, List.class);
}
private static Map<String, Object> findByIssuerUrl(List<Map<String, Object>>
list,