fixing validate access for multi-tenancy (#4196)

* fixing validate access for multi-tenancy

Signed-off-by: Dhrubo Saha <dhrubo@amazon.com>

* adding unit test

Signed-off-by: Dhrubo Saha <dhrubo@amazon.com>

---------

Signed-off-by: Dhrubo Saha <dhrubo@amazon.com>

Author: dhrubo-os

plugin/src/main/java/org/opensearch/ml/helper/ModelAccessControlHelper.java

@@ -132,12 +132,20 @@ public void validateModelGroupAccess(
         SdkClient sdkClient,
         ActionListener<Boolean> listener
     ) {
-        if (modelGroupId == null
-            || (!mlFeatureEnabledSetting.isMultiTenancyEnabled()
-                && (isAdmin(user) || !isSecurityEnabledAndModelAccessControlEnabled(user)))) {
+        if (modelGroupId == null) {
             listener.onResponse(true);
             return;
         }
+
+        if (mlFeatureEnabledSetting.isMultiTenancyEnabled()) {
+            listener.onResponse(true);  // Multi-tenancy handles access control
+            return;
+        }
+
+        if (isAdmin(user) || !isSecurityEnabledAndModelAccessControlEnabled(user)) {
+            listener.onResponse(true);  // Admin or security disabled
+            return;
+        }
         GetDataObjectRequest getModelGroupRequest = GetDataObjectRequest
             .builder()
             .index(ML_MODEL_GROUP_INDEX)

plugin/src/test/java/org/opensearch/ml/helper/ModelAccessControlHelperTests.java

@@ -418,6 +418,49 @@ public void test_CreateSearchSourceBuilder() {
         assertNotNull(modelAccessControlHelper.createSearchSourceBuilder(user));
     }
 
+    public void test_MultiTenancyEnabled_BypassesValidation() {
+        // Test that when multi-tenancy is enabled, validation is bypassed and returns true
+        User user = User.parse("owner|IT,HR|myTenant");
+        when(mlFeatureEnabledSetting.isMultiTenancyEnabled()).thenReturn(true);
+
+        modelAccessControlHelper
+            .validateModelGroupAccess(user, mlFeatureEnabledSetting, null, "testGroupID", client, sdkClient, actionListener);
+
+        ArgumentCaptor<Boolean> argumentCaptor = ArgumentCaptor.forClass(Boolean.class);
+        verify(actionListener).onResponse(argumentCaptor.capture());
+        assertTrue(argumentCaptor.getValue());
+    }
+
+    public void test_MultiTenancyDisabled_ProceedsWithValidation() throws IOException {
+        // Test that when multi-tenancy is disabled, normal validation logic proceeds
+        User user = User.parse("owner|IT,HR|myTenant");
+        when(mlFeatureEnabledSetting.isMultiTenancyEnabled()).thenReturn(false);
+
+        // Setup a public model group to ensure validation passes
+        String owner = "owner|IT,HR|myTenant";
+        List<String> backendRoles = Arrays.asList("IT", "HR");
+        setupModelGroup(owner, AccessMode.PUBLIC.getValue(), backendRoles);
+
+        PlainActionFuture<GetResponse> future = PlainActionFuture.newFuture();
+        future.onResponse(getResponse);
+        when(client.get(any())).thenReturn(future);
+
+        CountDownLatch latch = new CountDownLatch(1);
+        LatchedActionListener<Boolean> latchedActionListener = new LatchedActionListener<>(actionListener, latch);
+        modelAccessControlHelper
+            .validateModelGroupAccess(user, mlFeatureEnabledSetting, null, "testGroupID", client, sdkClient, latchedActionListener);
+
+        try {
+            latch.await(500, TimeUnit.MILLISECONDS);
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+        }
+
+        ArgumentCaptor<Boolean> argumentCaptor = ArgumentCaptor.forClass(Boolean.class);
+        verify(actionListener).onResponse(argumentCaptor.capture());
+        assertTrue(argumentCaptor.getValue());
+    }
+
     private GetResponse modelGroupBuilder(List<String> backendRoles, String access, String owner) throws IOException {
         MLModelGroup mlModelGroup = MLModelGroup
             .builder()