You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by sm...@apache.org on 2021/09/09 07:33:20 UTC

[knox] branch master updated: KNOX-2658 - Skipping in-memory lookup while fetching expiration/metadata for a token (#490)

This is an automated email from the ASF dual-hosted git repository.

smolnar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git


The following commit(s) were added to refs/heads/master by this push:
     new 486abb0  KNOX-2658 - Skipping in-memory lookup while fetching expiration/metadata for a token (#490)
486abb0 is described below

commit 486abb0c3ad1d9ad6d3deb1af2dbe73da1968970
Author: Sandor Molnar <sm...@apache.org>
AuthorDate: Thu Sep 9 09:33:15 2021 +0200

    KNOX-2658 - Skipping in-memory lookup while fetching expiration/metadata for a token (#490)
---
 .../services/token/impl/JDBCTokenStateService.java | 39 +++++++++-------------
 .../token/impl/JDBCTokenStateServiceTest.java      | 16 +++++++++
 2 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateService.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateService.java
index 909f7c8..a0b65e8 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateService.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateService.java
@@ -119,12 +119,8 @@ public class JDBCTokenStateService extends DefaultTokenStateService {
 
   @Override
   public long getTokenExpiration(String tokenId, boolean validate) throws UnknownTokenException {
-    try {
-      // check the in-memory cache first
-      return super.getTokenExpiration(tokenId, validate);
-    } catch (UnknownTokenException e) {
-      // It's not in memory
-    }
+    // To support HA, there is no in-memory lookup here; we should go directly to the DB
+    // See KNOX-2658 for more details.
 
     if (validate) {
       validateToken(tokenId);
@@ -278,28 +274,23 @@ public class JDBCTokenStateService extends DefaultTokenStateService {
 
   @Override
   public TokenMetadata getTokenMetadata(String tokenId) throws UnknownTokenException {
+    // To support HA, there is no in-memory lookup here; we should go directly to the DB.
+    // See KNOX-2658 for more details.
+
     TokenMetadata tokenMetadata = null;
+
     try {
-      tokenMetadata = super.getTokenMetadata(tokenId);
-    } catch (UnknownTokenException e) {
-      // This is expected if the metadata is not yet part of the in-memory record. In this case, the metadata will
-      // be retrieved from the database.
-    }
+      tokenMetadata = tokenDatabase.getTokenMetadata(tokenId);
 
-    if (tokenMetadata == null) {
-      try {
-        tokenMetadata = tokenDatabase.getTokenMetadata(tokenId);
-
-        if (tokenMetadata != null) {
-          log.fetchedMetadataFromDatabase(Tokens.getTokenIDDisplayText(tokenId));
-          // Update the in-memory cache to avoid subsequent DB look-ups for the same state
-          super.addMetadata(tokenId, tokenMetadata);
-        } else {
-          throw new UnknownTokenException(tokenId);
-        }
-      } catch (SQLException e) {
-        log.errorFetchingMetadataFromDatabase(Tokens.getTokenIDDisplayText(tokenId), e.getMessage(), e);
+      if (tokenMetadata != null) {
+        log.fetchedMetadataFromDatabase(Tokens.getTokenIDDisplayText(tokenId));
+        // Update the in-memory cache to avoid subsequent DB look-ups for the same state
+        super.addMetadata(tokenId, tokenMetadata);
+      } else {
+        throw new UnknownTokenException(tokenId);
       }
+    } catch (SQLException e) {
+      log.errorFetchingMetadataFromDatabase(Tokens.getTokenIDDisplayText(tokenId), e.getMessage(), e);
     }
     return tokenMetadata;
   }
diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateServiceTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateServiceTest.java
index 2ae6408..a9d7547 100644
--- a/gateway-server/src/test/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateServiceTest.java
+++ b/gateway-server/src/test/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateServiceTest.java
@@ -28,11 +28,14 @@ import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.util.Map;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.codec.digest.HmacAlgorithms;
+import org.apache.commons.lang3.reflect.FieldUtils;
 import org.apache.derby.drda.NetworkServerControl;
 import org.apache.knox.gateway.config.GatewayConfig;
 import org.apache.knox.gateway.services.security.AliasService;
@@ -144,6 +147,12 @@ public class JDBCTokenStateServiceTest {
     jdbcTokenStateService.addToken(tokenId, 1, 1, 1);
     jdbcTokenStateService.updateExpiration(tokenId, 2);
 
+    // set token expiration to 3 in-memory
+    // we still expect 2 because in-memory lookup should be skipped while fetching token expiration
+    final Map<String, Long> tokenExpirations = new ConcurrentHashMap<>();
+    tokenExpirations.put(tokenId, 3L);
+    FieldUtils.writeField(jdbcTokenStateService, "tokenExpirations", tokenExpirations, true);
+
     assertEquals(2, jdbcTokenStateService.getTokenExpiration(tokenId));
     assertEquals(2, getLongTokenAttributeFromDatabase(tokenId, TokenStateDatabase.GET_TOKEN_EXPIRATION_SQL));
   }
@@ -173,6 +182,13 @@ public class JDBCTokenStateServiceTest {
     //enable the token (it was disabled)
     tokenMetadata.setEnabled(true);
     jdbcTokenStateService.addMetadata(tokenId, tokenMetadata);
+
+    // set token metadata back to original in the in-memory cache with disabled=false
+    // we still expect an enabled token because in-memory lookup should be skipped while fetching token metadata
+    final Map<String, TokenMetadata> metadataMap = new ConcurrentHashMap<>();
+    metadataMap.put(tokenId, tokenMetadata);
+    FieldUtils.writeField(jdbcTokenStateService, "metadataMap", metadataMap, true);
+
     assertTrue(jdbcTokenStateService.getTokenMetadata(tokenId).isEnabled());
     assertEquals("true", getStringTokenAttributeFromDatabase(tokenId, getSelectMetadataSql(TokenMetadata.ENABLED)));