This is an automated email from the ASF dual-hosted git repository.
lmccay 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 dd4dc8ffe KNOX-3145 - Ensure Client Credentials flow client_secret
belongs to t… (#1039)
dd4dc8ffe is described below
commit dd4dc8ffefe39ea4de53c5a49720e8523cd0a515
Author: lmccay <[email protected]>
AuthorDate: Tue May 6 18:04:44 2025 -0400
KNOX-3145 - Ensure Client Credentials flow client_secret belongs to t…
(#1039)
* KNOX-3145 - Ensure Client Credentials flow client_secret belongs to the
presented client_id
---
.../federation/jwt/filter/JWTFederationFilter.java | 30 +++++-
...lientIdAndClientSecretFederationFilterTest.java | 119 +++++++++++++++++++--
2 files changed, 136 insertions(+), 13 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 0fefd76fc..b7cc89131 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
@@ -70,6 +70,8 @@ public class JWTFederationFilter extends AbstractJWTFilter {
public static final String GRANT_TYPE = "grant_type";
public static final String CLIENT_CREDENTIALS = "client_credentials";
public static final String CLIENT_SECRET = "client_secret";
+ public static final String CLIENT_ID = "client_id";
+ public static final String MISMATCHING_CLIENT_ID_AND_CLIENT_SECRET = "Client
credentials flow with mismatching client_id and client_secret";
public enum TokenType {
JWT, Passcode;
@@ -240,17 +242,20 @@ 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 (validateToken((HttpServletRequest) request, (HttpServletResponse)
response, chain, tokenId, passcode)) {
+ if (prechecks && validateToken((HttpServletRequest) request,
(HttpServletResponse) response, chain, tokenId, passcode)) {
try {
Subject subject = createSubjectFromTokenIdentifier(tokenId);
continueWithEstablishedSecurityContext(subject,
(HttpServletRequest) request, (HttpServletResponse) response, chain);
@@ -266,6 +271,27 @@ 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);
+ }
+ }
+ }
+ return validated;
+ }
+
private String decodeBase64(String toBeDecoded) {
return new String(Base64.getDecoder().decode(toBeDecoded.getBytes(UTF_8)),
UTF_8);
}
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 980d7c5ec..1502684b6 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,24 +17,30 @@
package org.apache.knox.gateway.provider.federation;
-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.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;
+import static org.junit.Assert.assertNotNull;
+
public class ClientIdAndClientSecretFederationFilterTest extends
TokenIDAsHTTPBasicCredsFederationFilterTest {
@Override
@@ -56,9 +62,11 @@ public class ClientIdAndClientSecretFederationFilterTest
extends TokenIDAsHTTPBa
@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)).atLeastOnce();
+ EasyMock.expect(request.getInputStream()).andAnswer(() ->
produceServletInputStream(clientSecret + "&" +
+ clientID)).atLeastOnce();
EasyMock.replay(request);
handler.init(new TestFilterConfig(getProperties()));
@@ -71,6 +79,95 @@ public class ClientIdAndClientSecretFederationFilterTest
extends TokenIDAsHTTPBa
assertEquals(clientSecret, wireToken.getRight());
}
+ @Test
+ public void testVerifyClientCredentialsFlow() throws Exception {
+ final String topologyName = "jwt-topology";
+ final String tokenId = "4e0c548b-6568-4061-a3dc-62908087650a";
+ final String passcode = "0138aaed-ca2a-47f1-8ed8-e0c397596f95";
+ String passcodeToken =
"TkdVd1l6VTBPR0l0TmpVMk9DMDBNRFl4TFdFelpHTXROakk1TURnd09EYzJOVEJoOjpNREV6T0dGaFpXUXRZMkV5WVMwME4yWXhMVGhsWkRndFpUQmpNemszTlRrMlpqazE=";
+
+ final TokenStateService tokenStateService =
EasyMock.createNiceMock(TokenStateService.class);
+
EasyMock.expect(tokenStateService.getTokenExpiration(tokenId)).andReturn(Long.MAX_VALUE).anyTimes();
+
+ final TokenMetadata tokenMetadata =
EasyMock.createNiceMock(TokenMetadata.class);
+ EasyMock.expect(tokenMetadata.isEnabled()).andReturn(true).anyTimes();
+
EasyMock.expect(tokenMetadata.getPasscode()).andReturn(passcodeToken).anyTimes();
+
EasyMock.expect(tokenStateService.getTokenMetadata(EasyMock.anyString())).andReturn(tokenMetadata).anyTimes();
+
+ final Properties filterConfigProps = getProperties();
+ filterConfigProps.put(TokenStateService.CONFIG_SERVER_MANAGED,
Boolean.toString(true));
+ filterConfigProps.put(TestFilterConfig.TOPOLOGY_NAME_PROP,
topologyName);
+ final FilterConfig filterConfig = new
TestFilterConfig(filterConfigProps, tokenStateService);
+ handler.init(filterConfig);
+
+ final HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
+ EasyMock.expect(request.getRequestURL()).andReturn(new
StringBuffer(SERVICE_URL)).anyTimes();
+
+ // 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();
+
+ final HttpServletResponse response =
EasyMock.createNiceMock(HttpServletResponse.class);
+// response.setStatus(HttpServletResponse.SC_OK);
+// EasyMock.expectLastCall().once();
+ EasyMock.replay(tokenStateService, tokenMetadata, request, response);
+
+ SignatureVerificationCache.getInstance(topologyName,
filterConfig).recordSignatureVerification(passcode);
+
+ final TestFilterChain chain = new TestFilterChain();
+ handler.doFilter(request, response, chain);
+
+ EasyMock.verify(response);
+ Assert.assertTrue(chain.doFilterCalled);
+ Assert.assertNotNull(chain.subject);
+ }
+
+ @Test
+ public void testFailedVerifyClientCredentialsFlow() throws Exception {
+ final String topologyName = "jwt-topology";
+ final String tokenId = "4e0c548b-6568-4061-a3dc-62908087650a";
+ final String passcode = "0138aaed-ca2a-47f1-8ed8-e0c397596f95";
+ String passcodeToken =
"TkdVd1l6VTBPR0l0TmpVMk9DMDBNRFl4TFdFelpHTXROakk1TURnd09EYzJOVEJoOjpNREV6T0dGaFpXUXRZMkV5WVMwME4yWXhMVGhsWkRndFpUQmpNemszTlRrMlpqazE=";
+
+ final TokenStateService tokenStateService =
EasyMock.createNiceMock(TokenStateService.class);
+
EasyMock.expect(tokenStateService.getTokenExpiration(tokenId)).andReturn(Long.MAX_VALUE).anyTimes();
+
+ final TokenMetadata tokenMetadata =
EasyMock.createNiceMock(TokenMetadata.class);
+ EasyMock.expect(tokenMetadata.isEnabled()).andReturn(true).anyTimes();
+
EasyMock.expect(tokenMetadata.getPasscode()).andReturn(passcodeToken).anyTimes();
+
EasyMock.expect(tokenStateService.getTokenMetadata(EasyMock.anyString())).andReturn(tokenMetadata).anyTimes();
+
+ final Properties filterConfigProps = getProperties();
+ filterConfigProps.put(TokenStateService.CONFIG_SERVER_MANAGED,
Boolean.toString(true));
+ filterConfigProps.put(TestFilterConfig.TOPOLOGY_NAME_PROP,
topologyName);
+ final FilterConfig filterConfig = new
TestFilterConfig(filterConfigProps, tokenStateService);
+ handler.init(filterConfig);
+
+ final HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
+ EasyMock.expect(request.getRequestURL()).andReturn(new
StringBuffer(SERVICE_URL)).anyTimes();
+
+ // 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();
+
+ final HttpServletResponse response =
EasyMock.createNiceMock(HttpServletResponse.class);
+ response.sendError(HttpServletResponse.SC_UNAUTHORIZED,
+ JWTFederationFilter.MISMATCHING_CLIENT_ID_AND_CLIENT_SECRET);
+ EasyMock.expectLastCall().once();
+ EasyMock.replay(tokenStateService, tokenMetadata, request, response);
+
+ SignatureVerificationCache.getInstance(topologyName,
filterConfig).recordSignatureVerification(passcode);
+
+ final TestFilterChain chain = new TestFilterChain();
+ handler.doFilter(request, response, chain);
+
+ EasyMock.verify(response);
+ Assert.assertFalse(chain.doFilterCalled);
+ Assert.assertNull(chain.subject);
+ }
+
@Test(expected = SecurityException.class)
public void shouldFailIfClientSecretIsPassedInQueryParams() throws
Exception {
final HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
