From 6b8d31771c2532c50024e6f1f4158603e1401826 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Mon, 21 Dec 2020 17:24:00 +1100 Subject: [PATCH 01/16] wip --- .../common/settings/Setting.java | 20 ++++++++++-- .../xpack/security/Security.java | 3 +- .../operator/OperatorOnlyRegistry.java | 32 +++++++++++++++++++ .../security/transport/filter/IPFilter.java | 4 +-- .../operator/OperatorOnlyRegistryTests.java | 6 +++- 5 files changed, 59 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index 98f8b7b64c60a..6fb3a03104469 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -97,6 +97,11 @@ public enum Property { */ Dynamic, + /** + * Dynamic and operator only + */ + DynamicOperator, + /** * mark this setting as final, not updateable even when the context is not dynamic * ie. Setting this property on an index scoped setting will fail update when the index is closed @@ -167,9 +172,13 @@ private Setting(Key key, @Nullable Setting fallbackSetting, Function propertiesAsSet = EnumSet.copyOf(Arrays.asList(properties)); - if (propertiesAsSet.contains(Property.Dynamic) && propertiesAsSet.contains(Property.Final)) { + if ((propertiesAsSet.contains(Property.Dynamic) || propertiesAsSet.contains(Property.DynamicOperator)) + && propertiesAsSet.contains(Property.Final)) { throw new IllegalArgumentException("final setting [" + key + "] cannot be dynamic"); } + if (propertiesAsSet.contains(Property.Dynamic) && propertiesAsSet.contains(Property.DynamicOperator)) { + throw new IllegalArgumentException("setting [" + key + "] cannot be both dynamic and dynamic operator"); + } checkPropertyRequiresIndexScope(propertiesAsSet, Property.NotCopyableOnResize); checkPropertyRequiresIndexScope(propertiesAsSet, Property.InternalIndex); checkPropertyRequiresIndexScope(propertiesAsSet, Property.PrivateIndex); @@ -294,7 +303,14 @@ public final Key getRawKey() { * Returns true if this setting is dynamically updateable, otherwise false */ public final boolean isDynamic() { - return properties.contains(Property.Dynamic); + return properties.contains(Property.Dynamic) || properties.contains(Property.DynamicOperator); + } + + /** + * Returns true if this setting is dynamically updateable by operators, otherwise false + */ + public final boolean isDynamicOperator() { + return properties.contains(Property.DynamicOperator); } /** diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index c0de59af088e0..4f23b95e568ce 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -481,7 +481,8 @@ Collection createComponents(Client client, ThreadPool threadPool, Cluste final OperatorPrivilegesService operatorPrivilegesService; if (OPERATOR_PRIVILEGES_ENABLED.get(settings)) { operatorPrivilegesService = new OperatorPrivileges.DefaultOperatorPrivilegesService(getLicenseState(), - new FileOperatorUsersStore(environment, resourceWatcherService), new OperatorOnlyRegistry()); + new FileOperatorUsersStore(environment, resourceWatcherService), + new OperatorOnlyRegistry(clusterService.getClusterSettings())); } else { operatorPrivilegesService = OperatorPrivileges.NOOP_OPERATOR_PRIVILEGES_SERVICE; } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java index b3b633199d8f8..33d891e777884 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java @@ -8,11 +8,19 @@ import org.elasticsearch.action.admin.cluster.configuration.AddVotingConfigExclusionsAction; import org.elasticsearch.action.admin.cluster.configuration.ClearVotingConfigExclusionsAction; +import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsAction; +import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.license.DeleteLicenseAction; import org.elasticsearch.license.PutLicenseAction; import org.elasticsearch.transport.TransportRequest; +import java.util.List; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; public class OperatorOnlyRegistry { @@ -26,6 +34,12 @@ public class OperatorOnlyRegistry { "cluster:admin/autoscaling/get_autoscaling_policy", "cluster:admin/autoscaling/get_autoscaling_capacity"); + private final ClusterSettings clusterSettings; + + public OperatorOnlyRegistry(ClusterSettings clusterSettings) { + this.clusterSettings = clusterSettings; + } + /** * Check whether the given action and request qualify as operator-only. The method returns * null if the action+request is NOT operator-only. Other it returns a violation object @@ -34,6 +48,24 @@ public class OperatorOnlyRegistry { public OperatorPrivilegesViolation check(String action, TransportRequest request) { if (SIMPLE_ACTIONS.contains(action)) { return () -> "action [" + action + "]"; + } else if (ClusterUpdateSettingsAction.NAME.equals(action)) { + assert request instanceof ClusterUpdateSettingsRequest; + return checkClusterUpdateSettings((ClusterUpdateSettingsRequest) request); + } else { + return null; + } + } + + private OperatorPrivilegesViolation checkClusterUpdateSettings(ClusterUpdateSettingsRequest request) { + List operatorOnlySettingKeys = Stream.concat( + request.transientSettings().keySet().stream(), request.persistentSettings().keySet().stream() + ).filter(k -> { + final Setting setting = clusterSettings.get(k); + return setting != null && setting.isDynamicOperator(); + }).collect(Collectors.toList()); + if (false == operatorOnlySettingKeys.isEmpty()) { + return () -> (operatorOnlySettingKeys.size() == 1 ? "setting" : "settings") + + " [" + Strings.collectionToDelimitedString(operatorOnlySettingKeys, ",") + "]"; } else { return null; } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java index 6a6b4c748a707..b1e729f92f307 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java @@ -49,10 +49,10 @@ public class IPFilter { Setting.boolSetting(setting("filter.always_allow_bound_address"), true, Property.NodeScope); public static final Setting IP_FILTER_ENABLED_HTTP_SETTING = Setting.boolSetting(setting("http.filter.enabled"), - true, Property.Dynamic, Property.NodeScope); + true, Property.DynamicOperator, Property.NodeScope); public static final Setting IP_FILTER_ENABLED_SETTING = Setting.boolSetting(setting("transport.filter.enabled"), - true, Property.Dynamic, Property.NodeScope); + true, Property.DynamicOperator, Property.NodeScope); public static final Setting> TRANSPORT_FILTER_ALLOW_SETTING = Setting.listSetting(setting("transport.filter.allow"), Collections.emptyList(), Function.identity(), Property.Dynamic, Property.NodeScope); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java index 71ce0aa488c2a..2c6be5350ec77 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java @@ -6,6 +6,8 @@ package org.elasticsearch.xpack.security.operator; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.junit.Before; @@ -17,7 +19,9 @@ public class OperatorOnlyRegistryTests extends ESTestCase { @Before public void init() { - operatorOnlyRegistry = new OperatorOnlyRegistry(); + operatorOnlyRegistry = new OperatorOnlyRegistry( + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); } public void testSimpleOperatorOnlyApi() { From 6b104bd72d5ef20c5cab118adae17457857ae3b7 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Mon, 21 Dec 2020 21:08:57 +1100 Subject: [PATCH 02/16] Skip operator settings in restore when user is not operator --- .../restore/RestoreSnapshotRequest.java | 9 + .../snapshots/RestoreService.java | 19 ++ .../qa/operator-privileges-tests/build.gradle | 8 + .../operator/OperatorPrivilegesIT.java | 162 +++++++++++++++++- .../src/javaRestTest/resources/roles.yml | 2 + .../xpack/security/Security.java | 3 +- .../security/authz/AuthorizationService.java | 1 + .../security/operator/OperatorPrivileges.java | 23 +++ 8 files changed, 217 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index bd1e84cdff58b..346817d1df812 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -61,6 +61,7 @@ public class RestoreSnapshotRequest extends MasterNodeRequest operatorSettingKeys = settings.keySet().stream() + .filter(k -> { + final Setting setting = clusterSettings.get(k); + return setting != null && setting.isDynamicOperator(); + }) + .collect(Collectors.toSet()); + if (false == operatorSettingKeys.isEmpty()) { + settings = Settings.builder() + .put(settings.filter(k -> false == operatorSettingKeys.contains(k))) + .put(currentState.metadata().persistentSettings().filter(operatorSettingKeys::contains)) + .build(); + } + } mdBuilder.persistentSettings(settings); } if (metadata.templates() != null) { diff --git a/x-pack/plugin/security/qa/operator-privileges-tests/build.gradle b/x-pack/plugin/security/qa/operator-privileges-tests/build.gradle index f4afcb9d30c58..13d4941518a14 100644 --- a/x-pack/plugin/security/qa/operator-privileges-tests/build.gradle +++ b/x-pack/plugin/security/qa/operator-privileges-tests/build.gradle @@ -16,6 +16,13 @@ dependencies { javaRestTestImplementation project.sourceSets.main.runtimeClasspath } +File repoDir = file("$buildDir/testclusters/repo") + +javaRestTest { + /* To support taking snapshots, we have to set path.repo setting */ + systemProperty 'tests.path.repo', repoDir +} + testClusters.all { testDistribution = 'DEFAULT' numberOfNodes = 3 @@ -27,6 +34,7 @@ testClusters.all { setting 'xpack.security.enabled', 'true' setting 'xpack.security.http.ssl.enabled', 'false' setting 'xpack.security.operator_privileges.enabled', "true" + setting 'path.repo', repoDir.absolutePath user username: "test_admin", password: 'x-pack-test-password', role: "superuser" user username: "test_operator", password: 'x-pack-test-password', role: "limited_operator" diff --git a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java index d71b3f766440c..ae6a19835f057 100644 --- a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java +++ b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java @@ -8,15 +8,20 @@ import org.elasticsearch.client.Request; import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.util.set.Sets; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.rest.ESRestTestCase; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Base64; +import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -28,6 +33,9 @@ public class OperatorPrivilegesIT extends ESRestTestCase { + private static final String OPERATOR_AUTH_HEADER = "Basic " + + Base64.getEncoder().encodeToString("test_operator:x-pack-test-password".getBytes(StandardCharsets.UTF_8));; + @Override protected Settings restClientSettings() { String token = basicAuthHeaderValue("test_admin", new SecureString("x-pack-test-password".toCharArray())); @@ -46,17 +54,13 @@ public void testNonOperatorSuperuserWillFailToCallOperatorOnlyApiWhenOperatorPri public void testOperatorUserWillSucceedToCallOperatorOnlyApi() throws IOException { final Request postVotingConfigExclusionsRequest = new Request("POST", "_cluster/voting_config_exclusions?node_names=foo"); - final String authHeader = "Basic " - + Base64.getEncoder().encodeToString("test_operator:x-pack-test-password".getBytes(StandardCharsets.UTF_8)); - postVotingConfigExclusionsRequest.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", authHeader)); + postVotingConfigExclusionsRequest.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", OPERATOR_AUTH_HEADER)); client().performRequest(postVotingConfigExclusionsRequest); } public void testOperatorUserWillFailToCallOperatorOnlyApiIfRbacFails() throws IOException { final Request deleteVotingConfigExclusionsRequest = new Request("DELETE", "_cluster/voting_config_exclusions"); - final String authHeader = "Basic " - + Base64.getEncoder().encodeToString("test_operator:x-pack-test-password".getBytes(StandardCharsets.UTF_8)); - deleteVotingConfigExclusionsRequest.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", authHeader)); + deleteVotingConfigExclusionsRequest.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", OPERATOR_AUTH_HEADER)); final ResponseException responseException = expectThrows( ResponseException.class, () -> client().performRequest(deleteVotingConfigExclusionsRequest) @@ -67,9 +71,7 @@ public void testOperatorUserWillFailToCallOperatorOnlyApiIfRbacFails() throws IO public void testOperatorUserCanCallNonOperatorOnlyApi() throws IOException { final Request mainRequest = new Request("GET", "/"); - final String authHeader = "Basic " - + Base64.getEncoder().encodeToString("test_operator:x-pack-test-password".getBytes(StandardCharsets.UTF_8)); - mainRequest.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", authHeader)); + mainRequest.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", OPERATOR_AUTH_HEADER)); client().performRequest(mainRequest); } @@ -100,4 +102,146 @@ public void testOperatorPrivilegesXpackUsage() throws IOException { assertTrue((boolean) operatorPrivileges.get("available")); assertTrue((boolean) operatorPrivileges.get("enabled")); } + + public void testUpdateOperatorSettings() throws IOException { + final Map settings = new HashMap<>( + Map.of("xpack.security.http.filter.enabled", "false", "xpack.security.transport.filter.enabled", "false") + ); + final boolean extraSettings = randomBoolean(); + if (extraSettings) { + settings.put("search.allow_expensive_queries", false); + } + final ResponseException responseException = expectThrows(ResponseException.class, () -> updateSettings(settings, null)); + assertThat(responseException.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + assertThat(responseException.getMessage(), containsString("is unauthorized for user")); + assertTrue(getPersistentSettings().isEmpty()); + + updateSettings(settings, OPERATOR_AUTH_HEADER); + + Map persistentSettings = getPersistentSettings(); + assertThat(persistentSettings.get("xpack.security.http.filter.enabled"), equalTo("false")); + assertThat(persistentSettings.get("xpack.security.transport.filter.enabled"), equalTo("false")); + if (extraSettings) { + assertThat(persistentSettings.get("search.allow_expensive_queries"), equalTo("false")); + } + } + + @SuppressWarnings("unchecked") + public void testSnapshotRestoreBehaviourOfOperatorSettings() throws IOException { + final String repoName = "repo"; + final String snapshotName = "snap"; + createSnapshotRepo(repoName); + // Initial values + updateSettings( + Map.of( + "xpack.security.http.filter.enabled", + "false", + "xpack.security.transport.filter.enabled", + "false", + "search.default_keep_alive", + "10m" + ), + OPERATOR_AUTH_HEADER + ); + takeSnapshot(repoName, snapshotName); + // change to different values + updateSettings( + Map.of( + "xpack.security.transport.filter.enabled", + "true", + "search.default_keep_alive", + "1m", + "search.allow_expensive_queries", + "false" + ), + OPERATOR_AUTH_HEADER + ); + deleteSettings(List.of("xpack.security.http.filter.enabled"), OPERATOR_AUTH_HEADER); + + // Restore with non-operator and the operator settings will not be touched + restoreSnapshot(repoName, snapshotName, null); + Map persistentSettings = getPersistentSettings(); + assertNull(persistentSettings.get("xpack.security.http.filter.enabled")); + assertThat(persistentSettings.get("xpack.security.transport.filter.enabled"), equalTo("true")); + assertThat(persistentSettings.get("search.default_keep_alive"), equalTo("10m")); + assertNull(persistentSettings.get("search.allow_expensive_queries")); + + // Now restore with operator user and the operator settings will also be restored + restoreSnapshot(repoName, snapshotName, OPERATOR_AUTH_HEADER); + persistentSettings = getPersistentSettings(); + assertThat(persistentSettings.get("xpack.security.http.filter.enabled"), equalTo("false")); + assertThat(persistentSettings.get("xpack.security.transport.filter.enabled"), equalTo("false")); + assertThat(persistentSettings.get("search.default_keep_alive"), equalTo("10m")); + } + + private void createSnapshotRepo(String repoName) throws IOException { + Request request = new Request("PUT", "/_snapshot/" + repoName); + request.setJsonEntity( + Strings.toString( + JsonXContent.contentBuilder() + .startObject() + .field("type", "fs") + .startObject("settings") + .field("location", System.getProperty("tests.path.repo")) + .endObject() + .endObject() + ) + ); + assertOK(client().performRequest(request)); + } + + private void updateSettings(Map settings, String authHeader) throws IOException { + final Request request = new Request("PUT", "/_cluster/settings"); + if (authHeader != null) { + request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", authHeader)); + } + request.setJsonEntity( + Strings.toString( + JsonXContent.contentBuilder().startObject().startObject("persistent").mapContents(settings).endObject().endObject() + ) + ); + assertOK(client().performRequest(request)); + } + + private void deleteSettings(Collection settingKeys, String authHeader) throws IOException { + final Request request = new Request("PUT", "/_cluster/settings"); + if (authHeader != null) { + request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", authHeader)); + } + final XContentBuilder builder = JsonXContent.contentBuilder().startObject().startObject("persistent"); + for (String k : settingKeys) { + builder.nullField(k); + } + builder.endObject().endObject(); + request.setJsonEntity(Strings.toString(builder)); + assertOK(client().performRequest(request)); + } + + private void takeSnapshot(String repoName, String snapshotName) throws IOException { + final Request request = new Request("POST", "/_snapshot/" + repoName + "/" + snapshotName); + request.addParameter("wait_for_completion", "true"); + request.setJsonEntity( + Strings.toString(JsonXContent.contentBuilder().startObject().field("include_global_state", true).endObject()) + ); + assertOK(client().performRequest(request)); + } + + private void restoreSnapshot(String repoName, String snapshotName, String authHeader) throws IOException { + final Request request = new Request("POST", "/_snapshot/" + repoName + "/" + snapshotName + "/_restore"); + if (authHeader != null) { + request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", authHeader)); + } + request.addParameter("wait_for_completion", "true"); + request.setJsonEntity( + Strings.toString(JsonXContent.contentBuilder().startObject().field("include_global_state", true).endObject()) + ); + assertOK(client().performRequest(request)); + } + + @SuppressWarnings("unchecked") + private Map getPersistentSettings() throws IOException { + final Request getSettingsRequest = new Request("GET", "/_cluster/settings?flat_settings"); + Map response = entityAsMap(client().performRequest(getSettingsRequest)); + return (Map) response.get("persistent"); + } } diff --git a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/resources/roles.yml b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/resources/roles.yml index ac6d3a00dacad..a63ebf977a61c 100644 --- a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/resources/roles.yml +++ b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/resources/roles.yml @@ -2,3 +2,5 @@ limited_operator: cluster: - "cluster:admin/voting_config/add_exclusions" - "monitor" + - "cluster:admin/settings/update" + - "cluster:admin/snapshot/restore" diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 4f23b95e568ce..d5617a01c07af 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -479,7 +479,8 @@ Collection createComponents(Client client, ThreadPool threadPool, Cluste final AuthenticationFailureHandler failureHandler = createAuthenticationFailureHandler(realms, extensionComponents); final OperatorPrivilegesService operatorPrivilegesService; - if (OPERATOR_PRIVILEGES_ENABLED.get(settings)) { + final boolean operatorPrivilegesEnabled = OPERATOR_PRIVILEGES_ENABLED.get(settings); + if (operatorPrivilegesEnabled) { operatorPrivilegesService = new OperatorPrivileges.DefaultOperatorPrivilegesService(getLicenseState(), new FileOperatorUsersStore(environment, resourceWatcherService), new OperatorOnlyRegistry(clusterService.getClusterSettings())); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index ba110c185eeee..d0829431e43be 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -213,6 +213,7 @@ public void authorize(final Authentication authentication, final String action, listener.onFailure(denialException(authentication, action, operatorException)); return; } + operatorPrivilegesService.maybeInterceptRequest(threadContext, originalRequest); if (SystemUser.is(authentication.getUser())) { // this never goes async so no need to wrap the listener diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java index 98083b20ad54e..735d3987be23b 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java @@ -9,6 +9,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.ElasticsearchSecurityException; +import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.license.XPackLicenseState; @@ -35,6 +36,11 @@ public interface OperatorPrivilegesService { * @return An exception if user is an non-operator and the request is operator-only. Otherwise returns null. */ ElasticsearchSecurityException check(String action, TransportRequest request, ThreadContext threadContext); + + /** + * Check the threadContext to see whether the current authenticating user is an operator user + */ + void maybeInterceptRequest(ThreadContext threadContext, TransportRequest request); } public static final class DefaultOperatorPrivilegesService implements OperatorPrivilegesService { @@ -78,6 +84,19 @@ public ElasticsearchSecurityException check(String action, TransportRequest requ return null; } + public void maybeInterceptRequest(ThreadContext threadContext, TransportRequest request) { + if (false == shouldProcess()) { + return; + } + if (request instanceof RestoreSnapshotRequest) { + final boolean isOperatorInContext = AuthenticationField.PRIVILEGE_CATEGORY_VALUE_OPERATOR.equals( + threadContext.getHeader(AuthenticationField.PRIVILEGE_CATEGORY_KEY)); + if (false == isOperatorInContext) { + ((RestoreSnapshotRequest) request).filterOperatorSettings(true); + } + } + } + private boolean shouldProcess() { return licenseState.checkFeature(XPackLicenseState.Feature.OPERATOR_PRIVILEGES); } @@ -92,5 +111,9 @@ public void maybeMarkOperatorUser(Authentication authentication, ThreadContext t public ElasticsearchSecurityException check(String action, TransportRequest request, ThreadContext threadContext) { return null; } + + @Override + public void maybeInterceptRequest(ThreadContext threadContext, TransportRequest request) { + } }; } From 5f18f660bb1afdcb36089b4907c578f26b4d28fe Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Mon, 21 Dec 2020 21:28:46 +1100 Subject: [PATCH 03/16] tweak --- .../org/elasticsearch/snapshots/RestoreService.java | 4 +++- .../security/operator/OperatorPrivilegesIT.java | 12 ++++++++---- .../xpack/security/transport/filter/IPFilter.java | 12 ++++++------ 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index 930727ad4d268..44fbd93ebe116 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -92,6 +92,7 @@ import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; +import java.util.stream.Stream; import static java.util.Collections.unmodifiableSet; import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS; @@ -449,7 +450,8 @@ restoreUUID, snapshot, overallState(RestoreInProgress.State.INIT, shards), // * operator setting, any value from the snapshot is ignored and any value from current cluster state // is retained if (request.filterOperatorSettings()) { - Set operatorSettingKeys = settings.keySet().stream() + Set operatorSettingKeys = Stream.concat( + settings.keySet().stream(), currentState.metadata().persistentSettings().keySet().stream()) .filter(k -> { final Setting setting = clusterSettings.get(k); return setting != null && setting.isDynamicOperator(); diff --git a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java index ae6a19835f057..03ba095302d25 100644 --- a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java +++ b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java @@ -136,8 +136,8 @@ public void testSnapshotRestoreBehaviourOfOperatorSettings() throws IOException Map.of( "xpack.security.http.filter.enabled", "false", - "xpack.security.transport.filter.enabled", - "false", + "xpack.security.http.filter.allow", + "example.com", "search.default_keep_alive", "10m" ), @@ -145,10 +145,13 @@ public void testSnapshotRestoreBehaviourOfOperatorSettings() throws IOException ); takeSnapshot(repoName, snapshotName); // change to different values + deleteSettings(List.of("xpack.security.http.filter.enabled"), OPERATOR_AUTH_HEADER); updateSettings( Map.of( "xpack.security.transport.filter.enabled", "true", + "xpack.security.http.filter.allow", + "tutorial.com", "search.default_keep_alive", "1m", "search.allow_expensive_queries", @@ -156,13 +159,13 @@ public void testSnapshotRestoreBehaviourOfOperatorSettings() throws IOException ), OPERATOR_AUTH_HEADER ); - deleteSettings(List.of("xpack.security.http.filter.enabled"), OPERATOR_AUTH_HEADER); // Restore with non-operator and the operator settings will not be touched restoreSnapshot(repoName, snapshotName, null); Map persistentSettings = getPersistentSettings(); assertNull(persistentSettings.get("xpack.security.http.filter.enabled")); assertThat(persistentSettings.get("xpack.security.transport.filter.enabled"), equalTo("true")); + assertThat(persistentSettings.get("xpack.security.http.filter.allow"), equalTo("tutorial.com")); assertThat(persistentSettings.get("search.default_keep_alive"), equalTo("10m")); assertNull(persistentSettings.get("search.allow_expensive_queries")); @@ -170,7 +173,8 @@ public void testSnapshotRestoreBehaviourOfOperatorSettings() throws IOException restoreSnapshot(repoName, snapshotName, OPERATOR_AUTH_HEADER); persistentSettings = getPersistentSettings(); assertThat(persistentSettings.get("xpack.security.http.filter.enabled"), equalTo("false")); - assertThat(persistentSettings.get("xpack.security.transport.filter.enabled"), equalTo("false")); + assertNull(persistentSettings.get("xpack.security.transport.filter.enabled")); + assertThat(persistentSettings.get("xpack.security.http.filter.allow"), equalTo("example.com")); assertThat(persistentSettings.get("search.default_keep_alive"), equalTo("10m")); } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java index b1e729f92f307..72fadcfb86c83 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java @@ -55,29 +55,29 @@ public class IPFilter { true, Property.DynamicOperator, Property.NodeScope); public static final Setting> TRANSPORT_FILTER_ALLOW_SETTING = Setting.listSetting(setting("transport.filter.allow"), - Collections.emptyList(), Function.identity(), Property.Dynamic, Property.NodeScope); + Collections.emptyList(), Function.identity(), Property.DynamicOperator, Property.NodeScope); public static final Setting> TRANSPORT_FILTER_DENY_SETTING = Setting.listSetting(setting("transport.filter.deny"), - Collections.emptyList(), Function.identity(), Property.Dynamic, Property.NodeScope); + Collections.emptyList(), Function.identity(), Property.DynamicOperator, Property.NodeScope); public static final Setting.AffixSetting> PROFILE_FILTER_DENY_SETTING = Setting.affixKeySetting("transport.profiles.", "xpack.security.filter.deny", key -> Setting.listSetting(key, Collections.emptyList(), Function.identity(), - Property.Dynamic, Property.NodeScope)); + Property.DynamicOperator, Property.NodeScope)); public static final Setting.AffixSetting> PROFILE_FILTER_ALLOW_SETTING = Setting.affixKeySetting("transport.profiles.", "xpack.security.filter.allow", key -> Setting.listSetting(key, Collections.emptyList(), Function.identity(), - Property.Dynamic, Property.NodeScope)); + Property.DynamicOperator, Property.NodeScope)); private static final Setting> HTTP_FILTER_ALLOW_FALLBACK = Setting.listSetting("transport.profiles.default.xpack.security.filter.allow", TRANSPORT_FILTER_ALLOW_SETTING, s -> s, Property.NodeScope); public static final Setting> HTTP_FILTER_ALLOW_SETTING = Setting.listSetting(setting("http.filter.allow"), - HTTP_FILTER_ALLOW_FALLBACK, Function.identity(), Property.Dynamic, Property.NodeScope); + HTTP_FILTER_ALLOW_FALLBACK, Function.identity(), Property.DynamicOperator, Property.NodeScope); private static final Setting> HTTP_FILTER_DENY_FALLBACK = Setting.listSetting("transport.profiles.default.xpack.security.filter.deny", TRANSPORT_FILTER_DENY_SETTING, s -> s, Property.NodeScope); public static final Setting> HTTP_FILTER_DENY_SETTING = Setting.listSetting(setting("http.filter.deny"), - HTTP_FILTER_DENY_FALLBACK, Function.identity(), Property.Dynamic, Property.NodeScope); + HTTP_FILTER_DENY_FALLBACK, Function.identity(), Property.DynamicOperator, Property.NodeScope); public static final Map DISABLED_USAGE_STATS = Map.of( "http", false, From 2933b6885c649cc4bc7a115bca279b106d3965d9 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 22 Dec 2020 21:11:24 +1100 Subject: [PATCH 04/16] Add ml settings for operator only --- .../elasticsearch/xpack/ml/MachineLearning.java | 14 +++++++------- .../ml/MlConfigMigrationEligibilityCheck.java | 2 +- .../utils/persistence/ResultsPersisterService.java | 2 +- .../security/operator/OperatorPrivileges.java | 4 +--- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java index 70a4b6382d7e8..abd619487474c 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java @@ -418,7 +418,7 @@ public Set getRoles() { public static final String MACHINE_MEMORY_NODE_ATTR = "ml.machine_memory"; public static final String MAX_JVM_SIZE_NODE_ATTR = "ml.max_jvm_size"; public static final Setting CONCURRENT_JOB_ALLOCATIONS = - Setting.intSetting("xpack.ml.node_concurrent_job_allocations", 2, 0, Property.Dynamic, Property.NodeScope); + Setting.intSetting("xpack.ml.node_concurrent_job_allocations", 2, 0, Property.DynamicOperator, Property.NodeScope); /** * The amount of memory needed to load the ML native code shared libraries. The assumption is that the first * ML job to run on a given node will do this, and then subsequent ML jobs on the same node will reuse the @@ -432,7 +432,7 @@ public Set getRoles() { // controls the types of jobs that can be created, and each job alone is considerably smaller than what each node // can handle. public static final Setting MAX_MACHINE_MEMORY_PERCENT = - Setting.intSetting("xpack.ml.max_machine_memory_percent", 30, 5, 200, Property.Dynamic, Property.NodeScope); + Setting.intSetting("xpack.ml.max_machine_memory_percent", 30, 5, 200, Property.DynamicOperator, Property.NodeScope); /** * This boolean value indicates if `max_machine_memory_percent` should be ignored and a automatic calculation is used instead. * @@ -448,10 +448,10 @@ public Set getRoles() { public static final Setting USE_AUTO_MACHINE_MEMORY_PERCENT = Setting.boolSetting( "xpack.ml.use_auto_machine_memory_percent", false, - Property.Dynamic, + Property.DynamicOperator, Property.NodeScope); public static final Setting MAX_LAZY_ML_NODES = - Setting.intSetting("xpack.ml.max_lazy_ml_nodes", 0, 0, Property.Dynamic, Property.NodeScope); + Setting.intSetting("xpack.ml.max_lazy_ml_nodes", 0, 0, Property.DynamicOperator, Property.NodeScope); // Before 8.0.0 this needs to match the max allowed value for xpack.ml.max_open_jobs, // as the current node could be running in a cluster where some nodes are still using @@ -466,7 +466,7 @@ public Set getRoles() { public static final Setting PROCESS_CONNECT_TIMEOUT = Setting.timeSetting("xpack.ml.process_connect_timeout", TimeValue.timeValueSeconds(10), - TimeValue.timeValueSeconds(5), Setting.Property.Dynamic, Setting.Property.NodeScope); + TimeValue.timeValueSeconds(5), Property.DynamicOperator, Setting.Property.NodeScope); // Undocumented setting for integration test purposes public static final Setting MIN_DISK_SPACE_OFF_HEAP = @@ -485,7 +485,7 @@ public Set getRoles() { } return value; }, - Property.Dynamic, + Property.DynamicOperator, Property.NodeScope ); @@ -498,7 +498,7 @@ public Set getRoles() { public static final Setting MAX_ML_NODE_SIZE = Setting.byteSizeSetting( "xpack.ml.max_ml_node_size", ByteSizeValue.ZERO, - Property.Dynamic, + Property.DynamicOperator, Property.NodeScope); private static final Logger logger = LogManager.getLogger(MachineLearning.class); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java index a1b9c2529dccb..d2500864311f0 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java @@ -23,7 +23,7 @@ public class MlConfigMigrationEligibilityCheck { public static final Setting ENABLE_CONFIG_MIGRATION = Setting.boolSetting( - "xpack.ml.enable_config_migration", true, Setting.Property.Dynamic, Setting.Property.NodeScope); + "xpack.ml.enable_config_migration", true, Setting.Property.DynamicOperator, Setting.Property.NodeScope); private volatile boolean isConfigMigrationEnabled; diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/persistence/ResultsPersisterService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/persistence/ResultsPersisterService.java index d629028cffe6f..4393375771da5 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/persistence/ResultsPersisterService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/persistence/ResultsPersisterService.java @@ -76,7 +76,7 @@ public class ResultsPersisterService { 20, 0, 50, - Setting.Property.Dynamic, + Setting.Property.DynamicOperator, Setting.Property.NodeScope); private static final int MAX_RETRY_SLEEP_MILLIS = (int)Duration.ofMinutes(15).toMillis(); private static final int MIN_RETRY_SLEEP_MILLIS = 50; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java index 735d3987be23b..d1526cfae6a10 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java @@ -91,9 +91,7 @@ public void maybeInterceptRequest(ThreadContext threadContext, TransportRequest if (request instanceof RestoreSnapshotRequest) { final boolean isOperatorInContext = AuthenticationField.PRIVILEGE_CATEGORY_VALUE_OPERATOR.equals( threadContext.getHeader(AuthenticationField.PRIVILEGE_CATEGORY_KEY)); - if (false == isOperatorInContext) { - ((RestoreSnapshotRequest) request).filterOperatorSettings(true); - } + ((RestoreSnapshotRequest) request).filterOperatorSettings(false == isOperatorInContext); } } From 0f225e012f73b67f7436c29fb66586c25783d0cb Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 28 Jan 2021 22:38:58 +1100 Subject: [PATCH 05/16] Update according to discussion email --- .../restore/RestoreSnapshotRequest.java | 10 +++++----- .../elasticsearch/common/settings/Setting.java | 12 ++++++------ .../elasticsearch/snapshots/RestoreService.java | 7 ++----- .../elasticsearch/xpack/ml/MachineLearning.java | 14 +++++++------- .../ml/MlConfigMigrationEligibilityCheck.java | 2 +- .../persistence/ResultsPersisterService.java | 2 +- .../security/operator/OperatorPrivilegesIT.java | 12 ++---------- .../security/operator/OperatorPrivileges.java | 4 +--- .../security/transport/filter/IPFilter.java | 16 ++++++++-------- 9 files changed, 33 insertions(+), 46 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index 346817d1df812..45857fab71844 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -61,7 +61,7 @@ public class RestoreSnapshotRequest extends MasterNodeRequest fallbackSetting, Function propertiesAsSet = EnumSet.copyOf(Arrays.asList(properties)); - if ((propertiesAsSet.contains(Property.Dynamic) || propertiesAsSet.contains(Property.DynamicOperator)) + if ((propertiesAsSet.contains(Property.Dynamic) || propertiesAsSet.contains(Property.OperatorDynamic)) && propertiesAsSet.contains(Property.Final)) { throw new IllegalArgumentException("final setting [" + key + "] cannot be dynamic"); } - if (propertiesAsSet.contains(Property.Dynamic) && propertiesAsSet.contains(Property.DynamicOperator)) { + if (propertiesAsSet.contains(Property.Dynamic) && propertiesAsSet.contains(Property.OperatorDynamic)) { throw new IllegalArgumentException("setting [" + key + "] cannot be both dynamic and dynamic operator"); } checkPropertyRequiresIndexScope(propertiesAsSet, Property.NotCopyableOnResize); @@ -304,14 +304,14 @@ public final Key getRawKey() { * Returns true if this setting is dynamically updateable, otherwise false */ public final boolean isDynamic() { - return properties.contains(Property.Dynamic) || properties.contains(Property.DynamicOperator); + return properties.contains(Property.Dynamic) || properties.contains(Property.OperatorDynamic); } /** * Returns true if this setting is dynamically updateable by operators, otherwise false */ public final boolean isDynamicOperator() { - return properties.contains(Property.DynamicOperator); + return properties.contains(Property.OperatorDynamic); } /** diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index 14313c9526e5f..c8da8b4f4f63d 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -445,11 +445,8 @@ restoreUUID, snapshot, overallState(RestoreInProgress.State.INIT, shards), if (metadata.persistentSettings() != null) { Settings settings = metadata.persistentSettings(); clusterSettings.validateUpdate(settings); - // Filter out operator only settings if required so that the behaviour is as follows: - // * normal setting, will be either overridden from snapshot or wiped if snapshot does not contain it - // * operator setting, any value from the snapshot is ignored and any value from current cluster state - // is retained - if (request.filterOperatorSettings()) { + if (request.skipOperatorSettings()) { + // Skip any operator-only settings from the snapshot. This happens when operator privileges are enabled Set operatorSettingKeys = Stream.concat( settings.keySet().stream(), currentState.metadata().persistentSettings().keySet().stream()) .filter(k -> { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java index 8287fc495a1f4..3b1f6736e651d 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java @@ -415,7 +415,7 @@ public Set getRoles() { public static final String MACHINE_MEMORY_NODE_ATTR = "ml.machine_memory"; public static final String MAX_JVM_SIZE_NODE_ATTR = "ml.max_jvm_size"; public static final Setting CONCURRENT_JOB_ALLOCATIONS = - Setting.intSetting("xpack.ml.node_concurrent_job_allocations", 2, 0, Property.DynamicOperator, Property.NodeScope); + Setting.intSetting("xpack.ml.node_concurrent_job_allocations", 2, 0, Property.OperatorDynamic, Property.NodeScope); /** * The amount of memory needed to load the ML native code shared libraries. The assumption is that the first * ML job to run on a given node will do this, and then subsequent ML jobs on the same node will reuse the @@ -429,7 +429,7 @@ public Set getRoles() { // controls the types of jobs that can be created, and each job alone is considerably smaller than what each node // can handle. public static final Setting MAX_MACHINE_MEMORY_PERCENT = - Setting.intSetting("xpack.ml.max_machine_memory_percent", 30, 5, 200, Property.DynamicOperator, Property.NodeScope); + Setting.intSetting("xpack.ml.max_machine_memory_percent", 30, 5, 200, Property.OperatorDynamic, Property.NodeScope); /** * This boolean value indicates if `max_machine_memory_percent` should be ignored and a automatic calculation is used instead. * @@ -445,10 +445,10 @@ public Set getRoles() { public static final Setting USE_AUTO_MACHINE_MEMORY_PERCENT = Setting.boolSetting( "xpack.ml.use_auto_machine_memory_percent", false, - Property.DynamicOperator, + Property.OperatorDynamic, Property.NodeScope); public static final Setting MAX_LAZY_ML_NODES = - Setting.intSetting("xpack.ml.max_lazy_ml_nodes", 0, 0, Property.DynamicOperator, Property.NodeScope); + Setting.intSetting("xpack.ml.max_lazy_ml_nodes", 0, 0, Property.OperatorDynamic, Property.NodeScope); // Before 8.0.0 this needs to match the max allowed value for xpack.ml.max_open_jobs, // as the current node could be running in a cluster where some nodes are still using @@ -463,7 +463,7 @@ public Set getRoles() { public static final Setting PROCESS_CONNECT_TIMEOUT = Setting.timeSetting("xpack.ml.process_connect_timeout", TimeValue.timeValueSeconds(10), - TimeValue.timeValueSeconds(5), Property.DynamicOperator, Setting.Property.NodeScope); + TimeValue.timeValueSeconds(5), Property.OperatorDynamic, Setting.Property.NodeScope); // Undocumented setting for integration test purposes public static final Setting MIN_DISK_SPACE_OFF_HEAP = @@ -482,7 +482,7 @@ public Set getRoles() { } return value; }, - Property.DynamicOperator, + Property.OperatorDynamic, Property.NodeScope ); @@ -495,7 +495,7 @@ public Set getRoles() { public static final Setting MAX_ML_NODE_SIZE = Setting.byteSizeSetting( "xpack.ml.max_ml_node_size", ByteSizeValue.ZERO, - Property.DynamicOperator, + Property.OperatorDynamic, Property.NodeScope); private static final Logger logger = LogManager.getLogger(MachineLearning.class); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java index d2500864311f0..b7e90c3c27508 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java @@ -23,7 +23,7 @@ public class MlConfigMigrationEligibilityCheck { public static final Setting ENABLE_CONFIG_MIGRATION = Setting.boolSetting( - "xpack.ml.enable_config_migration", true, Setting.Property.DynamicOperator, Setting.Property.NodeScope); + "xpack.ml.enable_config_migration", true, Setting.Property.OperatorDynamic, Setting.Property.NodeScope); private volatile boolean isConfigMigrationEnabled; diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/persistence/ResultsPersisterService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/persistence/ResultsPersisterService.java index 4393375771da5..85e53704f6ce8 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/persistence/ResultsPersisterService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/persistence/ResultsPersisterService.java @@ -76,7 +76,7 @@ public class ResultsPersisterService { 20, 0, 50, - Setting.Property.DynamicOperator, + Setting.Property.OperatorDynamic, Setting.Property.NodeScope); private static final int MAX_RETRY_SLEEP_MILLIS = (int)Duration.ofMinutes(15).toMillis(); private static final int MIN_RETRY_SLEEP_MILLIS = 50; diff --git a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java index 03ba095302d25..bb9a5be598c35 100644 --- a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java +++ b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java @@ -160,22 +160,14 @@ public void testSnapshotRestoreBehaviourOfOperatorSettings() throws IOException OPERATOR_AUTH_HEADER ); - // Restore with non-operator and the operator settings will not be touched - restoreSnapshot(repoName, snapshotName, null); + // Restore with either operator or non-operator and the operator settings will not be touched + restoreSnapshot(repoName, snapshotName, randomFrom(OPERATOR_AUTH_HEADER, null)); Map persistentSettings = getPersistentSettings(); assertNull(persistentSettings.get("xpack.security.http.filter.enabled")); assertThat(persistentSettings.get("xpack.security.transport.filter.enabled"), equalTo("true")); assertThat(persistentSettings.get("xpack.security.http.filter.allow"), equalTo("tutorial.com")); assertThat(persistentSettings.get("search.default_keep_alive"), equalTo("10m")); assertNull(persistentSettings.get("search.allow_expensive_queries")); - - // Now restore with operator user and the operator settings will also be restored - restoreSnapshot(repoName, snapshotName, OPERATOR_AUTH_HEADER); - persistentSettings = getPersistentSettings(); - assertThat(persistentSettings.get("xpack.security.http.filter.enabled"), equalTo("false")); - assertNull(persistentSettings.get("xpack.security.transport.filter.enabled")); - assertThat(persistentSettings.get("xpack.security.http.filter.allow"), equalTo("example.com")); - assertThat(persistentSettings.get("search.default_keep_alive"), equalTo("10m")); } private void createSnapshotRepo(String repoName) throws IOException { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java index d1526cfae6a10..5007c2df58eb4 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java @@ -89,9 +89,7 @@ public void maybeInterceptRequest(ThreadContext threadContext, TransportRequest return; } if (request instanceof RestoreSnapshotRequest) { - final boolean isOperatorInContext = AuthenticationField.PRIVILEGE_CATEGORY_VALUE_OPERATOR.equals( - threadContext.getHeader(AuthenticationField.PRIVILEGE_CATEGORY_KEY)); - ((RestoreSnapshotRequest) request).filterOperatorSettings(false == isOperatorInContext); + ((RestoreSnapshotRequest) request).skipOperatorSettings(true); } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java index 72fadcfb86c83..64fff81d4a3f5 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java @@ -49,35 +49,35 @@ public class IPFilter { Setting.boolSetting(setting("filter.always_allow_bound_address"), true, Property.NodeScope); public static final Setting IP_FILTER_ENABLED_HTTP_SETTING = Setting.boolSetting(setting("http.filter.enabled"), - true, Property.DynamicOperator, Property.NodeScope); + true, Property.OperatorDynamic, Property.NodeScope); public static final Setting IP_FILTER_ENABLED_SETTING = Setting.boolSetting(setting("transport.filter.enabled"), - true, Property.DynamicOperator, Property.NodeScope); + true, Property.OperatorDynamic, Property.NodeScope); public static final Setting> TRANSPORT_FILTER_ALLOW_SETTING = Setting.listSetting(setting("transport.filter.allow"), - Collections.emptyList(), Function.identity(), Property.DynamicOperator, Property.NodeScope); + Collections.emptyList(), Function.identity(), Property.OperatorDynamic, Property.NodeScope); public static final Setting> TRANSPORT_FILTER_DENY_SETTING = Setting.listSetting(setting("transport.filter.deny"), - Collections.emptyList(), Function.identity(), Property.DynamicOperator, Property.NodeScope); + Collections.emptyList(), Function.identity(), Property.OperatorDynamic, Property.NodeScope); public static final Setting.AffixSetting> PROFILE_FILTER_DENY_SETTING = Setting.affixKeySetting("transport.profiles.", "xpack.security.filter.deny", key -> Setting.listSetting(key, Collections.emptyList(), Function.identity(), - Property.DynamicOperator, Property.NodeScope)); + Property.OperatorDynamic, Property.NodeScope)); public static final Setting.AffixSetting> PROFILE_FILTER_ALLOW_SETTING = Setting.affixKeySetting("transport.profiles.", "xpack.security.filter.allow", key -> Setting.listSetting(key, Collections.emptyList(), Function.identity(), - Property.DynamicOperator, Property.NodeScope)); + Property.OperatorDynamic, Property.NodeScope)); private static final Setting> HTTP_FILTER_ALLOW_FALLBACK = Setting.listSetting("transport.profiles.default.xpack.security.filter.allow", TRANSPORT_FILTER_ALLOW_SETTING, s -> s, Property.NodeScope); public static final Setting> HTTP_FILTER_ALLOW_SETTING = Setting.listSetting(setting("http.filter.allow"), - HTTP_FILTER_ALLOW_FALLBACK, Function.identity(), Property.DynamicOperator, Property.NodeScope); + HTTP_FILTER_ALLOW_FALLBACK, Function.identity(), Property.OperatorDynamic, Property.NodeScope); private static final Setting> HTTP_FILTER_DENY_FALLBACK = Setting.listSetting("transport.profiles.default.xpack.security.filter.deny", TRANSPORT_FILTER_DENY_SETTING, s -> s, Property.NodeScope); public static final Setting> HTTP_FILTER_DENY_SETTING = Setting.listSetting(setting("http.filter.deny"), - HTTP_FILTER_DENY_FALLBACK, Function.identity(), Property.DynamicOperator, Property.NodeScope); + HTTP_FILTER_DENY_FALLBACK, Function.identity(), Property.OperatorDynamic, Property.NodeScope); public static final Map DISABLED_USAGE_STATS = Map.of( "http", false, From 57f518c3ba88bc32caa4ec095895e0acf3f0e386 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Fri, 29 Jan 2021 09:59:36 +1100 Subject: [PATCH 06/16] tweak --- .../snapshots/restore/RestoreSnapshotRequest.java | 10 +++++----- .../org/elasticsearch/snapshots/RestoreService.java | 3 ++- .../xpack/security/operator/OperatorPrivileges.java | 8 ++++---- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index 45857fab71844..84316887cfc85 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -61,7 +61,7 @@ public class RestoreSnapshotRequest extends MasterNodeRequest operatorSettingKeys = Stream.concat( settings.keySet().stream(), currentState.metadata().persistentSettings().keySet().stream()) @@ -474,6 +474,7 @@ restoreUUID, snapshot, overallState(RestoreInProgress.State.INIT, shards), if (RepositoriesMetadata.TYPE.equals(cursor.key) == false && DataStreamMetadata.TYPE.equals(cursor.key) == false && cursor.value instanceof Metadata.NonRestorableCustom == false) { + // TODO: Check request.skipOperatorOnly for Autoscaling policies (NonRestorableCustom) // Don't restore repositories while we are working with them // TODO: Should we restore them at the end? // Also, don't restore data streams here, we already added them to the metadata builder above diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java index 5007c2df58eb4..f6376bbec832b 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java @@ -85,11 +85,8 @@ public ElasticsearchSecurityException check(String action, TransportRequest requ } public void maybeInterceptRequest(ThreadContext threadContext, TransportRequest request) { - if (false == shouldProcess()) { - return; - } if (request instanceof RestoreSnapshotRequest) { - ((RestoreSnapshotRequest) request).skipOperatorSettings(true); + ((RestoreSnapshotRequest) request).skipOperatorOnly(shouldProcess()); } } @@ -110,6 +107,9 @@ public ElasticsearchSecurityException check(String action, TransportRequest requ @Override public void maybeInterceptRequest(ThreadContext threadContext, TransportRequest request) { + if (request instanceof RestoreSnapshotRequest) { + ((RestoreSnapshotRequest) request).skipOperatorOnly(false); + } } }; } From 80504794c19102a98006cd51c4d1e4734ecec78a Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Mon, 1 Feb 2021 17:02:12 +1100 Subject: [PATCH 07/16] add tests --- .../restore/RestoreSnapshotRequest.java | 7 +- .../common/settings/Setting.java | 2 +- .../restore/RestoreSnapshotRequestTests.java | 21 +++++ .../common/settings/SettingTests.java | 15 ++- .../OperatorPrivilegesSingleNodeTests.java | 42 ++++++++- .../operator/OperatorOnlyRegistryTests.java | 92 ++++++++++++++++++- .../operator/OperatorPrivilegesTests.java | 41 +++++++-- 7 files changed, 198 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index 84316887cfc85..6fe4fcdbfa564 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -61,7 +61,7 @@ public class RestoreSnapshotRequest extends MasterNodeRequest fallbackSetting, Function map = parser.mapOrdered(); + assertFalse(map.containsKey("skip_operator_only")); + + // Nor does it serialise to streamInput + final BytesStreamOutput streamOutput = new BytesStreamOutput(); + original.writeTo(streamOutput); + final RestoreSnapshotRequest deserialized = new RestoreSnapshotRequest(streamOutput.bytes().streamInput()); + assertFalse(deserialized.skipOperatorOnly()); + } } diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java index ac88a7091b5d7..95276c30d73dd 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -950,10 +950,16 @@ public void testRejectNullProperties() { public void testRejectConflictingDynamicAndFinalProperties() { IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> Setting.simpleString("foo.bar", Property.Final, Property.Dynamic)); + () -> Setting.simpleString("foo.bar", Property.Final, randomFrom(Property.Dynamic, Property.OperatorDynamic))); assertThat(ex.getMessage(), containsString("final setting [foo.bar] cannot be dynamic")); } + public void testRejectConflictingDynamicAndOperatorDynamicProperties() { + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, + () -> Setting.simpleString("foo.bar", Property.Dynamic, Property.OperatorDynamic)); + assertThat(ex.getMessage(), containsString("setting [foo.bar] cannot be both dynamic and operator dynamic")); + } + public void testRejectNonIndexScopedNotCopyableOnResizeSetting() { final IllegalArgumentException e = expectThrows( IllegalArgumentException.class, @@ -1251,4 +1257,11 @@ public boolean innerMatch(LogEvent event) { mockLogAppender.stop(); } } + + public void testDynamicTest() { + final Property property = randomFrom(Property.Dynamic, Property.OperatorDynamic); + final Setting setting = Setting.simpleString("foo.bar", property); + assertTrue(setting.isDynamic()); + assertEquals(setting.isDynamicOperator(), property == Property.OperatorDynamic); + } } diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesSingleNodeTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesSingleNodeTests.java index 5aada3954e96a..48bb1d4c9c71a 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesSingleNodeTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesSingleNodeTests.java @@ -9,12 +9,15 @@ import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.action.admin.cluster.configuration.ClearVotingConfigExclusionsAction; import org.elasticsearch.action.admin.cluster.configuration.ClearVotingConfigExclusionsRequest; +import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsAction; +import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.elasticsearch.client.Client; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.SecuritySingleNodeTestCase; import org.elasticsearch.xpack.core.security.action.user.GetUsersAction; import org.elasticsearch.xpack.core.security.action.user.GetUsersRequest; +import org.elasticsearch.xpack.security.transport.filter.IPFilter; import java.util.Map; @@ -39,6 +42,7 @@ protected String configRoles() { + "limited_operator:\n" + " cluster:\n" + " - 'cluster:admin/voting_config/clear_exclusions'\n" + + " - 'cluster:admin/settings/update'\n" + " - 'monitor'\n"; } @@ -63,20 +67,52 @@ protected Settings nodeSettings() { return builder.build(); } - public void testOutcomeOfSuperuserPerformingOperatorOnlyActionWillDependOnWhetherFeatureIsEnabled() { + public void testOperatorOnlyActionOrSettingWillNotBeActionableByNormalSuperuser() { final Client client = client(); final ClearVotingConfigExclusionsRequest clearVotingConfigExclusionsRequest = new ClearVotingConfigExclusionsRequest(); - final ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, + ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, () -> client.execute(ClearVotingConfigExclusionsAction.INSTANCE, clearVotingConfigExclusionsRequest).actionGet()); assertThat(e.getCause().getMessage(), containsString("Operator privileges are required for action")); + + final Settings settings = Settings.builder().put(IPFilter.IP_FILTER_ENABLED_SETTING.getKey(), "null").build(); + final ClusterUpdateSettingsRequest clusterUpdateSettingsRequest = new ClusterUpdateSettingsRequest(); + if (randomBoolean()) { + clusterUpdateSettingsRequest.transientSettings(settings); + } else { + clusterUpdateSettingsRequest.persistentSettings(settings); + } + e = expectThrows(ElasticsearchSecurityException.class, + () -> client.execute(ClusterUpdateSettingsAction.INSTANCE, clusterUpdateSettingsRequest).actionGet()); + assertThat(e.getCause().getMessage(), containsString("Operator privileges are required for setting")); } - public void testOperatorUserWillSucceedToCallOperatorOnlyAction() { + public void testOperatorUserWillSucceedToCallOperatorOnlyActionOrSetting() { final Client client = client().filterWithHeader(Map.of( "Authorization", basicAuthHeaderValue(OPERATOR_USER_NAME, new SecureString(TEST_PASSWORD.toCharArray())))); final ClearVotingConfigExclusionsRequest clearVotingConfigExclusionsRequest = new ClearVotingConfigExclusionsRequest(); client.execute(ClearVotingConfigExclusionsAction.INSTANCE, clearVotingConfigExclusionsRequest).actionGet(); + + final ClusterUpdateSettingsRequest clusterUpdateSettingsRequest = new ClusterUpdateSettingsRequest(); + final Settings settings = Settings.builder().put(IPFilter.IP_FILTER_ENABLED_SETTING.getKey(), false).build(); + final boolean useTransientSetting = randomBoolean(); + try { + if (useTransientSetting) { + clusterUpdateSettingsRequest.transientSettings(settings); + } else { + clusterUpdateSettingsRequest.persistentSettings(settings); + } + client.execute(ClusterUpdateSettingsAction.INSTANCE, clusterUpdateSettingsRequest).actionGet(); + } finally { + final ClusterUpdateSettingsRequest clearSettingsRequest = new ClusterUpdateSettingsRequest(); + final Settings clearSettings = Settings.builder().putNull(IPFilter.IP_FILTER_ENABLED_SETTING.getKey()).build(); + if (useTransientSetting) { + clearSettingsRequest.transientSettings(clearSettings); + } else { + clearSettingsRequest.persistentSettings(clearSettings); + } + client.execute(ClusterUpdateSettingsAction.INSTANCE, clearSettingsRequest).actionGet(); + } } public void testOperatorUserIsStillSubjectToRoleLimits() { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java index 2c6be5350ec77..8dc93e44e1efa 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java @@ -6,22 +6,53 @@ package org.elasticsearch.xpack.security.operator; +import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsAction; +import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.junit.Before; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; + +import static org.elasticsearch.xpack.security.transport.filter.IPFilter.HTTP_FILTER_ALLOW_SETTING; +import static org.elasticsearch.xpack.security.transport.filter.IPFilter.HTTP_FILTER_DENY_SETTING; +import static org.elasticsearch.xpack.security.transport.filter.IPFilter.IP_FILTER_ENABLED_HTTP_SETTING; +import static org.elasticsearch.xpack.security.transport.filter.IPFilter.IP_FILTER_ENABLED_SETTING; +import static org.elasticsearch.xpack.security.transport.filter.IPFilter.PROFILE_FILTER_ALLOW_SETTING; +import static org.elasticsearch.xpack.security.transport.filter.IPFilter.PROFILE_FILTER_DENY_SETTING; +import static org.elasticsearch.xpack.security.transport.filter.IPFilter.TRANSPORT_FILTER_ALLOW_SETTING; +import static org.elasticsearch.xpack.security.transport.filter.IPFilter.TRANSPORT_FILTER_DENY_SETTING; import static org.hamcrest.Matchers.containsString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class OperatorOnlyRegistryTests extends ESTestCase { + private static final Set> DYNAMIC_SETTINGS = ClusterSettings.BUILT_IN_CLUSTER_SETTINGS.stream() + .filter(Setting::isDynamic) + .filter(setting -> false == setting.isDynamicOperator()) + .collect(Collectors.toSet()); + private static final Set> IP_FILTER_SETTINGS = Set.of( + IP_FILTER_ENABLED_HTTP_SETTING, + IP_FILTER_ENABLED_SETTING, + TRANSPORT_FILTER_ALLOW_SETTING, + TRANSPORT_FILTER_DENY_SETTING, + PROFILE_FILTER_DENY_SETTING, + PROFILE_FILTER_ALLOW_SETTING, + HTTP_FILTER_ALLOW_SETTING, + HTTP_FILTER_DENY_SETTING); + private OperatorOnlyRegistry operatorOnlyRegistry; @Before public void init() { - operatorOnlyRegistry = new OperatorOnlyRegistry( - new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) - ); + final Set> settingsSet = new HashSet<>(IP_FILTER_SETTINGS); + settingsSet.addAll(DYNAMIC_SETTINGS); + operatorOnlyRegistry = new OperatorOnlyRegistry(new ClusterSettings(Settings.EMPTY, settingsSet)); } public void testSimpleOperatorOnlyApi() { @@ -38,6 +69,59 @@ public void testNonOperatorOnlyApi() { assertNull(operatorOnlyRegistry.check(actionName, null)); } - // TODO: not tests for settings yet since it's not settled whether it will be part of phase 1 + public void testOperatorOnlySettings() { + final ClusterUpdateSettingsRequest request; + final Setting transientSetting; + final Setting persistentSetting; + final OperatorOnlyRegistry.OperatorPrivilegesViolation violation; + + switch (randomIntBetween(0, 3)) { + case 0: + transientSetting = convertToConcreteSettingIfNecessary(randomFrom(IP_FILTER_SETTINGS)); + persistentSetting = convertToConcreteSettingIfNecessary( + randomValueOtherThan(transientSetting, () -> randomFrom(IP_FILTER_SETTINGS))); + request = prepareClusterUpdateSettingsRequest(transientSetting, persistentSetting); + violation = operatorOnlyRegistry.check(ClusterUpdateSettingsAction.NAME, request); + assertThat(violation.message(), containsString(String.format("settings [%s,%s]", + transientSetting.getKey(), persistentSetting.getKey()))); + break; + case 1: + transientSetting = convertToConcreteSettingIfNecessary(randomFrom(IP_FILTER_SETTINGS)); + persistentSetting = convertToConcreteSettingIfNecessary(randomFrom(DYNAMIC_SETTINGS)); + request = prepareClusterUpdateSettingsRequest(transientSetting, persistentSetting); + violation = operatorOnlyRegistry.check(ClusterUpdateSettingsAction.NAME, request); + assertThat(violation.message(), containsString(String.format("setting [%s]", transientSetting.getKey()))); + break; + case 2: + transientSetting = convertToConcreteSettingIfNecessary(randomFrom(DYNAMIC_SETTINGS)); + persistentSetting = convertToConcreteSettingIfNecessary(randomFrom(IP_FILTER_SETTINGS)); + request = prepareClusterUpdateSettingsRequest(transientSetting, persistentSetting); + violation = operatorOnlyRegistry.check(ClusterUpdateSettingsAction.NAME, request); + assertThat(violation.message(), containsString(String.format("setting [%s]", persistentSetting.getKey()))); + break; + case 3: + transientSetting = convertToConcreteSettingIfNecessary(randomFrom(DYNAMIC_SETTINGS)); + persistentSetting = convertToConcreteSettingIfNecessary(randomFrom(DYNAMIC_SETTINGS)); + request = prepareClusterUpdateSettingsRequest(transientSetting, persistentSetting); + assertNull(operatorOnlyRegistry.check(ClusterUpdateSettingsAction.NAME, request)); + break; + } + + } + + private Setting convertToConcreteSettingIfNecessary(Setting setting) { + if (setting instanceof Setting.AffixSetting) { + return ((Setting.AffixSetting) setting).getConcreteSettingForNamespace(randomAlphaOfLengthBetween(4, 8)); + } else { + return setting; + } + } + + private ClusterUpdateSettingsRequest prepareClusterUpdateSettingsRequest(Setting transientSetting, Setting persistentSetting) { + final ClusterUpdateSettingsRequest request = mock(ClusterUpdateSettingsRequest.class); + when(request.transientSettings()).thenReturn(Settings.builder().put(transientSetting.getKey(), "null").build()); + when(request.persistentSettings()).thenReturn(Settings.builder().put(persistentSetting.getKey(), "null").build()); + return request; + } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesTests.java index d51f18377cd7b..aa0a6e4b128cd 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.security.operator; import org.elasticsearch.ElasticsearchSecurityException; +import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.license.XPackLicenseState; @@ -24,6 +25,7 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; @@ -32,12 +34,14 @@ public class OperatorPrivilegesTests extends ESTestCase { private XPackLicenseState xPackLicenseState; private FileOperatorUsersStore fileOperatorUsersStore; private OperatorOnlyRegistry operatorOnlyRegistry; + private OperatorPrivilegesService operatorPrivilegesService; @Before public void init() { xPackLicenseState = mock(XPackLicenseState.class); fileOperatorUsersStore = mock(FileOperatorUsersStore.class); operatorOnlyRegistry = mock(OperatorOnlyRegistry.class); + operatorPrivilegesService = new DefaultOperatorPrivilegesService(xPackLicenseState, fileOperatorUsersStore, operatorOnlyRegistry); } public void testWillNotProcessWhenFeatureIsDisabledOrLicenseDoesNotSupport() { @@ -45,9 +49,6 @@ public void testWillNotProcessWhenFeatureIsDisabledOrLicenseDoesNotSupport() { .put("xpack.security.operator_privileges.enabled", randomBoolean()) .build(); when(xPackLicenseState.checkFeature(XPackLicenseState.Feature.OPERATOR_PRIVILEGES)).thenReturn(false); - - final OperatorPrivilegesService operatorPrivilegesService = - new DefaultOperatorPrivilegesService(xPackLicenseState, fileOperatorUsersStore, operatorOnlyRegistry); final ThreadContext threadContext = new ThreadContext(settings); operatorPrivilegesService.maybeMarkOperatorUser(mock(Authentication.class), threadContext); @@ -69,9 +70,6 @@ public void testMarkOperatorUser() { when(operatorAuth.getUser()).thenReturn(new User("operator_user")); when(fileOperatorUsersStore.isOperatorUser(operatorAuth)).thenReturn(true); when(fileOperatorUsersStore.isOperatorUser(nonOperatorAuth)).thenReturn(false); - - final OperatorPrivilegesService operatorPrivilegesService = - new DefaultOperatorPrivilegesService(xPackLicenseState, fileOperatorUsersStore, operatorOnlyRegistry); ThreadContext threadContext = new ThreadContext(settings); operatorPrivilegesService.maybeMarkOperatorUser(operatorAuth, threadContext); @@ -94,11 +92,8 @@ public void testCheck() { final String message = "[" + operatorAction + "]"; when(operatorOnlyRegistry.check(eq(operatorAction), any())).thenReturn(() -> message); when(operatorOnlyRegistry.check(eq(nonOperatorAction), any())).thenReturn(null); - - final OperatorPrivilegesService operatorPrivilegesService = - new DefaultOperatorPrivilegesService(xPackLicenseState, fileOperatorUsersStore, operatorOnlyRegistry); - ThreadContext threadContext = new ThreadContext(settings); + if (randomBoolean()) { threadContext.putHeader(AuthenticationField.PRIVILEGE_CATEGORY_KEY, AuthenticationField.PRIVILEGE_CATEGORY_VALUE_OPERATOR); assertNull(operatorPrivilegesService.check(operatorAction, mock(TransportRequest.class), threadContext)); @@ -112,6 +107,22 @@ public void testCheck() { assertNull(operatorPrivilegesService.check(nonOperatorAction, mock(TransportRequest.class), threadContext)); } + public void testMaybeInterceptRequest() { + final boolean licensed = randomBoolean(); + when(xPackLicenseState.checkFeature(XPackLicenseState.Feature.OPERATOR_PRIVILEGES)).thenReturn(licensed); + + final RestoreSnapshotRequest restoreSnapshotRequest = mock(RestoreSnapshotRequest.class); + operatorPrivilegesService.maybeInterceptRequest(new ThreadContext(Settings.EMPTY), restoreSnapshotRequest); + + verify(restoreSnapshotRequest).skipOperatorOnly(licensed); + } + + public void testMaybeInterceptRequestWillNotInterceptRequestsOtherThanRestoreSnapshotRequest() { + final TransportRequest transportRequest = mock(TransportRequest.class); + operatorPrivilegesService.maybeInterceptRequest(new ThreadContext(Settings.EMPTY), transportRequest); + verifyZeroInteractions(xPackLicenseState); + } + public void testNoOpService() { final Authentication authentication = mock(Authentication.class); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); @@ -125,4 +136,14 @@ public void testNoOpService() { verifyZeroInteractions(request); } + public void testNoOpServiceMaybeInterceptRequest() { + final RestoreSnapshotRequest restoreSnapshotRequest = mock(RestoreSnapshotRequest.class); + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + NOOP_OPERATOR_PRIVILEGES_SERVICE.maybeInterceptRequest(threadContext, restoreSnapshotRequest); + verify(restoreSnapshotRequest).skipOperatorOnly(false); + + // The test just makes sure that other requests are also accepted without any error + NOOP_OPERATOR_PRIVILEGES_SERVICE.maybeInterceptRequest(threadContext, mock(TransportRequest.class)); + } + } From 2b9aea91628183a5d072592cd89192539bef4d28 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Mon, 1 Feb 2021 17:33:22 +1100 Subject: [PATCH 08/16] forbidden api fix --- .../security/operator/OperatorOnlyRegistryTests.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java index 8dc93e44e1efa..c178dc4933d3c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java @@ -15,6 +15,7 @@ import org.junit.Before; import java.util.HashSet; +import java.util.Locale; import java.util.Set; import java.util.stream.Collectors; @@ -82,7 +83,7 @@ public void testOperatorOnlySettings() { randomValueOtherThan(transientSetting, () -> randomFrom(IP_FILTER_SETTINGS))); request = prepareClusterUpdateSettingsRequest(transientSetting, persistentSetting); violation = operatorOnlyRegistry.check(ClusterUpdateSettingsAction.NAME, request); - assertThat(violation.message(), containsString(String.format("settings [%s,%s]", + assertThat(violation.message(), containsString(String.format(Locale.ROOT, "settings [%s,%s]", transientSetting.getKey(), persistentSetting.getKey()))); break; case 1: @@ -90,14 +91,16 @@ public void testOperatorOnlySettings() { persistentSetting = convertToConcreteSettingIfNecessary(randomFrom(DYNAMIC_SETTINGS)); request = prepareClusterUpdateSettingsRequest(transientSetting, persistentSetting); violation = operatorOnlyRegistry.check(ClusterUpdateSettingsAction.NAME, request); - assertThat(violation.message(), containsString(String.format("setting [%s]", transientSetting.getKey()))); + assertThat(violation.message(), containsString(String.format(Locale.ROOT, "setting [%s]", + transientSetting.getKey()))); break; case 2: transientSetting = convertToConcreteSettingIfNecessary(randomFrom(DYNAMIC_SETTINGS)); persistentSetting = convertToConcreteSettingIfNecessary(randomFrom(IP_FILTER_SETTINGS)); request = prepareClusterUpdateSettingsRequest(transientSetting, persistentSetting); violation = operatorOnlyRegistry.check(ClusterUpdateSettingsAction.NAME, request); - assertThat(violation.message(), containsString(String.format("setting [%s]", persistentSetting.getKey()))); + assertThat(violation.message(), containsString(String.format(Locale.ROOT, "setting [%s]", + persistentSetting.getKey()))); break; case 3: transientSetting = convertToConcreteSettingIfNecessary(randomFrom(DYNAMIC_SETTINGS)); From 8c798074ec4351700ae05b8feac73009687ff12e Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 4 Feb 2021 14:38:33 +1100 Subject: [PATCH 09/16] Update server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java Co-authored-by: Tim Vernum --- .../admin/cluster/snapshots/restore/RestoreSnapshotRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index 6fe4fcdbfa564..5e10fe9d91b31 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -61,7 +61,7 @@ public class RestoreSnapshotRequest extends MasterNodeRequest Date: Thu, 4 Feb 2021 14:40:40 +1100 Subject: [PATCH 10/16] Address feedback --- .../common/settings/Setting.java | 2 +- .../snapshots/RestoreService.java | 2 +- .../common/settings/SettingTests.java | 2 +- .../OperatorPrivilegesSingleNodeTests.java | 32 +++++++++++-------- .../operator/OperatorOnlyRegistry.java | 2 +- .../operator/OperatorOnlyRegistryTests.java | 2 +- 6 files changed, 24 insertions(+), 18 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index 542f84ce90585..af693628102ff 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -299,7 +299,7 @@ public final boolean isDynamic() { /** * Returns true if this setting is dynamically updateable by operators, otherwise false */ - public final boolean isDynamicOperator() { + public final boolean isOperatorOnly() { return properties.contains(Property.OperatorDynamic); } diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index 9b8f17fcf198e..cd99c1207d41f 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -436,7 +436,7 @@ restoreUUID, snapshot, overallState(RestoreInProgress.State.INIT, shards), settings.keySet().stream(), currentState.metadata().persistentSettings().keySet().stream()) .filter(k -> { final Setting setting = clusterSettings.get(k); - return setting != null && setting.isDynamicOperator(); + return setting != null && setting.isOperatorOnly(); }) .collect(Collectors.toSet()); if (false == operatorSettingKeys.isEmpty()) { diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java index 71c1847901efc..4a3f1368045ae 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -1251,6 +1251,6 @@ public void testDynamicTest() { final Property property = randomFrom(Property.Dynamic, Property.OperatorDynamic); final Setting setting = Setting.simpleString("foo.bar", property); assertTrue(setting.isDynamic()); - assertEquals(setting.isDynamicOperator(), property == Property.OperatorDynamic); + assertEquals(setting.isOperatorOnly(), property == Property.OperatorDynamic); } } diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesSingleNodeTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesSingleNodeTests.java index c6c8985654123..657bca473590e 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesSingleNodeTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesSingleNodeTests.java @@ -68,13 +68,15 @@ protected Settings nodeSettings() { return builder.build(); } - public void testOperatorOnlyActionOrSettingWillNotBeActionableByNormalSuperuser() { - final Client client = client(); + public void testNormalSuperuserWillFailToCallOperatorOnlyAction() { final ClearVotingConfigExclusionsRequest clearVotingConfigExclusionsRequest = new ClearVotingConfigExclusionsRequest(); - ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, - () -> client.execute(ClearVotingConfigExclusionsAction.INSTANCE, clearVotingConfigExclusionsRequest).actionGet()); + final ElasticsearchSecurityException e = expectThrows( + ElasticsearchSecurityException.class, + () -> client().execute(ClearVotingConfigExclusionsAction.INSTANCE, clearVotingConfigExclusionsRequest).actionGet()); assertThat(e.getCause().getMessage(), containsString("Operator privileges are required for action")); + } + public void testNormalSuperuserWillFailToSetOperatorOnlySettings() { final Settings settings = Settings.builder().put(IPFilter.IP_FILTER_ENABLED_SETTING.getKey(), "null").build(); final ClusterUpdateSettingsRequest clusterUpdateSettingsRequest = new ClusterUpdateSettingsRequest(); if (randomBoolean()) { @@ -82,18 +84,19 @@ public void testOperatorOnlyActionOrSettingWillNotBeActionableByNormalSuperuser( } else { clusterUpdateSettingsRequest.persistentSettings(settings); } - e = expectThrows(ElasticsearchSecurityException.class, - () -> client.execute(ClusterUpdateSettingsAction.INSTANCE, clusterUpdateSettingsRequest).actionGet()); + final ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, + () -> client().execute(ClusterUpdateSettingsAction.INSTANCE, clusterUpdateSettingsRequest).actionGet()); assertThat(e.getCause().getMessage(), containsString("Operator privileges are required for setting")); } - public void testOperatorUserWillSucceedToCallOperatorOnlyActionOrSetting() { - final Client client = client().filterWithHeader(Map.of( - "Authorization", - basicAuthHeaderValue(OPERATOR_USER_NAME, new SecureString(TEST_PASSWORD.toCharArray())))); + public void testOperatorUserWillSucceedToCallOperatorOnlyAction() { + final Client client = createOperatorClient(); final ClearVotingConfigExclusionsRequest clearVotingConfigExclusionsRequest = new ClearVotingConfigExclusionsRequest(); client.execute(ClearVotingConfigExclusionsAction.INSTANCE, clearVotingConfigExclusionsRequest).actionGet(); + } + public void testOperatorUserWillSucceedToSetOperatorOnlySettings() { + final Client client = createOperatorClient(); final ClusterUpdateSettingsRequest clusterUpdateSettingsRequest = new ClusterUpdateSettingsRequest(); final Settings settings = Settings.builder().put(IPFilter.IP_FILTER_ENABLED_SETTING.getKey(), false).build(); final boolean useTransientSetting = randomBoolean(); @@ -117,12 +120,15 @@ public void testOperatorUserWillSucceedToCallOperatorOnlyActionOrSetting() { } public void testOperatorUserIsStillSubjectToRoleLimits() { - final Client client = client().filterWithHeader(Map.of( - "Authorization", - basicAuthHeaderValue(OPERATOR_USER_NAME, new SecureString(TEST_PASSWORD.toCharArray())))); + final Client client = createOperatorClient(); final GetUsersRequest getUsersRequest = new GetUsersRequest(); final ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, () -> client.execute(GetUsersAction.INSTANCE, getUsersRequest).actionGet()); assertThat(e.getMessage(), containsString("is unauthorized for user")); } + + private Client createOperatorClient() { + return client().filterWithHeader(Map.of("Authorization", + basicAuthHeaderValue(OPERATOR_USER_NAME, new SecureString(TEST_PASSWORD.toCharArray())))); + } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java index 2413bf0b7b676..6ec5ea7404e88 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java @@ -62,7 +62,7 @@ private OperatorPrivilegesViolation checkClusterUpdateSettings(ClusterUpdateSett request.transientSettings().keySet().stream(), request.persistentSettings().keySet().stream() ).filter(k -> { final Setting setting = clusterSettings.get(k); - return setting != null && setting.isDynamicOperator(); + return setting != null && setting.isOperatorOnly(); }).collect(Collectors.toList()); if (false == operatorOnlySettingKeys.isEmpty()) { return () -> (operatorOnlySettingKeys.size() == 1 ? "setting" : "settings") diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java index deba8cd5fd0db..69618d33f2aa6 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistryTests.java @@ -36,7 +36,7 @@ public class OperatorOnlyRegistryTests extends ESTestCase { private static final Set> DYNAMIC_SETTINGS = ClusterSettings.BUILT_IN_CLUSTER_SETTINGS.stream() .filter(Setting::isDynamic) - .filter(setting -> false == setting.isDynamicOperator()) + .filter(setting -> false == setting.isOperatorOnly()) .collect(Collectors.toSet()); private static final Set> IP_FILTER_SETTINGS = Set.of( IP_FILTER_ENABLED_HTTP_SETTING, From 45b6e4d18ab262afb40bef3889493e2f791d99f3 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 4 Feb 2021 14:43:11 +1100 Subject: [PATCH 11/16] fix names --- .../cluster/snapshots/restore/RestoreSnapshotRequest.java | 8 ++++---- .../java/org/elasticsearch/snapshots/RestoreService.java | 2 +- .../snapshots/restore/RestoreSnapshotRequestTests.java | 6 +++--- .../xpack/security/operator/OperatorPrivileges.java | 4 ++-- .../xpack/security/operator/OperatorPrivilegesTests.java | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index 5d7c2e35ee327..a075b607ce67a 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -438,12 +438,12 @@ public String snapshotUuid() { return snapshotUuid; } - public boolean skipOperatorOnly() { - return skipOperatorOnly; + public boolean skipOperatorOnlyState() { + return skipOperatorOnlyState; } - public void skipOperatorOnly(boolean skipOperatorOnly) { - this.skipOperatorOnly = skipOperatorOnly; + public void skipOperatorOnlyState(boolean skipOperatorOnlyState) { + this.skipOperatorOnlyState = skipOperatorOnlyState; } /** diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index cd99c1207d41f..f79540a3e986a 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -430,7 +430,7 @@ restoreUUID, snapshot, overallState(RestoreInProgress.State.INIT, shards), if (metadata.persistentSettings() != null) { Settings settings = metadata.persistentSettings(); clusterSettings.validateUpdate(settings); - if (request.skipOperatorOnly()) { + if (request.skipOperatorOnlyState()) { // Skip any operator-only settings from the snapshot. This happens when operator privileges are enabled Set operatorSettingKeys = Stream.concat( settings.keySet().stream(), currentState.metadata().persistentSettings().keySet().stream()) diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java index 4e3798b6b27c7..f89ce8d116022 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java @@ -127,9 +127,9 @@ public void testSource() throws IOException { public void testSkipOperatorOnlyWillNotBeSerialised() throws IOException { RestoreSnapshotRequest original = createTestInstance(); - assertFalse(original.skipOperatorOnly()); // default is false + assertFalse(original.skipOperatorOnlyState()); // default is false if (randomBoolean()) { - original.skipOperatorOnly(true); + original.skipOperatorOnlyState(true); } // It is not serialised as xcontent XContentBuilder builder = original.toXContent(XContentFactory.jsonBuilder(), new ToXContent.MapParams(Collections.emptyMap())); @@ -142,6 +142,6 @@ public void testSkipOperatorOnlyWillNotBeSerialised() throws IOException { final BytesStreamOutput streamOutput = new BytesStreamOutput(); original.writeTo(streamOutput); final RestoreSnapshotRequest deserialized = new RestoreSnapshotRequest(streamOutput.bytes().streamInput()); - assertFalse(deserialized.skipOperatorOnly()); + assertFalse(deserialized.skipOperatorOnlyState()); } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java index abd1178554a89..bb8967d78c58c 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorPrivileges.java @@ -87,7 +87,7 @@ public ElasticsearchSecurityException check(String action, TransportRequest requ public void maybeInterceptRequest(ThreadContext threadContext, TransportRequest request) { if (request instanceof RestoreSnapshotRequest) { - ((RestoreSnapshotRequest) request).skipOperatorOnly(shouldProcess()); + ((RestoreSnapshotRequest) request).skipOperatorOnlyState(shouldProcess()); } } @@ -109,7 +109,7 @@ public ElasticsearchSecurityException check(String action, TransportRequest requ @Override public void maybeInterceptRequest(ThreadContext threadContext, TransportRequest request) { if (request instanceof RestoreSnapshotRequest) { - ((RestoreSnapshotRequest) request).skipOperatorOnly(false); + ((RestoreSnapshotRequest) request).skipOperatorOnlyState(false); } } }; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesTests.java index 53eaf4bcc0f5c..45512fce7796e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesTests.java @@ -115,7 +115,7 @@ public void testMaybeInterceptRequest() { final RestoreSnapshotRequest restoreSnapshotRequest = mock(RestoreSnapshotRequest.class); operatorPrivilegesService.maybeInterceptRequest(new ThreadContext(Settings.EMPTY), restoreSnapshotRequest); - verify(restoreSnapshotRequest).skipOperatorOnly(licensed); + verify(restoreSnapshotRequest).skipOperatorOnlyState(licensed); } public void testMaybeInterceptRequestWillNotInterceptRequestsOtherThanRestoreSnapshotRequest() { @@ -141,7 +141,7 @@ public void testNoOpServiceMaybeInterceptRequest() { final RestoreSnapshotRequest restoreSnapshotRequest = mock(RestoreSnapshotRequest.class); final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); NOOP_OPERATOR_PRIVILEGES_SERVICE.maybeInterceptRequest(threadContext, restoreSnapshotRequest); - verify(restoreSnapshotRequest).skipOperatorOnly(false); + verify(restoreSnapshotRequest).skipOperatorOnlyState(false); // The test just makes sure that other requests are also accepted without any error NOOP_OPERATOR_PRIVILEGES_SERVICE.maybeInterceptRequest(threadContext, mock(TransportRequest.class)); From d51630fbee51481cf8dd5c0e85fa1839724e5a5c Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 4 Feb 2021 14:44:21 +1100 Subject: [PATCH 12/16] more names --- .../cluster/snapshots/restore/RestoreSnapshotRequest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index a075b607ce67a..41492fd34a5f0 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -564,13 +564,13 @@ public boolean equals(Object o) { Objects.equals(indexSettings, that.indexSettings) && Arrays.equals(ignoreIndexSettings, that.ignoreIndexSettings) && Objects.equals(snapshotUuid, that.snapshotUuid) && - skipOperatorOnly == that.skipOperatorOnly; + skipOperatorOnlyState == that.skipOperatorOnlyState; } @Override public int hashCode() { int result = Objects.hash(snapshot, repository, indicesOptions, renamePattern, renameReplacement, waitForCompletion, - includeGlobalState, partial, includeAliases, indexSettings, snapshotUuid, skipOperatorOnly); + includeGlobalState, partial, includeAliases, indexSettings, snapshotUuid, skipOperatorOnlyState); result = 31 * result + Arrays.hashCode(indices); result = 31 * result + Arrays.hashCode(ignoreIndexSettings); return result; From cb8e8a26966222a783ec01bd42d61120e84d87c0 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 4 Feb 2021 16:05:09 +1100 Subject: [PATCH 13/16] address more feedback --- .../restore/RestoreSnapshotRequest.java | 20 ++++-- .../restore/RestoreSnapshotRequestTests.java | 7 +++ .../operator/OperatorPrivilegesIT.java | 1 - ...eratorPrivilegesDisabledIntegTestCase.java | 13 ++++ .../20_operator_privileges_disabled.yml | 63 +++++++++++++++++++ 5 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesDisabledIntegTestCase.java create mode 100644 x-pack/plugin/src/test/resources/rest-api-spec/test/snapshot/20_operator_privileges_disabled.yml diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index 41492fd34a5f0..f2d412d7baab9 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -50,7 +50,9 @@ public class RestoreSnapshotRequest extends MasterNodeRequest source) { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); + toXContentFragment(builder, params); + builder.endObject(); + return builder; + } + + private void toXContentFragment(XContentBuilder builder, Params params) throws IOException { builder.startArray("indices"); for (String index : indices) { builder.value(index); @@ -537,8 +545,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.value(ignoreIndexSetting); } builder.endArray(); - builder.endObject(); - return builder; } @Override @@ -578,6 +584,12 @@ public int hashCode() { @Override public String toString() { - return Strings.toString(this); + return Strings.toString((ToXContentObject) (builder, params) -> { + builder.startObject(); + toXContentFragment(builder, params); + builder.field("skipOperatorOnlyState", skipOperatorOnlyState); + builder.endObject(); + return builder; + }); } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java index f89ce8d116022..1cbc29124eb17 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java @@ -30,6 +30,8 @@ import java.util.List; import java.util.Map; +import static org.hamcrest.Matchers.containsString; + public class RestoreSnapshotRequestTests extends AbstractWireSerializingTestCase { private RestoreSnapshotRequest randomState(RestoreSnapshotRequest instance) { if (randomBoolean()) { @@ -144,4 +146,9 @@ public void testSkipOperatorOnlyWillNotBeSerialised() throws IOException { final RestoreSnapshotRequest deserialized = new RestoreSnapshotRequest(streamOutput.bytes().streamInput()); assertFalse(deserialized.skipOperatorOnlyState()); } + + public void testToStringWillIncludeSkipOperatorOnlyState() { + RestoreSnapshotRequest original = createTestInstance(); + assertThat(original.toString(), containsString("skipOperatorOnlyState")); + } } diff --git a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java index f069f1671bd36..cce019b97ecdf 100644 --- a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java +++ b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java @@ -127,7 +127,6 @@ public void testUpdateOperatorSettings() throws IOException { } } - @SuppressWarnings("unchecked") public void testSnapshotRestoreBehaviourOfOperatorSettings() throws IOException { final String repoName = "repo"; final String snapshotName = "snap"; diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesDisabledIntegTestCase.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesDisabledIntegTestCase.java new file mode 100644 index 0000000000000..82b6f50b8bfc3 --- /dev/null +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesDisabledIntegTestCase.java @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.security.operator; + +import org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase; + +public class OperatorPrivilegesDisabledIntegTestCase extends AbstractSnapshotIntegTestCase { +} diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/snapshot/20_operator_privileges_disabled.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/snapshot/20_operator_privileges_disabled.yml new file mode 100644 index 0000000000000..6487888b03a4c --- /dev/null +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/snapshot/20_operator_privileges_disabled.yml @@ -0,0 +1,63 @@ +--- +setup: + + - do: + snapshot.create_repository: + repository: test_repo_restore_1 + body: + type: source + settings: + delegate_type: fs + location: "test_repo_restore_1_loc" + + - do: + cluster.health: + wait_for_status: green + +--- +"Operator only settings can be set and restored by non-operator user when operator privileges is disabled": + - skip: + features: ["allowed_warnings"] + + - do: + cluster.put_settings: + body: + persistent: + xpack.security.http.filter.deny: example.com + xpack.security.transport.filter.deny: example.com + + - do: + snapshot.create: + repository: test_repo_restore_1 + snapshot: test_snapshot + wait_for_completion: true + body: | + { "include_global_state": true } + + - match: { snapshot.snapshot: test_snapshot } + - match: { snapshot.state : SUCCESS } + - is_true: snapshot.include_global_state + - is_true: snapshot.version + - gt: { snapshot.version_id: 0} + + - do: + cluster.put_settings: + body: + persistent: + xpack.security.http.filter.deny: tutorial.com + xpack.security.transport.filter.deny: tutorial.com + + - do: + snapshot.restore: + repository: test_repo_restore_1 + snapshot: test_snapshot + wait_for_completion: true + body: | + { "include_global_state": true } + + - do: + cluster.get_settings: {} + + - match: { persistent.xpack.security.http.filter.deny: "example.com" } + - match: { persistent.xpack.security.transport.filter.deny: "example.com" } + From 6d5ad5210795fb88e4c52ff8675cfbef0b1112bf Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 4 Feb 2021 16:25:10 +1100 Subject: [PATCH 14/16] more feedback --- .../elasticsearch/snapshots/RestoreService.java | 2 +- .../restore/RestoreSnapshotRequestTests.java | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index f79540a3e986a..a73cc6d35e822 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -429,7 +429,6 @@ restoreUUID, snapshot, overallState(RestoreInProgress.State.INIT, shards), if (request.includeGlobalState()) { if (metadata.persistentSettings() != null) { Settings settings = metadata.persistentSettings(); - clusterSettings.validateUpdate(settings); if (request.skipOperatorOnlyState()) { // Skip any operator-only settings from the snapshot. This happens when operator privileges are enabled Set operatorSettingKeys = Stream.concat( @@ -446,6 +445,7 @@ restoreUUID, snapshot, overallState(RestoreInProgress.State.INIT, shards), .build(); } } + clusterSettings.validateUpdate(settings); mdBuilder.persistentSettings(settings); } if (metadata.templates() != null) { diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java index 1cbc29124eb17..a8a52931ad888 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java @@ -133,13 +133,14 @@ public void testSkipOperatorOnlyWillNotBeSerialised() throws IOException { if (randomBoolean()) { original.skipOperatorOnlyState(true); } + Map map = convertRequestToMap(original); // It is not serialised as xcontent - XContentBuilder builder = original.toXContent(XContentFactory.jsonBuilder(), new ToXContent.MapParams(Collections.emptyMap())); - XContentParser parser = XContentType.JSON.xContent().createParser( - NamedXContentRegistry.EMPTY, null, BytesReference.bytes(builder).streamInput()); - Map map = parser.mapOrdered(); assertFalse(map.containsKey("skip_operator_only")); + // Xcontent is not affected by the value of skipOperatorOnlyState + original.skipOperatorOnlyState(original.skipOperatorOnlyState() == false); + assertEquals(map, convertRequestToMap(original)); + // Nor does it serialise to streamInput final BytesStreamOutput streamOutput = new BytesStreamOutput(); original.writeTo(streamOutput); @@ -151,4 +152,11 @@ public void testToStringWillIncludeSkipOperatorOnlyState() { RestoreSnapshotRequest original = createTestInstance(); assertThat(original.toString(), containsString("skipOperatorOnlyState")); } + + private Map convertRequestToMap(RestoreSnapshotRequest request) throws IOException { + XContentBuilder builder = request.toXContent(XContentFactory.jsonBuilder(), new ToXContent.MapParams(Collections.emptyMap())); + XContentParser parser = XContentType.JSON.xContent().createParser( + NamedXContentRegistry.EMPTY, null, BytesReference.bytes(builder).streamInput()); + return parser.mapOrdered(); + } } From c9406afa81c51524bf82b7e209d42678a61e9292 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 4 Feb 2021 16:57:30 +1100 Subject: [PATCH 15/16] fix test --- .../20_operator_privileges_disabled.yml | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/snapshot/20_operator_privileges_disabled.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/snapshot/20_operator_privileges_disabled.yml index 6487888b03a4c..ccf5eb1c3df94 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/snapshot/20_operator_privileges_disabled.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/snapshot/20_operator_privileges_disabled.yml @@ -3,12 +3,11 @@ setup: - do: snapshot.create_repository: - repository: test_repo_restore_1 + repository: test_repo_restore_2 body: - type: source + type: fs settings: - delegate_type: fs - location: "test_repo_restore_1_loc" + location: "test_repo_restore_2_loc" - do: cluster.health: @@ -28,13 +27,13 @@ setup: - do: snapshot.create: - repository: test_repo_restore_1 - snapshot: test_snapshot + repository: test_repo_restore_2 + snapshot: test_snapshot_2 wait_for_completion: true body: | { "include_global_state": true } - - match: { snapshot.snapshot: test_snapshot } + - match: { snapshot.snapshot: test_snapshot_2 } - match: { snapshot.state : SUCCESS } - is_true: snapshot.include_global_state - is_true: snapshot.version @@ -49,8 +48,8 @@ setup: - do: snapshot.restore: - repository: test_repo_restore_1 - snapshot: test_snapshot + repository: test_repo_restore_2 + snapshot: test_snapshot_2 wait_for_completion: true body: | { "include_global_state": true } From 67840a905a814cab494ca86362cd34a8f7701273 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 4 Feb 2021 17:08:56 +1100 Subject: [PATCH 16/16] add teardown --- .../test/snapshot/20_operator_privileges_disabled.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/snapshot/20_operator_privileges_disabled.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/snapshot/20_operator_privileges_disabled.yml index ccf5eb1c3df94..70925dff2e765 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/snapshot/20_operator_privileges_disabled.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/snapshot/20_operator_privileges_disabled.yml @@ -13,6 +13,17 @@ setup: cluster.health: wait_for_status: green +--- +teardown: + + - do: + snapshot.delete: + repository: test_repo_restore_2 + snapshot: test_snapshot_2 + - do: + snapshot.delete_repository: + repository: test_repo_restore_2 + --- "Operator only settings can be set and restored by non-operator user when operator privileges is disabled": - skip: