[GitHub] [knox] pzampino commented on a change in pull request #437: KNOX-2579 - Saving token.id claim in a hashed from in the DB

[GitBox <gi...@apache.org>]

pzampino commented on a change in pull request #437:
URL: https://github.com/apache/knox/pull/437#discussion_r630379211



##########
File path: gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java
##########
@@ -141,9 +142,21 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
         }
       } else if (TokenType.Passcode.equals(tokenType)) {
         // Validate the token based on the server-managed metadata
-        if (validateToken((HttpServletRequest) request, (HttpServletResponse) response, chain, tokenValue)) {
+        // The received token value must be a Base64 encoded value of Base64(tokenId)::Base64(rawPasscode)
+        String tokenId = null, passcode = null;

Review comment:
       nit: This is not typical Knox coding style

##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java
##########
@@ -316,6 +334,35 @@ protected long convertCharArrayToLong(char[] charArray) {
     return Long.parseLong(new String(charArray));
   }
 
+  @Override
+  public long getTokenIssueTime(String tokenId) throws UnknownTokenException {
+    // Check the in-memory collection first, to avoid costly keystore access when possible
+    try {
+      // check the in-memory cache first
+      return super.getTokenIssueTime(tokenId);
+    } catch (UnknownTokenException e) {
+      // It's not in memory
+    }
+
+    // If there is no associated state in the in-memory cache, proceed to check the alias service
+    long issueTime = 0;
+    try {
+      char[] issueTimeStr = getPasswordUsingAliasService(tokenId + TOKEN_ISSUE_TIME_POSTFIX);
+      if (issueTimeStr == null) {
+        throw new UnknownTokenException(tokenId);
+      }
+      issueTime = convertCharArrayToLong(issueTimeStr);
+      // Update the in-memory cache to avoid subsequent keystore look-ups for the same state
+      super.setIssueTime(tokenId, issueTime);

Review comment:
       Should use the setIssueTimeInMemory() method here?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org