You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by sm...@apache.org on 2024/05/09 20:59:39 UTC
(knox) branch master updated: KNOX-3037 - Client credentials flow accepts essential parameters in the request body only (#906)
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 <sm...@apache.org>
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);
+ }
+
+}