Support audit ignore policy by index privileges

Addressing review comments + changing approach:
- use permission check instead of simple "checkIfGrants"
- adding more testing

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java

@@ -57,9 +57,13 @@ public class ManageOwnApiKeyClusterPrivilege implements NamedClusterPrivilege {
 
         @Override
         protected boolean extendedCheck(String action, TransportRequest request, Authentication authentication) {
-            if (request == null || authentication == null) {
+            if(authentication == null) {
+                //No valid credentials found
+                return false;
+            }
+            if (request == null) {
                 throw new IllegalArgumentException(
-                    "manage own cluster permission check only supported in context of request and authentication");
+                    "manage own cluster permission check only supported in context of an API key request");
             }
 
             if (request instanceof CreateApiKeyRequest) {

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/audit/logfile/AuditTrailSettingsUpdateTests.java

@@ -24,6 +24,9 @@ import java.util.HashSet;
 import java.util.List;
 import static org.elasticsearch.test.ESIntegTestCase.Scope.TEST;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+import static org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrailFilterTests.randomNonEmptyListOfFilteredClusterActions;
+import static org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrailFilterTests.randomNonEmptyListOfFilteredIndexActions;
+import static org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrailFilterTests.randomNonEmptyListOfFilteredPrivileges;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.startsWith;
 import static org.hamcrest.Matchers.is;
@@ -97,13 +100,44 @@ public class AuditTrailSettingsUpdateTests extends SecurityIntegTestCase {
                 "xpack.security.audit.logfile.events.ignore_filters.invalid.realms",
                 "xpack.security.audit.logfile.events.ignore_filters.invalid.roles",
                 "xpack.security.audit.logfile.events.ignore_filters.invalid.indices",
-                "xpack.security.audit.logfile.events.ignore_filters.invalid.index_privileges"};
+                "xpack.security.audit.logfile.events.ignore_filters.invalid.index_privileges",
+                "xpack.security.audit.logfile.events.ignore_filters.invalid.cluster_privileges"};
         settingsBuilder.put(randomFrom(allSettingsKeys), invalidLuceneRegex);
         final IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
                 () -> client().admin().cluster().prepareUpdateSettings().setTransientSettings(settingsBuilder.build()).get());
         assertThat(e.getMessage(), containsString("invalid pattern [/invalid]"));
     }
 
+    public void testInvalidPrivilegesFilterSettings() throws Exception {
+        final Settings.Builder settingsBuilder1 = Settings.builder();
+        settingsBuilder1.putList("xpack.security.audit.logfile.events.ignore_filters.invalid.index_privileges", "hkrgbkj");
+
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> updateSettings(settingsBuilder1.build(), randomBoolean()));
+        assertThat(e.getMessage(), containsString("illegal value can't update"));
+
+        final Settings.Builder settingsBuilder2 = Settings.builder();
+        settingsBuilder1.putList("xpack.security.audit.logfile.events.ignore_filters.invalid.index_privileges", "hkrgbkj");
+
+        e = expectThrows(IllegalArgumentException.class,
+            () -> updateSettings(settingsBuilder2.build(), randomBoolean()));
+        assertThat(e.getMessage(), containsString("illegal value can't update"));
+    }
+
+    public void testInvalidBothPrivilegesFilterSettings() throws Exception {
+        final Settings.Builder settingsBuilder1 = Settings.builder();
+        settingsBuilder1.putList("xpack.security.audit.logfile.events.ignore_filters.BothPrivilegesFilter.index_privileges",
+            "read");
+        updateSettings(settingsBuilder1.build(), true);
+
+        final Settings.Builder settingsBuilder2 = Settings.builder();
+        settingsBuilder2.putList("xpack.security.audit.logfile.events.ignore_filters.BothPrivilegesFilter.cluster_privileges", "monitor");
+
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> updateSettings(settingsBuilder2.build(), true));
+        assertThat(e.getMessage(), containsString("illegal value can't update"));
+    }
+
     public void testDynamicHostSettings() {
         final boolean persistent = randomBoolean();
         final Settings.Builder settingsBuilder = Settings.builder();
@@ -225,10 +259,16 @@ public class AuditTrailSettingsUpdateTests extends SecurityIntegTestCase {
                 settingsBuilder.putList("xpack.security.audit.logfile.events.ignore_filters." + policyName + ".indices", filteredIndices);
             }
             if (randomBoolean()) {
-                // filter by privileges
+                // filter by index privileges
-                final List<String> filteredPrivileges = randomNonEmptyListOfFilteredNames();
+                final List<String> filteredIndexActions = randomNonEmptyListOfFilteredIndexActions();
+                final List<String> filteredPrivileges = randomNonEmptyListOfFilteredPrivileges(filteredIndexActions);
                 settingsBuilder.putList("xpack.security.audit.logfile.events.ignore_filters." + policyName + ".index_privileges",
                     filteredPrivileges);
+            } else {
+                final List<String> filteredClusterActions = randomNonEmptyListOfFilteredClusterActions();
+                final List<String> filteredPrivileges = randomNonEmptyListOfFilteredPrivileges(filteredClusterActions);
+                settingsBuilder.putList("xpack.security.audit.logfile.events.ignore_filters." + policyName + ".cluster_privileges",
+                    filteredPrivileges);
             }
         } while (settingsBuilder.build().isEmpty());
 

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java

@@ -96,6 +96,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -300,14 +301,17 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
             this.eventFilterPolicyRegistry.set(policyName, newPolicy);
         }, (policyName, filtersList) -> {
             EventFilterPolicy.parsePredicate(filtersList);
-            EventFilterPolicy.testtest(filtersList);
+            eventFilterPolicyRegistry.get(policyName).orElse(new EventFilterPolicy(policyName, settings)).
-            });
+                validateIndexPrivilege(filtersList); });
         clusterService.getClusterSettings().addAffixUpdateConsumer(FILTER_POLICY_IGNORE_CLUSTER_PRIVILEGES, (policyName, filtersList) -> {
             final Optional<EventFilterPolicy> policy = eventFilterPolicyRegistry.get(policyName);
             final EventFilterPolicy newPolicy = policy.orElse(new EventFilterPolicy(policyName, settings)).
                 changeClusterPrivilegesFilter(filtersList);
             this.eventFilterPolicyRegistry.set(policyName, newPolicy);
-        }, (policyName, filtersList) -> EventFilterPolicy.parsePredicate(filtersList));
+        }, (policyName, filtersList) -> {
+            EventFilterPolicy.parsePredicate(filtersList);
+            eventFilterPolicyRegistry.get(policyName).orElse(new EventFilterPolicy(policyName, settings)).
+                validateClusterPrivilege(filtersList); });
         // this log filter ensures that audit events are not filtered out because of the log level
         final LoggerContext ctx = LoggerContext.getContext(false);
         MarkerFilter auditMarkerFilter = MarkerFilter.createFilter(AUDIT_MARKER.getName(), Result.ACCEPT, Result.NEUTRAL);
@@ -1422,7 +1426,7 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
             this.ignoreIndexPrivilegesPredicate = ignoreIndexPrivilegesPredicate;
             this.ignoreClusterPrivilegesPredicate = ignoreClusterPrivilegesPredicate;
 
-            if (!ignoreIndexPrivilegesPredicate.test("") && !ignoreClusterPrivilegesPredicate.test("")) {
+            if (ignoreIndexPrivilegesPredicate.test("") == false && ignoreClusterPrivilegesPredicate.test("") == false) {
                 final String message = String.format(
                     Locale.ROOT,
                     "Both Index and Cluster privilege ignore filters are set for policy [%s]. " +
@@ -1430,7 +1434,7 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
                 throw new IllegalArgumentException(message);
             }
 
-            if (!ignoreClusterPrivilegesPredicate.test("")) {
+            if (ignoreClusterPrivilegesPredicate.test("") == false) {
                 ClusterPermission.Builder builder = ClusterPermission.builder();
                 String[] clusterPrivileges = ignoreClusterPrivilegesPredicate.toString().split("\\|");
                 for (String privilege : clusterPrivileges) {
@@ -1441,10 +1445,11 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
                 this.clusterPermission = ClusterPermission.NONE;
             }
 
-            if (!ignoreIndexPrivilegesPredicate.test("")) {
+            if (ignoreIndexPrivilegesPredicate.test("") == false) {
                 List<IndicesPermission.Group> groups = new ArrayList<>();
-                groups.add(new IndicesPermission.Group(IndexPrivilege.get(Set.of(ignoreIndexPrivilegesPredicate.toString().split("\\|"))),
+                Set<String> set = new HashSet<>(Arrays.stream(ignoreIndexPrivilegesPredicate.toString().split("\\|"))
-                    FieldPermissions.DEFAULT, null, false, "*"));
+                    .collect(Collectors.toSet()));
+                groups.add(new IndicesPermission.Group(IndexPrivilege.get(set), FieldPermissions.DEFAULT, null, false, "*"));
                 this.indexPermission = groups.isEmpty() ? IndicesPermission.NONE :
                     new IndicesPermission(groups.toArray(new IndicesPermission.Group[groups.size()]));
             } else {
@@ -1477,22 +1482,39 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
                 return new EventFilterPolicy(name, ignorePrincipalsPredicate, ignoreRealmsPredicate,
                     ignoreRolesPredicate, ignoreIndicesPredicate, parsePredicate(filtersList), ignoreClusterPrivilegesPredicate);
             } catch (IllegalArgumentException e) {
-                return new EventFilterPolicy(name, ignorePrincipalsPredicate, ignoreRealmsPredicate,
+                return this;
-                    ignoreRolesPredicate, ignoreIndicesPredicate, ignoreIndexPrivilegesPredicate, ignoreClusterPrivilegesPredicate);
             }
         }
 
         private EventFilterPolicy changeClusterPrivilegesFilter(List<String> filtersList) {
+            try {
                 return new EventFilterPolicy(name, ignorePrincipalsPredicate, ignoreRealmsPredicate, ignoreRolesPredicate,
                     ignoreIndicesPredicate, ignoreIndexPrivilegesPredicate, parsePredicate(filtersList));
+            } catch (IllegalArgumentException e) {
+                return this;
+            }
         }
 
         static Predicate<String> parsePredicate(List<String> l) {
             return Automatons.predicate(emptyStringBuildsEmptyAutomaton(l));
         }
 
-        static void testtest(List<String> l) {
+        private void validateIndexPrivilege(List<String> l) {
-            throw new IllegalStateException("Cannot dynamically update SSL settings for the exporter");
+            try {
+                new EventFilterPolicy(name, ignorePrincipalsPredicate, ignoreRealmsPredicate,
+                    ignoreRolesPredicate, ignoreIndicesPredicate, parsePredicate(l), ignoreClusterPrivilegesPredicate);
+            } catch (IllegalArgumentException e) {
+                throw new IllegalArgumentException("Invalid setting for `index_privileges` audit policy filter found: " + e.getMessage());
+            }
+        }
+
+        private void validateClusterPrivilege(List<String> l) {
+            try {
+                new EventFilterPolicy(name, ignorePrincipalsPredicate, ignoreRealmsPredicate,
+                    ignoreRolesPredicate, ignoreIndicesPredicate, ignoreIndexPrivilegesPredicate, parsePredicate(l));
+            } catch (IllegalArgumentException e) {
+                throw new IllegalArgumentException("Invalid setting for `index_privileges` audit policy filter found: " + e.getMessage());
+            }
         }
 
         /**

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

@@ -155,6 +155,10 @@ public class AuthorizationService {
         getAuthorizationEngine(authentication).getUserPrivileges(authentication, getAuthorizationInfoFromContext(), request, listener);
     }
 
+    public static boolean isIndexAction(String action) {
+        return IndexPrivilege.ACTION_MATCHER.test(action);
+    }
+
     private AuthorizationInfo getAuthorizationInfoFromContext() {
         return Objects.requireNonNull(threadContext.getTransient(AUTHORIZATION_INFO_KEY), "authorization info is missing from context");
     }
@@ -574,10 +578,6 @@ public class AuthorizationService {
         return new IllegalArgumentException(message);
     }
 
-    public static boolean isIndexAction(String action) {
-        return IndexPrivilege.ACTION_MATCHER.test(action);
-    }
-
     private static String getAction(BulkItemRequest item) {
         final DocWriteRequest<?> docWriteRequest = item.request();
         switch (docWriteRequest.opType()) {