Refactor request marking for serverless and operator modes (elastic#1…

…10370)

The internal REST parameter `path_restricted` was introduced to signal to APIs that a request should be subject to partial restrictions in Serverless mode. 

The semantics of that parameter have since changed, subtly: in the initial iteration, only requests that corresponded to APIs with partial restrictions were marked. Since then, we have shifted to mark _all_ requests to public APIs by non-operator users, and relying on downstream API handlers to apply partial restrictions (if they had any). 

This PR replaces the parameter with two new parameters: `serverless_request` and `operator_request`. These parameters are set independently of each other: 

*  `serverless_request` is set for all requests made in serverless mode (regardless of whether the APIs are public or internal)
* `operator_request` is set for equests made by operators, both in serverless and in stateful mode

This gives REST handlers and other downstream code more flexibility in toggling behavior and decouples the concept of serverless mode from operator mode, which are technically two orthogonal things. 

Relates: ES-8753

server/src/main/java/org/elasticsearch/cluster/metadata/DataStreamLifecycle.java

@@ -370,7 +370,7 @@ public static DataStreamLifecycle fromXContent(XContentParser parser) throws IOE
      * Adds a retention param to signal that this serialisation should include the effective retention metadata
      */
     public static ToXContent.Params maybeAddEffectiveRetentionParams(ToXContent.Params params) {
-        boolean shouldAddEffectiveRetention = Objects.equals(params.param(RestRequest.PATH_RESTRICTED), "serverless");
+        boolean shouldAddEffectiveRetention = params.paramAsBoolean(RestRequest.SERVERLESS_REQUEST, false);
         return new DelegatingMapParams(
             Map.of(INCLUDE_EFFECTIVE_RETENTION_PARAM_NAME, Boolean.toString(shouldAddEffectiveRetention)),
             params

server/src/main/java/org/elasticsearch/rest/BaseRestHandler.java

@@ -83,7 +83,14 @@ public final void handleRequest(RestRequest request, RestChannel channel, NodeCl
         // check if the query has any parameters that are not in the supported set (if declared)
         Set<String> supported = allSupportedParameters();
         if (supported != null) {
-            var allSupported = Sets.union(RestResponse.RESPONSE_PARAMS, ALWAYS_SUPPORTED, supported);
+            var allSupported = Sets.union(
+                RestResponse.RESPONSE_PARAMS,
+                ALWAYS_SUPPORTED,
+                // these internal parameters cannot be set by end-users, but are used by Elasticsearch internally.
+                // they must be accepted by all handlers
+                RestRequest.INTERNAL_MARKER_REQUEST_PARAMETERS,
+                supported
+            );
             if (allSupported.containsAll(request.params().keySet()) == false) {
                 Set<String> unsupported = Sets.difference(request.params().keySet(), allSupported);
                 throw new IllegalArgumentException(unrecognized(request, unsupported, allSupported, "parameter"));

server/src/main/java/org/elasticsearch/rest/RestController.java

@@ -480,6 +480,14 @@ private void dispatchRequest(
             } else {
                 threadContext.putHeader(SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY, Boolean.TRUE.toString());
             }
+
+            if (apiProtections.isEnabled()) {
+                // API protections are only enabled in serverless; therefore we can use this as an indicator to mark the
+                // request as a serverless mode request here, so downstream handlers can use the marker
+                request.markAsServerlessRequest();
+                logger.trace("Marked request for uri [{}] as serverless request", request.uri());
+            }
+
             final var finalChannel = responseChannel;
             this.interceptor.intercept(request, responseChannel, handler.getConcreteRestHandler(), new ActionListener<>() {
                 @Override

server/src/main/java/org/elasticsearch/rest/RestRequest.java

@@ -48,7 +48,31 @@
 
 public class RestRequest implements ToXContent.Params, Traceable {
 
-    public static final String PATH_RESTRICTED = "pathRestricted";
+    /**
+     * Internal marker request parameter to indicate that a request was made in serverless mode. Use this parameter, together with
+     * {@link #OPERATOR_REQUEST} if you need to toggle behavior for serverless, for example to enforce partial API restrictions
+     * (prevent request fields, omit response fields) for an API.
+     * Requests not made in serverless mode, will *not* have this parameter set.
+     * Given a request instance, you can use {@link #isServerlessRequest()} to determine if the parameter is set or not.
+     * This is also available from {@code ToXContent.Params}. For example:
+     * {@code params.paramAsBoolean(RestRequest.SERVERLESS_REQUEST, false)}
+     */
+    public static final String SERVERLESS_REQUEST = "serverlessRequest";
+    /**
+     * Internal marker request parameter to indicate that a request was made by an operator user.
+     * Requests made by regular users (users without operator privileges), will *not* have this parameter set.
+     * Given a request instance, you can use {@link #isOperatorRequest()} to determine if the parameter is set or not.
+     * This is also available from {@code ToXContent.Params}. For example:
+     * {@code params.paramAsBoolean(RestRequest.OPERATOR_REQUEST, false)}
+     */
+    public static final String OPERATOR_REQUEST = "operatorRequest";
+
+    /**
+     * Internal request parameters used as markers to indicate various operations modes such as serverless mode, or operator mode.
+     * These can never be set directly by end-users. Instead, they are set internally by Elasticsearch and must be supported by all
+     * request handlers.
+     */
+    public static final Set<String> INTERNAL_MARKER_REQUEST_PARAMETERS = Set.of(SERVERLESS_REQUEST, OPERATOR_REQUEST);
     // tchar pattern as defined by RFC7230 section 3.2.6
     private static final Pattern TCHAR_PATTERN = Pattern.compile("[a-zA-Z0-9!#$%&'*+\\-.\\^_`|~]+");
 
@@ -616,13 +640,41 @@ public boolean hasExplicitRestApiVersion() {
         return restApiVersion.isPresent();
     }
 
-    public void markPathRestricted(String restriction) {
-        if (params.containsKey(PATH_RESTRICTED)) {
-            throw new IllegalArgumentException("The parameter [" + PATH_RESTRICTED + "] is already defined.");
+    /**
+     * See {@link #SERVERLESS_REQUEST}
+     */
+    public void markAsServerlessRequest() {
+        setParamTrueOnceAndConsume(SERVERLESS_REQUEST);
+    }
+
+    /**
+     * See {@link #SERVERLESS_REQUEST}
+     */
+    public boolean isServerlessRequest() {
+        return paramAsBoolean(SERVERLESS_REQUEST, false);
+    }
+
+    /**
+     * See {@link #OPERATOR_REQUEST}
+     */
+    public void markAsOperatorRequest() {
+        setParamTrueOnceAndConsume(OPERATOR_REQUEST);
+    }
+
+    /**
+     * See {@link #OPERATOR_REQUEST}
+     */
+    public boolean isOperatorRequest() {
+        return paramAsBoolean(OPERATOR_REQUEST, false);
+    }
+
+    private void setParamTrueOnceAndConsume(String param) {
+        if (params.containsKey(param)) {
+            throw new IllegalArgumentException("The parameter [" + param + "] is already defined.");
         }
-        params.put(PATH_RESTRICTED, restriction);
+        params.put(param, "true");
         // this parameter is intended be consumed via ToXContent.Params.param(..), not this.params(..) so don't require it is consumed here
-        consumedParams.add(PATH_RESTRICTED);
+        consumedParams.add(param);
     }
 
     @Override

server/src/main/java/org/elasticsearch/rest/RestUtils.java

@@ -23,7 +23,7 @@
 import java.util.regex.Pattern;
 
 import static org.elasticsearch.action.support.master.AcknowledgedRequest.DEFAULT_ACK_TIMEOUT;
-import static org.elasticsearch.rest.RestRequest.PATH_RESTRICTED;
+import static org.elasticsearch.rest.RestRequest.INTERNAL_MARKER_REQUEST_PARAMETERS;
 
 public class RestUtils {
 
@@ -85,8 +85,10 @@ private static String decodeQueryStringParam(final String s) {
     }
 
     private static void addParam(Map<String, String> params, String name, String value) {
-        if (PATH_RESTRICTED.equalsIgnoreCase(name)) {
-            throw new IllegalArgumentException("parameter [" + PATH_RESTRICTED + "] is reserved and may not set");
+        for (var reservedParameter : INTERNAL_MARKER_REQUEST_PARAMETERS) {
+            if (reservedParameter.equalsIgnoreCase(name)) {
+                throw new IllegalArgumentException("parameter [" + name + "] is reserved and may not be set");
+            }
         }
         params.put(name, value);
     }

server/src/main/java/org/elasticsearch/rest/ServerlessScope.java

@@ -22,9 +22,4 @@
 @Target(ElementType.TYPE)
 public @interface ServerlessScope {
     Scope value();
-
-    /**
-     * A value used when restricting a response of a serverless endpoints.
-     */
-    String SERVERLESS_RESTRICTION = "serverless";
 }

server/src/test/java/org/elasticsearch/action/datastreams/lifecycle/GetDataStreamLifecycleActionTests.java

@@ -22,7 +22,7 @@
 import java.io.IOException;
 import java.util.Map;
 
-import static org.elasticsearch.rest.RestRequest.PATH_RESTRICTED;
+import static org.elasticsearch.rest.RestRequest.SERVERLESS_REQUEST;
 import static org.hamcrest.Matchers.equalTo;
 
 public class GetDataStreamLifecycleActionTests extends ESTestCase {
@@ -75,7 +75,7 @@ private Map<String, Object> getXContentMap(
         TimeValue globalMaxRetention
     ) throws IOException {
         try (XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent())) {
-            ToXContent.Params params = new ToXContent.MapParams(Map.of(PATH_RESTRICTED, "serverless"));
+            ToXContent.Params params = new ToXContent.MapParams(Map.of(SERVERLESS_REQUEST, "true"));
             RolloverConfiguration rolloverConfiguration = null;
             DataStreamGlobalRetention globalRetention = new DataStreamGlobalRetention(globalDefaultRetention, globalMaxRetention);
             dataStreamLifecycle.toXContent(builder, params, rolloverConfiguration, globalRetention);

server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamLifecycleTests.java

@@ -39,7 +39,7 @@
 import static org.elasticsearch.cluster.metadata.DataStreamLifecycle.RetentionSource.DATA_STREAM_CONFIGURATION;
 import static org.elasticsearch.cluster.metadata.DataStreamLifecycle.RetentionSource.DEFAULT_GLOBAL_RETENTION;
 import static org.elasticsearch.cluster.metadata.DataStreamLifecycle.RetentionSource.MAX_GLOBAL_RETENTION;
-import static org.elasticsearch.rest.RestRequest.PATH_RESTRICTED;
+import static org.elasticsearch.rest.RestRequest.SERVERLESS_REQUEST;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.not;
@@ -354,13 +354,7 @@ public void testEffectiveRetentionParams() {
         }
         {
             ToXContent.Params params = DataStreamLifecycle.maybeAddEffectiveRetentionParams(
-                new ToXContent.MapParams(Map.of(PATH_RESTRICTED, "not-serverless"))
-            );
-            assertThat(params.paramAsBoolean(DataStreamLifecycle.INCLUDE_EFFECTIVE_RETENTION_PARAM_NAME, false), equalTo(false));
-        }
-        {
-            ToXContent.Params params = DataStreamLifecycle.maybeAddEffectiveRetentionParams(
-                new ToXContent.MapParams(Map.of(PATH_RESTRICTED, "serverless"))
+                new ToXContent.MapParams(Map.of(SERVERLESS_REQUEST, "true"))
             );
             assertThat(params.paramAsBoolean(DataStreamLifecycle.INCLUDE_EFFECTIVE_RETENTION_PARAM_NAME, false), equalTo(true));
         }

server/src/test/java/org/elasticsearch/rest/RestRequestTests.java

@@ -31,7 +31,8 @@
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonMap;
-import static org.elasticsearch.rest.RestRequest.PATH_RESTRICTED;
+import static org.elasticsearch.rest.RestRequest.OPERATOR_REQUEST;
+import static org.elasticsearch.rest.RestRequest.SERVERLESS_REQUEST;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
@@ -249,16 +250,30 @@ public void testRequiredContent() {
         assertEquals("unknown content type", e.getMessage());
     }
 
-    public void testMarkPathRestricted() {
+    public void testIsServerlessRequest() {
         RestRequest request1 = contentRestRequest("content", new HashMap<>());
-        request1.markPathRestricted("foo");
-        assertEquals(request1.param(PATH_RESTRICTED), "foo");
-        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> request1.markPathRestricted("foo"));
-        assertThat(exception.getMessage(), is("The parameter [" + PATH_RESTRICTED + "] is already defined."));
-
-        RestRequest request2 = contentRestRequest("content", Map.of(PATH_RESTRICTED, "foo"));
-        exception = expectThrows(IllegalArgumentException.class, () -> request2.markPathRestricted("bar"));
-        assertThat(exception.getMessage(), is("The parameter [" + PATH_RESTRICTED + "] is already defined."));
+        request1.markAsServerlessRequest();
+        assertEquals(request1.param(SERVERLESS_REQUEST), "true");
+        assertTrue(request1.isServerlessRequest());
+        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, request1::markAsServerlessRequest);
+        assertThat(exception.getMessage(), is("The parameter [" + SERVERLESS_REQUEST + "] is already defined."));
+
+        RestRequest request2 = contentRestRequest("content", Map.of(SERVERLESS_REQUEST, "true"));
+        exception = expectThrows(IllegalArgumentException.class, request2::markAsServerlessRequest);
+        assertThat(exception.getMessage(), is("The parameter [" + SERVERLESS_REQUEST + "] is already defined."));
+    }
+
+    public void testIsOperatorRequest() {
+        RestRequest request1 = contentRestRequest("content", new HashMap<>());
+        request1.markAsOperatorRequest();
+        assertEquals(request1.param(OPERATOR_REQUEST), "true");
+        assertTrue(request1.isOperatorRequest());
+        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, request1::markAsOperatorRequest);
+        assertThat(exception.getMessage(), is("The parameter [" + OPERATOR_REQUEST + "] is already defined."));
+
+        RestRequest request2 = contentRestRequest("content", Map.of(OPERATOR_REQUEST, "true"));
+        exception = expectThrows(IllegalArgumentException.class, request2::markAsOperatorRequest);
+        assertThat(exception.getMessage(), is("The parameter [" + OPERATOR_REQUEST + "] is already defined."));
     }
 
     public static RestRequest contentRestRequest(String content, Map<String, String> params) {

server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java

@@ -18,7 +18,7 @@
 import java.util.Map;
 import java.util.regex.Pattern;
 
-import static org.elasticsearch.rest.RestRequest.PATH_RESTRICTED;
+import static org.elasticsearch.rest.RestRequest.INTERNAL_MARKER_REQUEST_PARAMETERS;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
@@ -160,13 +160,15 @@ public void testCrazyURL() {
     }
 
     public void testReservedParameters() {
-        Map<String, String> params = new HashMap<>();
-        String uri = "something?" + PATH_RESTRICTED + "=value";
-        IllegalArgumentException exception = expectThrows(
-            IllegalArgumentException.class,
-            () -> RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params)
-        );
-        assertEquals(exception.getMessage(), "parameter [" + PATH_RESTRICTED + "] is reserved and may not set");
+        for (var reservedParam : INTERNAL_MARKER_REQUEST_PARAMETERS) {
+            Map<String, String> params = new HashMap<>();
+            String uri = "something?" + reservedParam + "=value";
+            IllegalArgumentException exception = expectThrows(
+                IllegalArgumentException.class,
+                () -> RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params)
+            );
+            assertEquals(exception.getMessage(), "parameter [" + reservedParam + "] is reserved and may not be set");
+        }
     }
 
     private void assertCorsSettingRegexIsNull(String settingsValue) {

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java

@@ -8,7 +8,6 @@
 package org.elasticsearch.xpack.security.operator;
 
 import org.elasticsearch.ElasticsearchException;
-import org.elasticsearch.ElasticsearchStatusException;
 import org.elasticsearch.rest.RestHandler;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.transport.TransportRequest;
@@ -23,21 +22,10 @@ public interface OperatorOnlyRegistry {
     OperatorPrivilegesViolation check(String action, TransportRequest request);
 
     /**
-     * Checks to see if a given {@link RestHandler} is subject to operator-only restrictions for the REST API.
-     *
-     * Any REST API may be fully or partially restricted.
-     * A fully restricted REST API mandates that the implementation of this method throw an
-     * {@link org.elasticsearch.ElasticsearchStatusException} with an appropriate status code and error message.
-     *
-     * A partially restricted REST API mandates that the {@link RestRequest} is marked as restricted so that the downstream handler can
-     * behave appropriately.
-     * For example, to restrict the REST response the implementation
-     * should call {@link RestRequest#markPathRestricted(String)} so that the downstream handler can properly restrict the response
-     * before returning to the client. Note - a partial restriction should not throw an exception.
-     *
-     * @param restHandler The {@link RestHandler} to check for any restrictions
-     * @param restRequest The {@link RestRequest} to check for any restrictions and mark any partially restricted REST API's
-     * @throws ElasticsearchStatusException if the request should be denied in its entirety (fully restricted)
+     * This method is only called if the user is not an operator.
+     * Implementations should fail the request if the {@link RestRequest} is not allowed to proceed by throwing an
+     * {@link org.elasticsearch.ElasticsearchException}. If the request should be handled by the associated {@link RestHandler},
+     * then this implementations should do nothing.
      */
     void checkRest(RestHandler restHandler, RestRequest restRequest) throws ElasticsearchException;
 

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java

@@ -182,6 +182,9 @@ public boolean checkRest(RestHandler restHandler, RestRequest restRequest, RestC
                     );
                     throw e;
                 }
+            } else {
+                restRequest.markAsOperatorRequest();
+                logger.trace("Marked request for uri [{}] as operator request", restRequest.uri());
             }
             return true;
         }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/privilege/RestGetBuiltinPrivilegesAction.java

@@ -85,9 +85,9 @@ public RestResponse buildResponse(GetBuiltinPrivilegesResponse response, XConten
 
     @Override
     protected Exception innerCheckFeatureAvailable(RestRequest request) {
-        final boolean restrictPath = request.hasParam(RestRequest.PATH_RESTRICTED);
-        assert false == restrictPath || DiscoveryNode.isStateless(settings);
-        if (false == restrictPath) {
+        final boolean shouldRestrictForServerless = shouldRestrictForServerless(request);
+        assert false == shouldRestrictForServerless || DiscoveryNode.isStateless(settings);
+        if (false == shouldRestrictForServerless) {
             return super.innerCheckFeatureAvailable(request);
         }
         // This is a temporary hack: we are re-using the native roles setting as an overall feature flag for custom roles.
@@ -106,4 +106,7 @@ protected Exception innerCheckFeatureAvailable(RestRequest request) {
         }
     }
 
+    private boolean shouldRestrictForServerless(RestRequest request) {
+        return request.isServerlessRequest() && false == request.isOperatorRequest();
+    }
 }