From 73f0307f111de7155d53fe92b8048c400ccfa560 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 26 Nov 2024 17:21:42 +0100 Subject: [PATCH 01/53] Make reserved built-in roles queryable --- .../core/security/authz/RoleDescriptor.java | 5 +- .../xpack/security/QueryRoleIT.java | 15 +- .../security/SecurityInBasicRestTestCase.java | 1 + .../xpack/security/Security.java | 24 +- .../xpack/security/SecurityFeatures.java | 8 +- .../authz/store/NativeRolesStore.java | 58 ++- .../store/QueryableBuiltInRolesStore.java | 51 +++ .../support/QueryableBuiltInRoles.java | 52 +++ .../QueryableBuiltInRolesSynchronizer.java | 413 ++++++++++++++++++ .../support/QueryableBuiltInRolesUtils.java | 50 +++ .../QueryableReservedRolesProvider.java | 51 +++ 11 files changed, 706 insertions(+), 22 deletions(-) create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRoles.java create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java 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 9f5aaa8562a88..495a7a9fca5a0 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 @@ -50,6 +50,7 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.TreeMap; import static org.elasticsearch.common.xcontent.XContentHelper.createParserNotCompressed; import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS; @@ -189,9 +190,9 @@ public RoleDescriptor( this.indicesPrivileges = indicesPrivileges != null ? indicesPrivileges : IndicesPrivileges.NONE; this.applicationPrivileges = applicationPrivileges != null ? applicationPrivileges : ApplicationResourcePrivileges.NONE; this.runAs = runAs != null ? runAs : Strings.EMPTY_ARRAY; - this.metadata = metadata != null ? Collections.unmodifiableMap(metadata) : Collections.emptyMap(); + this.metadata = metadata != null ? Collections.unmodifiableMap(new TreeMap<>(metadata)) : Collections.emptyMap(); this.transientMetadata = transientMetadata != null - ? Collections.unmodifiableMap(transientMetadata) + ? Collections.unmodifiableMap(new TreeMap<>(transientMetadata)) : Collections.singletonMap("enabled", true); this.remoteIndicesPrivileges = remoteIndicesPrivileges != null ? remoteIndicesPrivileges : RemoteIndicesPrivileges.NONE; this.remoteClusterPermissions = remoteClusterPermissions != null && remoteClusterPermissions.hasAnyPrivileges() diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryRoleIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryRoleIT.java index 1588749b9a331..30423dd91eee8 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryRoleIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryRoleIT.java @@ -49,15 +49,16 @@ public final class QueryRoleIT extends SecurityInBasicRestTestCase { private static final String READ_SECURITY_USER_AUTH_HEADER = "Basic cmVhZF9zZWN1cml0eV91c2VyOnJlYWQtc2VjdXJpdHktcGFzc3dvcmQ="; - public void testSimpleQueryAllRoles() throws IOException { - assertQuery("", 0, roles -> assertThat(roles, emptyIterable())); - RoleDescriptor createdRole = createRandomRole(); - assertQuery("", 1, roles -> { - assertThat(roles, iterableWithSize(1)); - assertRoleMap(roles.get(0), createdRole); + public void testSimpleQueryAllRoles() throws Exception { + createRandomRole(); + + // 31 built-in reserved roles + 1 random role + assertQuery("", 1 + 31, roles -> { + // default size is 10 + assertThat(roles, iterableWithSize(10)); }); assertQuery(""" - {"query":{"match_all":{}},"from":1}""", 1, roles -> assertThat(roles, emptyIterable())); + {"query":{"match_all":{}},"from":32}""", 1 + 31, roles -> assertThat(roles, emptyIterable())); } public void testDisallowedFields() throws Exception { diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityInBasicRestTestCase.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityInBasicRestTestCase.java index 7cb8c09545bb1..21c5c4c62ac80 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityInBasicRestTestCase.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityInBasicRestTestCase.java @@ -54,6 +54,7 @@ public abstract class SecurityInBasicRestTestCase extends ESRestTestCase { .user(API_KEY_USER, API_KEY_USER_PASSWORD.toString(), "api_key_user_role", false) .user(API_KEY_ADMIN_USER, API_KEY_ADMIN_USER_PASSWORD.toString(), "api_key_admin_role", false) .user(READ_SECURITY_USER, READ_SECURITY_PASSWORD.toString(), "read_security_user_role", false) + .systemProperty("es.queryable_built_in_roles_enabled", "true") .build(); @Override 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 ef66392a87260..70582d56351a7 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 @@ -337,6 +337,7 @@ import org.elasticsearch.xpack.security.authz.store.FileRolesStore; import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore; import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; +import org.elasticsearch.xpack.security.authz.store.QueryableBuiltInRolesStore; import org.elasticsearch.xpack.security.authz.store.RoleProviders; import org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor; import org.elasticsearch.xpack.security.operator.DefaultOperatorOnlyRegistry; @@ -411,6 +412,9 @@ import org.elasticsearch.xpack.security.rest.action.user.RestSetEnabledAction; import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry; import org.elasticsearch.xpack.security.support.ExtensionComponents; +import org.elasticsearch.xpack.security.support.QueryableBuiltInRoles; +import org.elasticsearch.xpack.security.support.QueryableBuiltInRolesSynchronizer; +import org.elasticsearch.xpack.security.support.QueryableReservedRolesProvider; import org.elasticsearch.xpack.security.support.ReloadableSecurityComponent; import org.elasticsearch.xpack.security.support.SecurityIndexManager; import org.elasticsearch.xpack.security.support.SecurityMigrationExecutor; @@ -461,6 +465,7 @@ import static org.elasticsearch.xpack.core.security.SecurityField.FIELD_LEVEL_SECURITY_FEATURE; import static org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore.INCLUDED_RESERVED_ROLES_SETTING; import static org.elasticsearch.xpack.security.operator.OperatorPrivileges.OPERATOR_PRIVILEGES_ENABLED; +import static org.elasticsearch.xpack.security.support.QueryableBuiltInRolesSynchronizer.QUERYABLE_BUILT_IN_ROLES_ENABLED; import static org.elasticsearch.xpack.security.transport.SSLEngineUtils.extractClientCertificates; public class Security extends Plugin @@ -631,7 +636,7 @@ public class Security extends Plugin private final SetOnce reservedRoleNameCheckerFactory = new SetOnce<>(); private final SetOnce fileRoleValidator = new SetOnce<>(); private final SetOnce secondaryAuthActions = new SetOnce<>(); - + private final SetOnce queryableRolesProvider = new SetOnce<>(); private final SetOnce securityMigrationExecutor = new SetOnce<>(); // Node local retry count for migration jobs that's checked only on the master node to make sure @@ -1202,6 +1207,22 @@ Collection createComponents( reservedRoleMappingAction.set(new ReservedRoleMappingAction()); + if (QUERYABLE_BUILT_IN_ROLES_ENABLED) { + if (queryableRolesProvider.get() == null) { + queryableRolesProvider.set(new QueryableReservedRolesProvider()); + } + components.add( + new QueryableBuiltInRolesSynchronizer( + clusterService, + featureService, + queryableRolesProvider.get(), + new QueryableBuiltInRolesStore(nativeRolesStore), + systemIndices.getMainIndexManager(), + threadPool + ) + ); + } + cacheInvalidatorRegistry.validate(); final List reloadableComponents = new ArrayList<>(); @@ -2317,6 +2338,7 @@ public void loadExtensions(ExtensionLoader loader) { loadSingletonExtensionAndSetOnce(loader, grantApiKeyRequestTranslator, RestGrantApiKeyAction.RequestTranslator.class); loadSingletonExtensionAndSetOnce(loader, fileRoleValidator, FileRoleValidator.class); loadSingletonExtensionAndSetOnce(loader, secondaryAuthActions, SecondaryAuthActions.class); + loadSingletonExtensionAndSetOnce(loader, queryableRolesProvider, QueryableBuiltInRoles.Provider.class); } private void loadSingletonExtensionAndSetOnce(ExtensionLoader loader, SetOnce setOnce, Class clazz) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/SecurityFeatures.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/SecurityFeatures.java index 53ecafa280715..84749d895a44e 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/SecurityFeatures.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/SecurityFeatures.java @@ -12,6 +12,7 @@ import java.util.Set; +import static org.elasticsearch.xpack.security.support.QueryableBuiltInRolesSynchronizer.QUERYABLE_BUILT_IN_ROLES_FEATURE; import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MIGRATION_FRAMEWORK; import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_ROLES_METADATA_FLATTENED; import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_ROLE_MAPPING_CLEANUP; @@ -20,6 +21,11 @@ public class SecurityFeatures implements FeatureSpecification { @Override public Set getFeatures() { - return Set.of(SECURITY_ROLE_MAPPING_CLEANUP, SECURITY_ROLES_METADATA_FLATTENED, SECURITY_MIGRATION_FRAMEWORK); + return Set.of( + SECURITY_ROLE_MAPPING_CLEANUP, + SECURITY_ROLES_METADATA_FLATTENED, + SECURITY_MIGRATION_FRAMEWORK, + QUERYABLE_BUILT_IN_ROLES_FEATURE + ); } } 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 4ae17a679d205..6168e76308d7a 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 @@ -63,13 +63,13 @@ import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivileges; 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.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -169,6 +169,10 @@ public NativeRolesStore( this.enabled = settings.getAsBoolean(NATIVE_ROLES_ENABLED, true); } + public boolean isEnabled() { + return enabled; + } + @Override public void accept(Set names, ActionListener listener) { getRoleDescriptors(names, listener); @@ -263,6 +267,10 @@ public boolean isMetadataSearchable() { } public void queryRoleDescriptors(SearchSourceBuilder searchSourceBuilder, ActionListener listener) { + if (enabled == false) { + listener.onFailure(new IllegalStateException("Native role management is disabled")); + return; + } SearchRequest searchRequest = new SearchRequest(new String[] { SECURITY_MAIN_ALIAS }, searchSourceBuilder); SecurityIndexManager frozenSecurityIndex = securityIndex.defensiveCopy(); if (frozenSecurityIndex.indexExists() == false) { @@ -345,6 +353,16 @@ public void deleteRoles( final List roleNames, WriteRequest.RefreshPolicy refreshPolicy, final ActionListener listener + ) { + deleteRoles(null, roleNames, refreshPolicy, true, listener); + } + + void deleteRoles( + SecurityIndexManager securityIndexManager, + final Collection roleNames, + WriteRequest.RefreshPolicy refreshPolicy, + boolean validateRoles, + final ActionListener listener ) { if (enabled == false) { listener.onFailure(new IllegalStateException("Native role management is disabled")); @@ -355,7 +373,7 @@ public void deleteRoles( Map validationErrorByRoleName = new HashMap<>(); for (String roleName : roleNames) { - if (reservedRoleNameChecker.isReserved(roleName)) { + if (validateRoles && reservedRoleNameChecker.isReserved(roleName)) { validationErrorByRoleName.put( roleName, new IllegalArgumentException("role [" + roleName + "] is reserved and cannot be deleted") @@ -370,7 +388,9 @@ public void deleteRoles( return; } - final SecurityIndexManager frozenSecurityIndex = securityIndex.defensiveCopy(); + final SecurityIndexManager frozenSecurityIndex = securityIndexManager != null + ? securityIndexManager + : securityIndex.defensiveCopy(); if (frozenSecurityIndex.indexExists() == false) { logger.debug("security index does not exist"); listener.onResponse(new BulkRolesResponse(List.of())); @@ -402,7 +422,7 @@ public void onFailure(Exception e) { } private void bulkResponseAndRefreshRolesCache( - List roleNames, + Collection roleNames, BulkResponse bulkResponse, Map validationErrorByRoleName, ActionListener listener @@ -430,7 +450,7 @@ private void bulkResponseAndRefreshRolesCache( } private void bulkResponseWithOnlyValidationErrors( - List roleNames, + Collection roleNames, Map validationErrorByRoleName, ActionListener listener ) { @@ -542,7 +562,17 @@ public void onFailure(Exception e) { public void putRoles( final WriteRequest.RefreshPolicy refreshPolicy, - final List roles, + final Collection roles, + final ActionListener listener + ) { + putRoles(securityIndex, refreshPolicy, roles, true, listener); + } + + void putRoles( + SecurityIndexManager securityIndexManager, + final WriteRequest.RefreshPolicy refreshPolicy, + final Collection roles, + boolean validateRoles, final ActionListener listener ) { if (enabled == false) { @@ -555,7 +585,7 @@ public void putRoles( for (RoleDescriptor role : roles) { Exception validationException; try { - validationException = validateRoleDescriptor(role); + validationException = validateRoles ? validateRoleDescriptor(role) : null; } catch (Exception e) { validationException = e; } @@ -578,7 +608,7 @@ public void putRoles( return; } - securityIndex.prepareIndexIfNeededThenExecute( + securityIndexManager.prepareIndexIfNeededThenExecute( listener::onFailure, () -> executeAsyncWithOrigin( client.threadPool().getThreadContext(), @@ -621,8 +651,6 @@ private DeleteRequest createRoleDeleteRequest(final String roleName) { // Package private for testing 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"; XContentBuilder builder = jsonBuilder().startObject(); @@ -671,7 +699,11 @@ public void usageStats(ActionListener> listener) { client.prepareMultiSearch() .add( client.prepareSearch(SECURITY_MAIN_ALIAS) - .setQuery(QueryBuilders.termQuery(RoleDescriptor.Fields.TYPE.getPreferredName(), ROLE_TYPE)) + .setQuery( + QueryBuilders.boolQuery() + .must(QueryBuilders.termQuery(RoleDescriptor.Fields.TYPE.getPreferredName(), ROLE_TYPE)) + .mustNot(QueryBuilders.termQuery("metadata_flattened._reserved", true)) + ) .setTrackTotalHits(true) .setSize(0) ) @@ -680,6 +712,7 @@ public void usageStats(ActionListener> listener) { .setQuery( QueryBuilders.boolQuery() .must(QueryBuilders.termQuery(RoleDescriptor.Fields.TYPE.getPreferredName(), ROLE_TYPE)) + .mustNot(QueryBuilders.termQuery("metadata_flattened._reserved", true)) .must( QueryBuilders.boolQuery() .should(existsQuery("indices.field_security.grant")) @@ -697,6 +730,7 @@ public void usageStats(ActionListener> listener) { .setQuery( QueryBuilders.boolQuery() .must(QueryBuilders.termQuery(RoleDescriptor.Fields.TYPE.getPreferredName(), ROLE_TYPE)) + .mustNot(QueryBuilders.termQuery("metadata_flattened._reserved", true)) .filter(existsQuery("indices.query")) ) .setTrackTotalHits(true) @@ -708,6 +742,7 @@ public void usageStats(ActionListener> listener) { .setQuery( QueryBuilders.boolQuery() .must(QueryBuilders.termQuery(RoleDescriptor.Fields.TYPE.getPreferredName(), ROLE_TYPE)) + .mustNot(QueryBuilders.termQuery("metadata_flattened._reserved", true)) .filter(existsQuery("remote_indices")) ) .setTrackTotalHits(true) @@ -718,6 +753,7 @@ public void usageStats(ActionListener> listener) { .setQuery( QueryBuilders.boolQuery() .must(QueryBuilders.termQuery(RoleDescriptor.Fields.TYPE.getPreferredName(), ROLE_TYPE)) + .mustNot(QueryBuilders.termQuery("metadata_flattened._reserved", true)) .filter(existsQuery("remote_cluster")) ) .setTrackTotalHits(true) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java new file mode 100644 index 0000000000000..db67692432a48 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java @@ -0,0 +1,51 @@ +/* + * 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.authz.store; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.xpack.core.security.action.role.BulkRolesResponse; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.support.MetadataUtils; +import org.elasticsearch.xpack.security.support.SecurityIndexManager; + +import java.util.Collection; + +/** + * Wrapper around the {@link NativeRolesStore} that provides a way to create, update and delete built-in roles. + */ +public final class QueryableBuiltInRolesStore { + + private final NativeRolesStore nativeRolesStore; + + public QueryableBuiltInRolesStore(NativeRolesStore nativeRolesStore) { + this.nativeRolesStore = nativeRolesStore; + } + + public void putRoles( + final SecurityIndexManager securityIndexManager, + final Collection roles, + final ActionListener listener + ) { + assert roles.stream().allMatch(role -> (Boolean) role.getMetadata().get(MetadataUtils.RESERVED_METADATA_KEY)); + nativeRolesStore.putRoles(securityIndexManager, WriteRequest.RefreshPolicy.IMMEDIATE, roles, false, listener); + } + + public void deleteRoles( + final SecurityIndexManager securityIndexManager, + final Collection roleNames, + final ActionListener listener + ) { + nativeRolesStore.deleteRoles(securityIndexManager, roleNames, WriteRequest.RefreshPolicy.IMMEDIATE, false, listener); + } + + public boolean isEnabled() { + return nativeRolesStore.isEnabled(); + } + +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRoles.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRoles.java new file mode 100644 index 0000000000000..ec38e4951f45c --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRoles.java @@ -0,0 +1,52 @@ +/* + * 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.support; + +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; + +import java.util.Collection; +import java.util.Map; + +/** + * A class that holds the built-in roles and their hash digests. + */ +public record QueryableBuiltInRoles(Map rolesDigest, Collection roleDescriptors) { + + /** + * A listener that is notified when the built-in roles change. + */ + public interface Listener { + + /** + * Called when the built-in roles change. + * + * @param roles the new built-in roles. + */ + void onRolesChanged(QueryableBuiltInRoles roles); + + } + + /** + * A provider that provides the built-in roles and can notify subscribed listeners when the built-in roles change. + */ + public interface Provider { + + /** + * @return the built-in roles. + */ + QueryableBuiltInRoles getRoles(); + + /** + * Adds a listener to be notified when the built-in roles change. + * + * @param listener the listener to add. + */ + void addListener(Listener listener); + + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java new file mode 100644 index 0000000000000..eb7387b1a0976 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -0,0 +1,413 @@ +/* + * 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.support; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.Version; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.UnavailableShardsException; +import org.elasticsearch.cluster.ClusterChangedEvent; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateListener; +import org.elasticsearch.cluster.ClusterStateTaskListener; +import org.elasticsearch.cluster.NotMasterException; +import org.elasticsearch.cluster.SimpleBatchedExecutor; +import org.elasticsearch.cluster.metadata.IndexAbstraction; +import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.node.DiscoveryNodes; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.cluster.service.MasterServiceTaskQueue; +import org.elasticsearch.common.Priority; +import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.component.LifecycleListener; +import org.elasticsearch.common.util.set.Sets; +import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.Tuple; +import org.elasticsearch.features.FeatureService; +import org.elasticsearch.features.NodeFeature; +import org.elasticsearch.index.Index; +import org.elasticsearch.index.IndexNotFoundException; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.security.action.role.BulkRolesResponse; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.security.authz.store.QueryableBuiltInRolesStore; + +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MAIN_ALIAS; + +public class QueryableBuiltInRolesSynchronizer implements ClusterStateListener { + + private static final Logger logger = LogManager.getLogger(QueryableBuiltInRolesSynchronizer.class); + + public static final boolean QUERYABLE_BUILT_IN_ROLES_ENABLED; + + static { + final var propertyValue = System.getProperty("es.queryable_built_in_roles_enabled"); + if (propertyValue == null || propertyValue.isEmpty() || "false".equals(propertyValue)) { + QUERYABLE_BUILT_IN_ROLES_ENABLED = false; + } else if ("true".equals(propertyValue)) { + QUERYABLE_BUILT_IN_ROLES_ENABLED = true; + } else { + throw new IllegalStateException( + "system property [es.queryable_built_in_roles_enabled] may only be set to [true] or [false], but was [" + + propertyValue + + "]" + ); + } + } + + public static final NodeFeature QUERYABLE_BUILT_IN_ROLES_FEATURE = new NodeFeature("security.queryable_built_in_roles"); + + public static final String METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST = "queryable_built_in_roles_digest"; + + private static final SimpleBatchedExecutor> MARK_ROLES_AS_SYNCED_TASK_EXECUTOR = + new SimpleBatchedExecutor<>() { + @Override + public Tuple> executeTask(MarkBuiltinRolesAsSyncedTask task, ClusterState clusterState) { + return task.execute(clusterState); + } + + @Override + public void taskSucceeded(MarkBuiltinRolesAsSyncedTask task, Map value) { + task.success(value); + } + }; + + private final MasterServiceTaskQueue markRolesAsSyncedTaskQueue; + + private final ClusterService clusterService; + private final FeatureService featureService; + private final QueryableBuiltInRoles.Provider builtinRolesProvider; + private final SecurityIndexManager securityIndex; + private final QueryableBuiltInRolesStore queryableBuiltInRolesStore; + private final Executor executor; + private final AtomicBoolean synchronizationInProgress = new AtomicBoolean(false); + + private volatile boolean securityIndexDeleted = false; + + public QueryableBuiltInRolesSynchronizer( + ClusterService clusterService, + FeatureService featureService, + QueryableBuiltInRoles.Provider rolesProvider, + QueryableBuiltInRolesStore queryableBuiltInRolesStore, + SecurityIndexManager securityIndex, + ThreadPool threadPool + ) { + this.clusterService = clusterService; + this.featureService = featureService; + this.builtinRolesProvider = rolesProvider; + this.queryableBuiltInRolesStore = queryableBuiltInRolesStore; + this.securityIndex = securityIndex; + this.executor = threadPool.generic(); + this.markRolesAsSyncedTaskQueue = clusterService.createTaskQueue( + "mark-built-in-roles-as-synced-task-queue", + Priority.LOW, + MARK_ROLES_AS_SYNCED_TASK_EXECUTOR + ); + this.builtinRolesProvider.addListener(this::builtInRolesChanged); + + this.clusterService.addLifecycleListener(new LifecycleListener() { + @Override + public void beforeStop() { + clusterService.removeListener(QueryableBuiltInRolesSynchronizer.this); + } + + @Override + public void beforeStart() { + clusterService.addListener(QueryableBuiltInRolesSynchronizer.this); + } + }); + } + + private void builtInRolesChanged(QueryableBuiltInRoles roles) { + logger.debug("Built-in roles changed, syncing to security index"); + final ClusterState state = clusterService.state(); + if (shouldSyncBuiltInRoles(state)) { + syncBuiltInRoles(roles, state); + } + } + + @Override + public void clusterChanged(ClusterChangedEvent event) { + final ClusterState state = event.state(); + if (isSecurityIndexDeleted(event)) { + this.securityIndexDeleted = true; + logger.trace("Security index has been deleted, skipping built-in roles synchronization"); + return; + } else if (isSecurityIndexCreated(event)) { + this.securityIndexDeleted = false; + logger.trace("Security index has been created, attempting to sync built-in roles"); + } + if (shouldSyncBuiltInRoles(state)) { + final QueryableBuiltInRoles roles = builtinRolesProvider.getRoles(); + syncBuiltInRoles(roles, state); + } + } + + private boolean isSecurityIndexDeleted(ClusterChangedEvent event) { + final IndexMetadata previousSecurityIndexMetadata = resolveSecurityIndexMetadata(event.previousState().metadata()); + final IndexMetadata currentSecurityIndexMetadata = resolveSecurityIndexMetadata(event.state().metadata()); + return previousSecurityIndexMetadata != null && currentSecurityIndexMetadata == null; + } + + private boolean isSecurityIndexCreated(ClusterChangedEvent event) { + final IndexMetadata previousSecurityIndexMetadata = resolveSecurityIndexMetadata(event.previousState().metadata()); + final IndexMetadata currentSecurityIndexMetadata = resolveSecurityIndexMetadata(event.state().metadata()); + return previousSecurityIndexMetadata == null && currentSecurityIndexMetadata != null; + } + + private void syncBuiltInRoles(QueryableBuiltInRoles roles, ClusterState state) { + final Map indexedRolesDigests = readIndexedBuiltInRolesDigests(state); + if (roles.rolesDigest().equals(indexedRolesDigests)) { + logger.debug("Security index already contains the latest built-in roles indexed, skipping synchronization"); + return; + } + if (synchronizationInProgress.compareAndSet(false, true)) { + executor.execute(() -> doSyncBuiltinRoles(indexedRolesDigests, roles, ActionListener.wrap(v -> { + logger.info("Successfully synced built-in roles to security index"); + synchronizationInProgress.set(false); + }, e -> { + if (false == e instanceof UnavailableShardsException + && false == e instanceof IndexNotFoundException + && false == e instanceof NotMasterException) { + logger.warn("Failed to sync built-in roles to security index", e); + } else { + logger.trace("Failed to sync built-in roles to security index", e); + } + synchronizationInProgress.set(false); + }))); + } + } + + private boolean shouldSyncBuiltInRoles(ClusterState state) { + if (securityIndexDeleted) { + logger.debug("Security index has been deleted, skipping built-in roles synchronization"); + return false; + } + if (queryableBuiltInRolesStore.isEnabled() == false) { + logger.trace("Role store is not enabled, skipping built-in roles synchronization"); + return false; + } + if (false == state.clusterRecovered()) { + logger.debug("Cluster state has not recovered yet, skipping built-in roles synchronization"); + return false; + } + if (false == state.nodes().isLocalNodeElectedMaster()) { + logger.trace("Local node is not the master, skipping built-in roles synchronization"); + return false; + } + if (state.nodes().getDataNodes().isEmpty()) { + logger.debug("No data nodes in the cluster, skipping built-in roles synchronization"); + return false; + } + // to keep things simple and avoid potential overwrites with an older version of built-in roles, + // we only sync built-in roles if all nodes are on the same version + if (isMixedVersionCluster(state.nodes())) { + logger.debug("Not all nodes are on the same version, skipping built-in roles synchronization"); + return false; + } + if (false == featureService.clusterHasFeature(state, QUERYABLE_BUILT_IN_ROLES_FEATURE)) { + logger.debug("Not all nodes support queryable built-in roles feature, skipping built-in roles synchronization"); + return false; + } + return true; + } + + private static boolean isMixedVersionCluster(DiscoveryNodes nodes) { + Version version = null; + for (DiscoveryNode node : nodes) { + if (version == null) { + version = node.getVersion(); + } else if (version.equals(node.getVersion()) == false) { + return true; + } + } + return false; + } + + private void doSyncBuiltinRoles(Map indexedRolesDigests, QueryableBuiltInRoles roles, ActionListener listener) { + // This will create .security index if it does not exist + securityIndex.prepareIndexIfNeededThenExecute(listener::onFailure, () -> { + final SecurityIndexManager frozenSecurityIndex = securityIndex.defensiveCopy(); + if (frozenSecurityIndex.isAvailable(SecurityIndexManager.Availability.PRIMARY_SHARDS) == false) { + listener.onFailure(frozenSecurityIndex.getUnavailableReason(SecurityIndexManager.Availability.PRIMARY_SHARDS)); + } else if (frozenSecurityIndex.indexIsClosed()) { + listener.onFailure(new ElasticsearchException(frozenSecurityIndex.getConcreteIndexName() + " is closed")); + } else { + final Collection rolesToUpsert = rolesToUpsert(roles, indexedRolesDigests); + indexRoles(rolesToUpsert, frozenSecurityIndex, ActionListener.wrap(onResponse -> { + final Set rolesToDelete = rolesToDelete(roles, indexedRolesDigests); + if (rolesToDelete.isEmpty()) { + markRolesAsSynced(frozenSecurityIndex.getConcreteIndexName(), indexedRolesDigests, roles.rolesDigest(), listener); + } else { + deleteRoles(frozenSecurityIndex, roles, rolesToDelete, indexedRolesDigests, listener); + } + }, listener::onFailure)); + } + }); + } + + private static Set rolesToDelete(QueryableBuiltInRoles roles, Map indexedRolesDigests) { + return indexedRolesDigests == null ? Set.of() : Sets.difference(indexedRolesDigests.keySet(), roles.rolesDigest().keySet()); + } + + private static Collection rolesToUpsert(QueryableBuiltInRoles roles, Map indexedRolesDigests) { + final Set rolesToUpsert = new HashSet<>(); + for (var role : roles.roleDescriptors()) { + final String roleDigest = roles.rolesDigest().get(role.getName()); + if (indexedRolesDigests == null || indexedRolesDigests.containsKey(role.getName()) == false) { + rolesToUpsert.add(role); + } else if (indexedRolesDigests.get(role.getName()).equals(roleDigest) == false) { + rolesToUpsert.add(role); + } + } + return rolesToUpsert; + } + + private void deleteRoles( + SecurityIndexManager securityIndex, + QueryableBuiltInRoles roles, + Set rolesToDelete, + Map indexedRolesDigests, + ActionListener listener + ) { + queryableBuiltInRolesStore.deleteRoles(securityIndex, rolesToDelete, ActionListener.wrap(deleteResponse -> { + if (deleteResponse.getItems().stream().anyMatch(BulkRolesResponse.Item::isFailed)) { + listener.onFailure(new IllegalStateException("Automatic deletion of built-in roles failed")); + } else { + markRolesAsSynced(securityIndex.getConcreteIndexName(), indexedRolesDigests, roles.rolesDigest(), listener); + } + }, listener::onFailure)); + } + + private void indexRoles(Collection rolesToUpsert, SecurityIndexManager securityIndex, ActionListener listener) { + queryableBuiltInRolesStore.putRoles(securityIndex, rolesToUpsert, ActionListener.wrap(response -> { + if (response.getItems().stream().anyMatch(BulkRolesResponse.Item::isFailed)) { + listener.onFailure(new IllegalStateException("Automatic indexing of built-in roles failed")); + } else { + listener.onResponse(null); + } + }, listener::onFailure)); + } + + private void markRolesAsSynced( + String concreteSecurityIndexName, + Map expectedRolesDigests, + Map newRolesDigests, + ActionListener listener + ) { + markRolesAsSyncedTaskQueue.submitTask( + "mark built-in roles as synced task", + new MarkBuiltinRolesAsSyncedTask(ActionListener.wrap(response -> { + if (newRolesDigests.equals(response) == false) { + // TODO: This should be expected and can happen if other node have already marked the roles as synced + listener.onFailure(new IllegalStateException("Failed to mark built-in roles as synced.")); + } else { + listener.onResponse(null); + } + }, listener::onFailure), concreteSecurityIndexName, expectedRolesDigests, newRolesDigests), + null + ); + } + + private Map readIndexedBuiltInRolesDigests(ClusterState state) { + final IndexMetadata indexMetadata = resolveSecurityIndexMetadata(state.metadata()); + if (indexMetadata == null) { + return null; + } + return indexMetadata.getCustomData(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST); + } + + private static IndexMetadata resolveSecurityIndexMetadata(final Metadata metadata) { + final Index index = resolveConcreteSecurityIndex(metadata); + if (index != null) { + return metadata.getIndexSafe(index); + } + return null; + } + + private static Index resolveConcreteSecurityIndex(final Metadata metadata) { + final IndexAbstraction indexAbstraction = metadata.getIndicesLookup().get(SECURITY_MAIN_ALIAS); + if (indexAbstraction != null) { + final List indices = indexAbstraction.getIndices(); + if (indexAbstraction.getType() != IndexAbstraction.Type.CONCRETE_INDEX && indices.size() > 1) { + throw new IllegalStateException("Alias [" + SECURITY_MAIN_ALIAS + "] points to more than one index: " + indices); + } + return indices.get(0); + } + return null; + } + + static class MarkBuiltinRolesAsSyncedTask implements ClusterStateTaskListener { + + private final ActionListener> listener; + private final String index; + private final Map expected; + private final Map value; + + MarkBuiltinRolesAsSyncedTask( + ActionListener> listener, + String index, + @Nullable Map expected, + @Nullable Map value + ) { + this.listener = listener; + this.index = index; + this.expected = expected; + this.value = value; + } + + Tuple> execute(ClusterState state) { + IndexMetadata indexMetadata = state.metadata().index(index); + if (indexMetadata == null) { + throw new IndexNotFoundException(index); + } + Map existingValue = indexMetadata.getCustomData(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST); + if (Objects.equals(expected, existingValue)) { + IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(indexMetadata); + if (value != null) { + indexMetadataBuilder.putCustom(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST, value); + } else { + indexMetadataBuilder.removeCustom(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST); + } + indexMetadataBuilder.version(indexMetadataBuilder.version() + 1); + ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(state.metadata().indices()); + builder.put(index, indexMetadataBuilder.build()); + return new Tuple<>( + ClusterState.builder(state).metadata(Metadata.builder(state.metadata()).indices(builder.build()).build()).build(), + value + ); + } else { + // returns existing value when expectation is not met + return new Tuple<>(state, existingValue); + } + } + + void success(Map value) { + listener.onResponse(value); + } + + @Override + public void onFailure(Exception e) { + listener.onFailure(e); + } + } + +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java new file mode 100644 index 0000000000000..1f5eea0b1da73 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java @@ -0,0 +1,50 @@ +/* + * 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.support; + +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.hash.MessageDigests; +import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; + +import java.io.IOException; +import java.security.MessageDigest; +import java.util.Base64; + +/** + * Utility class which provides helper method for calculating the hash of a role descriptor. + */ +public final class QueryableBuiltInRolesUtils { + + /** + * Calculates the hash of the given role descriptor by serializing it by calling {@link RoleDescriptor#writeTo(StreamOutput)} method + * and then SHA256 hashing the bytes. + * + * @param roleDescriptor the role descriptor to hash + * @return the base64 encoded SHA256 hash of the role descriptor + */ + public static String calculateHash(final RoleDescriptor roleDescriptor) { + final MessageDigest hash = MessageDigests.sha256(); + try (BytesStreamOutput out = new BytesStreamOutput();) { + try { + roleDescriptor.writeTo(out); + } catch (IOException e) { + throw new IllegalStateException("failed to serialize role descriptor", e); + } + hash.update(BytesReference.toBytes(out.bytes())); + } + // HEX vs Base64 encoding is a trade-off between readability and space efficiency + // opting for Base64 here to reduce the size of the cluster state + return Base64.getEncoder().encodeToString(hash.digest()); + } + + private QueryableBuiltInRolesUtils() { + throw new IllegalAccessError("not allowed"); + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java new file mode 100644 index 0000000000000..6b23a5787ee93 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java @@ -0,0 +1,51 @@ +/* + * 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.support; + +import org.elasticsearch.common.util.CachedSupplier; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; + +import java.util.Collection; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +/** + * A provider of the built-in reserved roles. + *

+ * This provider fetches all reserved roles from the {@link ReservedRolesStore} and calculates their hashes lazily. + * The reserved roles are static and do not change during runtime, hence this provider will never notify any listeners. + *

+ */ +public final class QueryableReservedRolesProvider implements QueryableBuiltInRoles.Provider { + + private final Supplier reservedRolesSupplier; + + /** + * Constructs a new reserved roles provider. + */ + public QueryableReservedRolesProvider() { + this.reservedRolesSupplier = CachedSupplier.wrap(() -> { + final Collection roleDescriptors = ReservedRolesStore.roleDescriptors(); + return new QueryableBuiltInRoles( + roleDescriptors.stream().collect(Collectors.toMap(RoleDescriptor::getName, QueryableBuiltInRolesUtils::calculateHash)), + roleDescriptors + ); + }); + } + + @Override + public QueryableBuiltInRoles getRoles() { + return reservedRolesSupplier.get(); + } + + @Override + public void addListener(QueryableBuiltInRoles.Listener listener) { + // no-op: reserved roles are static and do not change + } +} From 3c781265f3e1f1455bc3864c971bdf648fd4ff26 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 27 Nov 2024 13:16:58 +0100 Subject: [PATCH 02/53] export queryable classes --- x-pack/plugin/security/src/main/java/module-info.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/security/src/main/java/module-info.java b/x-pack/plugin/security/src/main/java/module-info.java index a072b34da7e96..67200726b3bed 100644 --- a/x-pack/plugin/security/src/main/java/module-info.java +++ b/x-pack/plugin/security/src/main/java/module-info.java @@ -70,6 +70,7 @@ exports org.elasticsearch.xpack.security.slowlog to org.elasticsearch.server; exports org.elasticsearch.xpack.security.authc.support to org.elasticsearch.internal.security; exports org.elasticsearch.xpack.security.rest.action.apikey to org.elasticsearch.internal.security; + exports org.elasticsearch.xpack.security.support to org.elasticsearch.internal.security; provides org.elasticsearch.index.SlowLogFieldProvider with org.elasticsearch.xpack.security.slowlog.SecuritySlowLogFieldProvider; From 252e140c8106c3c9b0f3e8e0b092a5226823d84c Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 28 Nov 2024 12:10:55 +0100 Subject: [PATCH 03/53] suppress 'this-escape' warning --- .../xpack/security/support/FeatureNotEnabledException.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/FeatureNotEnabledException.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/FeatureNotEnabledException.java index 87c23284c5819..8ba3ebad8a851 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/FeatureNotEnabledException.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/FeatureNotEnabledException.java @@ -29,6 +29,7 @@ public enum Feature { } } + @SuppressWarnings("this-escape") public FeatureNotEnabledException(Feature feature, String message, Object... args) { super(message, args); addMetadata(DISABLED_FEATURE_METADATA, feature.featureName); From c3b41abb746bb54fc89fdddd621178473ddc3168 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 28 Nov 2024 13:06:09 +0100 Subject: [PATCH 04/53] introduce a factory interface in order to be able to inject different providers --- .../xpack/security/Security.java | 15 ++++++------ .../QueryableBuiltInRolesProviderFactory.java | 23 +++++++++++++++++++ .../QueryableBuiltInRolesSynchronizer.java | 8 +++++-- .../QueryableReservedRolesProvider.java | 5 +++- 4 files changed, 41 insertions(+), 10 deletions(-) create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesProviderFactory.java 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 70582d56351a7..e025dcda6afdd 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 @@ -412,9 +412,8 @@ import org.elasticsearch.xpack.security.rest.action.user.RestSetEnabledAction; import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry; import org.elasticsearch.xpack.security.support.ExtensionComponents; -import org.elasticsearch.xpack.security.support.QueryableBuiltInRoles; +import org.elasticsearch.xpack.security.support.QueryableBuiltInRolesProviderFactory; import org.elasticsearch.xpack.security.support.QueryableBuiltInRolesSynchronizer; -import org.elasticsearch.xpack.security.support.QueryableReservedRolesProvider; import org.elasticsearch.xpack.security.support.ReloadableSecurityComponent; import org.elasticsearch.xpack.security.support.SecurityIndexManager; import org.elasticsearch.xpack.security.support.SecurityMigrationExecutor; @@ -636,7 +635,7 @@ public class Security extends Plugin private final SetOnce reservedRoleNameCheckerFactory = new SetOnce<>(); private final SetOnce fileRoleValidator = new SetOnce<>(); private final SetOnce secondaryAuthActions = new SetOnce<>(); - private final SetOnce queryableRolesProvider = new SetOnce<>(); + private final SetOnce queryableRolesProviderFactory = new SetOnce<>(); private final SetOnce securityMigrationExecutor = new SetOnce<>(); // Node local retry count for migration jobs that's checked only on the master node to make sure @@ -1208,15 +1207,17 @@ Collection createComponents( reservedRoleMappingAction.set(new ReservedRoleMappingAction()); if (QUERYABLE_BUILT_IN_ROLES_ENABLED) { - if (queryableRolesProvider.get() == null) { - queryableRolesProvider.set(new QueryableReservedRolesProvider()); + if (queryableRolesProviderFactory.get() == null) { + queryableRolesProviderFactory.set(new QueryableBuiltInRolesProviderFactory.Default()); } components.add( new QueryableBuiltInRolesSynchronizer( clusterService, featureService, - queryableRolesProvider.get(), + queryableRolesProviderFactory.get(), new QueryableBuiltInRolesStore(nativeRolesStore), + reservedRolesStore, + fileRolesStore.get(), systemIndices.getMainIndexManager(), threadPool ) @@ -2338,7 +2339,7 @@ public void loadExtensions(ExtensionLoader loader) { loadSingletonExtensionAndSetOnce(loader, grantApiKeyRequestTranslator, RestGrantApiKeyAction.RequestTranslator.class); loadSingletonExtensionAndSetOnce(loader, fileRoleValidator, FileRoleValidator.class); loadSingletonExtensionAndSetOnce(loader, secondaryAuthActions, SecondaryAuthActions.class); - loadSingletonExtensionAndSetOnce(loader, queryableRolesProvider, QueryableBuiltInRoles.Provider.class); + loadSingletonExtensionAndSetOnce(loader, queryableRolesProviderFactory, QueryableBuiltInRolesProviderFactory.class); } private void loadSingletonExtensionAndSetOnce(ExtensionLoader loader, SetOnce setOnce, Class clazz) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesProviderFactory.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesProviderFactory.java new file mode 100644 index 0000000000000..c29b64836d1a5 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesProviderFactory.java @@ -0,0 +1,23 @@ +/* + * 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.support; + +import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; +import org.elasticsearch.xpack.security.authz.store.FileRolesStore; + +public interface QueryableBuiltInRolesProviderFactory { + + QueryableBuiltInRoles.Provider createProvider(ReservedRolesStore reservedRolesStore, FileRolesStore fileRolesStore); + + class Default implements QueryableBuiltInRolesProviderFactory { + @Override + public QueryableBuiltInRoles.Provider createProvider(ReservedRolesStore reservedRolesStore, FileRolesStore fileRolesStore) { + return new QueryableReservedRolesProvider(reservedRolesStore); + } + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index eb7387b1a0976..e95e8b443eb1a 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -39,6 +39,8 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.security.action.role.BulkRolesResponse; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; +import org.elasticsearch.xpack.security.authz.store.FileRolesStore; import org.elasticsearch.xpack.security.authz.store.QueryableBuiltInRolesStore; import java.util.Collection; @@ -105,14 +107,16 @@ public void taskSucceeded(MarkBuiltinRolesAsSyncedTask task, Map public QueryableBuiltInRolesSynchronizer( ClusterService clusterService, FeatureService featureService, - QueryableBuiltInRoles.Provider rolesProvider, + QueryableBuiltInRolesProviderFactory rolesProviderFactory, QueryableBuiltInRolesStore queryableBuiltInRolesStore, + ReservedRolesStore reservedRolesStore, + FileRolesStore fileRolesStore, SecurityIndexManager securityIndex, ThreadPool threadPool ) { this.clusterService = clusterService; this.featureService = featureService; - this.builtinRolesProvider = rolesProvider; + this.builtinRolesProvider = rolesProviderFactory.createProvider(reservedRolesStore, fileRolesStore); this.queryableBuiltInRolesStore = queryableBuiltInRolesStore; this.securityIndex = securityIndex; this.executor = threadPool.generic(); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java index 6b23a5787ee93..d2d6f347ffd0d 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java @@ -28,8 +28,11 @@ public final class QueryableReservedRolesProvider implements QueryableBuiltInRol /** * Constructs a new reserved roles provider. + * + * @param reservedRolesStore the store to fetch the reserved roles from. + * Having a store reference here is necessary to ensure that static fields are initialized. */ - public QueryableReservedRolesProvider() { + public QueryableReservedRolesProvider(ReservedRolesStore reservedRolesStore) { this.reservedRolesSupplier = CachedSupplier.wrap(() -> { final Collection roleDescriptors = ReservedRolesStore.roleDescriptors(); return new QueryableBuiltInRoles( From b83410c68ff23433174c756ed6a680d176b3fd19 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 28 Nov 2024 13:14:19 +0100 Subject: [PATCH 05/53] export org.elasticsearch.xpack.security.authz.store --- x-pack/plugin/security/src/main/java/module-info.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/security/src/main/java/module-info.java b/x-pack/plugin/security/src/main/java/module-info.java index 67200726b3bed..947211559b0c2 100644 --- a/x-pack/plugin/security/src/main/java/module-info.java +++ b/x-pack/plugin/security/src/main/java/module-info.java @@ -71,6 +71,7 @@ exports org.elasticsearch.xpack.security.authc.support to org.elasticsearch.internal.security; exports org.elasticsearch.xpack.security.rest.action.apikey to org.elasticsearch.internal.security; exports org.elasticsearch.xpack.security.support to org.elasticsearch.internal.security; + exports org.elasticsearch.xpack.security.authz.store to org.elasticsearch.internal.security; provides org.elasticsearch.index.SlowLogFieldProvider with org.elasticsearch.xpack.security.slowlog.SecuritySlowLogFieldProvider; From cb70eff3cda4cecc318e9d37378883acab16772f Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 28 Nov 2024 14:03:45 +0100 Subject: [PATCH 06/53] allow returning all file role definitions --- .../xpack/security/authz/store/FileRolesStore.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java index 7618135c8662f..87378ac0b9f25 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java @@ -44,6 +44,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -173,6 +174,14 @@ public Path getFile() { return file; } + /** + * @return a map of all file role definitions. The returned map is unmodifiable. + */ + public Map getAllRoleDescriptors() { + final Map localPermissions = permissions; + return Collections.unmodifiableMap(localPermissions); + } + // package private for testing Set getAllRoleNames() { return permissions.keySet(); From 31b4fb29ccfdb7919e6fb44b984f8b6b7634cdcf Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 29 Nov 2024 11:56:13 +0100 Subject: [PATCH 07/53] mark query roles API as public in serverless --- .../xpack/security/rest/action/role/RestQueryRoleAction.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestQueryRoleAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestQueryRoleAction.java index c2dc7166bd3b6..37d0455c9def2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestQueryRoleAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestQueryRoleAction.java @@ -32,6 +32,7 @@ import static org.elasticsearch.rest.RestRequest.Method.POST; import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg; +@ServerlessScope(Scope.PUBLIC) public final class RestQueryRoleAction extends NativeRoleBaseRestHandler { @SuppressWarnings("unchecked") From 4d6bd5704fe55018fb916ec45e871af39cecb200 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 29 Nov 2024 11:57:40 +0100 Subject: [PATCH 08/53] imports are important --- .../xpack/security/rest/action/role/RestQueryRoleAction.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestQueryRoleAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestQueryRoleAction.java index 37d0455c9def2..862ff2552b4e3 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestQueryRoleAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestQueryRoleAction.java @@ -14,6 +14,8 @@ import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.Scope; +import org.elasticsearch.rest.ServerlessScope; import org.elasticsearch.rest.action.RestToXContentListener; import org.elasticsearch.search.searchafter.SearchAfterBuilder; import org.elasticsearch.search.sort.FieldSortBuilder; From 8ca36ad50e67ae3b5ba2409f8b9c1f851ffa0e67 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 29 Nov 2024 12:48:39 +0100 Subject: [PATCH 09/53] remove assertion as it's not true in all use cases --- .../xpack/security/authz/store/QueryableBuiltInRolesStore.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java index db67692432a48..364f315510a67 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java @@ -32,7 +32,6 @@ public void putRoles( final Collection roles, final ActionListener listener ) { - assert roles.stream().allMatch(role -> (Boolean) role.getMetadata().get(MetadataUtils.RESERVED_METADATA_KEY)); nativeRolesStore.putRoles(securityIndexManager, WriteRequest.RefreshPolicy.IMMEDIATE, roles, false, listener); } From 7b278e61db019f1ae17d29483e739216409101ce Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 29 Nov 2024 13:00:07 +0100 Subject: [PATCH 10/53] remove unused import --- .../xpack/security/authz/store/QueryableBuiltInRolesStore.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java index 364f315510a67..b0efa5751c1a0 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java @@ -11,7 +11,6 @@ import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.xpack.core.security.action.role.BulkRolesResponse; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; -import org.elasticsearch.xpack.core.security.support.MetadataUtils; import org.elasticsearch.xpack.security.support.SecurityIndexManager; import java.util.Collection; From 8c71c73eddc28063a420c583909bd4d2144c4c8b Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 2 Dec 2024 16:58:01 +0100 Subject: [PATCH 11/53] test reserved roles are all indexed and cannot be modified via API --- .../xpack/security/QueryRoleIT.java | 2 +- .../security/QueryableReservedRolesIT.java | 79 +++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryRoleIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryRoleIT.java index 30423dd91eee8..08d21db710f91 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryRoleIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryRoleIT.java @@ -497,7 +497,7 @@ private RoleDescriptor createRole( ); } - private void assertQuery(String body, int total, Consumer>> roleVerifier) throws IOException { + static void assertQuery(String body, int total, Consumer>> roleVerifier) throws IOException { assertQuery(client(), body, total, roleVerifier); } diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java new file mode 100644 index 0000000000000..13b2a805ff070 --- /dev/null +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java @@ -0,0 +1,79 @@ +/* + * 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; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; +import org.elasticsearch.xpack.security.support.SecurityMigrations; +import org.junit.BeforeClass; + +import static org.elasticsearch.xpack.security.QueryRoleIT.assertQuery; +import static org.elasticsearch.xpack.security.QueryRoleIT.waitForMigrationCompletion; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.iterableWithSize; +import static org.hamcrest.Matchers.oneOf; + +public class QueryableReservedRolesIT extends SecurityInBasicRestTestCase { + + @BeforeClass + public static void setup() { + new ReservedRolesStore(); + } + + public void testQueryDeleteOrUpdateReservedRoles() throws Exception { + waitForMigrationCompletion(adminClient(), SecurityMigrations.ROLE_METADATA_FLATTENED_MIGRATION_VERSION); + + final String[] allReservedRoles = ReservedRolesStore.names().toArray(new String[0]); + assertQuery(""" + { "query": { "bool": { "must": { "term": { "metadata._reserved": true } } } }, "size": 100 } + """, 31, roles -> { + assertThat(roles, iterableWithSize(31)); + for (var role : roles) { + assertThat((String) role.get("name"), is(oneOf(allReservedRoles))); + } + }); + + final String roleName = randomFrom(allReservedRoles); + assertQuery(String.format(""" + { "query": { "bool": { "must": { "term": { "name": "%s" } } } } } + """, roleName), 1, roles -> { + assertThat(roles, iterableWithSize(1)); + assertThat((String) roles.get(0).get("name"), equalTo(roleName)); + }); + + assertDeleteReservedRole(roleName); + assertCreateOrUpdateReservedRole(roleName); + } + + private void assertDeleteReservedRole(String roleName) throws Exception { + Request request = new Request("DELETE", "/_security/role/" + roleName); + var e = expectThrows(ResponseException.class, () -> adminClient().performRequest(request)); + assertThat(e.getMessage(), containsString("role [" + roleName + "] is reserved and cannot be deleted")); + } + + private void assertCreateOrUpdateReservedRole(String roleName) throws Exception { + Request request = new Request(randomBoolean() ? "PUT" : "POST", "/_security/role/" + roleName); + request.setJsonEntity(""" + { + "cluster": ["all"], + "indices": [ + { + "names": ["*"], + "privileges": ["all"] + } + ] + } + """); + var e = expectThrows(ResponseException.class, () -> adminClient().performRequest(request)); + assertThat(e.getMessage(), containsString("Role [" + roleName + "] is reserved and may not be used.")); + } + +} From 9cae158bcb82d1123369cf7ca4ede13dc4e865e1 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 2 Dec 2024 17:49:28 +0100 Subject: [PATCH 12/53] document feature flag --- .../support/QueryableBuiltInRolesSynchronizer.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index e95e8b443eb1a..068943db13142 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -58,8 +58,14 @@ public class QueryableBuiltInRolesSynchronizer implements ClusterStateListener { private static final Logger logger = LogManager.getLogger(QueryableBuiltInRolesSynchronizer.class); + /** + * This is a temporary feature flag to allow enabling the synchronization of built-in roles to the .security index. + * Initially, it is disabled by default due to the number of tests that need to be adjusted now that .security index + * is created earlier in the cluster lifecycle. + *

+ * Once all tests are adjusted, this flag will be set to enabled by default and later removed altogether. + */ public static final boolean QUERYABLE_BUILT_IN_ROLES_ENABLED; - static { final var propertyValue = System.getProperty("es.queryable_built_in_roles_enabled"); if (propertyValue == null || propertyValue.isEmpty() || "false".equals(propertyValue)) { From 57cac14bf0651500b7d4313d56c86cd035490d4a Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 3 Dec 2024 11:18:38 +0100 Subject: [PATCH 13/53] code cleanup --- .../QueryableBuiltInRolesSynchronizer.java | 253 +++++++++++------- 1 file changed, 159 insertions(+), 94 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 068943db13142..a80ea02047a59 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -10,9 +10,10 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.Version; +import org.elasticsearch.ExceptionsHelper; +import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.UnavailableShardsException; +import org.elasticsearch.action.support.TransportActions; import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateListener; @@ -22,8 +23,6 @@ import org.elasticsearch.cluster.metadata.IndexAbstraction; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.Metadata; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.cluster.service.MasterServiceTaskQueue; import org.elasticsearch.common.Priority; @@ -31,11 +30,16 @@ import org.elasticsearch.common.component.LifecycleListener; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.Strings; import org.elasticsearch.core.Tuple; import org.elasticsearch.features.FeatureService; import org.elasticsearch.features.NodeFeature; import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexNotFoundException; +import org.elasticsearch.index.engine.DocumentMissingException; +import org.elasticsearch.index.engine.VersionConflictEngineException; +import org.elasticsearch.indices.IndexClosedException; +import org.elasticsearch.indices.IndexPrimaryShardNotAllocatedException; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.security.action.role.BulkRolesResponse; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; @@ -48,12 +52,24 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MAIN_ALIAS; +/** + * Synchronizes built-in roles to the .security index. + * The .security index is created if it does not exist. + *

+ * The synchronization is executed only on the elected master node + * after the cluster has recovered and roles need to be synced. + * The goal is to reduce the potential for conflicting operations. + * While in most cases, there should be only a single node that’s + * attempting to create/update/delete roles, it’s still possible + * that the master node changes in the middle of the syncing process. + */ public class QueryableBuiltInRolesSynchronizer implements ClusterStateListener { private static final Logger logger = LogManager.getLogger(QueryableBuiltInRolesSynchronizer.class); @@ -83,26 +99,31 @@ public class QueryableBuiltInRolesSynchronizer implements ClusterStateListener { public static final NodeFeature QUERYABLE_BUILT_IN_ROLES_FEATURE = new NodeFeature("security.queryable_built_in_roles"); - public static final String METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST = "queryable_built_in_roles_digest"; + /** + * Index metadata key of the digest of built-in roles indexed in the .security index. + *

+ * The value is a map of built-in role names to their digests (calculated by sha256 of the role definition). + */ + public static final String METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST_KEY = "queryable_built_in_roles_digest"; - private static final SimpleBatchedExecutor> MARK_ROLES_AS_SYNCED_TASK_EXECUTOR = + private static final SimpleBatchedExecutor> MARK_ROLES_AS_SYNCED_TASK_EXECUTOR = new SimpleBatchedExecutor<>() { @Override - public Tuple> executeTask(MarkBuiltinRolesAsSyncedTask task, ClusterState clusterState) { + public Tuple> executeTask(MarkRolesAsSyncedTask task, ClusterState clusterState) { return task.execute(clusterState); } @Override - public void taskSucceeded(MarkBuiltinRolesAsSyncedTask task, Map value) { + public void taskSucceeded(MarkRolesAsSyncedTask task, Map value) { task.success(value); } }; - private final MasterServiceTaskQueue markRolesAsSyncedTaskQueue; + private final MasterServiceTaskQueue markRolesAsSyncedTaskQueue; private final ClusterService clusterService; private final FeatureService featureService; - private final QueryableBuiltInRoles.Provider builtinRolesProvider; + private final QueryableBuiltInRoles.Provider rolesProvider; private final SecurityIndexManager securityIndex; private final QueryableBuiltInRolesStore queryableBuiltInRolesStore; private final Executor executor; @@ -122,7 +143,7 @@ public QueryableBuiltInRolesSynchronizer( ) { this.clusterService = clusterService; this.featureService = featureService; - this.builtinRolesProvider = rolesProviderFactory.createProvider(reservedRolesStore, fileRolesStore); + this.rolesProvider = rolesProviderFactory.createProvider(reservedRolesStore, fileRolesStore); this.queryableBuiltInRolesStore = queryableBuiltInRolesStore; this.securityIndex = securityIndex; this.executor = threadPool.generic(); @@ -131,8 +152,7 @@ public QueryableBuiltInRolesSynchronizer( Priority.LOW, MARK_ROLES_AS_SYNCED_TASK_EXECUTOR ); - this.builtinRolesProvider.addListener(this::builtInRolesChanged); - + this.rolesProvider.addListener(this::builtInRolesChanged); this.clusterService.addLifecycleListener(new LifecycleListener() { @Override public void beforeStop() { @@ -147,7 +167,7 @@ public void beforeStart() { } private void builtInRolesChanged(QueryableBuiltInRoles roles) { - logger.debug("Built-in roles changed, syncing to security index"); + logger.debug("Built-in roles changed, attempting to sync to .security index"); final ClusterState state = clusterService.state(); if (shouldSyncBuiltInRoles(state)) { syncBuiltInRoles(roles, state); @@ -166,23 +186,11 @@ public void clusterChanged(ClusterChangedEvent event) { logger.trace("Security index has been created, attempting to sync built-in roles"); } if (shouldSyncBuiltInRoles(state)) { - final QueryableBuiltInRoles roles = builtinRolesProvider.getRoles(); + final QueryableBuiltInRoles roles = rolesProvider.getRoles(); syncBuiltInRoles(roles, state); } } - private boolean isSecurityIndexDeleted(ClusterChangedEvent event) { - final IndexMetadata previousSecurityIndexMetadata = resolveSecurityIndexMetadata(event.previousState().metadata()); - final IndexMetadata currentSecurityIndexMetadata = resolveSecurityIndexMetadata(event.state().metadata()); - return previousSecurityIndexMetadata != null && currentSecurityIndexMetadata == null; - } - - private boolean isSecurityIndexCreated(ClusterChangedEvent event) { - final IndexMetadata previousSecurityIndexMetadata = resolveSecurityIndexMetadata(event.previousState().metadata()); - final IndexMetadata currentSecurityIndexMetadata = resolveSecurityIndexMetadata(event.state().metadata()); - return previousSecurityIndexMetadata == null && currentSecurityIndexMetadata != null; - } - private void syncBuiltInRoles(QueryableBuiltInRoles roles, ClusterState state) { final Map indexedRolesDigests = readIndexedBuiltInRolesDigests(state); if (roles.rolesDigest().equals(indexedRolesDigests)) { @@ -191,75 +199,87 @@ private void syncBuiltInRoles(QueryableBuiltInRoles roles, ClusterState state) { } if (synchronizationInProgress.compareAndSet(false, true)) { executor.execute(() -> doSyncBuiltinRoles(indexedRolesDigests, roles, ActionListener.wrap(v -> { - logger.info("Successfully synced built-in roles to security index"); + logger.info("Successfully synced built-in roles to .security index"); synchronizationInProgress.set(false); }, e -> { - if (false == e instanceof UnavailableShardsException - && false == e instanceof IndexNotFoundException - && false == e instanceof NotMasterException) { - logger.warn("Failed to sync built-in roles to security index", e); + if (isExpectedFailure(e)) { + logger.trace("Failed to sync built-in roles to .security index", e); } else { - logger.trace("Failed to sync built-in roles to security index", e); + logger.warn("Failed to sync built-in roles to .security index due to unexpected exception", e); } synchronizationInProgress.set(false); }))); } } + /** + * Some failures are expected and should not be logged as errors. + * These exceptions are either: + * - transient (e.g. connection errors), + * - recoverable (e.g. no longer master, index reallocating or caused by concurrent operations) + * - not recoverable but expected (e.g. index closed). + * + * @param e to check + * @return {@code true} if the exception is expected and should not be logged as an error + */ + private boolean isExpectedFailure(final Exception e) { + final Throwable cause = ExceptionsHelper.unwrapCause(e); + return ExceptionsHelper.isNodeOrShardUnavailableTypeException(e) + || TransportActions.isShardNotAvailableException(e) + || e instanceof IndexClosedException + || e instanceof IndexPrimaryShardNotAllocatedException + || e instanceof NotMasterException + || cause instanceof ResourceAlreadyExistsException + || cause instanceof VersionConflictEngineException + || cause instanceof DocumentMissingException + || (cause instanceof ElasticsearchException + && "Failed to mark built-in roles as synced. The expected roles digests have changed.".equals(cause.getMessage())); + } + private boolean shouldSyncBuiltInRoles(ClusterState state) { - if (securityIndexDeleted) { - logger.debug("Security index has been deleted, skipping built-in roles synchronization"); + if (false == state.nodes().isLocalNodeElectedMaster()) { + logger.trace("Local node is not the master, skipping built-in roles synchronization"); + return false; + } + if (false == state.clusterRecovered()) { + logger.trace("Cluster state has not recovered yet, skipping built-in roles synchronization"); return false; } if (queryableBuiltInRolesStore.isEnabled() == false) { logger.trace("Role store is not enabled, skipping built-in roles synchronization"); return false; } - if (false == state.clusterRecovered()) { - logger.debug("Cluster state has not recovered yet, skipping built-in roles synchronization"); + if (state.nodes().getDataNodes().isEmpty()) { + logger.trace("No data nodes in the cluster, skipping built-in roles synchronization"); return false; } - if (false == state.nodes().isLocalNodeElectedMaster()) { - logger.trace("Local node is not the master, skipping built-in roles synchronization"); + if (state.nodes().isMixedVersionCluster()) { + // To keep things simple and avoid potential overwrites with an older version of built-in roles, + // we only sync built-in roles if all nodes are on the same version. + logger.trace("Not all nodes are on the same version, skipping built-in roles synchronization"); return false; } - if (state.nodes().getDataNodes().isEmpty()) { - logger.debug("No data nodes in the cluster, skipping built-in roles synchronization"); + if (false == featureService.clusterHasFeature(state, QUERYABLE_BUILT_IN_ROLES_FEATURE)) { + logger.trace("Not all nodes support queryable built-in roles feature, skipping built-in roles synchronization"); return false; } - // to keep things simple and avoid potential overwrites with an older version of built-in roles, - // we only sync built-in roles if all nodes are on the same version - if (isMixedVersionCluster(state.nodes())) { - logger.debug("Not all nodes are on the same version, skipping built-in roles synchronization"); + if (securityIndexDeleted) { + logger.trace("Security index has been deleted, skipping built-in roles synchronization"); return false; } - if (false == featureService.clusterHasFeature(state, QUERYABLE_BUILT_IN_ROLES_FEATURE)) { - logger.debug("Not all nodes support queryable built-in roles feature, skipping built-in roles synchronization"); + if (isSecurityIndexClosed(state)) { + logger.trace("Security index is closed, skipping built-in roles synchronization"); return false; } return true; } - private static boolean isMixedVersionCluster(DiscoveryNodes nodes) { - Version version = null; - for (DiscoveryNode node : nodes) { - if (version == null) { - version = node.getVersion(); - } else if (version.equals(node.getVersion()) == false) { - return true; - } - } - return false; - } - private void doSyncBuiltinRoles(Map indexedRolesDigests, QueryableBuiltInRoles roles, ActionListener listener) { - // This will create .security index if it does not exist securityIndex.prepareIndexIfNeededThenExecute(listener::onFailure, () -> { final SecurityIndexManager frozenSecurityIndex = securityIndex.defensiveCopy(); if (frozenSecurityIndex.isAvailable(SecurityIndexManager.Availability.PRIMARY_SHARDS) == false) { listener.onFailure(frozenSecurityIndex.getUnavailableReason(SecurityIndexManager.Availability.PRIMARY_SHARDS)); - } else if (frozenSecurityIndex.indexIsClosed()) { - listener.onFailure(new ElasticsearchException(frozenSecurityIndex.getConcreteIndexName() + " is closed")); + } else { final Collection rolesToUpsert = rolesToUpsert(roles, indexedRolesDigests); indexRoles(rolesToUpsert, frozenSecurityIndex, ActionListener.wrap(onResponse -> { @@ -283,9 +303,9 @@ private static Collection rolesToUpsert(QueryableBuiltInRoles ro for (var role : roles.roleDescriptors()) { final String roleDigest = roles.rolesDigest().get(role.getName()); if (indexedRolesDigests == null || indexedRolesDigests.containsKey(role.getName()) == false) { - rolesToUpsert.add(role); + rolesToUpsert.add(role); // a new role to create } else if (indexedRolesDigests.get(role.getName()).equals(roleDigest) == false) { - rolesToUpsert.add(role); + rolesToUpsert.add(role); // an existing role that needs to be updated } } return rolesToUpsert; @@ -299,8 +319,13 @@ private void deleteRoles( ActionListener listener ) { queryableBuiltInRolesStore.deleteRoles(securityIndex, rolesToDelete, ActionListener.wrap(deleteResponse -> { - if (deleteResponse.getItems().stream().anyMatch(BulkRolesResponse.Item::isFailed)) { - listener.onFailure(new IllegalStateException("Automatic deletion of built-in roles failed")); + final Optional deleteFailure = deleteResponse.getItems() + .stream() + .filter(BulkRolesResponse.Item::isFailed) + .map(BulkRolesResponse.Item::getCause) + .findAny(); + if (deleteFailure.isPresent()) { + listener.onFailure(deleteFailure.get()); } else { markRolesAsSynced(securityIndex.getConcreteIndexName(), indexedRolesDigests, roles.rolesDigest(), listener); } @@ -309,14 +334,45 @@ private void deleteRoles( private void indexRoles(Collection rolesToUpsert, SecurityIndexManager securityIndex, ActionListener listener) { queryableBuiltInRolesStore.putRoles(securityIndex, rolesToUpsert, ActionListener.wrap(response -> { - if (response.getItems().stream().anyMatch(BulkRolesResponse.Item::isFailed)) { - listener.onFailure(new IllegalStateException("Automatic indexing of built-in roles failed")); + final Optional indexFailure = response.getItems() + .stream() + .filter(BulkRolesResponse.Item::isFailed) + .map(BulkRolesResponse.Item::getCause) + .findAny(); + if (indexFailure.isPresent()) { + listener.onFailure(indexFailure.get()); } else { listener.onResponse(null); } }, listener::onFailure)); } + private boolean isSecurityIndexDeleted(ClusterChangedEvent event) { + final IndexMetadata previousSecurityIndexMetadata = resolveSecurityIndexMetadata(event.previousState().metadata()); + final IndexMetadata currentSecurityIndexMetadata = resolveSecurityIndexMetadata(event.state().metadata()); + return previousSecurityIndexMetadata != null && currentSecurityIndexMetadata == null; + } + + private boolean isSecurityIndexCreated(ClusterChangedEvent event) { + final IndexMetadata previousSecurityIndexMetadata = resolveSecurityIndexMetadata(event.previousState().metadata()); + final IndexMetadata currentSecurityIndexMetadata = resolveSecurityIndexMetadata(event.state().metadata()); + return previousSecurityIndexMetadata == null && currentSecurityIndexMetadata != null; + } + + private boolean isSecurityIndexClosed(ClusterState state) { + final IndexMetadata indexMetadata = resolveSecurityIndexMetadata(state.metadata()); + return indexMetadata != null && indexMetadata.getState() == IndexMetadata.State.CLOSE; + } + + /** + * This method marks the built-in roles as synced in the .security index + * by setting the new roles digests in the metadata of the .security index. + *

+ * The marking is done as a compare and swap operation to ensure that the roles + * are marked as synced only when new roles are indexed. The operation is idempotent + * and will succeed if the expected roles digests are equal to the digests in the + * .security index or if they are equal to the new roles digests. + */ private void markRolesAsSynced( String concreteSecurityIndexName, Map expectedRolesDigests, @@ -325,10 +381,19 @@ private void markRolesAsSynced( ) { markRolesAsSyncedTaskQueue.submitTask( "mark built-in roles as synced task", - new MarkBuiltinRolesAsSyncedTask(ActionListener.wrap(response -> { + new MarkRolesAsSyncedTask(ActionListener.wrap(response -> { if (newRolesDigests.equals(response) == false) { - // TODO: This should be expected and can happen if other node have already marked the roles as synced - listener.onFailure(new IllegalStateException("Failed to mark built-in roles as synced.")); + logger.trace( + () -> Strings.format( + "Another master node most probably indexed a newer versions of built-in roles in the meantime. " + + "Expected: [%s], Actual: [%s]", + newRolesDigests, + response + ) + ); + listener.onFailure( + new ElasticsearchException("Failed to mark built-in roles as synced. The expected roles digests have changed.") + ); } else { listener.onResponse(null); } @@ -342,7 +407,7 @@ private Map readIndexedBuiltInRolesDigests(ClusterState state) { if (indexMetadata == null) { return null; } - return indexMetadata.getCustomData(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST); + return indexMetadata.getCustomData(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST_KEY); } private static IndexMetadata resolveSecurityIndexMetadata(final Metadata metadata) { @@ -358,55 +423,55 @@ private static Index resolveConcreteSecurityIndex(final Metadata metadata) { if (indexAbstraction != null) { final List indices = indexAbstraction.getIndices(); if (indexAbstraction.getType() != IndexAbstraction.Type.CONCRETE_INDEX && indices.size() > 1) { - throw new IllegalStateException("Alias [" + SECURITY_MAIN_ALIAS + "] points to more than one index: " + indices); + throw new IllegalStateException("alias [" + SECURITY_MAIN_ALIAS + "] points to more than one index [" + indices + "]"); } return indices.get(0); } return null; } - static class MarkBuiltinRolesAsSyncedTask implements ClusterStateTaskListener { + static class MarkRolesAsSyncedTask implements ClusterStateTaskListener { private final ActionListener> listener; - private final String index; - private final Map expected; - private final Map value; + private final String concreteSecurityIndexName; + private final Map expectedRoleDigests; + private final Map newRoleDigests; - MarkBuiltinRolesAsSyncedTask( + MarkRolesAsSyncedTask( ActionListener> listener, - String index, - @Nullable Map expected, - @Nullable Map value + String concreteSecurityIndexName, + @Nullable Map expectedRoleDigests, + @Nullable Map newRoleDigests ) { this.listener = listener; - this.index = index; - this.expected = expected; - this.value = value; + this.concreteSecurityIndexName = concreteSecurityIndexName; + this.expectedRoleDigests = expectedRoleDigests; + this.newRoleDigests = newRoleDigests; } Tuple> execute(ClusterState state) { - IndexMetadata indexMetadata = state.metadata().index(index); + IndexMetadata indexMetadata = state.metadata().index(concreteSecurityIndexName); if (indexMetadata == null) { - throw new IndexNotFoundException(index); + throw new IndexNotFoundException(concreteSecurityIndexName); } - Map existingValue = indexMetadata.getCustomData(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST); - if (Objects.equals(expected, existingValue)) { + Map existingRoleDigests = indexMetadata.getCustomData(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST_KEY); + if (Objects.equals(expectedRoleDigests, existingRoleDigests)) { IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(indexMetadata); - if (value != null) { - indexMetadataBuilder.putCustom(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST, value); + if (newRoleDigests != null) { + indexMetadataBuilder.putCustom(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST_KEY, newRoleDigests); } else { - indexMetadataBuilder.removeCustom(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST); + indexMetadataBuilder.removeCustom(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST_KEY); } indexMetadataBuilder.version(indexMetadataBuilder.version() + 1); ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(state.metadata().indices()); - builder.put(index, indexMetadataBuilder.build()); + builder.put(concreteSecurityIndexName, indexMetadataBuilder.build()); return new Tuple<>( ClusterState.builder(state).metadata(Metadata.builder(state.metadata()).indices(builder.build()).build()).build(), - value + newRoleDigests ); } else { // returns existing value when expectation is not met - return new Tuple<>(state, existingValue); + return new Tuple<>(state, existingRoleDigests); } } From e8217102f9fd31e62b53d96e6188eb0c4a22a8d7 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 3 Dec 2024 11:58:37 +0100 Subject: [PATCH 14/53] test that hash calculation produces consistent hash digest --- .../QueryableBuiltInRolesSynchronizer.java | 2 +- .../QueryableBuiltInRolesUtilsTests.java | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index a80ea02047a59..9654ac798b2f6 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -70,7 +70,7 @@ * attempting to create/update/delete roles, it’s still possible * that the master node changes in the middle of the syncing process. */ -public class QueryableBuiltInRolesSynchronizer implements ClusterStateListener { +public final class QueryableBuiltInRolesSynchronizer implements ClusterStateListener { private static final Logger logger = LogManager.getLogger(QueryableBuiltInRolesSynchronizer.class); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java new file mode 100644 index 0000000000000..fece8ac4e1a12 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java @@ -0,0 +1,23 @@ +/* + * 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.support; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; + +import static org.hamcrest.Matchers.equalTo; + +public class QueryableBuiltInRolesUtilsTests extends ESTestCase { + + public void testCalculateHash() { + assertThat( + QueryableBuiltInRolesUtils.calculateHash(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR), + equalTo("lRRmA3kPO1/ztr3ESAlTetOuDjgUC3fKcGS3ZCqM+6k=") + ); + } +} From d27c91b8ef3f21522a8dedf79b61f143278df631 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 3 Dec 2024 12:14:47 +0100 Subject: [PATCH 15/53] update log message --- .../security/support/QueryableBuiltInRolesSynchronizer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 9654ac798b2f6..083c0df8ce7c4 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -199,7 +199,7 @@ private void syncBuiltInRoles(QueryableBuiltInRoles roles, ClusterState state) { } if (synchronizationInProgress.compareAndSet(false, true)) { executor.execute(() -> doSyncBuiltinRoles(indexedRolesDigests, roles, ActionListener.wrap(v -> { - logger.info("Successfully synced built-in roles to .security index"); + logger.info("Successfully synced [" + roles.roleDescriptors().size() + "] built-in roles to .security index"); synchronizationInProgress.set(false); }, e -> { if (isExpectedFailure(e)) { From 9e7076dd1130774fcb428ad37b029baf7baebd26 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 3 Dec 2024 12:55:49 +0100 Subject: [PATCH 16/53] simple unit test for reserved roles provider --- .../QueryableReservedRolesProviderTests.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProviderTests.java diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProviderTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProviderTests.java new file mode 100644 index 0000000000000..e7beb702b09a3 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProviderTests.java @@ -0,0 +1,32 @@ +/* + * 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.support; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; + +import java.util.stream.Collectors; + +import static org.hamcrest.Matchers.equalTo; + +public class QueryableReservedRolesProviderTests extends ESTestCase { + + public void testReservedRoleProvider() { + QueryableReservedRolesProvider provider = new QueryableReservedRolesProvider(new ReservedRolesStore()); + assertNotNull(provider.getRoles()); + assertThat(provider.getRoles(), equalTo(provider.getRoles())); + assertThat(provider.getRoles().roleDescriptors(), equalTo(ReservedRolesStore.roleDescriptors())); + assertThat(provider.getRoles().rolesDigest().size(), equalTo(ReservedRolesStore.roleDescriptors().size())); + assertThat( + provider.getRoles().rolesDigest().keySet(), + equalTo(ReservedRolesStore.roleDescriptors().stream().map(RoleDescriptor::getName).collect(Collectors.toSet())) + ); + } + +} From 1b56b95109f1a299a531dcf323e582bc6f58e69f Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 3 Dec 2024 12:58:08 +0100 Subject: [PATCH 17/53] make collections immutable --- .../security/support/QueryableReservedRolesProvider.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java index d2d6f347ffd0d..710e94b7ac879 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java @@ -12,6 +12,7 @@ import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; import java.util.Collection; +import java.util.Collections; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -34,9 +35,10 @@ public final class QueryableReservedRolesProvider implements QueryableBuiltInRol */ public QueryableReservedRolesProvider(ReservedRolesStore reservedRolesStore) { this.reservedRolesSupplier = CachedSupplier.wrap(() -> { - final Collection roleDescriptors = ReservedRolesStore.roleDescriptors(); + final Collection roleDescriptors = Collections.unmodifiableCollection(ReservedRolesStore.roleDescriptors()); return new QueryableBuiltInRoles( - roleDescriptors.stream().collect(Collectors.toMap(RoleDescriptor::getName, QueryableBuiltInRolesUtils::calculateHash)), + roleDescriptors.stream() + .collect(Collectors.toUnmodifiableMap(RoleDescriptor::getName, QueryableBuiltInRolesUtils::calculateHash)), roleDescriptors ); }); From f35ba07b8f208afd8ab5c3885acbbdb7855053d0 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 3 Dec 2024 13:16:34 +0100 Subject: [PATCH 18/53] fix failing test --- .../security/support/QueryableReservedRolesProviderTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProviderTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProviderTests.java index e7beb702b09a3..6d01da7534e1f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProviderTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProviderTests.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.security.support; +import com.carrotsearch.randomizedtesting.annotations.Repeat; + import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; @@ -21,7 +23,6 @@ public void testReservedRoleProvider() { QueryableReservedRolesProvider provider = new QueryableReservedRolesProvider(new ReservedRolesStore()); assertNotNull(provider.getRoles()); assertThat(provider.getRoles(), equalTo(provider.getRoles())); - assertThat(provider.getRoles().roleDescriptors(), equalTo(ReservedRolesStore.roleDescriptors())); assertThat(provider.getRoles().rolesDigest().size(), equalTo(ReservedRolesStore.roleDescriptors().size())); assertThat( provider.getRoles().rolesDigest().keySet(), From 71ed79ba3da0e3ab639c78cb67dab766e676c490 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 3 Dec 2024 13:17:12 +0100 Subject: [PATCH 19/53] remove unused import --- .../security/support/QueryableReservedRolesProviderTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProviderTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProviderTests.java index 6d01da7534e1f..7beb078795b29 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProviderTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProviderTests.java @@ -7,8 +7,6 @@ package org.elasticsearch.xpack.security.support; -import com.carrotsearch.randomizedtesting.annotations.Repeat; - import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; From f4e0292ed27d9ca606eee96c065fcd3a93a0fd5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Slobodan=20Adamovi=C4=87?= Date: Tue, 3 Dec 2024 13:42:19 +0100 Subject: [PATCH 20/53] Update docs/changelog/117581.yaml --- docs/changelog/117581.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/117581.yaml diff --git a/docs/changelog/117581.yaml b/docs/changelog/117581.yaml new file mode 100644 index 0000000000000..b88017f45e9c9 --- /dev/null +++ b/docs/changelog/117581.yaml @@ -0,0 +1,5 @@ +pr: 117581 +summary: Make reserved built-in roles queryable +area: Authorization +type: enhancement +issues: [] From d94da64975d0f9d18ae1da44f352288d45cdb8c2 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 3 Dec 2024 15:05:01 +0100 Subject: [PATCH 21/53] mark query API as internal for now - will mark it as public in a followup which also updates the specification --- .../security/rest/action/apikey/RestQueryApiKeyAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java index 6f62b87ea715e..9602b03e4cd82 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java @@ -43,7 +43,7 @@ /** * Rest action to search for API keys */ -@ServerlessScope(Scope.PUBLIC) +@ServerlessScope(Scope.INTERNAL) public final class RestQueryApiKeyAction extends ApiKeyBaseRestHandler { @SuppressWarnings("unchecked") From ada74e13f7825395bc2f2ae6ac20e55470666c19 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 3 Dec 2024 15:06:33 +0100 Subject: [PATCH 22/53] mark correct API as internal --- .../security/rest/action/apikey/RestQueryApiKeyAction.java | 2 +- .../xpack/security/rest/action/role/RestQueryRoleAction.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java index 9602b03e4cd82..6f62b87ea715e 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java @@ -43,7 +43,7 @@ /** * Rest action to search for API keys */ -@ServerlessScope(Scope.INTERNAL) +@ServerlessScope(Scope.PUBLIC) public final class RestQueryApiKeyAction extends ApiKeyBaseRestHandler { @SuppressWarnings("unchecked") diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestQueryRoleAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestQueryRoleAction.java index 862ff2552b4e3..3637159479463 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestQueryRoleAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestQueryRoleAction.java @@ -34,7 +34,7 @@ import static org.elasticsearch.rest.RestRequest.Method.POST; import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg; -@ServerlessScope(Scope.PUBLIC) +@ServerlessScope(Scope.INTERNAL) public final class RestQueryRoleAction extends NativeRoleBaseRestHandler { @SuppressWarnings("unchecked") From 32d4494a1795cfa2513a51c388a3c79fc4b1bae9 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 3 Dec 2024 22:56:04 +0100 Subject: [PATCH 23/53] test get reserved roles --- .../security/QueryableReservedRolesIT.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java index 13b2a805ff070..755b143c2b4d6 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.security; import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; import org.elasticsearch.xpack.security.support.SecurityMigrations; @@ -28,6 +29,11 @@ public static void setup() { new ReservedRolesStore(); } + @Override + protected boolean preserveClusterUponCompletion() { + return true; + } + public void testQueryDeleteOrUpdateReservedRoles() throws Exception { waitForMigrationCompletion(adminClient(), SecurityMigrations.ROLE_METADATA_FLATTENED_MIGRATION_VERSION); @@ -53,6 +59,17 @@ public void testQueryDeleteOrUpdateReservedRoles() throws Exception { assertCreateOrUpdateReservedRole(roleName); } + public void testGetReservedRoles() throws Exception { + final String[] allReservedRoles = ReservedRolesStore.names().toArray(new String[0]); + final String roleName = randomFrom(allReservedRoles); + Request request = new Request("GET", "/_security/role/" + roleName); + Response response = adminClient().performRequest(request); + assertOK(response); + var responseMap = responseAsMap(response); + assertThat(responseMap.size(), equalTo(1)); + assertThat(responseMap.containsKey(roleName), is(true)); + } + private void assertDeleteReservedRole(String roleName) throws Exception { Request request = new Request("DELETE", "/_security/role/" + roleName); var e = expectThrows(ResponseException.class, () -> adminClient().performRequest(request)); From 21646f532f4bbe0e42021e6cb88b5360ed58f023 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 9 Dec 2024 11:31:55 +0100 Subject: [PATCH 24/53] remove QueryableBuiltInRolesStore and depend on NativeRolesStore --- .../xpack/security/Security.java | 3 +- .../authz/store/NativeRolesStore.java | 16 ++- .../store/QueryableBuiltInRolesStore.java | 49 --------- .../QueryableBuiltInRolesSynchronizer.java | 99 +++++++------------ .../support/SecurityIndexManager.java | 2 +- 5 files changed, 46 insertions(+), 123 deletions(-) delete mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java 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 e025dcda6afdd..9273d1bf64b6a 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 @@ -337,7 +337,6 @@ import org.elasticsearch.xpack.security.authz.store.FileRolesStore; import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore; import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; -import org.elasticsearch.xpack.security.authz.store.QueryableBuiltInRolesStore; import org.elasticsearch.xpack.security.authz.store.RoleProviders; import org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor; import org.elasticsearch.xpack.security.operator.DefaultOperatorOnlyRegistry; @@ -1215,7 +1214,7 @@ Collection createComponents( clusterService, featureService, queryableRolesProviderFactory.get(), - new QueryableBuiltInRolesStore(nativeRolesStore), + nativeRolesStore, reservedRolesStore, fileRolesStore.get(), systemIndices.getMainIndexManager(), 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 6168e76308d7a..04e90c2acea86 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 @@ -354,11 +354,10 @@ public void deleteRoles( WriteRequest.RefreshPolicy refreshPolicy, final ActionListener listener ) { - deleteRoles(null, roleNames, refreshPolicy, true, listener); + deleteRoles(roleNames, refreshPolicy, true, listener); } - void deleteRoles( - SecurityIndexManager securityIndexManager, + public void deleteRoles( final Collection roleNames, WriteRequest.RefreshPolicy refreshPolicy, boolean validateRoles, @@ -388,9 +387,7 @@ void deleteRoles( return; } - final SecurityIndexManager frozenSecurityIndex = securityIndexManager != null - ? securityIndexManager - : securityIndex.defensiveCopy(); + final SecurityIndexManager frozenSecurityIndex = securityIndex.defensiveCopy(); if (frozenSecurityIndex.indexExists() == false) { logger.debug("security index does not exist"); listener.onResponse(new BulkRolesResponse(List.of())); @@ -565,11 +562,10 @@ public void putRoles( final Collection roles, final ActionListener listener ) { - putRoles(securityIndex, refreshPolicy, roles, true, listener); + putRoles(refreshPolicy, roles, true, listener); } - void putRoles( - SecurityIndexManager securityIndexManager, + public void putRoles( final WriteRequest.RefreshPolicy refreshPolicy, final Collection roles, boolean validateRoles, @@ -608,7 +604,7 @@ void putRoles( return; } - securityIndexManager.prepareIndexIfNeededThenExecute( + securityIndex.prepareIndexIfNeededThenExecute( listener::onFailure, () -> executeAsyncWithOrigin( client.threadPool().getThreadContext(), diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java deleted file mode 100644 index b0efa5751c1a0..0000000000000 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/QueryableBuiltInRolesStore.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * 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.authz.store; - -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.support.WriteRequest; -import org.elasticsearch.xpack.core.security.action.role.BulkRolesResponse; -import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; -import org.elasticsearch.xpack.security.support.SecurityIndexManager; - -import java.util.Collection; - -/** - * Wrapper around the {@link NativeRolesStore} that provides a way to create, update and delete built-in roles. - */ -public final class QueryableBuiltInRolesStore { - - private final NativeRolesStore nativeRolesStore; - - public QueryableBuiltInRolesStore(NativeRolesStore nativeRolesStore) { - this.nativeRolesStore = nativeRolesStore; - } - - public void putRoles( - final SecurityIndexManager securityIndexManager, - final Collection roles, - final ActionListener listener - ) { - nativeRolesStore.putRoles(securityIndexManager, WriteRequest.RefreshPolicy.IMMEDIATE, roles, false, listener); - } - - public void deleteRoles( - final SecurityIndexManager securityIndexManager, - final Collection roleNames, - final ActionListener listener - ) { - nativeRolesStore.deleteRoles(securityIndexManager, roleNames, WriteRequest.RefreshPolicy.IMMEDIATE, false, listener); - } - - public boolean isEnabled() { - return nativeRolesStore.isEnabled(); - } - -} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 083c0df8ce7c4..0faded3b5bddb 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -14,13 +14,13 @@ import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.TransportActions; +import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateListener; import org.elasticsearch.cluster.ClusterStateTaskListener; import org.elasticsearch.cluster.NotMasterException; import org.elasticsearch.cluster.SimpleBatchedExecutor; -import org.elasticsearch.cluster.metadata.IndexAbstraction; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.service.ClusterService; @@ -45,11 +45,10 @@ import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; import org.elasticsearch.xpack.security.authz.store.FileRolesStore; -import org.elasticsearch.xpack.security.authz.store.QueryableBuiltInRolesStore; +import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; import java.util.Collection; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -124,8 +123,7 @@ public void taskSucceeded(MarkRolesAsSyncedTask task, Map value) private final ClusterService clusterService; private final FeatureService featureService; private final QueryableBuiltInRoles.Provider rolesProvider; - private final SecurityIndexManager securityIndex; - private final QueryableBuiltInRolesStore queryableBuiltInRolesStore; + private final NativeRolesStore nativeRolesStore; private final Executor executor; private final AtomicBoolean synchronizationInProgress = new AtomicBoolean(false); @@ -135,7 +133,7 @@ public QueryableBuiltInRolesSynchronizer( ClusterService clusterService, FeatureService featureService, QueryableBuiltInRolesProviderFactory rolesProviderFactory, - QueryableBuiltInRolesStore queryableBuiltInRolesStore, + NativeRolesStore nativeRolesStore, ReservedRolesStore reservedRolesStore, FileRolesStore fileRolesStore, SecurityIndexManager securityIndex, @@ -144,8 +142,7 @@ public QueryableBuiltInRolesSynchronizer( this.clusterService = clusterService; this.featureService = featureService; this.rolesProvider = rolesProviderFactory.createProvider(reservedRolesStore, fileRolesStore); - this.queryableBuiltInRolesStore = queryableBuiltInRolesStore; - this.securityIndex = securityIndex; + this.nativeRolesStore = nativeRolesStore; this.executor = threadPool.generic(); this.markRolesAsSyncedTaskQueue = clusterService.createTaskQueue( "mark-built-in-roles-as-synced-task-queue", @@ -191,14 +188,21 @@ public void clusterChanged(ClusterChangedEvent event) { } } - private void syncBuiltInRoles(QueryableBuiltInRoles roles, ClusterState state) { - final Map indexedRolesDigests = readIndexedBuiltInRolesDigests(state); + private void syncBuiltInRoles(QueryableBuiltInRoles roles, ClusterState clusterState) { + final IndexMetadata securityIndexMetadata = resolveSecurityIndexMetadata(clusterState.metadata()); + if (securityIndexMetadata == null) { + logger.debug("Security index does not exist, skipping built-in roles synchronization"); + return; + } + final Map indexedRolesDigests = securityIndexMetadata.getCustomData(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST_KEY); if (roles.rolesDigest().equals(indexedRolesDigests)) { logger.debug("Security index already contains the latest built-in roles indexed, skipping synchronization"); return; } + final Index securityIndex = securityIndexMetadata.getIndex(); + if (synchronizationInProgress.compareAndSet(false, true)) { - executor.execute(() -> doSyncBuiltinRoles(indexedRolesDigests, roles, ActionListener.wrap(v -> { + executor.execute(() -> doSyncBuiltinRoles(securityIndex, indexedRolesDigests, roles, ActionListener.wrap(v -> { logger.info("Successfully synced [" + roles.roleDescriptors().size() + "] built-in roles to .security index"); synchronizationInProgress.set(false); }, e -> { @@ -245,8 +249,8 @@ private boolean shouldSyncBuiltInRoles(ClusterState state) { logger.trace("Cluster state has not recovered yet, skipping built-in roles synchronization"); return false; } - if (queryableBuiltInRolesStore.isEnabled() == false) { - logger.trace("Role store is not enabled, skipping built-in roles synchronization"); + if (nativeRolesStore.isEnabled() == false) { + logger.trace("Native roles store is not enabled, skipping built-in roles synchronization"); return false; } if (state.nodes().getDataNodes().isEmpty()) { @@ -274,24 +278,21 @@ private boolean shouldSyncBuiltInRoles(ClusterState state) { return true; } - private void doSyncBuiltinRoles(Map indexedRolesDigests, QueryableBuiltInRoles roles, ActionListener listener) { - securityIndex.prepareIndexIfNeededThenExecute(listener::onFailure, () -> { - final SecurityIndexManager frozenSecurityIndex = securityIndex.defensiveCopy(); - if (frozenSecurityIndex.isAvailable(SecurityIndexManager.Availability.PRIMARY_SHARDS) == false) { - listener.onFailure(frozenSecurityIndex.getUnavailableReason(SecurityIndexManager.Availability.PRIMARY_SHARDS)); - + private void doSyncBuiltinRoles( + Index securityIndex, + Map indexedRolesDigests, + QueryableBuiltInRoles roles, + ActionListener listener + ) { + final Collection rolesToUpsert = rolesToUpsert(roles, indexedRolesDigests); + indexRoles(rolesToUpsert, ActionListener.wrap(onResponse -> { + final Set rolesToDelete = rolesToDelete(roles, indexedRolesDigests); + if (rolesToDelete.isEmpty()) { + markRolesAsSynced(securityIndex, indexedRolesDigests, roles.rolesDigest(), listener); } else { - final Collection rolesToUpsert = rolesToUpsert(roles, indexedRolesDigests); - indexRoles(rolesToUpsert, frozenSecurityIndex, ActionListener.wrap(onResponse -> { - final Set rolesToDelete = rolesToDelete(roles, indexedRolesDigests); - if (rolesToDelete.isEmpty()) { - markRolesAsSynced(frozenSecurityIndex.getConcreteIndexName(), indexedRolesDigests, roles.rolesDigest(), listener); - } else { - deleteRoles(frozenSecurityIndex, roles, rolesToDelete, indexedRolesDigests, listener); - } - }, listener::onFailure)); + deleteRoles(securityIndex, roles, rolesToDelete, indexedRolesDigests, listener); } - }); + }, listener::onFailure)); } private static Set rolesToDelete(QueryableBuiltInRoles roles, Map indexedRolesDigests) { @@ -312,13 +313,13 @@ private static Collection rolesToUpsert(QueryableBuiltInRoles ro } private void deleteRoles( - SecurityIndexManager securityIndex, + Index concreteSecurityIndex, QueryableBuiltInRoles roles, Set rolesToDelete, Map indexedRolesDigests, ActionListener listener ) { - queryableBuiltInRolesStore.deleteRoles(securityIndex, rolesToDelete, ActionListener.wrap(deleteResponse -> { + nativeRolesStore.deleteRoles(rolesToDelete, WriteRequest.RefreshPolicy.IMMEDIATE, false, ActionListener.wrap(deleteResponse -> { final Optional deleteFailure = deleteResponse.getItems() .stream() .filter(BulkRolesResponse.Item::isFailed) @@ -327,13 +328,13 @@ private void deleteRoles( if (deleteFailure.isPresent()) { listener.onFailure(deleteFailure.get()); } else { - markRolesAsSynced(securityIndex.getConcreteIndexName(), indexedRolesDigests, roles.rolesDigest(), listener); + markRolesAsSynced(concreteSecurityIndex, indexedRolesDigests, roles.rolesDigest(), listener); } }, listener::onFailure)); } - private void indexRoles(Collection rolesToUpsert, SecurityIndexManager securityIndex, ActionListener listener) { - queryableBuiltInRolesStore.putRoles(securityIndex, rolesToUpsert, ActionListener.wrap(response -> { + private void indexRoles(Collection rolesToUpsert, ActionListener listener) { + nativeRolesStore.putRoles(WriteRequest.RefreshPolicy.IMMEDIATE, rolesToUpsert, false, ActionListener.wrap(response -> { final Optional indexFailure = response.getItems() .stream() .filter(BulkRolesResponse.Item::isFailed) @@ -374,7 +375,7 @@ private boolean isSecurityIndexClosed(ClusterState state) { * .security index or if they are equal to the new roles digests. */ private void markRolesAsSynced( - String concreteSecurityIndexName, + Index concreteSecurityIndex, Map expectedRolesDigests, Map newRolesDigests, ActionListener listener @@ -397,37 +398,13 @@ private void markRolesAsSynced( } else { listener.onResponse(null); } - }, listener::onFailure), concreteSecurityIndexName, expectedRolesDigests, newRolesDigests), + }, listener::onFailure), concreteSecurityIndex.getName(), expectedRolesDigests, newRolesDigests), null ); } - private Map readIndexedBuiltInRolesDigests(ClusterState state) { - final IndexMetadata indexMetadata = resolveSecurityIndexMetadata(state.metadata()); - if (indexMetadata == null) { - return null; - } - return indexMetadata.getCustomData(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST_KEY); - } - private static IndexMetadata resolveSecurityIndexMetadata(final Metadata metadata) { - final Index index = resolveConcreteSecurityIndex(metadata); - if (index != null) { - return metadata.getIndexSafe(index); - } - return null; - } - - private static Index resolveConcreteSecurityIndex(final Metadata metadata) { - final IndexAbstraction indexAbstraction = metadata.getIndicesLookup().get(SECURITY_MAIN_ALIAS); - if (indexAbstraction != null) { - final List indices = indexAbstraction.getIndices(); - if (indexAbstraction.getType() != IndexAbstraction.Type.CONCRETE_INDEX && indices.size() > 1) { - throw new IllegalStateException("alias [" + SECURITY_MAIN_ALIAS + "] points to more than one index [" + indices + "]"); - } - return indices.get(0); - } - return null; + return SecurityIndexManager.resolveConcreteIndex(SECURITY_MAIN_ALIAS, metadata); } static class MarkRolesAsSyncedTask implements ClusterStateTaskListener { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index f3222a74b530c..78f7209c06e3a 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -586,7 +586,7 @@ private static int readMappingVersion(String indexName, MappingMetadata mappingM * Resolves a concrete index name or alias to a {@link IndexMetadata} instance. Requires * that if supplied with an alias, the alias resolves to at most one concrete index. */ - private static IndexMetadata resolveConcreteIndex(final String indexOrAliasName, final Metadata metadata) { + public static IndexMetadata resolveConcreteIndex(final String indexOrAliasName, final Metadata metadata) { final IndexAbstraction indexAbstraction = metadata.getIndicesLookup().get(indexOrAliasName); if (indexAbstraction != null) { final List indices = indexAbstraction.getIndices(); From a229e82fd1f48b8deb17b7d6acc027b526edd559 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 9 Dec 2024 12:54:48 +0100 Subject: [PATCH 25/53] move concrete index name resolution after index gets created --- .../QueryableBuiltInRolesSynchronizer.java | 43 ++++++++----------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 0faded3b5bddb..e2a1e7923bdb2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -189,20 +189,13 @@ public void clusterChanged(ClusterChangedEvent event) { } private void syncBuiltInRoles(QueryableBuiltInRoles roles, ClusterState clusterState) { - final IndexMetadata securityIndexMetadata = resolveSecurityIndexMetadata(clusterState.metadata()); - if (securityIndexMetadata == null) { - logger.debug("Security index does not exist, skipping built-in roles synchronization"); - return; - } - final Map indexedRolesDigests = securityIndexMetadata.getCustomData(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST_KEY); - if (roles.rolesDigest().equals(indexedRolesDigests)) { - logger.debug("Security index already contains the latest built-in roles indexed, skipping synchronization"); - return; - } - final Index securityIndex = securityIndexMetadata.getIndex(); - if (synchronizationInProgress.compareAndSet(false, true)) { - executor.execute(() -> doSyncBuiltinRoles(securityIndex, indexedRolesDigests, roles, ActionListener.wrap(v -> { + final Map indexedRolesDigests = readIndexedBuiltInRolesDigests(clusterService.state()); + if (roles.rolesDigest().equals(indexedRolesDigests)) { + logger.debug("Security index already contains the latest built-in roles indexed, skipping synchronization"); + return; + } + executor.execute(() -> doSyncBuiltinRoles(indexedRolesDigests, roles, ActionListener.wrap(v -> { logger.info("Successfully synced [" + roles.roleDescriptors().size() + "] built-in roles to .security index"); synchronizationInProgress.set(false); }, e -> { @@ -278,19 +271,14 @@ private boolean shouldSyncBuiltInRoles(ClusterState state) { return true; } - private void doSyncBuiltinRoles( - Index securityIndex, - Map indexedRolesDigests, - QueryableBuiltInRoles roles, - ActionListener listener - ) { + private void doSyncBuiltinRoles(Map indexedRolesDigests, QueryableBuiltInRoles roles, ActionListener listener) { final Collection rolesToUpsert = rolesToUpsert(roles, indexedRolesDigests); indexRoles(rolesToUpsert, ActionListener.wrap(onResponse -> { final Set rolesToDelete = rolesToDelete(roles, indexedRolesDigests); if (rolesToDelete.isEmpty()) { - markRolesAsSynced(securityIndex, indexedRolesDigests, roles.rolesDigest(), listener); + markRolesAsSynced(indexedRolesDigests, roles.rolesDigest(), listener); } else { - deleteRoles(securityIndex, roles, rolesToDelete, indexedRolesDigests, listener); + deleteRoles(roles, rolesToDelete, indexedRolesDigests, listener); } }, listener::onFailure)); } @@ -313,7 +301,6 @@ private static Collection rolesToUpsert(QueryableBuiltInRoles ro } private void deleteRoles( - Index concreteSecurityIndex, QueryableBuiltInRoles roles, Set rolesToDelete, Map indexedRolesDigests, @@ -328,7 +315,7 @@ private void deleteRoles( if (deleteFailure.isPresent()) { listener.onFailure(deleteFailure.get()); } else { - markRolesAsSynced(concreteSecurityIndex, indexedRolesDigests, roles.rolesDigest(), listener); + markRolesAsSynced(indexedRolesDigests, roles.rolesDigest(), listener); } }, listener::onFailure)); } @@ -375,11 +362,11 @@ private boolean isSecurityIndexClosed(ClusterState state) { * .security index or if they are equal to the new roles digests. */ private void markRolesAsSynced( - Index concreteSecurityIndex, Map expectedRolesDigests, Map newRolesDigests, ActionListener listener ) { + final Index concreteSecurityIndex = resolveSecurityIndexMetadata(clusterService.state().metadata()).getIndex(); markRolesAsSyncedTaskQueue.submitTask( "mark built-in roles as synced task", new MarkRolesAsSyncedTask(ActionListener.wrap(response -> { @@ -403,6 +390,14 @@ private void markRolesAsSynced( ); } + private Map readIndexedBuiltInRolesDigests(ClusterState state) { + final IndexMetadata indexMetadata = resolveSecurityIndexMetadata(state.metadata()); + if (indexMetadata == null) { + return null; + } + return indexMetadata.getCustomData(METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST_KEY); + } + private static IndexMetadata resolveSecurityIndexMetadata(final Metadata metadata) { return SecurityIndexManager.resolveConcreteIndex(SECURITY_MAIN_ALIAS, metadata); } From 8e551b78c608cb1f3244bdd62d8d725363fa9dc8 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 9 Dec 2024 23:44:05 +0100 Subject: [PATCH 26/53] revert changes to QueryRoleIT --- .../xpack/security/QueryRoleIT.java | 15 ++--- .../security/QueryableReservedRolesIT.java | 66 +++++++++++++++++-- .../security/SecurityInBasicRestTestCase.java | 1 - 3 files changed, 66 insertions(+), 16 deletions(-) diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryRoleIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryRoleIT.java index 08d21db710f91..311510352d805 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryRoleIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryRoleIT.java @@ -49,16 +49,15 @@ public final class QueryRoleIT extends SecurityInBasicRestTestCase { private static final String READ_SECURITY_USER_AUTH_HEADER = "Basic cmVhZF9zZWN1cml0eV91c2VyOnJlYWQtc2VjdXJpdHktcGFzc3dvcmQ="; - public void testSimpleQueryAllRoles() throws Exception { - createRandomRole(); - - // 31 built-in reserved roles + 1 random role - assertQuery("", 1 + 31, roles -> { - // default size is 10 - assertThat(roles, iterableWithSize(10)); + public void testSimpleQueryAllRoles() throws IOException { + assertQuery("", 0, roles -> assertThat(roles, emptyIterable())); + RoleDescriptor createdRole = createRandomRole(); + assertQuery("", 1, roles -> { + assertThat(roles, iterableWithSize(1)); + assertRoleMap(roles.get(0), createdRole); }); assertQuery(""" - {"query":{"match_all":{}},"from":32}""", 1 + 31, roles -> assertThat(roles, emptyIterable())); + {"query":{"match_all":{}},"from":1}""", 1, roles -> assertThat(roles, emptyIterable())); } public void testDisallowedFields() throws Exception { diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java index 755b143c2b4d6..ee2ea1675a9d0 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java @@ -10,9 +10,18 @@ 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.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.cluster.local.distribution.DistributionType; +import org.elasticsearch.test.cluster.local.model.User; +import org.elasticsearch.test.cluster.util.resource.Resource; +import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; import org.elasticsearch.xpack.security.support.SecurityMigrations; import org.junit.BeforeClass; +import org.junit.ClassRule; import static org.elasticsearch.xpack.security.QueryRoleIT.assertQuery; import static org.elasticsearch.xpack.security.QueryRoleIT.waitForMigrationCompletion; @@ -22,7 +31,15 @@ import static org.hamcrest.Matchers.iterableWithSize; import static org.hamcrest.Matchers.oneOf; -public class QueryableReservedRolesIT extends SecurityInBasicRestTestCase { +public class QueryableReservedRolesIT extends ESRestTestCase { + + protected static final String REST_USER = "security_test_user"; + private static final SecureString REST_PASSWORD = new SecureString("security-test-password".toCharArray()); + private static final String ADMIN_USER = "admin_user"; + private static final SecureString ADMIN_PASSWORD = new SecureString("admin-password".toCharArray()); + + protected static final String READ_SECURITY_USER = "read_security_user"; + private static final SecureString READ_SECURITY_PASSWORD = new SecureString("read-security-password".toCharArray()); @BeforeClass public static void setup() { @@ -34,11 +51,46 @@ protected boolean preserveClusterUponCompletion() { return true; } + @ClassRule + public static ElasticsearchCluster cluster = ElasticsearchCluster.local() + .nodes(2) + .distribution(DistributionType.DEFAULT) + .setting("xpack.license.self_generated.type", "basic") + .setting("xpack.security.enabled", "true") + .setting("xpack.security.ssl.diagnose.trust", "true") + .setting("xpack.security.http.ssl.enabled", "false") + .setting("xpack.security.transport.ssl.enabled", "false") + .setting("xpack.security.authc.token.enabled", "true") + .setting("xpack.security.authc.api_key.enabled", "true") + .rolesFile(Resource.fromClasspath("roles.yml")) + .user(ADMIN_USER, ADMIN_PASSWORD.toString(), User.ROOT_USER_ROLE, true) + .user(REST_USER, REST_PASSWORD.toString(), "security_test_role", false) + .user(READ_SECURITY_USER, READ_SECURITY_PASSWORD.toString(), "read_security_user_role", false) + .systemProperty("es.queryable_built_in_roles_enabled", "true") + .build(); + + @Override + protected String getTestRestCluster() { + return cluster.getHttpAddresses(); + } + + @Override + protected Settings restAdminSettings() { + String token = basicAuthHeaderValue(ADMIN_USER, ADMIN_PASSWORD); + return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token).build(); + } + + @Override + protected Settings restClientSettings() { + String token = basicAuthHeaderValue(REST_USER, REST_PASSWORD); + return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token).build(); + } + public void testQueryDeleteOrUpdateReservedRoles() throws Exception { waitForMigrationCompletion(adminClient(), SecurityMigrations.ROLE_METADATA_FLATTENED_MIGRATION_VERSION); final String[] allReservedRoles = ReservedRolesStore.names().toArray(new String[0]); - assertQuery(""" + assertQuery(client(), """ { "query": { "bool": { "must": { "term": { "metadata._reserved": true } } } }, "size": 100 } """, 31, roles -> { assertThat(roles, iterableWithSize(31)); @@ -48,15 +100,15 @@ public void testQueryDeleteOrUpdateReservedRoles() throws Exception { }); final String roleName = randomFrom(allReservedRoles); - assertQuery(String.format(""" + assertQuery(client(), String.format(""" { "query": { "bool": { "must": { "term": { "name": "%s" } } } } } """, roleName), 1, roles -> { assertThat(roles, iterableWithSize(1)); assertThat((String) roles.get(0).get("name"), equalTo(roleName)); }); - assertDeleteReservedRole(roleName); - assertCreateOrUpdateReservedRole(roleName); + assertCannotDeleteReservedRole(roleName); + assertCannotCreateOrUpdateReservedRole(roleName); } public void testGetReservedRoles() throws Exception { @@ -70,13 +122,13 @@ public void testGetReservedRoles() throws Exception { assertThat(responseMap.containsKey(roleName), is(true)); } - private void assertDeleteReservedRole(String roleName) throws Exception { + private void assertCannotDeleteReservedRole(String roleName) throws Exception { Request request = new Request("DELETE", "/_security/role/" + roleName); var e = expectThrows(ResponseException.class, () -> adminClient().performRequest(request)); assertThat(e.getMessage(), containsString("role [" + roleName + "] is reserved and cannot be deleted")); } - private void assertCreateOrUpdateReservedRole(String roleName) throws Exception { + private void assertCannotCreateOrUpdateReservedRole(String roleName) throws Exception { Request request = new Request(randomBoolean() ? "PUT" : "POST", "/_security/role/" + roleName); request.setJsonEntity(""" { diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityInBasicRestTestCase.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityInBasicRestTestCase.java index 21c5c4c62ac80..7cb8c09545bb1 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityInBasicRestTestCase.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityInBasicRestTestCase.java @@ -54,7 +54,6 @@ public abstract class SecurityInBasicRestTestCase extends ESRestTestCase { .user(API_KEY_USER, API_KEY_USER_PASSWORD.toString(), "api_key_user_role", false) .user(API_KEY_ADMIN_USER, API_KEY_ADMIN_USER_PASSWORD.toString(), "api_key_admin_role", false) .user(READ_SECURITY_USER, READ_SECURITY_PASSWORD.toString(), "read_security_user_role", false) - .systemProperty("es.queryable_built_in_roles_enabled", "true") .build(); @Override From c6330376c4609979f27b7d343072753c338f4ac4 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 9 Dec 2024 23:55:46 +0100 Subject: [PATCH 27/53] test that bulk delete of built-in roles fails --- .../security/QueryableReservedRolesIT.java | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java index ee2ea1675a9d0..da3e2b41f5231 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java @@ -107,7 +107,7 @@ public void testQueryDeleteOrUpdateReservedRoles() throws Exception { assertThat((String) roles.get(0).get("name"), equalTo(roleName)); }); - assertCannotDeleteReservedRole(roleName); + assertCannotDeleteReservedRoles(); assertCannotCreateOrUpdateReservedRole(roleName); } @@ -122,10 +122,31 @@ public void testGetReservedRoles() throws Exception { assertThat(responseMap.containsKey(roleName), is(true)); } - private void assertCannotDeleteReservedRole(String roleName) throws Exception { - Request request = new Request("DELETE", "/_security/role/" + roleName); - var e = expectThrows(ResponseException.class, () -> adminClient().performRequest(request)); - assertThat(e.getMessage(), containsString("role [" + roleName + "] is reserved and cannot be deleted")); + private void assertCannotDeleteReservedRoles() throws Exception { + { + String roleName = randomFrom(ReservedRolesStore.names()); + Request request = new Request("DELETE", "/_security/role/" + roleName); + var e = expectThrows(ResponseException.class, () -> adminClient().performRequest(request)); + assertThat(e.getMessage(), containsString("role [" + roleName + "] is reserved and cannot be deleted")); + } + { + Request request = new Request("DELETE", "/_security/role/"); + request.setJsonEntity( + """ + { + "names": [%s] + } + """.formatted( + ReservedRolesStore.names().stream().map(name -> "\"" + name + "\"").reduce((a, b) -> a + ", " + b).orElse("") + ) + ); + Response response = adminClient().performRequest(request); + assertOK(response); + String responseAsString = responseAsMap(response).toString(); + for (String roleName : ReservedRolesStore.names()) { + assertThat(responseAsString, containsString("role [" + roleName + "] is reserved and cannot be deleted")); + } + } } private void assertCannotCreateOrUpdateReservedRole(String roleName) throws Exception { From d4f99f25be360b289e8656c19a3f741503c33731 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 10 Dec 2024 00:00:48 +0100 Subject: [PATCH 28/53] log 'expected' errors at info level --- .../security/support/QueryableBuiltInRolesSynchronizer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index e2a1e7923bdb2..832b6359464eb 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -200,7 +200,7 @@ private void syncBuiltInRoles(QueryableBuiltInRoles roles, ClusterState clusterS synchronizationInProgress.set(false); }, e -> { if (isExpectedFailure(e)) { - logger.trace("Failed to sync built-in roles to .security index", e); + logger.info("Failed to sync built-in roles to .security index", e); } else { logger.warn("Failed to sync built-in roles to .security index due to unexpected exception", e); } From 8cc05abfd663b8d781a82e68786e2ad955659323 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 10 Dec 2024 10:32:03 +0100 Subject: [PATCH 29/53] use delegateFailureAndWrap --- .../support/QueryableBuiltInRolesSynchronizer.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 832b6359464eb..6216b8f24d1aa 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -369,7 +369,7 @@ private void markRolesAsSynced( final Index concreteSecurityIndex = resolveSecurityIndexMetadata(clusterService.state().metadata()).getIndex(); markRolesAsSyncedTaskQueue.submitTask( "mark built-in roles as synced task", - new MarkRolesAsSyncedTask(ActionListener.wrap(response -> { + new MarkRolesAsSyncedTask(listener.delegateFailureAndWrap((l, response) -> { if (newRolesDigests.equals(response) == false) { logger.trace( () -> Strings.format( @@ -379,13 +379,13 @@ private void markRolesAsSynced( response ) ); - listener.onFailure( + l.onFailure( new ElasticsearchException("Failed to mark built-in roles as synced. The expected roles digests have changed.") ); } else { - listener.onResponse(null); + l.onResponse(null); } - }, listener::onFailure), concreteSecurityIndex.getName(), expectedRolesDigests, newRolesDigests), + }), concreteSecurityIndex.getName(), expectedRolesDigests, newRolesDigests), null ); } From 085755d9d532a9b63fbe0700e39e864aa49b5bb9 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 10 Dec 2024 10:37:34 +0100 Subject: [PATCH 30/53] sanity check rolesToDelete and rolesToUpsert --- .../support/QueryableBuiltInRolesSynchronizer.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 6216b8f24d1aa..bb775a010e283 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -55,6 +55,7 @@ import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MAIN_ALIAS; @@ -272,9 +273,11 @@ private boolean shouldSyncBuiltInRoles(ClusterState state) { } private void doSyncBuiltinRoles(Map indexedRolesDigests, QueryableBuiltInRoles roles, ActionListener listener) { - final Collection rolesToUpsert = rolesToUpsert(roles, indexedRolesDigests); + final Set rolesToUpsert = rolesToUpsert(roles, indexedRolesDigests); + final Set rolesToDelete = rolesToDelete(roles, indexedRolesDigests); + assert Sets.intersection(rolesToUpsert.stream().map(RoleDescriptor::getName).collect(Collectors.toSet()), rolesToDelete).isEmpty() + : "The roles to upsert and delete should not have any common roles"; indexRoles(rolesToUpsert, ActionListener.wrap(onResponse -> { - final Set rolesToDelete = rolesToDelete(roles, indexedRolesDigests); if (rolesToDelete.isEmpty()) { markRolesAsSynced(indexedRolesDigests, roles.rolesDigest(), listener); } else { @@ -287,7 +290,7 @@ private static Set rolesToDelete(QueryableBuiltInRoles roles, Map rolesToUpsert(QueryableBuiltInRoles roles, Map indexedRolesDigests) { + private static Set rolesToUpsert(QueryableBuiltInRoles roles, Map indexedRolesDigests) { final Set rolesToUpsert = new HashSet<>(); for (var role : roles.roleDescriptors()) { final String roleDigest = roles.rolesDigest().get(role.getName()); From c3ee0b50361917de72a655214d8d973f74d9a86b Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 10 Dec 2024 10:43:36 +0100 Subject: [PATCH 31/53] return empty query result if native roles are disabled --- .../xpack/security/authz/store/NativeRolesStore.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 04e90c2acea86..44c3c025477fd 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 @@ -268,7 +268,7 @@ public boolean isMetadataSearchable() { public void queryRoleDescriptors(SearchSourceBuilder searchSourceBuilder, ActionListener listener) { if (enabled == false) { - listener.onFailure(new IllegalStateException("Native role management is disabled")); + listener.onResponse(QueryRoleResult.EMPTY); return; } SearchRequest searchRequest = new SearchRequest(new String[] { SECURITY_MAIN_ALIAS }, searchSourceBuilder); From a6bc0778e1c6e5df0d1305fe8954e5f33ac4bc33 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 10 Dec 2024 10:45:24 +0100 Subject: [PATCH 32/53] naming nit validateRoles -> allowReservedRoleNames --- .../xpack/security/authz/store/NativeRolesStore.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 44c3c025477fd..c9e0f00257494 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 @@ -360,7 +360,7 @@ public void deleteRoles( public void deleteRoles( final Collection roleNames, WriteRequest.RefreshPolicy refreshPolicy, - boolean validateRoles, + boolean allowReservedRoleNames, final ActionListener listener ) { if (enabled == false) { @@ -372,7 +372,7 @@ public void deleteRoles( Map validationErrorByRoleName = new HashMap<>(); for (String roleName : roleNames) { - if (validateRoles && reservedRoleNameChecker.isReserved(roleName)) { + if (allowReservedRoleNames && reservedRoleNameChecker.isReserved(roleName)) { validationErrorByRoleName.put( roleName, new IllegalArgumentException("role [" + roleName + "] is reserved and cannot be deleted") @@ -568,7 +568,7 @@ public void putRoles( public void putRoles( final WriteRequest.RefreshPolicy refreshPolicy, final Collection roles, - boolean validateRoles, + boolean allowReservedRoleNames, final ActionListener listener ) { if (enabled == false) { @@ -581,7 +581,7 @@ public void putRoles( for (RoleDescriptor role : roles) { Exception validationException; try { - validationException = validateRoles ? validateRoleDescriptor(role) : null; + validationException = allowReservedRoleNames ? validateRoleDescriptor(role) : null; } catch (Exception e) { validationException = e; } From e12c3e822f4ba7448c869b602d2d68e5a4ff2da2 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 10 Dec 2024 10:50:10 +0100 Subject: [PATCH 33/53] trace -> debug --- .../security/support/QueryableBuiltInRolesSynchronizer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index bb775a010e283..5066218a46dfd 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -374,7 +374,7 @@ private void markRolesAsSynced( "mark built-in roles as synced task", new MarkRolesAsSyncedTask(listener.delegateFailureAndWrap((l, response) -> { if (newRolesDigests.equals(response) == false) { - logger.trace( + logger.debug( () -> Strings.format( "Another master node most probably indexed a newer versions of built-in roles in the meantime. " + "Expected: [%s], Actual: [%s]", From 70181d67f69de7297707fbf875e2b0b0c80c648b Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 10 Dec 2024 21:19:57 +0100 Subject: [PATCH 34/53] change role hashing implementation to hash ordered and flattened JSON map --- .../core/security/authz/RoleDescriptor.java | 5 ++-- .../QueryableBuiltInRolesSynchronizer.java | 6 ++--- .../support/QueryableBuiltInRolesUtils.java | 27 +++++++++++++------ .../QueryableBuiltInRolesUtilsTests.java | 2 +- 4 files changed, 25 insertions(+), 15 deletions(-) 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 495a7a9fca5a0..9f5aaa8562a88 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 @@ -50,7 +50,6 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; -import java.util.TreeMap; import static org.elasticsearch.common.xcontent.XContentHelper.createParserNotCompressed; import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS; @@ -190,9 +189,9 @@ public RoleDescriptor( this.indicesPrivileges = indicesPrivileges != null ? indicesPrivileges : IndicesPrivileges.NONE; this.applicationPrivileges = applicationPrivileges != null ? applicationPrivileges : ApplicationResourcePrivileges.NONE; this.runAs = runAs != null ? runAs : Strings.EMPTY_ARRAY; - this.metadata = metadata != null ? Collections.unmodifiableMap(new TreeMap<>(metadata)) : Collections.emptyMap(); + this.metadata = metadata != null ? Collections.unmodifiableMap(metadata) : Collections.emptyMap(); this.transientMetadata = transientMetadata != null - ? Collections.unmodifiableMap(new TreeMap<>(transientMetadata)) + ? Collections.unmodifiableMap(transientMetadata) : Collections.singletonMap("enabled", true); this.remoteIndicesPrivileges = remoteIndicesPrivileges != null ? remoteIndicesPrivileges : RemoteIndicesPrivileges.NONE; this.remoteClusterPermissions = remoteClusterPermissions != null && remoteClusterPermissions.hasAnyPrivileges() diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 5066218a46dfd..8d9c72a381c6e 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -179,9 +179,9 @@ public void clusterChanged(ClusterChangedEvent event) { this.securityIndexDeleted = true; logger.trace("Security index has been deleted, skipping built-in roles synchronization"); return; - } else if (isSecurityIndexCreated(event)) { + } else if (isSecurityIndexCreatedOrRecovered(event)) { this.securityIndexDeleted = false; - logger.trace("Security index has been created, attempting to sync built-in roles"); + logger.trace("Security index has been created/recovered, attempting to sync built-in roles"); } if (shouldSyncBuiltInRoles(state)) { final QueryableBuiltInRoles roles = rolesProvider.getRoles(); @@ -344,7 +344,7 @@ private boolean isSecurityIndexDeleted(ClusterChangedEvent event) { return previousSecurityIndexMetadata != null && currentSecurityIndexMetadata == null; } - private boolean isSecurityIndexCreated(ClusterChangedEvent event) { + private boolean isSecurityIndexCreatedOrRecovered(ClusterChangedEvent event) { final IndexMetadata previousSecurityIndexMetadata = resolveSecurityIndexMetadata(event.previousState().metadata()); final IndexMetadata currentSecurityIndexMetadata = resolveSecurityIndexMetadata(event.state().metadata()); return previousSecurityIndexMetadata == null && currentSecurityIndexMetadata != null; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java index 1f5eea0b1da73..e2d9f32e7fe37 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java @@ -9,13 +9,21 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.hash.MessageDigests; -import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.util.Maps; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentFactory; +import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.util.Base64; +import java.util.Map; + +import static org.elasticsearch.xcontent.ToXContent.EMPTY_PARAMS; /** * Utility class which provides helper method for calculating the hash of a role descriptor. @@ -31,13 +39,16 @@ public final class QueryableBuiltInRolesUtils { */ public static String calculateHash(final RoleDescriptor roleDescriptor) { final MessageDigest hash = MessageDigests.sha256(); - try (BytesStreamOutput out = new BytesStreamOutput();) { - try { - roleDescriptor.writeTo(out); - } catch (IOException e) { - throw new IllegalStateException("failed to serialize role descriptor", e); - } - hash.update(BytesReference.toBytes(out.bytes())); + try (XContentBuilder jsonBuilder = XContentFactory.jsonBuilder()) { + roleDescriptor.toXContent(jsonBuilder, EMPTY_PARAMS); + final Map flattenMap = Maps.flatten( + XContentHelper.convertToMap(BytesReference.bytes(jsonBuilder), true, XContentType.JSON).v2(), + false, + true + ); + hash.update(flattenMap.toString().getBytes(StandardCharsets.UTF_8)); + } catch (IOException e) { + throw new IllegalStateException("failed to compute digest for [" + roleDescriptor.getName() + "] role", e); } // HEX vs Base64 encoding is a trade-off between readability and space efficiency // opting for Base64 here to reduce the size of the cluster state diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java index fece8ac4e1a12..a760349bd1dbd 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java @@ -17,7 +17,7 @@ public class QueryableBuiltInRolesUtilsTests extends ESTestCase { public void testCalculateHash() { assertThat( QueryableBuiltInRolesUtils.calculateHash(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR), - equalTo("lRRmA3kPO1/ztr3ESAlTetOuDjgUC3fKcGS3ZCqM+6k=") + equalTo("bWEFdFo4WX229wdhdecfiz5QHMYEssh3ex8hizRgg+Q=") ); } } From 7dce761035ae011316c187c86bc3e818b7a005d2 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 11 Dec 2024 09:58:40 +0100 Subject: [PATCH 35/53] avoid magic numbers in assertion --- .../xpack/security/QueryableReservedRolesIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java index da3e2b41f5231..09ef0f5dc5222 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java @@ -92,8 +92,8 @@ public void testQueryDeleteOrUpdateReservedRoles() throws Exception { final String[] allReservedRoles = ReservedRolesStore.names().toArray(new String[0]); assertQuery(client(), """ { "query": { "bool": { "must": { "term": { "metadata._reserved": true } } } }, "size": 100 } - """, 31, roles -> { - assertThat(roles, iterableWithSize(31)); + """, allReservedRoles.length, roles -> { + assertThat(roles, iterableWithSize(allReservedRoles.length)); for (var role : roles) { assertThat((String) role.get("name"), is(oneOf(allReservedRoles))); } From d400669b5a06f1bf3d7c2a99ce1ae7d4d8df9f61 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 11 Dec 2024 16:18:57 +0100 Subject: [PATCH 36/53] introduce a test plugin in order to test reserved roles change --- .../security/qa/security-basic/build.gradle | 16 +++- .../security/QueryableReservedRolesIT.java | 74 ++++++++++++++++--- .../src/main/java/module-info.java | 6 ++ .../role/QueryableBuiltInRolesTestPlugin.java | 22 ++++++ .../action/role/TransportGetRolesAction.java | 8 +- 5 files changed, 109 insertions(+), 17 deletions(-) create mode 100644 x-pack/plugin/security/qa/security-basic/src/main/java/module-info.java create mode 100644 x-pack/plugin/security/qa/security-basic/src/main/java/org/elasticsearch/xpack/security/role/QueryableBuiltInRolesTestPlugin.java diff --git a/x-pack/plugin/security/qa/security-basic/build.gradle b/x-pack/plugin/security/qa/security-basic/build.gradle index 8740354646346..ca02fb673f5d7 100644 --- a/x-pack/plugin/security/qa/security-basic/build.gradle +++ b/x-pack/plugin/security/qa/security-basic/build.gradle @@ -4,12 +4,22 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ - +apply plugin: 'elasticsearch.base-internal-es-plugin' apply plugin: 'elasticsearch.internal-java-rest-test' + +esplugin { + name 'queryable-reserved-roles-test' + description 'A test plugin for testing that changes to reserved roles are made queryable' + classname 'org.elasticsearch.xpack.security.role.QueryableBuiltInRolesTestPlugin' + extendedPlugins = ['x-pack-core', 'x-pack-security'] +} dependencies { javaRestTestImplementation(testArtifact(project(xpackModule('security')))) javaRestTestImplementation(testArtifact(project(xpackModule('core')))) + compileOnly project(':x-pack:plugin:core') + compileOnly project(':x-pack:plugin:security') + clusterPlugins project(':x-pack:plugin:security:qa:security-basic') } tasks.named('javaRestTest') { @@ -17,7 +27,7 @@ tasks.named('javaRestTest') { } -if (buildParams.inFipsJvm){ +if (buildParams.inFipsJvm) { // This test cluster is using a BASIC license and FIPS 140 mode is not supported in BASIC - tasks.named("javaRestTest").configure{enabled = false } + tasks.named("javaRestTest").configure { enabled = false } } diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java index 09ef0f5dc5222..eff25ab24b65b 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java @@ -7,13 +7,19 @@ package org.elasticsearch.xpack.security; +import com.carrotsearch.randomizedtesting.annotations.TestCaseOrdering; + import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.test.AnnotationTestOrdering; +import org.elasticsearch.test.AnnotationTestOrdering.Order; import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.cluster.MutableSettingsProvider; import org.elasticsearch.test.cluster.local.distribution.DistributionType; import org.elasticsearch.test.cluster.local.model.User; import org.elasticsearch.test.cluster.util.resource.Resource; @@ -23,6 +29,11 @@ import org.junit.BeforeClass; import org.junit.ClassRule; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + import static org.elasticsearch.xpack.security.QueryRoleIT.assertQuery; import static org.elasticsearch.xpack.security.QueryRoleIT.waitForMigrationCompletion; import static org.hamcrest.Matchers.containsString; @@ -31,13 +42,13 @@ import static org.hamcrest.Matchers.iterableWithSize; import static org.hamcrest.Matchers.oneOf; +@TestCaseOrdering(AnnotationTestOrdering.class) public class QueryableReservedRolesIT extends ESRestTestCase { protected static final String REST_USER = "security_test_user"; private static final SecureString REST_PASSWORD = new SecureString("security-test-password".toCharArray()); private static final String ADMIN_USER = "admin_user"; private static final SecureString ADMIN_PASSWORD = new SecureString("admin-password".toCharArray()); - protected static final String READ_SECURITY_USER = "read_security_user"; private static final SecureString READ_SECURITY_PASSWORD = new SecureString("read-security-password".toCharArray()); @@ -51,24 +62,30 @@ protected boolean preserveClusterUponCompletion() { return true; } + private static MutableSettingsProvider clusterSettings = new MutableSettingsProvider() { + { + put("xpack.license.self_generated.type", "basic"); + put("xpack.security.enabled", "true"); + put("xpack.security.http.ssl.enabled", "false"); + put("xpack.security.transport.ssl.enabled", "false"); + } + }; + @ClassRule public static ElasticsearchCluster cluster = ElasticsearchCluster.local() - .nodes(2) .distribution(DistributionType.DEFAULT) - .setting("xpack.license.self_generated.type", "basic") - .setting("xpack.security.enabled", "true") - .setting("xpack.security.ssl.diagnose.trust", "true") - .setting("xpack.security.http.ssl.enabled", "false") - .setting("xpack.security.transport.ssl.enabled", "false") - .setting("xpack.security.authc.token.enabled", "true") - .setting("xpack.security.authc.api_key.enabled", "true") + .nodes(2) + .settings(clusterSettings) .rolesFile(Resource.fromClasspath("roles.yml")) .user(ADMIN_USER, ADMIN_PASSWORD.toString(), User.ROOT_USER_ROLE, true) .user(REST_USER, REST_PASSWORD.toString(), "security_test_role", false) .user(READ_SECURITY_USER, READ_SECURITY_PASSWORD.toString(), "read_security_user_role", false) .systemProperty("es.queryable_built_in_roles_enabled", "true") + .plugin("queryable-reserved-roles-test") .build(); + private static Set CONFIGURED_RESERVED_ROLES; + @Override protected String getTestRestCluster() { return cluster.getHttpAddresses(); @@ -86,6 +103,7 @@ protected Settings restClientSettings() { return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token).build(); } + @Order(10) public void testQueryDeleteOrUpdateReservedRoles() throws Exception { waitForMigrationCompletion(adminClient(), SecurityMigrations.ROLE_METADATA_FLATTENED_MIGRATION_VERSION); @@ -111,6 +129,7 @@ public void testQueryDeleteOrUpdateReservedRoles() throws Exception { assertCannotCreateOrUpdateReservedRole(roleName); } + @Order(11) public void testGetReservedRoles() throws Exception { final String[] allReservedRoles = ReservedRolesStore.names().toArray(new String[0]); final String roleName = randomFrom(allReservedRoles); @@ -122,6 +141,43 @@ public void testGetReservedRoles() throws Exception { assertThat(responseMap.containsKey(roleName), is(true)); } + @Order(20) + public void testRestartForConfiguringReservedRoles() throws Exception { + CONFIGURED_RESERVED_ROLES = new HashSet<>(); + CONFIGURED_RESERVED_ROLES.add("superuser"); + CONFIGURED_RESERVED_ROLES.addAll( + randomSubsetOf(List.of("editor", "viewer", "kibana_system", "apm_system", "beats_system", "logstash_system")) + ); + clusterSettings.put("xpack.security.reserved_roles.include", Strings.collectionToCommaDelimitedString(CONFIGURED_RESERVED_ROLES)); + cluster.restart(false); + closeClients(); + } + + @Order(30) + public void testConfiguredReservedRoles() throws Exception { + assert CONFIGURED_RESERVED_ROLES != null; + + // Test query roles API + assertBusy(() -> { + assertQuery(client(), """ + { "query": { "bool": { "must": { "term": { "metadata._reserved": true } } } }, "size": 100 } + """, CONFIGURED_RESERVED_ROLES.size(), roles -> { + assertThat(roles, iterableWithSize(CONFIGURED_RESERVED_ROLES.size())); + for (var role : roles) { + assertThat((String) role.get("name"), is(oneOf(CONFIGURED_RESERVED_ROLES.toArray(new String[0])))); + } + }); + }); + + // Test get roles API + assertBusy(() -> { + final Response response = adminClient().performRequest(new Request("GET", "/_security/role")); + assertOK(response); + final Map responseMap = responseAsMap(response); + assertThat(responseMap.keySet(), equalTo(CONFIGURED_RESERVED_ROLES)); + }); + } + private void assertCannotDeleteReservedRoles() throws Exception { { String roleName = randomFrom(ReservedRolesStore.names()); diff --git a/x-pack/plugin/security/qa/security-basic/src/main/java/module-info.java b/x-pack/plugin/security/qa/security-basic/src/main/java/module-info.java new file mode 100644 index 0000000000000..00c8e480cfbaf --- /dev/null +++ b/x-pack/plugin/security/qa/security-basic/src/main/java/module-info.java @@ -0,0 +1,6 @@ +module org.elasticsearch.internal.security { + requires org.elasticsearch.base; + requires org.elasticsearch.server; + requires org.elasticsearch.xcore; + requires org.elasticsearch.security; +} diff --git a/x-pack/plugin/security/qa/security-basic/src/main/java/org/elasticsearch/xpack/security/role/QueryableBuiltInRolesTestPlugin.java b/x-pack/plugin/security/qa/security-basic/src/main/java/org/elasticsearch/xpack/security/role/QueryableBuiltInRolesTestPlugin.java new file mode 100644 index 0000000000000..ba5538d992cfb --- /dev/null +++ b/x-pack/plugin/security/qa/security-basic/src/main/java/org/elasticsearch/xpack/security/role/QueryableBuiltInRolesTestPlugin.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.security.role; + +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; + +import java.util.List; + +public class QueryableBuiltInRolesTestPlugin extends Plugin { + + @Override + public List> getSettings() { + return List.of(ReservedRolesStore.INCLUDED_RESERVED_ROLES_SETTING); + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java index e019f168cf8c0..ba69dca84b61c 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java @@ -20,11 +20,9 @@ import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; -import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -52,7 +50,7 @@ protected void doExecute(Task task, final GetRolesRequest request, final ActionL } final Set rolesToSearchFor = new HashSet<>(); - final List reservedRoles = new ArrayList<>(); + final Set reservedRoles = new HashSet<>(); if (specificRolesRequested) { for (String role : requestedRoles) { if (ReservedRolesStore.isReserved(role)) { @@ -80,10 +78,10 @@ protected void doExecute(Task task, final GetRolesRequest request, final ActionL } private void getNativeRoles(Set rolesToSearchFor, ActionListener listener) { - getNativeRoles(rolesToSearchFor, new ArrayList<>(), listener); + getNativeRoles(rolesToSearchFor, new HashSet<>(), listener); } - private void getNativeRoles(Set rolesToSearchFor, List foundRoles, ActionListener listener) { + private void getNativeRoles(Set rolesToSearchFor, Set foundRoles, ActionListener listener) { nativeRolesStore.getRoleDescriptors(rolesToSearchFor, ActionListener.wrap((retrievalResult) -> { if (retrievalResult.isSuccess()) { foundRoles.addAll(retrievalResult.getDescriptors()); From 78ea6956fe981eeab7a5bdda6ebd420e153785b2 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 11 Dec 2024 16:28:05 +0100 Subject: [PATCH 37/53] ignore javadoc in test plugin --- x-pack/plugin/security/qa/security-basic/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/security/qa/security-basic/build.gradle b/x-pack/plugin/security/qa/security-basic/build.gradle index ca02fb673f5d7..e6caf943dc023 100644 --- a/x-pack/plugin/security/qa/security-basic/build.gradle +++ b/x-pack/plugin/security/qa/security-basic/build.gradle @@ -26,6 +26,7 @@ tasks.named('javaRestTest') { usesDefaultDistribution() } +tasks.named("javadoc").configure { enabled = false } if (buildParams.inFipsJvm) { // This test cluster is using a BASIC license and FIPS 140 mode is not supported in BASIC From cde5382551cb410eb2b7f1e1cafaef9ad7b64d41 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 12 Dec 2024 16:49:29 +0100 Subject: [PATCH 38/53] test closing and deleting .security index --- .../test/rest/ESRestTestCase.java | 2 +- .../security/QueryableReservedRolesIT.java | 133 ++++++++++++++++-- 2 files changed, 123 insertions(+), 12 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java index 4428afaaeabe5..fa525705a9b39 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java @@ -1138,7 +1138,7 @@ protected static void wipeAllIndices(boolean preserveSecurityIndices) throws IOE } } - private static boolean ignoreSystemIndexAccessWarnings(List warnings) { + protected static boolean ignoreSystemIndexAccessWarnings(List warnings) { for (String warning : warnings) { if (warning.startsWith("this request accesses system indices:")) { SUITE_LOGGER.warn("Ignoring system index access warning during test cleanup: {}", warning); diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java index eff25ab24b65b..d57f68a76b48a 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java @@ -10,12 +10,14 @@ import com.carrotsearch.randomizedtesting.annotations.TestCaseOrdering; import org.elasticsearch.client.Request; +import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.test.AnnotationTestOrdering; import org.elasticsearch.test.AnnotationTestOrdering.Order; import org.elasticsearch.test.cluster.ElasticsearchCluster; @@ -24,23 +26,26 @@ import org.elasticsearch.test.cluster.local.model.User; import org.elasticsearch.test.cluster.util.resource.Resource; import org.elasticsearch.test.rest.ESRestTestCase; +import org.elasticsearch.test.rest.ObjectPath; +import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; +import org.elasticsearch.xpack.core.security.test.TestRestrictedIndices; +import org.elasticsearch.xpack.security.support.QueryableBuiltInRolesSynchronizer; import org.elasticsearch.xpack.security.support.SecurityMigrations; import org.junit.BeforeClass; import org.junit.ClassRule; +import java.io.IOException; +import java.io.InputStream; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import static org.elasticsearch.xpack.core.security.test.TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7; import static org.elasticsearch.xpack.security.QueryRoleIT.assertQuery; import static org.elasticsearch.xpack.security.QueryRoleIT.waitForMigrationCompletion; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.iterableWithSize; -import static org.hamcrest.Matchers.oneOf; +import static org.hamcrest.Matchers.*; @TestCaseOrdering(AnnotationTestOrdering.class) public class QueryableReservedRolesIT extends ESRestTestCase { @@ -84,6 +89,7 @@ protected boolean preserveClusterUponCompletion() { .plugin("queryable-reserved-roles-test") .build(); + private static Set PREVIOUS_RESERVED_ROLES; private static Set CONFIGURED_RESERVED_ROLES; @Override @@ -143,12 +149,7 @@ public void testGetReservedRoles() throws Exception { @Order(20) public void testRestartForConfiguringReservedRoles() throws Exception { - CONFIGURED_RESERVED_ROLES = new HashSet<>(); - CONFIGURED_RESERVED_ROLES.add("superuser"); - CONFIGURED_RESERVED_ROLES.addAll( - randomSubsetOf(List.of("editor", "viewer", "kibana_system", "apm_system", "beats_system", "logstash_system")) - ); - clusterSettings.put("xpack.security.reserved_roles.include", Strings.collectionToCommaDelimitedString(CONFIGURED_RESERVED_ROLES)); + configureReservedRoles(List.of("editor", "viewer", "kibana_system", "apm_system", "beats_system", "logstash_system")); cluster.restart(false); closeClients(); } @@ -178,6 +179,116 @@ public void testConfiguredReservedRoles() throws Exception { }); } + @Order(40) + public void testRestartForConfiguringReservedRolesAndClosingIndex() throws Exception { + configureReservedRoles(List.of("editor", "viewer")); + closeSecurityIndex(); + cluster.restart(false); + closeClients(); + } + + @Order(50) + public void testConfiguredReservedRolesAfterClosingAndOpeningIndex() throws Exception { + assert CONFIGURED_RESERVED_ROLES != null; + assert PREVIOUS_RESERVED_ROLES != null; + assertThat(PREVIOUS_RESERVED_ROLES, is(not(equalTo(CONFIGURED_RESERVED_ROLES)))); + + // Test configured roles did not get updated because the security index is closed + assertMetadataContainsBuiltInRoles(PREVIOUS_RESERVED_ROLES); + + // Open the security index + openSecurityIndex(); + + // Test that the roles are now updated after index got opened + assertBusy(() -> { + assertQuery(client(), """ + { "query": { "bool": { "must": { "term": { "metadata._reserved": true } } } }, "size": 100 } + """, CONFIGURED_RESERVED_ROLES.size(), roles -> { + assertThat(roles, iterableWithSize(CONFIGURED_RESERVED_ROLES.size())); + for (var role : roles) { + assertThat((String) role.get("name"), is(oneOf(CONFIGURED_RESERVED_ROLES.toArray(new String[0])))); + } + }); + }); + + } + + @Order(60) + public void testDeletingAndCreatingSecurityIndexTriggersSynchronization() throws Exception { + deleteSecurityIndex(); + + // Creating a user will trigger .security index creation + createUser("superman", "superman", "superuser"); + + // Test that the roles are now updated after index got created + assertBusy(() -> { + assertQuery(client(), """ + { "query": { "bool": { "must": { "term": { "metadata._reserved": true } } } }, "size": 100 } + """, CONFIGURED_RESERVED_ROLES.size(), roles -> { + assertThat(roles, iterableWithSize(CONFIGURED_RESERVED_ROLES.size())); + for (var role : roles) { + assertThat((String) role.get("name"), is(oneOf(CONFIGURED_RESERVED_ROLES.toArray(new String[0])))); + } + }); + }); + } + + private void createUser(String name, String password, String role) throws IOException { + Request request = new Request("PUT", "/_security/user/" + name); + request.setJsonEntity("{ \"password\": \"" + password + "\", \"roles\": [ \"" + role + "\"] }"); + assertOK(adminClient().performRequest(request)); + } + + private void deleteSecurityIndex() throws IOException { + final Request deleteRequest = new Request("DELETE", INTERNAL_SECURITY_MAIN_INDEX_7); + deleteRequest.setOptions(RequestOptions.DEFAULT.toBuilder().setWarningsHandler(ESRestTestCase::ignoreSystemIndexAccessWarnings)); + final Response response = adminClient().performRequest(deleteRequest); + try (InputStream is = response.getEntity().getContent()) { + assertTrue((boolean) XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true).get("acknowledged")); + } + } + + private void assertMetadataContainsBuiltInRoles(Set builtInRoles) throws IOException { + final Request request = new Request("GET", "_cluster/state/metadata/" + INTERNAL_SECURITY_MAIN_INDEX_7); + final Response response = adminClient().performRequest(request); + assertOK(response); + final Map builtInRolesDigests = ObjectPath.createFromResponse(response) + .evaluate("metadata.indices.\\.security-7." + QueryableBuiltInRolesSynchronizer.METADATA_QUERYABLE_BUILT_IN_ROLES_DIGEST_KEY); + assertThat(builtInRolesDigests.keySet(), equalTo(builtInRoles)); + } + + private void configureReservedRoles(List reservedRoles) throws Exception { + PREVIOUS_RESERVED_ROLES = CONFIGURED_RESERVED_ROLES; + CONFIGURED_RESERVED_ROLES = new HashSet<>(); + CONFIGURED_RESERVED_ROLES.add("superuser"); // superuser must always be included + CONFIGURED_RESERVED_ROLES.addAll(reservedRoles); + clusterSettings.put("xpack.security.reserved_roles.include", Strings.collectionToCommaDelimitedString(CONFIGURED_RESERVED_ROLES)); + } + + private void closeSecurityIndex() throws Exception { + Request request = new Request("POST", "/" + TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7 + "/_close"); + request.setOptions( + expectWarnings( + "this request accesses system indices: [.security-7], but in a future major version, " + + "direct access to system indices will be prevented by default" + ) + ); + Response response = adminClient().performRequest(request); + assertOK(response); + } + + private void openSecurityIndex() throws Exception { + Request request = new Request("POST", "/" + TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7 + "/_open"); + request.setOptions( + expectWarnings( + "this request accesses system indices: [.security-7], but in a future major version, " + + "direct access to system indices will be prevented by default" + ) + ); + Response response = adminClient().performRequest(request); + assertOK(response); + } + private void assertCannotDeleteReservedRoles() throws Exception { { String roleName = randomFrom(ReservedRolesStore.names()); From 1b54b5c10c65cdc12691a8be6398d2591a1caf5a Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 12 Dec 2024 16:59:27 +0100 Subject: [PATCH 39/53] imports --- .../xpack/security/QueryableReservedRolesIT.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java index d57f68a76b48a..dfc89afc03bc0 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java @@ -45,7 +45,12 @@ import static org.elasticsearch.xpack.core.security.test.TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7; import static org.elasticsearch.xpack.security.QueryRoleIT.assertQuery; import static org.elasticsearch.xpack.security.QueryRoleIT.waitForMigrationCompletion; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.iterableWithSize; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.oneOf; @TestCaseOrdering(AnnotationTestOrdering.class) public class QueryableReservedRolesIT extends ESRestTestCase { From f0d48109ce32dd8d71f7c2c593a8f560a830a391 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 12 Dec 2024 17:46:44 +0100 Subject: [PATCH 40/53] wait a bit longer after cluster restart --- .../xpack/security/QueryableReservedRolesIT.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java index dfc89afc03bc0..03727286c83ce 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java @@ -41,6 +41,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.TimeUnit; import static org.elasticsearch.xpack.core.security.test.TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7; import static org.elasticsearch.xpack.security.QueryRoleIT.assertQuery; @@ -173,7 +174,7 @@ public void testConfiguredReservedRoles() throws Exception { assertThat((String) role.get("name"), is(oneOf(CONFIGURED_RESERVED_ROLES.toArray(new String[0])))); } }); - }); + }, 30, TimeUnit.SECONDS); // Test get roles API assertBusy(() -> { @@ -214,7 +215,7 @@ public void testConfiguredReservedRolesAfterClosingAndOpeningIndex() throws Exce assertThat((String) role.get("name"), is(oneOf(CONFIGURED_RESERVED_ROLES.toArray(new String[0])))); } }); - }); + }, 30, TimeUnit.SECONDS); } @@ -235,7 +236,7 @@ public void testDeletingAndCreatingSecurityIndexTriggersSynchronization() throws assertThat((String) role.get("name"), is(oneOf(CONFIGURED_RESERVED_ROLES.toArray(new String[0])))); } }); - }); + }, 30, TimeUnit.SECONDS); } private void createUser(String name, String password, String role) throws IOException { From 1b88a5d40817f65f7312889bbe3bff27d47592c3 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 12 Dec 2024 17:47:14 +0100 Subject: [PATCH 41/53] move static methods to utility class --- .../QueryableBuiltInRolesSynchronizer.java | 46 +++++++++---------- .../support/QueryableBuiltInRolesUtils.java | 40 +++++++++++++++- 2 files changed, 60 insertions(+), 26 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 8d9c72a381c6e..89888cae91253 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -48,7 +48,6 @@ import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; import java.util.Collection; -import java.util.HashSet; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -273,34 +272,23 @@ private boolean shouldSyncBuiltInRoles(ClusterState state) { } private void doSyncBuiltinRoles(Map indexedRolesDigests, QueryableBuiltInRoles roles, ActionListener listener) { - final Set rolesToUpsert = rolesToUpsert(roles, indexedRolesDigests); - final Set rolesToDelete = rolesToDelete(roles, indexedRolesDigests); + final Set rolesToUpsert = QueryableBuiltInRolesUtils.determineRolesToUpsert(roles, indexedRolesDigests); + final Set rolesToDelete = QueryableBuiltInRolesUtils.determineRolesToDelete(roles, indexedRolesDigests); + assert Sets.intersection(rolesToUpsert.stream().map(RoleDescriptor::getName).collect(Collectors.toSet()), rolesToDelete).isEmpty() : "The roles to upsert and delete should not have any common roles"; - indexRoles(rolesToUpsert, ActionListener.wrap(onResponse -> { - if (rolesToDelete.isEmpty()) { - markRolesAsSynced(indexedRolesDigests, roles.rolesDigest(), listener); - } else { - deleteRoles(roles, rolesToDelete, indexedRolesDigests, listener); - } - }, listener::onFailure)); - } - - private static Set rolesToDelete(QueryableBuiltInRoles roles, Map indexedRolesDigests) { - return indexedRolesDigests == null ? Set.of() : Sets.difference(indexedRolesDigests.keySet(), roles.rolesDigest().keySet()); - } - private static Set rolesToUpsert(QueryableBuiltInRoles roles, Map indexedRolesDigests) { - final Set rolesToUpsert = new HashSet<>(); - for (var role : roles.roleDescriptors()) { - final String roleDigest = roles.rolesDigest().get(role.getName()); - if (indexedRolesDigests == null || indexedRolesDigests.containsKey(role.getName()) == false) { - rolesToUpsert.add(role); // a new role to create - } else if (indexedRolesDigests.get(role.getName()).equals(roleDigest) == false) { - rolesToUpsert.add(role); // an existing role that needs to be updated - } + if (rolesToUpsert.isEmpty() && rolesToDelete.isEmpty()) { + logger.debug("no built-in roles to sync to .security index"); + listener.onResponse(null); + return; } - return rolesToUpsert; + + indexRoles(rolesToUpsert, listener.delegateFailureAndWrap((l1, indexResponse) -> { + deleteRoles(roles, rolesToDelete, indexedRolesDigests, l1.delegateFailureAndWrap((l2, deleteResponse) -> { + markRolesAsSynced(indexedRolesDigests, roles.rolesDigest(), l2); + })); + })); } private void deleteRoles( @@ -309,6 +297,10 @@ private void deleteRoles( Map indexedRolesDigests, ActionListener listener ) { + if (rolesToDelete.isEmpty()) { + listener.onResponse(null); + return; + } nativeRolesStore.deleteRoles(rolesToDelete, WriteRequest.RefreshPolicy.IMMEDIATE, false, ActionListener.wrap(deleteResponse -> { final Optional deleteFailure = deleteResponse.getItems() .stream() @@ -324,6 +316,10 @@ private void deleteRoles( } private void indexRoles(Collection rolesToUpsert, ActionListener listener) { + if (rolesToUpsert.isEmpty()) { + listener.onResponse(null); + return; + } nativeRolesStore.putRoles(WriteRequest.RefreshPolicy.IMMEDIATE, rolesToUpsert, false, ActionListener.wrap(response -> { final Optional indexFailure = response.getItems() .stream() diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java index e2d9f32e7fe37..763efc3dd9390 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java @@ -11,6 +11,7 @@ import org.elasticsearch.common.hash.MessageDigests; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.util.Maps; +import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentFactory; @@ -21,12 +22,16 @@ import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.util.Base64; +import java.util.Collections; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import static org.elasticsearch.xcontent.ToXContent.EMPTY_PARAMS; /** - * Utility class which provides helper method for calculating the hash of a role descriptor. + * Utility class which provides helper method for calculating the hash of a role descriptor, + * determining the roles to upsert and the roles to delete. */ public final class QueryableBuiltInRolesUtils { @@ -55,6 +60,39 @@ public static String calculateHash(final RoleDescriptor roleDescriptor) { return Base64.getEncoder().encodeToString(hash.digest()); } + /** + * Determines the roles to delete by comparing the indexed roles with the roles in the built-in roles. + * @return the set of roles to delete + */ + public static Set determineRolesToDelete(final QueryableBuiltInRoles roles, final Map indexedRolesDigests) { + if (indexedRolesDigests == null) { + // nothing indexed, nothing to delete + return Set.of(); + } + final Set rolesToDelete = Sets.difference(indexedRolesDigests.keySet(), roles.rolesDigest().keySet()); + return Collections.unmodifiableSet(rolesToDelete); + } + + /** + * Determines the roles to upsert by comparing the indexed roles and their digests with the current built-in roles. + * @return the set of roles to upsert (create or update) + */ + public static Set determineRolesToUpsert( + final QueryableBuiltInRoles roles, + final Map indexedRolesDigests + ) { + final Set rolesToUpsert = new HashSet<>(); + for (RoleDescriptor role : roles.roleDescriptors()) { + final String roleDigest = roles.rolesDigest().get(role.getName()); + if (indexedRolesDigests == null || indexedRolesDigests.containsKey(role.getName()) == false) { + rolesToUpsert.add(role); // a new role to create + } else if (indexedRolesDigests.get(role.getName()).equals(roleDigest) == false) { + rolesToUpsert.add(role); // an existing role that needs to be updated + } + } + return Collections.unmodifiableSet(rolesToUpsert); + } + private QueryableBuiltInRolesUtils() { throw new IllegalAccessError("not allowed"); } From fec25ae67b21051182e9f7912badf0b9aff7a68a Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 13 Dec 2024 10:11:29 +0100 Subject: [PATCH 42/53] better exception handling and code cleanup --- .../xpack/security/Security.java | 1 - .../QueryableBuiltInRolesSynchronizer.java | 176 ++++++++++++------ 2 files changed, 122 insertions(+), 55 deletions(-) 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 9273d1bf64b6a..fd530a338b26c 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 @@ -1217,7 +1217,6 @@ Collection createComponents( nativeRolesStore, reservedRolesStore, fileRolesStore.get(), - systemIndices.getMainIndexManager(), threadPool ) ); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 89888cae91253..7bce52eb87659 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -9,7 +9,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.action.ActionListener; @@ -50,12 +49,14 @@ import java.util.Collection; import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.Collectors; +import static java.util.stream.Collectors.toMap; +import static java.util.stream.Collectors.toSet; +import static org.elasticsearch.xpack.security.support.QueryableBuiltInRolesUtils.determineRolesToDelete; +import static org.elasticsearch.xpack.security.support.QueryableBuiltInRolesUtils.determineRolesToUpsert; import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MAIN_ALIAS; /** @@ -129,6 +130,17 @@ public void taskSucceeded(MarkRolesAsSyncedTask task, Map value) private volatile boolean securityIndexDeleted = false; + /** + * Constructs a new built-in roles synchronizer. + * + * @param clusterService the cluster service to register as a listener + * @param featureService the feature service to check if the cluster has the queryable built-in roles feature + * @param rolesProviderFactory the factory to create the built-in roles provider + * @param nativeRolesStore the native roles store to sync the built-in roles to + * @param reservedRolesStore the reserved roles store to fetch the built-in roles from + * @param fileRolesStore the file roles store to fetch the built-in roles from + * @param threadPool the thread pool + */ public QueryableBuiltInRolesSynchronizer( ClusterService clusterService, FeatureService featureService, @@ -136,7 +148,6 @@ public QueryableBuiltInRolesSynchronizer( NativeRolesStore nativeRolesStore, ReservedRolesStore reservedRolesStore, FileRolesStore fileRolesStore, - SecurityIndexManager securityIndex, ThreadPool threadPool ) { this.clusterService = clusterService; @@ -167,7 +178,7 @@ private void builtInRolesChanged(QueryableBuiltInRoles roles) { logger.debug("Built-in roles changed, attempting to sync to .security index"); final ClusterState state = clusterService.state(); if (shouldSyncBuiltInRoles(state)) { - syncBuiltInRoles(roles, state); + syncBuiltInRoles(roles); } } @@ -184,11 +195,11 @@ public void clusterChanged(ClusterChangedEvent event) { } if (shouldSyncBuiltInRoles(state)) { final QueryableBuiltInRoles roles = rolesProvider.getRoles(); - syncBuiltInRoles(roles, state); + syncBuiltInRoles(roles); } } - private void syncBuiltInRoles(QueryableBuiltInRoles roles, ClusterState clusterState) { + private void syncBuiltInRoles(final QueryableBuiltInRoles roles) { if (synchronizationInProgress.compareAndSet(false, true)) { final Map indexedRolesDigests = readIndexedBuiltInRolesDigests(clusterService.state()); if (roles.rolesDigest().equals(indexedRolesDigests)) { @@ -199,16 +210,34 @@ private void syncBuiltInRoles(QueryableBuiltInRoles roles, ClusterState clusterS logger.info("Successfully synced [" + roles.roleDescriptors().size() + "] built-in roles to .security index"); synchronizationInProgress.set(false); }, e -> { - if (isExpectedFailure(e)) { - logger.info("Failed to sync built-in roles to .security index", e); - } else { - logger.warn("Failed to sync built-in roles to .security index due to unexpected exception", e); - } + handleException(e); synchronizationInProgress.set(false); }))); } } + private static void handleException(Exception e) { + if (e instanceof BulkRolesResponseException bulkException) { + final boolean isBulkDeleteFailure = bulkException instanceof BulkDeleteRolesResponseException; + for (final Map.Entry bulkFailure : bulkException.getFailures().entrySet()) { + final String logMessage = Strings.format( + "Failed to [%s] built-in role [%s]", + isBulkDeleteFailure ? "delete" : "create/update", + bulkFailure.getKey() + ); + if (isExpectedFailure(bulkFailure.getValue())) { + logger.info(logMessage, bulkFailure.getValue()); + } else { + logger.warn(logMessage, bulkFailure.getKey(), bulkFailure.getValue()); + } + } + } else if (isExpectedFailure(e)) { + logger.info("Failed to sync built-in roles to .security index", e); + } else { + logger.warn("Failed to sync built-in roles to .security index due to unexpected exception", e); + } + } + /** * Some failures are expected and should not be logged as errors. * These exceptions are either: @@ -219,21 +248,20 @@ private void syncBuiltInRoles(QueryableBuiltInRoles roles, ClusterState clusterS * @param e to check * @return {@code true} if the exception is expected and should not be logged as an error */ - private boolean isExpectedFailure(final Exception e) { + private static boolean isExpectedFailure(final Exception e) { final Throwable cause = ExceptionsHelper.unwrapCause(e); - return ExceptionsHelper.isNodeOrShardUnavailableTypeException(e) - || TransportActions.isShardNotAvailableException(e) - || e instanceof IndexClosedException - || e instanceof IndexPrimaryShardNotAllocatedException - || e instanceof NotMasterException + return ExceptionsHelper.isNodeOrShardUnavailableTypeException(cause) + || TransportActions.isShardNotAvailableException(cause) + || cause instanceof IndexClosedException + || cause instanceof IndexPrimaryShardNotAllocatedException + || cause instanceof NotMasterException || cause instanceof ResourceAlreadyExistsException || cause instanceof VersionConflictEngineException || cause instanceof DocumentMissingException - || (cause instanceof ElasticsearchException - && "Failed to mark built-in roles as synced. The expected roles digests have changed.".equals(cause.getMessage())); + || cause instanceof FailedToMarkBuiltInRolesAsSyncedException; } - private boolean shouldSyncBuiltInRoles(ClusterState state) { + private boolean shouldSyncBuiltInRoles(final ClusterState state) { if (false == state.nodes().isLocalNodeElectedMaster()) { logger.trace("Local node is not the master, skipping built-in roles synchronization"); return false; @@ -271,82 +299,79 @@ private boolean shouldSyncBuiltInRoles(ClusterState state) { return true; } - private void doSyncBuiltinRoles(Map indexedRolesDigests, QueryableBuiltInRoles roles, ActionListener listener) { - final Set rolesToUpsert = QueryableBuiltInRolesUtils.determineRolesToUpsert(roles, indexedRolesDigests); - final Set rolesToDelete = QueryableBuiltInRolesUtils.determineRolesToDelete(roles, indexedRolesDigests); + private void doSyncBuiltinRoles( + final Map indexedRolesDigests, + final QueryableBuiltInRoles roles, + final ActionListener listener + ) { + final Set rolesToUpsert = determineRolesToUpsert(roles, indexedRolesDigests); + final Set rolesToDelete = determineRolesToDelete(roles, indexedRolesDigests); - assert Sets.intersection(rolesToUpsert.stream().map(RoleDescriptor::getName).collect(Collectors.toSet()), rolesToDelete).isEmpty() + assert Sets.intersection(rolesToUpsert.stream().map(RoleDescriptor::getName).collect(toSet()), rolesToDelete).isEmpty() : "The roles to upsert and delete should not have any common roles"; if (rolesToUpsert.isEmpty() && rolesToDelete.isEmpty()) { - logger.debug("no built-in roles to sync to .security index"); + logger.debug("No changes to built-in roles to sync to .security index"); listener.onResponse(null); return; } indexRoles(rolesToUpsert, listener.delegateFailureAndWrap((l1, indexResponse) -> { - deleteRoles(roles, rolesToDelete, indexedRolesDigests, l1.delegateFailureAndWrap((l2, deleteResponse) -> { + deleteRoles(rolesToDelete, l1.delegateFailureAndWrap((l2, deleteResponse) -> { markRolesAsSynced(indexedRolesDigests, roles.rolesDigest(), l2); })); })); } - private void deleteRoles( - QueryableBuiltInRoles roles, - Set rolesToDelete, - Map indexedRolesDigests, - ActionListener listener - ) { + private void deleteRoles(final Set rolesToDelete, final ActionListener listener) { if (rolesToDelete.isEmpty()) { listener.onResponse(null); return; } nativeRolesStore.deleteRoles(rolesToDelete, WriteRequest.RefreshPolicy.IMMEDIATE, false, ActionListener.wrap(deleteResponse -> { - final Optional deleteFailure = deleteResponse.getItems() + final Map deleteFailure = deleteResponse.getItems() .stream() .filter(BulkRolesResponse.Item::isFailed) - .map(BulkRolesResponse.Item::getCause) - .findAny(); - if (deleteFailure.isPresent()) { - listener.onFailure(deleteFailure.get()); + .collect(toMap(BulkRolesResponse.Item::getRoleName, BulkRolesResponse.Item::getCause)); + if (deleteFailure.isEmpty()) { + listener.onResponse(null); } else { - markRolesAsSynced(indexedRolesDigests, roles.rolesDigest(), listener); + listener.onFailure(new BulkDeleteRolesResponseException(deleteFailure)); } }, listener::onFailure)); } - private void indexRoles(Collection rolesToUpsert, ActionListener listener) { + private void indexRoles(final Collection rolesToUpsert, final ActionListener listener) { if (rolesToUpsert.isEmpty()) { listener.onResponse(null); return; } nativeRolesStore.putRoles(WriteRequest.RefreshPolicy.IMMEDIATE, rolesToUpsert, false, ActionListener.wrap(response -> { - final Optional indexFailure = response.getItems() + final Map indexFailures = response.getItems() .stream() .filter(BulkRolesResponse.Item::isFailed) - .map(BulkRolesResponse.Item::getCause) - .findAny(); - if (indexFailure.isPresent()) { - listener.onFailure(indexFailure.get()); - } else { + .collect(toMap(BulkRolesResponse.Item::getRoleName, BulkRolesResponse.Item::getCause)); + if (indexFailures.isEmpty()) { listener.onResponse(null); + } else { + listener.onFailure(new BulkIndexRolesResponseException(indexFailures)); } }, listener::onFailure)); } - private boolean isSecurityIndexDeleted(ClusterChangedEvent event) { + private boolean isSecurityIndexDeleted(final ClusterChangedEvent event) { final IndexMetadata previousSecurityIndexMetadata = resolveSecurityIndexMetadata(event.previousState().metadata()); final IndexMetadata currentSecurityIndexMetadata = resolveSecurityIndexMetadata(event.state().metadata()); return previousSecurityIndexMetadata != null && currentSecurityIndexMetadata == null; } - private boolean isSecurityIndexCreatedOrRecovered(ClusterChangedEvent event) { + private boolean isSecurityIndexCreatedOrRecovered(final ClusterChangedEvent event) { final IndexMetadata previousSecurityIndexMetadata = resolveSecurityIndexMetadata(event.previousState().metadata()); final IndexMetadata currentSecurityIndexMetadata = resolveSecurityIndexMetadata(event.state().metadata()); return previousSecurityIndexMetadata == null && currentSecurityIndexMetadata != null; } - private boolean isSecurityIndexClosed(ClusterState state) { + private boolean isSecurityIndexClosed(final ClusterState state) { final IndexMetadata indexMetadata = resolveSecurityIndexMetadata(state.metadata()); return indexMetadata != null && indexMetadata.getState() == IndexMetadata.State.CLOSE; } @@ -361,9 +386,9 @@ private boolean isSecurityIndexClosed(ClusterState state) { * .security index or if they are equal to the new roles digests. */ private void markRolesAsSynced( - Map expectedRolesDigests, - Map newRolesDigests, - ActionListener listener + final Map expectedRolesDigests, + final Map newRolesDigests, + final ActionListener listener ) { final Index concreteSecurityIndex = resolveSecurityIndexMetadata(clusterService.state().metadata()).getIndex(); markRolesAsSyncedTaskQueue.submitTask( @@ -379,7 +404,9 @@ private void markRolesAsSynced( ) ); l.onFailure( - new ElasticsearchException("Failed to mark built-in roles as synced. The expected roles digests have changed.") + new FailedToMarkBuiltInRolesAsSyncedException( + "Failed to mark built-in roles as synced. The expected role digests have changed." + ) ); } else { l.onResponse(null); @@ -389,7 +416,7 @@ private void markRolesAsSynced( ); } - private Map readIndexedBuiltInRolesDigests(ClusterState state) { + private Map readIndexedBuiltInRolesDigests(final ClusterState state) { final IndexMetadata indexMetadata = resolveSecurityIndexMetadata(state.metadata()); if (indexMetadata == null) { return null; @@ -456,4 +483,45 @@ public void onFailure(Exception e) { } } + private static class BulkDeleteRolesResponseException extends BulkRolesResponseException { + + BulkDeleteRolesResponseException(Map failures) { + super("Failed to bulk delete built-in roles", failures); + } + + } + + private static class BulkIndexRolesResponseException extends BulkRolesResponseException { + + BulkIndexRolesResponseException(Map failures) { + super("Failed to bulk create/update built-in roles", failures); + } + + } + + private abstract static class BulkRolesResponseException extends RuntimeException { + + private final Map failures; + + BulkRolesResponseException(String message, Map failures) { + super(message); + assert failures != null && failures.isEmpty() == false; + this.failures = failures; + failures.values().forEach(this::addSuppressed); + } + + Map getFailures() { + return failures; + } + + } + + private static class FailedToMarkBuiltInRolesAsSyncedException extends RuntimeException { + + FailedToMarkBuiltInRolesAsSyncedException(String message) { + super(message); + } + + } + } From 1fa03d7c2c98379bcb1da7171e723b935a67ffc0 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 13 Dec 2024 10:38:12 +0100 Subject: [PATCH 43/53] fix logger usage --- .../security/support/QueryableBuiltInRolesSynchronizer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 7bce52eb87659..1f63404ca785f 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -228,7 +228,7 @@ private static void handleException(Exception e) { if (isExpectedFailure(bulkFailure.getValue())) { logger.info(logMessage, bulkFailure.getValue()); } else { - logger.warn(logMessage, bulkFailure.getKey(), bulkFailure.getValue()); + logger.warn(logMessage, bulkFailure.getValue()); } } } else if (isExpectedFailure(e)) { From 36f1085e49a4c2b068618db7e041be675fa09594 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 13 Dec 2024 12:14:50 +0100 Subject: [PATCH 44/53] unit test rolesToUpsert and rolesToDelete --- .../support/QueryableBuiltInRolesUtils.java | 2 + .../QueryableBuiltInRolesUtilsTests.java | 207 ++++++++++++++++++ 2 files changed, 209 insertions(+) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java index 763efc3dd9390..2d2eb345594ed 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtils.java @@ -65,6 +65,7 @@ public static String calculateHash(final RoleDescriptor roleDescriptor) { * @return the set of roles to delete */ public static Set determineRolesToDelete(final QueryableBuiltInRoles roles, final Map indexedRolesDigests) { + assert roles != null; if (indexedRolesDigests == null) { // nothing indexed, nothing to delete return Set.of(); @@ -81,6 +82,7 @@ public static Set determineRolesToUpsert( final QueryableBuiltInRoles roles, final Map indexedRolesDigests ) { + assert roles != null; final Set rolesToUpsert = new HashSet<>(); for (RoleDescriptor role : roles.roleDescriptors()) { final String roleDigest = roles.rolesDigest().get(role.getName()); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java index a760349bd1dbd..d8f7655e5f030 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java @@ -8,16 +8,223 @@ package org.elasticsearch.xpack.security.support; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +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.store.ReservedRolesStore; +import org.elasticsearch.xpack.core.security.support.MetadataUtils; +import org.junit.BeforeClass; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +import static org.elasticsearch.xpack.security.support.QueryableBuiltInRolesUtils.determineRolesToDelete; +import static org.elasticsearch.xpack.security.support.QueryableBuiltInRolesUtils.determineRolesToUpsert; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; public class QueryableBuiltInRolesUtilsTests extends ESTestCase { + @BeforeClass + public static void setupReservedRolesStore() { + new ReservedRolesStore(); // initialize the store + } + public void testCalculateHash() { assertThat( QueryableBuiltInRolesUtils.calculateHash(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR), equalTo("bWEFdFo4WX229wdhdecfiz5QHMYEssh3ex8hizRgg+Q=") ); } + + public void testEmptyOrNullRolesToUpsertOrDelete() { + // test empty roles and index digests + final QueryableBuiltInRoles emptyRoles = new QueryableBuiltInRoles(Map.of(), Set.of()); + assertThat(determineRolesToDelete(emptyRoles, Map.of()), is(empty())); + assertThat(determineRolesToUpsert(emptyRoles, Map.of()), is(empty())); + + // test empty roles and null indexed digests + assertThat(determineRolesToDelete(emptyRoles, null), is(empty())); + assertThat(determineRolesToUpsert(emptyRoles, null), is(empty())); + } + + public void testNoRolesToUpsertOrDelete() { + QueryableBuiltInRoles currentBuiltInRoles = buildQueryableBuiltInRoles( + Set.of( + ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, + ReservedRolesStore.roleDescriptor("viewer"), + ReservedRolesStore.roleDescriptor("editor") + ) + ); + + // no roles to delete or upsert since the built-in roles are the same as the indexed roles + assertThat(determineRolesToDelete(currentBuiltInRoles, currentBuiltInRoles.rolesDigest()), is(empty())); + assertThat(determineRolesToUpsert(currentBuiltInRoles, currentBuiltInRoles.rolesDigest()), is(empty())); + } + + public void testRolesToDeleteOnly() { + Map indexedDigests = buildDigests( + Set.of( + ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, + ReservedRolesStore.roleDescriptor("viewer"), + ReservedRolesStore.roleDescriptor("editor"), + supermanRole("monitor", "read", "view_index_metadata", "read_cross_cluster") + ) + ); + + QueryableBuiltInRoles currentBuiltInRoles = buildQueryableBuiltInRoles( + Set.of( + ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, + ReservedRolesStore.roleDescriptor("viewer"), + ReservedRolesStore.roleDescriptor("editor") + ) + ); + + // superman is the only role that needs to be deleted since it is not in a current built-in role + assertThat(determineRolesToDelete(currentBuiltInRoles, indexedDigests), containsInAnyOrder("superman")); + assertThat(determineRolesToUpsert(currentBuiltInRoles, indexedDigests), is(empty())); + + // passing empty built-in roles should result in all indexed roles needing to be deleted + QueryableBuiltInRoles emptyBuiltInRoles = new QueryableBuiltInRoles(Map.of(), Set.of()); + assertThat( + determineRolesToDelete(emptyBuiltInRoles, indexedDigests), + containsInAnyOrder("superman", "viewer", "editor", "superuser") + ); + assertThat(determineRolesToUpsert(emptyBuiltInRoles, indexedDigests), is(empty())); + } + + public void testRolesToUpdateOnly() { + Map indexedDigests = buildDigests( + Set.of( + ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, + ReservedRolesStore.roleDescriptor("viewer"), + ReservedRolesStore.roleDescriptor("editor"), + supermanRole("monitor", "read", "write") + ) + ); + + RoleDescriptor updatedSupermanRole = supermanRole("monitor", "read", "view_index_metadata", "read_cross_cluster"); + QueryableBuiltInRoles currentBuiltInRoles = buildQueryableBuiltInRoles( + Set.of( + ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, + ReservedRolesStore.roleDescriptor("viewer"), + ReservedRolesStore.roleDescriptor("editor"), + updatedSupermanRole + ) + ); + + // superman is the only role that needs to be updated since its definition has changed + assertThat(determineRolesToDelete(currentBuiltInRoles, indexedDigests), is(empty())); + assertThat(determineRolesToUpsert(currentBuiltInRoles, indexedDigests), containsInAnyOrder(updatedSupermanRole)); + assertThat(currentBuiltInRoles.rolesDigest().get("superman"), is(not(equalTo(indexedDigests.get("superman"))))); + } + + public void testRolesToCreateOnly() { + Map indexedDigests = buildDigests( + Set.of( + ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, + ReservedRolesStore.roleDescriptor("viewer"), + ReservedRolesStore.roleDescriptor("editor") + ) + ); + + RoleDescriptor newSupermanRole = supermanRole("monitor", "read", "view_index_metadata", "read_cross_cluster"); + QueryableBuiltInRoles currentBuiltInRoles = buildQueryableBuiltInRoles( + Set.of( + ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, + ReservedRolesStore.roleDescriptor("viewer"), + ReservedRolesStore.roleDescriptor("editor"), + newSupermanRole + ) + ); + + // superman is the only role that needs to be created since it is not in the indexed roles + assertThat(determineRolesToDelete(currentBuiltInRoles, indexedDigests), is(empty())); + assertThat(determineRolesToUpsert(currentBuiltInRoles, indexedDigests), containsInAnyOrder(newSupermanRole)); + + // passing empty indexed roles should result in all roles needing to be created + assertThat(determineRolesToDelete(currentBuiltInRoles, Map.of()), is(empty())); + assertThat( + determineRolesToUpsert(currentBuiltInRoles, Map.of()), + containsInAnyOrder(currentBuiltInRoles.roleDescriptors().toArray(new RoleDescriptor[0])) + ); + } + + public void testRolesToUpsertAndDelete() { + Map indexedDigests = buildDigests( + Set.of( + ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, + ReservedRolesStore.roleDescriptor("viewer"), + ReservedRolesStore.roleDescriptor("editor") + ) + ); + + RoleDescriptor newSupermanRole = supermanRole("monitor"); + QueryableBuiltInRoles currentBuiltInRoles = buildQueryableBuiltInRoles( + Set.of(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, newSupermanRole) + ); + + // superman is the only role that needs to be updated since its definition has changed + assertThat(determineRolesToDelete(currentBuiltInRoles, indexedDigests), containsInAnyOrder("viewer", "editor")); + assertThat(determineRolesToUpsert(currentBuiltInRoles, indexedDigests), containsInAnyOrder(newSupermanRole)); + } + + private static RoleDescriptor supermanRole(String... indicesPrivileges) { + return new RoleDescriptor( + "superman", + new String[] { "all" }, + new RoleDescriptor.IndicesPrivileges[] { + RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").allowRestrictedIndices(false).build(), + RoleDescriptor.IndicesPrivileges.builder() + .indices("*") + .privileges(indicesPrivileges) + .allowRestrictedIndices(true) + .build() }, + new RoleDescriptor.ApplicationResourcePrivileges[] { + RoleDescriptor.ApplicationResourcePrivileges.builder().application("*").privileges("*").resources("*").build() }, + null, + new String[] { "*" }, + MetadataUtils.DEFAULT_RESERVED_METADATA, + Collections.emptyMap(), + new RoleDescriptor.RemoteIndicesPrivileges[] { + new RoleDescriptor.RemoteIndicesPrivileges( + RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").allowRestrictedIndices(false).build(), + "*" + ), + new RoleDescriptor.RemoteIndicesPrivileges( + RoleDescriptor.IndicesPrivileges.builder() + .indices("*") + .privileges(indicesPrivileges) + .allowRestrictedIndices(true) + .build(), + "*" + ) }, + new RemoteClusterPermissions().addGroup( + new RemoteClusterPermissionGroup( + RemoteClusterPermissions.getSupportedRemoteClusterPermissions().toArray(new String[0]), + new String[] { "*" } + ) + ), + null, + "Grants full access to cluster management and data indices." + ); + } + + private static QueryableBuiltInRoles buildQueryableBuiltInRoles(Set roles) { + final Map digests = buildDigests(roles); + return new QueryableBuiltInRoles(digests, roles); + } + + private static Map buildDigests(Set roles) { + final Map digests = new HashMap<>(); + for (RoleDescriptor role : roles) { + digests.put(role.getName(), QueryableBuiltInRolesUtils.calculateHash(role)); + } + return digests; + } } From 5aacc30e7ad0383d9e4049f8b76eca5a783db20f Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 13 Dec 2024 16:23:17 +0100 Subject: [PATCH 45/53] wait for the index to be deleted --- .../xpack/security/QueryableReservedRolesIT.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java index 03727286c83ce..7adff21d8df4f 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryableReservedRolesIT.java @@ -51,6 +51,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.iterableWithSize; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.oneOf; @TestCaseOrdering(AnnotationTestOrdering.class) @@ -223,6 +224,8 @@ public void testConfiguredReservedRolesAfterClosingAndOpeningIndex() throws Exce public void testDeletingAndCreatingSecurityIndexTriggersSynchronization() throws Exception { deleteSecurityIndex(); + assertBusy(this::assertSecurityIndexDeleted, 30, TimeUnit.SECONDS); + // Creating a user will trigger .security index creation createUser("superman", "superman", "superuser"); @@ -263,6 +266,15 @@ private void assertMetadataContainsBuiltInRoles(Set builtInRoles) throws assertThat(builtInRolesDigests.keySet(), equalTo(builtInRoles)); } + private void assertSecurityIndexDeleted() throws IOException { + final Request request = new Request("GET", "_cluster/state/metadata/" + INTERNAL_SECURITY_MAIN_INDEX_7); + final Response response = adminClient().performRequest(request); + assertOK(response); + final Map securityIndexMetadata = ObjectPath.createFromResponse(response) + .evaluate("metadata.indices.\\.security-7"); + assertThat(securityIndexMetadata, is(nullValue())); + } + private void configureReservedRoles(List reservedRoles) throws Exception { PREVIOUS_RESERVED_ROLES = CONFIGURED_RESERVED_ROLES; CONFIGURED_RESERVED_ROLES = new HashSet<>(); From ac5c8378c13927f7a1c55695787d1dfbbc3d8f76 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 13 Dec 2024 16:29:21 +0100 Subject: [PATCH 46/53] handle cases when .security index gets deleted in the mean time --- .../support/QueryableBuiltInRolesSynchronizer.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 1f63404ca785f..68d301b4bd5f2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -390,7 +390,12 @@ private void markRolesAsSynced( final Map newRolesDigests, final ActionListener listener ) { - final Index concreteSecurityIndex = resolveSecurityIndexMetadata(clusterService.state().metadata()).getIndex(); + final IndexMetadata securityIndexMetadata = resolveSecurityIndexMetadata(clusterService.state().metadata()); + if (securityIndexMetadata == null) { + listener.onFailure(new IndexNotFoundException(SECURITY_MAIN_ALIAS)); + return; + } + final Index concreteSecurityIndex = securityIndexMetadata.getIndex(); markRolesAsSyncedTaskQueue.submitTask( "mark built-in roles as synced task", new MarkRolesAsSyncedTask(listener.delegateFailureAndWrap((l, response) -> { From 123c890e33ce94fb78a1bd5a21be7e5653f33093 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 16 Dec 2024 11:54:11 +0100 Subject: [PATCH 47/53] switch to LinkedHashSet to keep ordering same as before (when List was used) --- .../security/action/role/TransportGetRolesAction.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java index ba69dca84b61c..cdeac51e1f492 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java @@ -22,7 +22,7 @@ import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Set; import java.util.stream.Collectors; @@ -49,8 +49,8 @@ protected void doExecute(Task task, final GetRolesRequest request, final ActionL return; } - final Set rolesToSearchFor = new HashSet<>(); - final Set reservedRoles = new HashSet<>(); + final Set rolesToSearchFor = new LinkedHashSet<>(); + final Set reservedRoles = new LinkedHashSet<>(); if (specificRolesRequested) { for (String role : requestedRoles) { if (ReservedRolesStore.isReserved(role)) { @@ -78,7 +78,7 @@ protected void doExecute(Task task, final GetRolesRequest request, final ActionL } private void getNativeRoles(Set rolesToSearchFor, ActionListener listener) { - getNativeRoles(rolesToSearchFor, new HashSet<>(), listener); + getNativeRoles(rolesToSearchFor, new LinkedHashSet<>(), listener); } private void getNativeRoles(Set rolesToSearchFor, Set foundRoles, ActionListener listener) { From ee484b3f235efb54414ca4db6361699b767188fb Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 16 Dec 2024 16:28:49 +0100 Subject: [PATCH 48/53] validateRoleDescriptors --- .../xpack/security/authz/store/NativeRolesStore.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 d4ab5b3621007..444218e8402f2 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 @@ -568,7 +568,7 @@ public void putRoles( public void putRoles( final WriteRequest.RefreshPolicy refreshPolicy, final Collection roles, - boolean allowReservedRoleNames, + boolean validateRoleDescriptors, final ActionListener listener ) { if (enabled == false) { @@ -581,7 +581,7 @@ public void putRoles( for (RoleDescriptor role : roles) { Exception validationException; try { - validationException = allowReservedRoleNames ? validateRoleDescriptor(role) : null; + validationException = validateRoleDescriptors ? validateRoleDescriptor(role) : null; } catch (Exception e) { validationException = e; } From 5cd4e81bdf98d0491b1b46cb0f3158cdf47632ac Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 16 Dec 2024 16:33:26 +0100 Subject: [PATCH 49/53] deduplicate log messages --- .../security/support/QueryableBuiltInRolesSynchronizer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 68d301b4bd5f2..0ee8e67ea4055 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -187,7 +187,7 @@ public void clusterChanged(ClusterChangedEvent event) { final ClusterState state = event.state(); if (isSecurityIndexDeleted(event)) { this.securityIndexDeleted = true; - logger.trace("Security index has been deleted, skipping built-in roles synchronization"); + logger.trace("Received security index deletion cluster state event, skipping built-in roles synchronization"); return; } else if (isSecurityIndexCreatedOrRecovered(event)) { this.securityIndexDeleted = false; @@ -289,7 +289,7 @@ private boolean shouldSyncBuiltInRoles(final ClusterState state) { return false; } if (securityIndexDeleted) { - logger.trace("Security index has been deleted, skipping built-in roles synchronization"); + logger.trace("Security index is deleted, skipping built-in roles synchronization"); return false; } if (isSecurityIndexClosed(state)) { From 7d09bbf4555ef766784eeb9d81c237e8e15830e5 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 16 Dec 2024 16:35:07 +0100 Subject: [PATCH 50/53] validateRoleNames --- .../xpack/security/authz/store/NativeRolesStore.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 444218e8402f2..0a5865ecfe9bf 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 @@ -360,7 +360,7 @@ public void deleteRoles( public void deleteRoles( final Collection roleNames, WriteRequest.RefreshPolicy refreshPolicy, - boolean allowReservedRoleNames, + boolean validateRoleNames, final ActionListener listener ) { if (enabled == false) { @@ -372,7 +372,7 @@ public void deleteRoles( Map validationErrorByRoleName = new HashMap<>(); for (String roleName : roleNames) { - if (allowReservedRoleNames && reservedRoleNameChecker.isReserved(roleName)) { + if (validateRoleNames && reservedRoleNameChecker.isReserved(roleName)) { validationErrorByRoleName.put( roleName, new IllegalArgumentException("role [" + roleName + "] is reserved and cannot be deleted") From ca6485ada627ca5c9c33eeffa8783d1c6f901f11 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 16 Dec 2024 16:53:27 +0100 Subject: [PATCH 51/53] test with randomized metadata order --- .../QueryableBuiltInRolesSynchronizer.java | 2 +- .../QueryableBuiltInRolesUtilsTests.java | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java index 0ee8e67ea4055..60163434e212f 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java @@ -187,7 +187,7 @@ public void clusterChanged(ClusterChangedEvent event) { final ClusterState state = event.state(); if (isSecurityIndexDeleted(event)) { this.securityIndexDeleted = true; - logger.trace("Received security index deletion cluster state event, skipping built-in roles synchronization"); + logger.trace("Received security index deletion event, skipping built-in roles synchronization"); return; } else if (isSecurityIndexCreatedOrRecovered(event)) { this.securityIndexDeleted = false; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java index d8f7655e5f030..0e74a28df247b 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java @@ -12,14 +12,15 @@ 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.store.ReservedRolesStore; -import org.elasticsearch.xpack.core.security.support.MetadataUtils; import org.junit.BeforeClass; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; +import static org.elasticsearch.xpack.core.security.support.MetadataUtils.RESERVED_METADATA_KEY; import static org.elasticsearch.xpack.security.support.QueryableBuiltInRolesUtils.determineRolesToDelete; import static org.elasticsearch.xpack.security.support.QueryableBuiltInRolesUtils.determineRolesToUpsert; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -189,7 +190,7 @@ private static RoleDescriptor supermanRole(String... indicesPrivileges) { RoleDescriptor.ApplicationResourcePrivileges.builder().application("*").privileges("*").resources("*").build() }, null, new String[] { "*" }, - MetadataUtils.DEFAULT_RESERVED_METADATA, + randomlyOrderedSupermanMetadata(), Collections.emptyMap(), new RoleDescriptor.RemoteIndicesPrivileges[] { new RoleDescriptor.RemoteIndicesPrivileges( @@ -215,6 +216,20 @@ private static RoleDescriptor supermanRole(String... indicesPrivileges) { ); } + private static Map randomlyOrderedSupermanMetadata() { + final LinkedHashMap metadata = new LinkedHashMap<>(); + if (randomBoolean()) { + metadata.put("foo", "bar"); + metadata.put("baz", "qux"); + metadata.put(RESERVED_METADATA_KEY, true); + } else { + metadata.put(RESERVED_METADATA_KEY, true); + metadata.put("foo", "bar"); + metadata.put("baz", "qux"); + } + return metadata; + } + private static QueryableBuiltInRoles buildQueryableBuiltInRoles(Set roles) { final Map digests = buildDigests(roles); return new QueryableBuiltInRoles(digests, roles); From d9f69f761a1679ffd7ffd74fb5a43471af1c7b77 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 16 Dec 2024 16:59:20 +0100 Subject: [PATCH 52/53] test no updates with different digests instances --- .../QueryableBuiltInRolesUtilsTests.java | 45 ++++++++++++++----- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java index 0e74a28df247b..e2e90047f119d 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java @@ -55,17 +55,42 @@ public void testEmptyOrNullRolesToUpsertOrDelete() { } public void testNoRolesToUpsertOrDelete() { - QueryableBuiltInRoles currentBuiltInRoles = buildQueryableBuiltInRoles( - Set.of( - ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, - ReservedRolesStore.roleDescriptor("viewer"), - ReservedRolesStore.roleDescriptor("editor") - ) - ); + { + QueryableBuiltInRoles currentBuiltInRoles = buildQueryableBuiltInRoles( + Set.of( + ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, + ReservedRolesStore.roleDescriptor("viewer"), + ReservedRolesStore.roleDescriptor("editor") + ) + ); + + // no roles to delete or upsert since the built-in roles are the same as the indexed roles + assertThat(determineRolesToDelete(currentBuiltInRoles, currentBuiltInRoles.rolesDigest()), is(empty())); + assertThat(determineRolesToUpsert(currentBuiltInRoles, currentBuiltInRoles.rolesDigest()), is(empty())); + } + { + QueryableBuiltInRoles currentBuiltInRoles = buildQueryableBuiltInRoles( + Set.of( + ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, + ReservedRolesStore.roleDescriptor("viewer"), + ReservedRolesStore.roleDescriptor("editor"), + supermanRole("monitor", "read") + ) + ); - // no roles to delete or upsert since the built-in roles are the same as the indexed roles - assertThat(determineRolesToDelete(currentBuiltInRoles, currentBuiltInRoles.rolesDigest()), is(empty())); - assertThat(determineRolesToUpsert(currentBuiltInRoles, currentBuiltInRoles.rolesDigest()), is(empty())); + Map digests = buildDigests( + Set.of( + ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, + ReservedRolesStore.roleDescriptor("viewer"), + ReservedRolesStore.roleDescriptor("editor"), + supermanRole("monitor", "read") + ) + ); + + // no roles to delete or upsert since the built-in roles are the same as the indexed roles + assertThat(determineRolesToDelete(currentBuiltInRoles, digests), is(empty())); + assertThat(determineRolesToUpsert(currentBuiltInRoles, digests), is(empty())); + } } public void testRolesToDeleteOnly() { From d6914664289c04c63fe80e3baf911b8f06f92089 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Mon, 16 Dec 2024 17:09:28 +0100 Subject: [PATCH 53/53] test no updates needed with randomized role and its copy --- .../QueryableBuiltInRolesUtilsTests.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java index e2e90047f119d..5b4787f25ae7f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesUtilsTests.java @@ -9,6 +9,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptorTestHelper; 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.store.ReservedRolesStore; @@ -91,6 +92,31 @@ public void testNoRolesToUpsertOrDelete() { assertThat(determineRolesToDelete(currentBuiltInRoles, digests), is(empty())); assertThat(determineRolesToUpsert(currentBuiltInRoles, digests), is(empty())); } + { + final RoleDescriptor randomRole = RoleDescriptorTestHelper.randomRoleDescriptor(); + final QueryableBuiltInRoles currentBuiltInRoles = buildQueryableBuiltInRoles(Set.of(randomRole)); + final Map digests = buildDigests( + Set.of( + new RoleDescriptor( + randomRole.getName(), + randomRole.getClusterPrivileges(), + randomRole.getIndicesPrivileges(), + randomRole.getApplicationPrivileges(), + randomRole.getConditionalClusterPrivileges(), + randomRole.getRunAs(), + randomRole.getMetadata(), + randomRole.getTransientMetadata(), + randomRole.getRemoteIndicesPrivileges(), + randomRole.getRemoteClusterPermissions(), + randomRole.getRestriction(), + randomRole.getDescription() + ) + ) + ); + + assertThat(determineRolesToDelete(currentBuiltInRoles, digests), is(empty())); + assertThat(determineRolesToUpsert(currentBuiltInRoles, digests), is(empty())); + } } public void testRolesToDeleteOnly() {