Skip to content

Commit

Permalink
Bring back operator and serverless request marking (elastic#112554)
Browse files Browse the repository at this point in the history
  • Loading branch information
n1v0lg authored Sep 6, 2024
1 parent d2433c4 commit ee68d0c
Show file tree
Hide file tree
Showing 13 changed files with 151 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.cluster.metadata.DataStreamLifecycle;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.RestUtils;
Expand Down Expand Up @@ -61,17 +62,19 @@ public Set<String> supportedCapabilities() {

@Override
public Set<String> supportedQueryParameters() {
return Set.of(
"name",
"include_defaults",
"timeout",
"master_timeout",
RestRequest.PATH_RESTRICTED,
IndicesOptions.WildcardOptions.EXPAND_WILDCARDS,
IndicesOptions.ConcreteTargetOptions.IGNORE_UNAVAILABLE,
IndicesOptions.WildcardOptions.ALLOW_NO_INDICES,
IndicesOptions.GatekeeperOptions.IGNORE_THROTTLED,
"verbose"
return Sets.union(
RestRequest.INTERNAL_MARKER_REQUEST_PARAMETERS,
Set.of(
"name",
"include_defaults",
"timeout",
"master_timeout",
IndicesOptions.WildcardOptions.EXPAND_WILDCARDS,
IndicesOptions.ConcreteTargetOptions.IGNORE_UNAVAILABLE,
IndicesOptions.WildcardOptions.ALLOW_NO_INDICES,
IndicesOptions.GatekeeperOptions.IGNORE_THROTTLED,
"verbose"
)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
64 changes: 58 additions & 6 deletions server/src/main/java/org/elasticsearch/rest/RestRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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!#$%&'*+\\-.\\^_`|~]+");

Expand Down Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions server/src/main/java/org/elasticsearch/rest/RestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
35 changes: 25 additions & 10 deletions server/src/test/java/org/elasticsearch/rest/RestRequestTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
18 changes: 10 additions & 8 deletions server/src/test/java/org/elasticsearch/rest/RestUtilsTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -106,4 +106,7 @@ protected Exception innerCheckFeatureAvailable(RestRequest request) {
}
}

private boolean shouldRestrictForServerless(RestRequest request) {
return request.isServerlessRequest() && false == request.isOperatorRequest();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
package org.elasticsearch.xpack.security.rest.action.role;

import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.RestApiVersion;
Expand Down Expand Up @@ -54,9 +53,9 @@ public String getName() {
@Override
public RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClient client) throws IOException {
final String[] roles = request.paramAsStringArray("name", Strings.EMPTY_ARRAY);
final boolean restrictRequest = isPathRestricted(request);
final boolean restrictToNativeRolesOnly = request.isServerlessRequest() && false == request.isOperatorRequest();
return channel -> new GetRolesRequestBuilder(client).names(roles)
.nativeOnly(restrictRequest)
.nativeOnly(restrictToNativeRolesOnly)
.execute(new RestBuilderListener<>(channel) {
@Override
public RestResponse buildResponse(GetRolesResponse response, XContentBuilder builder) throws Exception {
Expand Down Expand Up @@ -84,17 +83,10 @@ protected Exception innerCheckFeatureAvailable(RestRequest request) {
// Note: For non-restricted requests this action handles both reserved roles and native
// roles, and should still be available even if native role management is disabled.
// For restricted requests it should only be available if native role management is enabled
final boolean restrictPath = isPathRestricted(request);
if (false == restrictPath) {
if (false == request.isServerlessRequest() || request.isOperatorRequest()) {
return null;
} else {
return super.innerCheckFeatureAvailable(request);
}
}

private boolean isPathRestricted(RestRequest request) {
final boolean restrictRequest = request.hasParam(RestRequest.PATH_RESTRICTED);
assert false == restrictRequest || DiscoveryNode.isStateless(settings);
return restrictRequest;
}
}
Loading

0 comments on commit ee68d0c

Please sign in to comment.