[Backport to main]fix security IT failure caused by weak password (opensearch-project#951) (opensearch-project#1257)

zane-neo · ylwu-amzn · web-flow · commit a47db3ab42a5 · 2023-08-28T14:21:44.000+08:00
* fix security IT failure caused by weak password (opensearch-project#951) Signed-off-by: Yaliang Wu <ylwu@amazon.com> * Fix pre-trained model metadata parse exception Signed-off-by: zane-neo <zaniu@amazon.com> --------- Signed-off-by: Yaliang Wu <ylwu@amazon.com> Signed-off-by: zane-neo <zaniu@amazon.com> Co-authored-by: Yaliang Wu <ylwu@amazon.com>
diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/ModelHelper.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/ModelHelper.java
@@ -150,16 +150,10 @@ public boolean isModelAllowed(MLRegisterModelInput registerModelInput, List mode
         String version = registerModelInput.getVersion();
         MLModelFormat modelFormat = registerModelInput.getModelFormat();
         for (Object meta: modelMetaList) {
-            Map<String, Object> metaMap = (Map<String, Object>) meta;
-            String name = (String) metaMap.get("name");
-            Map<String, Object> versions = (Map<String, Object>) metaMap.get("versions");
-            Object versionObj = versions.get(version);
-            if (versionObj == null) return false;
-            Map<String, Object> versionMap = (Map<String, Object>) versionObj;
-            Object formatObj = versionMap.get("format");
-            if (formatObj == null) return false;
-            List<String> formats = (List<String>) formatObj;
-            if (name.equals(modelName) && versions.containsKey(version.toLowerCase(Locale.ROOT)) && formats.contains(modelFormat.toString().toLowerCase(Locale.ROOT))) {
+            String name = (String) ((Map<String, Object>)meta).get("name");
+            List<String> versions = (List) ((Map<String, Object>)meta).get("version");
+            List<String> formats = (List) ((Map<String, Object>)meta).get("format");
+            if (name.equals(modelName) && versions.contains(version.toLowerCase(Locale.ROOT)) && formats.contains(modelFormat.toString().toLowerCase(Locale.ROOT))) {
                 return true;
             }
         }
diff --git a/ml-algorithms/src/test/java/org/opensearch/ml/engine/algorithms/text_embedding/ModelHelperTest.java b/ml-algorithms/src/test/java/org/opensearch/ml/engine/algorithms/text_embedding/ModelHelperTest.java
@@ -184,7 +184,7 @@ public void testDownloadPrebuiltModelMetaList() throws PrivilegedActionException
                 .modelNodeIds(new String[]{"node_id1"})
                 .build();
         List modelMetaList = modelHelper.downloadPrebuiltModelMetaList(taskId, registerModelInput);
-        assertEquals("huggingface/sentence-transformers/all-MiniLM-L12-v2", ((Map<String, String>)modelMetaList.get(0)).get("name"));
+        assertEquals("huggingface/sentence-transformers/all-distilroberta-v1", ((Map<String, String>)modelMetaList.get(0)).get("name"));
     }
 
     @Test
diff --git a/plugin/src/test/java/org/opensearch/ml/rest/MLModelGroupRestIT.java b/plugin/src/test/java/org/opensearch/ml/rest/MLModelGroupRestIT.java
@@ -58,6 +58,7 @@ public class MLModelGroupRestIT extends MLCommonsRestTestCase {
     public ExpectedException exceptionRule = ExpectedException.none();
 
     private String modelGroupId;
+    private String password = "IntegTest@MLModelGroupRestIT123";
 
     @Before
     public void setup() throws IOException {
@@ -77,56 +78,43 @@ public void setup() throws IOException {
         }
         createSearchRole(indexSearchAccessRole, "*");
 
-        createUser(mlNoAccessUser, mlNoAccessUser, ImmutableList.of(opensearchBackendRole));
-        mlNoAccessClient = new SecureRestClientBuilder(
-            getClusterHosts().toArray(new HttpHost[0]),
-            isHttps(),
-            mlNoAccessUser,
-            mlNoAccessUser
-        ).setSocketTimeout(60000).build();
+        createUser(mlNoAccessUser, password, ImmutableList.of(opensearchBackendRole));
+        mlNoAccessClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), mlNoAccessUser, password)
+            .setSocketTimeout(60000)
+            .build();
 
-        createUser(mlReadOnlyUser, mlReadOnlyUser, ImmutableList.of(opensearchBackendRole));
-        mlReadOnlyClient = new SecureRestClientBuilder(
-            getClusterHosts().toArray(new HttpHost[0]),
-            isHttps(),
-            mlReadOnlyUser,
-            mlReadOnlyUser
-        ).setSocketTimeout(60000).build();
+        createUser(mlReadOnlyUser, password, ImmutableList.of(opensearchBackendRole));
+        mlReadOnlyClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), mlReadOnlyUser, password)
+            .setSocketTimeout(60000)
+            .build();
 
-        createUser(mlFullAccessNoIndexAccessUser, mlFullAccessNoIndexAccessUser, ImmutableList.of(opensearchBackendRole));
+        createUser(mlFullAccessNoIndexAccessUser, password, ImmutableList.of(opensearchBackendRole));
         mlFullAccessNoIndexAccessClient = new SecureRestClientBuilder(
             getClusterHosts().toArray(new HttpHost[0]),
             isHttps(),
             mlFullAccessNoIndexAccessUser,
-            mlFullAccessNoIndexAccessUser
+            password
         ).setSocketTimeout(60000).build();
 
-        createUser(mlFullAccessUser, mlFullAccessUser, ImmutableList.of(opensearchBackendRole));
-        mlFullAccessClient = new SecureRestClientBuilder(
-            getClusterHosts().toArray(new HttpHost[0]),
-            isHttps(),
-            mlFullAccessUser,
-            mlFullAccessUser
-        ).setSocketTimeout(60000).build();
+        createUser(mlFullAccessUser, password, ImmutableList.of(opensearchBackendRole));
+        mlFullAccessClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), mlFullAccessUser, password)
+            .setSocketTimeout(60000)
+            .build();
 
-        createUser(mlNonAdminFullAccessWithoutBackendRoleUser, mlNonAdminFullAccessWithoutBackendRoleUser, ImmutableList.of());
+        createUser(mlNonAdminFullAccessWithoutBackendRoleUser, password, ImmutableList.of());
         mlNonAdminFullAccessWithoutBackendRoleClient = new SecureRestClientBuilder(
             getClusterHosts().toArray(new HttpHost[0]),
             isHttps(),
             mlNonAdminFullAccessWithoutBackendRoleUser,
-            mlNonAdminFullAccessWithoutBackendRoleUser
+            password
         ).setSocketTimeout(60000).build();
 
-        createUser(
-            mlNonOwnerFullAccessWithBackendRoleUser,
-            mlNonOwnerFullAccessWithBackendRoleUser,
-            ImmutableList.of(opensearchBackendRole)
-        );
+        createUser(mlNonOwnerFullAccessWithBackendRoleUser, password, ImmutableList.of(opensearchBackendRole));
         mlNonOwnerFullAccessWithBackendRoleClient = new SecureRestClientBuilder(
             getClusterHosts().toArray(new HttpHost[0]),
             isHttps(),
             mlNonOwnerFullAccessWithBackendRoleUser,
-            mlNonOwnerFullAccessWithBackendRoleUser
+            password
         ).setSocketTimeout(60000).build();
 
         createRoleMapping("ml_read_access", ImmutableList.of(mlReadOnlyUser));
diff --git a/plugin/src/test/java/org/opensearch/ml/rest/SecureMLRestIT.java b/plugin/src/test/java/org/opensearch/ml/rest/SecureMLRestIT.java
@@ -58,6 +58,7 @@ public class SecureMLRestIT extends MLCommonsRestTestCase {
     public ExpectedException exceptionRule = ExpectedException.none();
 
     private String modelGroupId;
+    private String password = "IntegTest@SecureMLRestIT123";
 
     @Before
     public void setup() throws IOException, ParseException {
@@ -77,37 +78,28 @@ public void setup() throws IOException, ParseException {
         }
         createSearchRole(indexSearchAccessRole, "*");
 
-        createUser(mlNoAccessUser, mlNoAccessUser, new ArrayList<>(Arrays.asList(opensearchBackendRole)));
-        mlNoAccessClient = new SecureRestClientBuilder(
-            getClusterHosts().toArray(new HttpHost[0]),
-            isHttps(),
-            mlNoAccessUser,
-            mlNoAccessUser
-        ).setSocketTimeout(60000).build();
+        createUser(mlNoAccessUser, password, new ArrayList<>(Arrays.asList(opensearchBackendRole)));
+        mlNoAccessClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), mlNoAccessUser, password)
+            .setSocketTimeout(60000)
+            .build();
 
-        createUser(mlReadOnlyUser, mlReadOnlyUser, new ArrayList<>(Arrays.asList(opensearchBackendRole)));
-        mlReadOnlyClient = new SecureRestClientBuilder(
-            getClusterHosts().toArray(new HttpHost[0]),
-            isHttps(),
-            mlReadOnlyUser,
-            mlReadOnlyUser
-        ).setSocketTimeout(60000).build();
+        createUser(mlReadOnlyUser, password, new ArrayList<>(Arrays.asList(opensearchBackendRole)));
+        mlReadOnlyClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), mlReadOnlyUser, password)
+            .setSocketTimeout(60000)
+            .build();
 
-        createUser(mlFullAccessNoIndexAccessUser, mlFullAccessNoIndexAccessUser, new ArrayList<>(Arrays.asList(opensearchBackendRole)));
+        createUser(mlFullAccessNoIndexAccessUser, password, new ArrayList<>(Arrays.asList(opensearchBackendRole)));
         mlFullAccessNoIndexAccessClient = new SecureRestClientBuilder(
             getClusterHosts().toArray(new HttpHost[0]),
             isHttps(),
             mlFullAccessNoIndexAccessUser,
-            mlFullAccessNoIndexAccessUser
+            password
         ).setSocketTimeout(60000).build();
 
-        createUser(mlFullAccessUser, mlFullAccessUser, new ArrayList<>(Arrays.asList(opensearchBackendRole)));
-        mlFullAccessClient = new SecureRestClientBuilder(
-            getClusterHosts().toArray(new HttpHost[0]),
-            isHttps(),
-            mlFullAccessUser,
-            mlFullAccessUser
-        ).setSocketTimeout(60000).build();
+        createUser(mlFullAccessUser, password, new ArrayList<>(Arrays.asList(opensearchBackendRole)));
+        mlFullAccessClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), mlFullAccessUser, password)
+            .setSocketTimeout(60000)
+            .build();
 
         createRoleMapping("ml_read_access", new ArrayList<>(Arrays.asList(mlReadOnlyUser)));
         createRoleMapping("ml_full_access", new ArrayList<>(Arrays.asList(mlFullAccessNoIndexAccessUser, mlFullAccessUser)));

Original file line number	Diff line number	Diff line change
`@@ -184,7 +184,7 @@ public void testDownloadPrebuiltModelMetaList() throws PrivilegedActionException`
`184`	`184`	`.modelNodeIds(new String[]{"node_id1"})`
`185`	`185`	`.build();`
`186`	`186`	`List modelMetaList = modelHelper.downloadPrebuiltModelMetaList(taskId, registerModelInput);`
`187`		`- assertEquals("huggingface/sentence-transformers/all-MiniLM-L12-v2", ((Map<String, String>)modelMetaList.get(0)).get("name"));`
	`187`	`+ assertEquals("huggingface/sentence-transformers/all-distilroberta-v1", ((Map<String, String>)modelMetaList.get(0)).get("name"));`
`188`	`188`	`}`
`189`	`189`
`190`	`190`	`@Test`