Merge pull request #337 from sujithvm/opendistro-0.10

Security 0.10.1.2 changes
ViaQ · Apr 3, 2020 · 1cfe046 · 1cfe046
2 parents 4b4eabe + 0a4ee27
commit 1cfe046
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 23 deletions.
diff --git a/build.gradle b/build.gradle
@@ -14,7 +14,7 @@ ext {
 group = "com.amazon.opendistroforelasticsearch"
 // Increment the final digit when there's a new plugin versions for the same opendistro version
 // Reset the final digit to 0 when upgrading to a new opendistro version
-version = "${opendistroVersion}.1" + (isSnapshot ? "-SNAPSHOT" : "")
+version = "${opendistroVersion}.2" + (isSnapshot ? "-SNAPSHOT" : "")
 
 
 if (!project.hasProperty("archivePath")) {

diff --git a/opendistro-elasticsearch-security.release-notes b/opendistro-elasticsearch-security.release-notes
@@ -1,4 +1,10 @@
-## 2020-03-05, Version 0.10.1.1 (Current)
+## 2020-04-02, Version 0.10.1.2 (Current)
+
+- Fixed bug which caused user to lose roles on account password update
+- Performance improvement by memorizing results of resolveIndexPatterns for Bulk requests
+- Performance improvement by implementing faster version of implies type perm
+
+## 2020-03-05, Version 0.10.1.1
 
 - Adding capability to hot reload ssl certificates
 - Added changes for SuperAdmin to update/add/delete reserved configs

diff --git a/plugin-descriptor.properties b/plugin-descriptor.properties
@@ -3,7 +3,7 @@
 description=Provide access control related features for Elasticsearch 6
 #
 # 'version': plugin's version
-version=0.10.1.1
+version=0.10.1.2
 #
 # 'name': the plugin name
 name=opendistro_security

diff --git a/pom.xml b/pom.xml
@@ -35,7 +35,7 @@
     <groupId>com.amazon.opendistroforelasticsearch</groupId>
     <artifactId>opendistro_security</artifactId>
     <packaging>jar</packaging>
-    <version>0.10.1.1</version>
+    <version>0.10.1.2</version>
     <name>Open Distro Security for Elasticsearch</name>
     <description>Open Distro For Elasticsearch Security</description>
     <url>https://github.com/opendistro-for-elasticsearch/security</url>
@@ -52,7 +52,7 @@
         <url>https://github.com/opendistro-for-elasticsearch/security</url>
         <connection>scm:git:[email protected]:opendistro-for-elasticsearch/security.git</connection>
         <developerConnection>scm:git:[email protected]:opendistro-for-elasticsearch/security.git</developerConnection>
-        <tag>0.10.1.1</tag>
+        <tag>0.10.1.2</tag>
     </scm>
 
     <developers>

diff --git a/...n/java/com/amazon/opendistroforelasticsearch/security/dlic/rest/api/AccountApiAction.java b/...n/java/com/amazon/opendistroforelasticsearch/security/dlic/rest/api/AccountApiAction.java
@@ -32,6 +32,7 @@
 import org.elasticsearch.action.index.IndexResponse;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.settings.Settings;
@@ -200,32 +201,29 @@ protected void handlePut(RestChannel channel, final RestRequest request, final C
             return;
         }
 
-        final Settings settings = indexBaseConfigurationRepository.getConfiguration(ConfigConstants.CONFIGNAME_INTERNAL_USERS);
         final String currentPassword = additionalSettingsBuilder.get("current_password");
-        final String hash = settings.get(username + ".hash");
-        if (hash == null || !OpenBSDBCrypt.checkPassword(hash, currentPassword.toCharArray())) {
+        @SuppressWarnings("unchecked")
+        Map<String, String> internalUserEntry = (Map<String, String>) config.get(username);
+        final String currentHash = internalUserEntry.get("hash");
+        if (currentHash == null || !OpenBSDBCrypt.checkPassword(currentHash, currentPassword.toCharArray())) {
             badRequestResponse(channel, "Could not validate your current password.");
             return;
         }
 
-        additionalSettingsBuilder.remove("current_password");
-
         // if password is set, it takes precedence over hash
-        final String plainTextPassword = additionalSettingsBuilder.get("password");
-        final String origHash = additionalSettingsBuilder.get("hash");
-        if (plainTextPassword != null && plainTextPassword.length() > 0) {
-            additionalSettingsBuilder.remove("password");
-            additionalSettingsBuilder.put("hash", hash(plainTextPassword.toCharArray()));
-        } else if (origHash != null && origHash.length() > 0) {
-            additionalSettingsBuilder.remove("password");
-        } else if (plainTextPassword != null && plainTextPassword.isEmpty() && origHash == null) {
-            additionalSettingsBuilder.remove("password");
+        final String password = additionalSettingsBuilder.get("password");
+        final String hash;
+        if (Strings.isNullOrEmpty(password)) {
+            hash = additionalSettingsBuilder.get("hash");
+        } else {
+            hash = hash(password.toCharArray());
+        }
+        if (Strings.isNullOrEmpty(hash)) {
+            badRequestResponse(channel, "Both provided password and hash cannot be null/empty.");
+            return;
         }
 
-        config.remove(username);
-
-        // checks complete, create or update the user
-        config.put(username, Utils.convertJsonToxToStructuredMap(additionalSettingsBuilder.build()));
+        internalUserEntry.put("hash", hash);
 
         saveAnUpdateConfigs(client, request, ConfigConstants.CONFIGNAME_INTERNAL_USERS, Utils.convertStructuredMapToBytes(config), new OnSucessActionListener<IndexResponse>(channel) {
             @Override

diff --git a/...est/java/com/amazon/opendistroforelasticsearch/security/dlic/rest/api/AccountApiTest.java b/...est/java/com/amazon/opendistroforelasticsearch/security/dlic/rest/api/AccountApiTest.java
@@ -94,6 +94,26 @@ public void testPutAccount() throws Exception {
         response = rh.executePutRequest(ENDPOINT, payload, encodeBasicHeader(testUser, testPass));
         assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
 
+        // test - bad request as hash/password is missing
+        payload = "{\"current_password\":\"" + testPass + "\"}";
+        response = rh.executePutRequest(ENDPOINT, payload, encodeBasicHeader(testUser, testPass));
+        assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
+        // test - bad request as password is empty
+        payload = "{\"password\":\"" + "" + "\", \"current_password\":\"" + testPass + "\"}";
+        response = rh.executePutRequest(ENDPOINT, payload, encodeBasicHeader(testUser, testPass));
+        assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
+        // test - bad request as hash is empty
+        payload = "{\"hash\":\"" + "" + "\", \"current_password\":\"" + testPass + "\"}";
+        response = rh.executePutRequest(ENDPOINT, payload, encodeBasicHeader(testUser, testPass));
+        assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
+        // test - bad request as hash and password are empty
+        payload = "{\"hash\": \"\", \"password\": \"\", \"current_password\":\"" + testPass + "\"}";
+        response = rh.executePutRequest(ENDPOINT, payload, encodeBasicHeader(testUser, testPass));
+        assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
         // test - bad request as invalid parameters are present
         payload = "{\"password\":\"new-pass\", \"current_password\":\"" + testPass + "\", \"backend_roles\": []}";
         response = rh.executePutRequest(ENDPOINT, payload, encodeBasicHeader(testUser, testPass));
@@ -152,4 +172,43 @@ public void testPutAccount() throws Exception {
         response = rh.executePutRequest(ENDPOINT, payload, encodeBasicHeader("admin", "admin"));
         assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode());
     }
+
+    @Test
+    public void testPutAccountRetainsAccountInformation() throws Exception {
+        // arrange
+        setup();
+        final String testUsername = "test";
+        final String testPassword = "test-password";
+        final String newPassword = "new-password";
+        final String createInternalUserPayload = "{\n" +
+                "  \"password\": \"" + testPassword + "\",\n" +
+                "  \"roles\": [\"test-backend-role-1\"],\n" +
+                "  \"attributes\": {\n" +
+                "    \"attribute1\": \"value1\"\n" +
+                "  }\n" +
+                "}";
+        final String changePasswordPayload = "{\"password\":\"" + newPassword + "\", \"current_password\":\"" + testPassword + "\"}";
+        final String internalUserEndpoint = "/_opendistro/_security/api/internalusers/" + testUsername;
+
+        // create user
+        rh.sendAdminCertificate = true;
+        HttpResponse response = rh.executePutRequest(internalUserEndpoint, createInternalUserPayload);
+        assertEquals(HttpStatus.SC_OK, response.getStatusCode());
+        rh.sendAdminCertificate = false;
+
+        // change password to new-password
+        response = rh.executePutRequest(ENDPOINT, changePasswordPayload, encodeBasicHeader(testUsername, testPassword));
+        assertEquals(HttpStatus.SC_OK, response.getStatusCode());
+
+        // assert account information has not changed
+        rh.sendAdminCertificate = true;
+        response = rh.executeGetRequest(internalUserEndpoint);
+        assertEquals(HttpStatus.SC_OK, response.getStatusCode());
+        Settings responseBody = Settings.builder()
+                .loadFromSource(response.getBody(), XContentType.JSON)
+                .build()
+                .getAsSettings(testUsername);
+        assertTrue(responseBody.getAsList("roles").contains("test-backend-role-1"));
+        assertEquals(responseBody.getAsSettings("attributes").get("attribute1"), "value1");
+    }
 }