From 8c61cabe876316d62f147951c63b05f7772bc830 Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 6 Dec 2017 00:00:09 +0100 Subject: [PATCH] remove audit logging changes added for delayed actions that are now removed This commit reverts part of elastic/x-pack-elasticsearch#2210 Original commit: elastic/x-pack-elasticsearch@75fda798516816507a1b998636c8feceafe81678 --- .../xpack/security/audit/AuditTrail.java | 6 +- .../security/audit/AuditTrailService.java | 12 +-- .../security/audit/index/IndexAuditTrail.java | 12 +-- .../audit/logfile/LoggingAuditTrail.java | 10 +- .../security/authz/AuthorizationService.java | 68 ++++++------ .../authz/IndicesAndAliasesResolver.java | 3 +- .../SecuritySearchOperationListener.java | 2 +- .../audit/AuditTrailServiceTests.java | 27 +---- .../index/IndexAuditTrailMutedTests.java | 9 +- .../audit/index/IndexAuditTrailTests.java | 35 ++---- .../audit/logfile/LoggingAuditTrailTests.java | 50 ++------- .../authz/AuthorizationServiceTests.java | 102 +++++++++--------- .../SecuritySearchOperationListenerTests.java | 12 +-- 13 files changed, 129 insertions(+), 219 deletions(-) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrail.java b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrail.java index 36741043547..58d7dafac50 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrail.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrail.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.xpack.security.audit; -import org.elasticsearch.common.Nullable; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.transport.TransportMessage; import org.elasticsearch.xpack.security.authc.AuthenticationToken; @@ -13,7 +12,6 @@ import org.elasticsearch.xpack.security.transport.filter.SecurityIpFilterRule; import org.elasticsearch.xpack.security.user.User; import java.net.InetAddress; -import java.util.Set; public interface AuditTrail { @@ -39,9 +37,9 @@ public interface AuditTrail { void authenticationFailed(String realm, AuthenticationToken token, RestRequest request); - void accessGranted(User user, String action, TransportMessage message, String[] roleNames, @Nullable Set specificIndices); + void accessGranted(User user, String action, TransportMessage message, String[] roleNames); - void accessDenied(User user, String action, TransportMessage message, String[] roleNames, @Nullable Set specificIndices); + void accessDenied(User user, String action, TransportMessage message, String[] roleNames); void tamperedRequest(RestRequest request); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrailService.java b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrailService.java index 48feeaccb26..5882eb84be7 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrailService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrailService.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.xpack.security.audit; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.license.XPackLicenseState; @@ -18,7 +17,6 @@ import org.elasticsearch.xpack.security.user.User; import java.net.InetAddress; import java.util.Collections; import java.util.List; -import java.util.Set; public class AuditTrailService extends AbstractComponent implements AuditTrail { @@ -132,21 +130,19 @@ public class AuditTrailService extends AbstractComponent implements AuditTrail { } @Override - public void accessGranted(User user, String action, TransportMessage message, String[] roleNames, - @Nullable Set specificIndices) { + public void accessGranted(User user, String action, TransportMessage message, String[] roleNames) { if (licenseState.isAuditingAllowed()) { for (AuditTrail auditTrail : auditTrails) { - auditTrail.accessGranted(user, action, message, roleNames, specificIndices); + auditTrail.accessGranted(user, action, message, roleNames); } } } @Override - public void accessDenied(User user, String action, TransportMessage message, String[] roleNames, - @Nullable Set specificIndices) { + public void accessDenied(User user, String action, TransportMessage message, String[] roleNames) { if (licenseState.isAuditingAllowed()) { for (AuditTrail auditTrail : auditTrails) { - auditTrail.accessDenied(user, action, message, roleNames, specificIndices); + auditTrail.accessDenied(user, action, message, roleNames); } } } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrail.java b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrail.java index 9b0b57e3d06..5cf92b18116 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrail.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrail.java @@ -473,15 +473,13 @@ public class IndexAuditTrail extends AbstractComponent implements AuditTrail { } @Override - public void accessGranted(User user, String action, TransportMessage message, String[] roleNames, - @Nullable Set specificIndices) { + public void accessGranted(User user, String action, TransportMessage message, String[] roleNames) { final boolean isSystem = SystemUser.is(user) || XPackUser.is(user); final boolean logSystemAccessGranted = isSystem && events.contains(SYSTEM_ACCESS_GRANTED); final boolean shouldLog = logSystemAccessGranted || (isSystem == false && events.contains(ACCESS_GRANTED)); if (shouldLog) { try { - Set indices = specificIndices == null ? indices(message) : specificIndices; - enqueue(message("access_granted", action, user, roleNames, null, indices, message), "access_granted"); + enqueue(message("access_granted", action, user, roleNames, null, indices(message), message), "access_granted"); } catch (Exception e) { logger.warn("failed to index audit event: [access_granted]", e); } @@ -489,12 +487,10 @@ public class IndexAuditTrail extends AbstractComponent implements AuditTrail { } @Override - public void accessDenied(User user, String action, TransportMessage message, String[] roleNames, - @Nullable Set specificIndices) { + public void accessDenied(User user, String action, TransportMessage message, String[] roleNames) { if (events.contains(ACCESS_DENIED) && XPackUser.is(user) == false) { try { - Set indices = specificIndices == null ? indices(message) : specificIndices; - enqueue(message("access_denied", action, user, roleNames, null, indices, message), "access_denied"); + enqueue(message("access_denied", action, user, roleNames, null, indices(message), message), "access_denied"); } catch (Exception e) { logger.warn("failed to index audit event: [access_denied]", e); } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java index d9b6ea139bf..13789b6aac2 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java @@ -255,13 +255,12 @@ public class LoggingAuditTrail extends AbstractComponent implements AuditTrail, } @Override - public void accessGranted(User user, String action, TransportMessage message, String[] roleNames, - @Nullable Set specificIndices) { + public void accessGranted(User user, String action, TransportMessage message, String[] roleNames) { final boolean isSystem = SystemUser.is(user) || XPackUser.is(user); final boolean logSystemAccessGranted = isSystem && events.contains(SYSTEM_ACCESS_GRANTED); final boolean shouldLog = logSystemAccessGranted || (isSystem == false && events.contains(ACCESS_GRANTED)); if (shouldLog) { - String indices = specificIndices == null ? indicesString(message) : collectionToCommaDelimitedString(specificIndices); + String indices = indicesString(message); final LocalNodeInfo localNodeInfo = this.localNodeInfo; if (indices != null) { logger.info("{}[transport] [access_granted]\t{}, {}, roles=[{}], action=[{}], indices=[{}], request=[{}]", @@ -276,10 +275,9 @@ public class LoggingAuditTrail extends AbstractComponent implements AuditTrail, } @Override - public void accessDenied(User user, String action, TransportMessage message, String[] roleNames, - @Nullable Set specificIndices) { + public void accessDenied(User user, String action, TransportMessage message, String[] roleNames) { if (events.contains(ACCESS_DENIED)) { - String indices = specificIndices == null ? indicesString(message) : collectionToCommaDelimitedString(specificIndices); + String indices = indicesString(message); final LocalNodeInfo localNodeInfo = this.localNodeInfo; if (indices != null) { diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index d3f84872508..17ffdeac89c 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -29,7 +29,6 @@ import org.elasticsearch.action.termvectors.MultiTermVectorsAction; import org.elasticsearch.action.update.UpdateAction; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.settings.Setting; @@ -154,10 +153,10 @@ public class AuthorizationService extends AbstractComponent { if (SystemUser.isAuthorized(action)) { putTransientIfNonExisting(INDICES_PERMISSIONS_KEY, IndicesAccessControl.ALLOW_ALL); putTransientIfNonExisting(ROLE_NAMES_KEY, new String[] { SystemUser.ROLE_NAME }); - grant(authentication, action, request, new String[] { SystemUser.ROLE_NAME }, null); + grant(authentication, action, request, new String[] { SystemUser.ROLE_NAME }); return; } - throw denial(authentication, action, request, new String[] { SystemUser.ROLE_NAME }, null); + throw denial(authentication, action, request, new String[] { SystemUser.ROLE_NAME }); } // get the roles of the authenticated user, which may be different than the effective @@ -184,15 +183,15 @@ public class AuthorizationService extends AbstractComponent { ClusterPermission cluster = permission.cluster(); if (cluster.check(action) || checkSameUserPermissions(action, request, authentication)) { putTransientIfNonExisting(INDICES_PERMISSIONS_KEY, IndicesAccessControl.ALLOW_ALL); - grant(authentication, action, request, permission.names(), null); + grant(authentication, action, request, permission.names()); return; } - throw denial(authentication, action, request, permission.names(), null); + throw denial(authentication, action, request, permission.names()); } // ok... this is not a cluster action, let's verify it's an indices action if (!IndexPrivilege.ACTION_MATCHER.test(action)) { - throw denial(authentication, action, request, permission.names(), null); + throw denial(authentication, action, request, permission.names()); } //composite actions are explicitly listed and will be authorized at the sub-request / shard level @@ -203,10 +202,10 @@ public class AuthorizationService extends AbstractComponent { } // we check if the user can execute the action, without looking at indices, which will be authorized at the shard level if (permission.indices().check(action)) { - grant(authentication, action, request, permission.names(), null); + grant(authentication, action, request, permission.names()); return; } - throw denial(authentication, action, request, permission.names(), null); + throw denial(authentication, action, request, permission.names()); } else if (isTranslatedToBulkAction(action)) { if (request instanceof CompositeIndicesRequest == false) { throw new IllegalStateException("Bulk translated actions must implement " + CompositeIndicesRequest.class.getSimpleName() @@ -214,10 +213,10 @@ public class AuthorizationService extends AbstractComponent { } // we check if the user can execute the action, without looking at indices, which will be authorized at the shard level if (permission.indices().check(action)) { - grant(authentication, action, request, permission.names(), null); + grant(authentication, action, request, permission.names()); return; } - throw denial(authentication, action, request, permission.names(), null); + throw denial(authentication, action, request, permission.names()); } else if (TransportActionProxy.isProxyAction(action)) { // we authorize proxied actions once they are "unwrapped" on the next node if (TransportActionProxy.isProxyRequest(originalRequest) == false) { @@ -225,12 +224,12 @@ public class AuthorizationService extends AbstractComponent { + action + "] is a proxy action"); } if (permission.indices().check(action)) { - grant(authentication, action, request, permission.names(), null); + grant(authentication, action, request, permission.names()); return; } else { // we do this here in addition to the denial below since we might run into an assertion on scroll request below if we // don't have permission to read cross cluster but wrap a scroll request. - throw denial(authentication, action, request, permission.names(), null); + throw denial(authentication, action, request, permission.names()); } } @@ -249,18 +248,18 @@ public class AuthorizationService extends AbstractComponent { // index and if they cannot, we can fail the request early before we allow the execution of the action and in // turn the shard actions if (SearchScrollAction.NAME.equals(action) && permission.indices().check(action) == false) { - throw denial(authentication, action, request, permission.names(), null); + throw denial(authentication, action, request, permission.names()); } else { // we store the request as a transient in the ThreadContext in case of a authorization failure at the shard // level. If authorization fails we will audit a access_denied message and will use the request to retrieve // information such as the index and the incoming address of the request - grant(authentication, action, request, permission.names(), null); + grant(authentication, action, request, permission.names()); return; } } else { assert false : "only scroll related requests are known indices api that don't support retrieving the indices they relate to"; - throw denial(authentication, action, request, permission.names(), null); + throw denial(authentication, action, request, permission.names()); } } @@ -270,12 +269,12 @@ public class AuthorizationService extends AbstractComponent { // If this request does not allow remote indices // then the user must have permission to perform this action on at least 1 local index if (allowsRemoteIndices == false && permission.indices().check(action) == false) { - throw denial(authentication, action, request, permission.names(), null); + throw denial(authentication, action, request, permission.names()); } final MetaData metaData = clusterService.state().metaData(); final AuthorizedIndices authorizedIndices = new AuthorizedIndices(authentication.getUser(), permission, action, metaData); - final ResolvedIndices resolvedIndices = resolveIndexNames(authentication, action, request, request, + final ResolvedIndices resolvedIndices = resolveIndexNames(authentication, action, request, metaData, authorizedIndices, permission); assert !resolvedIndices.isEmpty() : "every indices request needs to have its indices set thus the resolved indices must not be empty"; @@ -283,21 +282,21 @@ public class AuthorizationService extends AbstractComponent { // If this request does reference any remote indices // then the user must have permission to perform this action on at least 1 local index if (resolvedIndices.getRemote().isEmpty() && permission.indices().check(action) == false) { - throw denial(authentication, action, request, permission.names(), null); + throw denial(authentication, action, request, permission.names()); } //all wildcard expressions have been resolved and only the security plugin could have set '-*' here. //'-*' matches no indices so we allow the request to go through, which will yield an empty response if (resolvedIndices.isNoIndicesPlaceholder()) { putTransientIfNonExisting(INDICES_PERMISSIONS_KEY, IndicesAccessControl.ALLOW_NO_INDICES); - grant(authentication, action, request, permission.names(), null); + grant(authentication, action, request, permission.names()); return; } final Set localIndices = new HashSet<>(resolvedIndices.getLocal()); IndicesAccessControl indicesAccessControl = permission.authorize(action, localIndices, metaData, fieldPermissionsCache); if (!indicesAccessControl.isGranted()) { - throw denial(authentication, action, request, permission.names(), null); + throw denial(authentication, action, request, permission.names()); } else if (hasSecurityIndexAccess(indicesAccessControl) && MONITOR_INDEX_PREDICATE.test(action) == false && isSuperuser(authentication.getUser()) == false) { @@ -305,7 +304,7 @@ public class AuthorizationService extends AbstractComponent { // purposes. These monitor requests also sometimes resolve indices concretely and then requests them logger.debug("user [{}] attempted to directly perform [{}] against the security index [{}]", authentication.getUser().principal(), action, SecurityLifecycleService.SECURITY_INDEX_NAME); - throw denial(authentication, action, request, permission.names(), null); + throw denial(authentication, action, request, permission.names()); } else { putTransientIfNonExisting(INDICES_PERMISSIONS_KEY, indicesAccessControl); } @@ -321,7 +320,7 @@ public class AuthorizationService extends AbstractComponent { } indicesAccessControl = permission.authorize("indices:admin/aliases", aliasesAndIndices, metaData, fieldPermissionsCache); if (!indicesAccessControl.isGranted()) { - throw denial(authentication, "indices:admin/aliases", request, permission.names(), null); + throw denial(authentication, "indices:admin/aliases", request, permission.names()); } // no need to re-add the indicesAccessControl in the context, // because the create index call doesn't do anything FLS or DLS @@ -336,7 +335,7 @@ public class AuthorizationService extends AbstractComponent { authorizeBulkItems(authentication, (BulkShardRequest) request, permission, metaData, localIndices, authorizedIndices); } - grant(authentication, action, originalRequest, permission.names(), null); + grant(authentication, action, originalRequest, permission.names()); } private boolean hasSecurityIndexAccess(IndicesAccessControl indicesAccessControl) { @@ -356,7 +355,7 @@ public class AuthorizationService extends AbstractComponent { * and then checks whether that action is allowed on the targeted index. Items * that fail this checks are {@link BulkItemRequest#abort(String, Exception) * aborted}, with an - * {@link #denial(Authentication, String, TransportRequest, String[], Set) access + * {@link #denial(Authentication, String, TransportRequest, String[]) access * denied} exception. Because a shard level request is for exactly 1 index, and * there are a small number of possible item {@link DocWriteRequest.OpType * types}, the number of distinct authorization checks that need to be performed @@ -395,7 +394,7 @@ public class AuthorizationService extends AbstractComponent { return itemAccessControl.isGranted(); }); if (granted == false) { - item.abort(resolvedIndex, denial(authentication, itemAction, request, permission.names(), null)); + item.abort(resolvedIndex, denial(authentication, itemAction, request, permission.names())); } } } @@ -419,13 +418,12 @@ public class AuthorizationService extends AbstractComponent { throw new IllegalArgumentException("No equivalent action for opType [" + docWriteRequest.opType() + "]"); } - private ResolvedIndices resolveIndexNames(Authentication authentication, String action, Object indicesRequest, - TransportRequest mainRequest,MetaData metaData, - AuthorizedIndices authorizedIndices, Role permission) { + private ResolvedIndices resolveIndexNames(Authentication authentication, String action, TransportRequest request, + MetaData metaData, AuthorizedIndices authorizedIndices, Role permission) { try { - return indicesAndAliasesResolver.resolve(indicesRequest, metaData, authorizedIndices); + return indicesAndAliasesResolver.resolve(request, metaData, authorizedIndices); } catch (Exception e) { - auditTrail.accessDenied(authentication.getUser(), action, mainRequest, permission.names(), null); + auditTrail.accessDenied(authentication.getUser(), action, request, permission.names()); throw e; } } @@ -550,9 +548,8 @@ public class AuthorizationService extends AbstractComponent { return ReservedRealm.TYPE.equals(realmType) || NativeRealm.TYPE.equals(realmType); } - ElasticsearchSecurityException denial(Authentication authentication, String action, TransportRequest request, - String[] roleNames, @Nullable Set specificIndices) { - auditTrail.accessDenied(authentication.getUser(), action, request, roleNames, specificIndices); + ElasticsearchSecurityException denial(Authentication authentication, String action, TransportRequest request, String[] roleNames) { + auditTrail.accessDenied(authentication.getUser(), action, request, roleNames); return denialException(authentication, action); } @@ -562,9 +559,8 @@ public class AuthorizationService extends AbstractComponent { return denialException(authentication, action); } - private void grant(Authentication authentication, String action, TransportRequest request, - String[] roleNames, @Nullable Set specificIndices) { - auditTrail.accessGranted(authentication.getUser(), action, request, roleNames, specificIndices); + private void grant(Authentication authentication, String action, TransportRequest request, String[] roleNames) { + auditTrail.accessGranted(authentication.getUser(), action, request, roleNames); } private void grantRunAs(Authentication authentication, String action, TransportRequest request, String[] roleNames) { diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java index ac6e0f98002..85b6c5ee0fd 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java @@ -24,6 +24,7 @@ import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.transport.RemoteClusterAware; +import org.elasticsearch.transport.TransportRequest; import org.elasticsearch.xpack.graph.action.GraphExploreRequest; import java.net.InetSocketAddress; @@ -83,7 +84,7 @@ public class IndicesAndAliasesResolver { *
* Otherwise, N will be added to the local index list. */ - public ResolvedIndices resolve(Object request, MetaData metaData, AuthorizedIndices authorizedIndices) { + public ResolvedIndices resolve(TransportRequest request, MetaData metaData, AuthorizedIndices authorizedIndices) { if (request instanceof IndicesAliasesRequest) { ResolvedIndices indices = ResolvedIndices.empty(); IndicesAliasesRequest indicesAliasesRequest = (IndicesAliasesRequest) request; diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListener.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListener.java index da6d3bb85b2..0917aad4f27 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListener.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListener.java @@ -92,7 +92,7 @@ public final class SecuritySearchOperationListener implements SearchOperationLis final boolean sameUser = samePrincipal && sameRealmType; if (sameUser == false) { - auditTrailService.accessDenied(current.getUser(), action, request, roleNames, null); + auditTrailService.accessDenied(current.getUser(), action, request, roleNames); throw new SearchContextMissingException(id); } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailServiceTests.java index ac59e62b8bd..385fb350027 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailServiceTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailServiceTests.java @@ -18,12 +18,9 @@ import org.junit.Before; import java.net.InetAddress; import java.util.ArrayList; -import java.util.HashSet; import java.util.List; -import java.util.Set; import static java.util.Collections.unmodifiableList; -import static java.util.Collections.unmodifiableSet; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; @@ -142,12 +139,11 @@ public class AuditTrailServiceTests extends ESTestCase { public void testAccessGranted() throws Exception { User user = new User("_username", "r1"); String[] roles = new String[] { randomAlphaOfLengthBetween(1, 6) }; - Set specificIndices = randomBoolean() ? randomSpecificIndices() : null; - service.accessGranted(user, "_action", message, roles, specificIndices); + service.accessGranted(user, "_action", message, roles); verify(licenseState).isAuditingAllowed(); if (isAuditingAllowed) { for (AuditTrail auditTrail : auditTrails) { - verify(auditTrail).accessGranted(user, "_action", message, roles, specificIndices); + verify(auditTrail).accessGranted(user, "_action", message, roles); } } else { verifyZeroInteractions(auditTrails.toArray((Object[]) new AuditTrail[auditTrails.size()])); @@ -157,12 +153,11 @@ public class AuditTrailServiceTests extends ESTestCase { public void testAccessDenied() throws Exception { User user = new User("_username", "r1"); String[] roles = new String[] { randomAlphaOfLengthBetween(1, 6) }; - Set specificIndices = randomBoolean() ? randomSpecificIndices() : null; - service.accessDenied(user, "_action", message, roles, specificIndices); + service.accessDenied(user, "_action", message, roles); verify(licenseState).isAuditingAllowed(); if (isAuditingAllowed) { for (AuditTrail auditTrail : auditTrails) { - verify(auditTrail).accessDenied(user, "_action", message, roles, specificIndices); + verify(auditTrail).accessDenied(user, "_action", message, roles); } } else { verifyZeroInteractions(auditTrails.toArray((Object[]) new AuditTrail[auditTrails.size()])); @@ -224,18 +219,4 @@ public class AuditTrailServiceTests extends ESTestCase { verifyZeroInteractions(auditTrails.toArray((Object[]) new AuditTrail[auditTrails.size()])); } } - - /** - * Generates a semi-believable random value for the specificIndices parameter sent - * to the {@link AuditTrail#accessGranted(User, String, TransportMessage, String[], Set)} or - * {@link AuditTrail#accessDenied(User, String, TransportMessage, String[], Set)} methods. - */ - public static Set randomSpecificIndices() { - Set specificIndices = new HashSet<>(); - int count = between(1, 10); - while (specificIndices.size() < count) { - specificIndices.add(randomAlphaOfLength(5)); - } - return unmodifiableSet(specificIndices); - } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailMutedTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailMutedTests.java index 36ae8bd6e5c..ea9093bb81f 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailMutedTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailMutedTests.java @@ -35,7 +35,6 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.atomic.AtomicBoolean; -import static org.elasticsearch.xpack.security.audit.AuditTrailServiceTests.randomSpecificIndices; import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verifyZeroInteractions; @@ -174,8 +173,7 @@ public class IndexAuditTrailMutedTests extends ESTestCase { createAuditTrail(new String[] { "access_granted" }); TransportMessage message = mock(TransportMessage.class); User user = mock(User.class); - auditTrail.accessGranted(user, randomAlphaOfLengthBetween(6, 40), message, new String[] { "role" }, - randomBoolean() ? randomSpecificIndices() : null); + auditTrail.accessGranted(user, randomAlphaOfLengthBetween(6, 40), message, new String[] { "role" }); assertThat(messageEnqueued.get(), is(false)); assertThat(clientCalled.get(), is(false)); @@ -186,7 +184,7 @@ public class IndexAuditTrailMutedTests extends ESTestCase { createAuditTrail(randomFrom(new String[] { "access_granted" }, null)); TransportMessage message = mock(TransportMessage.class); User user = SystemUser.INSTANCE; - auditTrail.accessGranted(user, "internal:foo", message, new String[] { "role" }, randomBoolean() ? randomSpecificIndices() : null); + auditTrail.accessGranted(user, "internal:foo", message, new String[] { "role" }); assertThat(messageEnqueued.get(), is(false)); assertThat(clientCalled.get(), is(false)); @@ -197,8 +195,7 @@ public class IndexAuditTrailMutedTests extends ESTestCase { createAuditTrail(new String[] { "access_denied" }); TransportMessage message = mock(TransportMessage.class); User user = mock(User.class); - auditTrail.accessDenied(user, randomAlphaOfLengthBetween(6, 40), message, new String[] { "role" }, - randomBoolean() ? randomSpecificIndices() : null); + auditTrail.accessDenied(user, randomAlphaOfLengthBetween(6, 40), message, new String[] { "role" }); assertThat(messageEnqueued.get(), is(false)); assertThat(clientCalled.get(), is(false)); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailTests.java index a1520205751..484b0e7acf0 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailTests.java @@ -68,7 +68,6 @@ import java.util.function.Function; import static org.elasticsearch.test.ESIntegTestCase.Scope.SUITE; import static org.elasticsearch.test.InternalTestCluster.clusterName; -import static org.elasticsearch.xpack.security.audit.AuditTrailServiceTests.randomSpecificIndices; import static org.elasticsearch.xpack.security.audit.index.IndexNameResolver.Rollover.DAILY; import static org.elasticsearch.xpack.security.audit.index.IndexNameResolver.Rollover.HOURLY; import static org.elasticsearch.xpack.security.audit.index.IndexNameResolver.Rollover.MONTHLY; @@ -508,9 +507,8 @@ public class IndexAuditTrailTests extends SecurityIntegTestCase { } else { user = new User("_username", new String[]{"r1"}); } - Set specificIndices = randomBoolean() ? randomSpecificIndices() : null; String role = randomAlphaOfLengthBetween(1, 6); - auditor.accessGranted(user, "_action", message, new String[] { role }, specificIndices); + auditor.accessGranted(user, "_action", message, new String[] { role }); SearchHit hit = getIndexedAuditMessage(enqueuedMessage.get()); assertAuditMessage(hit, "transport", "access_granted"); @@ -523,18 +521,10 @@ public class IndexAuditTrailTests extends SecurityIntegTestCase { assertEquals("_username", sourceMap.get("principal")); } assertEquals("_action", sourceMap.get("action")); - assertThat((Iterable) sourceMap.get(IndexAuditTrail.Field.ROLE_NAMES), containsInAnyOrder(role)); - - List indices = (List) sourceMap.get("indices"); - if (specificIndices == null) { - if (message instanceof IndicesRequest) { - assertThat(indices, containsInAnyOrder((Object[]) ((IndicesRequest)message).indices())); - } else { - assertThat(indices, nullValue()); - } - } else { - assertThat(indices, containsInAnyOrder(specificIndices.toArray())); + if (message instanceof IndicesRequest) { + List indices = (List) sourceMap.get("indices"); + assertThat(indices, containsInAnyOrder((Object[]) ((IndicesRequest)message).indices())); } assertEquals(sourceMap.get("request"), message.getClass().getSimpleName()); } @@ -543,7 +533,7 @@ public class IndexAuditTrailTests extends SecurityIntegTestCase { initialize(new String[] { "system_access_granted" }, null); TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); String role = randomAlphaOfLengthBetween(1, 6); - auditor.accessGranted(SystemUser.INSTANCE, "internal:_action", message, new String[] { role }, null); + auditor.accessGranted(SystemUser.INSTANCE, "internal:_action", message, new String[] { role }); SearchHit hit = getIndexedAuditMessage(enqueuedMessage.get()); assertAuditMessage(hit, "transport", "access_granted"); @@ -565,9 +555,8 @@ public class IndexAuditTrailTests extends SecurityIntegTestCase { } else { user = new User("_username", new String[]{"r1"}); } - Set specificIndices = randomBoolean() ? randomSpecificIndices() : null; String role = randomAlphaOfLengthBetween(1, 6); - auditor.accessDenied(user, "_action", message, new String[] { role }, specificIndices); + auditor.accessDenied(user, "_action", message, new String[] { role }); SearchHit hit = getIndexedAuditMessage(enqueuedMessage.get()); Map sourceMap = hit.getSourceAsMap(); @@ -580,15 +569,9 @@ public class IndexAuditTrailTests extends SecurityIntegTestCase { assertEquals("_username", sourceMap.get("principal")); } assertEquals("_action", sourceMap.get("action")); - List indices = (List) sourceMap.get("indices"); - if (specificIndices == null) { - if (message instanceof IndicesRequest) { - assertThat(indices, containsInAnyOrder((Object[]) ((IndicesRequest)message).indices())); - } else { - assertThat(indices, nullValue()); - } - } else { - assertThat(indices, containsInAnyOrder(specificIndices.toArray())); + if (message instanceof IndicesRequest) { + List indices = (List) sourceMap.get("indices"); + assertThat(indices, containsInAnyOrder((Object[]) ((IndicesRequest)message).indices())); } assertEquals(sourceMap.get("request"), message.getClass().getSimpleName()); assertThat((Iterable) sourceMap.get(IndexAuditTrail.Field.ROLE_NAMES), containsInAnyOrder(role)); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java index 0ece841fe2d..ba0252ab337 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java @@ -27,7 +27,6 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.test.rest.FakeRestRequest.Builder; import org.elasticsearch.transport.TransportMessage; -import org.elasticsearch.xpack.security.audit.AuditTrail; import org.elasticsearch.xpack.security.audit.AuditUtil; import org.elasticsearch.xpack.security.authc.AuthenticationToken; import org.elasticsearch.xpack.security.rest.RemoteHostHeader; @@ -43,13 +42,10 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Set; -import static java.util.Collections.unmodifiableSet; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.mock; @@ -342,7 +338,7 @@ public class LoggingAuditTrailTests extends ESTestCase { user = new User("_username", new String[]{"r1"}); } String role = randomAlphaOfLengthBetween(1, 6); - auditTrail.accessGranted(user, "_action", message, new String[] { role }, null); + auditTrail.accessGranted(user, "_action", message, new String[] { role }); String userInfo = (runAs ? "principal=[running as], run_by_principal=[_username]" : "principal=[_username]") + ", roles=[" + role + "]"; if (message instanceof IndicesRequest) { @@ -353,19 +349,11 @@ public class LoggingAuditTrailTests extends ESTestCase { ", action=[_action], request=[MockMessage]"); } - // test specificIndices - CapturingLogger.output(logger.getName(), Level.INFO).clear(); - Set specificIndices = randomSpecificIndices(); - auditTrail.accessGranted(user, "_action", message, new String[] { role }, specificIndices); - assertMsg(logger, Level.INFO, prefix + "[transport] [access_granted]\t" + origins + ", " + userInfo + - ", action=[_action], indices=[" + Strings.collectionToCommaDelimitedString(specificIndices) + - "], request=[" + message.getClass().getSimpleName() + "]"); - // test disabled CapturingLogger.output(logger.getName(), Level.INFO).clear(); settings = Settings.builder().put(settings).put("xpack.security.audit.logfile.events.exclude", "access_granted").build(); auditTrail = new LoggingAuditTrail(settings, clusterService, logger, threadContext); - auditTrail.accessGranted(user, "_action", message, new String[] { role }, randomBoolean() ? specificIndices : null); + auditTrail.accessGranted(user, "_action", message, new String[] { role }); assertEmptyLog(logger); } @@ -374,14 +362,14 @@ public class LoggingAuditTrailTests extends ESTestCase { LoggingAuditTrail auditTrail = new LoggingAuditTrail(settings, clusterService, logger, threadContext); TransportMessage message = randomBoolean() ? new MockMessage(threadContext) : new MockIndicesRequest(threadContext); String role = randomAlphaOfLengthBetween(1, 6); - auditTrail.accessGranted(SystemUser.INSTANCE, "internal:_action", message, new String[] { role }, null); + auditTrail.accessGranted(SystemUser.INSTANCE, "internal:_action", message, new String[] { role }); assertEmptyLog(logger); // test enabled settings = Settings.builder().put(settings).put("xpack.security.audit.logfile.events.include", "system_access_granted").build(); auditTrail = new LoggingAuditTrail(settings, clusterService, logger, threadContext); String origins = LoggingAuditTrail.originAttributes(threadContext, message, auditTrail.localNodeInfo); - auditTrail.accessGranted(SystemUser.INSTANCE, "internal:_action", message, new String[] { role }, null); + auditTrail.accessGranted(SystemUser.INSTANCE, "internal:_action", message, new String[] { role }); if (message instanceof IndicesRequest) { assertMsg(logger, Level.INFO, prefix + "[transport] [access_granted]\t" + origins + ", principal=[" + SystemUser.INSTANCE.principal() @@ -406,7 +394,7 @@ public class LoggingAuditTrailTests extends ESTestCase { user = new User("_username", new String[]{"r1"}); } String role = randomAlphaOfLengthBetween(1, 6); - auditTrail.accessGranted(user, "internal:_action", message, new String[] { role }, null); + auditTrail.accessGranted(user, "internal:_action", message, new String[] { role }); String userInfo = (runAs ? "principal=[running as], run_by_principal=[_username]" : "principal=[_username]") + ", roles=[" + role + "]"; if (message instanceof IndicesRequest) { @@ -421,7 +409,7 @@ public class LoggingAuditTrailTests extends ESTestCase { CapturingLogger.output(logger.getName(), Level.INFO).clear(); settings = Settings.builder().put(settings).put("xpack.security.audit.logfile.events.exclude", "access_granted").build(); auditTrail = new LoggingAuditTrail(settings, clusterService, logger, threadContext); - auditTrail.accessGranted(user, "internal:_action", message, new String[] { role }, null); + auditTrail.accessGranted(user, "internal:_action", message, new String[] { role }); assertEmptyLog(logger); } @@ -438,7 +426,7 @@ public class LoggingAuditTrailTests extends ESTestCase { user = new User("_username", new String[]{"r1"}); } String role = randomAlphaOfLengthBetween(1, 6); - auditTrail.accessDenied(user, "_action", message, new String[] { role }, null); + auditTrail.accessDenied(user, "_action", message, new String[] { role }); String userInfo = (runAs ? "principal=[running as], run_by_principal=[_username]" : "principal=[_username]") + ", roles=[" + role + "]"; if (message instanceof IndicesRequest) { @@ -449,19 +437,11 @@ public class LoggingAuditTrailTests extends ESTestCase { ", action=[_action], request=[MockMessage]"); } - // test specificIndices - CapturingLogger.output(logger.getName(), Level.INFO).clear(); - Set specificIndices = randomSpecificIndices(); - auditTrail.accessDenied(user, "_action", message, new String[]{ role }, specificIndices); - assertMsg(logger, Level.INFO, prefix + "[transport] [access_denied]\t" + origins + ", " + userInfo + - ", action=[_action], indices=[" + Strings.collectionToCommaDelimitedString(specificIndices) + - "], request=[" + message.getClass().getSimpleName() + "]"); - // test disabled CapturingLogger.output(logger.getName(), Level.INFO).clear(); settings = Settings.builder().put(settings).put("xpack.security.audit.logfile.events.exclude", "access_denied").build(); auditTrail = new LoggingAuditTrail(settings, clusterService, logger, threadContext); - auditTrail.accessDenied(user, "_action", message, new String[] { role }, null); + auditTrail.accessDenied(user, "_action", message, new String[] { role }); assertEmptyLog(logger); } @@ -746,20 +726,6 @@ public class LoggingAuditTrailTests extends ESTestCase { return Strings.collectionToCommaDelimitedString(AuditUtil.indices(message)); } - /** - * Generates a semi-believable random value for the specificIndices parameter sent - * to the {@link AuditTrail#accessGranted(User, String, TransportMessage, String[], Set)} or - * {@link AuditTrail#accessDenied(User, String, TransportMessage, String[], Set)} methods. - */ - private static Set randomSpecificIndices() { - Set specificIndices = new HashSet<>(); - int count = between(1, 10); - while (specificIndices.size() < count) { - specificIndices.add(randomAlphaOfLength(5)); - } - return unmodifiableSet(specificIndices); - } - private static class MockMessage extends TransportMessage { private MockMessage(ThreadContext threadContext) throws IOException { diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index ca74888939e..0c362dc2a56 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -219,11 +219,10 @@ public class AuthorizationServiceTests extends ESTestCase { // A failure would throw an exception authorize(createAuthentication(SystemUser.INSTANCE), "indices:monitor/whatever", request); - verify(auditTrail).accessGranted(SystemUser.INSTANCE, "indices:monitor/whatever", request, new String[] { SystemUser.ROLE_NAME }, - null); + verify(auditTrail).accessGranted(SystemUser.INSTANCE, "indices:monitor/whatever", request, new String[] { SystemUser.ROLE_NAME }); authorize(createAuthentication(SystemUser.INSTANCE), "internal:whatever", request); - verify(auditTrail).accessGranted(SystemUser.INSTANCE, "internal:whatever", request, new String[] { SystemUser.ROLE_NAME }, null); + verify(auditTrail).accessGranted(SystemUser.INSTANCE, "internal:whatever", request, new String[] { SystemUser.ROLE_NAME }); verifyNoMoreInteractions(auditTrail); } @@ -232,7 +231,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(SystemUser.INSTANCE), "indices:", request), "indices:", SystemUser.INSTANCE.principal()); - verify(auditTrail).accessDenied(SystemUser.INSTANCE, "indices:", request, new String[] { SystemUser.ROLE_NAME }, null); + verify(auditTrail).accessDenied(SystemUser.INSTANCE, "indices:", request, new String[] { SystemUser.ROLE_NAME }); verifyNoMoreInteractions(auditTrail); } @@ -241,8 +240,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(SystemUser.INSTANCE), "cluster:admin/whatever", request), "cluster:admin/whatever", SystemUser.INSTANCE.principal()); - verify(auditTrail).accessDenied(SystemUser.INSTANCE, "cluster:admin/whatever", request, new String[] { SystemUser.ROLE_NAME }, - null); + verify(auditTrail).accessDenied(SystemUser.INSTANCE, "cluster:admin/whatever", request, new String[] { SystemUser.ROLE_NAME }); verifyNoMoreInteractions(auditTrail); } @@ -252,7 +250,7 @@ public class AuthorizationServiceTests extends ESTestCase { () -> authorize(createAuthentication(SystemUser.INSTANCE), "cluster:admin/snapshot/status", request), "cluster:admin/snapshot/status", SystemUser.INSTANCE.principal()); verify(auditTrail).accessDenied(SystemUser.INSTANCE, "cluster:admin/snapshot/status", request, - new String[] { SystemUser.ROLE_NAME }, null); + new String[] { SystemUser.ROLE_NAME }); verifyNoMoreInteractions(auditTrail); } @@ -263,7 +261,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), "indices:a", request), "indices:a", "test user"); - verify(auditTrail).accessDenied(user, "indices:a", request, Role.EMPTY.names(), null); + verify(auditTrail).accessDenied(user, "indices:a", request, Role.EMPTY.names()); verifyNoMoreInteractions(auditTrail); } @@ -273,7 +271,7 @@ public class AuthorizationServiceTests extends ESTestCase { User user = new User("test user"); mockEmptyMetaData(); authorize(createAuthentication(user), SearchAction.NAME, request); - verify(auditTrail).accessGranted(user, SearchAction.NAME, request, Role.EMPTY.names(), null); + verify(auditTrail).accessGranted(user, SearchAction.NAME, request, Role.EMPTY.names()); verifyNoMoreInteractions(auditTrail); } @@ -290,7 +288,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), SearchAction.NAME, request), SearchAction.NAME, "test user"); - verify(auditTrail).accessDenied(user, SearchAction.NAME, request, Role.EMPTY.names(), null); + verify(auditTrail).accessDenied(user, SearchAction.NAME, request, Role.EMPTY.names()); verifyNoMoreInteractions(auditTrail); } @@ -306,7 +304,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), SearchAction.NAME, request), SearchAction.NAME, "test user"); - verify(auditTrail).accessDenied(user, SearchAction.NAME, request, Role.EMPTY.names(), null); + verify(auditTrail).accessDenied(user, SearchAction.NAME, request, Role.EMPTY.names()); verifyNoMoreInteractions(auditTrail); } @@ -317,7 +315,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), SqlAction.NAME, request), SqlAction.NAME, "test user"); - verify(auditTrail).accessDenied(user, SqlAction.NAME, request, Role.EMPTY.names(), null); + verify(auditTrail).accessDenied(user, SqlAction.NAME, request, Role.EMPTY.names()); verifyNoMoreInteractions(auditTrail); } @@ -333,7 +331,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), DeleteIndexAction.NAME, request), DeleteIndexAction.NAME, "test user"); - verify(auditTrail).accessDenied(user, DeleteIndexAction.NAME, request, Role.EMPTY.names(), null); + verify(auditTrail).accessDenied(user, DeleteIndexAction.NAME, request, Role.EMPTY.names()); verifyNoMoreInteractions(auditTrail); } @@ -350,7 +348,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), action, request), action, "test user"); - verify(auditTrail).accessDenied(user, action, request, Role.EMPTY.names(), null); + verify(auditTrail).accessDenied(user, action, request, Role.EMPTY.names()); verifyNoMoreInteractions(auditTrail); } @@ -364,7 +362,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), "whatever", request), "whatever", "test user"); - verify(auditTrail).accessDenied(user, "whatever", request, new String[] { role.getName() }, null); + verify(auditTrail).accessDenied(user, "whatever", request, new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); } @@ -384,7 +382,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), action, request), action, "test user"); - verify(auditTrail).accessDenied(user, action, request, new String[] { role.getName() }, null); + verify(auditTrail).accessDenied(user, action, request, new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); } @@ -393,7 +391,7 @@ public class AuthorizationServiceTests extends ESTestCase { Tuple request = randomCompositeRequest(); authorize(createAuthentication(user), request.v1(), request.v2()); - verify(auditTrail).accessGranted(user, request.v1(), request.v2(), new String[] { ElasticUser.ROLE_NAME }, null); + verify(auditTrail).accessGranted(user, request.v1(), request.v2(), new String[] { ElasticUser.ROLE_NAME }); } public void testSearchAgainstEmptyCluster() { @@ -411,7 +409,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), SearchAction.NAME, searchRequest), SearchAction.NAME, "test user"); - verify(auditTrail).accessDenied(user, SearchAction.NAME, searchRequest, new String[] { role.getName() }, null); + verify(auditTrail).accessDenied(user, SearchAction.NAME, searchRequest, new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); } @@ -420,7 +418,7 @@ public class AuthorizationServiceTests extends ESTestCase { SearchRequest searchRequest = new SearchRequest("does_not_exist") .indicesOptions(IndicesOptions.fromOptions(true, true, true, false)); authorize(createAuthentication(user), SearchAction.NAME, searchRequest); - verify(auditTrail).accessGranted(user, SearchAction.NAME, searchRequest, new String[] { role.getName() }, null); + verify(auditTrail).accessGranted(user, SearchAction.NAME, searchRequest, new String[] { role.getName() }); IndicesAccessControl indicesAccessControl = threadContext.getTransient(AuthorizationService.INDICES_PERMISSIONS_KEY); IndicesAccessControl.IndexAccessControl indexAccessControl = indicesAccessControl.getIndexPermissions(IndicesAndAliasesResolver.NO_INDEX_PLACEHOLDER); @@ -438,33 +436,33 @@ public class AuthorizationServiceTests extends ESTestCase { ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); authorize(createAuthentication(user), ClearScrollAction.NAME, clearScrollRequest); - verify(auditTrail).accessGranted(user, ClearScrollAction.NAME, clearScrollRequest, new String[] { role.getName() }, null); + verify(auditTrail).accessGranted(user, ClearScrollAction.NAME, clearScrollRequest, new String[] { role.getName() }); SearchScrollRequest searchScrollRequest = new SearchScrollRequest(); authorize(createAuthentication(user), SearchScrollAction.NAME, searchScrollRequest); - verify(auditTrail).accessGranted(user, SearchScrollAction.NAME, searchScrollRequest, new String[] { role.getName() }, null); + verify(auditTrail).accessGranted(user, SearchScrollAction.NAME, searchScrollRequest, new String[] { role.getName() }); // We have to use a mock request for other Scroll actions as the actual requests are package private to SearchTransportService TransportRequest request = mock(TransportRequest.class); authorize(createAuthentication(user), SearchTransportService.CLEAR_SCROLL_CONTEXTS_ACTION_NAME, request); verify(auditTrail).accessGranted(user, SearchTransportService.CLEAR_SCROLL_CONTEXTS_ACTION_NAME, request, - new String[] { role.getName() }, null); + new String[] { role.getName() }); authorize(createAuthentication(user), SearchTransportService.FETCH_ID_SCROLL_ACTION_NAME, request); verify(auditTrail).accessGranted(user, SearchTransportService.FETCH_ID_SCROLL_ACTION_NAME, request, - new String[] { role.getName() }, null); + new String[] { role.getName() }); authorize(createAuthentication(user), SearchTransportService.QUERY_FETCH_SCROLL_ACTION_NAME, request); verify(auditTrail).accessGranted(user, SearchTransportService.QUERY_FETCH_SCROLL_ACTION_NAME, request, - new String[] { role.getName() }, null); + new String[] { role.getName() }); authorize(createAuthentication(user), SearchTransportService.QUERY_SCROLL_ACTION_NAME, request); verify(auditTrail).accessGranted(user, SearchTransportService.QUERY_SCROLL_ACTION_NAME, request, - new String[] { role.getName() }, null); + new String[] { role.getName() }); authorize(createAuthentication(user), SearchTransportService.FREE_CONTEXT_SCROLL_ACTION_NAME, request); verify(auditTrail).accessGranted(user, SearchTransportService.FREE_CONTEXT_SCROLL_ACTION_NAME, request, - new String[] { role.getName() }, null); + new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); } @@ -479,7 +477,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), "indices:a", request), "indices:a", "test user"); - verify(auditTrail).accessDenied(user, "indices:a", request, new String[] { role.getName() }, null); + verify(auditTrail).accessDenied(user, "indices:a", request, new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); verify(clusterService, times(1)).state(); verify(state, times(1)).metaData(); @@ -497,7 +495,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), CreateIndexAction.NAME, request), IndicesAliasesAction.NAME, "test user"); - verify(auditTrail).accessDenied(user, IndicesAliasesAction.NAME, request, new String[] { role.getName() }, null); + verify(auditTrail).accessDenied(user, IndicesAliasesAction.NAME, request, new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); verify(clusterService).state(); verify(state, times(1)).metaData(); @@ -514,7 +512,7 @@ public class AuthorizationServiceTests extends ESTestCase { authorize(createAuthentication(user), CreateIndexAction.NAME, request); - verify(auditTrail).accessGranted(user, CreateIndexAction.NAME, request, new String[] { role.getName()}, null); + verify(auditTrail).accessGranted(user, CreateIndexAction.NAME, request, new String[] { role.getName()}); verifyNoMoreInteractions(auditTrail); verify(clusterService).state(); verify(state, times(1)).metaData(); @@ -536,7 +534,7 @@ public class AuthorizationServiceTests extends ESTestCase { () -> authorize(createAuthentication(anonymousUser), "indices:a", request), "indices:a", anonymousUser.principal()); - verify(auditTrail).accessDenied(anonymousUser, "indices:a", request, new String[] { role.getName() }, null); + verify(auditTrail).accessDenied(anonymousUser, "indices:a", request, new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); verify(clusterService, times(1)).state(); verify(state, times(1)).metaData(); @@ -560,7 +558,7 @@ public class AuthorizationServiceTests extends ESTestCase { ElasticsearchSecurityException securityException = expectThrows(ElasticsearchSecurityException.class, () -> authorize(createAuthentication(anonymousUser), "indices:a", request)); assertAuthenticationException(securityException, containsString("action [indices:a] requires authentication")); - verify(auditTrail).accessDenied(anonymousUser, "indices:a", request, new String[] { role.getName() }, null); + verify(auditTrail).accessDenied(anonymousUser, "indices:a", request, new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); verify(clusterService, times(1)).state(); verify(state, times(1)).metaData(); @@ -580,7 +578,7 @@ public class AuthorizationServiceTests extends ESTestCase { () -> authorize(createAuthentication(user), GetIndexAction.NAME, request)); assertThat(nfe.getIndex(), is(notNullValue())); assertThat(nfe.getIndex().getName(), is("not-an-index-*")); - verify(auditTrail).accessDenied(user, GetIndexAction.NAME, request, new String[]{ role.getName() }, null); + verify(auditTrail).accessDenied(user, GetIndexAction.NAME, request, new String[]{ role.getName() }); verifyNoMoreInteractions(auditTrail); verify(clusterService).state(); verify(state, times(1)).metaData(); @@ -658,9 +656,9 @@ public class AuthorizationServiceTests extends ESTestCase { "indices:a", "test user", "run as me"); verify(auditTrail).runAsGranted(user, "indices:a", request, new String[] { runAsRole.getName() }); if (indexExists) { - verify(auditTrail).accessDenied(user, "indices:a", request, new String[] { bRole.getName() }, null); + verify(auditTrail).accessDenied(user, "indices:a", request, new String[] { bRole.getName() }); } else { - verify(auditTrail).accessDenied(user, "indices:a", request, Role.EMPTY.names(), null); + verify(auditTrail).accessDenied(user, "indices:a", request, Role.EMPTY.names()); } verifyNoMoreInteractions(auditTrail); } @@ -687,7 +685,7 @@ public class AuthorizationServiceTests extends ESTestCase { authorize(createAuthentication(user), "indices:a", request); verify(auditTrail).runAsGranted(user, "indices:a", request, new String[] { runAsRole.getName() }); - verify(auditTrail).accessGranted(user, "indices:a", request, new String[] { bRole.getName() }, null); + verify(auditTrail).accessGranted(user, "indices:a", request, new String[] { bRole.getName() }); verifyNoMoreInteractions(auditTrail); } @@ -725,19 +723,19 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), action, request), action, "all_access_user"); - verify(auditTrail).accessDenied(user, action, request, new String[] { role.getName() }, null); + verify(auditTrail).accessDenied(user, action, request, new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); } // we should allow waiting for the health of the index or any index if the user has this permission TransportRequest request = new ClusterHealthRequest(SecurityLifecycleService.SECURITY_INDEX_NAME); authorize(createAuthentication(user), ClusterHealthAction.NAME, request); - verify(auditTrail).accessGranted(user, ClusterHealthAction.NAME, request, new String[] { role.getName() }, null); + verify(auditTrail).accessGranted(user, ClusterHealthAction.NAME, request, new String[] { role.getName() }); // multiple indices request = new ClusterHealthRequest(SecurityLifecycleService.SECURITY_INDEX_NAME, "foo", "bar"); authorize(createAuthentication(user), ClusterHealthAction.NAME, request); - verify(auditTrail).accessGranted(user, ClusterHealthAction.NAME, request, new String[] { role.getName() }, null); + verify(auditTrail).accessGranted(user, ClusterHealthAction.NAME, request, new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); SearchRequest searchRequest = new SearchRequest("_all"); @@ -774,7 +772,7 @@ public class AuthorizationServiceTests extends ESTestCase { String action = requestTuple.v1(); TransportRequest request = requestTuple.v2(); authorize(createAuthentication(user), action, request); - verify(auditTrail).accessGranted(user, action, request, new String[] { role.getName() }, null); + verify(auditTrail).accessGranted(user, action, request, new String[] { role.getName() }); } } @@ -813,7 +811,7 @@ public class AuthorizationServiceTests extends ESTestCase { String action = requestTuple.v1(); TransportRequest request = requestTuple.v2(); authorize(createAuthentication(superuser), action, request); - verify(auditTrail).accessGranted(superuser, action, request, superuser.roles(), null); + verify(auditTrail).accessGranted(superuser, action, request, superuser.roles()); } } @@ -831,7 +829,7 @@ public class AuthorizationServiceTests extends ESTestCase { String action = SearchAction.NAME; SearchRequest request = new SearchRequest("_all"); authorize(createAuthentication(superuser), action, request); - verify(auditTrail).accessGranted(superuser, action, request, superuser.roles(), null); + verify(auditTrail).accessGranted(superuser, action, request, superuser.roles()); assertThat(request.indices(), arrayContaining(".security")); } @@ -886,7 +884,7 @@ public class AuthorizationServiceTests extends ESTestCase { roleMap.put("no_indices", role); assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), action, request), action, "test user"); - verify(auditTrail).accessDenied(user, action, request, new String[] { role.getName() }, null); + verify(auditTrail).accessDenied(user, action, request, new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); } @@ -901,7 +899,7 @@ public class AuthorizationServiceTests extends ESTestCase { null); roleMap.put("role", role); authorize(createAuthentication(user), action, request); - verify(auditTrail).accessGranted(user, action, request, new String[] { role.getName() }, null); + verify(auditTrail).accessGranted(user, action, request, new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); } @@ -984,9 +982,9 @@ public class AuthorizationServiceTests extends ESTestCase { mockEmptyMetaData(); authorize(createAuthentication(user), action, request); - verify(auditTrail).accessDenied(user, DeleteAction.NAME, request, new String[] { role.getName() }, null); // alias-1 delete - verify(auditTrail).accessDenied(user, IndexAction.NAME, request, new String[] { role.getName() }, null); // alias-2 index - verify(auditTrail).accessGranted(user, action, request, new String[] { role.getName() }, null); // bulk request is allowed + verify(auditTrail).accessDenied(user, DeleteAction.NAME, request, new String[] { role.getName() }); // alias-1 delete + verify(auditTrail).accessDenied(user, IndexAction.NAME, request, new String[] { role.getName() }); // alias-2 index + verify(auditTrail).accessGranted(user, action, request, new String[] { role.getName() }); // bulk request is allowed verifyNoMoreInteractions(auditTrail); } @@ -1009,10 +1007,10 @@ public class AuthorizationServiceTests extends ESTestCase { mockEmptyMetaData(); authorize(createAuthentication(user), action, request); - verify(auditTrail, Mockito.times(2)).accessDenied(user, DeleteAction.NAME, request, new String[] { role.getName() }, null); // both + verify(auditTrail, Mockito.times(2)).accessDenied(user, DeleteAction.NAME, request, new String[] { role.getName() }); // both // deletes // should fail - verify(auditTrail).accessGranted(user, action, request, new String[] { role.getName() }, null); // bulk request is allowed + verify(auditTrail).accessGranted(user, action, request, new String[] { role.getName() }); // bulk request is allowed verifyNoMoreInteractions(auditTrail); } @@ -1242,7 +1240,7 @@ public class AuthorizationServiceTests extends ESTestCase { roleMap.put("no_indices", role); assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), action, transportRequest), action, "test user"); - verify(auditTrail).accessDenied(user, action, proxiedRequest, new String[] { role.getName() }, null); + verify(auditTrail).accessDenied(user, action, proxiedRequest, new String[] { role.getName() }); verifyNoMoreInteractions(auditTrail); } @@ -1258,7 +1256,7 @@ public class AuthorizationServiceTests extends ESTestCase { TransportRequest transportRequest = TransportActionProxy.wrapRequest(node, clearScrollRequest); String action = TransportActionProxy.getProxyAction(SearchTransportService.CLEAR_SCROLL_CONTEXTS_ACTION_NAME); authorize(createAuthentication(user), action, transportRequest); - verify(auditTrail).accessGranted(user, action, clearScrollRequest, new String[] { role.getName() }, null); + verify(auditTrail).accessGranted(user, action, clearScrollRequest, new String[] { role.getName() }); } public void testProxyRequestAuthenticationGranted() { @@ -1273,7 +1271,7 @@ public class AuthorizationServiceTests extends ESTestCase { TransportRequest transportRequest = TransportActionProxy.wrapRequest(node, clearScrollRequest); String action = TransportActionProxy.getProxyAction(SearchTransportService.CLEAR_SCROLL_CONTEXTS_ACTION_NAME); authorize(createAuthentication(user), action, transportRequest); - verify(auditTrail).accessGranted(user, action, clearScrollRequest, new String[] { role.getName() }, null); + verify(auditTrail).accessGranted(user, action, clearScrollRequest, new String[] { role.getName() }); } public void testProxyRequestAuthenticationDeniedWithReadPrivileges() { @@ -1288,6 +1286,6 @@ public class AuthorizationServiceTests extends ESTestCase { String action = TransportActionProxy.getProxyAction(SearchTransportService.CLEAR_SCROLL_CONTEXTS_ACTION_NAME); assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), action, transportRequest), action, "test user"); - verify(auditTrail).accessDenied(user, action, clearScrollRequest, new String[] { role.getName() }, null); + verify(auditTrail).accessDenied(user, action, clearScrollRequest, new String[] { role.getName() }); } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListenerTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListenerTests.java index da5f9efdeb9..10d13da16fb 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListenerTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListenerTests.java @@ -117,7 +117,7 @@ public class SecuritySearchOperationListenerTests extends ESTestCase { expectThrows(SearchContextMissingException.class, () -> listener.validateSearchContext(testSearchContext, request)); assertEquals(testSearchContext.id(), expected.id()); verify(licenseState, times(3)).isAuthAllowed(); - verify(auditTrailService).accessDenied(authentication.getUser(), "action", request, authentication.getUser().roles(), null); + verify(auditTrailService).accessDenied(authentication.getUser(), "action", request, authentication.getUser().roles()); } // another user running as the original user @@ -151,7 +151,7 @@ public class SecuritySearchOperationListenerTests extends ESTestCase { expectThrows(SearchContextMissingException.class, () -> listener.validateSearchContext(testSearchContext, request)); assertEquals(testSearchContext.id(), expected.id()); verify(licenseState, times(5)).isAuthAllowed(); - verify(auditTrailService).accessDenied(authentication.getUser(), "action", request, authentication.getUser().roles(), null); + verify(auditTrailService).accessDenied(authentication.getUser(), "action", request, authentication.getUser().roles()); } } @@ -188,7 +188,7 @@ public class SecuritySearchOperationListenerTests extends ESTestCase { () -> ensureAuthenticatedUserIsSame(original, differentRealmType, auditTrail, id, action, request, original.getUser().roles())); assertEquals(id, e.id()); - verify(auditTrail).accessDenied(differentRealmType.getUser(), action, request, original.getUser().roles(), null); + verify(auditTrail).accessDenied(differentRealmType.getUser(), action, request, original.getUser().roles()); // wrong user Authentication differentUser = @@ -196,7 +196,7 @@ public class SecuritySearchOperationListenerTests extends ESTestCase { e = expectThrows(SearchContextMissingException.class, () -> ensureAuthenticatedUserIsSame(original, differentUser, auditTrail, id, action, request, original.getUser().roles())); assertEquals(id, e.id()); - verify(auditTrail).accessDenied(differentUser.getUser(), action, request, original.getUser().roles(), null); + verify(auditTrail).accessDenied(differentUser.getUser(), action, request, original.getUser().roles()); // run as different user Authentication diffRunAs = new Authentication(new User(new User("test2", "role"), new User("authenticated", "runas")), @@ -204,7 +204,7 @@ public class SecuritySearchOperationListenerTests extends ESTestCase { e = expectThrows(SearchContextMissingException.class, () -> ensureAuthenticatedUserIsSame(original, diffRunAs, auditTrail, id, action, request, original.getUser().roles())); assertEquals(id, e.id()); - verify(auditTrail).accessDenied(diffRunAs.getUser(), action, request, original.getUser().roles(), null); + verify(auditTrail).accessDenied(diffRunAs.getUser(), action, request, original.getUser().roles()); // run as different looked up by type Authentication runAsDiffType = new Authentication(user, new RealmRef("realm", "file", "node"), @@ -212,7 +212,7 @@ public class SecuritySearchOperationListenerTests extends ESTestCase { e = expectThrows(SearchContextMissingException.class, () -> ensureAuthenticatedUserIsSame(runAs, runAsDiffType, auditTrail, id, action, request, original.getUser().roles())); assertEquals(id, e.id()); - verify(auditTrail).accessDenied(runAsDiffType.getUser(), action, request, original.getUser().roles(), null); + verify(auditTrail).accessDenied(runAsDiffType.getUser(), action, request, original.getUser().roles()); } static class TestScrollSearchContext extends TestSearchContext {