[
https://issues.apache.org/jira/browse/KNOX-3145?focusedWorklogId=968610&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-968610
]
ASF GitHub Bot logged work on KNOX-3145:
----------------------------------------
Author: ASF GitHub Bot
Created on: 06/May/25 09:04
Start Date: 06/May/25 09:04
Worklog Time Spent: 10m
Work Description: smolnar82 commented on code in PR #1039:
URL: https://github.com/apache/knox/pull/1039#discussion_r2075043467
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java:
##########
@@ -240,17 +242,34 @@ public void doFilter(ServletRequest request,
ServletResponse response, FilterCha
// 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
+ 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)) {
+ prechecks = false;
+ log.wrongPasscodeToken(tokenId);
+ handleValidationError((HttpServletRequest) request,
(HttpServletResponse) response,
+ HttpServletResponse.SC_UNAUTHORIZED,
+ MISMATCHING_CLIENT_ID_AND_CLIENT_SECRET);
+ }
+ }
+ }
Review Comment:
nit: I might have created a new `boolean validateClientCredentialsFlow(...)`
method and assigned that to `prechecks`:
```
try {
...
prechecks = validateClientCredentialsFlow(...);
} catch (Exception e) {
...
}
...
// if this is a client credentials flow request, then ensure the presented
clientId is
// the actual owner of the client_secret
private boolean validateClientCredentialsFlow(...) {
final String requestBodyString = getRequestBodyString(request);
if (StringUtils.isNotBlank(requestBodyString )) {
...
}
```
Issue Time Tracking
-------------------
Worklog Id: (was: 968610)
Time Spent: 0.5h (was: 20m)
> Ensure that the CLIENT_ID presented with a CLIENT_SECRET is the owner of the
> secret
> -----------------------------------------------------------------------------------
>
> Key: KNOX-3145
> URL: https://issues.apache.org/jira/browse/KNOX-3145
> Project: Apache Knox
> Issue Type: Improvement
> Components: Server
> Reporter: Larry McCay
> Assignee: Larry McCay
> Priority: Major
> Fix For: 2.2.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Currently, the support for client_id and client_secret treats the inclusion
> of the CLIENT_ID as a formality of the client credentials flow and since the
> actual client_id is resolvable from the client_secret, it is ignored.
> While there it may be arguable whether we need to enforce this, it seems a
> reasonable expectation that they should match. Let's close that gap.
> We may need to decide whether we want to make that configurable. Is there a
> feature hidden in there somewhere?
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
