implemented required changes as per review:

- removed unnecessary instantiation of PasswordHasher in tests - improved the exception message when password hashing alg is not supported Signed-off-by: Dan Cecoi <[email protected]>
opensearch-project · Jul 11, 2024 · 7823689 · 7823689
1 parent ffa68f2
commit 7823689
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 12 deletions.
diff --git a/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java
@@ -79,6 +79,10 @@ public abstract class AbstractApiIntegrationTest extends RandomizedTest {
 
     public static final ToXContentObject EMPTY_BODY = (builder, params) -> builder.startObject().endObject();
 
+    public static final PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(
+        Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT).build()
+    );
+
     public static Path configurationFolder;
 
     public static ImmutableMap.Builder<String, Object> clusterSettings = ImmutableMap.builder();
@@ -87,10 +91,6 @@ public abstract class AbstractApiIntegrationTest extends RandomizedTest {
 
     public static LocalCluster localCluster;
 
-    public static PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(
-        Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT).build()
-    );
-
     @BeforeClass
     public static void startCluster() throws IOException {
         configurationFolder = ConfigurationFiles.createConfigurationDirectory();

diff --git a/...integrationTest/java/org/opensearch/security/api/InternalUsersRestApiIntegrationTest.java b/...integrationTest/java/org/opensearch/security/api/InternalUsersRestApiIntegrationTest.java
@@ -27,15 +27,11 @@
 import org.junit.Assert;
 import org.junit.Test;
 
-import org.opensearch.common.settings.Settings;
 import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.core.common.Strings;
 import org.opensearch.core.xcontent.ToXContentObject;
 import org.opensearch.security.DefaultObjectMapper;
 import org.opensearch.security.dlic.rest.api.Endpoint;
-import org.opensearch.security.hasher.PasswordHasher;
-import org.opensearch.security.hasher.PasswordHasherFactory;
-import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.test.framework.TestSecurityConfig;
 import org.opensearch.test.framework.cluster.TestRestClient;
 import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;
@@ -61,9 +57,6 @@ public class InternalUsersRestApiIntegrationTest extends AbstractConfigEntityApi
 
     private final static String SOME_ROLE = "some-role";
 
-    private final PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(
-        Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT).build()
-    );
     static {
         testSecurityConfig.withRestAdminUser(REST_API_ADMIN_INTERNAL_USERS_ONLY, restAdminPermission(Endpoint.INTERNALUSERS))
             .user(new TestSecurityConfig.User(SERVICE_ACCOUNT_USER).attr("service", "true").attr("enabled", "true"))

diff --git a/src/main/java/org/opensearch/security/hasher/PasswordHasherFactory.java b/src/main/java/org/opensearch/security/hasher/PasswordHasherFactory.java
@@ -38,7 +38,7 @@ public static PasswordHasher createPasswordHasher(Settings settings) {
                 passwordHasher = getPBKDF2Hasher(settings);
                 break;
             default:
-                throw new IllegalArgumentException("Password hashing algorithm not supported");
+                throw new IllegalArgumentException(String.format("Password hashing algorithm '%s' not supported.", algorithm));
         }
         return passwordHasher;
     }