Security User Refactor (opensearch-project#2594)

--------- Signed-off-by: Stephen Crawford <steecraw@amazon.com> Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
stephen-crawford · Apr 21, 2023 · 7c8fea0 · 7c8fea0
1 parent 130e410
commit 7c8fea0
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 25 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
@@ -31,14 +31,12 @@
 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;
@@ -146,26 +144,9 @@ protected void handlePut(RestChannel channel, final RestRequest request, final C
         }
         catch (IOException ex) {
             throw new IOException(ex);
-        }
 
-        // 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);
         }
 
-        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) {
 
 

diff --git a/src/main/java/org/opensearch/security/user/UserService.java b/src/main/java/org/opensearch/security/user/UserService.java
@@ -57,7 +57,6 @@ public class UserService {
     String securityIndex;
     Client client;
 
-    User tokenUser;
     final static String NO_PASSWORD_OR_HASH_MESSAGE = "Please specify either 'hash' or 'password' when creating a new internal user.";
     final static String RESTRICTED_CHARACTER_USE_MESSAGE = "A restricted character(s) was detected in the account name. Please remove: ";
 
@@ -70,6 +69,7 @@ public class UserService {
     final static String FAILED_ACCOUNT_RETRIEVAL_MESSAGE = "The account specified could not be accessed at this time.";
     final static String AUTH_TOKEN_GENERATION_MESSAGE = "An auth token could not be generated for the specified account.";
     private static CType getUserConfigName() {
+
         return CType.INTERNALUSERS;
     }
 
@@ -115,13 +115,16 @@ public SecurityDynamicConfiguration<?> createOrUpdateAccount(ObjectNode contentA
         SecurityJsonNode securityJsonNode = new SecurityJsonNode(contentAsNode);
 
         final SecurityDynamicConfiguration<?> internalUsersConfiguration = load(getUserConfigName(), false);
+
         String accountName = securityJsonNode.get("name").asString();
 
         if (accountName == null || accountName.length() == 0) { // Fail if field is present but empty
             throw new UserServiceException(NO_ACCOUNT_NAME_MESSAGE);
         }
 
-        if (!securityJsonNode.get("attributes").get("owner").isNull() && !securityJsonNode.get("attributes").get("owner").asString().equals(accountName)) { // If this is a service account
+
+        if (!securityJsonNode.get("attributes").get("owner").isNull() && !securityJsonNode.get("attributes").get("owner").equals(accountName)) { // If this is a service account
+
             verifyServiceAccount(securityJsonNode, accountName);
             String password = generatePassword();
             contentAsNode.put("hash", hash(password.toCharArray()));
@@ -172,7 +175,6 @@ public SecurityDynamicConfiguration<?> createOrUpdateAccount(ObjectNode contentA
 
     private void verifyServiceAccount(SecurityJsonNode securityJsonNode, String accountName) {
 
-
         final String plainTextPassword = securityJsonNode.get("password").asString();
         final String origHash = securityJsonNode.get("hash").asString();
 

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
@@ -328,9 +328,6 @@ private void verifyPatch(final boolean sendAdminCert, Header... restAdminHeader)
         response = rh.executePutRequest(ENDPOINT + "/internalusers/happyServiceLive",
                 ENABLED_SERVICE_ACCOUNT_BODY, restAdminHeader);
         Assert.assertEquals(response.getBody(), HttpStatus.SC_CREATED, response.getStatusCode());
-        rh.sendAdminCertificate = sendAdminCert;
-        response = rh.executeGetRequest(ENDPOINT + "/internalusers/happyServiceLive", restAdminHeader);
-        Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode());
 
 
         // Add disabled service account
@@ -339,6 +336,11 @@ private void verifyPatch(final boolean sendAdminCert, Header... restAdminHeader)
         Assert.assertEquals(response.getBody(), HttpStatus.SC_CREATED, response.getStatusCode());
 
 
+        // Add enabled non-service account
+        response = rh.executePutRequest(ENDPOINT + "/internalusers/user_is_owner_1",
+                ENABLED_NOT_SERVICE_ACCOUNT_BODY, restAdminHeader);
+        Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode());
+
         // Add service account with password -- Should Fail
         response = rh.executePutRequest(ENDPOINT + "/internalusers/passwordService",
                 PASSWORD_SERVICE, restAdminHeader);