From 76d0e2fcba9b9fdfa225e567fd954e850a6db804 Mon Sep 17 00:00:00 2001 From: Patrick Doyle <810052+prdoyle@users.noreply.github.com> Date: Tue, 2 Jul 2024 09:19:50 -0400 Subject: [PATCH 1/3] parseHeapRatioOrDeprecatedByteSizeValue for indices.breaker.total.limit (#110236) * parseHeapRatioOrDeprecatedByteSizeValue for indices.breaker.total.limit * Fix tests for indices.breaker.total.limit warning * Spotless * Warn if below minimum percent * Update docs/changelog/110236.yaml * Changelog * Pick correct area for changelog entry * Spotless again dammit * assertCriticalWarnings in circuit breaker test * Expect another warning --- docs/changelog/110236.yaml | 21 ++++++++ .../common/unit/MemorySizeValue.java | 54 +++++++++++++++---- .../HierarchyCircuitBreakerService.java | 4 +- .../settings/MemorySizeSettingsTests.java | 5 ++ .../HierarchyCircuitBreakerServiceTests.java | 26 ++++++++- .../org/elasticsearch/test/ESTestCase.java | 4 +- .../sequence/CircuitBreakerTests.java | 5 +- 7 files changed, 104 insertions(+), 15 deletions(-) create mode 100644 docs/changelog/110236.yaml diff --git a/docs/changelog/110236.yaml b/docs/changelog/110236.yaml new file mode 100644 index 0000000000000..e2dbff7fbf768 --- /dev/null +++ b/docs/changelog/110236.yaml @@ -0,0 +1,21 @@ +pr: 110236 +summary: '`ParseHeapRatioOrDeprecatedByteSizeValue` for `indices.breaker.total.limit`' +area: Infra/Settings +type: deprecation +issues: [] +deprecation: + title: 'Deprecate absolute size values for `indices.breaker.total.limit` setting' + area: Cluster and node setting + details: Previously, the value of `indices.breaker.total.limit` could be specified as + an absolute size in bytes. This setting controls the overal amount of + memory the server is allowed to use before taking remedial actions. Setting + this to a specific number of bytes led to strange behaviour when the node + maximum heap size changed because the circut breaker limit would remain + unchanged. This would either leave the value too low, causing part of the + heap to remain unused; or it would leave the value too high, causing the + circuit breaker to be ineffective at preventing OOM errors. The only + reasonable behaviour for this setting is that it scales with the size of + the heap, and so absolute byte limits are now deprecated. + impact: Users must change their configuration to specify a percentage instead of + an absolute number of bytes for `indices.breaker.total.limit`, or else + accept the default, which is already specified as a percentage. diff --git a/server/src/main/java/org/elasticsearch/common/unit/MemorySizeValue.java b/server/src/main/java/org/elasticsearch/common/unit/MemorySizeValue.java index bfe4e18367a74..274a4e67367c7 100644 --- a/server/src/main/java/org/elasticsearch/common/unit/MemorySizeValue.java +++ b/server/src/main/java/org/elasticsearch/common/unit/MemorySizeValue.java @@ -9,6 +9,9 @@ package org.elasticsearch.common.unit; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.cluster.routing.allocation.allocator.BalancedShardsAllocator; +import org.elasticsearch.common.logging.DeprecationCategory; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.monitor.jvm.JvmInfo; import java.util.Objects; @@ -25,18 +28,49 @@ public enum MemorySizeValue { public static ByteSizeValue parseBytesSizeValueOrHeapRatio(String sValue, String settingName) { settingName = Objects.requireNonNull(settingName); if (sValue != null && sValue.endsWith("%")) { - final String percentAsString = sValue.substring(0, sValue.length() - 1); - try { - final double percent = Double.parseDouble(percentAsString); - if (percent < 0 || percent > 100) { - throw new ElasticsearchParseException("percentage should be in [0-100], got [{}]", percentAsString); - } - return ByteSizeValue.ofBytes((long) ((percent / 100) * JvmInfo.jvmInfo().getMem().getHeapMax().getBytes())); - } catch (NumberFormatException e) { - throw new ElasticsearchParseException("failed to parse [{}] as a double", e, percentAsString); - } + return parseHeapRatio(sValue, settingName, 0); } else { return parseBytesSizeValue(sValue, settingName); } } + + public static ByteSizeValue parseHeapRatioOrDeprecatedByteSizeValue(String sValue, String settingName, double minHeapPercent) { + settingName = Objects.requireNonNull(settingName); + if (sValue != null && sValue.endsWith("%")) { + return parseHeapRatio(sValue, settingName, minHeapPercent); + } else { + DeprecationLogger.getLogger(BalancedShardsAllocator.class) + .critical( + DeprecationCategory.SETTINGS, + "absolute_size_not_supported", + "[{}] should be specified using a percentage of the heap. Absolute size settings will be forbidden in a future release", + settingName + ); + return parseBytesSizeValue(sValue, settingName); + } + } + + private static ByteSizeValue parseHeapRatio(String sValue, String settingName, double minHeapPercent) { + final String percentAsString = sValue.substring(0, sValue.length() - 1); + try { + final double percent = Double.parseDouble(percentAsString); + if (percent < 0 || percent > 100) { + throw new ElasticsearchParseException("percentage should be in [0-100], got [{}]", percentAsString); + } else if (percent < minHeapPercent) { + DeprecationLogger.getLogger(MemorySizeValue.class) + .warn( + DeprecationCategory.SETTINGS, + "memory_size_below_minimum", + "[{}] setting of [{}] is below the recommended minimum of {}% of the heap", + settingName, + sValue, + minHeapPercent + ); + } + return ByteSizeValue.ofBytes((long) ((percent / 100) * JvmInfo.jvmInfo().getMem().getHeapMax().getBytes())); + } catch (NumberFormatException e) { + throw new ElasticsearchParseException("failed to parse [{}] as a double", e, percentAsString); + } + } + } diff --git a/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java b/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java index a2e30b9e18098..d91b19fda1185 100644 --- a/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java +++ b/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java @@ -20,6 +20,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; +import org.elasticsearch.common.unit.MemorySizeValue; import org.elasticsearch.common.util.concurrent.ReleasableLock; import org.elasticsearch.core.Booleans; import org.elasticsearch.core.SuppressForbidden; @@ -65,7 +66,7 @@ public class HierarchyCircuitBreakerService extends CircuitBreakerService { Property.NodeScope ); - public static final Setting TOTAL_CIRCUIT_BREAKER_LIMIT_SETTING = Setting.memorySizeSetting( + public static final Setting TOTAL_CIRCUIT_BREAKER_LIMIT_SETTING = new Setting<>( "indices.breaker.total.limit", settings -> { if (USE_REAL_MEMORY_USAGE_SETTING.get(settings)) { @@ -74,6 +75,7 @@ public class HierarchyCircuitBreakerService extends CircuitBreakerService { return "70%"; } }, + (s) -> MemorySizeValue.parseHeapRatioOrDeprecatedByteSizeValue(s, "indices.breaker.total.limit", 50), Property.Dynamic, Property.NodeScope ); diff --git a/server/src/test/java/org/elasticsearch/common/settings/MemorySizeSettingsTests.java b/server/src/test/java/org/elasticsearch/common/settings/MemorySizeSettingsTests.java index 98da24fc75c96..5321079896b08 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/MemorySizeSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/MemorySizeSettingsTests.java @@ -71,6 +71,11 @@ public void testCircuitBreakerSettings() { "indices.breaker.total.limit", ByteSizeValue.ofBytes((long) (JvmInfo.jvmInfo().getMem().getHeapMax().getBytes() * defaultTotalPercentage)) ); + assertWarnings( + "[indices.breaker.total.limit] setting of [25%] is below the recommended minimum of 50.0% of the heap", + "[indices.breaker.total.limit] should be specified using a percentage of the heap. " + + "Absolute size settings will be forbidden in a future release" + ); assertMemorySizeSetting( HierarchyCircuitBreakerService.FIELDDATA_CIRCUIT_BREAKER_LIMIT_SETTING, "indices.breaker.fielddata.limit", diff --git a/server/src/test/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerServiceTests.java b/server/src/test/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerServiceTests.java index 8d2255df9e7e8..ff2f55c791dd3 100644 --- a/server/src/test/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerServiceTests.java +++ b/server/src/test/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerServiceTests.java @@ -254,6 +254,8 @@ public void testBorrowingSiblingBreakerMemory() { assertThat(exception.getMessage(), containsString("request=157286400/150mb")); assertThat(exception.getDurability(), equalTo(CircuitBreaker.Durability.TRANSIENT)); } + + assertCircuitBreakerLimitWarning(); } public void testParentBreaksOnRealMemoryUsage() { @@ -325,6 +327,8 @@ long currentMemoryUsage() { memoryUsage.set(100); requestBreaker.addEstimateBytesAndMaybeBreak(reservationInBytes, "request"); assertEquals(0, requestBreaker.getTrippedCount()); + + assertCircuitBreakerLimitWarning(); } /** @@ -749,6 +753,7 @@ public void testTrippedCircuitBreakerDurability() { equalTo(expectedDurability) ); } + assertCircuitBreakerLimitWarning(); } public void testAllocationBucketsBreaker() { @@ -785,6 +790,8 @@ public void testAllocationBucketsBreaker() { assertThat(exception.getMessage(), containsString("[parent] Data too large, data for [allocated_buckets] would be")); assertThat(exception.getMessage(), containsString("which is larger than the limit of [100/100b]")); } + + assertCircuitBreakerLimitWarning(); } public void testRegisterCustomCircuitBreakers_WithDuplicates() { @@ -891,7 +898,7 @@ public void testApplySettingForUpdatingUseRealMemory() { service.getParentLimit() ); - // total.limit defaults to 70% of the JVM heap if use_real_memory set to true + // total.limit defaults to 95% of the JVM heap if use_real_memory set to true clusterSettings.applySettings(Settings.builder().put(useRealMemoryUsageSetting, true).build()); assertEquals( MemorySizeValue.parseBytesSizeValueOrHeapRatio("95%", totalCircuitBreakerLimitSetting).getBytes(), @@ -900,6 +907,15 @@ public void testApplySettingForUpdatingUseRealMemory() { } } + public void testSizeBelowMinimumWarning() { + ByteSizeValue sizeValue = MemorySizeValue.parseHeapRatioOrDeprecatedByteSizeValue( + "19%", + HierarchyCircuitBreakerService.TOTAL_CIRCUIT_BREAKER_LIMIT_SETTING.getKey(), + 20 + ); + assertWarnings("[indices.breaker.total.limit] setting of [19%] is below the recommended minimum of 20.0% of the heap"); + } + public void testBuildParentTripMessage() { class TestChildCircuitBreaker extends NoopCircuitBreaker { private final long used; @@ -972,4 +988,12 @@ public double getOverhead() { HierarchyCircuitBreakerService.permitNegativeValues = false; } } + + void assertCircuitBreakerLimitWarning() { + assertWarnings( + "[indices.breaker.total.limit] should be specified using a percentage of the heap. " + + "Absolute size settings will be forbidden in a future release" + ); + + } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 0d20c613b27a8..ca6be72fd585b 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -649,7 +649,7 @@ protected final void assertSettingDeprecationsAndWarnings(final Setting[] set /** * Convenience method to assert warnings for settings deprecations and general deprecation warnings. All warnings passed to this method * are assumed to be at WARNING level. - * @param expectedWarnings expected general deprecation warnings. + * @param expectedWarnings expected general deprecation warning messages. */ protected final void assertWarnings(String... expectedWarnings) { assertWarnings( @@ -663,7 +663,7 @@ protected final void assertWarnings(String... expectedWarnings) { /** * Convenience method to assert warnings for settings deprecations and general deprecation warnings. All warnings passed to this method * are assumed to be at CRITICAL level. - * @param expectedWarnings expected general deprecation warnings. + * @param expectedWarnings expected general deprecation warning messages. */ protected final void assertCriticalWarnings(String... expectedWarnings) { assertWarnings( diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/sequence/CircuitBreakerTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/sequence/CircuitBreakerTests.java index 9293ffc40ec53..c001b312d5578 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/sequence/CircuitBreakerTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/sequence/CircuitBreakerTests.java @@ -245,7 +245,9 @@ public void testEqlCBCleanedUp_on_ParentCBBreak() { final int searchRequestsExpectedCount = 2; // let the parent circuit breaker fail, setting its limit to zero - Settings settings = Settings.builder().put(HierarchyCircuitBreakerService.TOTAL_CIRCUIT_BREAKER_LIMIT_SETTING.getKey(), 0).build(); + Settings settings = Settings.builder() + .put(HierarchyCircuitBreakerService.TOTAL_CIRCUIT_BREAKER_LIMIT_SETTING.getKey(), "0%") + .build(); try ( CircuitBreakerService service = new HierarchyCircuitBreakerService( @@ -277,6 +279,7 @@ public void testEqlCBCleanedUp_on_ParentCBBreak() { TumblingWindow window = new TumblingWindow(eqlClient, criteria, null, matcher, Collections.emptyList()); window.execute(wrap(p -> fail(), ex -> assertTrue(ex instanceof CircuitBreakingException))); } + assertCriticalWarnings("[indices.breaker.total.limit] setting of [0%] is below the recommended minimum of 50.0% of the heap"); } private List breakerSettings() { From 55476041d9daa4e1ccad2b732c3e4e5b0a785194 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20Fred=C3=A9n?= <109296772+jfreden@users.noreply.github.com> Date: Tue, 2 Jul 2024 15:45:39 +0200 Subject: [PATCH 2/3] Add BulkPutRoles API (#109339) * Add BulkPutRoles API --- docs/reference/rest-api/security.asciidoc | 2 + .../security/bulk-create-roles.asciidoc | 328 ++++++++++++++++ .../api/security.bulk_put_role.json | 43 +++ .../core/security/action/ActionTypes.java | 3 + .../role/BulkPutRoleRequestBuilder.java | 65 ++++ .../BulkPutRoleRequestBuilderFactory.java | 22 ++ .../action/role/BulkPutRolesRequest.java | 69 ++++ .../action/role/BulkPutRolesResponse.java | 145 +++++++ .../core/security/authz/RoleDescriptor.java | 17 +- .../authz/privilege/PrivilegeTests.java | 1 + .../xpack/security/operator/Constants.java | 1 + .../SecurityOnTrialLicenseRestTestCase.java | 59 +++ .../security/role/BulkPutRoleRestIT.java | 231 +++++++++++ .../role/RoleWithDescriptionRestIT.java | 69 +--- ...RoleWithRemoteIndicesPrivilegesRestIT.java | 36 +- .../RoleWithWorkflowsRestrictionRestIT.java | 22 +- .../xpack/security/Security.java | 31 +- .../role/TransportBulkPutRolesAction.java | 34 ++ .../action/role/TransportPutRoleAction.java | 57 +-- .../authz/store/NativeRolesStore.java | 260 ++++++++++--- .../action/role/RestBulkPutRolesAction.java | 56 +++ .../role/TransportPutRoleActionTests.java | 237 +----------- .../authz/store/NativeRolesStoreTests.java | 363 +++++++++++++++--- .../test/roles/60_bulk_roles.yml | 83 ++++ 24 files changed, 1724 insertions(+), 510 deletions(-) create mode 100644 docs/reference/rest-api/security/bulk-create-roles.asciidoc create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/api/security.bulk_put_role.json create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRoleRequestBuilder.java create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRoleRequestBuilderFactory.java create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRolesRequest.java create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRolesResponse.java create mode 100644 x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/BulkPutRoleRestIT.java create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportBulkPutRolesAction.java create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestBulkPutRolesAction.java create mode 100644 x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/roles/60_bulk_roles.yml diff --git a/docs/reference/rest-api/security.asciidoc b/docs/reference/rest-api/security.asciidoc index 4571d963179a6..80734ca51b989 100644 --- a/docs/reference/rest-api/security.asciidoc +++ b/docs/reference/rest-api/security.asciidoc @@ -45,6 +45,7 @@ Use the following APIs to add, remove, update, and retrieve role mappings: Use the following APIs to add, remove, update, and retrieve roles in the native realm: * <> +* <> * <> * <> * <> @@ -171,6 +172,7 @@ include::security/create-api-keys.asciidoc[] include::security/put-app-privileges.asciidoc[] include::security/create-role-mappings.asciidoc[] include::security/create-roles.asciidoc[] +include::security/bulk-create-roles.asciidoc[] include::security/create-users.asciidoc[] include::security/create-service-token.asciidoc[] include::security/delegate-pki-authentication.asciidoc[] diff --git a/docs/reference/rest-api/security/bulk-create-roles.asciidoc b/docs/reference/rest-api/security/bulk-create-roles.asciidoc new file mode 100644 index 0000000000000..a8072b7ba549a --- /dev/null +++ b/docs/reference/rest-api/security/bulk-create-roles.asciidoc @@ -0,0 +1,328 @@ +[role="xpack"] +[[security-api-bulk-put-role]] +=== Bulk create or update roles API +preview::[] +++++ +Bulk create or update roles API +++++ + +Bulk adds and updates roles in the native realm. + +[[security-api-bulk-put-role-request]] +==== {api-request-title} + +`POST /_security/role/` + + +[[security-api-bulk-put-role-prereqs]] +==== {api-prereq-title} + +* To use this API, you must have at least the `manage_security` cluster +privilege. + +[[security-api-bulk-put-role-desc]] +==== {api-description-title} + +The role management APIs are generally the preferred way to manage roles, rather than using +<>. The bulk create +or update roles API cannot update roles that are defined in roles files. + +[[security-api-bulk-put-role-path-params]] +==== {api-path-parms-title} + +`refresh`:: +Optional setting of the {ref}/docs-refresh.html[refresh policy] for the write request. Defaults to Immediate. + +[[security-api-bulk-put-role-request-body]] +==== {api-request-body-title} + +The following parameters can be specified in the body of a POST request +and pertain to adding a set of roles: + +`roles`:: +(object) The roles to add as a role name to role map. + +==== +`` (required):: (string) The role name. +`applications`:: (list) A list of application privilege entries. +`application` (required)::: (string) The name of the application to which this entry applies. +`privileges`::: (list) A list of strings, where each element is the name of an application +privilege or action. +`resources`::: (list) A list resources to which the privileges are applied. + +`cluster`:: (list) A list of cluster privileges. These privileges define the +cluster level actions that users with this role are able to execute. + +`global`:: (object) An object defining global privileges. A global privilege is +a form of cluster privilege that is request-aware. Support for global privileges +is currently limited to the management of application privileges. + +`indices`:: (list) A list of indices permissions entries. +`field_security`::: (object) The document fields that the owners of the role have +read access to. For more information, see +<>. +`names` (required)::: (list) A list of indices (or index name patterns) to which the +permissions in this entry apply. +`privileges`(required)::: (list) The index level privileges that the owners of the role +have on the specified indices. +`query`::: A search query that defines the documents the owners of the role have +read access to. A document within the specified indices must match this query in +order for it to be accessible by the owners of the role. + +`metadata`:: (object) Optional meta-data. Within the `metadata` object, keys +that begin with `_` are reserved for system usage. + +`run_as`:: (list) A list of users that the owners of this role can impersonate. +For more information, see +<>. + +`remote_indices`:: beta:[] (list) A list of remote indices permissions entries. ++ +-- +NOTE: Remote indices are effective for <>. +They have no effect for remote clusters configured with the <>. +-- +`clusters` (required)::: (list) A list of cluster aliases to which the permissions +in this entry apply. +`field_security`::: (object) The document fields that the owners of the role have +read access to. For more information, see +<>. +`names` (required)::: (list) A list of indices (or index name patterns) on the remote clusters +(specified with `clusters`) to which the permissions in this entry apply. +`privileges`(required)::: (list) The index level privileges that the owners of the role +have on the specified indices. +`query`::: A search query that defines the documents the owners of the role have +read access to. A document within the specified indices must match this query in +order for it to be accessible by the owners of the role. + +For more information, see <>. +==== + +[[security-bulk-api-put-role-example]] +==== {api-examples-title} + +The following example adds the roles called `my_admin_role` and `my_user_role`: + +[source,console] +-------------------------------------------------- +POST /_security/role +{ + "roles": { + "my_admin_role": { + "cluster": [ + "all" + ], + "indices": [ + { + "names": [ + "index1", + "index2" + ], + "privileges": [ + "all" + ], + "field_security": { + "grant": [ + "title", + "body" + ] + }, + "query": "{\"match\": {\"title\": \"foo\"}}" + } + ], + "applications": [ + { + "application": "myapp", + "privileges": [ + "admin", + "read" + ], + "resources": [ + "*" + ] + } + ], + "run_as": [ + "other_user" + ], + "metadata": { + "version": 1 + } + }, + "my_user_role": { + "cluster": [ + "all" + ], + "indices": [ + { + "names": [ + "index1" + ], + "privileges": [ + "read" + ], + "field_security": { + "grant": [ + "title", + "body" + ] + }, + "query": "{\"match\": {\"title\": \"foo\"}}" + } + ], + "applications": [ + { + "application": "myapp", + "privileges": [ + "admin", + "read" + ], + "resources": [ + "*" + ] + } + ], + "run_as": [ + "other_user" + ], + "metadata": { + "version": 1 + } + } + } +} +-------------------------------------------------- + +A successful call returns a JSON structure that shows whether the role has been +created, updated, or had no changes made. + +[source,console-result] +-------------------------------------------------- +{ + "created": [ <1> + "my_admin_role", <2> + "my_user_role" + ] +} +-------------------------------------------------- + +<1> Result type, one of `created`, `updated`, `noop`, `errors`. +<2> A list of the roles that were created. + +Because errors are handled individually for each role create or update, the API allows partial success. + +The following query would throw an error for `my_admin_role` because the privilege `bad_cluster_privilege` +doesn't exist, but would be successful for the `my_user_role`. + +[source,console] +-------------------------------------------------- +POST /_security/role +{ + "roles": { + "my_admin_role": { + "cluster": [ + "bad_cluster_privilege" + ], + "indices": [ + { + "names": [ + "index1", + "index2" + ], + "privileges": ["all"], + "field_security": { + "grant": [ + "title", + "body" + ] + }, + "query": "{\"match\": {\"title\": \"foo\"}}" + } + ], + "applications": [ + { + "application": "myapp", + "privileges": [ + "admin", + "read" + ], + "resources": [ + "*" + ] + } + ], + "run_as": [ + "other_user" + ], + "metadata": { + "version": 1 + } + }, + "my_user_role": { + "cluster": [ + "all" + ], + "indices": [ + { + "names": [ + "index1" + ], + "privileges": [ + "read" + ], + "field_security": { + "grant": [ + "title", + "body" + ] + }, + "query": "{\"match\": {\"title\": \"foo\"}}" + } + ], + "applications": [ + { + "application": "myapp", + "privileges": [ + "admin", + "read" + ], + "resources": [ + "*" + ] + } + ], + "run_as": [ + "other_user" + ], + "metadata": { + "version": 1 + } + } + } +} +-------------------------------------------------- + +The result would then have the `errors` field set to `true` and hold the error for the `my_admin_role` update. + + +[source,console-result] +-------------------------------------------------- +{ + "created": [ + "my_user_role" <1> + ], + "errors": { <2> + "count": 1, <3> + "details": { + "my_admin_role": { <4> + "type": "action_request_validation_exception", + "reason": "Validation Failed: 1: unknown cluster privilege [bad_cluster_privilege]. a privilege must be either one of the predefined cluster privilege names [manage_own_api_key,none,cancel_task,cross_cluster_replication,cross_cluster_search,delegate_pki,grant_api_key,manage_autoscaling,manage_index_templates,manage_logstash_pipelines,manage_oidc,manage_saml,manage_search_application,manage_search_query_rules,manage_search_synonyms,manage_service_account,manage_token,manage_user_profile,monitor_connector,monitor_data_stream_global_retention,monitor_enrich,monitor_inference,monitor_ml,monitor_rollup,monitor_snapshot,monitor_text_structure,monitor_watcher,post_behavioral_analytics_event,read_ccr,read_connector_secrets,read_fleet_secrets,read_ilm,read_pipeline,read_security,read_slm,transport_client,write_connector_secrets,write_fleet_secrets,create_snapshot,manage_behavioral_analytics,manage_ccr,manage_connector,manage_data_stream_global_retention,manage_enrich,manage_ilm,manage_inference,manage_ml,manage_rollup,manage_slm,manage_watcher,monitor_data_frame_transforms,monitor_transform,manage_api_key,manage_ingest_pipelines,manage_pipeline,manage_data_frame_transforms,manage_transform,manage_security,monitor,manage,all] or a pattern over one of the available cluster actions;" + } + } + } +} +-------------------------------------------------- + +<1> The successfully created role. +<2> The errors encountered. +<3> The number of put role requests that resulted in an error. +<4> The error keyed by role name. diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/security.bulk_put_role.json b/rest-api-spec/src/main/resources/rest-api-spec/api/security.bulk_put_role.json new file mode 100644 index 0000000000000..f8916a48b31e6 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/security.bulk_put_role.json @@ -0,0 +1,43 @@ +{ + "security.bulk_put_role": { + "documentation": { + "url": "https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-bulk-put-role.html", + "description": "Bulk adds and updates roles in the native realm." + }, + "stability": "stable", + "visibility": "public", + "headers": { + "accept": [ + "application/json" + ], + "content_type": [ + "application/json" + ] + }, + "url": { + "paths": [ + { + "path": "/_security/role", + "methods": [ + "POST" + ] + } + ] + }, + "params": { + "refresh": { + "type": "enum", + "options": [ + "true", + "false", + "wait_for" + ], + "description": "If `true` (the default) then refresh the affected shards to make this operation visible to search, if `wait_for` then wait for a refresh to make this operation visible to search, if `false` then do nothing with refreshes." + } + }, + "body": { + "description": "The roles to add", + "required": true + } + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/ActionTypes.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/ActionTypes.java index 30ad5e7902d19..43e914f873a83 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/ActionTypes.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/ActionTypes.java @@ -9,6 +9,7 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.ActionType; +import org.elasticsearch.xpack.core.security.action.role.BulkPutRolesResponse; import org.elasticsearch.xpack.core.security.action.user.QueryUserResponse; /** @@ -23,4 +24,6 @@ public final class ActionTypes { ); public static final ActionType QUERY_USER_ACTION = new ActionType<>("cluster:admin/xpack/security/user/query"); + + public static final ActionType BULK_PUT_ROLES = new ActionType<>("cluster:admin/xpack/security/role/bulk_put"); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRoleRequestBuilder.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRoleRequestBuilder.java new file mode 100644 index 0000000000000..c601bbdd79396 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRoleRequestBuilder.java @@ -0,0 +1,65 @@ +/* + * 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.core.security.action.role; + +import org.elasticsearch.action.ActionRequestBuilder; +import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.client.internal.ElasticsearchClient; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.xcontent.ConstructingObjectParser; +import org.elasticsearch.xcontent.ParseField; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentType; +import org.elasticsearch.xpack.core.security.action.ActionTypes; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; + +import java.io.IOException; +import java.util.List; + +import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg; + +/** + * Builder for requests to bulk add a roles to the security index + */ +public class BulkPutRoleRequestBuilder extends ActionRequestBuilder { + + private static final RoleDescriptor.Parser ROLE_DESCRIPTOR_PARSER = RoleDescriptor.parserBuilder().allowDescription(true).build(); + @SuppressWarnings("unchecked") + private static final ConstructingObjectParser, Void> PARSER = new ConstructingObjectParser<>( + "bulk_update_roles_request_payload", + a -> (List) a[0] + ); + + static { + PARSER.declareNamedObjects(optionalConstructorArg(), (p, c, n) -> { + p.nextToken(); + return ROLE_DESCRIPTOR_PARSER.parse(n, p, false); + }, new ParseField("roles")); + } + + public BulkPutRoleRequestBuilder(ElasticsearchClient client) { + super(client, ActionTypes.BULK_PUT_ROLES, new BulkPutRolesRequest()); + } + + public BulkPutRoleRequestBuilder content(BytesReference content, XContentType xContentType) throws IOException { + XContentParser parser = XContentHelper.createParserNotCompressed( + LoggingDeprecationHandler.XCONTENT_PARSER_CONFIG, + content, + xContentType + ); + List roles = PARSER.parse(parser, null); + request.setRoles(roles); + return this; + } + + public BulkPutRoleRequestBuilder setRefreshPolicy(WriteRequest.RefreshPolicy refreshPolicy) { + request.setRefreshPolicy(refreshPolicy); + return this; + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRoleRequestBuilderFactory.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRoleRequestBuilderFactory.java new file mode 100644 index 0000000000000..a0c93c70363b0 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRoleRequestBuilderFactory.java @@ -0,0 +1,22 @@ +/* + * 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.core.security.action.role; + +import org.elasticsearch.client.internal.Client; + +public interface BulkPutRoleRequestBuilderFactory { + BulkPutRoleRequestBuilder create(Client client); + + class Default implements BulkPutRoleRequestBuilderFactory { + @Override + public BulkPutRoleRequestBuilder create(Client client) { + // This needs to be added when Bulk API is made public in serverless + return new BulkPutRoleRequestBuilder(client); + } + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRolesRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRolesRequest.java new file mode 100644 index 0000000000000..a812c33eb41b7 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRolesRequest.java @@ -0,0 +1,69 @@ +/* + * 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.core.security.action.role; + +import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.support.TransportAction; +import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; + +import java.io.IOException; +import java.util.List; +import java.util.Objects; + +public class BulkPutRolesRequest extends ActionRequest { + + private List roles; + + public BulkPutRolesRequest() {} + + public void setRoles(List roles) { + this.roles = roles; + } + + private WriteRequest.RefreshPolicy refreshPolicy = WriteRequest.RefreshPolicy.IMMEDIATE; + + @Override + public ActionRequestValidationException validate() { + // Handle validation where put role is handled to produce partial success if validation fails + return null; + } + + public List getRoles() { + return roles; + } + + public BulkPutRolesRequest setRefreshPolicy(WriteRequest.RefreshPolicy refreshPolicy) { + this.refreshPolicy = refreshPolicy; + return this; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass() || super.equals(o)) return false; + + BulkPutRolesRequest that = (BulkPutRolesRequest) o; + return Objects.equals(roles, that.roles); + } + + @Override + public int hashCode() { + return Objects.hash(roles); + } + + public WriteRequest.RefreshPolicy getRefreshPolicy() { + return refreshPolicy; + } + + public void writeTo(StreamOutput out) throws IOException { + TransportAction.localOnly(); + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRolesResponse.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRolesResponse.java new file mode 100644 index 0000000000000..15870806f25fd --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/BulkPutRolesResponse.java @@ -0,0 +1,145 @@ +/* + * 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.core.security.action.role; + +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.DocWriteResponse; +import org.elasticsearch.action.support.TransportAction; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.xcontent.ToXContentObject; +import org.elasticsearch.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +public class BulkPutRolesResponse extends ActionResponse implements ToXContentObject { + + private final List items; + + public static class Builder { + + private final List items = new LinkedList<>(); + + public Builder addItem(Item item) { + items.add(item); + return this; + } + + public BulkPutRolesResponse build() { + return new BulkPutRolesResponse(items); + } + } + + public BulkPutRolesResponse(List items) { + this.items = items; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + Map> itemsByType = items.stream().collect(Collectors.groupingBy(Item::getResultType)); + + for (var resultEntry : itemsByType.entrySet()) { + if (resultEntry.getKey().equals("errors") == false) { + builder.startArray(resultEntry.getKey()); + for (var item : resultEntry.getValue()) { + item.toXContent(builder, params); + } + builder.endArray(); + } else { + builder.startObject("errors"); + builder.field("count", resultEntry.getValue().size()); + builder.startObject("details"); + for (var item : resultEntry.getValue()) { + builder.startObject(item.roleName); + item.toXContent(builder, params); + builder.endObject(); + } + builder.endObject(); + builder.endObject(); + } + } + + builder.endObject(); + return builder; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + TransportAction.localOnly(); + } + + public static class Item implements ToXContentObject { + private final Exception cause; + private final String roleName; + + private final DocWriteResponse.Result resultType; + + private Item(String roleName, DocWriteResponse.Result resultType, Exception cause) { + this.roleName = roleName; + this.resultType = resultType; + this.cause = cause; + } + + Item(StreamInput in) throws IOException { + roleName = in.readString(); + resultType = DocWriteResponse.Result.readFrom(in); + cause = in.readException(); + } + + public Exception getCause() { + return cause; + } + + public String getResultType() { + return resultType == null ? "errors" : resultType.getLowercase(); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + if (resultType == null) { + ElasticsearchException.generateThrowableXContent(builder, params, cause); + } else { + builder.value(roleName); + } + return builder; + } + + public static Item success(String roleName, DocWriteResponse.Result result) { + return new Item(roleName, result, null); + } + + public static Item failure(String roleName, Exception cause) { + return new Item(roleName, null, cause); + } + + public String getRoleName() { + return roleName; + } + + public boolean isFailed() { + return cause != null; + } + + public String getFailureMessage() { + if (cause != null) { + return cause.getMessage(); + } + return null; + } + } + + public List getItems() { + return items; + } + +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index baf72a3411cde..08e774006ad32 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -545,12 +545,17 @@ public RoleDescriptor parse(String name, BytesReference source, XContentType xCo } public RoleDescriptor parse(String name, XContentParser parser) throws IOException { - // validate name - Validation.Error validationError = Validation.Roles.validateRoleName(name, true); - if (validationError != null) { - ValidationException ve = new ValidationException(); - ve.addValidationError(validationError.toString()); - throw ve; + return parse(name, parser, true); + } + + public RoleDescriptor parse(String name, XContentParser parser, boolean validate) throws IOException { + if (validate) { + Validation.Error validationError = Validation.Roles.validateRoleName(name, true); + if (validationError != null) { + ValidationException ve = new ValidationException(); + ve.addValidationError(validationError.toString()); + throw ve; + } } // advance to the START_OBJECT token if needed diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java index 1ade22179ab59..54af9d947a9e8 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java @@ -297,6 +297,7 @@ public void testReadSecurityPrivilege() { PutUserAction.NAME, DeleteUserAction.NAME, PutRoleAction.NAME, + ActionTypes.BULK_PUT_ROLES.name(), DeleteRoleAction.NAME, PutRoleMappingAction.NAME, DeleteRoleMappingAction.NAME, diff --git a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java index 878d01abd02e3..092c8e6ccf391 100644 --- a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java +++ b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java @@ -279,6 +279,7 @@ public class Constants { "cluster:admin/xpack/security/role/delete", "cluster:admin/xpack/security/role/get", "cluster:admin/xpack/security/role/put", + "cluster:admin/xpack/security/role/bulk_put", "cluster:admin/xpack/security/role_mapping/delete", "cluster:admin/xpack/security/role_mapping/get", "cluster:admin/xpack/security/role_mapping/put", diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityOnTrialLicenseRestTestCase.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityOnTrialLicenseRestTestCase.java index bdbd5c659c479..d877ae63d0037 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityOnTrialLicenseRestTestCase.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityOnTrialLicenseRestTestCase.java @@ -7,9 +7,15 @@ package org.elasticsearch.xpack.security; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.client.methods.HttpPut; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.core.Strings; import org.elasticsearch.core.Tuple; import org.elasticsearch.test.TestSecurityClient; import org.elasticsearch.test.cluster.ElasticsearchCluster; @@ -24,9 +30,13 @@ import java.io.IOException; import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static org.elasticsearch.test.cluster.local.model.User.ROOT_USER_ROLE; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; public abstract class SecurityOnTrialLicenseRestTestCase extends ESRestTestCase { private TestSecurityClient securityClient; @@ -128,4 +138,53 @@ protected ApiKey getApiKey(String id) throws IOException { final TestSecurityClient client = getSecurityClient(); return client.getApiKey(id); } + + protected void upsertRole(String roleDescriptor, String roleName) throws IOException { + Request createRoleRequest = roleRequest(roleDescriptor, roleName); + Response createRoleResponse = adminClient().performRequest(createRoleRequest); + assertOK(createRoleResponse); + } + + protected Request roleRequest(String roleDescriptor, String roleName) { + Request createRoleRequest; + if (randomBoolean()) { + createRoleRequest = new Request(randomFrom(HttpPut.METHOD_NAME, HttpPost.METHOD_NAME), "/_security/role/" + roleName); + createRoleRequest.setJsonEntity(roleDescriptor); + } else { + createRoleRequest = new Request(HttpPost.METHOD_NAME, "/_security/role"); + createRoleRequest.setJsonEntity(Strings.format(""" + {"roles": {"%s": %s}} + """, roleName, roleDescriptor)); + } + return createRoleRequest; + } + + @SuppressWarnings("unchecked") + protected void assertSendRequestThrowsError(Request request, String expectedError) throws IOException { + String errorMessage; + if (request.getEndpoint().endsWith("/role")) { + Map response = responseAsMap(adminClient().performRequest(request)); + + Map errors = (Map) response.get("errors"); + Map failedItems = (Map) errors.get("details"); + assertEquals(failedItems.size(), 1); + Map error = (Map) failedItems.values().stream().findFirst().orElseThrow(); + errorMessage = (String) error.get("reason"); + } else { + ResponseException e = expectThrows(ResponseException.class, () -> adminClient().performRequest(request)); + assertEquals(400, e.getResponse().getStatusLine().getStatusCode()); + errorMessage = e.getMessage(); + } + assertThat(errorMessage, containsString(expectedError)); + } + + protected void fetchRoleAndAssertEqualsExpected(final String roleName, final RoleDescriptor expectedRoleDescriptor) throws IOException { + final Response getRoleResponse = adminClient().performRequest(new Request("GET", "/_security/role/" + roleName)); + assertOK(getRoleResponse); + final Map actual = responseAsParser(getRoleResponse).map( + HashMap::new, + p -> RoleDescriptor.parserBuilder().allowDescription(true).build().parse(expectedRoleDescriptor.getName(), p) + ); + assertThat(actual, equalTo(Map.of(expectedRoleDescriptor.getName(), expectedRoleDescriptor))); + } } diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/BulkPutRoleRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/BulkPutRoleRestIT.java new file mode 100644 index 0000000000000..6e111c8f54552 --- /dev/null +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/BulkPutRoleRestIT.java @@ -0,0 +1,231 @@ +/* + * 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.role; + +import org.apache.http.client.methods.HttpPost; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.security.SecurityOnTrialLicenseRestTestCase; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.not; + +public class BulkPutRoleRestIT extends SecurityOnTrialLicenseRestTestCase { + public void testPutManyValidRoles() throws Exception { + Map responseMap = upsertRoles(""" + {"roles": {"test1": {"cluster": ["all"],"indices": [{"names": ["*"],"privileges": ["all"]}]}, "test2": + {"cluster": ["all"],"indices": [{"names": ["*"],"privileges": ["read"]}]}, "test3": + {"cluster": ["all"],"indices": [{"names": ["*"],"privileges": ["write"]}]}}}"""); + assertThat(responseMap, not(hasKey("errors"))); + fetchRoleAndAssertEqualsExpected( + "test1", + new RoleDescriptor( + "test1", + new String[] { "all" }, + new RoleDescriptor.IndicesPrivileges[] { + RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").build() }, + null, + null, + null, + null, + null, + null, + null, + null, + null + ) + ); + fetchRoleAndAssertEqualsExpected( + "test2", + new RoleDescriptor( + "test2", + new String[] { "all" }, + new RoleDescriptor.IndicesPrivileges[] { + RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("read").build() }, + null, + null, + null, + null, + null, + null, + null, + null, + null + ) + ); + fetchRoleAndAssertEqualsExpected( + "test3", + new RoleDescriptor( + "test3", + new String[] { "all" }, + new RoleDescriptor.IndicesPrivileges[] { + RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("write").build() }, + null, + null, + null, + null, + null, + null, + null, + null, + null + ) + ); + } + + @SuppressWarnings("unchecked") + public void testPutMixedValidInvalidRoles() throws Exception { + Map responseMap = upsertRoles(""" + {"roles": {"test1": {"cluster": ["all"],"indices": [{"names": ["*"],"privileges": ["all"]}]}, "test2": + {"cluster": ["bad_privilege"],"indices": [{"names": ["*"],"privileges": ["read"]}]}, "test3": + {"cluster": ["all"],"indices": [{"names": ["*"],"privileges": ["write"]}]}}}"""); + + assertThat(responseMap, hasKey("errors")); + + List created = (List) responseMap.get("created"); + assertThat(created, hasSize(2)); + assertThat(created, contains("test1", "test3")); + + Map errors = (Map) responseMap.get("errors"); + Map failedItems = (Map) errors.get("details"); + assertEquals(failedItems.size(), 1); + + for (var entry : failedItems.entrySet()) { + Map error = (Map) entry.getValue(); + assertThat((String) error.get("reason"), containsString("unknown cluster privilege [bad_privilege]")); + } + + fetchRoleAndAssertEqualsExpected( + "test1", + new RoleDescriptor( + "test1", + new String[] { "all" }, + new RoleDescriptor.IndicesPrivileges[] { + RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").build() }, + null, + null, + null, + null, + null, + null, + null, + null, + null + ) + ); + + fetchRoleAndAssertEqualsExpected( + "test3", + new RoleDescriptor( + "test3", + new String[] { "all" }, + new RoleDescriptor.IndicesPrivileges[] { + RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("write").build() }, + null, + null, + null, + null, + null, + null, + null, + null, + null + ) + ); + + final ResponseException e = expectThrows( + ResponseException.class, + () -> adminClient().performRequest(new Request("GET", "/_security/role/test2")) + ); + assertEquals(404, e.getResponse().getStatusLine().getStatusCode()); + } + + @SuppressWarnings("unchecked") + public void testPutNoValidRoles() throws Exception { + Map responseMap = upsertRoles(""" + {"roles": {"test1": {"cluster": ["bad_privilege"],"indices": [{"names": ["*"],"privileges": ["all"]}]}, "test2": + {"cluster": ["bad_privilege"],"indices": [{"names": ["*"],"privileges": ["read"]}]}, "test3": + {"cluster": ["bad_privilege"],"indices": [{"names": ["*"],"privileges": ["write"]}]}}}"""); + + assertThat(responseMap, hasKey("errors")); + Map errors = (Map) responseMap.get("errors"); + Map failedItems = (Map) errors.get("details"); + assertEquals(failedItems.size(), 3); + + for (var entry : failedItems.entrySet()) { + Map error = (Map) entry.getValue(); + assertThat((String) error.get("reason"), containsString("unknown cluster privilege [bad_privilege]")); + } + + for (String name : List.of("test1", "test2", "test3")) { + final ResponseException e = expectThrows( + ResponseException.class, + () -> adminClient().performRequest(new Request("GET", "/_security/role/" + name)) + ); + assertEquals(404, e.getResponse().getStatusLine().getStatusCode()); + } + } + + @SuppressWarnings("unchecked") + public void testBulkUpdates() throws Exception { + String request = """ + {"roles": {"test1": {"cluster": ["all"],"indices": [{"names": ["*"],"privileges": ["all"]}]}, "test2": + {"cluster": ["all"],"indices": [{"names": ["*"],"privileges": ["read"]}]}, "test3": + {"cluster": ["all"],"indices": [{"names": ["*"],"privileges": ["write"]}]}}}"""; + + { + Map responseMap = upsertRoles(request); + assertThat(responseMap, not(hasKey("errors"))); + + List> items = (List>) responseMap.get("created"); + assertEquals(3, items.size()); + } + { + Map responseMap = upsertRoles(request); + assertThat(responseMap, not(hasKey("errors"))); + + List> items = (List>) responseMap.get("noop"); + assertEquals(3, items.size()); + } + { + request = """ + {"roles": {"test1": {"cluster": ["all"],"indices": [{"names": ["*"],"privileges": ["read"]}]}, "test2": + {"cluster": ["all"],"indices": [{"names": ["*"],"privileges": ["all"]}]}, "test3": + {"cluster": ["all"],"indices": [{"names": ["*"],"privileges": ["all"]}]}}}"""; + + Map responseMap = upsertRoles(request); + assertThat(responseMap, not(hasKey("errors"))); + List> items = (List>) responseMap.get("updated"); + assertEquals(3, items.size()); + } + } + + protected Map upsertRoles(String roleDescriptorsByName) throws IOException { + Request request = rolesRequest(roleDescriptorsByName); + Response response = adminClient().performRequest(request); + assertOK(response); + return responseAsMap(response); + } + + protected Request rolesRequest(String roleDescriptorsByName) { + Request rolesRequest; + rolesRequest = new Request(HttpPost.METHOD_NAME, "/_security/role"); + rolesRequest.setJsonEntity(org.elasticsearch.core.Strings.format(roleDescriptorsByName)); + return rolesRequest; + } + +} diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/RoleWithDescriptionRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/RoleWithDescriptionRestIT.java index 95a650737d452..33c78f2dd6324 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/RoleWithDescriptionRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/RoleWithDescriptionRestIT.java @@ -7,22 +7,13 @@ package org.elasticsearch.xpack.security.role; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.client.methods.HttpPut; import org.elasticsearch.client.Request; -import org.elasticsearch.client.Response; -import org.elasticsearch.client.ResponseException; import org.elasticsearch.core.Strings; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.support.Validation; import org.elasticsearch.xpack.security.SecurityOnTrialLicenseRestTestCase; import java.io.IOException; -import java.util.HashMap; -import java.util.Map; - -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; public class RoleWithDescriptionRestIT extends SecurityOnTrialLicenseRestTestCase { @@ -30,15 +21,13 @@ public void testCreateOrUpdateRoleWithDescription() throws Exception { final String roleName = "role_with_description"; final String initialRoleDescription = randomAlphaOfLengthBetween(0, 10); { - Request createRoleRequest = new Request(HttpPut.METHOD_NAME, "/_security/role/" + roleName); - createRoleRequest.setJsonEntity(Strings.format(""" + upsertRole(Strings.format(""" { "description": "%s", "cluster": ["all"], "indices": [{"names": ["*"], "privileges": ["all"]}] - }""", initialRoleDescription)); - Response createResponse = adminClient().performRequest(createRoleRequest); - assertOK(createResponse); + }""", initialRoleDescription), roleName); + fetchRoleAndAssertEqualsExpected( roleName, new RoleDescriptor( @@ -60,15 +49,12 @@ public void testCreateOrUpdateRoleWithDescription() throws Exception { } { final String newRoleDescription = randomValueOtherThan(initialRoleDescription, () -> randomAlphaOfLengthBetween(0, 10)); - Request updateRoleRequest = new Request(HttpPost.METHOD_NAME, "/_security/role/" + roleName); - updateRoleRequest.setJsonEntity(Strings.format(""" + upsertRole(Strings.format(""" { "description": "%s", "cluster": ["all"], "indices": [{"names": ["index-*"], "privileges": ["all"]}] - }""", newRoleDescription)); - Response updateResponse = adminClient().performRequest(updateRoleRequest); - assertOK(updateResponse); + }""", newRoleDescription), roleName); fetchRoleAndAssertEqualsExpected( roleName, @@ -91,56 +77,37 @@ public void testCreateOrUpdateRoleWithDescription() throws Exception { } } - public void testCreateRoleWithInvalidDescriptionFails() { - Request createRoleRequest = new Request(HttpPut.METHOD_NAME, "/_security/role/role_with_large_description"); - createRoleRequest.setJsonEntity(Strings.format(""" + public void testCreateRoleWithInvalidDescriptionFails() throws IOException { + Request request = roleRequest(Strings.format(""" { "description": "%s", "cluster": ["all"], "indices": [{"names": ["*"], "privileges": ["all"]}] - }""", randomAlphaOfLength(Validation.Roles.MAX_DESCRIPTION_LENGTH + randomIntBetween(1, 5)))); + }""", randomAlphaOfLength(Validation.Roles.MAX_DESCRIPTION_LENGTH + randomIntBetween(1, 5))), "role_with_large_description"); - ResponseException e = expectThrows(ResponseException.class, () -> adminClient().performRequest(createRoleRequest)); - assertEquals(400, e.getResponse().getStatusLine().getStatusCode()); - assertThat( - e.getMessage(), - containsString("Role description must be less than " + Validation.Roles.MAX_DESCRIPTION_LENGTH + " characters.") + assertSendRequestThrowsError( + request, + "Role description must be less than " + Validation.Roles.MAX_DESCRIPTION_LENGTH + " characters." ); } public void testUpdateRoleWithInvalidDescriptionFails() throws IOException { - Request createRoleRequest = new Request(HttpPut.METHOD_NAME, "/_security/role/my_role"); - createRoleRequest.setJsonEntity(""" + upsertRole(""" { "cluster": ["all"], "indices": [{"names": ["*"], "privileges": ["all"]}] - }"""); - Response createRoleResponse = adminClient().performRequest(createRoleRequest); - assertOK(createRoleResponse); + }""", "my_role"); - Request updateRoleRequest = new Request(HttpPost.METHOD_NAME, "/_security/role/my_role"); - updateRoleRequest.setJsonEntity(Strings.format(""" + Request updateRoleRequest = roleRequest(Strings.format(""" { "description": "%s", "cluster": ["all"], "indices": [{"names": ["index-*"], "privileges": ["all"]}] - }""", randomAlphaOfLength(Validation.Roles.MAX_DESCRIPTION_LENGTH + randomIntBetween(1, 5)))); - - ResponseException e = expectThrows(ResponseException.class, () -> adminClient().performRequest(updateRoleRequest)); - assertEquals(400, e.getResponse().getStatusLine().getStatusCode()); - assertThat( - e.getMessage(), - containsString("Role description must be less than " + Validation.Roles.MAX_DESCRIPTION_LENGTH + " characters.") - ); - } + }""", randomAlphaOfLength(Validation.Roles.MAX_DESCRIPTION_LENGTH + randomIntBetween(1, 5))), "my_role"); - private void fetchRoleAndAssertEqualsExpected(final String roleName, final RoleDescriptor expectedRoleDescriptor) throws IOException { - final Response getRoleResponse = adminClient().performRequest(new Request("GET", "/_security/role/" + roleName)); - assertOK(getRoleResponse); - final Map actual = responseAsParser(getRoleResponse).map( - HashMap::new, - p -> RoleDescriptor.parserBuilder().allowDescription(true).build().parse(expectedRoleDescriptor.getName(), p) + assertSendRequestThrowsError( + updateRoleRequest, + "Role description must be less than " + Validation.Roles.MAX_DESCRIPTION_LENGTH + " characters." ); - assertThat(actual, equalTo(Map.of(expectedRoleDescriptor.getName(), expectedRoleDescriptor))); } } diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/RoleWithRemoteIndicesPrivilegesRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/RoleWithRemoteIndicesPrivilegesRestIT.java index aa5967ea7277a..93dc6c3761482 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/RoleWithRemoteIndicesPrivilegesRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/RoleWithRemoteIndicesPrivilegesRestIT.java @@ -50,8 +50,7 @@ public void cleanup() throws IOException { } public void testRemoteIndexPrivileges() throws IOException { - var putRoleRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); - putRoleRequest.setJsonEntity(""" + upsertRole(""" { "remote_indices": [ { @@ -64,9 +63,7 @@ public void testRemoteIndexPrivileges() throws IOException { } } ] - }"""); - final Response putRoleResponse1 = adminClient().performRequest(putRoleRequest); - assertOK(putRoleResponse1); + }""", REMOTE_SEARCH_ROLE); final Response getRoleResponse = adminClient().performRequest(new Request("GET", "/_security/role/" + REMOTE_SEARCH_ROLE)); assertOK(getRoleResponse); @@ -106,8 +103,7 @@ public void testRemoteIndexPrivileges() throws IOException { assertThat(e.getMessage(), containsString("action [" + TransportSearchAction.TYPE.name() + "] is unauthorized for user")); // Add local privileges and check local authorization works - putRoleRequest = new Request("PUT", "_security/role/" + REMOTE_SEARCH_ROLE); - putRoleRequest.setJsonEntity(""" + upsertRole(""" { "cluster": ["all"], "indices": [ @@ -127,9 +123,8 @@ public void testRemoteIndexPrivileges() throws IOException { } } ] - }"""); - final Response putRoleResponse2 = adminClient().performRequest(putRoleRequest); - assertOK(putRoleResponse2); + }""", REMOTE_SEARCH_ROLE); + final Response searchResponse = client().performRequest(searchRequest); assertOK(searchResponse); @@ -171,8 +166,7 @@ public void testRemoteIndexPrivileges() throws IOException { } public void testGetUserPrivileges() throws IOException { - final var putRoleRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); - putRoleRequest.setJsonEntity(""" + upsertRole(""" { "remote_indices": [ { @@ -191,9 +185,7 @@ public void testGetUserPrivileges() throws IOException { "clusters": ["remote-a", "*"] } ] - }"""); - final Response putRoleResponse1 = adminClient().performRequest(putRoleRequest); - assertOK(putRoleResponse1); + }""", REMOTE_SEARCH_ROLE); final Response getUserPrivilegesResponse1 = executeAsRemoteSearchUser(new Request("GET", "/_security/user/_privileges")); assertOK(getUserPrivilegesResponse1); @@ -222,8 +214,7 @@ public void testGetUserPrivileges() throws IOException { ] }"""))); - final var putRoleRequest2 = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); - putRoleRequest2.setJsonEntity(""" + upsertRole(""" { "cluster": ["all"], "indices": [ @@ -245,9 +236,7 @@ public void testGetUserPrivileges() throws IOException { "clusters": ["remote-c"] } ] - }"""); - final Response putRoleResponse2 = adminClient().performRequest(putRoleRequest2); - assertOK(putRoleResponse2); + }""", REMOTE_SEARCH_ROLE); final Response getUserPrivilegesResponse2 = executeAsRemoteSearchUser(new Request("GET", "/_security/user/_privileges")); assertOK(getUserPrivilegesResponse2); @@ -282,8 +271,7 @@ public void testGetUserPrivileges() throws IOException { } public void testGetUserPrivilegesWithMultipleFlsDlsDefinitionsPreservesGroupPerIndexPrivilege() throws IOException { - final var putRoleRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); - putRoleRequest.setJsonEntity(""" + upsertRole(""" { "remote_indices": [ { @@ -305,9 +293,7 @@ public void testGetUserPrivilegesWithMultipleFlsDlsDefinitionsPreservesGroupPerI } } ] - }"""); - final Response putRoleResponse1 = adminClient().performRequest(putRoleRequest); - assertOK(putRoleResponse1); + }""", REMOTE_SEARCH_ROLE); final Response getUserPrivilegesResponse1 = executeAsRemoteSearchUser(new Request("GET", "/_security/user/_privileges")); assertOK(getUserPrivilegesResponse1); diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/RoleWithWorkflowsRestrictionRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/RoleWithWorkflowsRestrictionRestIT.java index d2fc27fb3fcae..979fe87ec4bb5 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/RoleWithWorkflowsRestrictionRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/RoleWithWorkflowsRestrictionRestIT.java @@ -7,10 +7,7 @@ package org.elasticsearch.xpack.security.role; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.client.methods.HttpPut; import org.elasticsearch.client.Request; -import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; import org.elasticsearch.xpack.security.SecurityOnTrialLicenseRestTestCase; @@ -21,8 +18,7 @@ public class RoleWithWorkflowsRestrictionRestIT extends SecurityOnTrialLicenseRestTestCase { public void testCreateRoleWithWorkflowsRestrictionFail() { - Request createRoleRequest = new Request(HttpPut.METHOD_NAME, "/_security/role/role_with_restriction"); - createRoleRequest.setJsonEntity(""" + Request request = roleRequest(""" { "cluster": ["all"], "indices": [ @@ -34,16 +30,15 @@ public void testCreateRoleWithWorkflowsRestrictionFail() { "restriction":{ "workflows": ["foo", "bar"] } - }"""); + }""", "role_with_restriction"); - ResponseException e = expectThrows(ResponseException.class, () -> adminClient().performRequest(createRoleRequest)); + ResponseException e = expectThrows(ResponseException.class, () -> adminClient().performRequest(request)); assertEquals(400, e.getResponse().getStatusLine().getStatusCode()); assertThat(e.getMessage(), containsString("failed to parse role [role_with_restriction]. unexpected field [restriction]")); } public void testUpdateRoleWithWorkflowsRestrictionFail() throws IOException { - Request createRoleRequest = new Request(HttpPut.METHOD_NAME, "/_security/role/my_role"); - createRoleRequest.setJsonEntity(""" + upsertRole(""" { "cluster": ["all"], "indices": [ @@ -52,12 +47,9 @@ public void testUpdateRoleWithWorkflowsRestrictionFail() throws IOException { "privileges": ["all"] } ] - }"""); - Response createRoleResponse = adminClient().performRequest(createRoleRequest); - assertOK(createRoleResponse); + }""", "my_role"); - Request updateRoleRequest = new Request(HttpPost.METHOD_NAME, "/_security/role/my_role"); - updateRoleRequest.setJsonEntity(""" + Request updateRoleRequest = roleRequest(""" { "cluster": ["all"], "indices": [ @@ -69,7 +61,7 @@ public void testUpdateRoleWithWorkflowsRestrictionFail() throws IOException { "restriction":{ "workflows": ["foo", "bar"] } - }"""); + }""", "my_role"); ResponseException e = expectThrows(ResponseException.class, () -> adminClient().performRequest(updateRoleRequest)); assertEquals(400, e.getResponse().getStatusLine().getStatusCode()); 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 bbb1feeef8d44..a38710332313f 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 @@ -160,6 +160,7 @@ import org.elasticsearch.xpack.core.security.action.profile.SuggestProfilesAction; import org.elasticsearch.xpack.core.security.action.profile.UpdateProfileDataAction; import org.elasticsearch.xpack.core.security.action.realm.ClearRealmCacheAction; +import org.elasticsearch.xpack.core.security.action.role.BulkPutRoleRequestBuilderFactory; import org.elasticsearch.xpack.core.security.action.role.ClearRolesCacheAction; import org.elasticsearch.xpack.core.security.action.role.DeleteRoleAction; import org.elasticsearch.xpack.core.security.action.role.GetRolesAction; @@ -253,6 +254,7 @@ import org.elasticsearch.xpack.security.action.profile.TransportSuggestProfilesAction; import org.elasticsearch.xpack.security.action.profile.TransportUpdateProfileDataAction; import org.elasticsearch.xpack.security.action.realm.TransportClearRealmCacheAction; +import org.elasticsearch.xpack.security.action.role.TransportBulkPutRolesAction; import org.elasticsearch.xpack.security.action.role.TransportClearRolesCacheAction; import org.elasticsearch.xpack.security.action.role.TransportDeleteRoleAction; import org.elasticsearch.xpack.security.action.role.TransportGetRolesAction; @@ -370,6 +372,7 @@ import org.elasticsearch.xpack.security.rest.action.profile.RestSuggestProfilesAction; import org.elasticsearch.xpack.security.rest.action.profile.RestUpdateProfileDataAction; import org.elasticsearch.xpack.security.rest.action.realm.RestClearRealmCacheAction; +import org.elasticsearch.xpack.security.rest.action.role.RestBulkPutRolesAction; import org.elasticsearch.xpack.security.rest.action.role.RestClearRolesCacheAction; import org.elasticsearch.xpack.security.rest.action.role.RestDeleteRoleAction; import org.elasticsearch.xpack.security.rest.action.role.RestGetRolesAction; @@ -601,6 +604,7 @@ public class Security extends Plugin private final SetOnce scriptServiceReference = new SetOnce<>(); private final SetOnce operatorOnlyRegistry = new SetOnce<>(); private final SetOnce putRoleRequestBuilderFactory = new SetOnce<>(); + private final SetOnce bulkPutRoleRequestBuilderFactory = new SetOnce<>(); private final SetOnce createApiKeyRequestBuilderFactory = new SetOnce<>(); private final SetOnce updateApiKeyRequestTranslator = new SetOnce<>(); private final SetOnce bulkUpdateApiKeyRequestTranslator = new SetOnce<>(); @@ -911,19 +915,14 @@ Collection createComponents( dlsBitsetCache.set(new DocumentSubsetBitsetCache(settings, threadPool)); final FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(settings); - final NativeRolesStore nativeRolesStore = new NativeRolesStore( - settings, - client, - getLicenseState(), - systemIndices.getMainIndexManager(), - clusterService, - featureService - ); RoleDescriptor.setFieldPermissionsCache(fieldPermissionsCache); // Need to set to default if it wasn't set by an extension if (putRoleRequestBuilderFactory.get() == null) { putRoleRequestBuilderFactory.set(new PutRoleRequestBuilderFactory.Default()); } + if (bulkPutRoleRequestBuilderFactory.get() == null) { + bulkPutRoleRequestBuilderFactory.set(new BulkPutRoleRequestBuilderFactory.Default()); + } if (createApiKeyRequestBuilderFactory.get() == null) { createApiKeyRequestBuilderFactory.set(new CreateApiKeyRequestBuilderFactory.Default()); } @@ -951,7 +950,7 @@ Collection createComponents( this.fileRolesStore.set( new FileRolesStore(settings, environment, resourceWatcherService, getLicenseState(), xContentRegistry, fileRoleValidator.get()) ); - final ReservedRoleNameChecker reservedRoleNameChecker = reservedRoleNameCheckerFactory.get().create(fileRolesStore.get()::exists); + ReservedRoleNameChecker reservedRoleNameChecker = reservedRoleNameCheckerFactory.get().create(fileRolesStore.get()::exists); components.add(new PluginComponentBinding<>(ReservedRoleNameChecker.class, reservedRoleNameChecker)); final Map, ActionListener>>> customRoleProviders = new LinkedHashMap<>(); @@ -964,6 +963,17 @@ Collection createComponents( } } + final NativeRolesStore nativeRolesStore = new NativeRolesStore( + settings, + client, + getLicenseState(), + systemIndices.getMainIndexManager(), + clusterService, + featureService, + reservedRoleNameChecker, + xContentRegistry + ); + final ApiKeyService apiKeyService = new ApiKeyService( settings, Clock.systemUTC(), @@ -1526,6 +1536,7 @@ public void onIndexModule(IndexModule module) { new ActionHandler<>(DeleteUserAction.INSTANCE, TransportDeleteUserAction.class), new ActionHandler<>(GetRolesAction.INSTANCE, TransportGetRolesAction.class), new ActionHandler<>(PutRoleAction.INSTANCE, TransportPutRoleAction.class), + new ActionHandler<>(ActionTypes.BULK_PUT_ROLES, TransportBulkPutRolesAction.class), new ActionHandler<>(DeleteRoleAction.INSTANCE, TransportDeleteRoleAction.class), new ActionHandler<>(TransportChangePasswordAction.TYPE, TransportChangePasswordAction.class), new ActionHandler<>(AuthenticateAction.INSTANCE, TransportAuthenticateAction.class), @@ -1619,6 +1630,7 @@ public List getRestHandlers( new RestPutUserAction(settings, getLicenseState()), new RestDeleteUserAction(settings, getLicenseState()), new RestGetRolesAction(settings, getLicenseState()), + new RestBulkPutRolesAction(settings, getLicenseState(), bulkPutRoleRequestBuilderFactory.get()), new RestPutRoleAction(settings, getLicenseState(), putRoleRequestBuilderFactory.get()), new RestDeleteRoleAction(settings, getLicenseState()), new RestChangePasswordAction(settings, securityContext.get(), getLicenseState()), @@ -2257,6 +2269,7 @@ public void loadExtensions(ExtensionLoader loader) { securityExtensions.addAll(loader.loadExtensions(SecurityExtension.class)); loadSingletonExtensionAndSetOnce(loader, operatorOnlyRegistry, OperatorOnlyRegistry.class); loadSingletonExtensionAndSetOnce(loader, putRoleRequestBuilderFactory, PutRoleRequestBuilderFactory.class); + // TODO add bulkPutRoleRequestBuilderFactory loading here when available loadSingletonExtensionAndSetOnce(loader, getBuiltinPrivilegesResponseTranslator, GetBuiltinPrivilegesResponseTranslator.class); loadSingletonExtensionAndSetOnce(loader, updateApiKeyRequestTranslator, UpdateApiKeyRequestTranslator.class); loadSingletonExtensionAndSetOnce(loader, bulkUpdateApiKeyRequestTranslator, BulkUpdateApiKeyRequestTranslator.class); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportBulkPutRolesAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportBulkPutRolesAction.java new file mode 100644 index 0000000000000..fca354d04c7c5 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportBulkPutRolesAction.java @@ -0,0 +1,34 @@ +/* + * 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.action.role; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.TransportAction; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.security.action.ActionTypes; +import org.elasticsearch.xpack.core.security.action.role.BulkPutRolesRequest; +import org.elasticsearch.xpack.core.security.action.role.BulkPutRolesResponse; +import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; + +public class TransportBulkPutRolesAction extends TransportAction { + + private final NativeRolesStore rolesStore; + + @Inject + public TransportBulkPutRolesAction(ActionFilters actionFilters, NativeRolesStore rolesStore, TransportService transportService) { + super(ActionTypes.BULK_PUT_ROLES.name(), actionFilters, transportService.getTaskManager()); + this.rolesStore = rolesStore; + } + + @Override + protected void doExecute(Task task, final BulkPutRolesRequest request, final ActionListener listener) { + rolesStore.putRoles(request.getRefreshPolicy(), request.getRoles(), listener); + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java index 87b9bb72884be..c22e460728dca 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java @@ -6,75 +6,36 @@ */ package org.elasticsearch.xpack.security.action.role; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; -import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xpack.core.security.action.role.PutRoleAction; import org.elasticsearch.xpack.core.security.action.role.PutRoleRequest; import org.elasticsearch.xpack.core.security.action.role.PutRoleResponse; -import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator; -import org.elasticsearch.xpack.security.authz.ReservedRoleNameChecker; import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; -import static org.elasticsearch.action.ValidateActions.addValidationError; - public class TransportPutRoleAction extends TransportAction { private final NativeRolesStore rolesStore; - private final NamedXContentRegistry xContentRegistry; - private final ReservedRoleNameChecker reservedRoleNameChecker; @Inject - public TransportPutRoleAction( - ActionFilters actionFilters, - NativeRolesStore rolesStore, - TransportService transportService, - NamedXContentRegistry xContentRegistry, - ReservedRoleNameChecker reservedRoleNameChecker - ) { + public TransportPutRoleAction(ActionFilters actionFilters, NativeRolesStore rolesStore, TransportService transportService) { super(PutRoleAction.NAME, actionFilters, transportService.getTaskManager()); this.rolesStore = rolesStore; - this.xContentRegistry = xContentRegistry; - this.reservedRoleNameChecker = reservedRoleNameChecker; } @Override protected void doExecute(Task task, final PutRoleRequest request, final ActionListener listener) { - final Exception validationException = validateRequest(request); - if (validationException != null) { - listener.onFailure(validationException); - } else { - rolesStore.putRole(request, request.roleDescriptor(), listener.safeMap(created -> { - if (created) { - logger.info("added role [{}]", request.name()); - } else { - logger.info("updated role [{}]", request.name()); - } - return new PutRoleResponse(created); - })); - } - } - - private Exception validateRequest(final PutRoleRequest request) { - // TODO we can remove this -- `execute()` already calls `request.validate()` before `doExecute()` - ActionRequestValidationException validationException = request.validate(); - if (validationException != null) { - return validationException; - } - if (reservedRoleNameChecker.isReserved(request.name())) { - throw addValidationError("Role [" + request.name() + "] is reserved and may not be used.", null); - } - try { - DLSRoleQueryValidator.validateQueryField(request.roleDescriptor().getIndicesPrivileges(), xContentRegistry); - } catch (ElasticsearchException | IllegalArgumentException e) { - return e; - } - return null; + rolesStore.putRole(request.getRefreshPolicy(), request.roleDescriptor(), listener.safeMap(created -> { + if (created) { + logger.info("added role [{}]", request.name()); + } else { + logger.info("updated role [{}]", request.name()); + } + return new PutRoleResponse(created); + })); } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java index 5bd837c7d817c..b4afc82ff1816 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java @@ -11,8 +11,12 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.TransportVersions; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.DelegatingActionListener; import org.elasticsearch.action.DocWriteResponse; +import org.elasticsearch.action.bulk.BulkItemResponse; +import org.elasticsearch.action.bulk.BulkRequest; +import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.action.delete.DeleteRequest; import org.elasticsearch.action.delete.DeleteResponse; import org.elasticsearch.action.get.GetResponse; @@ -24,6 +28,8 @@ import org.elasticsearch.action.search.MultiSearchResponse.Item; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.support.ContextPreservingActionListener; +import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.action.update.UpdateRequest; import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.bytes.BytesReference; @@ -36,26 +42,32 @@ import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.license.LicenseUtils; import org.elasticsearch.license.XPackLicenseState; +import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.core.security.ScrollHelper; +import org.elasticsearch.xpack.core.security.action.role.BulkPutRolesResponse; import org.elasticsearch.xpack.core.security.action.role.ClearRolesCacheAction; import org.elasticsearch.xpack.core.security.action.role.ClearRolesCacheRequest; import org.elasticsearch.xpack.core.security.action.role.ClearRolesCacheResponse; import org.elasticsearch.xpack.core.security.action.role.DeleteRoleRequest; -import org.elasticsearch.xpack.core.security.action.role.PutRoleRequest; +import org.elasticsearch.xpack.core.security.action.role.RoleDescriptorRequestValidator; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges; import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; +import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator; import org.elasticsearch.xpack.core.security.support.NativeRealmValidationUtil; +import org.elasticsearch.xpack.security.authz.ReservedRoleNameChecker; import org.elasticsearch.xpack.security.support.SecurityIndexManager; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -64,6 +76,7 @@ import java.util.function.Supplier; import static org.elasticsearch.TransportVersions.ROLE_REMOTE_CLUSTER_PRIVS; +import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.index.query.QueryBuilders.existsQuery; import static org.elasticsearch.search.SearchService.DEFAULT_KEEPALIVE_SETTING; import static org.elasticsearch.transport.RemoteClusterPortSettings.TRANSPORT_VERSION_ADVANCED_REMOTE_CLUSTER_SECURITY; @@ -106,6 +119,12 @@ public class NativeRolesStore implements BiConsumer, ActionListener< .allowDescription(true) .build(); + private static final Set UPDATE_ROLES_REFRESH_CACHE_RESULTS = Set.of( + DocWriteResponse.Result.CREATED, + DocWriteResponse.Result.UPDATED, + DocWriteResponse.Result.DELETED + ); + private final Settings settings; private final Client client; private final XPackLicenseState licenseState; @@ -117,13 +136,19 @@ public class NativeRolesStore implements BiConsumer, ActionListener< private final FeatureService featureService; + private final ReservedRoleNameChecker reservedRoleNameChecker; + + private final NamedXContentRegistry xContentRegistry; + public NativeRolesStore( Settings settings, Client client, XPackLicenseState licenseState, SecurityIndexManager securityIndex, ClusterService clusterService, - FeatureService featureService + FeatureService featureService, + ReservedRoleNameChecker reservedRoleNameChecker, + NamedXContentRegistry xContentRegistry ) { this.settings = settings; this.client = client; @@ -131,6 +156,8 @@ public NativeRolesStore( this.securityIndex = securityIndex; this.clusterService = clusterService; this.featureService = featureService; + this.reservedRoleNameChecker = reservedRoleNameChecker; + this.xContentRegistry = xContentRegistry; this.enabled = settings.getAsBoolean(NATIVE_ROLES_ENABLED, true); } @@ -258,89 +285,198 @@ public void onFailure(Exception e) { } } - public void putRole(final PutRoleRequest request, final RoleDescriptor role, final ActionListener listener) { - if (enabled == false) { - listener.onFailure(new IllegalStateException("Native role management is disabled")); - return; + private Exception validateRoleDescriptor(RoleDescriptor role) { + ActionRequestValidationException validationException = null; + validationException = RoleDescriptorRequestValidator.validate(role, validationException); + + if (reservedRoleNameChecker.isReserved(role.getName())) { + throw addValidationError("Role [" + role.getName() + "] is reserved and may not be used.", validationException); } if (role.isUsingDocumentOrFieldLevelSecurity() && DOCUMENT_LEVEL_SECURITY_FEATURE.checkWithoutTracking(licenseState) == false) { - listener.onFailure(LicenseUtils.newComplianceException("field and document level security")); + return LicenseUtils.newComplianceException("field and document level security"); } else if (role.hasRemoteIndicesPrivileges() && clusterService.state().getMinTransportVersion().before(TRANSPORT_VERSION_ADVANCED_REMOTE_CLUSTER_SECURITY)) { - listener.onFailure( - new IllegalStateException( - "all nodes must have version [" - + TRANSPORT_VERSION_ADVANCED_REMOTE_CLUSTER_SECURITY.toReleaseVersion() - + "] or higher to support remote indices privileges" - ) + return new IllegalStateException( + "all nodes must have version [" + + TRANSPORT_VERSION_ADVANCED_REMOTE_CLUSTER_SECURITY.toReleaseVersion() + + "] or higher to support remote indices privileges" ); } else if (role.hasRemoteClusterPermissions() && clusterService.state().getMinTransportVersion().before(ROLE_REMOTE_CLUSTER_PRIVS)) { - listener.onFailure( - new IllegalStateException( - "all nodes must have version [" + ROLE_REMOTE_CLUSTER_PRIVS + "] or higher to support remote cluster privileges" - ) + return new IllegalStateException( + "all nodes must have version [" + ROLE_REMOTE_CLUSTER_PRIVS + "] or higher to support remote cluster privileges" ); } else if (role.hasDescription() && clusterService.state().getMinTransportVersion().before(TransportVersions.SECURITY_ROLE_DESCRIPTION)) { - listener.onFailure( - new IllegalStateException( - "all nodes must have version [" - + TransportVersions.SECURITY_ROLE_DESCRIPTION.toReleaseVersion() - + "] or higher to support specifying role description" - ) + return new IllegalStateException( + "all nodes must have version [" + + TransportVersions.SECURITY_ROLE_DESCRIPTION.toReleaseVersion() + + "] or higher to support specifying role description" ); - } else { - innerPutRole(request, role, listener); } + try { + DLSRoleQueryValidator.validateQueryField(role.getIndicesPrivileges(), xContentRegistry); + } catch (ElasticsearchException | IllegalArgumentException e) { + return e; + } + + return validationException; } - // pkg-private for testing - void innerPutRole(final PutRoleRequest request, final RoleDescriptor role, final ActionListener listener) { - final String roleName = role.getName(); - assert NativeRealmValidationUtil.validateRoleName(roleName, false) == null : "Role name was invalid or reserved: " + roleName; - assert false == role.hasRestriction() : "restriction is not supported for native roles"; + public void putRole(final WriteRequest.RefreshPolicy refreshPolicy, final RoleDescriptor role, final ActionListener listener) { + if (enabled == false) { + listener.onFailure(new IllegalStateException("Native role management is disabled")); + return; + } + Exception validationException = validateRoleDescriptor(role); + + if (validationException != null) { + listener.onFailure(validationException); + return; + } - securityIndex.prepareIndexIfNeededThenExecute(listener::onFailure, () -> { - final XContentBuilder xContentBuilder; + try { + IndexRequest indexRequest = createRoleIndexRequest(role); + indexRequest.setRefreshPolicy(refreshPolicy); + securityIndex.prepareIndexIfNeededThenExecute( + listener::onFailure, + () -> executeAsyncWithOrigin( + client.threadPool().getThreadContext(), + SECURITY_ORIGIN, + indexRequest, + new ActionListener() { + @Override + public void onResponse(DocWriteResponse indexResponse) { + final boolean created = indexResponse.getResult() == DocWriteResponse.Result.CREATED; + logger.trace("Created role: [{}]", indexRequest); + clearRoleCache(role.getName(), listener, created); + } + + @Override + public void onFailure(Exception e) { + logger.error(() -> "failed to put role [" + role.getName() + "]", e); + listener.onFailure(e); + } + }, + client::index + ) + ); + } catch (IOException exception) { + listener.onFailure(exception); + } + } + + public void putRoles( + final WriteRequest.RefreshPolicy refreshPolicy, + final List roles, + final ActionListener listener + ) { + if (enabled == false) { + listener.onFailure(new IllegalStateException("Native role management is disabled")); + return; + } + BulkRequest bulkRequest = new BulkRequest().setRefreshPolicy(refreshPolicy); + Map validationErrorByRoleName = new HashMap<>(); + + for (RoleDescriptor role : roles) { + Exception validationException; try { - xContentBuilder = role.toXContent( - jsonBuilder(), - ToXContent.EMPTY_PARAMS, - true, - featureService.clusterHasFeature(clusterService.state(), SECURITY_ROLES_METADATA_FLATTENED) - ); - } catch (IOException e) { - listener.onFailure(e); - return; + validationException = validateRoleDescriptor(role); + } catch (Exception e) { + validationException = e; + } + + if (validationException != null) { + validationErrorByRoleName.put(role.getName(), validationException); + } else { + try { + bulkRequest.add(createRoleUpsertRequest(role)); + } catch (IOException ioException) { + listener.onFailure(ioException); + } } - final IndexRequest indexRequest = client.prepareIndex(SECURITY_MAIN_ALIAS) - .setId(getIdForRole(roleName)) - .setSource(xContentBuilder) - .setRefreshPolicy(request.getRefreshPolicy()) - .request(); - executeAsyncWithOrigin( + } + + List roleNames = roles.stream().map(RoleDescriptor::getName).toList(); + + if (bulkRequest.numberOfActions() == 0) { + BulkPutRolesResponse.Builder bulkPutRolesResponseBuilder = new BulkPutRolesResponse.Builder(); + roleNames.stream() + .map(roleName -> BulkPutRolesResponse.Item.failure(roleName, validationErrorByRoleName.get(roleName))) + .forEach(bulkPutRolesResponseBuilder::addItem); + + listener.onResponse(bulkPutRolesResponseBuilder.build()); + return; + } + securityIndex.prepareIndexIfNeededThenExecute( + listener::onFailure, + () -> executeAsyncWithOrigin( client.threadPool().getThreadContext(), SECURITY_ORIGIN, - indexRequest, - new ActionListener() { + bulkRequest, + new ActionListener() { @Override - public void onResponse(DocWriteResponse indexResponse) { - final boolean created = indexResponse.getResult() == DocWriteResponse.Result.CREATED; - logger.trace("Created role: [{}]", indexRequest); - clearRoleCache(roleName, listener, created); + public void onResponse(BulkResponse bulkResponse) { + List rolesToRefreshInCache = new ArrayList<>(roleNames.size()); + + Iterator bulkItemResponses = bulkResponse.iterator(); + BulkPutRolesResponse.Builder bulkPutRolesResponseBuilder = new BulkPutRolesResponse.Builder(); + + roleNames.stream().map(roleName -> { + if (validationErrorByRoleName.containsKey(roleName)) { + return BulkPutRolesResponse.Item.failure(roleName, validationErrorByRoleName.get(roleName)); + } + BulkItemResponse resp = bulkItemResponses.next(); + if (resp.isFailed()) { + return BulkPutRolesResponse.Item.failure(roleName, resp.getFailure().getCause()); + } + if (UPDATE_ROLES_REFRESH_CACHE_RESULTS.contains(resp.getResponse().getResult())) { + rolesToRefreshInCache.add(roleName); + } + return BulkPutRolesResponse.Item.success(roleName, resp.getResponse().getResult()); + }).forEach(bulkPutRolesResponseBuilder::addItem); + + clearRoleCache(rolesToRefreshInCache.toArray(String[]::new), ActionListener.wrap(res -> { + listener.onResponse(bulkPutRolesResponseBuilder.build()); + }, listener::onFailure), bulkResponse); } @Override public void onFailure(Exception e) { - logger.error(() -> "failed to put role [" + roleName + "]", e); + logger.error(() -> "failed to put roles", e); listener.onFailure(e); } }, - client::index - ); - }); + client::bulk + ) + ); + } + + private IndexRequest createRoleIndexRequest(final RoleDescriptor role) throws IOException { + return client.prepareIndex(SECURITY_MAIN_ALIAS) + .setId(getIdForRole(role.getName())) + .setSource(createRoleXContentBuilder(role)) + .request(); + } + + private UpdateRequest createRoleUpsertRequest(final RoleDescriptor role) throws IOException { + return client.prepareUpdate(SECURITY_MAIN_ALIAS, getIdForRole(role.getName())) + .setDoc(createRoleXContentBuilder(role)) + .setDocAsUpsert(true) + .request(); + } + + private XContentBuilder createRoleXContentBuilder(RoleDescriptor role) throws IOException { + assert NativeRealmValidationUtil.validateRoleName(role.getName(), false) == null + : "Role name was invalid or reserved: " + role.getName(); + assert false == role.hasRestriction() : "restriction is not supported for native roles"; + return role.toXContent( + jsonBuilder(), + ToXContent.EMPTY_PARAMS, + true, + featureService.clusterHasFeature(clusterService.state(), SECURITY_ROLES_METADATA_FLATTENED) + ); } public void usageStats(ActionListener> listener) { @@ -498,7 +634,11 @@ private void executeGetRoleRequest(String role, ActionListener list } private void clearRoleCache(final String role, ActionListener listener, Response response) { - ClearRolesCacheRequest request = new ClearRolesCacheRequest().names(role); + clearRoleCache(new String[] { role }, listener, response); + } + + private void clearRoleCache(final String[] roles, ActionListener listener, Response response) { + ClearRolesCacheRequest request = new ClearRolesCacheRequest().names(roles); executeAsyncWithOrigin(client, SECURITY_ORIGIN, ClearRolesCacheAction.INSTANCE, request, new ActionListener<>() { @Override public void onResponse(ClearRolesCacheResponse nodes) { @@ -507,9 +647,9 @@ public void onResponse(ClearRolesCacheResponse nodes) { @Override public void onFailure(Exception e) { - logger.error(() -> "unable to clear cache for role [" + role + "]", e); + logger.error(() -> "unable to clear cache for roles [" + Arrays.toString(roles) + "]", e); ElasticsearchException exception = new ElasticsearchException( - "clearing the cache for [" + role + "] failed. please clear the role cache manually", + "clearing the cache for [" + Arrays.toString(roles) + "] failed. please clear the role cache manually", e ); listener.onFailure(exception); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestBulkPutRolesAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestBulkPutRolesAction.java new file mode 100644 index 0000000000000..f132da09c4ec0 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestBulkPutRolesAction.java @@ -0,0 +1,56 @@ +/* + * 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.rest.action.role; + +import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.client.internal.node.NodeClient; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.license.XPackLicenseState; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.action.RestToXContentListener; +import org.elasticsearch.xpack.core.security.action.role.BulkPutRoleRequestBuilder; +import org.elasticsearch.xpack.core.security.action.role.BulkPutRoleRequestBuilderFactory; + +import java.io.IOException; +import java.util.List; + +import static org.elasticsearch.rest.RestRequest.Method.POST; + +/** + * Rest endpoint to bulk add a Roles to the security index + */ +public class RestBulkPutRolesAction extends NativeRoleBaseRestHandler { + + private final BulkPutRoleRequestBuilderFactory builderFactory; + + public RestBulkPutRolesAction(Settings settings, XPackLicenseState licenseState, BulkPutRoleRequestBuilderFactory builderFactory) { + super(settings, licenseState); + this.builderFactory = builderFactory; + } + + @Override + public List routes() { + return List.of(Route.builder(POST, "/_security/role").build()); + } + + @Override + public String getName() { + return "security_bulk_put_roles_action"; + } + + @Override + protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClient client) throws IOException { + final BulkPutRoleRequestBuilder requestBuilder = builderFactory.create(client) + .content(request.requiredContent(), request.getXContentType()); + + if (request.param("refresh") != null) { + requestBuilder.setRefreshPolicy(WriteRequest.RefreshPolicy.parse(request.param("refresh"))); + } + + return channel -> requestBuilder.execute(new RestToXContentListener<>(channel)); + } +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java index 8610273f205c9..759bc80ac511f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java @@ -6,13 +6,9 @@ */ package org.elasticsearch.xpack.security.action.role; -import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; @@ -28,21 +24,16 @@ import org.elasticsearch.xcontent.ParseField; import org.elasticsearch.xpack.core.security.action.role.PutRoleRequest; import org.elasticsearch.xpack.core.security.action.role.PutRoleResponse; -import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; -import org.elasticsearch.xpack.security.authz.ReservedRoleNameChecker; import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; import org.junit.BeforeClass; -import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicReference; import static org.elasticsearch.test.ActionListenerUtils.anyActionListener; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; @@ -53,7 +44,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; public class TransportPutRoleActionTests extends ESTestCase { @@ -92,109 +82,6 @@ protected NamedXContentRegistry xContentRegistry() { ); } - public void testReservedRole() { - final String roleName = randomFrom(new ArrayList<>(ReservedRolesStore.names())); - NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService( - Settings.EMPTY, - mock(Transport.class), - mock(ThreadPool.class), - TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, - null, - Collections.emptySet() - ); - TransportPutRoleAction action = new TransportPutRoleAction( - mock(ActionFilters.class), - rolesStore, - transportService, - xContentRegistry(), - new ReservedRoleNameChecker.Default() - ); - - PutRoleRequest request = new PutRoleRequest(); - request.name(roleName); - - final AtomicReference throwableRef = new AtomicReference<>(); - final AtomicReference responseRef = new AtomicReference<>(); - action.doExecute(mock(Task.class), request, new ActionListener() { - @Override - public void onResponse(PutRoleResponse response) { - responseRef.set(response); - } - - @Override - public void onFailure(Exception e) { - throwableRef.set(e); - } - }); - - assertThat(responseRef.get(), is(nullValue())); - assertThat(throwableRef.get(), is(instanceOf(IllegalArgumentException.class))); - assertThat(throwableRef.get().getMessage(), containsString("is reserved and may not be used")); - verifyNoMoreInteractions(rolesStore); - } - - public void testValidRole() { - testValidRole(randomFrom("admin", "dept_a", "restricted")); - } - - public void testValidRoleWithInternalRoleName() { - testValidRole(AuthenticationTestHelper.randomInternalRoleName()); - } - - private void testValidRole(String roleName) { - NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService( - Settings.EMPTY, - mock(Transport.class), - mock(ThreadPool.class), - TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, - null, - Collections.emptySet() - ); - TransportPutRoleAction action = new TransportPutRoleAction( - mock(ActionFilters.class), - rolesStore, - transportService, - xContentRegistry(), - new ReservedRoleNameChecker.Default() - ); - - final boolean created = randomBoolean(); - PutRoleRequest request = new PutRoleRequest(); - request.name(roleName); - - doAnswer(invocation -> { - Object[] args = invocation.getArguments(); - assert args.length == 3; - @SuppressWarnings("unchecked") - ActionListener listener = (ActionListener) args[2]; - listener.onResponse(created); - return null; - }).when(rolesStore).putRole(eq(request), any(RoleDescriptor.class), anyActionListener()); - - final AtomicReference throwableRef = new AtomicReference<>(); - final AtomicReference responseRef = new AtomicReference<>(); - action.doExecute(mock(Task.class), request, new ActionListener() { - @Override - public void onResponse(PutRoleResponse response) { - responseRef.set(response); - } - - @Override - public void onFailure(Exception e) { - throwableRef.set(e); - } - }); - - assertThat(responseRef.get(), is(notNullValue())); - assertThat(responseRef.get().isCreated(), is(created)); - assertThat(throwableRef.get(), is(nullValue())); - verify(rolesStore, times(1)).putRole(eq(request), any(RoleDescriptor.class), anyActionListener()); - } - public void testException() { final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException()); final String roleName = randomFrom("admin", "dept_a", "restricted"); @@ -208,17 +95,10 @@ public void testException() { null, Collections.emptySet() ); - TransportPutRoleAction action = new TransportPutRoleAction( - mock(ActionFilters.class), - rolesStore, - transportService, - xContentRegistry(), - new ReservedRoleNameChecker.Default() - ); + TransportPutRoleAction action = new TransportPutRoleAction(mock(ActionFilters.class), rolesStore, transportService); PutRoleRequest request = new PutRoleRequest(); request.name(roleName); - doAnswer(invocation -> { Object[] args = invocation.getArguments(); assert args.length == 3; @@ -226,11 +106,11 @@ public void testException() { ActionListener listener = (ActionListener) args[2]; listener.onFailure(e); return null; - }).when(rolesStore).putRole(eq(request), any(RoleDescriptor.class), anyActionListener()); + }).when(rolesStore).putRole(eq(request.getRefreshPolicy()), any(RoleDescriptor.class), anyActionListener()); final AtomicReference throwableRef = new AtomicReference<>(); final AtomicReference responseRef = new AtomicReference<>(); - action.doExecute(mock(Task.class), request, new ActionListener() { + action.doExecute(mock(Task.class), request, new ActionListener<>() { @Override public void onResponse(PutRoleResponse response) { responseRef.set(response); @@ -245,115 +125,6 @@ public void onFailure(Exception e) { assertThat(responseRef.get(), is(nullValue())); assertThat(throwableRef.get(), is(notNullValue())); assertThat(throwableRef.get(), is(sameInstance(e))); - verify(rolesStore, times(1)).putRole(eq(request), any(RoleDescriptor.class), anyActionListener()); - } - - public void testCreationOfRoleWithMalformedQueryJsonFails() { - NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService( - Settings.EMPTY, - mock(Transport.class), - mock(ThreadPool.class), - TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, - null, - Collections.emptySet() - ); - TransportPutRoleAction action = new TransportPutRoleAction( - mock(ActionFilters.class), - rolesStore, - transportService, - xContentRegistry(), - new ReservedRoleNameChecker.Default() - ); - PutRoleRequest request = new PutRoleRequest(); - request.name("test"); - String[] malformedQueryJson = new String[] { - "{ \"match_all\": { \"unknown_field\": \"\" } }", - "{ malformed JSON }", - "{ \"unknown\": {\"\"} }", - "{}" }; - BytesReference query = new BytesArray(randomFrom(malformedQueryJson)); - request.addIndex(new String[] { "idx1" }, new String[] { "read" }, null, null, query, randomBoolean()); - - final AtomicReference throwableRef = new AtomicReference<>(); - final AtomicReference responseRef = new AtomicReference<>(); - action.doExecute(mock(Task.class), request, new ActionListener() { - @Override - public void onResponse(PutRoleResponse response) { - responseRef.set(response); - } - - @Override - public void onFailure(Exception e) { - throwableRef.set(e); - } - }); - - assertThat(responseRef.get(), is(nullValue())); - assertThat(throwableRef.get(), is(notNullValue())); - Throwable t = throwableRef.get(); - assertThat(t, instanceOf(ElasticsearchParseException.class)); - assertThat( - t.getMessage(), - containsString( - "failed to parse field 'query' for indices [" - + Strings.arrayToCommaDelimitedString(new String[] { "idx1" }) - + "] at index privilege [0] of role descriptor" - ) - ); - } - - public void testCreationOfRoleWithUnsupportedQueryFails() throws Exception { - NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService( - Settings.EMPTY, - mock(Transport.class), - mock(ThreadPool.class), - TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, - null, - Collections.emptySet() - ); - TransportPutRoleAction action = new TransportPutRoleAction( - mock(ActionFilters.class), - rolesStore, - transportService, - xContentRegistry(), - new ReservedRoleNameChecker.Default() - ); - PutRoleRequest request = new PutRoleRequest(); - request.name("test"); - String hasChildQuery = "{ \"has_child\": { \"type\": \"child\", \"query\": { \"match_all\": {} } } }"; - String hasParentQuery = "{ \"has_parent\": { \"parent_type\": \"parent\", \"query\": { \"match_all\": {} } } }"; - BytesReference query = new BytesArray(randomFrom(hasChildQuery, hasParentQuery)); - request.addIndex(new String[] { "idx1" }, new String[] { "read" }, null, null, query, randomBoolean()); - - final AtomicReference throwableRef = new AtomicReference<>(); - final AtomicReference responseRef = new AtomicReference<>(); - action.doExecute(mock(Task.class), request, new ActionListener() { - @Override - public void onResponse(PutRoleResponse response) { - responseRef.set(response); - } - - @Override - public void onFailure(Exception e) { - throwableRef.set(e); - } - }); - - assertThat(responseRef.get(), is(nullValue())); - assertThat(throwableRef.get(), is(notNullValue())); - Throwable t = throwableRef.get(); - assertThat(t, instanceOf(ElasticsearchParseException.class)); - assertThat( - t.getMessage(), - containsString( - "failed to parse field 'query' for indices [" - + Strings.arrayToCommaDelimitedString(new String[] { "idx1" }) - + "] at index privilege [0] of role descriptor" - ) - ); + verify(rolesStore, times(1)).putRole(eq(request.getRefreshPolicy()), any(RoleDescriptor.class), anyActionListener()); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStoreTests.java index e14a25088f749..e22883d80cb8d 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStoreTests.java @@ -12,12 +12,19 @@ import org.elasticsearch.TransportVersion; import org.elasticsearch.TransportVersions; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.bulk.BulkRequest; +import org.elasticsearch.action.index.IndexRequest; +import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.support.PlainActionFuture; +import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.action.update.UpdateRequestBuilder; import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.metadata.MappingMetadata; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.routing.IndexRoutingTable; import org.elasticsearch.cluster.routing.IndexShardRoutingTable; @@ -28,6 +35,7 @@ import org.elasticsearch.cluster.routing.UnassignedInfo.Reason; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.cluster.version.CompatibilityVersions; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; @@ -45,17 +53,21 @@ import org.elasticsearch.test.TransportVersionUtils; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentType; -import org.elasticsearch.xpack.core.security.action.role.PutRoleRequest; +import org.elasticsearch.xpack.core.security.action.role.BulkPutRolesResponse; +import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges; import org.elasticsearch.xpack.core.security.authz.RoleRestrictionTests; import org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissionGroup; import org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions; import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; +import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; +import org.elasticsearch.xpack.security.authz.ReservedRoleNameChecker; import org.elasticsearch.xpack.security.support.SecurityIndexManager; import org.elasticsearch.xpack.security.support.SecuritySystemIndices; import org.elasticsearch.xpack.security.test.SecurityTestUtils; @@ -68,12 +80,15 @@ import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_FORMAT_SETTING; +import static org.elasticsearch.indices.SystemIndexDescriptor.VERSION_META_KEY; import static org.elasticsearch.transport.RemoteClusterPortSettings.TRANSPORT_VERSION_ADVANCED_REMOTE_CLUSTER_SECURITY; import static org.elasticsearch.xpack.core.security.SecurityField.DOCUMENT_LEVEL_SECURITY_FEATURE; import static org.elasticsearch.xpack.core.security.authz.RoleDescriptorTestHelper.randomApplicationPrivileges; @@ -89,7 +104,10 @@ import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.core.IsNull.notNullValue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -97,16 +115,63 @@ public class NativeRolesStoreTests extends ESTestCase { private ThreadPool threadPool; + private final Client client = mock(Client.class); + @Before - public void createThreadPool() { + public void beforeNativeRoleStoreTests() { threadPool = new TestThreadPool("index audit trail update mapping tests"); + when(client.threadPool()).thenReturn(threadPool); + when(client.prepareIndex(SECURITY_MAIN_ALIAS)).thenReturn(new IndexRequestBuilder(client)); + when(client.prepareUpdate(any(), any())).thenReturn(new UpdateRequestBuilder(client)); } @After - public void terminateThreadPool() throws Exception { + public void terminateThreadPool() { terminate(threadPool); } + private NativeRolesStore createRoleStoreForTest() { + return createRoleStoreForTest(Settings.builder().build()); + } + + private NativeRolesStore createRoleStoreForTest(Settings settings) { + new ReservedRolesStore(Set.of("superuser")); + final ClusterService clusterService = mock(ClusterService.class); + final SecuritySystemIndices systemIndices = new SecuritySystemIndices(settings); + final FeatureService featureService = mock(FeatureService.class); + systemIndices.init(client, featureService, clusterService); + final SecurityIndexManager securityIndex = systemIndices.getMainIndexManager(); + // Create the index + securityIndex.clusterChanged(new ClusterChangedEvent("source", getClusterStateWithSecurityIndex(), getEmptyClusterState())); + + return new NativeRolesStore( + settings, + client, + TestUtils.newTestLicenseState(), + securityIndex, + clusterService, + mock(FeatureService.class), + new ReservedRoleNameChecker.Default(), + mock(NamedXContentRegistry.class) + ); + } + + private void putRole(NativeRolesStore rolesStore, RoleDescriptor roleDescriptor, ActionListener actionListener) + throws IOException { + if (randomBoolean()) { + rolesStore.putRole(WriteRequest.RefreshPolicy.IMMEDIATE, roleDescriptor, actionListener); + } else { + rolesStore.putRoles(WriteRequest.RefreshPolicy.IMMEDIATE, List.of(roleDescriptor), ActionListener.wrap(resp -> { + BulkPutRolesResponse.Item item = resp.getItems().get(0); + if (item.getResultType().equals("created")) { + actionListener.onResponse(true); + } else { + throw item.getCause(); + } + }, actionListener::onFailure)); + } + } + // test that we can read a role where field permissions are stored in 2.x format (fields:...) public void testBWCFieldPermissions() throws IOException { Path path = getDataPath("roles2xformat.json"); @@ -329,35 +394,28 @@ public void testPutOfRoleWithFlsDlsUnlicensed() throws IOException { final ClusterService clusterService = mockClusterServiceWithMinNodeVersion(TransportVersion.current()); final FeatureService featureService = mock(FeatureService.class); final XPackLicenseState licenseState = mock(XPackLicenseState.class); - final AtomicBoolean methodCalled = new AtomicBoolean(false); final SecuritySystemIndices systemIndices = new SecuritySystemIndices(clusterService.getSettings()); systemIndices.init(client, featureService, clusterService); final SecurityIndexManager securityIndex = systemIndices.getMainIndexManager(); - + // Init for validation + new ReservedRolesStore(Set.of("superuser")); final NativeRolesStore rolesStore = new NativeRolesStore( Settings.EMPTY, client, licenseState, securityIndex, clusterService, - mock(FeatureService.class) - ) { - @Override - void innerPutRole(final PutRoleRequest request, final RoleDescriptor role, final ActionListener listener) { - if (methodCalled.compareAndSet(false, true)) { - listener.onResponse(true); - } else { - fail("method called more than once!"); - } - } - }; + mock(FeatureService.class), + mock(ReservedRoleNameChecker.class), + mock(NamedXContentRegistry.class) + ); + // setup the roles store so the security index exists securityIndex.clusterChanged( new ClusterChangedEvent("fls_dls_license", getClusterStateWithSecurityIndex(), getEmptyClusterState()) ); - PutRoleRequest putRoleRequest = new PutRoleRequest(); RoleDescriptor flsRole = new RoleDescriptor( "fls", null, @@ -366,8 +424,9 @@ void innerPutRole(final PutRoleRequest request, final RoleDescriptor role, final null ); PlainActionFuture future = new PlainActionFuture<>(); - rolesStore.putRole(putRoleRequest, flsRole, future); + putRole(rolesStore, flsRole, future); ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, future::actionGet); + assertThat(e.getMessage(), containsString("field and document level security")); BytesReference matchAllBytes = XContentHelper.toXContent(QueryBuilders.matchAllQuery(), XContentType.JSON, false); @@ -378,7 +437,7 @@ void innerPutRole(final PutRoleRequest request, final RoleDescriptor role, final null ); future = new PlainActionFuture<>(); - rolesStore.putRole(putRoleRequest, dlsRole, future); + putRole(rolesStore, dlsRole, future); e = expectThrows(ElasticsearchSecurityException.class, future::actionGet); assertThat(e.getMessage(), containsString("field and document level security")); @@ -396,22 +455,14 @@ void innerPutRole(final PutRoleRequest request, final RoleDescriptor role, final null ); future = new PlainActionFuture<>(); - rolesStore.putRole(putRoleRequest, flsDlsRole, future); + putRole(rolesStore, flsDlsRole, future); e = expectThrows(ElasticsearchSecurityException.class, future::actionGet); assertThat(e.getMessage(), containsString("field and document level security")); - - RoleDescriptor noFlsDlsRole = new RoleDescriptor( - "no_fls_dls", - null, - new IndicesPrivileges[] { IndicesPrivileges.builder().indices("*").privileges("READ").build() }, - null - ); - future = new PlainActionFuture<>(); - rolesStore.putRole(putRoleRequest, noFlsDlsRole, future); - assertTrue(future.actionGet()); } - public void testPutRoleWithRemotePrivsUnsupportedMinNodeVersion() { + public void testPutRoleWithRemotePrivsUnsupportedMinNodeVersion() throws IOException { + // Init for validation + new ReservedRolesStore(Set.of("superuser")); enum TEST_MODE { REMOTE_INDICES_PRIVS, REMOTE_CLUSTER_PRIVS, @@ -449,7 +500,6 @@ enum TEST_MODE { final ClusterService clusterService = mockClusterServiceWithMinNodeVersion(minTransportVersion); final XPackLicenseState licenseState = mock(XPackLicenseState.class); - final AtomicBoolean methodCalled = new AtomicBoolean(false); final SecuritySystemIndices systemIndices = new SecuritySystemIndices(clusterService.getSettings()); final FeatureService featureService = mock(FeatureService.class); @@ -462,21 +512,13 @@ enum TEST_MODE { licenseState, securityIndex, clusterService, - mock(FeatureService.class) - ) { - @Override - void innerPutRole(final PutRoleRequest request, final RoleDescriptor role, final ActionListener listener) { - if (methodCalled.compareAndSet(false, true)) { - listener.onResponse(true); - } else { - fail("method called more than once!"); - } - } - }; + mock(FeatureService.class), + mock(ReservedRoleNameChecker.class), + mock(NamedXContentRegistry.class) + ); // setup the roles store so the security index exists securityIndex.clusterChanged(new ClusterChangedEvent("source", getClusterStateWithSecurityIndex(), getEmptyClusterState())); - PutRoleRequest putRoleRequest = new PutRoleRequest(); RoleDescriptor remoteIndicesRole = new RoleDescriptor( "remote", null, @@ -492,7 +534,7 @@ void innerPutRole(final PutRoleRequest request, final RoleDescriptor role, final null ); PlainActionFuture future = new PlainActionFuture<>(); - rolesStore.putRole(putRoleRequest, remoteIndicesRole, future); + putRole(rolesStore, remoteIndicesRole, future); IllegalStateException e = expectThrows( IllegalStateException.class, String.format(Locale.ROOT, "expected IllegalStateException, but not thrown for mode [%s]", testMode), @@ -515,22 +557,7 @@ void innerPutRole(final PutRoleRequest request, final RoleDescriptor role, final public void testGetRoleWhenDisabled() throws Exception { final Settings settings = Settings.builder().put(NativeRolesStore.NATIVE_ROLES_ENABLED, "false").build(); - final Client client = mock(Client.class); - final ClusterService clusterService = mock(ClusterService.class); - final XPackLicenseState licenseState = mock(XPackLicenseState.class); - final SecuritySystemIndices systemIndices = new SecuritySystemIndices(settings); - final FeatureService featureService = mock(FeatureService.class); - systemIndices.init(client, featureService, clusterService); - final SecurityIndexManager securityIndex = systemIndices.getMainIndexManager(); - - final NativeRolesStore store = new NativeRolesStore( - settings, - client, - licenseState, - securityIndex, - clusterService, - mock(FeatureService.class) - ); + NativeRolesStore store = createRoleStoreForTest(settings); final PlainActionFuture future = new PlainActionFuture<>(); store.getRoleDescriptors(Set.of(randomAlphaOfLengthBetween(4, 12)), future); @@ -541,6 +568,210 @@ public void testGetRoleWhenDisabled() throws Exception { Mockito.verifyNoInteractions(client); } + public void testReservedRole() { + final NativeRolesStore store = createRoleStoreForTest(); + final String roleName = randomFrom(new ArrayList<>(ReservedRolesStore.names())); + + RoleDescriptor roleDescriptor = new RoleDescriptor( + roleName, + randomSubsetOf(ClusterPrivilegeResolver.names()).toArray(String[]::new), + new IndicesPrivileges[] { + IndicesPrivileges.builder().privileges("READ").indices("*").grantedFields("*").deniedFields("foo").build() }, + randomApplicationPrivileges(), + randomClusterPrivileges(), + generateRandomStringArray(5, randomIntBetween(2, 8), true, true), + randomRoleDescriptorMetadata(ESTestCase.randomBoolean()), + null, + randomRemoteIndicesPrivileges(1, 2), + null, + null, + randomAlphaOfLengthBetween(0, 20) + ); + ActionRequestValidationException exception = assertThrows(ActionRequestValidationException.class, () -> { + PlainActionFuture future = new PlainActionFuture<>(); + putRole(store, roleDescriptor, future); + future.actionGet(); + }); + + assertThat(exception.getMessage(), containsString("is reserved and may not be used")); + } + + public void testValidRole() throws IOException { + testValidRole(randomFrom("admin", "dept_a", "restricted")); + } + + public void testValidRoleWithInternalRoleName() throws IOException { + testValidRole(AuthenticationTestHelper.randomInternalRoleName()); + } + + private void testValidRole(String roleName) throws IOException { + final NativeRolesStore rolesStore = createRoleStoreForTest(); + + RoleDescriptor roleDescriptor = new RoleDescriptor( + roleName, + randomSubsetOf(ClusterPrivilegeResolver.names()).toArray(String[]::new), + new IndicesPrivileges[] { + IndicesPrivileges.builder().privileges("READ").indices("*").grantedFields("*").deniedFields("foo").build() }, + randomApplicationPrivileges(), + randomClusterPrivileges(), + generateRandomStringArray(5, randomIntBetween(2, 8), true, true), + null, + null, + null, + null, + null, + null + ); + + putRole(rolesStore, roleDescriptor, ActionListener.wrap(response -> fail(), exception -> fail())); + boolean indexCalled = false; + try { + verify(client, times(1)).index(any(IndexRequest.class), any()); + indexCalled = true; + } catch (AssertionError assertionError) { + // Index wasn't called + } + + boolean bulkCalled = false; + try { + verify(client, times(1)).bulk(any(BulkRequest.class), any()); + bulkCalled = true; + } catch (AssertionError assertionError) { + // bulk wasn't called + } + + assertTrue(bulkCalled || indexCalled); + } + + public void testCreationOfRoleWithMalformedQueryJsonFails() throws IOException { + final NativeRolesStore rolesStore = createRoleStoreForTest(); + + String[] malformedQueryJson = new String[] { + "{ \"match_all\": { \"unknown_field\": \"\" } }", + "{ malformed JSON }", + "{ \"unknown\": {\"\"} }", + "{}" }; + + BytesReference query = new BytesArray(randomFrom(malformedQueryJson)); + + RoleDescriptor roleDescriptor = new RoleDescriptor( + "test", + randomSubsetOf(ClusterPrivilegeResolver.names()).toArray(String[]::new), + new IndicesPrivileges[] { + RoleDescriptor.IndicesPrivileges.builder() + .indices("idx1") + .privileges(new String[] { "read" }) + .query(query) + .allowRestrictedIndices(randomBoolean()) + .build() }, + randomApplicationPrivileges(), + randomClusterPrivileges(), + null, + null, + null, + null, + null, + null, + null + ); + + final AtomicReference throwableRef = new AtomicReference<>(); + final AtomicReference responseRef = new AtomicReference<>(); + + putRole(rolesStore, roleDescriptor, ActionListener.wrap(responseRef::set, throwableRef::set)); + + assertThat(responseRef.get(), is(nullValue())); + assertThat(throwableRef.get(), is(notNullValue())); + Throwable t = throwableRef.get(); + assertThat(t, instanceOf(ElasticsearchParseException.class)); + assertThat( + t.getMessage(), + containsString( + "failed to parse field 'query' for indices [" + + Strings.arrayToCommaDelimitedString(new String[] { "idx1" }) + + "] at index privilege [0] of role descriptor" + ) + ); + } + + public void testCreationOfRoleWithUnsupportedQueryFails() throws IOException { + final NativeRolesStore rolesStore = createRoleStoreForTest(); + + String hasChildQuery = "{ \"has_child\": { \"type\": \"child\", \"query\": { \"match_all\": {} } } }"; + String hasParentQuery = "{ \"has_parent\": { \"parent_type\": \"parent\", \"query\": { \"match_all\": {} } } }"; + + BytesReference query = new BytesArray(randomFrom(hasChildQuery, hasParentQuery)); + + RoleDescriptor roleDescriptor = new RoleDescriptor( + "test", + randomSubsetOf(ClusterPrivilegeResolver.names()).toArray(String[]::new), + new IndicesPrivileges[] { + RoleDescriptor.IndicesPrivileges.builder() + .indices("idx1") + .privileges(new String[] { "read" }) + .query(query) + .allowRestrictedIndices(randomBoolean()) + .build() }, + randomApplicationPrivileges(), + randomClusterPrivileges(), + null, + null, + null, + null, + null, + null, + null + ); + + final AtomicReference throwableRef = new AtomicReference<>(); + final AtomicReference responseRef = new AtomicReference<>(); + putRole(rolesStore, roleDescriptor, ActionListener.wrap(responseRef::set, throwableRef::set)); + + assertThat(responseRef.get(), is(nullValue())); + assertThat(throwableRef.get(), is(notNullValue())); + Throwable t = throwableRef.get(); + assertThat(t, instanceOf(ElasticsearchParseException.class)); + assertThat( + t.getMessage(), + containsString( + "failed to parse field 'query' for indices [" + + Strings.arrayToCommaDelimitedString(new String[] { "idx1" }) + + "] at index privilege [0] of role descriptor" + ) + ); + } + + public void testManyValidRoles() throws IOException { + final NativeRolesStore rolesStore = createRoleStoreForTest(); + List roleNames = List.of("test", "admin", "123"); + + List roleDescriptors = roleNames.stream() + .map( + roleName -> new RoleDescriptor( + roleName, + randomSubsetOf(ClusterPrivilegeResolver.names()).toArray(String[]::new), + new IndicesPrivileges[] { + IndicesPrivileges.builder().privileges("READ").indices("*").grantedFields("*").deniedFields("foo").build() }, + randomApplicationPrivileges(), + randomClusterPrivileges(), + generateRandomStringArray(5, randomIntBetween(2, 8), true, true), + null, + null, + null, + null, + null, + null + ) + ) + .toList(); + + AtomicReference response = new AtomicReference<>(); + AtomicReference exception = new AtomicReference<>(); + rolesStore.putRoles(WriteRequest.RefreshPolicy.IMMEDIATE, roleDescriptors, ActionListener.wrap(response::set, exception::set)); + assertNull(exception.get()); + verify(client, times(1)).bulk(any(BulkRequest.class), any()); + } + private ClusterService mockClusterServiceWithMinNodeVersion(TransportVersion transportVersion) { final ClusterService clusterService = mock(ClusterService.class, Mockito.RETURNS_DEEP_STUBS); when(clusterService.state().getMinTransportVersion()).thenReturn(transportVersion); @@ -552,8 +783,14 @@ private ClusterState getClusterStateWithSecurityIndex() { final boolean withAlias = randomBoolean(); final String securityIndexName = SECURITY_MAIN_ALIAS + (withAlias ? "-" + randomAlphaOfLength(5) : ""); + Settings.Builder settingsBuilder = indexSettings(IndexVersion.current(), 1, 0); + settingsBuilder.put(INDEX_FORMAT_SETTING.getKey(), SecuritySystemIndices.INTERNAL_MAIN_INDEX_FORMAT); + settingsBuilder.put(VERSION_META_KEY, 1); + MappingMetadata mappingMetadata = mock(MappingMetadata.class); + when(mappingMetadata.sourceAsMap()).thenReturn(Map.of("_meta", Map.of(VERSION_META_KEY, 1))); + when(mappingMetadata.getSha256()).thenReturn("test"); Metadata metadata = Metadata.builder() - .put(IndexMetadata.builder(securityIndexName).settings(indexSettings(IndexVersion.current(), 1, 0))) + .put(IndexMetadata.builder(securityIndexName).putMapping(mappingMetadata).settings(settingsBuilder)) .build(); if (withAlias) { diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/roles/60_bulk_roles.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/roles/60_bulk_roles.yml new file mode 100644 index 0000000000000..72a240ab92695 --- /dev/null +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/roles/60_bulk_roles.yml @@ -0,0 +1,83 @@ +--- +setup: + - skip: + features: headers + + - do: + cluster.health: + wait_for_status: yellow + - do: + security.put_user: + username: "joe" + body: > + { + "password": "s3krit-password", + "roles" : [ "admin_role" ] + } + +--- +teardown: + - do: + security.delete_user: + username: "joe" + ignore: 404 + - do: + security.delete_role: + name: "admin_role" + ignore: 404 + - do: + security.delete_role: + name: "role_with_description" + ignore: 404 +--- +"Test bulk put roles api": + - do: + security.bulk_put_role: + body: > + { + "roles": { + "admin_role": { + "cluster": [ + "all" + ], + "metadata": { + "key1": "val1", + "key2": "val2" + }, + "indices": [ + { + "names": "*", + "privileges": [ + "all" + ] + } + ] + }, + "role_with_description": { + "description": "Allows all security-related operations such as CRUD operations on users and roles and cache clearing.", + "cluster": [ + "manage_security" + ] + } + } + } + - match: { created: ["admin_role", "role_with_description"] } + + - do: + headers: + Authorization: "Basic am9lOnMza3JpdC1wYXNzd29yZA==" + security.get_role: + name: "admin_role" + - match: { admin_role.cluster.0: "all" } + - match: { admin_role.metadata.key1: "val1" } + - match: { admin_role.metadata.key2: "val2" } + - match: { admin_role.indices.0.names.0: "*" } + - match: { admin_role.indices.0.privileges.0: "all" } + + - do: + headers: + Authorization: "Basic am9lOnMza3JpdC1wYXNzd29yZA==" + security.get_role: + name: "role_with_description" + - match: { role_with_description.cluster.0: "manage_security" } + - match: { role_with_description.description: "Allows all security-related operations such as CRUD operations on users and roles and cache clearing." } From 0eb3ee235736504324d2467c9ad6a3db4da008a0 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Tue, 2 Jul 2024 10:50:50 -0400 Subject: [PATCH 3/3] fix compile error related to new PARTIAL_AGG data type (#110379) --- .../TopDoubleAggregatorFunction.java | 16 ++++++------- .../TopDoubleAggregatorFunctionSupplier.java | 3 +-- .../TopDoubleGroupingAggregatorFunction.java | 16 ++++++------- .../TopFloatAggregatorFunction.java | 16 ++++++------- .../TopFloatAggregatorFunctionSupplier.java | 3 +-- .../TopFloatGroupingAggregatorFunction.java | 16 ++++++------- .../aggregation/TopIntAggregatorFunction.java | 18 +++++++------- .../TopIntAggregatorFunctionSupplier.java | 3 +-- .../TopIntGroupingAggregatorFunction.java | 16 ++++++------- .../TopLongAggregatorFunction.java | 16 ++++++------- .../TopLongAggregatorFunctionSupplier.java | 3 +-- .../TopLongGroupingAggregatorFunction.java | 16 ++++++------- .../xpack/esql/action/TimeSeriesIT.java | 24 +++++++++---------- .../xpack/esql/action/PositionToXContent.java | 4 ++-- .../xpack/esql/action/ResponseValueUtils.java | 2 +- 15 files changed, 84 insertions(+), 88 deletions(-) diff --git a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopDoubleAggregatorFunction.java b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopDoubleAggregatorFunction.java index 3d658294c154f..8549da42c0d85 100644 --- a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopDoubleAggregatorFunction.java +++ b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopDoubleAggregatorFunction.java @@ -22,7 +22,7 @@ */ public final class TopDoubleAggregatorFunction implements AggregatorFunction { private static final List INTERMEDIATE_STATE_DESC = List.of( - new IntermediateStateDesc("topList", ElementType.DOUBLE) ); + new IntermediateStateDesc("top", ElementType.DOUBLE) ); private final DriverContext driverContext; @@ -35,7 +35,7 @@ public final class TopDoubleAggregatorFunction implements AggregatorFunction { private final boolean ascending; public TopDoubleAggregatorFunction(DriverContext driverContext, List channels, - TopDoubleAggregator.SingleState state, int limit, boolean ascending) { + TopDoubleAggregator.SingleState state, int limit, boolean ascending) { this.driverContext = driverContext; this.channels = channels; this.state = state; @@ -44,7 +44,7 @@ public TopDoubleAggregatorFunction(DriverContext driverContext, List ch } public static TopDoubleAggregatorFunction create(DriverContext driverContext, - List channels, int limit, boolean ascending) { + List channels, int limit, boolean ascending) { return new TopDoubleAggregatorFunction(driverContext, channels, TopDoubleAggregator.initSingle(driverContext.bigArrays(), limit, ascending), limit, ascending); } @@ -91,13 +91,13 @@ private void addRawBlock(DoubleBlock block) { public void addIntermediateInput(Page page) { assert channels.size() == intermediateBlockCount(); assert page.getBlockCount() >= channels.get(0) + intermediateStateDesc().size(); - Block topListUncast = page.getBlock(channels.get(0)); - if (topListUncast.areAllValuesNull()) { + Block topUncast = page.getBlock(channels.get(0)); + if (topUncast.areAllValuesNull()) { return; } - DoubleBlock topList = (DoubleBlock) topListUncast; - assert topList.getPositionCount() == 1; - TopDoubleAggregator.combineIntermediate(state, topList); + DoubleBlock top = (DoubleBlock) topUncast; + assert top.getPositionCount() == 1; + TopDoubleAggregator.combineIntermediate(state, top); } @Override diff --git a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopDoubleAggregatorFunctionSupplier.java b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopDoubleAggregatorFunctionSupplier.java index b781af87ddc82..36a8763b4a870 100644 --- a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopDoubleAggregatorFunctionSupplier.java +++ b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopDoubleAggregatorFunctionSupplier.java @@ -21,8 +21,7 @@ public final class TopDoubleAggregatorFunctionSupplier implements AggregatorFunc private final boolean ascending; - public TopDoubleAggregatorFunctionSupplier(List channels, int limit, - boolean ascending) { + public TopDoubleAggregatorFunctionSupplier(List channels, int limit, boolean ascending) { this.channels = channels; this.limit = limit; this.ascending = ascending; diff --git a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopDoubleGroupingAggregatorFunction.java b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopDoubleGroupingAggregatorFunction.java index 493e76d23a85f..c54dce5715846 100644 --- a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopDoubleGroupingAggregatorFunction.java +++ b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopDoubleGroupingAggregatorFunction.java @@ -24,7 +24,7 @@ */ public final class TopDoubleGroupingAggregatorFunction implements GroupingAggregatorFunction { private static final List INTERMEDIATE_STATE_DESC = List.of( - new IntermediateStateDesc("topList", ElementType.DOUBLE) ); + new IntermediateStateDesc("top", ElementType.DOUBLE) ); private final TopDoubleAggregator.GroupingState state; @@ -37,8 +37,8 @@ public final class TopDoubleGroupingAggregatorFunction implements GroupingAggreg private final boolean ascending; public TopDoubleGroupingAggregatorFunction(List channels, - TopDoubleAggregator.GroupingState state, DriverContext driverContext, int limit, - boolean ascending) { + TopDoubleAggregator.GroupingState state, DriverContext driverContext, int limit, + boolean ascending) { this.channels = channels; this.state = state; this.driverContext = driverContext; @@ -47,7 +47,7 @@ public TopDoubleGroupingAggregatorFunction(List channels, } public static TopDoubleGroupingAggregatorFunction create(List channels, - DriverContext driverContext, int limit, boolean ascending) { + DriverContext driverContext, int limit, boolean ascending) { return new TopDoubleGroupingAggregatorFunction(channels, TopDoubleAggregator.initGrouping(driverContext.bigArrays(), limit, ascending), driverContext, limit, ascending); } @@ -154,14 +154,14 @@ private void addRawInput(int positionOffset, IntBlock groups, DoubleVector value public void addIntermediateInput(int positionOffset, IntVector groups, Page page) { state.enableGroupIdTracking(new SeenGroupIds.Empty()); assert channels.size() == intermediateBlockCount(); - Block topListUncast = page.getBlock(channels.get(0)); - if (topListUncast.areAllValuesNull()) { + Block topUncast = page.getBlock(channels.get(0)); + if (topUncast.areAllValuesNull()) { return; } - DoubleBlock topList = (DoubleBlock) topListUncast; + DoubleBlock top = (DoubleBlock) topUncast; for (int groupPosition = 0; groupPosition < groups.getPositionCount(); groupPosition++) { int groupId = Math.toIntExact(groups.getInt(groupPosition)); - TopDoubleAggregator.combineIntermediate(state, groupId, topList, groupPosition + positionOffset); + TopDoubleAggregator.combineIntermediate(state, groupId, top, groupPosition + positionOffset); } } diff --git a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopFloatAggregatorFunction.java b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopFloatAggregatorFunction.java index 674b534667863..40ac1432caee8 100644 --- a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopFloatAggregatorFunction.java +++ b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopFloatAggregatorFunction.java @@ -22,7 +22,7 @@ */ public final class TopFloatAggregatorFunction implements AggregatorFunction { private static final List INTERMEDIATE_STATE_DESC = List.of( - new IntermediateStateDesc("topList", ElementType.FLOAT) ); + new IntermediateStateDesc("top", ElementType.FLOAT) ); private final DriverContext driverContext; @@ -35,7 +35,7 @@ public final class TopFloatAggregatorFunction implements AggregatorFunction { private final boolean ascending; public TopFloatAggregatorFunction(DriverContext driverContext, List channels, - TopFloatAggregator.SingleState state, int limit, boolean ascending) { + TopFloatAggregator.SingleState state, int limit, boolean ascending) { this.driverContext = driverContext; this.channels = channels; this.state = state; @@ -44,7 +44,7 @@ public TopFloatAggregatorFunction(DriverContext driverContext, List cha } public static TopFloatAggregatorFunction create(DriverContext driverContext, - List channels, int limit, boolean ascending) { + List channels, int limit, boolean ascending) { return new TopFloatAggregatorFunction(driverContext, channels, TopFloatAggregator.initSingle(driverContext.bigArrays(), limit, ascending), limit, ascending); } @@ -91,13 +91,13 @@ private void addRawBlock(FloatBlock block) { public void addIntermediateInput(Page page) { assert channels.size() == intermediateBlockCount(); assert page.getBlockCount() >= channels.get(0) + intermediateStateDesc().size(); - Block topListUncast = page.getBlock(channels.get(0)); - if (topListUncast.areAllValuesNull()) { + Block topUncast = page.getBlock(channels.get(0)); + if (topUncast.areAllValuesNull()) { return; } - FloatBlock topList = (FloatBlock) topListUncast; - assert topList.getPositionCount() == 1; - TopFloatAggregator.combineIntermediate(state, topList); + FloatBlock top = (FloatBlock) topUncast; + assert top.getPositionCount() == 1; + TopFloatAggregator.combineIntermediate(state, top); } @Override diff --git a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopFloatAggregatorFunctionSupplier.java b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopFloatAggregatorFunctionSupplier.java index f40bbce1d73f6..e01df8329a315 100644 --- a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopFloatAggregatorFunctionSupplier.java +++ b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopFloatAggregatorFunctionSupplier.java @@ -21,8 +21,7 @@ public final class TopFloatAggregatorFunctionSupplier implements AggregatorFunct private final boolean ascending; - public TopFloatAggregatorFunctionSupplier(List channels, int limit, - boolean ascending) { + public TopFloatAggregatorFunctionSupplier(List channels, int limit, boolean ascending) { this.channels = channels; this.limit = limit; this.ascending = ascending; diff --git a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopFloatGroupingAggregatorFunction.java b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopFloatGroupingAggregatorFunction.java index 2555c0aeafec5..4c00f4d2c237d 100644 --- a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopFloatGroupingAggregatorFunction.java +++ b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopFloatGroupingAggregatorFunction.java @@ -24,7 +24,7 @@ */ public final class TopFloatGroupingAggregatorFunction implements GroupingAggregatorFunction { private static final List INTERMEDIATE_STATE_DESC = List.of( - new IntermediateStateDesc("topList", ElementType.FLOAT) ); + new IntermediateStateDesc("top", ElementType.FLOAT) ); private final TopFloatAggregator.GroupingState state; @@ -37,8 +37,8 @@ public final class TopFloatGroupingAggregatorFunction implements GroupingAggrega private final boolean ascending; public TopFloatGroupingAggregatorFunction(List channels, - TopFloatAggregator.GroupingState state, DriverContext driverContext, int limit, - boolean ascending) { + TopFloatAggregator.GroupingState state, DriverContext driverContext, int limit, + boolean ascending) { this.channels = channels; this.state = state; this.driverContext = driverContext; @@ -47,7 +47,7 @@ public TopFloatGroupingAggregatorFunction(List channels, } public static TopFloatGroupingAggregatorFunction create(List channels, - DriverContext driverContext, int limit, boolean ascending) { + DriverContext driverContext, int limit, boolean ascending) { return new TopFloatGroupingAggregatorFunction(channels, TopFloatAggregator.initGrouping(driverContext.bigArrays(), limit, ascending), driverContext, limit, ascending); } @@ -154,14 +154,14 @@ private void addRawInput(int positionOffset, IntBlock groups, FloatVector values public void addIntermediateInput(int positionOffset, IntVector groups, Page page) { state.enableGroupIdTracking(new SeenGroupIds.Empty()); assert channels.size() == intermediateBlockCount(); - Block topListUncast = page.getBlock(channels.get(0)); - if (topListUncast.areAllValuesNull()) { + Block topUncast = page.getBlock(channels.get(0)); + if (topUncast.areAllValuesNull()) { return; } - FloatBlock topList = (FloatBlock) topListUncast; + FloatBlock top = (FloatBlock) topUncast; for (int groupPosition = 0; groupPosition < groups.getPositionCount(); groupPosition++) { int groupId = Math.toIntExact(groups.getInt(groupPosition)); - TopFloatAggregator.combineIntermediate(state, groupId, topList, groupPosition + positionOffset); + TopFloatAggregator.combineIntermediate(state, groupId, top, groupPosition + positionOffset); } } diff --git a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopIntAggregatorFunction.java b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopIntAggregatorFunction.java index 94163e4915944..f6e858b69a639 100644 --- a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopIntAggregatorFunction.java +++ b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopIntAggregatorFunction.java @@ -22,7 +22,7 @@ */ public final class TopIntAggregatorFunction implements AggregatorFunction { private static final List INTERMEDIATE_STATE_DESC = List.of( - new IntermediateStateDesc("topList", ElementType.INT) ); + new IntermediateStateDesc("top", ElementType.INT) ); private final DriverContext driverContext; @@ -35,7 +35,7 @@ public final class TopIntAggregatorFunction implements AggregatorFunction { private final boolean ascending; public TopIntAggregatorFunction(DriverContext driverContext, List channels, - TopIntAggregator.SingleState state, int limit, boolean ascending) { + TopIntAggregator.SingleState state, int limit, boolean ascending) { this.driverContext = driverContext; this.channels = channels; this.state = state; @@ -43,8 +43,8 @@ public TopIntAggregatorFunction(DriverContext driverContext, List chann this.ascending = ascending; } - public static TopIntAggregatorFunction create(DriverContext driverContext, - List channels, int limit, boolean ascending) { + public static TopIntAggregatorFunction create(DriverContext driverContext, List channels, + int limit, boolean ascending) { return new TopIntAggregatorFunction(driverContext, channels, TopIntAggregator.initSingle(driverContext.bigArrays(), limit, ascending), limit, ascending); } @@ -91,13 +91,13 @@ private void addRawBlock(IntBlock block) { public void addIntermediateInput(Page page) { assert channels.size() == intermediateBlockCount(); assert page.getBlockCount() >= channels.get(0) + intermediateStateDesc().size(); - Block topListUncast = page.getBlock(channels.get(0)); - if (topListUncast.areAllValuesNull()) { + Block topUncast = page.getBlock(channels.get(0)); + if (topUncast.areAllValuesNull()) { return; } - IntBlock topList = (IntBlock) topListUncast; - assert topList.getPositionCount() == 1; - TopIntAggregator.combineIntermediate(state, topList); + IntBlock top = (IntBlock) topUncast; + assert top.getPositionCount() == 1; + TopIntAggregator.combineIntermediate(state, top); } @Override diff --git a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopIntAggregatorFunctionSupplier.java b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopIntAggregatorFunctionSupplier.java index df6502350c06c..4481f2d5afaa8 100644 --- a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopIntAggregatorFunctionSupplier.java +++ b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopIntAggregatorFunctionSupplier.java @@ -21,8 +21,7 @@ public final class TopIntAggregatorFunctionSupplier implements AggregatorFunctio private final boolean ascending; - public TopIntAggregatorFunctionSupplier(List channels, int limit, - boolean ascending) { + public TopIntAggregatorFunctionSupplier(List channels, int limit, boolean ascending) { this.channels = channels; this.limit = limit; this.ascending = ascending; diff --git a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopIntGroupingAggregatorFunction.java b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopIntGroupingAggregatorFunction.java index dbbc5ea6df650..37384238b7297 100644 --- a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopIntGroupingAggregatorFunction.java +++ b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopIntGroupingAggregatorFunction.java @@ -22,7 +22,7 @@ */ public final class TopIntGroupingAggregatorFunction implements GroupingAggregatorFunction { private static final List INTERMEDIATE_STATE_DESC = List.of( - new IntermediateStateDesc("topList", ElementType.INT) ); + new IntermediateStateDesc("top", ElementType.INT) ); private final TopIntAggregator.GroupingState state; @@ -35,8 +35,8 @@ public final class TopIntGroupingAggregatorFunction implements GroupingAggregato private final boolean ascending; public TopIntGroupingAggregatorFunction(List channels, - TopIntAggregator.GroupingState state, DriverContext driverContext, int limit, - boolean ascending) { + TopIntAggregator.GroupingState state, DriverContext driverContext, int limit, + boolean ascending) { this.channels = channels; this.state = state; this.driverContext = driverContext; @@ -45,7 +45,7 @@ public TopIntGroupingAggregatorFunction(List channels, } public static TopIntGroupingAggregatorFunction create(List channels, - DriverContext driverContext, int limit, boolean ascending) { + DriverContext driverContext, int limit, boolean ascending) { return new TopIntGroupingAggregatorFunction(channels, TopIntAggregator.initGrouping(driverContext.bigArrays(), limit, ascending), driverContext, limit, ascending); } @@ -152,14 +152,14 @@ private void addRawInput(int positionOffset, IntBlock groups, IntVector values) public void addIntermediateInput(int positionOffset, IntVector groups, Page page) { state.enableGroupIdTracking(new SeenGroupIds.Empty()); assert channels.size() == intermediateBlockCount(); - Block topListUncast = page.getBlock(channels.get(0)); - if (topListUncast.areAllValuesNull()) { + Block topUncast = page.getBlock(channels.get(0)); + if (topUncast.areAllValuesNull()) { return; } - IntBlock topList = (IntBlock) topListUncast; + IntBlock top = (IntBlock) topUncast; for (int groupPosition = 0; groupPosition < groups.getPositionCount(); groupPosition++) { int groupId = Math.toIntExact(groups.getInt(groupPosition)); - TopIntAggregator.combineIntermediate(state, groupId, topList, groupPosition + positionOffset); + TopIntAggregator.combineIntermediate(state, groupId, top, groupPosition + positionOffset); } } diff --git a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopLongAggregatorFunction.java b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopLongAggregatorFunction.java index 1887e958344ee..c355e401478d8 100644 --- a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopLongAggregatorFunction.java +++ b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopLongAggregatorFunction.java @@ -22,7 +22,7 @@ */ public final class TopLongAggregatorFunction implements AggregatorFunction { private static final List INTERMEDIATE_STATE_DESC = List.of( - new IntermediateStateDesc("topList", ElementType.LONG) ); + new IntermediateStateDesc("top", ElementType.LONG) ); private final DriverContext driverContext; @@ -35,7 +35,7 @@ public final class TopLongAggregatorFunction implements AggregatorFunction { private final boolean ascending; public TopLongAggregatorFunction(DriverContext driverContext, List channels, - TopLongAggregator.SingleState state, int limit, boolean ascending) { + TopLongAggregator.SingleState state, int limit, boolean ascending) { this.driverContext = driverContext; this.channels = channels; this.state = state; @@ -44,7 +44,7 @@ public TopLongAggregatorFunction(DriverContext driverContext, List chan } public static TopLongAggregatorFunction create(DriverContext driverContext, - List channels, int limit, boolean ascending) { + List channels, int limit, boolean ascending) { return new TopLongAggregatorFunction(driverContext, channels, TopLongAggregator.initSingle(driverContext.bigArrays(), limit, ascending), limit, ascending); } @@ -91,13 +91,13 @@ private void addRawBlock(LongBlock block) { public void addIntermediateInput(Page page) { assert channels.size() == intermediateBlockCount(); assert page.getBlockCount() >= channels.get(0) + intermediateStateDesc().size(); - Block topListUncast = page.getBlock(channels.get(0)); - if (topListUncast.areAllValuesNull()) { + Block topUncast = page.getBlock(channels.get(0)); + if (topUncast.areAllValuesNull()) { return; } - LongBlock topList = (LongBlock) topListUncast; - assert topList.getPositionCount() == 1; - TopLongAggregator.combineIntermediate(state, topList); + LongBlock top = (LongBlock) topUncast; + assert top.getPositionCount() == 1; + TopLongAggregator.combineIntermediate(state, top); } @Override diff --git a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopLongAggregatorFunctionSupplier.java b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopLongAggregatorFunctionSupplier.java index 3a41143be46ad..1a39c7b5580ec 100644 --- a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopLongAggregatorFunctionSupplier.java +++ b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopLongAggregatorFunctionSupplier.java @@ -21,8 +21,7 @@ public final class TopLongAggregatorFunctionSupplier implements AggregatorFuncti private final boolean ascending; - public TopLongAggregatorFunctionSupplier(List channels, int limit, - boolean ascending) { + public TopLongAggregatorFunctionSupplier(List channels, int limit, boolean ascending) { this.channels = channels; this.limit = limit; this.ascending = ascending; diff --git a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopLongGroupingAggregatorFunction.java b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopLongGroupingAggregatorFunction.java index 64564d0c49756..7b199b2a81389 100644 --- a/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopLongGroupingAggregatorFunction.java +++ b/x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/TopLongGroupingAggregatorFunction.java @@ -24,7 +24,7 @@ */ public final class TopLongGroupingAggregatorFunction implements GroupingAggregatorFunction { private static final List INTERMEDIATE_STATE_DESC = List.of( - new IntermediateStateDesc("topList", ElementType.LONG) ); + new IntermediateStateDesc("top", ElementType.LONG) ); private final TopLongAggregator.GroupingState state; @@ -37,8 +37,8 @@ public final class TopLongGroupingAggregatorFunction implements GroupingAggregat private final boolean ascending; public TopLongGroupingAggregatorFunction(List channels, - TopLongAggregator.GroupingState state, DriverContext driverContext, int limit, - boolean ascending) { + TopLongAggregator.GroupingState state, DriverContext driverContext, int limit, + boolean ascending) { this.channels = channels; this.state = state; this.driverContext = driverContext; @@ -47,7 +47,7 @@ public TopLongGroupingAggregatorFunction(List channels, } public static TopLongGroupingAggregatorFunction create(List channels, - DriverContext driverContext, int limit, boolean ascending) { + DriverContext driverContext, int limit, boolean ascending) { return new TopLongGroupingAggregatorFunction(channels, TopLongAggregator.initGrouping(driverContext.bigArrays(), limit, ascending), driverContext, limit, ascending); } @@ -154,14 +154,14 @@ private void addRawInput(int positionOffset, IntBlock groups, LongVector values) public void addIntermediateInput(int positionOffset, IntVector groups, Page page) { state.enableGroupIdTracking(new SeenGroupIds.Empty()); assert channels.size() == intermediateBlockCount(); - Block topListUncast = page.getBlock(channels.get(0)); - if (topListUncast.areAllValuesNull()) { + Block topUncast = page.getBlock(channels.get(0)); + if (topUncast.areAllValuesNull()) { return; } - LongBlock topList = (LongBlock) topListUncast; + LongBlock top = (LongBlock) topUncast; for (int groupPosition = 0; groupPosition < groups.getPositionCount(); groupPosition++) { int groupId = Math.toIntExact(groups.getInt(groupPosition)); - TopLongAggregator.combineIntermediate(state, groupId, topList, groupPosition + positionOffset); + TopLongAggregator.combineIntermediate(state, groupId, top, groupPosition + positionOffset); } } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TimeSeriesIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TimeSeriesIT.java index 02cecc63dbd0f..93f8c75ddb088 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TimeSeriesIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TimeSeriesIT.java @@ -258,10 +258,10 @@ record RateKey(String cluster, String host) { resp.columns(), equalTo( List.of( - new ColumnInfo("max(rate(request_count))", "double"), - new ColumnInfo("min(rate(request_count))", "double"), - new ColumnInfo("min(cpu)", "double"), - new ColumnInfo("max(cpu)", "double") + new ColumnInfoImpl("max(rate(request_count))", "double"), + new ColumnInfoImpl("min(rate(request_count))", "double"), + new ColumnInfoImpl("min(cpu)", "double"), + new ColumnInfoImpl("max(cpu)", "double") ) ) ); @@ -629,10 +629,10 @@ METRICS hosts sum(rate(request_count)), max(cpu) BY ts=bucket(@timestamp, 1 minu resp.columns(), equalTo( List.of( - new ColumnInfo("sum(rate(request_count))", "double"), - new ColumnInfo("max(cpu)", "double"), - new ColumnInfo("ts", "date"), - new ColumnInfo("cluster", "keyword") + new ColumnInfoImpl("sum(rate(request_count))", "double"), + new ColumnInfoImpl("max(cpu)", "double"), + new ColumnInfoImpl("ts", "date"), + new ColumnInfoImpl("cluster", "keyword") ) ) ); @@ -666,10 +666,10 @@ METRICS hosts sum(rate(request_count)), avg(cpu) BY ts=bucket(@timestamp, 1 minu resp.columns(), equalTo( List.of( - new ColumnInfo("sum(rate(request_count))", "double"), - new ColumnInfo("avg(cpu)", "double"), - new ColumnInfo("ts", "date"), - new ColumnInfo("cluster", "keyword") + new ColumnInfoImpl("sum(rate(request_count))", "double"), + new ColumnInfoImpl("avg(cpu)", "double"), + new ColumnInfoImpl("ts", "date"), + new ColumnInfoImpl("cluster", "keyword") ) ) ); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PositionToXContent.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PositionToXContent.java index 2cdbd9f5f93f1..0bc1eb46abefe 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PositionToXContent.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/PositionToXContent.java @@ -165,8 +165,8 @@ protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Pa } } }; - case DATE_PERIOD, TIME_DURATION, DOC_DATA_TYPE, TSID_DATA_TYPE, SHORT, BYTE, OBJECT, NESTED, FLOAT, HALF_FLOAT, SCALED_FLOAT -> - throw new IllegalArgumentException("can't convert values of type [" + columnInfo.type() + "]"); + case DATE_PERIOD, TIME_DURATION, DOC_DATA_TYPE, TSID_DATA_TYPE, SHORT, BYTE, OBJECT, NESTED, FLOAT, HALF_FLOAT, SCALED_FLOAT, + PARTIAL_AGG -> throw new IllegalArgumentException("can't convert values of type [" + columnInfo.type() + "]"); }; } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/ResponseValueUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/ResponseValueUtils.java index d99da4500a3b0..290a816275a29 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/ResponseValueUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/ResponseValueUtils.java @@ -152,7 +152,7 @@ private static Object valueAt(DataType dataType, Block block, int offset, BytesR } } case SHORT, BYTE, FLOAT, HALF_FLOAT, SCALED_FLOAT, OBJECT, NESTED, DATE_PERIOD, TIME_DURATION, DOC_DATA_TYPE, TSID_DATA_TYPE, - NULL -> throw EsqlIllegalArgumentException.illegalDataType(dataType); + NULL, PARTIAL_AGG -> throw EsqlIllegalArgumentException.illegalDataType(dataType); }; }