Require that all app privileges have actions (#32272)

The javadoc and validation for ApplicationPrivileges supports the idea
that a privilege could have no actions. However, in that case every
privilege that had no actions grants every other action-less privilege
within the same action, including the NONE privilege.

This commit makes the following changes:
- It is not possible to PUT a privilege without any actions
- A permission with no actions, never grants another privilege even if
  that privilege is also a zero-action privilege (which, ideally would
  never exist, but can occur through missing privileges or index
  manipulation).

Author: tvernum

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/privilege/PutPrivilegesRequest.java

@@ -50,6 +50,9 @@ public ActionRequestValidationException validate() {
             } catch (IllegalArgumentException e) {
                 validationException = addValidationError(e.getMessage(), validationException);
             }
+            if (privilege.getActions().isEmpty()) {
+                validationException = addValidationError("Application privileges must have at least one action", validationException);
+            }
             for (String action : privilege.getActions()) {
                 if (action.indexOf('/') == -1 && action.indexOf('*') == -1 && action.indexOf(':') == -1) {
                     validationException = addValidationError("action [" + action + "] must contain one of [ '/' , '*' , ':' ]",

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java

@@ -97,6 +97,7 @@ private PermissionEntry(ApplicationPrivilege privilege, Automaton resources) {
 
         private boolean grants(ApplicationPrivilege other, Automaton resource) {
             return this.application.test(other.getApplication())
+                && Operations.isEmpty(privilege.getAutomaton()) == false
                 && Operations.subsetOf(other.getAutomaton(), privilege.getAutomaton())
                 && Operations.subsetOf(resource, this.resources);
         }

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

@@ -20,7 +20,7 @@
 
 /**
  * An application privilege has an application name (e.g. {@code "my-app"}) that identifies an application (that exists
- * outside of elasticsearch), a privilege name (e.g. {@code "admin}) that is meaningful to that application, and zero or
+ * outside of elasticsearch), a privilege name (e.g. {@code "admin}) that is meaningful to that application, and one or
  * more "action patterns" (e.g {@code "admin/user/*", "admin/team/*"}).
  * Action patterns must contain at least one special character from ({@code /}, {@code :}, {@code *}) to distinguish them
  * from privilege names.

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/privilege/PutPrivilegesRequestTests.java

@@ -57,6 +57,10 @@ public void testValidation() {
         assertValidationFailure(request(spaceName), "Application privilege names must match");
         assertValidationFailure(request(numericName), "Application privilege names must match");
 
+        // no actions
+        final ApplicationPrivilegeDescriptor nothing = descriptor("*", "nothing");
+        assertValidationFailure(request(nothing), "Application privileges must have at least one action");
+
         // reserved metadata
         final ApplicationPrivilegeDescriptor reservedMetadata = new ApplicationPrivilegeDescriptor("app", "all",
             Collections.emptySet(), Collections.singletonMap("_notAllowed", true)

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermissionTests.java

@@ -12,6 +12,7 @@
 import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -23,6 +24,7 @@ public class ApplicationPermissionTests extends ESTestCase {
     private List<ApplicationPrivilegeDescriptor> store = new ArrayList<>();
 
     private ApplicationPrivilege app1All = storePrivilege("app1", "all", "*");
+    private ApplicationPrivilege app1Empty = storePrivilege("app1", "empty");
     private ApplicationPrivilege app1Read = storePrivilege("app1", "read", "read/*");
     private ApplicationPrivilege app1Write = storePrivilege("app1", "write", "write/*");
     private ApplicationPrivilege app1Delete = storePrivilege("app1", "delete", "write/delete");
@@ -47,6 +49,15 @@ public void testCheckSimplePermission() {
         assertThat(hasPermission.grants(app1All, "foo"), equalTo(false));
     }
 
+    public void testNonePermission() {
+        final ApplicationPermission hasPermission = buildPermission(ApplicationPrivilege.NONE.apply("app1"), "*");
+        for (ApplicationPrivilege privilege : Arrays.asList(app1All, app1Empty, app1Create, app1Delete, app1Read, app1Write, app2Read)) {
+            assertThat("Privilege " + privilege + " on *", hasPermission.grants(privilege, "*"), equalTo(false));
+            final String resource = randomAlphaOfLengthBetween(1, 6);
+            assertThat("Privilege " + privilege + " on " + resource, hasPermission.grants(privilege, resource), equalTo(false));
+        }
+    }
+
     public void testResourceMatching() {
         final ApplicationPermission hasPermission = buildPermission(app1All, "dashboard/*", "audit/*", "user/12345");
 

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java

@@ -86,6 +86,20 @@ public void testValidationOfPrivilegeName() {
         }
     }
 
+    public void testNonePrivilege() {
+        final ApplicationPrivilege none = ApplicationPrivilege.NONE.apply("super-mega-app");
+        CharacterRunAutomaton run = new CharacterRunAutomaton(none.getAutomaton());
+        for (int i = randomIntBetween(5, 10); i > 0; i--) {
+            final String action;
+            if (randomBoolean()) {
+                action = randomAlphaOfLengthBetween(3, 12);
+            } else {
+                action = randomAlphaOfLengthBetween(3, 6) + randomFrom(":", "/") + randomAlphaOfLengthBetween(3, 8);
+            }
+            assertFalse("NONE should not grant " + action, run.run(action));
+        }
+    }
+
     public void testGetPrivilegeByName() {
         final ApplicationPrivilegeDescriptor descriptor = descriptor("my-app", "read", "data:read/*", "action:login");
         final ApplicationPrivilegeDescriptor myWrite = descriptor("my-app", "write", "data:write/*", "action:login");