From 7c3fec431083256c7ce236d5341af15d4282c6d8 Mon Sep 17 00:00:00 2001
From: Przemyslaw Witek <przemyslaw.witek@elastic.co>
Date: Wed, 8 Apr 2020 11:22:44 +0200
Subject: [PATCH] Add comment and unit test for
 MlUpgradeModeActionFilter::order method

---
 .../xpack/ml/MlUpgradeModeActionFilter.java        |  8 +++++++-
 .../xpack/ml/MlUpgradeModeActionFilterTests.java   | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlUpgradeModeActionFilter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlUpgradeModeActionFilter.java
index faf82a7c69c77..9466ebc17729d 100644
--- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlUpgradeModeActionFilter.java
+++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlUpgradeModeActionFilter.java
@@ -130,9 +130,15 @@ protected boolean apply(String action, ActionRequest request, ActionListener<?>
         return true;
     }
 
+    /**
+     * To prevent leaking information to unauthorized users, it is extremely important that this filter is executed *after* the
+     * {@code SecurityActionFilter}.
+     * To achieve that, the number returned by this method must be greater than the number returned by the
+     * {@code SecurityActionFilter::order} method.
+     */
     @Override
     public int order() {
-        return 666;
+        return Integer.MAX_VALUE;
     }
 
     // Visible for testing
diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlUpgradeModeActionFilterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlUpgradeModeActionFilterTests.java
index 68b17223d2083..6058906de284e 100644
--- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlUpgradeModeActionFilterTests.java
+++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlUpgradeModeActionFilterTests.java
@@ -9,21 +9,27 @@
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionResponse;
+import org.elasticsearch.action.support.ActionFilter;
 import org.elasticsearch.action.support.ActionFilterChain;
+import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.cluster.ClusterChangedEvent;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.xpack.core.ml.MlMetadata;
 import org.elasticsearch.xpack.core.ml.action.PutJobAction;
 import org.elasticsearch.xpack.core.ml.action.SetUpgradeModeAction;
+import org.elasticsearch.xpack.security.action.filter.SecurityActionFilter;
 import org.junit.After;
 import org.junit.Before;
 
+import static org.hamcrest.Matchers.arrayContaining;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.mockito.Mockito.mock;
@@ -89,6 +95,14 @@ public void testApply_ActionAllowedInUpgradeMode() {
         verify(chain, times(3)).proceed(task, ALLOWED_ACTION, request, listener);
     }
 
+    public void testOrder_UpgradeFilterIsExecutedAfterSecurityFilter() {
+        MlUpgradeModeActionFilter upgradeModeFilter = new MlUpgradeModeActionFilter(clusterService);
+        SecurityActionFilter securityFilter = new SecurityActionFilter(null, null, null, mock(ThreadPool.class), null, null);
+
+        ActionFilter[] actionFiltersInOrderOfExecution = new ActionFilters(Sets.newHashSet(upgradeModeFilter, securityFilter)).filters();
+        assertThat(actionFiltersInOrderOfExecution, is(arrayContaining(securityFilter, upgradeModeFilter)));
+    }
+
     private static ClusterChangedEvent createClusterChangedEvent(ClusterState clusterState) {
         return new ClusterChangedEvent("created-from-test", clusterState, clusterState);
     }