This is an automated email from the ASF dual-hosted git repository.
smolnar 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 a58344816 KNOX-3175 - Client credential flow attributes are read
without reading the entire request body (#1070)
a58344816 is described below
commit a58344816ab2e08fae9025318d7aee0d81f45159
Author: Sandor Molnar <smol...@apache.org>
AuthorDate: Fri Aug 1 09:45:09 2025 +0200
KNOX-3175 - Client credential flow attributes are read without reading the
entire request body (#1070)
---
.../federation/jwt/filter/JWTFederationFilter.java | 115 ++++++++-------------
...lientIdAndClientSecretFederationFilterTest.java | 45 ++++----
2 files changed, 64 insertions(+), 96 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 b7cc89131..f13b552a4 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
@@ -17,16 +17,31 @@
*/
package org.apache.knox.gateway.provider.federation.jwt.filter;
-import static java.nio.charset.StandardCharsets.UTF_8;
-import static
org.apache.knox.gateway.util.AuthFilterUtils.DEFAULT_AUTH_UNAUTHENTICATED_PATHS_PARAM;
+import com.nimbusds.jose.JOSEObjectType;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.provider.federation.jwt.JWTMessages;
+import org.apache.knox.gateway.security.PrimaryPrincipal;
+import org.apache.knox.gateway.services.security.token.UnknownTokenException;
+import org.apache.knox.gateway.services.security.token.impl.JWT;
+import org.apache.knox.gateway.services.security.token.impl.JWTToken;
+import org.apache.knox.gateway.util.AuthFilterUtils;
+import org.apache.knox.gateway.util.CertificateUtils;
+import org.apache.knox.gateway.util.CookieUtils;
-import java.io.BufferedReader;
+import javax.security.auth.Subject;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
-import java.io.InputStreamReader;
import java.net.URI;
import java.net.URISyntaxException;
-import java.net.URLDecoder;
-import java.nio.charset.StandardCharsets;
import java.text.ParseException;
import java.util.Arrays;
import java.util.Base64;
@@ -37,30 +52,8 @@ import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
-import javax.security.auth.Subject;
-import javax.servlet.FilterChain;
-import javax.servlet.FilterConfig;
-import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
-import javax.servlet.http.Cookie;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
-import org.apache.commons.lang3.StringUtils;
-import org.apache.commons.lang3.tuple.Pair;
-import org.apache.knox.gateway.i18n.messages.MessagesFactory;
-import org.apache.knox.gateway.provider.federation.jwt.JWTMessages;
-import org.apache.knox.gateway.security.PrimaryPrincipal;
-import org.apache.knox.gateway.services.security.token.UnknownTokenException;
-import org.apache.knox.gateway.services.security.token.impl.JWT;
-import org.apache.knox.gateway.services.security.token.impl.JWTToken;
-import org.apache.knox.gateway.util.AuthFilterUtils;
-import org.apache.knox.gateway.util.CertificateUtils;
-import org.apache.knox.gateway.util.CookieUtils;
-import org.apache.knox.gateway.util.RequestBodyUtils;
-
-import com.nimbusds.jose.JOSEObjectType;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static
org.apache.knox.gateway.util.AuthFilterUtils.DEFAULT_AUTH_UNAUTHENTICATED_PATHS_PARAM;
public class JWTFederationFilter extends AbstractJWTFilter {
@@ -274,19 +267,16 @@ public class JWTFederationFilter extends
AbstractJWTFilter {
private boolean validateClientCredentialsFlow(HttpServletRequest request,
HttpServletResponse response, String tokenId)
throws IOException {
boolean validated = true;
- final String requestBodyString = getRequestBodyString(request);
- if (requestBodyString != null && !requestBodyString.isEmpty()) {
- final String grantType =
RequestBodyUtils.getRequestBodyParameter(requestBodyString, GRANT_TYPE);
- if (grantType != null && !grantType.isEmpty()) {
- final String clientID =
RequestBodyUtils.getRequestBodyParameter(requestBodyString, 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((HttpServletRequest) request,
(HttpServletResponse) response,
- HttpServletResponse.SC_UNAUTHORIZED,
- MISMATCHING_CLIENT_ID_AND_CLIENT_SECRET);
- }
+ 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);
}
}
return validated;
@@ -346,39 +336,22 @@ public class JWTFederationFilter extends
AbstractJWTFilter {
&grant_type=client_credentials
*/
- if (request.getParameter(CLIENT_SECRET) != null) {
- throw new SecurityException();
+ final HttpServletRequest httpRequest = (HttpServletRequest) request;
+ final boolean clientSecretPresentAsQueryString =
httpRequest.getQueryString() != null &&
httpRequest.getQueryString().contains("client_secret=");
+ if (clientSecretPresentAsQueryString) {
+ throw new SecurityException("client_secret must not be sent as a query
parameter");
}
return getClientCredentialsFromRequestBody(request);
}
private Pair<TokenType, String>
getClientCredentialsFromRequestBody(ServletRequest request) throws IOException {
- try {
- final String requestBodyString = getRequestBodyString(request);
- final String grantType =
RequestBodyUtils.getRequestBodyParameter(requestBodyString, 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 =
RequestBodyUtils.getRequestBodyParameter(requestBodyString, CLIENT_SECRET);
- return Pair.of(TokenType.Passcode, clientSecret);
- }
- } catch (IOException e) {
- log.errorFetchingClientSecret(e.getMessage(), e);
- throw e;
- }
- return null;
- }
-
- private String getRequestBodyString(ServletRequest request) throws
IOException {
- if (request.getInputStream() != null) {
- final BufferedReader bufferedReader = new BufferedReader(new
InputStreamReader(request.getInputStream(), StandardCharsets.UTF_8));
- final StringBuilder requestBodyBuilder = new StringBuilder();
- String line;
- while ((line = bufferedReader.readLine()) != null) {
- requestBodyBuilder.append(line);
- }
- return URLDecoder.decode(requestBodyBuilder.toString(),
StandardCharsets.UTF_8.name());
+ 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);
}
return null;
}
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 1502684b6..004989c08 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
@@ -17,25 +17,19 @@
package org.apache.knox.gateway.provider.federation;
-import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.tuple.Pair;
import
org.apache.knox.gateway.provider.federation.jwt.filter.JWTFederationFilter;
import
org.apache.knox.gateway.provider.federation.jwt.filter.JWTFederationFilter.TokenType;
import
org.apache.knox.gateway.provider.federation.jwt.filter.SignatureVerificationCache;
import org.apache.knox.gateway.services.security.token.TokenMetadata;
import org.apache.knox.gateway.services.security.token.TokenStateService;
-import org.apache.knox.test.mock.MockServletInputStream;
import org.easymock.EasyMock;
import org.junit.Assert;
import org.junit.Test;
import javax.servlet.FilterConfig;
-import javax.servlet.ServletInputStream;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import java.io.IOException;
-import java.io.InputStream;
-import java.nio.charset.StandardCharsets;
import java.util.Properties;
import static org.junit.Assert.assertEquals;
@@ -46,29 +40,29 @@ public class ClientIdAndClientSecretFederationFilterTest
extends TokenIDAsHTTPBa
@Override
protected void setTokenOnRequest(HttpServletRequest request, String
authUsername, String authPassword) {
EasyMock.expect((Object)request.getHeader("Authorization")).andReturn("");
- try {
- EasyMock.expect(request.getInputStream()).andAnswer(() ->
produceServletInputStream(authPassword)).atLeastOnce();
- } catch (IOException e) {
- throw new RuntimeException("Error while setting up expectation for
getting client credentials from request body", e);
- }
+
EasyMock.expect((Object)request.getContentType()).andReturn("application/x-www-form-urlencoded").anyTimes();
+ ensureClientCredentials(request, authPassword, false);
}
- private ServletInputStream produceServletInputStream(String clientSecret) {
- final String requestBody = JWTFederationFilter.GRANT_TYPE + "=" +
JWTFederationFilter.CLIENT_CREDENTIALS + "&" +
JWTFederationFilter.CLIENT_SECRET + "="
- + clientSecret;
- final InputStream inputStream = IOUtils.toInputStream(requestBody,
StandardCharsets.UTF_8);
- return new MockServletInputStream(inputStream);
+ private void ensureClientCredentials(final HttpServletRequest request,
final String clientSecret) {
+ ensureClientCredentials(request, clientSecret, true);
+ }
+
+ private void ensureClientCredentials(final HttpServletRequest request,
final String clientSecret, final boolean excludeQueryString) {
+
EasyMock.expect(request.getParameter(JWTFederationFilter.GRANT_TYPE)).andReturn(JWTFederationFilter.CLIENT_CREDENTIALS).anyTimes();
+
EasyMock.expect(request.getParameter(JWTFederationFilter.CLIENT_SECRET)).andReturn(clientSecret).anyTimes();
+ if (excludeQueryString) {
+
EasyMock.expect(request.getQueryString()).andReturn(null).anyTimes();
+ }
}
@Test
public void testGetWireTokenUsingClientCredentialsFlow() throws Exception {
- final String clientID = "client_id=clientID";
final String clientSecret = "sup3r5ecreT!";
final HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
- EasyMock.expect(request.getInputStream()).andAnswer(() ->
produceServletInputStream(clientSecret + "&" +
- clientID)).atLeastOnce();
+ ensureClientCredentials(request, clientSecret);
+
EasyMock.expect(request.getHeader("Authorization")).andReturn(null).anyTimes();
EasyMock.replay(request);
-
handler.init(new TestFilterConfig(getProperties()));
final Pair<TokenType, String> wireToken = ((TestJWTFederationFilter)
handler).getWireToken(request);
@@ -105,8 +99,8 @@ public class ClientIdAndClientSecretFederationFilterTest
extends TokenIDAsHTTPBa
// LJM TODO: this will be needed later for client credentials as Basic
auth header
//EasyMock.expect(request.getHeader("Authorization")).andReturn(authTokenType +
passcodeToken);
- EasyMock.expect(request.getInputStream()).andAnswer(() ->
produceServletInputStream(passcodeToken +
- "&client_id=" + tokenId)).atLeastOnce();
+ ensureClientCredentials(request, passcodeToken);
+
EasyMock.expect(request.getParameter("client_id")).andReturn(tokenId).anyTimes();
final HttpServletResponse response =
EasyMock.createNiceMock(HttpServletResponse.class);
// response.setStatus(HttpServletResponse.SC_OK);
@@ -149,8 +143,8 @@ public class ClientIdAndClientSecretFederationFilterTest
extends TokenIDAsHTTPBa
// LJM TODO: this will be needed later for client credentials as Basic
auth header
//EasyMock.expect(request.getHeader("Authorization")).andReturn(authTokenType +
passcodeToken);
- EasyMock.expect(request.getInputStream()).andAnswer(() ->
produceServletInputStream(passcodeToken +
- "&client_id=" + tokenId +
"invalidating_string")).atLeastOnce();
+ ensureClientCredentials(request, passcodeToken);
+ EasyMock.expect(request.getParameter("client_id")).andReturn(tokenId +
"invalidating_string").anyTimes();
final HttpServletResponse response =
EasyMock.createNiceMock(HttpServletResponse.class);
response.sendError(HttpServletResponse.SC_UNAUTHORIZED,
@@ -171,7 +165,8 @@ public class ClientIdAndClientSecretFederationFilterTest
extends TokenIDAsHTTPBa
@Test(expected = SecurityException.class)
public void shouldFailIfClientSecretIsPassedInQueryParams() throws
Exception {
final HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
-
EasyMock.expect((Object)request.getParameter("client_secret")).andReturn("sup3r5ecreT!");
+ final String queryString =
"test=test&client_secret=sup3r5ecreT&otherTest=otherTestQueryParam";
+
EasyMock.expect(request.getQueryString()).andReturn(queryString).anyTimes();
EasyMock.replay(request);
handler.init(new TestFilterConfig(getProperties()));
