BACKPORT 7x Add new audit handler method for action responses (#63708)

x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlUpgradeModeActionFilterTests.java

@@ -97,7 +97,7 @@ public void testApply_ActionAllowedInUpgradeMode() {
 
     public void testOrder_UpgradeFilterIsExecutedAfterSecurityFilter() {
         MlUpgradeModeActionFilter upgradeModeFilter = new MlUpgradeModeActionFilter(clusterService);
-        SecurityActionFilter securityFilter = new SecurityActionFilter(null, null, null, mock(ThreadPool.class), null, null);
+        SecurityActionFilter securityFilter = new SecurityActionFilter(null, null, null, null, mock(ThreadPool.class), null, null);
 
         ActionFilter[] actionFiltersInOrderOfExecution = new ActionFilters(Sets.newHashSet(upgradeModeFilter, securityFilter)).filters();
         assertThat(actionFiltersInOrderOfExecution, is(arrayContaining(securityFilter, upgradeModeFilter)));

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

@@ -560,7 +560,7 @@ auditTrailService, failureHandler, threadPool, anonymousUser, getAuthorizationEn
         securityInterceptor.set(new SecurityServerTransportInterceptor(settings, threadPool, authcService.get(),
                 authzService, getLicenseState(), getSslService(), securityContext.get(), destructiveOperations, clusterService));
 
-        securityActionFilter.set(new SecurityActionFilter(authcService.get(), authzService, getLicenseState(),
+        securityActionFilter.set(new SecurityActionFilter(authcService.get(), authzService, auditTrailService, getLicenseState(),
             threadPool, securityContext.get(), destructiveOperations));
 
         return components;

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java

@@ -20,6 +20,7 @@
 import org.elasticsearch.action.support.ActionFilterChain;
 import org.elasticsearch.action.support.ContextPreservingActionListener;
 import org.elasticsearch.action.support.DestructiveOperations;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.license.LicenseUtils;
 import org.elasticsearch.license.XPackLicenseState;
@@ -33,11 +34,12 @@
 import org.elasticsearch.xpack.core.security.support.Automatons;
 import org.elasticsearch.xpack.core.security.user.SystemUser;
 import org.elasticsearch.xpack.security.action.SecurityActionMapper;
+import org.elasticsearch.xpack.security.audit.AuditTrailService;
+import org.elasticsearch.xpack.security.audit.AuditUtil;
 import org.elasticsearch.xpack.security.authc.AuthenticationService;
 import org.elasticsearch.xpack.security.authz.AuthorizationService;
 import org.elasticsearch.xpack.security.authz.AuthorizationUtils;
 
-import java.io.IOException;
 import java.util.function.Predicate;
 
 public class SecurityActionFilter implements ActionFilter {
@@ -48,17 +50,19 @@ public class SecurityActionFilter implements ActionFilter {
 
     private final AuthenticationService authcService;
     private final AuthorizationService authzService;
+    private final AuditTrailService auditTrailService;
     private final SecurityActionMapper actionMapper = new SecurityActionMapper();
     private final XPackLicenseState licenseState;
     private final ThreadContext threadContext;
     private final SecurityContext securityContext;
     private final DestructiveOperations destructiveOperations;
 
     public SecurityActionFilter(AuthenticationService authcService, AuthorizationService authzService,
-                                XPackLicenseState licenseState, ThreadPool threadPool,
+                                AuditTrailService auditTrailService, XPackLicenseState licenseState, ThreadPool threadPool,
                                 SecurityContext securityContext, DestructiveOperations destructiveOperations) {
         this.authcService = authcService;
         this.authzService = authzService;
+        this.auditTrailService = auditTrailService;
         this.licenseState = licenseState;
         this.threadContext = threadPool.getThreadContext();
         this.securityContext = securityContext;
@@ -83,29 +87,19 @@ public <Request extends ActionRequest, Response extends ActionResponse> void app
         if (licenseState.isSecurityEnabled()) {
             final ActionListener<Response> contextPreservingListener =
                     ContextPreservingActionListener.wrapPreservingContext(listener, threadContext);
-            ActionListener<Void> authenticatedListener = ActionListener.wrap(
-                    (aVoid) -> chain.proceed(task, action, request, contextPreservingListener), contextPreservingListener::onFailure);
             final boolean useSystemUser = AuthorizationUtils.shouldReplaceUserWithSystem(threadContext, action);
             try {
                 if (useSystemUser) {
                     securityContext.executeAsUser(SystemUser.INSTANCE, (original) -> {
-                        try {
-                            applyInternal(action, request, authenticatedListener);
-                        } catch (IOException e) {
-                            listener.onFailure(e);
-                        }
+                        applyInternal(task, chain, action, request, contextPreservingListener);
                     }, Version.CURRENT);
                 } else if (AuthorizationUtils.shouldSetUserBasedOnActionOrigin(threadContext)) {
                     AuthorizationUtils.switchUserBasedOnActionOriginAndExecute(threadContext, securityContext, (original) -> {
-                        try {
-                            applyInternal(action, request, authenticatedListener);
-                        } catch (IOException e) {
-                            listener.onFailure(e);
-                        }
+                        applyInternal(task, chain, action, request, contextPreservingListener);
                     });
                 } else {
                     try (ThreadContext.StoredContext ignore = threadContext.newStoredContext(true)) {
-                        applyInternal(action, request, authenticatedListener);
+                        applyInternal(task, chain, action, request, contextPreservingListener);
                     }
                 }
             } catch (Exception e) {
@@ -130,13 +124,13 @@ public int order() {
         return Integer.MIN_VALUE;
     }
 
-    private <Request extends ActionRequest> void applyInternal(String action, Request request,
-                                                               ActionListener<Void> listener) throws IOException {
+    private <Request extends ActionRequest, Response extends ActionResponse> void applyInternal(Task task,
+            ActionFilterChain<Request, Response> chain, String action, Request request, ActionListener<Response> listener) {
         if (CloseIndexAction.NAME.equals(action) || OpenIndexAction.NAME.equals(action) || DeleteIndexAction.NAME.equals(action)) {
             IndicesRequest indicesRequest = (IndicesRequest) request;
             try {
                 destructiveOperations.failDestructive(indicesRequest.indices());
-            } catch(IllegalArgumentException e) {
+            } catch (IllegalArgumentException e) {
                 listener.onFailure(e);
                 return;
             }
@@ -156,7 +150,17 @@ it to the action without an associated user (not via REST or transport - this is
         authcService.authenticate(securityAction, request, SystemUser.INSTANCE,
                 ActionListener.wrap((authc) -> {
                     if (authc != null) {
-                        authorizeRequest(authc, securityAction, request, listener);
+                        final String requestId = AuditUtil.extractRequestId(threadContext);
+                        assert Strings.hasText(requestId);
+                        authorizeRequest(authc, securityAction, request, ActionListener.delegateFailure(listener,
+                                (ignore, aVoid) -> {
+                                    chain.proceed(task, action, request, ActionListener.delegateFailure(listener,
+                                            (ignore2, response) -> {
+                                                auditTrailService.get().coordinatingActionResponse(requestId, authc, action, request,
+                                                        response);
+                                                listener.onResponse(response);
+                                            }));
+                                }));
                     } else if (licenseState.isSecurityEnabled() == false) {
                         listener.onResponse(null);
                     } else {
@@ -166,12 +170,11 @@ it to the action without an associated user (not via REST or transport - this is
     }
 
     private <Request extends ActionRequest> void authorizeRequest(Authentication authentication, String securityAction, Request request,
-                                                          ActionListener<Void> listener) {
+                                                                  ActionListener<Void> listener) {
         if (authentication == null) {
             listener.onFailure(new IllegalArgumentException("authentication must be non null for authorization"));
         } else {
-            authzService.authorize(authentication, securityAction, request, ActionListener.wrap(ignore -> listener.onResponse(null),
-                listener::onFailure));
+            authzService.authorize(authentication, securityAction, request, listener);
         }
     }
 }

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

@@ -8,6 +8,7 @@
 import org.elasticsearch.common.transport.TransportAddress;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.transport.TransportRequest;
+import org.elasticsearch.transport.TransportResponse;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationToken;
 import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.AuthorizationInfo;
@@ -81,4 +82,9 @@ void runAsDenied(String requestId, Authentication authentication, RestRequest re
     void explicitIndexAccessEvent(String requestId, AuditLevel eventType, Authentication authentication, String action, String indices,
                                   String requestName, TransportAddress remoteAddress, AuthorizationInfo authorizationInfo);
 
+    // this is the only audit method that is called *after* the action executed, when the response is available
+    // it is however *only called for coordinating actions*, which are the actions that a client invokes as opposed to
+    // the actions that a node invokes in order to service a client request
+    void coordinatingActionResponse(String requestId, Authentication authentication, String action, TransportRequest transportRequest,
+                                    TransportResponse transportResponse);
 }

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

@@ -12,6 +12,7 @@
 import org.elasticsearch.license.XPackLicenseState.Feature;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.transport.TransportRequest;
+import org.elasticsearch.transport.TransportResponse;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationToken;
 import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.AuthorizationInfo;
@@ -147,6 +148,11 @@ public void runAsDenied(String requestId, Authentication authentication, RestReq
         public void explicitIndexAccessEvent(String requestId, AuditLevel eventType, Authentication authentication,
                                              String action, String indices, String requestName, TransportAddress remoteAddress,
                                              AuthorizationInfo authorizationInfo) {}
+
+        @Override
+        public void coordinatingActionResponse(String requestId, Authentication authentication, String action,
+                                               TransportRequest transportRequest,
+                                               TransportResponse transportResponse) { }
     }
 
     private static class CompositeAuditTrail implements AuditTrail {
@@ -254,6 +260,15 @@ public void accessDenied(String requestId, Authentication authentication, String
             }
         }
 
+        @Override
+        public void coordinatingActionResponse(String requestId, Authentication authentication, String action,
+                                               TransportRequest transportRequest,
+                                               TransportResponse transportResponse) {
+            for (AuditTrail auditTrail : auditTrails) {
+                auditTrail.coordinatingActionResponse(requestId, authentication, action, transportRequest, transportResponse);
+            }
+        }
+
         @Override
         public void tamperedRequest(String requestId, RestRequest request) {
             for (AuditTrail auditTrail : auditTrails) {

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

@@ -39,6 +39,7 @@
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportRequest;
+import org.elasticsearch.transport.TransportResponse;
 import org.elasticsearch.xpack.core.security.action.CreateApiKeyAction;
 import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest;
 import org.elasticsearch.xpack.core.security.action.GrantApiKeyAction;
@@ -815,6 +816,13 @@ public void runAsDenied(String requestId, Authentication authentication, RestReq
         }
     }
 
+    @Override
+    public void coordinatingActionResponse(String requestId, Authentication authentication, String action,
+                                           TransportRequest transportRequest,
+                                           TransportResponse transportResponse) {
+        // not implemented yet
+    }
+
     private LogEntryBuilder securityChangeLogEntryBuilder(String requestId) {
         return new LogEntryBuilder(false)
                 .with(EVENT_TYPE_FIELD_NAME, SECURITY_CHANGE_ORIGIN_FIELD_VALUE)

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

@@ -61,11 +61,8 @@
 import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
 import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
 import org.elasticsearch.xpack.core.security.user.AnonymousUser;
-import org.elasticsearch.xpack.core.security.user.AsyncSearchUser;
 import org.elasticsearch.xpack.core.security.user.SystemUser;
 import org.elasticsearch.xpack.core.security.user.User;
-import org.elasticsearch.xpack.core.security.user.XPackSecurityUser;
-import org.elasticsearch.xpack.core.security.user.XPackUser;
 import org.elasticsearch.xpack.security.audit.AuditLevel;
 import org.elasticsearch.xpack.security.audit.AuditTrail;
 import org.elasticsearch.xpack.security.audit.AuditTrailService;
@@ -95,6 +92,7 @@
 import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.INDICES_PERMISSIONS_KEY;
 import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.ORIGINATING_ACTION_KEY;
 import static org.elasticsearch.xpack.core.security.support.Exceptions.authorizationError;
+import static org.elasticsearch.xpack.core.security.user.User.isInternal;
 import static org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail.PRINCIPAL_ROLES_FIELD_NAME;
 
 public class AuthorizationService {
@@ -190,14 +188,15 @@ public void authorize(final Authentication authentication, final String action,
             if (auditId == null) {
                 // We would like to assert that there is an existing request-id, but if this is a system action, then that might not be
                 // true because the request-id is generated during authentication
-                if (isInternalUser(authentication.getUser()) != false) {
+                if (isInternal(authentication.getUser())) {
                     auditId = AuditUtil.getOrGenerateRequestId(threadContext);
                 } else {
                     auditTrailService.get().tamperedRequest(null, authentication, action, originalRequest);
                     final String message = "Attempt to authorize action [" + action + "] for [" + authentication.getUser().principal()
                             + "] without an existing request-id";
                     assert false : message;
                     listener.onFailure(new ElasticsearchSecurityException(message));
+                    return;
                 }
             }
 
@@ -397,7 +396,7 @@ AuthorizationEngine getAuthorizationEngine(final Authentication authentication)
     private AuthorizationEngine getAuthorizationEngineForUser(final User user) {
         if (rbacEngine != authorizationEngine && licenseState.isSecurityEnabled() &&
             licenseState.checkFeature(Feature.SECURITY_AUTHORIZATION_ENGINE)) {
-            if (ClientReservedRealm.isReserved(user.principal(), settings) || isInternalUser(user)) {
+            if (ClientReservedRealm.isReserved(user.principal(), settings) || isInternal(user)) {
                 return rbacEngine;
             } else {
                 return authorizationEngine;
@@ -448,10 +447,6 @@ private TransportRequest maybeUnwrapRequest(Authentication authentication, Trans
         return request;
     }
 
-    private boolean isInternalUser(User user) {
-        return SystemUser.is(user) || XPackUser.is(user) || XPackSecurityUser.is(user) || AsyncSearchUser.is(user);
-    }
-
     private void authorizeRunAs(final RequestInfo requestInfo, final AuthorizationInfo authzInfo,
                                 final ActionListener<AuthorizationResult> listener) {
         final Authentication authentication = requestInfo.getAuthentication();