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 10ac9f6b2 KNOX-3272: Improved OAuth flow checks and return 401 in case
of missing client ID or invalid client secret (#1170)
10ac9f6b2 is described below
commit 10ac9f6b2728ee221efc30792915f61c9ea9e43e
Author: Sandor Molnar <[email protected]>
AuthorDate: Fri Mar 6 17:01:32 2026 +0100
KNOX-3272: Improved OAuth flow checks and return 401 in case of missing
client ID or invalid client secret (#1170)
---
.../federation/jwt/filter/JWTFederationFilter.java | 7 ++-
.../federation/OAuthFlowsFederationFilterTest.java | 67 +++++++++++++++++-----
...okenIDAsHTTPBasicCredsFederationFilterTest.java | 10 +++-
3 files changed, 64 insertions(+), 20 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 5795d9a0e..f89f65566 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
@@ -58,6 +58,7 @@ public class JWTFederationFilter extends AbstractJWTFilter {
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 INVALID_CLIENT_SECRET = "Error while parsing the
received client secret";
public static final String MISMATCHING_CLIENT_ID_AND_CLIENT_SECRET = "Client
credentials flow with mismatching client_id and client_secret";
public static final String REFRESH_TOKEN = "refresh_token";
public static final String REFRESH_TOKEN_PARAM = "refresh_token";
@@ -171,7 +172,7 @@ public class JWTFederationFilter extends AbstractJWTFilter {
try {
wireToken = getWireToken(request);
} catch (SecurityException e) {
- handleValidationError((HttpServletRequest) request,
(HttpServletResponse) response, HttpServletResponse.SC_BAD_REQUEST, null);
+ handleValidationError((HttpServletRequest) request,
(HttpServletResponse) response, HttpServletResponse.SC_UNAUTHORIZED,
e.getMessage());
throw e;
}
@@ -230,10 +231,10 @@ public class JWTFederationFilter extends
AbstractJWTFilter {
final String[] base64DecodedTokenIdAndPasscode =
decodeBase64(tokenValue).split("::");
tokenId = decodeBase64(base64DecodedTokenIdAndPasscode[0]);
} catch (Exception e) {
- throw new SecurityException("Error while parsing the received client
secret", e);
+ throw new SecurityException(INVALID_CLIENT_SECRET, e);
}
// if there is no client_id then this is not a client credentials flow
- if (clientID != null && !tokenId.equals(clientID)) {
+ if (!tokenId.equals(clientID)) {
throw new SecurityException(MISMATCHING_CLIENT_ID_AND_CLIENT_SECRET);
}
}
diff --git
a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/OAuthFlowsFederationFilterTest.java
b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/OAuthFlowsFederationFilterTest.java
index 6b72b9d9d..1df3b7543 100644
---
a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/OAuthFlowsFederationFilterTest.java
+++
b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/OAuthFlowsFederationFilterTest.java
@@ -17,6 +17,7 @@
package org.apache.knox.gateway.provider.federation;
+import com.nimbusds.jwt.SignedJWT;
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;
@@ -32,6 +33,7 @@ import javax.servlet.FilterConfig;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.nio.charset.StandardCharsets;
+import java.text.ParseException;
import java.util.Base64;
import java.util.Properties;
@@ -40,34 +42,63 @@ import static org.junit.Assert.assertNotNull;
public class OAuthFlowsFederationFilterTest extends
TokenIDAsHTTPBasicCredsFederationFilterTest {
+
@Override
protected void setTokenOnRequest(HttpServletRequest request, String
authUsername, String authPassword) {
EasyMock.expect((Object)request.getHeader("Authorization")).andReturn("");
EasyMock.expect((Object)request.getContentType()).andReturn("application/x-www-form-urlencoded").anyTimes();
- ensureClientCredentials(request, authPassword, false);
+ ensureClientCredentials(request, authUsername, authPassword, false);
+ }
+
+ @Override
+ protected String getAuthUserName(final String authUserName, final
SignedJWT jwt) throws ParseException {
+ return super.getTokenId(jwt);
}
- private void ensureClientCredentials(final HttpServletRequest request,
final String clientSecret) {
- ensureClientCredentials(request, clientSecret, true);
+ private void ensureClientCredentials(final HttpServletRequest request,
final String clientId, final String clientSecret) {
+ ensureClientCredentials(request, clientId, clientSecret, true);
}
- private void ensureClientCredentials(final HttpServletRequest request,
final String clientSecret, final boolean excludeQueryString) {
+ private void ensureClientCredentials(final HttpServletRequest request,
final String clientId, final String clientSecret, final boolean
excludeQueryString) {
EasyMock.expect(request.getParameter(JWTFederationFilter.GRANT_TYPE)).andReturn(JWTFederationFilter.CLIENT_CREDENTIALS).anyTimes();
+
EasyMock.expect(request.getParameter(JWTFederationFilter.CLIENT_ID)).andReturn(clientId).anyTimes();
EasyMock.expect(request.getParameter(JWTFederationFilter.CLIENT_SECRET)).andReturn(clientSecret).anyTimes();
if (excludeQueryString) {
EasyMock.expect(request.getQueryString()).andReturn(null).anyTimes();
}
}
+ @Test
+ public void testGetWireTokenUsingClientCredentialsFlowWithoutClientId()
throws Exception {
+ final String passcode =
"WTJ4cFpXNTBMV2xrTFRFeU16UTE6OlkyeHBaVzUwTFhObFkzSmxkQzB4TWpNME5RPT0=";
+
+ final HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
+ ensureClientCredentials(request, null, passcode);
+
EasyMock.expect(request.getHeader("Authorization")).andReturn(null).anyTimes();
+ final HttpServletResponse response =
EasyMock.createNiceMock(HttpServletResponse.class);
+ response.sendError(HttpServletResponse.SC_UNAUTHORIZED,
JWTFederationFilter.MISMATCHING_CLIENT_ID_AND_CLIENT_SECRET);
+ EasyMock.expectLastCall().atLeastOnce();
+ EasyMock.replay(request, response);
+ handler.init(new TestFilterConfig(getProperties()));
+ SecurityException securityException = null;
+ try {
+ handler.doFilter(request, response, new TestFilterChain());
+ } catch (final SecurityException e) {
+ securityException = e;
+ }
+ assertNotNull(securityException);
+
assertEquals(JWTFederationFilter.MISMATCHING_CLIENT_ID_AND_CLIENT_SECRET,
securityException.getMessage());
+ EasyMock.verify(response);
+ }
+
@Test
public void testGetWireTokenUsingClientCredentialsFlow() throws Exception {
final String clientId = "client-id-12345";
final String passcode =
"WTJ4cFpXNTBMV2xrTFRFeU16UTE6OlkyeHBaVzUwTFhObFkzSmxkQzB4TWpNME5RPT0=";
final HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
- ensureClientCredentials(request, passcode);
+ ensureClientCredentials(request, clientId, 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);
@@ -164,18 +195,27 @@ public class OAuthFlowsFederationFilterTest extends
TokenIDAsHTTPBasicCredsFeder
((TestJWTFederationFilter) handler).getWireToken(request);
}
- @Test(expected = SecurityException.class)
+ @Test
public void testInvalidClientSecret() throws Exception {
final String passcode = "sUpers3cret!";
final HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
- ensureClientCredentials(request, passcode);
+ ensureClientCredentials(request, "client_123", passcode);
EasyMock.expect(request.getHeader("Authorization")).andReturn(null).anyTimes();
- EasyMock.replay(request);
+ final HttpServletResponse response =
EasyMock.createNiceMock(HttpServletResponse.class);
+ response.sendError(HttpServletResponse.SC_UNAUTHORIZED,
JWTFederationFilter.INVALID_CLIENT_SECRET);
+ EasyMock.expectLastCall().atLeastOnce();
+ EasyMock.replay(request, response);
handler.init(new TestFilterConfig(getProperties()));
- ((TestJWTFederationFilter) handler).getWireToken(request);
-
- EasyMock.verify(request);
+ SecurityException securityException = null;
+ try {
+ handler.doFilter(request, response, new TestFilterChain());
+ } catch (SecurityException e) {
+ securityException = e;
+ }
+ Assert.assertNotNull(securityException);
+ Assert.assertEquals(JWTFederationFilter.INVALID_CLIENT_SECRET,
securityException.getMessage());
+ EasyMock.verify(request, response);
}
@Override
@@ -260,8 +300,7 @@ public class OAuthFlowsFederationFilterTest extends
TokenIDAsHTTPBasicCredsFeder
final HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
final String clientCredentials =
"TkdVd1l6VTBPR0l0TmpVMk9DMDBNRFl4TFdFelpHTXROakk1TURnd09EYzJOVEJoOjpNREV6T0dGaFpXUXRZMkV5WVMwME4yWXhMVGhsWkRndFpUQmpNemszTlRrMlpqazE=";
EasyMock.expect(request.getRequestURL()).andReturn(new
StringBuffer(SERVICE_URL)).anyTimes();
- ensureClientCredentials(request, clientCredentials);
-
EasyMock.expect(request.getParameter("client_id")).andReturn(clientId).anyTimes();
+ ensureClientCredentials(request, clientId, clientCredentials);
return request;
}
diff --git
a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/TokenIDAsHTTPBasicCredsFederationFilterTest.java
b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/TokenIDAsHTTPBasicCredsFederationFilterTest.java
index 17f6fd53b..b92158f89 100644
---
a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/TokenIDAsHTTPBasicCredsFederationFilterTest.java
+++
b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/TokenIDAsHTTPBasicCredsFederationFilterTest.java
@@ -79,7 +79,7 @@ public class TokenIDAsHTTPBasicCredsFederationFilterTest
extends JWTAsHTTPBasicC
final String subject = (String)
jwt.getJWTClaimsSet().getClaim(JWTToken.SUBJECT);
final String passcode = (String)
jwt.getJWTClaimsSet().getClaims().get(PASSCODE_CLAIM);
addTokenState(jwt, issueTime, subject, passcode);
- setTokenOnRequest(request, TestJWTFederationFilter.PASSCODE,
generatePasscodeField(getTokenId(jwt), passcode));
+ setTokenOnRequest(request,
getAuthUserName(TestJWTFederationFilter.PASSCODE, jwt),
generatePasscodeField(getTokenId(jwt), passcode));
} catch(ParseException e) {
Assert.fail(e.getMessage());
}
@@ -106,18 +106,22 @@ public class TokenIDAsHTTPBasicCredsFederationFilterTest
extends JWTAsHTTPBasicC
final String subject = (String)
jwt.getJWTClaimsSet().getClaim(JWTToken.SUBJECT);
final String passcode = (String)
jwt.getJWTClaimsSet().getClaims().get(PASSCODE_CLAIM);
addTokenState(jwt, issueTime, subject, passcode);
- setTokenOnRequest(request, authUsername,
generatePasscodeField(getTokenId(jwt), passcode));
+ setTokenOnRequest(request, getAuthUserName(authUsername, jwt),
generatePasscodeField(getTokenId(jwt), passcode));
} catch(ParseException e) {
Assert.fail(e.getMessage());
}
}
+ protected String getAuthUserName(final String authUserName, final
SignedJWT jwt) throws ParseException {
+ return authUserName;
+ }
+
@Override
protected void setGarbledTokenOnRequest(final HttpServletRequest request,
final SignedJWT jwt) {
setTokenOnRequest(request, TestJWTFederationFilter.PASSCODE, "junk" +
getTokenId(jwt));
}
- private String getTokenId(final SignedJWT jwt) {
+ protected String getTokenId(final SignedJWT jwt) {
String tokenId = null;
try {
tokenId = (String)
jwt.getJWTClaimsSet().getClaim(JWTToken.KNOX_ID_CLAIM);
