smolnar82 commented on code in PR #1154:
URL: https://github.com/apache/knox/pull/1154#discussion_r2839917284


##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java:
##########
@@ -334,23 +342,44 @@ private Pair<TokenType, String> 
parseFromClientCredentialsFlow(ServletRequest re
       if (clientSecretPresentAsQueryString) {
         throw new SecurityException("client_secret must not be sent as a query 
parameter");
       }
-      return getClientCredentialsFromRequestBody(request);
+      return getTokenFromRequestBody(request);
     }
 
-    private Pair<TokenType, String> 
getClientCredentialsFromRequestBody(ServletRequest request) {
+    private Pair<TokenType, String> getTokenFromRequestBody(ServletRequest 
request) {
         final 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
+          // client credentials flow: client_id and client_secret are expected
+          // the client_id will be in the token as the token_id
           final String clientSecret = request.getParameter(CLIENT_SECRET);
           validateClientID((HttpServletRequest) request, clientSecret);
           return Pair.of(TokenType.Passcode, clientSecret);
+        } else if (REFRESH_TOKEN.equals(grantType)) {
+          // refresh_token flow: the refresh_token parameter contains the 
actual token
+          final String refreshToken = 
request.getParameter(REFRESH_TOKEN_PARAM);
+          if (refreshToken != null) {
+            // determine if it's a JWT or passcode token
+            if (isJWT(refreshToken)) {
+              return Pair.of(TokenType.JWT, refreshToken);
+            } else {
+              return Pair.of(TokenType.Passcode, refreshToken);
+            }
+          }
+        } else if (TOKEN_EXCHANGE.equals(grantType)) {
+          // token_exchange flow: the subject_token parameter contains the 
token to be exchanged
+          final String subjectToken = request.getParameter(SUBJECT_TOKEN);
+          if (subjectToken != null) {
+            // determine if it's a JWT or passcode token
+            if (isJWT(subjectToken)) {
+              return Pair.of(TokenType.JWT, subjectToken);
+            } else {
+              return Pair.of(TokenType.Passcode, subjectToken);
+            }

Review Comment:
   This could be:
   ```
   } else if (REFRESH_TOKEN.equals(grantType)) {
     getRefreshOrSubjectToken(REFRESH_TOKEN_PARAM);
   } else if (TOKEN_EXCHANGE.equals(grantType)) {
     getRefreshOrSubjectToken(SUBJECT_TOKEN);
   }
   ...
   
   
   private Pair<TokenType, String> getRefreshOrSubjectToken(String 
requestParamName) {
     final String refreshOrSubjectToken = 
request.getParameter(requestParamName);
     if (refreshOrSubjectToken != null) {
        return isJWT(refreshOrSubjectToken) ? return Pair.of(TokenType.JWT, 
refreshOrSubjectToken) : return Pair.of(TokenType.Passcode, 
refreshOrSubjectToken);
     }
     return null;
   }
   ```



##########
gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/OAuthFlowsFederationFilterTest.java:
##########
@@ -76,6 +78,28 @@ public void testGetWireTokenUsingClientCredentialsFlow() 
throws Exception {
       assertEquals(passcode, wireToken.getRight());
     }
 
+    @Test
+    public void testGetWireTokenUsingClientCredentialsBasicAuth() throws 
Exception {

Review Comment:
   Many of the below test duplicate code, I think there might be a way to 
remove most of it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to