aggregating new filters to existing endpoint and filtering by request…

… query param. Signed-off-by: Sam <samuel.costa@eliatra.com>
samuelcostae · Aug 9, 2023 · 7bca971 · 7bca971
1 parent 448cdf6
commit 7bca971
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 32 deletions.
diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java
@@ -52,15 +52,19 @@
 import static org.opensearch.security.dlic.rest.support.Utils.hash;
 
 public class InternalUsersApiAction extends PatchableResourceApiAction {
+
+    @Override
+    protected void consumeParameters(final RestRequest request) {
+        request.param("name");
+        request.param("filterBy");
+    }
+
     static final List<String> RESTRICTED_FROM_USERNAME = ImmutableList.of(
         ":" // Not allowed in basic auth, see https://stackoverflow.com/a/33391003/533057
     );
 
     public static final String LEGACY_OPENDISTRO_PREFIX = "_opendistro/_security";
     public static final String PLUGINS_PREFIX = "_plugins/_security";
-    public static final String SERVICE_ACCOUNTS_ENDPOINT = "/internalusers/serviceaccounts";
-    public static final String INTERNAL_ACCOUNTS_ENDPOINT = "/internalusers/internalaccounts";
-
     private static final List<Route> routes = addRoutesPrefix(
         ImmutableList.of(
             new Route(Method.GET, "/user/{name}"),
@@ -72,8 +76,6 @@ public class InternalUsersApiAction extends PatchableResourceApiAction {
             // corrected mapping, introduced in OpenSearch Security
             new Route(Method.GET, "/internalusers/{name}"),
             new Route(Method.GET, "/internalusers/"),
-            new Route(Method.GET, SERVICE_ACCOUNTS_ENDPOINT),
-            new Route(Method.GET, INTERNAL_ACCOUNTS_ENDPOINT),
             new Route(Method.POST, "/internalusers/{name}/authtoken"),
             new Route(Method.DELETE, "/internalusers/{name}"),
             new Route(Method.PUT, "/internalusers/{name}"),
@@ -129,18 +131,12 @@ protected void handleGet(final RestChannel channel, RestRequest request, Client
         SecurityDynamicConfiguration<?> configuration = load(getConfigName(), true);
         filter(configuration);
 
-        String requestDestination = request.rawPath().split("/api")[1];
+        String filterBy = request.param("filterBy", "all");
 
-        if (requestDestination.equalsIgnoreCase(INTERNAL_ACCOUNTS_ENDPOINT)) {
-            userService.removeNonInternalAccounts(configuration);
-            successResponse(channel, configuration);
-            return;
-        }
-        if (requestDestination.equalsIgnoreCase(SERVICE_ACCOUNTS_ENDPOINT)) {
-            userService.removeNonServiceAccounts(configuration);
-            successResponse(channel, configuration);
-            return;
+        if (filterBy != "internal" && filterBy != "service") {
+            userService.filterAccountsByType(configuration, filterBy);
         }
+
         // no specific resource requested, return complete config
         if (resourcename == null || resourcename.length() == 0) {
             successResponse(channel, configuration);
@@ -152,7 +148,6 @@ protected void handleGet(final RestChannel channel, RestRequest request, Client
         }
         configuration.removeOthers(resourcename);
         successResponse(channel, configuration);
-
     }
 
     @Override

diff --git a/src/main/java/org/opensearch/security/user/UserService.java b/src/main/java/org/opensearch/security/user/UserService.java
@@ -13,12 +13,7 @@
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
-import java.util.Base64;
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.Random;
+import java.util.*;
 import java.util.stream.Collectors;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
@@ -316,17 +311,23 @@ public static void saveAndUpdateConfigs(
         }
     }
 
-public void filterAccountsByType(SecurityDynamicConfiguration<?> configuration, boolean isServiceAccount) {
-        List<String> filteredAccounts = new ArrayList<>();
+    public void filterAccountsByType(SecurityDynamicConfiguration<?> configuration, String requestedAccountType) {
+        List<String> toBeRemoved = new ArrayList<>();
+
         for (Map.Entry<String, ?> entry : configuration.getCEntries().entrySet()) {
             final InternalUserV7 internalUserEntry = (InternalUserV7) entry.getValue();
             final Map accountAttributes = internalUserEntry.getAttributes();
             final String accountName = entry.getKey();
-            if (accountAttributes.getOrDefault("service", "false").equals(isServiceAccount)) {
-                filteredAccounts.add(accountName);
+            boolean isServiceAccount = Boolean.parseBoolean(accountAttributes.getOrDefault("service", "false").toString());
+
+            if (requestedAccountType.equalsIgnoreCase("internal") && isServiceAccount) {
+                toBeRemoved.add(accountName);
+            } else if (requestedAccountType.equalsIgnoreCase("service") && isServiceAccount == false) {
+                toBeRemoved.add(accountName);
             }
+
         }
-        configuration.remove(filteredAccounts);
-}
+        configuration.remove(toBeRemoved);
+    }
 
 }
diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java
@@ -159,21 +159,26 @@ public void testUserFilters() throws Exception {
         final int SERVICE_ACCOUNTS_IN_SETTINGS = 1;
         final int INTERNAL_ACCOUNTS_IN_SETTINGS = 19;
 
-        response = rh.executeGetRequest(ENDPOINT + "/internalusers/internalaccounts");
+        response = rh.executeGetRequest(ENDPOINT + "/internalusers?filterBy=internal");
 
         Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
         JsonNode list = DefaultObjectMapper.readTree(response.getBody());
         Assert.assertEquals(INTERNAL_ACCOUNTS_IN_SETTINGS, list.size());
 
-        response = rh.executeGetRequest(ENDPOINT + "/internalusers/serviceaccounts");
+        response = rh.executeGetRequest(ENDPOINT + "/internalusers?filterBy=service");
         Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
         list = DefaultObjectMapper.readTree(response.getBody());
         Assert.assertEquals(SERVICE_ACCOUNTS_IN_SETTINGS, list.size());
 
-        response = rh.executeGetRequest(ENDPOINT + "/internalusers/serviceaccounts?wrongparameter=jhondoe");
+        response = rh.executeGetRequest(ENDPOINT + "/internalusers?filterBy=ssas");
+        Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
+        list = DefaultObjectMapper.readTree(response.getBody());
+        Assert.assertEquals(SERVICE_ACCOUNTS_IN_SETTINGS + INTERNAL_ACCOUNTS_IN_SETTINGS, list.size());
+
+        response = rh.executeGetRequest(ENDPOINT + "/internalusers?wrongparameter=jhondoe");
         Assert.assertEquals(response.getBody(), HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
 
-        response = rh.executePutRequest(ENDPOINT + "/internalusers/serviceaccounts", "{sample:value");
+        response = rh.executePutRequest(ENDPOINT + "/internalusers", "{sample:value");
         Assert.assertEquals(response.getBody(), HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode());
     }