Add backend filtering for legacy internal user and service accounts

src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java

@@ -27,6 +27,7 @@
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.inject.Inject;
 import org.opensearch.common.settings.Settings;
+import org.opensearch.core.common.Strings;
 import org.opensearch.rest.RestChannel;
 import org.opensearch.rest.RestController;
 import org.opensearch.rest.RestRequest;
@@ -44,6 +45,7 @@
 import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
 import org.opensearch.security.ssl.transport.PrincipalExtractor;
 import org.opensearch.security.support.SecurityJsonNode;
+import org.opensearch.security.user.AccountType;
 import org.opensearch.security.user.UserService;
 import org.opensearch.security.user.UserServiceException;
 import org.opensearch.threadpool.ThreadPool;
@@ -52,10 +54,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";
     private static final List<Route> routes = addRoutesPrefix(
         ImmutableList.of(
             new Route(Method.GET, "/user/{name}"),
@@ -115,12 +126,42 @@ protected Endpoint getEndpoint() {
         return Endpoint.INTERNALUSERS;
     }
 
+    @Override
+    protected void handleGet(final RestChannel channel, RestRequest request, Client client, final JsonNode content) throws IOException {
+        final String resourcename = request.param("name");
+
+        SecurityDynamicConfiguration<?> configuration = load(getConfigName(), true);
+        filter(configuration);
+
+        String includeIfType = request.param("filterBy", "any");
+
+        if (includeIfType != "any") {
+            try {
+                userService.includeAccountsIfType(configuration, AccountType.valueOf(includeIfType.toUpperCase()));
+            } catch (UserServiceException ex) {
+                badRequestResponse(channel, ex.getMessage());
+            }
+        }
+
+        // no specific resource requested, return complete config
+        if (Strings.isNullOrEmpty(resourcename)) {
+            successResponse(channel, configuration);
+            return;
+        }
+        if (!configuration.exists(resourcename)) {
+            notFound(channel, "Resource '" + resourcename + "' not found.");
+            return;
+        }
+        configuration.removeOthers(resourcename);
+        successResponse(channel, configuration);
+    }
+
     @Override
     protected void handlePut(RestChannel channel, final RestRequest request, final Client client, final JsonNode content)
         throws IOException {
 
         final String username = request.param("name");
-
+        final boolean service = content.get("service").asBoolean();
         SecurityDynamicConfiguration<?> internalUsersConfiguration = load(getConfigName(), false);
 
         if (!isWriteable(channel, internalUsersConfiguration, username)) {
@@ -145,8 +186,9 @@ protected void handlePut(RestChannel channel, final RestRequest request, final C
         // when updating an existing user password hash can be blank, which means no
         // changes
 
+
         try {
-            if (request.hasParam("service")) {
+            if (service) {
                 ((ObjectNode) content).put("service", request.param("service"));
             }
             if (request.hasParam("enabled")) {
@@ -165,7 +207,7 @@ protected void handlePut(RestChannel channel, final RestRequest request, final C
         if (userExisted && securityJsonNode.get("hash").asString() == null) {
             // sanity check, this should usually not happen
             final String hash = ((Hashed) internalUsersConfiguration.getCEntry(username)).getHash();
-            if (hash == null || hash.length() == 0) {
+            if (Strings.isNullOrEmpty(hash)) {
                 internalErrorResponse(
                     channel,
                     "Existing user " + username + " has no password, and no new password or hash was specified."
@@ -210,6 +252,7 @@ public void onResponse(IndexResponse response) {
      * @param content The content of the request parsed into a node
      * @throws IOException when parsing of configuration files fails (should not happen)
      */
+
     @Override
     protected void handlePost(final RestChannel channel, RestRequest request, Client client, final JsonNode content) throws IOException {
 
@@ -219,7 +262,7 @@ protected void handlePost(final RestChannel channel, RestRequest request, Client
         filter(internalUsersConfiguration); // Hides hashes
 
         // no specific resource requested
-        if (username == null || username.length() == 0) {
+        if (Strings.isNullOrEmpty(username)) {
 
             notImplemented(channel, Method.POST);
             return;
@@ -324,7 +367,8 @@ public Map<String, RequestContentValidator.DataType> allowedKeys() {
                     .put("opendistro_security_roles", DataType.ARRAY)
                     .put("hash", DataType.STRING)
                     .put("password", DataType.STRING)
-                    .build();
+                    .put("service", DataType.STRING)
+                     .build();
             }
         });
     }

src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java

@@ -30,6 +30,7 @@
 import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 
@@ -285,7 +286,11 @@ public SecurityDynamicConfiguration<T> deepClone() {
     @JsonIgnore
     public void remove(String key) {
         centries.remove(key);
+    }
 
+    @JsonIgnore
+    public void remove(List<String> keySet) {
+        keySet.stream().forEach(this::remove);
     }
 
     @SuppressWarnings({ "rawtypes", "unchecked" })

src/main/java/org/opensearch/security/user/AccountType.java

@@ -0,0 +1,26 @@
+package org.opensearch.security.user;
+
+public enum AccountType {
+
+    INTERNAL("internal"),
+    SERVICE("service");
+
+    private String name;
+
+    AccountType(String name){
+        this.name = name;
+    }
+
+    public String getName() {
+        return this.name;
+    }
+    public static AccountType fromString(String name) {
+        for (AccountType b : AccountType.values()) {
+            if (b.name.equalsIgnoreCase(name)) {
+                return b;
+            }
+        }
+        return null;
+    }
+
+}

src/main/java/org/opensearch/security/user/UserService.java

@@ -13,10 +13,12 @@
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.Arrays;
 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.stream.Collectors;
@@ -44,6 +46,7 @@
 import org.opensearch.security.securityconf.Hashed;
 import org.opensearch.security.securityconf.impl.CType;
 import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
+import org.opensearch.security.securityconf.impl.v7.InternalUserV7;
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.security.support.SecurityJsonNode;
 import org.passay.CharacterRule;
@@ -314,4 +317,49 @@ public static void saveAndUpdateConfigs(
             throw ExceptionsHelper.convertToOpenSearchException(e);
         }
     }
+
+    /**
+     * Removes accounts that are not of the requested type from the SecurityDynamicConfiguration object passed.
+     *
+     * Accounts with the 'service' attribute set to true, are considered of type 'service'.
+     * Accounts with the 'service' attribute set to false or without the 'service' attribute, are considered of type 'internal'.
+     *
+     * @param configuration SecurityDynamicConfiguration object containing all accounts
+     * @param requestedAccountType The type of account to be kept. Should be "service" or "internal"
+     *
+     */
+    public void includeAccountsIfType(SecurityDynamicConfiguration<?> configuration, AccountType requestedAccountType)
+        throws UserServiceException {
+        if (requestedAccountType != AccountType.INTERNAL && requestedAccountType != AccountType.SERVICE) {
+            throw new UserServiceException("Invalid user type {} " + requestedAccountType);
+        }
+        List<String> toBeRemoved = new ArrayList<>();
+
+        if (requestedAccountType == AccountType.SERVICE) {
+            for (Map.Entry<String, ?> entry : configuration.getCEntries().entrySet()) {
+                final InternalUserV7 internalUserEntry = (InternalUserV7) entry.getValue();
+                final Map accountAttributes = internalUserEntry.getAttributes();
+                final String accountName = entry.getKey();
+                final boolean isServiceAccount = Boolean.parseBoolean(accountAttributes.getOrDefault("service", "false").toString());
+
+                if (isServiceAccount == false) {
+                    toBeRemoved.add(accountName);
+                }
+            }
+        }
+        else if (requestedAccountType == AccountType.INTERNAL) {
+
+            for (Map.Entry<String, ?> entry : configuration.getCEntries().entrySet()) {
+                final InternalUserV7 internalUserEntry = (InternalUserV7) entry.getValue();
+                final Map accountAttributes = internalUserEntry.getAttributes();
+                final String accountName = entry.getKey();
+                final boolean isServiceAccount = Boolean.parseBoolean(accountAttributes.getOrDefault("service", "false").toString());
+
+                if (isServiceAccount == true) {
+                    toBeRemoved.add(accountName);
+                }
+            }
+        }
+        configuration.remove(toBeRemoved);
+        }
 }

src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java

@@ -15,14 +15,17 @@
 import java.util.Base64;
 import java.util.List;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.http.message.BasicHeader;
+import org.hamcrest.Matchers;
 import org.junit.Assert;
 import org.junit.Test;
 
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.xcontent.XContentType;
+import org.opensearch.security.DefaultObjectMapper;
 import org.opensearch.security.dlic.rest.validation.PasswordValidator;
 import org.opensearch.security.dlic.rest.validation.RequestContentValidator;
 import org.opensearch.security.securityconf.impl.CType;
@@ -55,7 +58,8 @@ protected String getEndpointPrefix() {
 
     private static final String ENABLED_SERVICE_ACCOUNT_BODY = "{"
         + " \"attributes\": { \"service\": \"true\", "
-        + "\"enabled\": \"true\"}"
+        + " \"enabled   \": \"true\"},"
+        + " \"service\": \"true\" "
         + " }\n";
 
     private static final String DISABLED_SERVICE_ACCOUNT_BODY = "{"
@@ -144,6 +148,57 @@ public void testParallelPutRequests() throws Exception {
         deleteUser("test1");
     }
 
+    @Test
+    public void testUserFilters() throws Exception {
+        setup();
+        rh.keystore = "restapi/kirk-keystore.jks";
+        rh.sendAdminCertificate = true;
+        final int SERVICE_ACCOUNTS_IN_SETTINGS = 1;
+        final int INTERNAL_ACCOUNTS_IN_SETTINGS = 19;
+        final String serviceAccountName = "JohnDoeService";
+        HttpResponse response;
+
+
+
+        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?filterBy=service");
+        Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
+        list = DefaultObjectMapper.readTree(response.getBody());
+        assertThat(list, Matchers.emptyIterable());
+
+        response = rh.executePutRequest(ENDPOINT + "/internalusers/" + serviceAccountName, ENABLED_SERVICE_ACCOUNT_BODY);
+
+
+        // repeat assertions after adding the service account
+
+        response = rh.executeGetRequest(ENDPOINT + "/internalusers?filterBy=internal");
+
+        Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
+        list = DefaultObjectMapper.readTree(response.getBody());
+        Assert.assertEquals(INTERNAL_ACCOUNTS_IN_SETTINGS, list.size());
+
+        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());
+        assertThat(response.findValueInJson(serviceAccountName), Matchers.notNullValue());
+        assertThat(response.findValueInJson(serviceAccountName + ".attributes.service"), containsString("true"));
+
+        response = rh.executeGetRequest(ENDPOINT + "/internalusers?filterBy=ssas");
+        Assert.assertEquals(response.getBody(), HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
+        response = rh.executeGetRequest(ENDPOINT + "/internalusers?wrongparameter=jhondoe");
+        Assert.assertEquals(response.getBody(), HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
+        response = rh.executePutRequest(ENDPOINT + "/internalusers", "{sample:value");
+        Assert.assertEquals(response.getBody(), HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode());
+    }
+
     @Test
     public void testUserApi() throws Exception {
 

src/test/java/org/opensearch/security/test/helper/rest/RestHelper.java

@@ -527,7 +527,7 @@ private void throwIfNotValueNode(final JsonNode node) {
             }
         }
 
-        private JsonNode findObjectInJson(final String jsonDotPath) {
+        public JsonNode findObjectInJson(final String jsonDotPath) {
             // Make sure its json / then parse it
             if (!isJsonContentType()) {
                 throw new RuntimeException("Response was expected to be JSON, body was: \n" + body);