From 8459caca2f9b4196cdfe17344ff5345996b471b6 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 30 Sep 2024 20:35:50 -0400 Subject: [PATCH] Fix --- .../RoleMappingFileSettingsIT.java | 3 +++ .../mapper/NativeRoleMappingStore.java | 25 ++++++------------- 2 files changed, 11 insertions(+), 17 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 408b7be46e722..12a09cabb080b 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 @@ -272,6 +272,9 @@ private void assertRoleMappingsSaveOK(CountDownLatch savedClusterState, AtomicLo // cluster-state role mappings are retrievable by the role mapping action since they are treated as reserved role mappings and need // to be surfaced via API for BWC assertGetResponseHasMappings("everyone_kibana", "everyone_fleet"); + // assert filtering by name works + assertGetResponseHasMappings("everyone_kibana"); + assertGetResponseHasMappings("everyone_fleet"); // role mappings (with the same names) cannot be modified via API calls since they are reserved expectThrows( diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java index 78b2c39582085..1ced74840a1c9 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java @@ -324,26 +324,17 @@ public void onFailure(Exception e) { } public void getRoleMappings(Set names, ActionListener> listener) { - innerGetRoleMappings( - names, - listener.delegateFailureAndWrap((l, mappings) -> l.onResponse(reservedRoleMappings.mergeWithReserved(mappings))) - ); - } - - /** - * Retrieves one or more mappings from the index. - * If names is null or {@link Set#isEmpty empty}, then this retrieves all mappings. - * Otherwise it retrieves the specified mappings by name. - */ - private void innerGetRoleMappings(Set names, ActionListener> listener) { + // TODO clean up the redundancy if (enabled == false) { - listener.onResponse(List.of()); + listener.onResponse(reservedRoleMappings.mergeWithReserved(List.of())); } else if (names == null || names.isEmpty()) { - getMappings(listener); + getMappings(listener.safeMap(reservedRoleMappings::mergeWithReserved)); } else { - // TODO make sure order in which we are filtering is as expected... i.e., name filter happens _after_ mergeWithReserved is - // called... listeners upon listeners... - getMappings(listener.safeMap(mappings -> mappings.stream().filter(m -> names.contains(m.getName())).toList())); + getMappings( + listener.safeMap( + mappings -> reservedRoleMappings.mergeWithReserved(mappings).stream().filter(m -> names.contains(m.getName())).toList() + ) + ); } }