Security User Refactor

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

@@ -175,6 +175,7 @@
 import org.opensearch.security.transport.InterClusterRequestEvaluator;
 import org.opensearch.security.transport.SecurityInterceptor;
 import org.opensearch.security.user.User;
+import org.opensearch.security.user.UserService;
 import org.opensearch.tasks.Task;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.RemoteClusterService;
@@ -203,6 +204,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
     private volatile SecurityRestFilter securityRestHandler;
     private volatile SecurityInterceptor si;
     private volatile PrivilegesEvaluator evaluator;
+    private volatile UserService userService;
     private volatile ThreadPool threadPool;
     private volatile ConfigurationRepository cr;
     private volatile AdminDNs adminDns;
@@ -486,6 +488,7 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
                                 evaluator,
                                 threadPool,
                                 Objects.requireNonNull(auditLog), sks,
+                                Objects.requireNonNull(userService),
                                 sslCertReloadEnabled)
                 );
                 log.debug("Added {} rest handler(s)", handlers.size());
@@ -811,9 +814,11 @@ public Collection<Object> createComponents(Client localClient, ClusterService cl
         sslExceptionHandler = new AuditLogSslExceptionHandler(auditLog);
 
         adminDns = new AdminDNs(settings);
-        
+
         cr = ConfigurationRepository.create(settings, this.configPath, threadPool, localClient, clusterService, auditLog);
 
+        userService = new UserService(cs, cr, settings, localClient);
+
         final XFFResolver xffResolver = new XFFResolver(threadPool);
         backendRegistry = new BackendRegistry(settings, adminDns, xffResolver, auditLog, threadPool);
 
@@ -870,6 +875,7 @@ public Collection<Object> createComponents(Client localClient, ClusterService cl
         components.add(evaluator);
         components.add(si);
         components.add(dcf);
+        components.add(userService);
 
 
         return components;
@@ -1177,7 +1183,6 @@ public static class GuiceHolder implements LifecycleComponent {
         private static RepositoriesService repositoriesService;
         private static RemoteClusterService remoteClusterService;
         private static IndicesService indicesService;
-
         private static PitService pitService;
 
         @Inject
@@ -1202,7 +1207,8 @@ public static IndicesService getIndicesService() {
         }
 
         public static PitService getPitService() { return pitService; }
-
+
+
         @Override
         public void close() {
         }

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

@@ -14,7 +14,6 @@
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.List;
-import java.util.stream.Collectors;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
@@ -32,18 +31,18 @@
 import org.opensearch.rest.RestController;
 import org.opensearch.rest.RestRequest;
 import org.opensearch.rest.RestRequest.Method;
-import org.opensearch.security.DefaultObjectMapper;
 import org.opensearch.security.auditlog.AuditLog;
 import org.opensearch.security.configuration.AdminDNs;
 import org.opensearch.security.configuration.ConfigurationRepository;
 import org.opensearch.security.dlic.rest.validation.AbstractConfigurationValidator;
 import org.opensearch.security.dlic.rest.validation.InternalUsersValidator;
 import org.opensearch.security.privileges.PrivilegesEvaluator;
-import org.opensearch.security.securityconf.Hashed;
 import org.opensearch.security.securityconf.impl.CType;
 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.UserService;
+import org.opensearch.security.user.UserServiceException;
 import org.opensearch.threadpool.ThreadPool;
 
 import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix;
@@ -69,13 +68,16 @@ public class InternalUsersApiAction extends PatchableResourceApiAction {
             new Route(Method.PATCH, "/internalusers/{name}")
     ));
 
+    UserService userService;
+
     @Inject
     public InternalUsersApiAction(final Settings settings, final Path configPath, final RestController controller,
                                   final Client client, final AdminDNs adminDNs, final ConfigurationRepository cl,
                                   final ClusterService cs, final PrincipalExtractor principalExtractor, final PrivilegesEvaluator evaluator,
-                                  ThreadPool threadPool, AuditLog auditLog) {
+                                  ThreadPool threadPool, UserService userService, AuditLog auditLog) {
         super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool,
                 auditLog);
+        this.userService = userService;
     }
 
     @Override
@@ -100,22 +102,7 @@ protected void handlePut(RestChannel channel, final RestRequest request, final C
 
         final String username = request.param("name");
 
-        if (username == null || username.length() == 0) {
-            badRequestResponse(channel, "No " + getResourceName() + " specified.");
-            return;
-        }
-
-        final List<String> foundRestrictedContents = RESTRICTED_FROM_USERNAME.stream().filter(username::contains).collect(Collectors.toList());
-        if (!foundRestrictedContents.isEmpty()) {
-            final String restrictedContents = foundRestrictedContents.stream().map(s -> "'" + s + "'").collect(Collectors.joining(","));
-            badRequestResponse(channel, "Username has restricted characters " + restrictedContents + " that are not permitted.");
-            return;
-        }
-
-        // TODO it might be sensible to consolidate this with the overridden method in
-        // order to minimize duplicated logic
-
-        final SecurityDynamicConfiguration<?> internalUsersConfiguration = load(getConfigName(), false);
+        SecurityDynamicConfiguration<?> internalUsersConfiguration = load(getConfigName(), false);
 
         if (!isWriteable(channel, internalUsersConfiguration, username)) {
             return;
@@ -128,50 +115,35 @@ protected void handlePut(RestChannel channel, final RestRequest request, final C
         final List<String> securityRoles = securityJsonNode.get("opendistro_security_roles").asList();
         if (securityRoles != null) {
             for (final String role: securityRoles) {
-                if (!isValidRolesMapping(channel, role)) return;
+                if (!isValidRolesMapping(channel, role)) {
+                    return;
+                }
             }
         }
 
-        // if password is set, it takes precedence over hash
-        final String plainTextPassword = securityJsonNode.get("password").asString();
-        final String origHash = securityJsonNode.get("hash").asString();
-        if (plainTextPassword != null && plainTextPassword.length() > 0) {
-            contentAsNode.remove("password");
-            contentAsNode.put("hash", hash(plainTextPassword.toCharArray()));
-        } else if (origHash != null && origHash.length() > 0) {
-            contentAsNode.remove("password");
-        } else if (plainTextPassword != null && plainTextPassword.isEmpty() && origHash == null) {
-            contentAsNode.remove("password");
-        }
-
         final boolean userExisted = internalUsersConfiguration.exists(username);
 
         // when updating an existing user password hash can be blank, which means no
         // changes
 
-        // sanity checks, hash is mandatory for newly created users
-        if (!userExisted && securityJsonNode.get("hash").asString() == null) {
-            badRequestResponse(channel, "Please specify either 'hash' or 'password' when creating a new internal user.");
+        try {
+            if (request.hasParam("owner")) {
+                ((ObjectNode) content).put("owner", request.param("owner"));
+            }
+            if (request.hasParam("isEnabled")) {
+                ((ObjectNode) content).put("isEnabled", request.param("isEnabled"));
+            }
+            ((ObjectNode) content).put("name", username);
+            internalUsersConfiguration = userService.createOrUpdateAccount((ObjectNode) content);
+        }
+        catch (UserServiceException ex) {
+            badRequestResponse(channel, ex.getMessage());
             return;
         }
-
-        // for existing users, hash is optional
-        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) {
-                internalErrorResponse(channel,
-                    "Existing user " + username + " has no password, and no new password or hash was specified.");
-                return;
-            }
-            contentAsNode.put("hash", hash);
+        catch (IOException ex) {
+            throw new IOException(ex);
         }
 
-        internalUsersConfiguration.remove(username);
-
-        // checks complete, create or update the user
-        internalUsersConfiguration.putCObject(username, DefaultObjectMapper.readTree(contentAsNode,  internalUsersConfiguration.getImplementingClass()));
-
         saveAndUpdateConfigs(this.securityIndexName,client, CType.INTERNALUSERS, internalUsersConfiguration, new OnSucessActionListener<IndexResponse>(channel) {
 
             @Override

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

@@ -28,6 +28,7 @@
 import org.opensearch.security.privileges.PrivilegesEvaluator;
 import org.opensearch.security.ssl.SecurityKeyStore;
 import org.opensearch.security.ssl.transport.PrincipalExtractor;
+import org.opensearch.security.user.UserService;
 import org.opensearch.threadpool.ThreadPool;
 
 public class SecurityRestApiActions {
@@ -44,9 +45,10 @@ public static Collection<RestHandler> getHandler(final Settings settings,
                                                      final ThreadPool threadPool,
                                                      final AuditLog auditLog,
                                                      final SecurityKeyStore securityKeyStore,
+                                                     final UserService userService,
                                                      final boolean certificatesReloadEnabled) {
         final List<RestHandler> handlers = new ArrayList<RestHandler>(16);
-        handlers.add(new InternalUsersApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog));
+        handlers.add(new InternalUsersApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, userService, auditLog));
         handlers.add(new RolesMappingApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog));
         handlers.add(new RolesApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog));
         handlers.add(new ActionGroupsApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog));