From f551cefa856f7829a60ca2fec175f19393015035 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 8 Oct 2024 16:47:10 +0200 Subject: [PATCH 1/3] Revert "Remove role mapping block check (#114223)" This reverts commit ce07060dce69f961c0906079529e91c7dd7d4b48. --- .../xpack/core/security/authz/RoleMappingMetadata.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java index da6ff6ad24c34..8f78fdbccd923 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java @@ -12,6 +12,7 @@ import org.elasticsearch.cluster.AbstractNamedDiffable; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.NamedDiff; +import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.common.collect.Iterators; import org.elasticsearch.common.io.stream.StreamInput; @@ -57,6 +58,7 @@ public final class RoleMappingMetadata extends AbstractNamedDiffable Date: Tue, 8 Oct 2024 16:48:33 +0200 Subject: [PATCH 2/3] Revert "Change read-only suffix for file based role mappings (#114205)" This reverts commit bc8f9dc7f3882a461d4b89d69c7554a4cb3858ac. --- .../RoleMappingFileSettingsIT.java | 18 +++++---------- .../role/TransportDeleteRoleAction.java | 22 ++----------------- .../TransportGetRoleMappingsAction.java | 10 ++------- .../mapper/ClusterStateRoleMapper.java | 2 +- .../role/TransportDeleteRoleActionTests.java | 10 +++------ .../TransportPutRoleMappingActionTests.java | 11 ++-------- 6 files changed, 15 insertions(+), 58 deletions(-) diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java index 19c18bf855b4e..9d75baf10376c 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java @@ -359,9 +359,9 @@ public void testGetRoleMappings() throws Exception { Arrays.stream(response.mappings()).map(ExpressionRoleMapping::getName).toList(), containsInAnyOrder( "everyone_kibana", - "everyone_kibana" + RESERVED_ROLE_MAPPING_SUFFIX, + "everyone_kibana " + RESERVED_ROLE_MAPPING_SUFFIX, "_everyone_kibana", - "everyone_fleet" + RESERVED_ROLE_MAPPING_SUFFIX, + "everyone_fleet " + RESERVED_ROLE_MAPPING_SUFFIX, "zzz_mapping", "123_mapping" ) @@ -378,16 +378,6 @@ public void testGetRoleMappings() throws Exception { // it's possible to delete overlapping native role mapping assertTrue(client().execute(DeleteRoleMappingAction.INSTANCE, deleteRequest("everyone_kibana")).actionGet().isFound()); - // Fetch a specific file based role - request = new GetRoleMappingsRequest(); - request.setNames("everyone_kibana" + RESERVED_ROLE_MAPPING_SUFFIX); - response = client().execute(GetRoleMappingsAction.INSTANCE, request).get(); - assertTrue(response.hasMappings()); - assertThat( - Arrays.stream(response.mappings()).map(ExpressionRoleMapping::getName).toList(), - containsInAnyOrder("everyone_kibana" + RESERVED_ROLE_MAPPING_SUFFIX) - ); - savedClusterState = setupClusterStateListenerForCleanup(internalCluster().getMasterName()); writeJSONFile(internalCluster().getMasterName(), emptyJSON, logger, versionCounter); awaitSuccessful = savedClusterState.v1().await(20, TimeUnit.SECONDS); @@ -566,7 +556,9 @@ private static void assertGetResponseHasMappings(boolean readOnly, String... map assertThat( Arrays.stream(response.mappings()).map(ExpressionRoleMapping::getName).toList(), containsInAnyOrder( - Arrays.stream(mappings).map(mapping -> mapping + (readOnly ? RESERVED_ROLE_MAPPING_SUFFIX : "")).toArray(String[]::new) + Arrays.stream(mappings) + .map(mapping -> mapping + (readOnly ? " " + RESERVED_ROLE_MAPPING_SUFFIX : "")) + .toArray(String[]::new) ) ); } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleAction.java index 569cdc1a79fd9..e8d248233415c 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleAction.java @@ -10,7 +10,6 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.TransportAction; -import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.injection.guice.Inject; import org.elasticsearch.tasks.Task; @@ -18,7 +17,6 @@ import org.elasticsearch.xpack.core.security.action.role.DeleteRoleAction; import org.elasticsearch.xpack.core.security.action.role.DeleteRoleRequest; import org.elasticsearch.xpack.core.security.action.role.DeleteRoleResponse; -import org.elasticsearch.xpack.security.authc.support.mapper.ClusterStateRoleMapper; import org.elasticsearch.xpack.security.authz.ReservedRoleNameChecker; import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; @@ -27,20 +25,16 @@ public class TransportDeleteRoleAction extends TransportAction { - if (clusterStateRoleMapper.hasMapping(request.name())) { - // Allow to delete a mapping with the same name in the native role mapping store as the file_settings namespace, but - // add a warning header to signal to the caller that this could be a problem. - HeaderWarning.addWarning( - "A read only role mapping with the same name [" - + request.name() - + "] has been previously been defined in a configuration file. " - + "The read only role mapping will still be active." - ); - } - return new DeleteRoleResponse(found); - })); + rolesStore.deleteRole(request, listener.safeMap(DeleteRoleResponse::new)); } catch (Exception e) { logger.error((Supplier) () -> "failed to delete role [" + request.name() + "]", e); listener.onFailure(e); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsAction.java index db0ee01af70e4..d5bc923ab6dca 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsAction.java @@ -25,7 +25,6 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; import java.util.stream.Stream; import static org.elasticsearch.xpack.security.authc.support.mapper.ClusterStateRoleMapper.RESERVED_ROLE_MAPPING_SUFFIX; @@ -64,12 +63,7 @@ protected void doExecute(Task task, final GetRoleMappingsRequest request, final roleMappingStore.getRoleMappings(names, ActionListener.wrap(mappings -> { List combinedRoleMappings = Stream.concat( mappings.stream(), - clusterStateRoleMapper.getMappings(names == null ? null : names.stream().map(name -> { - // If a read-only role is fetched by name including suffix, remove suffix - return name.endsWith(RESERVED_ROLE_MAPPING_SUFFIX) - ? name.substring(0, name.length() - RESERVED_ROLE_MAPPING_SUFFIX.length()) - : name; - }).collect(Collectors.toSet())) + clusterStateRoleMapper.getMappings(names) .stream() .map(this::cloneAndMarkAsReadOnly) .sorted(Comparator.comparing(ExpressionRoleMapping::getName)) @@ -81,7 +75,7 @@ protected void doExecute(Task task, final GetRoleMappingsRequest request, final private ExpressionRoleMapping cloneAndMarkAsReadOnly(ExpressionRoleMapping mapping) { // Mark role mappings from cluster state as "read only" by adding a suffix to their name return new ExpressionRoleMapping( - mapping.getName() + RESERVED_ROLE_MAPPING_SUFFIX, + mapping.getName() + " " + RESERVED_ROLE_MAPPING_SUFFIX, mapping.getExpression(), mapping.getRoles(), mapping.getRoleTemplates(), diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/ClusterStateRoleMapper.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/ClusterStateRoleMapper.java index baea5970b4637..df8d46f133ac7 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/ClusterStateRoleMapper.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/ClusterStateRoleMapper.java @@ -47,7 +47,7 @@ public class ClusterStateRoleMapper extends AbstractRoleMapperClearRealmCache im * */ public static final String CLUSTER_STATE_ROLE_MAPPINGS_ENABLED = "xpack.security.authc.cluster_state_role_mappings.enabled"; - public static final String RESERVED_ROLE_MAPPING_SUFFIX = "-read-only-operator-config"; + public static final String RESERVED_ROLE_MAPPING_SUFFIX = "(read only)"; private static final Logger logger = LogManager.getLogger(ClusterStateRoleMapper.class); private final ScriptService scriptService; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleActionTests.java index d647088017dc1..84e4dc402c767 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleActionTests.java @@ -19,7 +19,6 @@ import org.elasticsearch.xpack.core.security.action.role.DeleteRoleResponse; import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; -import org.elasticsearch.xpack.security.authc.support.mapper.ClusterStateRoleMapper; import org.elasticsearch.xpack.security.authz.ReservedRoleNameChecker; import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; import org.junit.BeforeClass; @@ -67,8 +66,7 @@ public void testReservedRole() { mock(ActionFilters.class), rolesStore, transportService, - new ReservedRoleNameChecker.Default(), - mock(ClusterStateRoleMapper.class) + new ReservedRoleNameChecker.Default() ); DeleteRoleRequest request = new DeleteRoleRequest(); @@ -117,8 +115,7 @@ private void testValidRole(String roleName) { mock(ActionFilters.class), rolesStore, transportService, - new ReservedRoleNameChecker.Default(), - mock(ClusterStateRoleMapper.class) + new ReservedRoleNameChecker.Default() ); DeleteRoleRequest request = new DeleteRoleRequest(); @@ -171,8 +168,7 @@ public void testException() { mock(ActionFilters.class), rolesStore, transportService, - new ReservedRoleNameChecker.Default(), - mock(ClusterStateRoleMapper.class) + new ReservedRoleNameChecker.Default() ); DeleteRoleRequest request = new DeleteRoleRequest(); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingActionTests.java index 0bb3e7dd4ac3e..91ac8b46e5a31 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingActionTests.java @@ -29,7 +29,6 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicReference; -import static org.elasticsearch.xpack.security.authc.support.mapper.ClusterStateRoleMapper.RESERVED_ROLE_MAPPING_SUFFIX; import static org.hamcrest.Matchers.aMapWithSize; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; @@ -98,18 +97,12 @@ public void testPutMappingWithInvalidName() { final FieldExpression expression = new FieldExpression("username", Collections.singletonList(new FieldExpression.FieldValue("*"))); IllegalArgumentException illegalArgumentException = expectThrows( IllegalArgumentException.class, - () -> put("anarchy" + RESERVED_ROLE_MAPPING_SUFFIX, expression, "superuser", Collections.singletonMap("dumb", true)) + () -> put("anarchy (read only)", expression, "superuser", Collections.singletonMap("dumb", true)) ); assertThat( illegalArgumentException.getMessage(), - equalTo( - "Invalid mapping name [anarchy" - + RESERVED_ROLE_MAPPING_SUFFIX - + "]. [" - + RESERVED_ROLE_MAPPING_SUFFIX - + "] is not an allowed suffix" - ) + equalTo("Invalid mapping name [anarchy (read only)]. [(read only)] is not an allowed suffix") ); } From 745861d7c688ef45501c2e51c5f21f72cc325004 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 8 Oct 2024 16:48:51 +0200 Subject: [PATCH 3/3] Revert "Fix BWC for file-settings based role mappings (#113900)" This reverts commit 763764c7fac0d5738534e632d7da327711a272d0. --- docs/changelog/113900.yaml | 5 - .../rolemapping/GetRoleMappingsResponse.java | 5 - .../security/authz/RoleMappingMetadata.java | 2 - .../RoleMappingFileSettingsIT.java | 158 +++++------------- .../xpack/security/Security.java | 3 +- .../TransportDeleteRoleMappingAction.java | 18 +- .../TransportGetRoleMappingsAction.java | 35 +--- .../TransportPutRoleMappingAction.java | 28 +--- .../mapper/ClusterStateRoleMapper.java | 31 +--- .../test/SecuritySettingsSource.java | 2 +- .../TransportGetRoleMappingsActionTests.java | 8 +- .../TransportPutRoleMappingActionTests.java | 23 +-- .../mapper/ClusterStateRoleMapperTests.java | 11 +- 13 files changed, 64 insertions(+), 265 deletions(-) delete mode 100644 docs/changelog/113900.yaml diff --git a/docs/changelog/113900.yaml b/docs/changelog/113900.yaml deleted file mode 100644 index 25f833d251784..0000000000000 --- a/docs/changelog/113900.yaml +++ /dev/null @@ -1,5 +0,0 @@ -pr: 113900 -summary: Fix BWC for file-settings based role mappings -area: Authentication -type: bug -issues: [] diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/rolemapping/GetRoleMappingsResponse.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/rolemapping/GetRoleMappingsResponse.java index 4f18411ac3af6..13a751829797f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/rolemapping/GetRoleMappingsResponse.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/rolemapping/GetRoleMappingsResponse.java @@ -11,7 +11,6 @@ import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping; import java.io.IOException; -import java.util.Collection; /** * Response to {@link GetRoleMappingsAction get role-mappings API}. @@ -22,10 +21,6 @@ public class GetRoleMappingsResponse extends ActionResponse { private final ExpressionRoleMapping[] mappings; - public GetRoleMappingsResponse(Collection mappings) { - this(mappings.toArray(new ExpressionRoleMapping[0])); - } - public GetRoleMappingsResponse(ExpressionRoleMapping... mappings) { this.mappings = mappings; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java index 8f78fdbccd923..da6ff6ad24c34 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java @@ -12,7 +12,6 @@ import org.elasticsearch.cluster.AbstractNamedDiffable; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.NamedDiff; -import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.common.collect.Iterators; import org.elasticsearch.common.io.stream.StreamInput; @@ -58,7 +57,6 @@ public final class RoleMappingMetadata extends AbstractNamedDiffable cleanup cluster settings..."); - { - // Deleting non-existent native role mappings returns not found even if they exist in config file - var response = client().execute(DeleteRoleMappingAction.INSTANCE, deleteRequest("everyone_kibana")).get(); - assertFalse(response.isFound()); - } - savedClusterState = setupClusterStateListenerForCleanup(internalCluster().getMasterName()); writeJSONFile(internalCluster().getMasterName(), emptyJSON, logger, versionCounter); @@ -320,85 +307,48 @@ public void testClusterStateRoleMappingsAddedThenDeleted() throws Exception { clusterStateResponse.getState().metadata().persistentSettings().get(INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING.getKey()) ); - // cluster-state role mapping was removed and is not returned in the API anymore + // native role mappings are not affected by the removal of the cluster-state based ones { var request = new GetRoleMappingsRequest(); request.setNames("everyone_kibana", "everyone_fleet"); var response = client().execute(GetRoleMappingsAction.INSTANCE, request).get(); - assertFalse(response.hasMappings()); + assertTrue(response.hasMappings()); + assertThat( + Arrays.stream(response.mappings()).map(ExpressionRoleMapping::getName).toList(), + containsInAnyOrder("everyone_kibana", "everyone_fleet") + ); } - // no role mappings means no roles are resolved + // and roles are resolved based on the native role mappings for (UserRoleMapper userRoleMapper : internalCluster().getInstances(UserRoleMapper.class)) { PlainActionFuture> resolveRolesFuture = new PlainActionFuture<>(); userRoleMapper.resolveRoles( new UserRoleMapper.UserData("anyUsername", null, List.of(), Map.of(), mock(RealmConfig.class)), resolveRolesFuture ); - assertThat(resolveRolesFuture.get(), empty()); + assertThat(resolveRolesFuture.get(), contains("kibana_user_native")); } - } - public void testGetRoleMappings() throws Exception { - ensureGreen(); - - final List nativeMappings = List.of("everyone_kibana", "_everyone_kibana", "zzz_mapping", "123_mapping"); - for (var mapping : nativeMappings) { - client().execute(PutRoleMappingAction.INSTANCE, sampleRestRequest(mapping)).actionGet(); + { + var request = new DeleteRoleMappingRequest(); + request.setName("everyone_kibana"); + var response = client().execute(DeleteRoleMappingAction.INSTANCE, request).get(); + assertTrue(response.isFound()); + request = new DeleteRoleMappingRequest(); + request.setName("everyone_fleet"); + response = client().execute(DeleteRoleMappingAction.INSTANCE, request).get(); + assertTrue(response.isFound()); } - var savedClusterState = setupClusterStateListener(internalCluster().getMasterName(), "everyone_kibana"); - writeJSONFile(internalCluster().getMasterName(), testJSON, logger, versionCounter); - boolean awaitSuccessful = savedClusterState.v1().await(20, TimeUnit.SECONDS); - assertTrue(awaitSuccessful); - - var request = new GetRoleMappingsRequest(); - var response = client().execute(GetRoleMappingsAction.INSTANCE, request).get(); - assertTrue(response.hasMappings()); - assertThat( - Arrays.stream(response.mappings()).map(ExpressionRoleMapping::getName).toList(), - containsInAnyOrder( - "everyone_kibana", - "everyone_kibana " + RESERVED_ROLE_MAPPING_SUFFIX, - "_everyone_kibana", - "everyone_fleet " + RESERVED_ROLE_MAPPING_SUFFIX, - "zzz_mapping", - "123_mapping" - ) - ); - - int readOnlyCount = 0; - // assert that cluster-state role mappings come last - for (ExpressionRoleMapping mapping : response.mappings()) { - readOnlyCount = mapping.getName().endsWith(RESERVED_ROLE_MAPPING_SUFFIX) ? readOnlyCount + 1 : readOnlyCount; + // no roles are resolved now, because both native and cluster-state based stores have been cleared + for (UserRoleMapper userRoleMapper : internalCluster().getInstances(UserRoleMapper.class)) { + PlainActionFuture> resolveRolesFuture = new PlainActionFuture<>(); + userRoleMapper.resolveRoles( + new UserRoleMapper.UserData("anyUsername", null, List.of(), Map.of(), mock(RealmConfig.class)), + resolveRolesFuture + ); + assertThat(resolveRolesFuture.get(), empty()); } - // Two sourced from cluster-state - assertEquals(readOnlyCount, 2); - - // it's possible to delete overlapping native role mapping - assertTrue(client().execute(DeleteRoleMappingAction.INSTANCE, deleteRequest("everyone_kibana")).actionGet().isFound()); - - savedClusterState = setupClusterStateListenerForCleanup(internalCluster().getMasterName()); - writeJSONFile(internalCluster().getMasterName(), emptyJSON, logger, versionCounter); - awaitSuccessful = savedClusterState.v1().await(20, TimeUnit.SECONDS); - assertTrue(awaitSuccessful); - - final ClusterStateResponse clusterStateResponse = clusterAdmin().state( - new ClusterStateRequest(TEST_REQUEST_TIMEOUT).waitForMetadataVersion(savedClusterState.v2().get()) - ).get(); - - assertNull( - clusterStateResponse.getState().metadata().persistentSettings().get(INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING.getKey()) - ); - - // Make sure remaining native mappings can still be fetched - request = new GetRoleMappingsRequest(); - response = client().execute(GetRoleMappingsAction.INSTANCE, request).get(); - assertTrue(response.hasMappings()); - assertThat( - Arrays.stream(response.mappings()).map(ExpressionRoleMapping::getName).toList(), - containsInAnyOrder("_everyone_kibana", "zzz_mapping", "123_mapping") - ); } public static Tuple setupClusterStateListenerForError( @@ -483,8 +433,11 @@ public void testRoleMappingApplyWithSecurityIndexClosed() throws Exception { boolean awaitSuccessful = savedClusterState.v1().await(20, TimeUnit.SECONDS); assertTrue(awaitSuccessful); - // even if index is closed, cluster-state role mappings are still returned - assertGetResponseHasMappings(true, "everyone_kibana", "everyone_fleet"); + // no native role mappings exist + var request = new GetRoleMappingsRequest(); + request.setNames("everyone_kibana", "everyone_fleet"); + var response = client().execute(GetRoleMappingsAction.INSTANCE, request).get(); + assertFalse(response.hasMappings()); // cluster state settings are also applied var clusterStateResponse = clusterAdmin().state( @@ -523,12 +476,6 @@ public void testRoleMappingApplyWithSecurityIndexClosed() throws Exception { } } - private DeleteRoleMappingRequest deleteRequest(String name) { - var request = new DeleteRoleMappingRequest(); - request.setName(name); - return request; - } - private PutRoleMappingRequest sampleRestRequest(String name) throws Exception { var json = """ { @@ -547,19 +494,4 @@ private PutRoleMappingRequest sampleRestRequest(String name) throws Exception { return new PutRoleMappingRequestBuilder(null).source(name, parser).request(); } } - - private static void assertGetResponseHasMappings(boolean readOnly, String... mappings) throws InterruptedException, ExecutionException { - var request = new GetRoleMappingsRequest(); - request.setNames(mappings); - var response = client().execute(GetRoleMappingsAction.INSTANCE, request).get(); - assertTrue(response.hasMappings()); - assertThat( - Arrays.stream(response.mappings()).map(ExpressionRoleMapping::getName).toList(), - containsInAnyOrder( - Arrays.stream(mappings) - .map(mapping -> mapping + (readOnly ? " " + RESERVED_ROLE_MAPPING_SUFFIX : "")) - .toArray(String[]::new) - ) - ); - } } 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 f4d9360d1ed84..79a00fa1293bd 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 @@ -897,8 +897,7 @@ Collection createComponents( reservedRealm ); components.add(nativeUsersStore); - components.add(clusterStateRoleMapper); - components.add(nativeRoleMappingStore); + components.add(new PluginComponentBinding<>(NativeRoleMappingStore.class, nativeRoleMappingStore)); components.add(new PluginComponentBinding<>(UserRoleMapper.class, userRoleMapper)); components.add(reservedRealm); components.add(realms); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportDeleteRoleMappingAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportDeleteRoleMappingAction.java index 467cc1c8a9027..74129facae70a 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportDeleteRoleMappingAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportDeleteRoleMappingAction.java @@ -9,7 +9,6 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; -import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.injection.guice.Inject; import org.elasticsearch.tasks.Task; @@ -17,20 +16,17 @@ import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingAction; import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingRequest; import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingResponse; -import org.elasticsearch.xpack.security.authc.support.mapper.ClusterStateRoleMapper; import org.elasticsearch.xpack.security.authc.support.mapper.NativeRoleMappingStore; public class TransportDeleteRoleMappingAction extends HandledTransportAction { private final NativeRoleMappingStore roleMappingStore; - private final ClusterStateRoleMapper clusterStateRoleMapper; @Inject public TransportDeleteRoleMappingAction( ActionFilters actionFilters, TransportService transportService, - NativeRoleMappingStore roleMappingStore, - ClusterStateRoleMapper clusterStateRoleMapper + NativeRoleMappingStore roleMappingStore ) { super( DeleteRoleMappingAction.NAME, @@ -40,22 +36,10 @@ public TransportDeleteRoleMappingAction( EsExecutors.DIRECT_EXECUTOR_SERVICE ); this.roleMappingStore = roleMappingStore; - this.clusterStateRoleMapper = clusterStateRoleMapper; } @Override protected void doExecute(Task task, DeleteRoleMappingRequest request, ActionListener listener) { - if (clusterStateRoleMapper.hasMapping(request.getName())) { - // Since it's allowed to add a mapping with the same name in the native role mapping store as the file_settings namespace, - // a warning header is added to signal to the caller that this could be a problem. - HeaderWarning.addWarning( - "A read only role mapping with the same name [" - + request.getName() - + "] has been previously been defined in a configuration file. The role mapping [" - + request.getName() - + "] defined in the configuration file is read only, will not be deleted, and will remain active." - ); - } roleMappingStore.deleteRoleMapping(request, listener.safeMap(DeleteRoleMappingResponse::new)); } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsAction.java index d5bc923ab6dca..ac0d3177cca09 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsAction.java @@ -17,29 +17,21 @@ import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsRequest; import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsResponse; import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping; -import org.elasticsearch.xpack.security.authc.support.mapper.ClusterStateRoleMapper; import org.elasticsearch.xpack.security.authc.support.mapper.NativeRoleMappingStore; import java.util.Arrays; -import java.util.Comparator; import java.util.HashSet; -import java.util.List; import java.util.Set; -import java.util.stream.Stream; - -import static org.elasticsearch.xpack.security.authc.support.mapper.ClusterStateRoleMapper.RESERVED_ROLE_MAPPING_SUFFIX; public class TransportGetRoleMappingsAction extends HandledTransportAction { private final NativeRoleMappingStore roleMappingStore; - private final ClusterStateRoleMapper clusterStateRoleMapper; @Inject public TransportGetRoleMappingsAction( ActionFilters actionFilters, TransportService transportService, - NativeRoleMappingStore nativeRoleMappingStore, - ClusterStateRoleMapper clusterStateRoleMapper + NativeRoleMappingStore nativeRoleMappingStore ) { super( GetRoleMappingsAction.NAME, @@ -49,7 +41,6 @@ public TransportGetRoleMappingsAction( EsExecutors.DIRECT_EXECUTOR_SERVICE ); this.roleMappingStore = nativeRoleMappingStore; - this.clusterStateRoleMapper = clusterStateRoleMapper; } @Override @@ -60,27 +51,9 @@ protected void doExecute(Task task, final GetRoleMappingsRequest request, final } else { names = new HashSet<>(Arrays.asList(request.getNames())); } - roleMappingStore.getRoleMappings(names, ActionListener.wrap(mappings -> { - List combinedRoleMappings = Stream.concat( - mappings.stream(), - clusterStateRoleMapper.getMappings(names) - .stream() - .map(this::cloneAndMarkAsReadOnly) - .sorted(Comparator.comparing(ExpressionRoleMapping::getName)) - ).toList(); - listener.onResponse(new GetRoleMappingsResponse(combinedRoleMappings)); + this.roleMappingStore.getRoleMappings(names, ActionListener.wrap(mappings -> { + ExpressionRoleMapping[] array = mappings.toArray(new ExpressionRoleMapping[mappings.size()]); + listener.onResponse(new GetRoleMappingsResponse(array)); }, listener::onFailure)); } - - private ExpressionRoleMapping cloneAndMarkAsReadOnly(ExpressionRoleMapping mapping) { - // Mark role mappings from cluster state as "read only" by adding a suffix to their name - return new ExpressionRoleMapping( - mapping.getName() + " " + RESERVED_ROLE_MAPPING_SUFFIX, - mapping.getExpression(), - mapping.getRoles(), - mapping.getRoleTemplates(), - mapping.getMetadata(), - mapping.isEnabled() - ); - } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingAction.java index 76f520bed517e..82a3b4f000064 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingAction.java @@ -9,7 +9,6 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; -import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.injection.guice.Inject; import org.elasticsearch.tasks.Task; @@ -17,52 +16,27 @@ import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingAction; import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingRequest; import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingResponse; -import org.elasticsearch.xpack.security.authc.support.mapper.ClusterStateRoleMapper; import org.elasticsearch.xpack.security.authc.support.mapper.NativeRoleMappingStore; -import static org.elasticsearch.xpack.security.authc.support.mapper.ClusterStateRoleMapper.RESERVED_ROLE_MAPPING_SUFFIX; - public class TransportPutRoleMappingAction extends HandledTransportAction { private final NativeRoleMappingStore roleMappingStore; - private final ClusterStateRoleMapper clusterStateRoleMapper; @Inject public TransportPutRoleMappingAction( ActionFilters actionFilters, TransportService transportService, - NativeRoleMappingStore roleMappingStore, - ClusterStateRoleMapper clusterStateRoleMapper + NativeRoleMappingStore roleMappingStore ) { super(PutRoleMappingAction.NAME, transportService, actionFilters, PutRoleMappingRequest::new, EsExecutors.DIRECT_EXECUTOR_SERVICE); this.roleMappingStore = roleMappingStore; - this.clusterStateRoleMapper = clusterStateRoleMapper; } @Override protected void doExecute(Task task, final PutRoleMappingRequest request, final ActionListener listener) { - validateMappingName(request.getName()); - if (clusterStateRoleMapper.hasMapping(request.getName())) { - // Allow to define a mapping with the same name in the native role mapping store as the file_settings namespace, but add a - // warning header to signal to the caller that this could be a problem. - HeaderWarning.addWarning( - "A read only role mapping with the same name [" - + request.getName() - + "] has been previously been defined in a configuration file. " - + "Both role mappings will be used to determine role assignments." - ); - } roleMappingStore.putRoleMapping( request, ActionListener.wrap(created -> listener.onResponse(new PutRoleMappingResponse(created)), listener::onFailure) ); } - - private static void validateMappingName(String mappingName) { - if (mappingName.endsWith(RESERVED_ROLE_MAPPING_SUFFIX)) { - throw new IllegalArgumentException( - "Invalid mapping name [" + mappingName + "]. [" + RESERVED_ROLE_MAPPING_SUFFIX + "] is not an allowed suffix" - ); - } - } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/ClusterStateRoleMapper.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/ClusterStateRoleMapper.java index df8d46f133ac7..9a6e9e75c4685 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/ClusterStateRoleMapper.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/ClusterStateRoleMapper.java @@ -14,16 +14,13 @@ import org.elasticsearch.cluster.ClusterStateListener; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.core.Nullable; import org.elasticsearch.script.ScriptService; import org.elasticsearch.xpack.core.security.authc.support.UserRoleMapper; import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping; import org.elasticsearch.xpack.core.security.authz.RoleMappingMetadata; -import java.util.Arrays; import java.util.Objects; import java.util.Set; -import java.util.stream.Collectors; import static org.elasticsearch.xpack.core.security.SecurityExtension.SecurityComponents; @@ -31,7 +28,8 @@ * A role mapper the reads the role mapping rules (i.e. {@link ExpressionRoleMapping}s) from the cluster state * (i.e. {@link RoleMappingMetadata}). This is not enabled by default. */ -public class ClusterStateRoleMapper extends AbstractRoleMapperClearRealmCache implements ClusterStateListener { +public final class ClusterStateRoleMapper extends AbstractRoleMapperClearRealmCache implements ClusterStateListener { + /** * This setting is never registered by the xpack security plugin - in order to enable the * cluster-state based role mapper another plugin must register it as a boolean setting @@ -47,7 +45,6 @@ public class ClusterStateRoleMapper extends AbstractRoleMapperClearRealmCache im * */ public static final String CLUSTER_STATE_ROLE_MAPPINGS_ENABLED = "xpack.security.authc.cluster_state_role_mappings.enabled"; - public static final String RESERVED_ROLE_MAPPING_SUFFIX = "(read only)"; private static final Logger logger = LogManager.getLogger(ClusterStateRoleMapper.class); private final ScriptService scriptService; @@ -57,8 +54,8 @@ public class ClusterStateRoleMapper extends AbstractRoleMapperClearRealmCache im public ClusterStateRoleMapper(Settings settings, ScriptService scriptService, ClusterService clusterService) { this.scriptService = scriptService; this.clusterService = clusterService; - // this role mapper is enabled by default and only code in other plugins can disable it - this.enabled = settings.getAsBoolean(CLUSTER_STATE_ROLE_MAPPINGS_ENABLED, true); + // this role mapper is disabled by default and only code in other plugins can enable it + this.enabled = settings.getAsBoolean(CLUSTER_STATE_ROLE_MAPPINGS_ENABLED, false); if (this.enabled) { clusterService.addListener(this); } @@ -84,30 +81,12 @@ public void clusterChanged(ClusterChangedEvent event) { } } - public boolean hasMapping(String name) { - return getMappings().stream().map(ExpressionRoleMapping::getName).anyMatch(name::equals); - } - - public Set getMappings(@Nullable Set names) { - if (enabled == false) { - return Set.of(); - } - final Set mappings = getMappings(); - if (names == null || names.isEmpty()) { - return mappings; - } - return mappings.stream().filter(it -> names.contains(it.getName())).collect(Collectors.toSet()); - } - private Set getMappings() { if (enabled == false) { return Set.of(); } else { final Set mappings = RoleMappingMetadata.getFromClusterState(clusterService.state()).getRoleMappings(); - logger.trace( - "Retrieved mapping(s) {} from cluster state", - Arrays.toString(mappings.stream().map(ExpressionRoleMapping::getName).toArray(String[]::new)) - ); + logger.trace("Retrieved [{}] mapping(s) from cluster state", mappings.size()); return mappings; } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java index ce5aaacdb92b9..6d7817db8ec05 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java @@ -403,7 +403,7 @@ public static class UnregisteredSecuritySettingsPlugin extends Plugin { ); public static final Setting CLUSTER_STATE_ROLE_MAPPINGS_ENABLED = Setting.boolSetting( "xpack.security.authc.cluster_state_role_mappings.enabled", - true, + false, Setting.Property.NodeScope ); public static final Setting NATIVE_ROLES_ENABLED = Setting.boolSetting( diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsActionTests.java index 799e0c334172c..6e8698f095d32 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsActionTests.java @@ -19,7 +19,6 @@ import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsRequest; import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsResponse; import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping; -import org.elasticsearch.xpack.security.authc.support.mapper.ClusterStateRoleMapper; import org.elasticsearch.xpack.security.authc.support.mapper.NativeRoleMappingStore; import org.hamcrest.Matchers; import org.junit.Before; @@ -35,16 +34,13 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.notNullValue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anySet; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class TransportGetRoleMappingsActionTests extends ESTestCase { private NativeRoleMappingStore store; - private ClusterStateRoleMapper clusterStateRoleMapper; private TransportGetRoleMappingsAction action; private AtomicReference> namesRef; private List result; @@ -53,8 +49,6 @@ public class TransportGetRoleMappingsActionTests extends ESTestCase { @Before public void setupMocks() { store = mock(NativeRoleMappingStore.class); - clusterStateRoleMapper = mock(ClusterStateRoleMapper.class); - when(clusterStateRoleMapper.getMappings(anySet())).thenReturn(Set.of()); TransportService transportService = new TransportService( Settings.EMPTY, mock(Transport.class), @@ -64,7 +58,7 @@ public void setupMocks() { null, Collections.emptySet() ); - action = new TransportGetRoleMappingsAction(mock(ActionFilters.class), transportService, store, clusterStateRoleMapper); + action = new TransportGetRoleMappingsAction(mock(ActionFilters.class), transportService, store); namesRef = new AtomicReference<>(null); result = Collections.emptyList(); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingActionTests.java index 91ac8b46e5a31..6f789a10a3a6c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingActionTests.java @@ -19,14 +19,12 @@ import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingResponse; import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping; import org.elasticsearch.xpack.core.security.authc.support.mapper.expressiondsl.FieldExpression; -import org.elasticsearch.xpack.security.authc.support.mapper.ClusterStateRoleMapper; import org.elasticsearch.xpack.security.authc.support.mapper.NativeRoleMappingStore; import org.junit.Before; import java.util.Arrays; import java.util.Collections; import java.util.Map; -import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import static org.hamcrest.Matchers.aMapWithSize; @@ -35,15 +33,12 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.iterableWithSize; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anySet; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class TransportPutRoleMappingActionTests extends ESTestCase { private NativeRoleMappingStore store; - private ClusterStateRoleMapper clusterStateRoleMapper; private TransportPutRoleMappingAction action; private AtomicReference requestRef; @@ -51,9 +46,6 @@ public class TransportPutRoleMappingActionTests extends ESTestCase { @Before public void setupMocks() { store = mock(NativeRoleMappingStore.class); - clusterStateRoleMapper = mock(ClusterStateRoleMapper.class); - when(clusterStateRoleMapper.getMappings(anySet())).thenReturn(Set.of()); - when(clusterStateRoleMapper.hasMapping(any())).thenReturn(false); TransportService transportService = new TransportService( Settings.EMPTY, mock(Transport.class), @@ -63,7 +55,7 @@ public void setupMocks() { null, Collections.emptySet() ); - action = new TransportPutRoleMappingAction(mock(ActionFilters.class), transportService, store, clusterStateRoleMapper); + action = new TransportPutRoleMappingAction(mock(ActionFilters.class), transportService, store); requestRef = new AtomicReference<>(null); @@ -93,19 +85,6 @@ public void testPutValidMapping() throws Exception { assertThat(mapping.getMetadata().get("dumb"), equalTo(true)); } - public void testPutMappingWithInvalidName() { - final FieldExpression expression = new FieldExpression("username", Collections.singletonList(new FieldExpression.FieldValue("*"))); - IllegalArgumentException illegalArgumentException = expectThrows( - IllegalArgumentException.class, - () -> put("anarchy (read only)", expression, "superuser", Collections.singletonMap("dumb", true)) - ); - - assertThat( - illegalArgumentException.getMessage(), - equalTo("Invalid mapping name [anarchy (read only)]. [(read only)] is not an allowed suffix") - ); - } - private PutRoleMappingResponse put(String name, FieldExpression expression, String role, Map metadata) throws Exception { final PutRoleMappingRequest request = new PutRoleMappingRequest(); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/ClusterStateRoleMapperTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/ClusterStateRoleMapperTests.java index 063245e004476..74221c1133271 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/ClusterStateRoleMapperTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/ClusterStateRoleMapperTests.java @@ -56,12 +56,12 @@ public void setup() { () -> 1L ); clusterService = mock(ClusterService.class); - disabledSettings = Settings.builder().put("xpack.security.authc.cluster_state_role_mappings.enabled", false).build(); + enabledSettings = Settings.builder().put("xpack.security.authc.cluster_state_role_mappings.enabled", true).build(); if (randomBoolean()) { - enabledSettings = Settings.builder().put("xpack.security.authc.cluster_state_role_mappings.enabled", true).build(); + disabledSettings = Settings.builder().put("xpack.security.authc.cluster_state_role_mappings.enabled", false).build(); } else { - // the cluster state role mapper is enabled by default - enabledSettings = Settings.EMPTY; + // the cluster state role mapper is disabled by default + disabledSettings = Settings.EMPTY; } } @@ -95,9 +95,6 @@ public void testRoleResolving() throws Exception { verify(mapping1).isEnabled(); verify(mapping2).isEnabled(); verify(mapping3).isEnabled(); - verify(mapping1).getName(); - verify(mapping2).getName(); - verify(mapping3).getName(); verify(mapping2).getExpression(); verify(mapping3).getExpression(); verify(mapping3).getRoleNames(same(scriptService), same(expressionModel));