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 a3a33aa99 KNOX-3037 - Client credentials flow accepts essential
parameters in the request body only (#906)
a3a33aa99 is described below
commit a3a33aa997241a824ec457bd8e45132fdac92bb6
Author: Sandor Molnar <[email protected]>
AuthorDate: Thu May 9 22:59:35 2024 +0200
KNOX-3037 - Client credentials flow accepts essential parameters in the
request body only (#906)
---
.../provider/federation/jwt/JWTMessages.java | 9 +--
.../federation/jwt/filter/JWTFederationFilter.java | 64 +++++++++++++-----
...lientIdAndClientSecretFederationFilterTest.java | 59 +++++++++++++++--
gateway-util-common/pom.xml | 5 ++
.../apache/knox/gateway/util/RequestBodyUtils.java | 75 ++++++++++++++++++++++
.../knox/gateway/util/RequestBodyUtilsTest.java | 64 ++++++++++++++++++
6 files changed, 252 insertions(+), 24 deletions(-)
diff --git
a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
index e6979921c..fb72d9bcb 100644
---
a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
+++
b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
@@ -82,12 +82,10 @@ public interface JWTMessages {
@Message( level = MessageLevel.WARN, text = "Unable to derive authentication
provider URL: {0}" )
void failedToDeriveAuthenticationProviderUrl(@StackTrace( level =
MessageLevel.ERROR) Exception e);
- @Message( level = MessageLevel.ERROR,
- text = "The configuration value ({0}) for maximum token
verification cache is invalid; Using the default value." )
+ @Message( level = MessageLevel.ERROR, text = "The configuration value ({0})
for maximum token verification cache is invalid; Using the default value." )
void invalidVerificationCacheMaxConfiguration(String value);
- @Message( level = MessageLevel.ERROR,
- text = "Missing token passcode." )
+ @Message( level = MessageLevel.ERROR, text = "Missing token passcode." )
void missingTokenPasscode();
@Message( level = MessageLevel.INFO, text = "Initialized token signature
verification cache for the {0} topology." )
@@ -114,4 +112,7 @@ public interface JWTMessages {
@Message( level = MessageLevel.INFO, text = "Idle timeout has been
configured to {0} seconds in {1}" )
void configuredIdleTimeout(long idleTimeout, String topology);
+ @Message(level = MessageLevel.ERROR, text = "Error while fetching grant type
and client secret from the request: {0}")
+ void errorFetchingClientSecret(String errorMessage, @StackTrace(level =
MessageLevel.DEBUG) Exception e);
+
}
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 9c2001057..a58622a0e 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
@@ -20,7 +20,11 @@ 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 java.io.BufferedReader;
import java.io.IOException;
+import java.io.InputStreamReader;
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
import java.text.ParseException;
import java.util.Base64;
import java.util.HashSet;
@@ -50,6 +54,7 @@ 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;
@@ -173,7 +178,13 @@ public class JWTFederationFilter extends AbstractJWTFilter
{
}
}
- final Pair<TokenType, String> wireToken = getWireToken(request);
+ Pair<TokenType, String> wireToken = null;
+ try {
+ wireToken = getWireToken(request);
+ } catch (SecurityException e) {
+ handleValidationError((HttpServletRequest) request,
(HttpServletResponse) response, HttpServletResponse.SC_BAD_REQUEST, null);
+ throw e;
+ }
if (wireToken != null && wireToken.getLeft() != null &&
wireToken.getRight() != null) {
TokenType tokenType = wireToken.getLeft();
@@ -224,7 +235,7 @@ public class JWTFederationFilter extends AbstractJWTFilter {
return new String(Base64.getDecoder().decode(toBeDecoded.getBytes(UTF_8)),
UTF_8);
}
- public Pair<TokenType, String> getWireToken(final ServletRequest request) {
+ public Pair<TokenType, String> getWireToken(final ServletRequest request)
throws IOException {
Pair<TokenType, String> parsed = null;
String token = null;
final String header =
((HttpServletRequest)request).getHeader("Authorization");
@@ -253,12 +264,9 @@ public class JWTFederationFilter extends AbstractJWTFilter
{
}
return parsed;
- }
-
- private Pair<TokenType, String>
parseFromClientCredentialsFlow(ServletRequest request) {
- Pair<TokenType, String> parsed = null;
- String token = null;
+ }
+ private Pair<TokenType, String>
parseFromClientCredentialsFlow(ServletRequest request) throws IOException {
/*
POST /{tenant}/oauth2/v2.0/token HTTP/1.1
Host: login.microsoftonline.com:443
@@ -270,15 +278,41 @@ public class JWTFederationFilter extends
AbstractJWTFilter {
&grant_type=client_credentials
*/
- 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
- token = request.getParameter(CLIENT_SECRET);
- parsed = Pair.of(TokenType.Passcode, token);
+ if (request.getParameter(CLIENT_SECRET) != null) {
+ throw new SecurityException();
}
- return parsed;
+ 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());
+ }
+ return null;
}
private Pair<TokenType, String> parseFromHTTPBasicCredentials(final String
header) {
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 806125ad1..980d7c5ec 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,19 +17,68 @@
package org.apache.knox.gateway.provider.federation;
-import org.easymock.EasyMock;
-import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import javax.servlet.ServletInputStream;
import javax.servlet.http.HttpServletRequest;
+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.test.mock.MockServletInputStream;
+import org.easymock.EasyMock;
+import org.junit.Test;
+
public class ClientIdAndClientSecretFederationFilterTest extends
TokenIDAsHTTPBasicCredsFederationFilterTest {
@Override
protected void setTokenOnRequest(HttpServletRequest request, String
authUsername, String authPassword) {
EasyMock.expect((Object)request.getHeader("Authorization")).andReturn("");
-
EasyMock.expect((Object)request.getParameter("grant_type")).andReturn("client_credentials");
-
EasyMock.expect((Object)request.getParameter("client_id")).andReturn(authUsername);
-
EasyMock.expect((Object)request.getParameter("client_secret")).andReturn(authPassword);
+ 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);
+ }
+ }
+
+ 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);
+ }
+
+ @Test
+ public void testGetWireTokenUsingClientCredentialsFlow() throws Exception {
+ final String clientSecret = "sup3r5ecreT!";
+ final HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
+ EasyMock.expect(request.getInputStream()).andAnswer(() ->
produceServletInputStream(clientSecret)).atLeastOnce();
+ EasyMock.replay(request);
+
+ handler.init(new TestFilterConfig(getProperties()));
+ final Pair<TokenType, String> wireToken = ((TestJWTFederationFilter)
handler).getWireToken(request);
+
+ EasyMock.verify(request);
+
+ assertNotNull(wireToken);
+ assertEquals(TokenType.Passcode, wireToken.getLeft());
+ assertEquals(clientSecret, wireToken.getRight());
+ }
+
+ @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!");
+ EasyMock.replay(request);
+
+ handler.init(new TestFilterConfig(getProperties()));
+ ((TestJWTFederationFilter) handler).getWireToken(request);
}
@Override
diff --git a/gateway-util-common/pom.xml b/gateway-util-common/pom.xml
index 104e87b82..e2f0d5c66 100644
--- a/gateway-util-common/pom.xml
+++ b/gateway-util-common/pom.xml
@@ -94,5 +94,10 @@
<artifactId>gateway-test-utils</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>commons-io</groupId>
+ <artifactId>commons-io</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
diff --git
a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RequestBodyUtils.java
b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RequestBodyUtils.java
new file mode 100644
index 000000000..acfc46071
--- /dev/null
+++
b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RequestBodyUtils.java
@@ -0,0 +1,75 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.util;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
+
+import javax.servlet.ServletRequest;
+
+public class RequestBodyUtils {
+
+ public static String getRequestBodyParameter(ServletRequest request, String
parameter) throws IOException {
+ return getRequestBodyParameter(request, parameter, false);
+ }
+
+ public static String getRequestBodyParameter(ServletRequest request, String
parameter, boolean decode) throws IOException {
+ return getRequestBodyParameter(request.getInputStream(), parameter,
decode);
+ }
+
+ public static String getRequestBodyParameter(InputStream inputStream, String
parameter) throws IOException {
+ return getRequestBodyParameter(inputStream, parameter, false);
+ }
+
+ public static String getRequestBodyParameter(InputStream inputStream, String
parameter, boolean decode) throws IOException {
+ return getRequestBodyParameter(new InputStreamReader(inputStream,
StandardCharsets.UTF_8), parameter, decode);
+ }
+
+ public static String getRequestBodyParameter(Reader reader, String
parameter) throws IOException {
+ return getRequestBodyParameter(reader, parameter, false);
+ }
+
+ public static String getRequestBodyParameter(Reader reader, String
parameter, boolean decode) throws IOException {
+ final BufferedReader bufferedReader = new BufferedReader(reader);
+ final StringBuilder requestBodyBuilder = new StringBuilder();
+ String line;
+ while ((line = bufferedReader.readLine()) != null) {
+ requestBodyBuilder.append(line);
+ }
+
+ final String requestBodyString = decode ?
URLDecoder.decode(requestBodyBuilder.toString(), StandardCharsets.UTF_8.name())
: requestBodyBuilder.toString();
+ return getRequestBodyParameter(requestBodyString, parameter);
+ }
+
+ public static String getRequestBodyParameter(String requestBodyString,
String parameter) {
+ if (requestBodyString != null) {
+ final String[] requestBodyParams = requestBodyString.split("&");
+ for (String requestBodyParam : requestBodyParams) {
+ String[] keyValue = requestBodyParam.split("=", 2);
+ if (parameter.equals(keyValue[0])) {
+ return keyValue[1];
+ }
+ }
+ }
+ return null;
+ }
+}
diff --git
a/gateway-util-common/src/test/java/org/apache/knox/gateway/util/RequestBodyUtilsTest.java
b/gateway-util-common/src/test/java/org/apache/knox/gateway/util/RequestBodyUtilsTest.java
new file mode 100644
index 000000000..300a08660
--- /dev/null
+++
b/gateway-util-common/src/test/java/org/apache/knox/gateway/util/RequestBodyUtilsTest.java
@@ -0,0 +1,64 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.util;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+
+import javax.servlet.ServletInputStream;
+import javax.servlet.ServletRequest;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.knox.test.mock.MockServletInputStream;
+import org.easymock.EasyMock;
+import org.junit.Test;
+
+public class RequestBodyUtilsTest {
+
+ private static final String REQUEST_BODY_PARAM_NAME = "myParam";
+ private static final String REQUEST_BODY_PARAM_VALUE_RAW = "This-is_my
sample text!";
+ private static final String REQUEST_BODY_PARAM_VALUE_ENCODED =
"This-is_my%20sample%20text%21";
+
+ @Test
+ public void testGetRequestBodyParameterEncoded() throws Exception {
+ testGetRequestBodyParameter(true);
+ }
+
+ @Test
+ public void testGetRequestBodyParameterRaw() throws Exception {
+ testGetRequestBodyParameter(false);
+ }
+
+ private void testGetRequestBodyParameter(boolean decode) throws Exception {
+ final ServletRequest request =
EasyMock.createNiceMock(ServletRequest.class);
+
EasyMock.expect(request.getInputStream()).andReturn(produceServletInputStream(decode)).anyTimes();
+
+ EasyMock.replay(request);
+
+ final String requestBodyParam =
RequestBodyUtils.getRequestBodyParameter(request, REQUEST_BODY_PARAM_NAME,
decode);
+ assertEquals(REQUEST_BODY_PARAM_VALUE_RAW, requestBodyParam);
+ }
+
+ private ServletInputStream produceServletInputStream(boolean encode) {
+ final String requestBody = REQUEST_BODY_PARAM_NAME + "=" + (encode ?
REQUEST_BODY_PARAM_VALUE_ENCODED : REQUEST_BODY_PARAM_VALUE_RAW);
+ final InputStream inputStream = IOUtils.toInputStream(requestBody,
StandardCharsets.UTF_8);
+ return new MockServletInputStream(inputStream);
+ }
+
+}
