Browse Source

Mandate Replacelabe for requests that allow remote indices (#78954)

This PR moves the allowsRemoteIndices method from IndicesRequest to its
sub-interface, Replaceable. This is necessary because:

1. Authorization currently assumes non-replaceable requests also do
   *not* allow remote indices
2. There is currently no requests that are both non-replaceable and
   allowing remote indices.

This change tightens the code logic by formalising the interfaces more
to prevent potential future mis-uses.
Yang Wang 4 năm trước cách đây
mục cha
commit
3446f07fcf

+ 15 - 7
server/src/main/java/org/elasticsearch/action/IndicesRequest.java

@@ -29,13 +29,6 @@ public interface IndicesRequest {
      */
     IndicesOptions indicesOptions();
 
-    /** 
-     * Determines whether the request can contain indices on a remote cluster.
-     */
-    default boolean allowsRemoteIndices() {
-        return false;
-    }
-
     /**
      * Determines whether the request should be applied to data streams. When {@code false}, none of the names or
      * wildcard expressions in {@link #indices} should be applied to or expanded to any data streams. All layers
@@ -50,5 +43,20 @@ public interface IndicesRequest {
          * Sets the indices that the action relates to.
          */
         IndicesRequest indices(String... indices);
+
+        /**
+         * Determines whether the request can contain indices on a remote cluster.
+         *
+         * NOTE in theory this method can belong to the {@link IndicesRequest} interface because whether a request
+         * allowing remote indices has no inherent relationship to whether it is {@link Replaceable} or not.
+         * However, we don't have an existing request that is non-replaceable but allows remote indices.
+         * In addition, authorization code currently relies on the fact that non-replaceable requests do not allow
+         * remote indices.
+         * That said, it is possible to remove this constraint should the needs arise in the future. We just need
+         * proceed with extra caution.
+         */
+        default boolean allowsRemoteIndices() {
+            return false;
+        }
     }
 }

+ 3 - 6
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java

@@ -70,7 +70,8 @@ class IndicesAndAliasesResolver {
      * that is consistent and does not change during the life of the request.
      * </p>
      * <p>
-     * If the provided <code>request</code> is of a type that {@link IndicesRequest#allowsRemoteIndices() allows remote indices},
+     * If the provided <code>request</code> is of a type that
+     * {@link IndicesRequest.Replaceable#allowsRemoteIndices() allows remote indices},
      * then the index names will be categorized into those that refer to {@link ResolvedIndices#getLocal() local indices}, and those that
      * refer to {@link ResolvedIndices#getRemote() remote indices}. This categorization follows the standard
      * {@link RemoteClusterAware#buildRemoteIndexName(String, String) remote index-name format} and also respects the currently defined
@@ -137,10 +138,6 @@ class IndicesAndAliasesResolver {
         if (indicesRequest instanceof IndicesRequest.Replaceable) {
             return true;
         }
-        // TODO: Strictly speaking we should also check for allowRemoteIndices because resolveIndicesAndAliasesWithoutWildcards
-        //       assumes everything is local indices. It is currently not a practical issue since we don't have any requests
-        //       that are non-replaceable but allowRemoteIndices. We will address this in a separate PR and keep this as is
-        //       to match the existing behaviour.
         return false;
     }
 
@@ -218,7 +215,7 @@ class IndicesAndAliasesResolver {
                 // we honour allow_no_indices like es core does.
             } else {
                 final ResolvedIndices split;
-                if (indicesRequest.allowsRemoteIndices()) {
+                if (replaceable.allowsRemoteIndices()) {
                     split = remoteClusterResolver.splitLocalAndRemoteIndexNames(indicesRequest.indices());
                 } else {
                     split = new ResolvedIndices(Arrays.asList(indicesRequest.indices()), Collections.emptyList());

+ 1 - 1
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java

@@ -313,7 +313,7 @@ public class RBACEngine implements AuthorizationEngine {
             listener.onResponse(
                 new IndexAuthorizationResult(true, requestInfo.getOriginatingAuthorizationContext().getIndicesAccessControl())
             );
-        } else if (((IndicesRequest) request).allowsRemoteIndices()) {
+        } else if (request instanceof IndicesRequest.Replaceable && ((IndicesRequest.Replaceable) request).allowsRemoteIndices()) {
             // remote indices are allowed
             indicesAsyncSupplier.getAsync(ActionListener.wrap(resolvedIndices -> {
                 assert resolvedIndices.isEmpty() == false