Skip to content

KNOX-3424: Switch to JSON error messages in Knox token validation paths #1354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hanicz wants to merge 2 commits into
base: master
Choose a base branch
from
Open

KNOX-3424: Switch to JSON error messages in Knox token validation paths #1354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ed7ba8
KNOX-3424: Switch to JSON error messages in Knox token validation paths
hanicz Aug 21, 2026
a87272e
KNOX-3423: Address review comments
hanicz Aug 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions ...main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.apache.knox.gateway.util.CookieUtils;
import org.apache.knox.gateway.util.ServletRequestUtils;
import org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants;
import org.apache.knox.gateway.util.knoxidf.KnoxIDFUtils;

import javax.security.auth.Subject;
import javax.servlet.FilterChain;
Expand Down Expand Up @@ -553,6 +554,10 @@ protected Set<URI> resolveRegisteredIssuerJwks(String issuer, HttpServletRequest
@Override
protected void handleValidationError(HttpServletRequest request, HttpServletResponse response, int status,
String error) throws IOException {
if (Boolean.TRUE.equals(request.getAttribute(TOKEN_EXCHANGE_REQUEST_ATTR))) {
handleValidationError(request, response, status, "invalid_request", error);
return;
}
if (error != null) {
response.sendError(status, error);
}
Expand All @@ -561,6 +566,17 @@ protected void handleValidationError(HttpServletRequest request, HttpServletResp
}
}

/**
* Emit an RFC 8693 / RFC 6749 §5.2 JSON error response ({@code {"error": ..., "error_description":
* ...}}) for a token-exchange request. Called directly by {@link TokenExchangeHandler} when it has
* an explicit OAuth error code (e.g. {@code invalid_request}), and indirectly by the four-argument
* {@link #handleValidationError} for shared-path errors.
*/
void handleValidationError(HttpServletRequest request, HttpServletResponse response, int status,
String error, String description) throws IOException {
KnoxIDFUtils.writeErrorResponse(response, status, error, description);
}

/**
* A function that let's configured unauthenticated path requests to
* pass through without requiring authentication.
Expand Down
14 changes: 7 additions & 7 deletions ...ain/java/org/apache/knox/gateway/provider/federation/jwt/filter/TokenExchangeHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,35 +92,35 @@ void handle(HttpServletRequest request, HttpServletResponse response, FilterChai
// RFC 8693 section 2.1: subject_token and subject_token_type are REQUIRED.
if (subjectTokenValue == null || subjectTokenValue.isEmpty()) {
filter.handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST,
"invalid_request: the subject_token parameter is required");
"invalid_request", "the subject_token parameter is required");
return;
}
if (subjectTokenType == null || subjectTokenType.isEmpty()) {
filter.handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST,
"invalid_request: the subject_token_type parameter is required");
"invalid_request", "the subject_token_type parameter is required");
return;
}
// RFC 8693 section 2.1: actor_token_type is REQUIRED when actor_token is present and MUST NOT
// be present otherwise.
if (hasActorToken && !hasActorTokenType) {
filter.handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST,
"invalid_request: actor_token_type is required when actor_token is present");
"invalid_request", "actor_token_type is required when actor_token is present");
return;
}
if (!hasActorToken && hasActorTokenType) {
filter.handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST,
"invalid_request: actor_token_type must not be present without actor_token");
"invalid_request", "actor_token_type must not be present without actor_token");
return;
}
// Only JWT-family token types are supported.
if (isNotSupportedTokenType(subjectTokenType)) {
filter.handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST,
"unsupported_token_type: unsupported subject_token_type " + subjectTokenType);
"invalid_request", "unsupported subject_token_type " + subjectTokenType);
return;
}
if (hasActorToken && isNotSupportedTokenType(actorTokenType)) {
filter.handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST,
"unsupported_token_type: unsupported actor_token_type " + actorTokenType);
"invalid_request", "unsupported actor_token_type " + actorTokenType);
return;
}

Expand Down Expand Up @@ -148,7 +148,7 @@ void handle(HttpServletRequest request, HttpServletResponse response, FilterChai
filter.continueWithEstablishedSecurityContext(subject, request, response, chain);
} catch (ParseException | UnknownTokenException e) {
filter.handleValidationError(request, response, HttpServletResponse.SC_UNAUTHORIZED,
"Failed to parse token in token exchange: " + e.getMessage());
"invalid_request", "Failed to parse token in token exchange: " + e.getMessage());
}
}

Expand Down
150 changes: 111 additions & 39 deletions ...ava/org/apache/knox/gateway/provider/federation/JWTFederationFilterTokenExchangeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.net.URI;
import java.util.Date;
import java.util.HashMap;
Expand Down Expand Up @@ -235,16 +238,16 @@ public void testSignatureVerificationFails() throws Exception {

final HttpServletRequest request = buildTokenExchangeRequest(
subjectJwt.serialize(), buildContextWithIssuerService(issuerSvc));
final HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class);
response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
EasyMock.expectLastCall().once();
EasyMock.replay(request, response, issuerSvc);
final JsonErrorResponse response = new JsonErrorResponse();
EasyMock.replay(request, response.mock, issuerSvc);

final TestFilterChain chain = new TestFilterChain();
handler.doFilter(request, response, chain);
handler.doFilter(request, response.mock, chain);

Assert.assertFalse(chain.doFilterCalled);
EasyMock.verify(mockAuth, issuerSvc, response);
Assert.assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.status());
Assert.assertTrue(response.body(), response.body().contains("\"error\":\"invalid_request\""));
EasyMock.verify(mockAuth, issuerSvc);
}

/**
Expand Down Expand Up @@ -278,19 +281,20 @@ public void testExpiredTokenRejectedOnDynamicPath() throws Exception {

final HttpServletRequest request = buildTokenExchangeRequest(
expiredJwt.serialize(), buildContextWithIssuerService(issuerSvc));
final HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class);
response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Token has expired");
EasyMock.expectLastCall().once();
EasyMock.replay(request, response, issuerSvc);
final JsonErrorResponse response = new JsonErrorResponse();
EasyMock.replay(request, response.mock, issuerSvc);

final TestFilterChain chain = new TestFilterChain();
handler.doFilter(request, response, chain);
handler.doFilter(request, response.mock, chain);

Assert.assertFalse(chain.doFilterCalled);
if (capturedJwt.hasCaptured()) {
Assert.assertEquals(EXTERNAL_ISSUER, capturedJwt.getValue().getIssuer());
}
EasyMock.verify(mockAuth, issuerSvc, response);
Assert.assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.status());
Assert.assertTrue(response.body(), response.body().contains("\"error\":\"invalid_request\""));
Assert.assertTrue(response.body(), response.body().contains("Token has expired"));
EasyMock.verify(mockAuth, issuerSvc);
}

/**
Expand Down Expand Up @@ -324,19 +328,20 @@ public void testFutureNbfRejectedOnDynamicPath() throws Exception {

final HttpServletRequest request = buildTokenExchangeRequest(
nbfJwt.serialize(), buildContextWithIssuerService(issuerSvc));
final HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class);
response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Bad request: the NotBefore check failed");
EasyMock.expectLastCall().once();
EasyMock.replay(request, response, issuerSvc);
final JsonErrorResponse response = new JsonErrorResponse();
EasyMock.replay(request, response.mock, issuerSvc);

final TestFilterChain chain = new TestFilterChain();
handler.doFilter(request, response, chain);
handler.doFilter(request, response.mock, chain);

Assert.assertFalse(chain.doFilterCalled);
if (capturedJwt.hasCaptured()) {
Assert.assertEquals(EXTERNAL_ISSUER, capturedJwt.getValue().getIssuer());
}
EasyMock.verify(mockAuth, issuerSvc, response);
Assert.assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.status());
Assert.assertTrue(response.body(), response.body().contains("\"error\":\"invalid_request\""));
Comment thread
hanicz marked this conversation as resolved.
Assert.assertTrue(response.body(), response.body().contains("the NotBefore check failed"));
EasyMock.verify(mockAuth, issuerSvc);
}

/**
Expand Down Expand Up @@ -371,19 +376,20 @@ public void testAudienceMismatchRejectedOnDynamicPath() throws Exception {

final HttpServletRequest request = buildTokenExchangeRequest(
subjectJwt.serialize(), buildContextWithIssuerService(issuerSvc));
final HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class);
response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Bad request: missing required token audience");
EasyMock.expectLastCall().once();
EasyMock.replay(request, response, issuerSvc);
final JsonErrorResponse response = new JsonErrorResponse();
EasyMock.replay(request, response.mock, issuerSvc);

final TestFilterChain chain = new TestFilterChain();
handler.doFilter(request, response, chain);
handler.doFilter(request, response.mock, chain);

Assert.assertFalse(chain.doFilterCalled);
if (capturedJwt.hasCaptured()) {
Assert.assertEquals(EXTERNAL_ISSUER, capturedJwt.getValue().getIssuer());
}
EasyMock.verify(mockAuth, issuerSvc, response);
Assert.assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.status());
Assert.assertTrue(response.body(), response.body().contains("\"error\":\"invalid_request\""));
Comment thread
hanicz marked this conversation as resolved.
Assert.assertTrue(response.body(), response.body().contains("missing required token audience"));
EasyMock.verify(mockAuth, issuerSvc);
}

// ---------------------------------------------------------------------------
Expand All @@ -408,16 +414,16 @@ public void testUntrustedIssuerRejectedNoHttpCall() throws Exception {

final HttpServletRequest request = buildTokenExchangeRequest(
subjectJwt.serialize(), buildContextWithIssuerService(issuerSvc));
final HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class);
response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
EasyMock.expectLastCall().once();
EasyMock.replay(request, response, issuerSvc);
final JsonErrorResponse response = new JsonErrorResponse();
EasyMock.replay(request, response.mock, issuerSvc);

final TestFilterChain chain = new TestFilterChain();
handler.doFilter(request, response, chain);
handler.doFilter(request, response.mock, chain);

Assert.assertFalse(chain.doFilterCalled);
EasyMock.verify(issuerSvc, response);
Assert.assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.status());
Assert.assertTrue(response.body(), response.body().contains("\"error\":\"invalid_request\""));
EasyMock.verify(issuerSvc);
}

/**
Expand All @@ -437,16 +443,15 @@ public void testServiceUnavailable() throws Exception {

final HttpServletRequest request = buildTokenExchangeRequest(
subjectJwt.serialize(), buildServletContext(gws));
final HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class);
response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
EasyMock.expectLastCall().once();
EasyMock.replay(request, response);
final JsonErrorResponse response = new JsonErrorResponse();
EasyMock.replay(request, response.mock);

final TestFilterChain chain = new TestFilterChain();
handler.doFilter(request, response, chain);
handler.doFilter(request, response.mock, chain);

Assert.assertFalse(chain.doFilterCalled);
EasyMock.verify(response);
Assert.assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.status());
Assert.assertTrue(response.body(), response.body().contains("\"error\":\"invalid_request\""));
}

/**
Expand Down Expand Up @@ -550,13 +555,15 @@ public void testInsecureDynamicJwksUriRejectedByDefault() throws Exception {

final HttpServletRequest request = buildTokenExchangeRequest(
subjectJwt.serialize(), buildContextWithIssuerService(issuerSvc));
final HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class);
EasyMock.replay(request, response, issuerSvc);
final JsonErrorResponse response = new JsonErrorResponse();
EasyMock.replay(request, response.mock, issuerSvc);

final TestFilterChain chain = new TestFilterChain();
handler.doFilter(request, response, chain);
handler.doFilter(request, response.mock, chain);

Assert.assertFalse("Insecure (non-HTTPS) dynamic JWKS URI must be rejected OOTB", chain.doFilterCalled);
Assert.assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.status());
Assert.assertTrue(response.body(), response.body().contains("\"error\":\"invalid_request\""));
EasyMock.verify(mockAuth, issuerSvc);
}

Expand Down Expand Up @@ -810,6 +817,33 @@ public Object getAttribute(String name) {
Assert.assertEquals(KNOX_ISSUER, capturedAttrs.get(KnoxIDFConstants.TOKEN_ISS_ATTRIBUTE));
}

@Test
public void testTokenExchangeParamErrorEmitsRfcJsonError() throws Exception {
handler.init(new TestFilterConfig(getProperties()));

// grant_type marks a token-exchange dispatch, but subject_token is absent (niceMock returns null).
final HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class);
EasyMock.expect(request.getRequestURL()).andReturn(new StringBuffer(SERVICE_URL)).anyTimes();
EasyMock.expect(request.getParameter(GRANT_TYPE)).andReturn(JWTFederationFilter.TOKEN_EXCHANGE).anyTimes();
EasyMock.expect(request.getServletContext())
.andReturn(buildContextWithIssuerService(EasyMock.createNiceMock(TrustedOidcIssuerService.class))).anyTimes();
mockRequestAttributeStore(request);

final JsonErrorResponse response = new JsonErrorResponse();
EasyMock.replay(request, response.mock);

final TestFilterChain chain = new TestFilterChain();
handler.doFilter(request, response.mock, chain);

Assert.assertFalse(chain.doFilterCalled);
Assert.assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.status());
Assert.assertEquals("application/json; charset=UTF-8", response.contentType());
Assert.assertEquals("no-store", response.header("Cache-Control"));
Assert.assertEquals("no-cache", response.header("Pragma"));
Assert.assertTrue(response.body(), response.body().contains("\"error\":\"invalid_request\""));
Assert.assertTrue(response.body(), response.body().contains("subject_token"));
}

// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -876,4 +910,42 @@ private static void mockRequestAttributeStore(final HttpServletRequest request)
.andAnswer(() -> attrs.get(EasyMock.getCurrentArguments()[0])).anyTimes();
}

private static final class JsonErrorResponse {
private final HttpServletResponse mock;
private final StringWriter body = new StringWriter();
private final Capture<Integer> status = EasyMock.newCapture();
private final Capture<String> contentType = EasyMock.newCapture();
private final Map<String, String> headers = new HashMap<>();

JsonErrorResponse() throws IOException {
mock = EasyMock.createNiceMock(HttpServletResponse.class);
mock.setStatus(EasyMock.captureInt(status));
EasyMock.expectLastCall().anyTimes();
mock.setContentType(EasyMock.capture(contentType));
EasyMock.expectLastCall().anyTimes();
mock.setHeader(EasyMock.anyString(), EasyMock.anyString());
EasyMock.expectLastCall().andAnswer(() -> {
headers.put((String) EasyMock.getCurrentArguments()[0], (String) EasyMock.getCurrentArguments()[1]);
return null;
}).anyTimes();
EasyMock.expect(mock.getWriter()).andReturn(new PrintWriter(body)).anyTimes();
}

int status() {
return status.getValue();
}

String body() {
return body.toString();
}

String contentType() {
return contentType.hasCaptured() ? contentType.getValue() : null;
}

String header(String name) {
return headers.get(name);
}
}

}
Loading
Loading