[Security] Fix concrete security index name (elastic/x-pack-elasticsearch#2905)

The 5.6 Upgrade API will reindex .security to .security-6 and create a .security alias.
But the 6.0 default was to create a .security-v6 index and a .security alias if none existed (e.g. fresh x-pack install)

Having two different index names based on the method of install/upgrade complicates the code and testing, so we're unifying on the .security-6 index name that already exists in the wild.

Original commit: elastic/x-pack-elasticsearch@d78f569c5f

plugin/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java

@@ -46,7 +46,7 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust
 
     public static final String SECURITY_INDEX_NAME = ".security";
     public static final String SECURITY_TEMPLATE_NAME = "security-index-template";
-    public static final String NEW_SECURITY_INDEX_NAME = SECURITY_INDEX_NAME + "-" + IndexLifecycleManager.NEW_INDEX_VERSION;
+    public static final String INTERNAL_SECURITY_INDEX = IndexLifecycleManager.INTERNAL_SECURITY_INDEX;
 
     private static final Version MIN_READ_VERSION = Version.V_5_0_0;
 
@@ -188,7 +188,7 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust
     }
 
     public static List<String> indexNames() {
-        return Collections.unmodifiableList(Arrays.asList(SECURITY_INDEX_NAME, NEW_SECURITY_INDEX_NAME));
+        return Collections.unmodifiableList(Arrays.asList(SECURITY_INDEX_NAME, INTERNAL_SECURITY_INDEX));
     }
 
     /**

plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

@@ -292,8 +292,7 @@ public class AuthorizationService extends AbstractComponent {
         IndicesAccessControl indicesAccessControl = permission.authorize(action, localIndices, metaData, fieldPermissionsCache);
         if (!indicesAccessControl.isGranted()) {
             throw denial(authentication, action, request);
-        } else if (indicesAccessControl.getIndexPermissions(SecurityLifecycleService.SECURITY_INDEX_NAME) != null
+        } else if (hasSecurityIndexAccess(indicesAccessControl)
-                && indicesAccessControl.getIndexPermissions(SecurityLifecycleService.SECURITY_INDEX_NAME).isGranted()
                 && MONITOR_INDEX_PREDICATE.test(action) == false
                 && isSuperuser(authentication.getUser()) == false) {
             // only the XPackUser is allowed to work with this index, but we should allow indices monitoring actions through for debugging
@@ -334,6 +333,16 @@ public class AuthorizationService extends AbstractComponent {
         grant(authentication, action, originalRequest);
     }
 
+    private boolean hasSecurityIndexAccess(IndicesAccessControl indicesAccessControl) {
+        for (String index : SecurityLifecycleService.indexNames()) {
+            final IndicesAccessControl.IndexAccessControl indexPermissions = indicesAccessControl.getIndexPermissions(index);
+            if (indexPermissions != null && indexPermissions.isGranted()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     /**
      * Performs authorization checks on the items within a {@link BulkShardRequest}.
      * This inspects the {@link BulkItemRequest items} within the request, computes an <em>implied</em> action for each item's

plugin/src/main/java/org/elasticsearch/xpack/security/support/IndexLifecycleManager.java

@@ -37,7 +37,6 @@ import org.elasticsearch.cluster.routing.IndexRoutingTable;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.mapper.MapperService;
-import org.elasticsearch.xpack.security.InternalClient;
 import org.elasticsearch.xpack.security.InternalSecurityClient;
 import org.elasticsearch.xpack.template.TemplateUtils;
 import org.elasticsearch.xpack.upgrade.IndexUpgradeCheck;
@@ -50,12 +49,11 @@ import static org.elasticsearch.xpack.security.SecurityLifecycleService.SECURITY
  */
 public class IndexLifecycleManager extends AbstractComponent {
 
-    public static final String INTERNAL_SECURITY_INDEX = ".security-v6";
+    public static final String INTERNAL_SECURITY_INDEX = ".security-" + IndexUpgradeCheck.UPRADE_VERSION;
     public static final int INTERNAL_INDEX_FORMAT = 6;
     public static final String SECURITY_VERSION_STRING = "security-version";
     public static final String TEMPLATE_VERSION_PATTERN =
             Pattern.quote("${security.template.version}");
-    public static int NEW_INDEX_VERSION = IndexUpgradeCheck.UPRADE_VERSION;
 
     private final String indexName;
     private final String templateName;

plugin/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java

@@ -504,14 +504,15 @@ public abstract class SecurityIntegTestCase extends ESIntegTestCase {
     }
 
     protected void deleteSecurityIndex() {
+        final InternalSecurityClient securityClient = internalSecurityClient();
         GetIndexRequest getIndexRequest = new GetIndexRequest();
         getIndexRequest.indices(SECURITY_INDEX_NAME);
         getIndexRequest.indicesOptions(IndicesOptions.lenientExpandOpen());
-        GetIndexResponse getIndexResponse = internalClient().admin().indices().getIndex(getIndexRequest).actionGet();
+        GetIndexResponse getIndexResponse = securityClient.admin().indices().getIndex(getIndexRequest).actionGet();
         if (getIndexResponse.getIndices().length > 0) {
             // this is a hack to clean up the .security index since only the XPack user can delete it
             DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(getIndexResponse.getIndices());
-            internalClient().admin().indices().delete(deleteIndexRequest).actionGet();
+            securityClient.admin().indices().delete(deleteIndexRequest).actionGet();
         }
     }
 

plugin/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java

@@ -196,6 +196,7 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase {
             final boolean done = awaitBusy(() -> tokenService.isExpirationInProgress() == false);
             assertTrue(done);
         }
+        super.deleteSecurityIndex();
     }
 
     public void testMetadataIsNotSentToClient() {

plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java

@@ -5,7 +5,6 @@
  */
 package org.elasticsearch.xpack.security.authz;
 
-import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -130,8 +129,6 @@ import org.elasticsearch.xpack.security.user.ElasticUser;
 import org.elasticsearch.xpack.security.user.SystemUser;
 import org.elasticsearch.xpack.security.user.User;
 import org.elasticsearch.xpack.security.user.XPackUser;
-import org.joda.time.Instant;
-import org.joda.time.format.DateTimeFormat;
 import org.junit.Before;
 import org.mockito.Mockito;
 

plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecurityScrollTests.java

@@ -16,6 +16,7 @@ import org.elasticsearch.search.SearchContextMissingException;
 import org.elasticsearch.test.SecurityIntegTestCase;
 import org.elasticsearch.test.SecuritySettingsSource;
 import org.elasticsearch.xpack.security.authc.support.UsernamePasswordToken;
+import org.junit.After;
 
 import java.util.Collections;
 
@@ -92,6 +93,11 @@ public class SecurityScrollTests extends SecurityIntegTestCase {
         }
     }
 
+    @After
+    public void cleanupSecurityIndex() throws Exception {
+        super.deleteSecurityIndex();
+    }
+
     @Override
     public String transportClientUsername() {
         return this.nodeClientUsername();

plugin/src/test/java/org/elasticsearch/xpack/security/support/IndexLifecycleManagerIntegTests.java

@@ -10,6 +10,7 @@ import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.test.SecurityIntegTestCase;
 import org.elasticsearch.xpack.security.action.user.PutUserRequest;
 import org.elasticsearch.xpack.security.action.user.PutUserResponse;
+import org.junit.After;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -37,4 +38,9 @@ public class IndexLifecycleManagerIntegTests extends SecurityIntegTestCase {
             assertTrue(future.actionGet().created());
         }
     }
+
+    @After
+    public void cleanupSecurityIndex() throws Exception {
+        super.deleteSecurityIndex();
+    }
 }

plugin/src/test/java/org/elasticsearch/xpack/security/user/XPackUserTests.java

@@ -31,7 +31,7 @@ public class XPackUserTests extends ESTestCase {
         final String action = randomFrom(GetAction.NAME, SearchAction.NAME, IndexAction.NAME);
         final Predicate<String> predicate = XPackUser.ROLE.indices().allowedIndicesMatcher(action);
         assertThat(predicate.test(SecurityLifecycleService.SECURITY_INDEX_NAME), Matchers.is(false));
-        assertThat(predicate.test(SecurityLifecycleService.NEW_SECURITY_INDEX_NAME), Matchers.is(false));
+        assertThat(predicate.test(SecurityLifecycleService.INTERNAL_SECURITY_INDEX), Matchers.is(false));
     }
 
     public void testXPackUserCanReadAuditTrail() {

plugin/src/test/resources/rest-api-spec/test/security/hidden-index/10_security_read.yml

@@ -0,0 +1,85 @@
+---
+setup:
+  - skip:
+      features: headers
+
+  - do:
+      cluster.health:
+          wait_for_status: yellow
+
+  - do:
+      xpack.security.put_role:
+        name: "all_access"
+        body:  >
+            {
+              "cluster": [ "all" ],
+              "indices": [
+                { "names": ["*"], "privileges": ["all"] }
+              ]
+            }
+
+  - do:
+      xpack.security.put_user:
+        username: "test_user"
+        body:  >
+          {
+            "password" : "x-pack-test-password",
+            "roles" : [ "all_access" ],
+            "full_name" : "user with all possible privileges (but not superuser)"
+          }
+
+---
+teardown:
+  - do:
+      xpack.security.delete_user:
+        username: "test_user"
+        ignore: 404
+
+  - do:
+      xpack.security.delete_role:
+        name: "all_access"
+        ignore: 404
+
+---
+"Test get security index metadata":
+
+  - do:
+      catch: forbidden
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      indices.get:
+        index: ".security"
+
+  - do:
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      indices.get:
+        index: ".secu*rity"
+  - length: { $body: 0 }
+
+---
+"Test get security document":
+
+  - do:
+      catch: forbidden
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      get:
+        index: ".security"
+        type: "doc"
+        id: "user-test_user"
+
+---
+"Test search security index":
+
+  - do:
+      catch: forbidden
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      search:
+        index: ".security"
+        type: "doc"
+
+  - do:
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      search:
+        index: ".secu*rity"
+        type: "doc"
+  - match: { hits.total: 0 }
+

plugin/src/test/resources/rest-api-spec/test/security/hidden-index/11_security-6_read.yml

@@ -0,0 +1,85 @@
+---
+setup:
+  - skip:
+      features: headers
+
+  - do:
+      cluster.health:
+          wait_for_status: yellow
+
+  - do:
+      xpack.security.put_role:
+        name: "all_access"
+        body:  >
+            {
+              "cluster": [ "all" ],
+              "indices": [
+                { "names": ["*"], "privileges": ["all"] }
+              ]
+            }
+
+  - do:
+      xpack.security.put_user:
+        username: "test_user"
+        body:  >
+          {
+            "password" : "x-pack-test-password",
+            "roles" : [ "all_access" ],
+            "full_name" : "user with all possible privileges (but not superuser)"
+          }
+
+---
+teardown:
+  - do:
+      xpack.security.delete_user:
+        username: "test_user"
+        ignore: 404
+
+  - do:
+      xpack.security.delete_role:
+        name: "all_access"
+        ignore: 404
+
+---
+"Test get security index metadata":
+
+  - do:
+      catch: forbidden
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      indices.get:
+        index: ".security-6"
+
+  - do:
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      indices.get:
+        index: ".security*6"
+  - length: { $body: 0 }
+
+---
+"Test get security document":
+
+  - do:
+      catch: forbidden
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      get:
+        index: ".security-6"
+        type: "doc"
+        id: "user-test_user"
+
+---
+"Test search security index":
+
+  - do:
+      catch: forbidden
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      search:
+        index: ".security-6"
+        type: "doc"
+
+  - do:
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      search:
+        index: ".security*6"
+        type: "doc"
+  - match: { hits.total: 0 }
+