Add comment and unit test for MlUpgradeModeActionFilter::order method

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

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);
     }