[GitHub] [knox] pzampino commented on a diff in pull request #562: KNOX-2734 - Passcode token is optional in TokenResource's response

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

pzampino commented on code in PR #562:
URL: https://github.com/apache/knox/pull/562#discussion_r855557121


##########
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##########
@@ -804,8 +804,11 @@ private Response getAuthenticationToken() {
         if (endpointPublicCert != null) {
           map.put(ENDPOINT_PUBLIC_CERT, endpointPublicCert);
         }
+
         final String passcode = UUID.randomUUID().toString();
-        map.put(PASSCODE, generatePasscodeField(tokenId, passcode));
+        if (tokenStateService != null && !tokenStateService.storeTokensInMemoryOnly()) {

Review Comment:
   This is where you could employ tokenStateService instanceof PersistentTokenStateService



##########
gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/TokenIDAsHTTPBasicCredsFederationFilterTest.java:
##########
@@ -372,6 +372,11 @@ public void start() throws ServiceLifecycleException {
         public void stop() throws ServiceLifecycleException {
         }
 
+        @Override
+        public boolean storeTokensInMemoryOnly() {

Review Comment:
   Rather than adding this method to all the implementations, I tend to think of these things in terms of  interfaces (e.g., PersistentTokenStateService).
   Then, the referencing code can check (tokenStateService instanceof PersistentTokenStore).
   Perhaps, the persistent implementations could extend an abstract PersistentTokenStateService class, which itself extends DefaultTokenStateService.



-- 
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: dev-unsubscribe@knox.apache.org

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