Fix

n1v0lg · Oct 1, 2024 · 8459cac · 8459cac
1 parent fbcad41
commit 8459cac
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 17 deletions.
diff --git a/...src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java b/...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/...in/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java b/...in/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java
@@ -324,26 +324,17 @@ public void onFailure(Exception e) {
     }
 
     public void getRoleMappings(Set<String> names, ActionListener<List<ExpressionRoleMapping>> listener) {
-        innerGetRoleMappings(
-            names,
-            listener.delegateFailureAndWrap((l, mappings) -> l.onResponse(reservedRoleMappings.mergeWithReserved(mappings)))
-        );
-    }
-
-    /**
-     * Retrieves one or more mappings from the index.
-     * If <code>names</code> is <code>null</code> or {@link Set#isEmpty empty}, then this retrieves all mappings.
-     * Otherwise it retrieves the specified mappings by name.
-     */
-    private void innerGetRoleMappings(Set<String> names, ActionListener<List<ExpressionRoleMapping>> 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()
+                )
+            );
         }
     }