From 6d8c033ed4fcde2fcff3469e6813e26de97691f8 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 29 Jul 2021 14:06:04 +0200 Subject: [PATCH] Make Authentication/Authorization Stacks Shallower/Simpler (#75662) (#75833) Same as #75252 pretty much just continuing to make this logic a little simpler for easier profiling and (very) maybe performance through saving some allocations/indirection. --- .../authz/permission/ClusterPermission.java | 7 +- .../action/filter/SecurityActionFilter.java | 12 +-- .../security/authc/AuthenticationService.java | 101 ++++++++--------- .../xpack/security/authz/RBACEngine.java | 102 +++++++++--------- .../BulkShardRequestInterceptor.java | 6 +- ...cumentLevelSecurityRequestInterceptor.java | 3 +- .../authc/AuthenticationServiceTests.java | 33 +++--- 7 files changed, 122 insertions(+), 142 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java index cb3ac74d59746..adfafe17a4715 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java @@ -45,7 +45,12 @@ private ClusterPermission(final Set clusterPrivileges, * @return {@code true} if the access is allowed else returns {@code false} */ public boolean check(final String action, final TransportRequest request, final Authentication authentication) { - return checks.stream().anyMatch(permission -> permission.check(action, request, authentication)); + for (PermissionCheck permission : checks) { + if (permission.check(action, request, authentication)) { + return true; + } + } + return false; } /** diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java index 31f3d309b1264..60f1439d3b0d4 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java @@ -29,7 +29,6 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.XPackField; import org.elasticsearch.xpack.core.security.SecurityContext; -import org.elasticsearch.xpack.core.security.authc.Authentication; import org.elasticsearch.xpack.core.security.authz.privilege.HealthAndStatsPrivilege; import org.elasticsearch.xpack.core.security.support.Automatons; import org.elasticsearch.xpack.core.security.user.SystemUser; @@ -156,7 +155,7 @@ it to the action without an associated user (not via REST or transport - this is if (authc != null) { final String requestId = AuditUtil.extractRequestId(threadContext); assert Strings.hasText(requestId); - authorizeRequest(authc, securityAction, request, listener.delegateFailure( + authzService.authorize(authc, securityAction, request, listener.delegateFailure( (ll, aVoid) -> chain.proceed(task, action, request, ll.delegateFailure((l, response) -> { auditTrailService.get().coordinatingActionResponse(requestId, authc, action, request, response); @@ -169,13 +168,4 @@ it to the action without an associated user (not via REST or transport - this is } }, listener::onFailure)); } - - private void authorizeRequest(Authentication authentication, String securityAction, Request request, - ActionListener listener) { - if (authentication == null) { - listener.onFailure(new IllegalArgumentException("authentication must be non null for authorization")); - } else { - authzService.authorize(authentication, securityAction, request, listener); - } - } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java index c9391a13d0cb4..c43fde2cd1481 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java @@ -193,7 +193,7 @@ public void authenticate(String action, TransportRequest transportRequest, boole */ public void authenticate(String action, TransportRequest transportRequest, AuthenticationToken token, ActionListener listener) { - new Authenticator(action, transportRequest, shouldFallbackToAnonymous(true), listener).authenticateToken(token); + new Authenticator(action, transportRequest, shouldFallbackToAnonymous(true), listener).consumeToken(token); } public void expire(String principal) { @@ -314,9 +314,9 @@ private Authenticator(AuditableRequest auditableRequest, User fallbackUser, bool * these operations are: * *
    - *
  1. look for existing authentication {@link #lookForExistingAuthentication(Consumer)}
    2. + *
  2. look for existing authentication {@link #lookForExistingAuthentication()}
    3. *
  3. look for a user token
    4. - *
  4. token extraction {@link #extractToken(Consumer)}
    5. + *
  5. token extraction {@link #extractToken()}
    6. *
  6. token authentication {@link #consumeToken(AuthenticationToken)}
    7. *
  7. user lookup for run as if necessary {@link #consumeUser(User, Map)} and * {@link #lookupRunAsUser(User, String, Consumer)}
    8. @@ -330,14 +330,19 @@ private void authenticateAsync() { logger.debug("No realms available, failing authentication"); listener.onResponse(null); } else { - lookForExistingAuthentication((authentication) -> { - if (authentication != null) { - logger.trace("Found existing authentication [{}] in request [{}]", authentication, request); - listener.onResponse(authentication); - } else { - checkForBearerToken(); - } - }); + final Authentication authentication; + try { + authentication = lookForExistingAuthentication(); + } catch (Exception e) { + listener.onFailure(e); + return; + } + if (authentication != null) { + logger.trace("Found existing authentication [{}] in request [{}]", authentication, request); + listener.onResponse(authentication); + } else { + checkForBearerToken(); + } } } @@ -393,7 +398,14 @@ private void checkForApiKey() { logger.warn("Authentication using apikey failed - {}", authResult.getMessage()); } } - extractToken(this::consumeToken); + final AuthenticationToken token; + try { + token = extractToken(); + } catch (Exception e) { + listener.onFailure(e); + return; + } + consumeToken(token); } }, e -> listener.onFailure(request.exceptionProcessingRequest(e, null)))); @@ -404,25 +416,20 @@ private void checkForApiKey() { * consumer is called if no exception was thrown while trying to read the authentication and may be called with a {@code null} * value */ - private void lookForExistingAuthentication(Consumer authenticationConsumer) { - Runnable action; + private Authentication lookForExistingAuthentication() { + final Authentication authentication; try { - final Authentication authentication = authenticationSerializer.readFromContext(threadContext); - if (authentication != null && request instanceof AuditableRestRequest) { - action = () -> listener.onFailure(request.tamperedRequest()); - } else { - action = () -> authenticationConsumer.accept(authentication); - } + authentication = authenticationSerializer.readFromContext(threadContext); } catch (Exception e) { logger.error((Supplier) () -> new ParameterizedMessage("caught exception while trying to read authentication from request [{}]", request), e); - action = () -> listener.onFailure(request.tamperedRequest()); + throw request.tamperedRequest(); } - - // While we could place this call in the try block, the issue is that we catch all exceptions and could catch exceptions that - // have nothing to do with a tampered request. - action.run(); + if (authentication != null && request instanceof AuditableRestRequest) { + throw request.tamperedRequest(); + } + return authentication; } /** @@ -431,28 +438,26 @@ private void lookForExistingAuthentication(Consumer authenticati * no exception was caught during the extraction process and may be called with a {@code null} token. */ // pkg-private accessor testing token extraction with a consumer - void extractToken(Consumer consumer) { - Runnable action = () -> consumer.accept(null); + AuthenticationToken extractToken() { try { if (authenticationToken != null) { - action = () -> consumer.accept(authenticationToken); + return authenticationToken; } else { for (Realm realm : defaultOrderedRealmList) { final AuthenticationToken token = realm.token(threadContext); if (token != null) { logger.trace("Found authentication credentials [{}] for principal [{}] in request [{}]", token.getClass().getName(), token.principal(), request); - action = () -> consumer.accept(token); - break; + return token; } } } } catch (Exception e) { logger.warn("An exception occurred while attempting to find authentication credentials", e); - action = () -> listener.onFailure(request.exceptionProcessingRequest(e, null)); + throw request.exceptionProcessingRequest(e, null); } - action.run(); + return null; } /** @@ -606,19 +611,12 @@ void handleNullToken() { authentication = null; } - Runnable action; if (authentication != null) { - action = () -> writeAuthToContext(authentication); + writeAuthToContext(authentication); } else { - action = () -> { - logger.debug("No valid credentials found in request [{}], rejecting", request); - listener.onFailure(request.anonymousAccessDenied()); - }; + logger.debug("No valid credentials found in request [{}], rejecting", request); + listener.onFailure(request.anonymousAccessDenied()); } - - // we assign the listener call to an action to avoid calling the listener within a try block and auditing the wrong thing when - // an exception bubbles up even after successful authentication - action.run(); } /** @@ -712,10 +710,6 @@ void finishAuthentication(User finalUser) { * successful */ void writeAuthToContext(Authentication authentication) { - Runnable action = () -> { - logger.trace("Established authentication [{}] for request [{}]", authentication, request); - listener.onResponse(authentication); - }; try { authenticationSerializer.writeToContext(authentication, threadContext); request.authenticationSuccess(authentication); @@ -723,21 +717,16 @@ void writeAuthToContext(Authentication authentication) { // i.e. not read from either header or transient header operatorPrivilegesService.maybeMarkOperatorUser(authentication, threadContext); } catch (Exception e) { - action = () -> { - logger.debug( + logger.debug( new ParameterizedMessage("Failed to store authentication [{}] for request [{}]", authentication, request), e); - listener.onFailure(request.exceptionProcessingRequest(e, authenticationToken)); - }; + listener.onFailure(request.exceptionProcessingRequest(e, authenticationToken)); + return; } - // we assign the listener call to an action to avoid calling the listener within a try block and auditing the wrong thing - // when an exception bubbles up even after successful authentication - action.run(); + logger.trace("Established authentication [{}] for request [{}]", authentication, request); + listener.onResponse(authentication); } - private void authenticateToken(AuthenticationToken token) { - this.consumeToken(token); - } } abstract static class AuditableRequest { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java index 501de7b7cb789..f1cd094b28b79 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java @@ -16,7 +16,6 @@ import org.elasticsearch.action.ActionRunnable; import org.elasticsearch.action.CompositeIndicesRequest; import org.elasticsearch.action.IndicesRequest; -import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.action.bulk.BulkAction; import org.elasticsearch.action.bulk.BulkShardRequest; import org.elasticsearch.action.delete.DeleteAction; @@ -243,12 +242,15 @@ public void authorizeIndexAction(RequestInfo requestInfo, AuthorizationInfo auth ActionListener listener) { final String action = requestInfo.getAction(); final TransportRequest request = requestInfo.getRequest(); - final Authentication authentication = requestInfo.getAuthentication(); if (TransportActionProxy.isProxyAction(action) || shouldAuthorizeIndexActionNameOnly(action, request)) { // we've already validated that the request is a proxy request so we can skip that but we still // need to validate that the action is allowed and then move on - authorizeIndexActionName(action, authorizationInfo, null, listener); - } else if (request instanceof IndicesRequest == false && request instanceof IndicesAliasesRequest == false) { + try { + listener.onResponse(authorizeIndexActionName(action, authorizationInfo, null)); + } catch (Exception e) { + listener.onFailure(e); + } + } else if (request instanceof IndicesRequest == false) { if (isScrollRelatedAction(action)) { // scroll is special // some APIs are indices requests that are not actually associated with indices. For example, @@ -268,7 +270,7 @@ public void authorizeIndexAction(RequestInfo requestInfo, AuthorizationInfo auth ActionRunnable.supply( ActionListener.wrap(parsedScrollId -> { if (parsedScrollId.hasLocalIndices()) { - authorizeIndexActionName(action, authorizationInfo, null, listener); + listener.onResponse(authorizeIndexActionName(action, authorizationInfo, null)); } else { listener.onResponse(new IndexAuthorizationResult(true, null)); } @@ -305,7 +307,7 @@ public void authorizeIndexAction(RequestInfo requestInfo, AuthorizationInfo auth listener.onFailure(new IllegalStateException("only scroll and async-search related requests are known indices " + "api that don't support retrieving the indices they relate to")); } - } else if (request instanceof IndicesRequest && ((IndicesRequest) request).allowsRemoteIndices()) { + } else if (((IndicesRequest) request).allowsRemoteIndices()) { // remote indices are allowed indicesAsyncSupplier.getAsync(ActionListener.wrap(resolvedIndices -> { assert resolvedIndices.isEmpty() == false @@ -314,48 +316,44 @@ public void authorizeIndexAction(RequestInfo requestInfo, AuthorizationInfo auth //'-*' matches no indices so we allow the request to go through, which will yield an empty response if (resolvedIndices.isNoIndicesPlaceholder()) { // check action name - authorizeIndexActionName(action, authorizationInfo, IndicesAccessControl.ALLOW_NO_INDICES, listener); + listener.onResponse(authorizeIndexActionName(action, authorizationInfo, IndicesAccessControl.ALLOW_NO_INDICES)); } else { - buildIndicesAccessControl(authentication, action, authorizationInfo, - Sets.newHashSet(resolvedIndices.getLocal()), aliasOrIndexLookup, listener); + listener.onResponse(buildIndicesAccessControl( + action, authorizationInfo, Sets.newHashSet(resolvedIndices.getLocal()), aliasOrIndexLookup)); } }, listener::onFailure)); } else { - authorizeIndexActionName(action, authorizationInfo, IndicesAccessControl.ALLOW_NO_INDICES, - ActionListener.wrap(indexAuthorizationResult -> { - if (indexAuthorizationResult.isGranted()) { - indicesAsyncSupplier.getAsync(ActionListener.wrap(resolvedIndices -> { - assert resolvedIndices.isEmpty() == false + try { + final IndexAuthorizationResult indexAuthorizationResult = + authorizeIndexActionName(action, authorizationInfo, IndicesAccessControl.ALLOW_NO_INDICES); + if (indexAuthorizationResult.isGranted()) { + indicesAsyncSupplier.getAsync(ActionListener.wrap(resolvedIndices -> { + assert resolvedIndices.isEmpty() == false : "every indices request needs to have its indices set thus the resolved indices must not be empty"; - //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()) { - listener.onResponse(new IndexAuthorizationResult(true, IndicesAccessControl.ALLOW_NO_INDICES)); - } else { - buildIndicesAccessControl(authentication, action, authorizationInfo, - Sets.newHashSet(resolvedIndices.getLocal()), aliasOrIndexLookup, listener); - } - }, listener::onFailure)); - } else { - listener.onResponse(indexAuthorizationResult); - } - }, listener::onFailure)); + //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()) { + listener.onResponse(new IndexAuthorizationResult(true, IndicesAccessControl.ALLOW_NO_INDICES)); + } else { + listener.onResponse(buildIndicesAccessControl( + action, authorizationInfo, Sets.newHashSet(resolvedIndices.getLocal()), aliasOrIndexLookup)); + } + }, listener::onFailure)); + } else { + listener.onResponse(indexAuthorizationResult); + } + } catch (Exception e) { + listener.onFailure(e); + } } } - private void authorizeIndexActionName(String action, AuthorizationInfo authorizationInfo, IndicesAccessControl grantedValue, - ActionListener listener) { - if (authorizationInfo instanceof RBACAuthorizationInfo) { - final Role role = ((RBACAuthorizationInfo) authorizationInfo).getRole(); - if (role.checkIndicesAction(action)) { - listener.onResponse(new IndexAuthorizationResult(true, grantedValue)); - } else { - listener.onResponse(new IndexAuthorizationResult(true, IndicesAccessControl.DENIED)); - } - } else { - listener.onFailure(new IllegalArgumentException("unsupported authorization info:" + - authorizationInfo.getClass().getSimpleName())); - } + private static IndexAuthorizationResult authorizeIndexActionName(String action, + AuthorizationInfo authorizationInfo, + IndicesAccessControl grantedValue) { + final Role role = ensureRBAC(authorizationInfo).getRole(); + return new IndexAuthorizationResult(true, role.checkIndicesAction(action) ? grantedValue : IndicesAccessControl.DENIED); + } @Override @@ -558,18 +556,20 @@ static Set resolveAuthorizedIndicesFromRole(Role role, RequestInfo reque return Collections.unmodifiableSet(indicesAndAliases); } - private void buildIndicesAccessControl(Authentication authentication, String action, - AuthorizationInfo authorizationInfo, Set indices, - Map aliasAndIndexLookup, - ActionListener listener) { - if (authorizationInfo instanceof RBACAuthorizationInfo) { - final Role role = ((RBACAuthorizationInfo) authorizationInfo).getRole(); - final IndicesAccessControl accessControl = role.authorize(action, indices, aliasAndIndexLookup, fieldPermissionsCache); - listener.onResponse(new IndexAuthorizationResult(true, accessControl)); - } else { - listener.onFailure(new IllegalArgumentException("unsupported authorization info:" + - authorizationInfo.getClass().getSimpleName())); + private IndexAuthorizationResult buildIndicesAccessControl(String action, + AuthorizationInfo authorizationInfo, + Set indices, + Map aliasAndIndexLookup) { + final Role role = ensureRBAC(authorizationInfo).getRole(); + final IndicesAccessControl accessControl = role.authorize(action, indices, aliasAndIndexLookup, fieldPermissionsCache); + return new IndexAuthorizationResult(true, accessControl); + } + + private static RBACAuthorizationInfo ensureRBAC(AuthorizationInfo authorizationInfo) { + if (authorizationInfo instanceof RBACAuthorizationInfo == false) { + throw new IllegalArgumentException("unsupported authorization info:" + authorizationInfo.getClass().getSimpleName()); } + return (RBACAuthorizationInfo) authorizationInfo; } private static boolean checkChangePasswordAction(Authentication authentication) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/interceptor/BulkShardRequestInterceptor.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/interceptor/BulkShardRequestInterceptor.java index 5796332e6a038..961d8772dc402 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/interceptor/BulkShardRequestInterceptor.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/interceptor/BulkShardRequestInterceptor.java @@ -43,9 +43,7 @@ public BulkShardRequestInterceptor(ThreadPool threadPool, XPackLicenseState lice @Override public void intercept(RequestInfo requestInfo, AuthorizationEngine authzEngine, AuthorizationInfo authorizationInfo, ActionListener listener) { - boolean shouldIntercept = licenseState.isSecurityEnabled(); - MemoizedSupplier licenseChecker = new MemoizedSupplier<>(() -> licenseState.checkFeature(Feature.SECURITY_DLS_FLS)); - if (requestInfo.getRequest() instanceof BulkShardRequest && shouldIntercept) { + if (requestInfo.getRequest() instanceof BulkShardRequest && licenseState.isSecurityEnabled()) { IndicesAccessControl indicesAccessControl = threadContext.getTransient(AuthorizationServiceField.INDICES_PERMISSIONS_KEY); BulkShardRequest bulkShardRequest = (BulkShardRequest) requestInfo.getRequest(); // this uses the {@code BulkShardRequest#index()} because the {@code bulkItemRequest#index()} @@ -53,6 +51,8 @@ public void intercept(RequestInfo requestInfo, AuthorizationEngine authzEngine, IndicesAccessControl.IndexAccessControl indexAccessControl = indicesAccessControl.getIndexPermissions(bulkShardRequest.index()); // TODO replace if condition with assertion if (indexAccessControl != null) { + MemoizedSupplier licenseChecker = + new MemoizedSupplier<>(() -> licenseState.checkFeature(Feature.SECURITY_DLS_FLS)); for (BulkItemRequest bulkItemRequest : bulkShardRequest.items()) { boolean found = false; if (bulkItemRequest.request() instanceof UpdateRequest) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/interceptor/FieldAndDocumentLevelSecurityRequestInterceptor.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/interceptor/FieldAndDocumentLevelSecurityRequestInterceptor.java index d246f9696009b..22aff0c3209df 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/interceptor/FieldAndDocumentLevelSecurityRequestInterceptor.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/interceptor/FieldAndDocumentLevelSecurityRequestInterceptor.java @@ -46,8 +46,7 @@ public void intercept(RequestInfo requestInfo, AuthorizationEngine authorization if (requestInfo.getRequest() instanceof IndicesRequest && false == TransportActionProxy.isProxyAction(requestInfo.getAction())) { IndicesRequest indicesRequest = (IndicesRequest) requestInfo.getRequest(); // TODO: should we check is DLS/FLS feature allowed here as part of shouldIntercept - boolean shouldIntercept = licenseState.isSecurityEnabled(); - if (supports(indicesRequest) && shouldIntercept) { + if (supports(indicesRequest) && licenseState.isSecurityEnabled()) { final MemoizedSupplier licenseChecker = new MemoizedSupplier<>(() -> licenseState.checkFeature(Feature.SECURITY_DLS_FLS)); final IndicesAccessControl indicesAccessControl diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java index 081877e9629b4..c158c55bfd114 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java @@ -299,11 +299,10 @@ public void testTokenFirstMissingSecondFound() throws Exception { PlainActionFuture future = new PlainActionFuture<>(); Authenticator authenticator = service.createAuthenticator("_action", transportRequest, true, future); - authenticator.extractToken((result) -> { - assertThat(result, notNullValue()); - assertThat(result, is(token)); - verifyZeroInteractions(auditTrail); - }); + AuthenticationToken result = authenticator.extractToken(); + assertThat(result, notNullValue()); + assertThat(result, is(token)); + verifyZeroInteractions(auditTrail); } public void testTokenMissing() throws Exception { @@ -328,15 +327,14 @@ public void testTokenMissing() throws Exception { } PlainActionFuture future = new PlainActionFuture<>(); Authenticator authenticator = service.createAuthenticator("_action", transportRequest, true, future); - authenticator.extractToken((token) -> { - assertThat(token, nullValue()); - if (requestIdAlreadyPresent) { - assertThat(expectAuditRequestId(threadContext), is(reqId.get())); - } else { - reqId.set(expectAuditRequestId(threadContext)); - } - authenticator.handleNullToken(); - }); + AuthenticationToken token = authenticator.extractToken(); + assertThat(token, nullValue()); + if (requestIdAlreadyPresent) { + assertThat(expectAuditRequestId(threadContext), is(reqId.get())); + } else { + reqId.set(expectAuditRequestId(threadContext)); + } + authenticator.handleNullToken(); ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, () -> future.actionGet()); assertThat(e.getMessage(), containsString("missing authentication credentials")); @@ -656,10 +654,9 @@ public void testTokenRestMissing() throws Exception { @SuppressWarnings("unchecked") Authenticator authenticator = service.createAuthenticator(restRequest, true, mock(ActionListener.class)); - authenticator.extractToken((token) -> { - expectAuditRequestId(threadContext); - assertThat(token, nullValue()); - }); + AuthenticationToken token = authenticator.extractToken(); + expectAuditRequestId(threadContext); + assertThat(token, nullValue()); } public void testAuthenticationInContextAndHeader() throws Exception {