This is an automated email from the ASF dual-hosted git repository.
pzampino pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new 0068c63b7 KNOX-3179: Moved client id validation into client secret
parse, Modify getParameter in UrlEncodedFormRequest (#1074)
0068c63b7 is described below
commit 0068c63b75fc688c80010deb6743a82b15a20fd2
Author: hanicz <[email protected]>
AuthorDate: Tue Aug 12 16:20:48 2025 +0200
KNOX-3179: Moved client id validation into client secret parse, Modify
getParameter in UrlEncodedFormRequest (#1074)
* KNOX-3179: Moved client id validation into client secret parse, Modify
getParameter in UrlEncodedFormRequest
* KNOX-3179: New test for UrlEncodedFormRequest getParameter
* KNOX-3179: Use existing mismatch message
---
.../federation/jwt/filter/JWTFederationFilter.java | 53 ++++++++++------------
...lientIdAndClientSecretFederationFilterTest.java | 35 ++++++++++++--
.../apache/knox/gateway/UrlEncodedFormRequest.java | 4 ++
.../knox/gateway/UrlEncodedFormRequestTest.java | 8 ++--
4 files changed, 63 insertions(+), 37 deletions(-)
diff --git
a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java
b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java
index f13b552a4..6e292370d 100644
---
a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java
+++
b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java
@@ -235,20 +235,16 @@ public class JWTFederationFilter extends
AbstractJWTFilter {
// The received token value must be a Base64 encoded value of
Base64(tokenId)::Base64(rawPasscode)
String tokenId = null;
String passcode = null;
- boolean prechecks = true;
try {
final String[] base64DecodedTokenIdAndPasscode =
decodeBase64(tokenValue).split("::");
tokenId = decodeBase64(base64DecodedTokenIdAndPasscode[0]);
passcode = decodeBase64(base64DecodedTokenIdAndPasscode[1]);
- // if this is a client credentials flow request then ensure the
presented clientId is
- // the actual owner of the client_secret
- prechecks = validateClientCredentialsFlow((HttpServletRequest)
request, (HttpServletResponse) response, tokenId);
} catch (Exception e) {
log.failedToParsePasscodeToken(e);
handleValidationError((HttpServletRequest) request,
(HttpServletResponse) response, HttpServletResponse.SC_UNAUTHORIZED,
"Error while parsing the received passcode token");
}
- if (prechecks && validateToken((HttpServletRequest) request,
(HttpServletResponse) response, chain, tokenId, passcode)) {
+ if (validateToken((HttpServletRequest) request, (HttpServletResponse)
response, chain, tokenId, passcode)) {
try {
Subject subject = createSubjectFromTokenIdentifier(tokenId);
continueWithEstablishedSecurityContext(subject,
(HttpServletRequest) request, (HttpServletResponse) response, chain);
@@ -264,22 +260,19 @@ public class JWTFederationFilter extends
AbstractJWTFilter {
}
}
- private boolean validateClientCredentialsFlow(HttpServletRequest request,
HttpServletResponse response, String tokenId)
- throws IOException {
- boolean validated = true;
- final String grantType = request.getParameter(GRANT_TYPE);
- if (grantType != null && !grantType.isEmpty()) {
- final String clientID = request.getParameter(CLIENT_ID);
- // if there is no client_id then this is not a client credentials flow
- if (clientID != null && !tokenId.equals(clientID)) {
- validated = false;
- log.wrongPasscodeToken(tokenId);
- handleValidationError(request, response,
- HttpServletResponse.SC_UNAUTHORIZED,
- MISMATCHING_CLIENT_ID_AND_CLIENT_SECRET);
- }
+ private void validateClientID(HttpServletRequest request, String tokenValue)
{
+ final String clientID = request.getParameter(CLIENT_ID);
+ String tokenId;
+ try {
+ final String[] base64DecodedTokenIdAndPasscode =
decodeBase64(tokenValue).split("::");
+ tokenId = decodeBase64(base64DecodedTokenIdAndPasscode[0]);
+ } catch (Exception e) {
+ throw new SecurityException("Error while parsing the received client
secret", e);
+ }
+ // if there is no client_id then this is not a client credentials flow
+ if (clientID != null && !tokenId.equals(clientID)) {
+ throw new SecurityException(MISMATCHING_CLIENT_ID_AND_CLIENT_SECRET);
}
- return validated;
}
private String decodeBase64(String toBeDecoded) {
@@ -344,15 +337,16 @@ public class JWTFederationFilter extends
AbstractJWTFilter {
return getClientCredentialsFromRequestBody(request);
}
- private Pair<TokenType, String>
getClientCredentialsFromRequestBody(ServletRequest request) throws IOException {
- final String grantType = request.getParameter(GRANT_TYPE);
- if (CLIENT_CREDENTIALS.equals(grantType)) {
- // this is indeed a client credentials flow client_id and
- // client_secret are expected now the client_id will be in
- // the token as the token_id so we will get that later
- final String clientSecret = request.getParameter( CLIENT_SECRET);
- return Pair.of(TokenType.Passcode, clientSecret);
- }
+ private Pair<TokenType, String>
getClientCredentialsFromRequestBody(ServletRequest request) {
+ final String grantType = request.getParameter(GRANT_TYPE);
+ if (CLIENT_CREDENTIALS.equals(grantType)) {
+ // this is indeed a client credentials flow client_id and
+ // client_secret are expected now the client_id will be in
+ // the token as the token_id so we will get that later
+ final String clientSecret = request.getParameter(CLIENT_SECRET);
+ validateClientID((HttpServletRequest) request, clientSecret);
+ return Pair.of(TokenType.Passcode, clientSecret);
+ }
return null;
}
@@ -450,5 +444,4 @@ public class JWTFederationFilter extends AbstractJWTFilter {
super("None of the presented cookies are valid.");
}
}
-
}
diff --git
a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/ClientIdAndClientSecretFederationFilterTest.java
b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/ClientIdAndClientSecretFederationFilterTest.java
index 004989c08..191f2c9a4 100644
---
a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/ClientIdAndClientSecretFederationFilterTest.java
+++
b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/ClientIdAndClientSecretFederationFilterTest.java
@@ -58,10 +58,13 @@ public class ClientIdAndClientSecretFederationFilterTest
extends TokenIDAsHTTPBa
@Test
public void testGetWireTokenUsingClientCredentialsFlow() throws Exception {
- final String clientSecret = "sup3r5ecreT!";
+ final String clientId = "client-id-12345";
+ final String passcode =
"WTJ4cFpXNTBMV2xrTFRFeU16UTE6OlkyeHBaVzUwTFhObFkzSmxkQzB4TWpNME5RPT0=";
+
final HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
- ensureClientCredentials(request, clientSecret);
+ ensureClientCredentials(request, passcode);
EasyMock.expect(request.getHeader("Authorization")).andReturn(null).anyTimes();
+
EasyMock.expect(request.getParameter(JWTFederationFilter.CLIENT_ID)).andReturn(clientId).anyTimes();
EasyMock.replay(request);
handler.init(new TestFilterConfig(getProperties()));
final Pair<TokenType, String> wireToken = ((TestJWTFederationFilter)
handler).getWireToken(request);
@@ -70,7 +73,7 @@ public class ClientIdAndClientSecretFederationFilterTest
extends TokenIDAsHTTPBa
assertNotNull(wireToken);
assertEquals(TokenType.Passcode, wireToken.getLeft());
- assertEquals(clientSecret, wireToken.getRight());
+ assertEquals(passcode, wireToken.getRight());
}
@Test
@@ -117,7 +120,7 @@ public class ClientIdAndClientSecretFederationFilterTest
extends TokenIDAsHTTPBa
Assert.assertNotNull(chain.subject);
}
- @Test
+ @Test(expected = SecurityException.class)
public void testFailedVerifyClientCredentialsFlow() throws Exception {
final String topologyName = "jwt-topology";
final String tokenId = "4e0c548b-6568-4061-a3dc-62908087650a";
@@ -173,6 +176,20 @@ public class ClientIdAndClientSecretFederationFilterTest
extends TokenIDAsHTTPBa
((TestJWTFederationFilter) handler).getWireToken(request);
}
+ @Test(expected = SecurityException.class)
+ public void testInvalidClientSecret() throws Exception {
+ final String passcode = "sUpers3cret!";
+
+ final HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
+ ensureClientCredentials(request, passcode);
+
EasyMock.expect(request.getHeader("Authorization")).andReturn(null).anyTimes();
+ EasyMock.replay(request);
+ handler.init(new TestFilterConfig(getProperties()));
+ ((TestJWTFederationFilter) handler).getWireToken(request);
+
+ EasyMock.verify(request);
+ }
+
@Override
@Test
public void testInvalidUsername() throws Exception {
@@ -192,4 +209,14 @@ public class ClientIdAndClientSecretFederationFilterTest
extends TokenIDAsHTTPBa
// set by the JWTProvider when determining that the request
// is a client credentials flow.
}
+
+ @Override
+ @Test
+ public void testInvalidPasscodeForJWT() throws Exception {
+ }
+
+ @Override
+ @Test
+ public void testUnableToParseJWT() throws Exception {
+ }
}
diff --git
a/gateway-server/src/main/java/org/apache/knox/gateway/UrlEncodedFormRequest.java
b/gateway-server/src/main/java/org/apache/knox/gateway/UrlEncodedFormRequest.java
index 2e2482aa1..88219d91c 100644
---
a/gateway-server/src/main/java/org/apache/knox/gateway/UrlEncodedFormRequest.java
+++
b/gateway-server/src/main/java/org/apache/knox/gateway/UrlEncodedFormRequest.java
@@ -26,6 +26,7 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import
org.apache.knox.gateway.provider.federation.jwt.filter.JWTFederationFilter;
import org.eclipse.jetty.util.MultiMap;
import org.eclipse.jetty.util.UrlEncoded;
@@ -77,6 +78,9 @@ public class UrlEncodedFormRequest extends
HttpServletRequestWrapper {
@Override
public String getParameter(String name) {
+ if(JWTFederationFilter.GRANT_TYPE.equals(name) ||
JWTFederationFilter.CLIENT_ID.equals(name) ||
JWTFederationFilter.CLIENT_SECRET.equals(name)) {
+ return super.getParameter(name);
+ }
return queryParams.getValue(name, 0);
}
diff --git
a/gateway-server/src/test/java/org/apache/knox/gateway/UrlEncodedFormRequestTest.java
b/gateway-server/src/test/java/org/apache/knox/gateway/UrlEncodedFormRequestTest.java
index 87a77a700..19f0b39b4 100644
---
a/gateway-server/src/test/java/org/apache/knox/gateway/UrlEncodedFormRequestTest.java
+++
b/gateway-server/src/test/java/org/apache/knox/gateway/UrlEncodedFormRequestTest.java
@@ -39,17 +39,19 @@ public class UrlEncodedFormRequestTest {
@Test
public void testParametersAreComingFromQueryStringOnly() throws Exception {
- MockHttpServletRequest originalRequest = makeRequest("a=1&b=2",
"query1=x&query2=y&query2=y2");
+ MockHttpServletRequest originalRequest =
makeRequest("a=1&b=2&client_id=payload_client_id",
"query1=x&query2=y&query2=y2&client_id=query_client_id");
assertEquals("1", originalRequest.getParameter("a"));
assertEquals("2", originalRequest.getParameter("b"));
+ assertEquals("payload_client_id",
originalRequest.getParameter("client_id"));
UrlEncodedFormRequest wrappedRequest = new
UrlEncodedFormRequest(originalRequest);
assertEquals("x", wrappedRequest.getParameter("query1"));
assertEquals("y", wrappedRequest.getParameter("query2"));
+ assertEquals("payload_client_id",
wrappedRequest.getParameter("client_id"));
assertNull(wrappedRequest.getParameter("a"));
assertNull(wrappedRequest.getParameter("b"));
assertArrayEquals(new String[]{"x"},
wrappedRequest.getParameterValues("query1"));
assertArrayEquals(new String[]{"y", "y2"},
wrappedRequest.getParameterValues("query2"));
- assertEquals(Arrays.asList("query1", "query2"),
Collections.list(wrappedRequest.getParameterNames()));
+ assertEquals(Arrays.asList("query1", "query2", "client_id"),
Collections.list(wrappedRequest.getParameterNames()));
assertArrayEquals(new String[]{"x"},
wrappedRequest.getParameterMap().get("query1"));
assertArrayEquals(new String[]{"y", "y2"},
wrappedRequest.getParameterMap().get("query2"));
assertNull(wrappedRequest.getParameterValues("unknown"));
@@ -81,4 +83,4 @@ public class UrlEncodedFormRequestTest {
new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8))));
return request;
}
-}
\ No newline at end of file
+}
