hanicz commented on code in PR #1208:
URL: https://github.com/apache/knox/pull/1208#discussion_r3129132574


##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java:
##########
@@ -405,22 +406,34 @@ public Subject createSubjectFromTokenIdentifier(final 
String tokenId) throws Unk
       // token id until it is created, the username is always the same
       // in the record. Using the token id makes it a unique username for
       // audit and the like.
-      final String username = metadata.isClientId() ? tokenId : 
metadata.getUserName();
-
-      return createSubjectFromTokenData(username, null);
+      // However, in case of service-to-service communications, it might
+      // be useful to return the actual userName during the token exchange
+      // step in the client credentials flow (this is controlled by the 
thirdPartyApp)
+      // flag that was (or wasn't) submitted while the client credentials were 
created
+      final String userName = (metadata.isClientId() && 
metadata.isThirdPartyApp()) ? tokenId : metadata.getUserName();
+      final String clientId = (metadata.isClientId() && 
!metadata.isThirdPartyApp()) ? tokenId : null;
+      return createSubjectFromTokenData(userName, null, clientId);
     }
     return null;
   }
 
   @SuppressWarnings("rawtypes")
   protected Subject createSubjectFromTokenData(final String principal, final 
String expectedPrincipalClaimValue) {
+    return createSubjectFromTokenData(principal, expectedPrincipalClaimValue, 
null);
+  }
+
+  @SuppressWarnings("rawtypes")
+  protected Subject createSubjectFromTokenData(final String principal, final 
String expectedPrincipalClaimValue, final String tokenId) {

Review Comment:
   nit: I think the last param should be called clientId as per the updated 
logic



##########
gateway-service-knoxtoken/src/test/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceResourceTest.java:
##########
@@ -1487,6 +1487,37 @@ public void 
passcodeShouldNotBeInResponseIfTokenStateServiceIsDisabled() throws
     testPasscodeToken(false, false, false);
   }
 
+  @Test
+  public void testClientCredentialsThirdPartyAppConfig() throws Exception {
+    tryClientCredentialsThirdPartyAppConfig(null, true);
+    tryClientCredentialsThirdPartyAppConfig("true", true);
+    tryClientCredentialsThirdPartyAppConfig("false", false);
+  }
+
+  private void tryClientCredentialsThirdPartyAppConfig(String configValue, 
boolean expectedValue) throws Exception {
+    try {
+      tss = new PersistentTestTokenStateService();
+      final Map<String, String> contextExpectations = new HashMap<>();
+      if (configValue != null) {
+        contextExpectations.put("clientid.thirdPartyApp", configValue);

Review Comment:
   Is this test correct? You don't use the `clientid.` prefix in 
`ClientCredentialsResource`



-- 
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