App permissions with action patterns do not retrieve privileges

docs/changelog/85455.yaml

@@ -0,0 +1,5 @@
+pr: 85455
+summary: App permissions with action patterns do not retrieve privileges
+area: Authorization
+type: enhancement
+issues: []

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

@@ -63,14 +63,8 @@ public ActionRequestValidationException validate() {
                     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 [ '/' , '*' , ':' ]",
-                            validationException
-                        );
-                    }
                     try {
-                        ApplicationPrivilege.validatePrivilegeOrActionName(action);
+                        ApplicationPrivilege.validateActionName(action);
                     } catch (IllegalArgumentException e) {
                         validationException = addValidationError(e.getMessage(), validationException);
                     }

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

@@ -163,17 +163,32 @@ public static void validatePrivilegeName(String name) {
         }
     }
 
-    private static boolean isValidPrivilegeName(String name) {
+    public static boolean isValidPrivilegeName(String name) {
         return VALID_NAME.matcher(name).matches();
     }
 
+    public static void validateActionName(String action) {
+        if (action.indexOf('/') == -1 && action.indexOf('*') == -1 && action.indexOf(':') == -1) {
+            throw new IllegalArgumentException("action [" + action + "] must contain one of [ '/' , '*' , ':' ]");
+        }
+        if (false == isValidPrivilegeOrActionName(action)) {
+            throw new IllegalArgumentException(
+                "Application privilege names and actions must match the pattern "
+                    + VALID_NAME_OR_ACTION.pattern()
+                    + " (found '"
+                    + action
+                    + "')"
+            );
+        }
+    }
+
     /**
      * Validate that the provided name is a valid privilege name or action name, and throws an exception otherwise
      *
      * @throws IllegalArgumentException if the name is not valid
      */
     public static void validatePrivilegeOrActionName(String name) {
-        if (VALID_NAME_OR_ACTION.matcher(name).matches() == false) {
+        if (isValidPrivilegeOrActionName(name) == false) {
             throw new IllegalArgumentException(
                 "Application privilege names and actions must match the pattern "
                     + VALID_NAME_OR_ACTION.pattern()
@@ -184,6 +199,10 @@ public static void validatePrivilegeOrActionName(String name) {
         }
     }
 
+    private static boolean isValidPrivilegeOrActionName(String name) {
+        return VALID_NAME_OR_ACTION.matcher(name).matches();
+    }
+
     /**
      * Finds or creates a collection of application privileges with the provided names.
      * If application is a wildcard, it will be expanded to all matching application names in {@code stored}

x-pack/plugin/security/qa/smoke-test-all-realms/build.gradle

@@ -1,5 +1,3 @@
-import org.elasticsearch.gradle.Version
-
 /*
  * This QA test is intended to smoke test all security realms with minimal dependencies.
  * That is, it makes sure a node that has every realm configured can start, and tests those realms that can be tested without needing external services.
@@ -92,4 +90,5 @@ testClusters.matching { it.name == 'javaRestTest' }.configureEach {
   rolesFile file('src/javaRestTest/resources/roles.yml')
   user username: "admin_user", password: "admin-password"
   user username: "security_test_user", password: "security-test-password", role: "security_test_role"
+  user username: "index_and_app_user", password: "security-test-password", role: "all_index_privileges,all_application_privileges"
 }

x-pack/plugin/security/qa/smoke-test-all-realms/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/FileRealmAuthIT.java

@@ -7,12 +7,17 @@
 
 package org.elasticsearch.xpack.security.authc;
 
+import org.apache.http.client.methods.HttpPost;
+import org.elasticsearch.client.Request;
 import org.elasticsearch.client.RequestOptions;
+import org.elasticsearch.client.WarningsHandler;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
 import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
+import org.elasticsearch.xpack.core.security.test.TestRestrictedIndices;
 
 import java.io.IOException;
+import java.util.List;
 import java.util.Map;
 
 /**
@@ -22,6 +27,7 @@ public class FileRealmAuthIT extends SecurityRealmSmokeTestCase {
 
     // Declared in build.gradle
     private static final String USERNAME = "security_test_user";
+    private static final String ANOTHER_USERNAME = "index_and_app_user";
     private static final SecureString PASSWORD = new SecureString("security-test-password".toCharArray());
     private static final String ROLE_NAME = "security_test_role";
 
@@ -36,4 +42,45 @@ public void testAuthenticationUsingFileRealm() throws IOException {
         assertNoApiKeyInfo(authenticate, Authentication.AuthenticationType.REALM);
     }
 
+    public void testAuthenticationUsingFileRealmAndNoSecurityIndex() throws IOException {
+        Map<String, Object> authenticate = super.authenticate(
+            RequestOptions.DEFAULT.toBuilder()
+                .addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue(ANOTHER_USERNAME, PASSWORD))
+        );
+
+        try {
+            // create user to ensure the .security-7 index exists
+            createUser("dummy", new SecureString("longpassword".toCharArray()), List.of("whatever"));
+            // close the .security-7 to simulate making it unavailable
+            Request closeRequest = new Request(HttpPost.METHOD_NAME, TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7 + "/_close");
+            closeRequest.setOptions(
+                RequestOptions.DEFAULT.toBuilder()
+                    .addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue(ANOTHER_USERNAME, PASSWORD))
+                    .setWarningsHandler(WarningsHandler.PERMISSIVE)
+            );
+            assertOK(client().performRequest(closeRequest));
+            // clear the authentication cache
+            Request clearCachesRequest = new Request(HttpPost.METHOD_NAME, "_security/realm/*/_clear_cache");
+            clearCachesRequest.setOptions(
+                RequestOptions.DEFAULT.toBuilder()
+                    .addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue(ANOTHER_USERNAME, PASSWORD))
+            );
+            assertOK(client().performRequest(clearCachesRequest));
+
+            // file-realm authentication still works when cache is cleared and .security-7 is out
+            assertUsername(authenticate, ANOTHER_USERNAME);
+            assertRealm(authenticate, "file", "file0");
+            assertRoles(authenticate, "all_index_privileges", "all_application_privileges");
+            assertNoApiKeyInfo(authenticate, Authentication.AuthenticationType.REALM);
+        } finally {
+            Request openRequest = new Request(HttpPost.METHOD_NAME, TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7 + "/_open");
+            openRequest.setOptions(
+                RequestOptions.DEFAULT.toBuilder()
+                    .addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue(ANOTHER_USERNAME, PASSWORD))
+                    .setWarningsHandler(WarningsHandler.PERMISSIVE)
+            );
+            assertOK(client().performRequest(openRequest));
+            deleteUser("dummy");
+        }
+    }
 }

x-pack/plugin/security/qa/smoke-test-all-realms/src/javaRestTest/resources/roles.yml

@@ -6,3 +6,17 @@ security_test_role:
   indices:
     - names: [ "index_allowed" ]
       privileges: [ "read", "write", "create_index" ]
+
+all_index_privileges:
+  cluster:
+    - "cluster:admin/xpack/security/realm/cache/clear"
+  indices:
+    - names: [ '*' ]
+      privileges: [ 'all' ]
+      allow_restricted_indices: true
+
+all_application_privileges:
+  applications:
+    - application: "*"
+      privileges: [ "*" ]
+      resources: [ "*" ]

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

@@ -45,6 +45,7 @@
 import org.elasticsearch.xpack.core.security.action.privilege.ClearPrivilegesCacheAction;
 import org.elasticsearch.xpack.core.security.action.privilege.ClearPrivilegesCacheRequest;
 import org.elasticsearch.xpack.core.security.action.privilege.ClearPrivilegesCacheResponse;
+import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
 import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
 import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry;
 import org.elasticsearch.xpack.security.support.LockingAtomicCounter;
@@ -130,6 +131,11 @@ public void getPrivileges(
         Collection<String> names,
         ActionListener<Collection<ApplicationPrivilegeDescriptor>> listener
     ) {
+        if (false == isEmpty(names) && names.stream().noneMatch(ApplicationPrivilege::isValidPrivilegeName)) {
+            logger.debug("no concrete privilege, only action patterns [{}], returning no application privilege descriptors", names);
+            listener.onResponse(Collections.emptySet());
+            return;
+        }
 
         final Set<String> applicationNamesCacheKey = (isEmpty(applications) || applications.contains("*"))
             ? Set.of("*")

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/NativePrivilegeStoreTests.java

@@ -37,6 +37,7 @@
 import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.xcontent.XContentType;
 import org.elasticsearch.xpack.core.security.action.privilege.ClearPrivilegesCacheRequest;
+import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
 import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
 import org.elasticsearch.xpack.core.security.test.TestRestrictedIndices;
 import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry;
@@ -81,6 +82,7 @@
 import static org.hamcrest.Matchers.not;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 
 public class NativePrivilegeStoreTests extends ESTestCase {
@@ -713,6 +715,42 @@ public void testPutPrivileges() throws Exception {
         assertThat(map.get("app2"), contains("all"));
     }
 
+    public void testRetrieveActionNamePatternsInsteadOfPrivileges() throws Exception {
+        // test disabling caching
+        final PlainActionFuture<Collection<ApplicationPrivilegeDescriptor>> future = new PlainActionFuture<>();
+        for (List<String> applications : List.<List<String>>of(
+            List.of("myapp"),
+            List.of("myapp*"),
+            List.of("myapp", "myapp*"),
+            List.of(),
+            List.of("*"),
+            List.of("myapp-2", "*")
+        )) {
+            Collection<String> actions = randomList(1, 4, () -> {
+                String actionName = randomAlphaOfLengthBetween(0, 3) + randomFrom("*", "/", ":") + randomAlphaOfLengthBetween(0, 3)
+                    + randomFrom("*", "/", ":", "");
+                ApplicationPrivilege.validateActionName(actionName);
+                return actionName;
+            });
+            Client mockClient = mock(Client.class);
+            SecurityIndexManager mockSecurityIndexManager = mock(SecurityIndexManager.class);
+            Settings settings = randomFrom(
+                Settings.builder().put("xpack.security.authz.store.privileges.cache.ttl", 0).build(),
+                Settings.EMPTY
+            );
+            NativePrivilegeStore store1 = new NativePrivilegeStore(
+                settings,
+                mockClient,
+                mockSecurityIndexManager,
+                new CacheInvalidatorRegistry()
+            );
+            store1.getPrivileges(applications, actions, future);
+            assertResult(emptyList(), future);
+            verifyNoInteractions(mockClient);
+            verifyNoInteractions(mockSecurityIndexManager);
+        }
+    }
+
     public void testDeletePrivileges() throws Exception {
         final List<String> privilegeNames = Arrays.asList("p1", "p2", "p3");