You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by am...@apache.org on 2022/05/27 10:59:20 UTC

[knox] branch master updated: KNOX-2747 RemoteAliasService generates password without checking if it already exists (#581)

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

amagyar 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 e2bfa1535 KNOX-2747 RemoteAliasService generates password without checking if it already exists (#581)
e2bfa1535 is described below

commit e2bfa1535317186c3e7b69de188f998aeb864431
Author: Attila Magyar <m....@gmail.com>
AuthorDate: Fri May 27 12:59:16 2022 +0200

    KNOX-2747 RemoteAliasService generates password without checking if it already exists (#581)
---
 .../services/security/impl/RemoteAliasService.java | 10 +--
 .../security/impl/RemoteAliasServiceTest.java      | 79 +++++++++++++++-------
 .../impl/RemoteAliasServiceTestProvider.java       |  3 +-
 3 files changed, 63 insertions(+), 29 deletions(-)

diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/RemoteAliasService.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/RemoteAliasService.java
index 863956e1d..440382204 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/RemoteAliasService.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/RemoteAliasService.java
@@ -153,11 +153,6 @@ public class RemoteAliasService extends AbstractAliasService {
     /* convert all alias names to lower case since JDK expects the same behaviour */
     final String alias = givenAlias.toLowerCase(Locale.ROOT);
 
-    /* Generate a new password  */
-    if (generate) {
-      generateAliasForCluster(clusterName, alias);
-    }
-
     char[] password = null;
 
     /* try to get it from remote registry */
@@ -176,6 +171,11 @@ public class RemoteAliasService extends AbstractAliasService {
       password = localAliasService.getPasswordFromAliasForCluster(clusterName, alias);
     }
 
+    if (password == null && generate) {
+      generateAliasForCluster(clusterName, alias); // generate password and save it in both local and remote keystore (if configured)
+      password = localAliasService.getPasswordFromAliasForCluster(clusterName, alias); // get the generated password from the local store
+    }
+
     /* found nothing */
     return password;
   }
diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/RemoteAliasServiceTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/RemoteAliasServiceTest.java
index 1c1596fa5..550ca6712 100644
--- a/gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/RemoteAliasServiceTest.java
+++ b/gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/RemoteAliasServiceTest.java
@@ -123,19 +123,65 @@ public class RemoteAliasServiceTest {
     Assert.assertEquals(aliases.size(), 1);
     Assert.assertTrue("Expected alias 'knox.test.alias' not found ",
         aliases.contains(expectedAlias));
+  }
 
-    /* Test auto-generate password for alias */
-    final String testAutoGeneratedpasswordAlias = "knox.test.alias.auto";
+  @Test
+  public void testDoesNotGeneratePasswordIfAlreadyExists() throws Exception {
+    GatewayConfig gatewayConfig = EasyMock.createNiceMock(GatewayConfig.class);
+    EasyMock.expect(gatewayConfig.isRemoteAliasServiceEnabled())
+            .andReturn(true).anyTimes();
+    String keystoreDir = testFolder.newFolder().getAbsolutePath();
+    EasyMock.expect(gatewayConfig.getGatewayKeystoreDir()).andReturn(keystoreDir).anyTimes();
+    EasyMock.expect(gatewayConfig.getCredentialStoreType()).andReturn(GatewayConfig.DEFAULT_CREDENTIAL_STORE_TYPE).anyTimes();
+    EasyMock.expect(gatewayConfig.getCredentialStoreAlgorithm()).andReturn(GatewayConfig.DEFAULT_CREDENTIAL_STORE_ALG).anyTimes();
 
-    final char[] autoGeneratedPassword = remoteAliasService
-        .getPasswordFromAliasForCluster(expectedClusterName,
-            testAutoGeneratedpasswordAlias, true);
-    aliases = remoteAliasService.getAliasesForCluster(expectedClusterName);
+    EasyMock.replay(gatewayConfig);
 
-    Assert.assertNotNull(autoGeneratedPassword);
-    Assert.assertEquals(aliases.size(), 2);
-    Assert.assertTrue("Expected alias 'knox.test.alias' not found ",
-        aliases.contains(testAutoGeneratedpasswordAlias));
+    final DefaultMasterService ms = EasyMock
+            .createNiceMock(DefaultMasterService.class);
+    EasyMock.expect(ms.getMasterSecret()).andReturn("knox".toCharArray())
+            .anyTimes();
+    EasyMock.replay(ms);
+
+    DefaultKeystoreService ks = new DefaultKeystoreService();
+    ks.setMasterService(ms);
+    ks.init(gatewayConfig, Collections.emptyMap());
+
+    DefaultAliasService defaultAlias = new DefaultAliasService();
+    defaultAlias.setKeystoreService(ks);
+    defaultAlias.setMasterService(ms);
+    defaultAlias.init(gatewayConfig, Collections.emptyMap());
+
+    final RemoteAliasService remoteAliasService = new RemoteAliasService(defaultAlias, ms);
+    remoteAliasService.init(gatewayConfig, Collections.emptyMap());
+    remoteAliasService.start();
+
+    defaultAlias.addAliasForCluster("cluster", "alias", "existing");
+    Assert.assertEquals("existing",
+            new String(remoteAliasService.getPasswordFromAliasForCluster("cluster", "alias", true)));
+  }
+
+  @Test
+  public void testGeneratesPasswordIfDoesNotExist() throws Exception {
+    char[] GENERATED_PASSWORD = "generated".toCharArray();
+    DefaultMasterService ms = EasyMock
+            .createNiceMock(DefaultMasterService.class);
+    EasyMock.expect(ms.getMasterSecret()).andReturn("knox".toCharArray()).anyTimes();
+
+    DefaultAliasService defaultAlias = EasyMock.createNiceMock(DefaultAliasService.class);
+    EasyMock.expect(defaultAlias.getPasswordFromAliasForCluster("cluster", "alias"))
+            .andReturn(null)
+            .andReturn(GENERATED_PASSWORD);
+
+    EasyMock.replay(ms, defaultAlias);
+
+    RemoteAliasService remoteAliasService = new RemoteAliasService(defaultAlias, ms);
+    remoteAliasService.start();
+
+    Assert.assertEquals("generated",
+            new String(remoteAliasService.getPasswordFromAliasForCluster("cluster", "alias", true)));
+
+    EasyMock.verify(defaultAlias);
   }
 
   @Test
@@ -327,19 +373,6 @@ public class RemoteAliasServiceTest {
     Assert.assertEquals(aliases.size(), 1);
     Assert.assertTrue("Expected alias 'knox.test.alias' not found ",
         aliases.contains(expectedAlias));
-
-    /* Test auto-generate password for alias */
-    final String testAutoGeneratedpasswordAlias = "knox.test.alias.auto";
-
-    final char[] autoGeneratedPassword = remoteAliasService
-                                             .getPasswordFromAliasForCluster(expectedClusterName,
-                                                 testAutoGeneratedpasswordAlias, true);
-    aliases = remoteAliasService.getAliasesForCluster(expectedClusterName);
-
-    Assert.assertNotNull(autoGeneratedPassword);
-    Assert.assertEquals(aliases.size(), 2);
-    Assert.assertTrue("Expected alias 'knox.test.alias' not found ",
-        aliases.contains(testAutoGeneratedpasswordAlias));
   }
 
   /*
diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/RemoteAliasServiceTestProvider.java b/gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/RemoteAliasServiceTestProvider.java
index e99aff3b3..67db9483b 100644
--- a/gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/RemoteAliasServiceTestProvider.java
+++ b/gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/RemoteAliasServiceTestProvider.java
@@ -88,7 +88,8 @@ public class RemoteAliasServiceTestProvider implements RemoteAliasServiceProvide
 
     @Override
     public char[] getPasswordFromAliasForCluster(String clusterName, String alias) {
-      return aliases.getOrDefault(clusterName, new HashMap<>()).get(alias).toCharArray();
+      String value = aliases.getOrDefault(clusterName, new HashMap<>()).get(alias);
+      return value == null ? null : value.toCharArray();
     }
 
     @Override