[
https://issues.apache.org/jira/browse/KNOX-3312?focusedWorklogId=1018799&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1018799
]
ASF GitHub Bot logged work on KNOX-3312:
----------------------------------------
Author: ASF GitHub Bot
Created on: 05/May/26 15:13
Start Date: 05/May/26 15:13
Worklog Time Spent: 10m
Work Description: lmccay commented on code in PR #1219:
URL: https://github.com/apache/knox/pull/1219#discussion_r3189553637
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java:
##########
@@ -361,12 +361,15 @@ private Pair<TokenType, String>
parseFromHTTPBasicCredentials(final String heade
String passcode = values[1].isEmpty() ? null : values[1];
if (TOKEN.equalsIgnoreCase(username) ||
PASSCODE.equalsIgnoreCase(username)) {
parsed = Pair.of(TOKEN.equalsIgnoreCase(username) ? TokenType.JWT :
TokenType.Passcode, passcode);
- } else if (request != null &&
CLIENT_CREDENTIALS.equals(request.getParameter(GRANT_TYPE))) {
- // Allow client_credentials flow where client_id/client_secret are
provided via HTTP Basic
- if (passcode != null) {
- validateClientID(username, passcode);
- parsed = Pair.of(TokenType.Passcode, passcode);
- }
+ } else if (request != null) {
+ HttpServletRequest unwrappedRequest =
ServletRequestUtils.unwrapHttpServletRequest(request);
Review Comment:
This touches on something that I've been discussing with @pzampino offline
about. It is really not clear why we hide these params in the first place. I
know that once you read them, the input stream has been opened and it can't be
done again, so it breaks certain usecases. In cases where we fully know that we
aren't passing it on to someone else to use, this is okay. These OAuth flows
are an example of when its okay. So, I have 2 reactions to your question:
1. A convenience method to get the wrapped param would be fine but should
make it clear that it is unwrapping with the name - getWrappedRequestParam. The
method you have above would maybe return a query param or a hidden request body
param. That would potentially break usecases without knowing it. Cracking open
the request body should be an explicit action unless/until we remove the hiding
of the params.
2. The fact that you've asked AND that you had to do the same elsewhere may
be another smell of this implementation and maybe we shouldn't hide them at all.
Bottomline: if we keep them hidden then the interface should make it very
clear what you are opting into. Additionally, we should check whether we really
need to hide them. The unwrapping code will still work if we stop hiding them
anyway.
Thoughts?
Issue Time Tracking
-------------------
Worklog Id: (was: 1018799)
Time Spent: 2h (was: 1h 50m)
> Client Credentials Flow with HTTP Basic needs Unwrapped Servlet Request
> -----------------------------------------------------------------------
>
> Key: KNOX-3312
> URL: https://issues.apache.org/jira/browse/KNOX-3312
> Project: Apache Knox
> Issue Type: Bug
> Components: JWT
> Reporter: Larry McCay
> Assignee: Larry McCay
> Priority: Major
> Fix For: 3.0.0
>
> Time Spent: 2h
> Remaining Estimate: 0h
>
> Current implementation can't get to the grant_type request param.
> Unit tests mock out the requests and make it hard to tease this out as an
> issue.
> When we know that there is an Authorization header and that it is Basic then
> we need to check whether there is the hardcoded username of token or passcode
> and if not, unwrap the request to check for a grant_type for OAuth
> client_credentials and handle it appropriately.
> Current implementation tries to check that but the params are hidden by the
> wrappers.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
