Skip to content

Commit 7bab8dd

Browse files
authored
Additional DEBUG logging on authc failures (#137941)
Adds two sets of DEBUG logging for authentication failures 1. In Pluggable authentication chain, log the failure reason if authentication is terminated. Without this the actual failure reason is lost becauser we return a generic error to the client. 2. For all authentication failures, add a general debug log in `AuthenticatorChain` so that we know that every authentication failure is logged somewhere, even if the specific chain handler doesn't log it. We also add a trace log for successful authentication
1 parent 263c178 commit 7bab8dd

File tree

6 files changed

+107
-11
lines changed

6 files changed

+107
-11
lines changed

docs/changelog/137941.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 137941
2+
summary: Additional DEBUG logging on authc failures
3+
area: Authentication
4+
type: enhancement
5+
issues: []

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.io.IOException;
2424
import java.util.ArrayList;
2525
import java.util.List;
26+
import java.util.stream.Collectors;
2627

2728
/**
2829
* The Authenticator interface represents an authentication mechanism or a group of similar authentication mechanisms.
@@ -241,5 +242,17 @@ public void addUnsuccessfulMessageToMetadata(final ElasticsearchSecurityExceptio
241242
ese.addMetadata("es.additional_unsuccessful_credentials", getUnsuccessfulMessages());
242243
}
243244
}
245+
246+
@Override
247+
public String toString() {
248+
return Strings.format(
249+
"%s{tokens=%s, messages=%s}",
250+
getClass().getSimpleName(),
251+
this.authenticationTokens.stream()
252+
.map(t -> t.getClass().getSimpleName() + ":" + t.principal())
253+
.collect(Collectors.joining(",", "[", "]")),
254+
this.unsuccessfulMessages
255+
);
256+
}
244257
}
245258
}

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.action.ActionListener;
1414
import org.elasticsearch.common.settings.Settings;
1515
import org.elasticsearch.common.util.concurrent.ThreadContext;
16+
import org.elasticsearch.core.Strings;
1617
import org.elasticsearch.node.Node;
1718
import org.elasticsearch.xpack.core.common.IteratingActionListener;
1819
import org.elasticsearch.xpack.core.security.authc.Authentication;
@@ -83,11 +84,16 @@ void authenticate(Authenticator.Context context, ActionListener<Authentication>
8384
assert false == context.getDefaultOrderedRealmList().isEmpty() : "realm list must not be empty";
8485
// Check whether authentication is an operator user and mark the threadContext if necessary
8586
// before returning the authentication object
86-
final ActionListener<Authentication> listener = originalListener.map(authentication -> {
87+
final ActionListener<Authentication> listener = ActionListener.wrap(authentication -> {
8788
assert authentication != null;
8889
operatorPrivilegesService.maybeMarkOperatorUser(authentication, context.getThreadContext());
89-
return authentication;
90+
logger.trace(() -> Strings.format("Authentication for [%s]", authentication));
91+
originalListener.onResponse(authentication);
92+
}, ex -> {
93+
logger.debug(() -> Strings.format("Authentication for context [%s] failed", context), ex);
94+
originalListener.onFailure(ex);
9095
});
96+
9197
// If a token is directly provided in the context, authenticate with it
9298
if (context.getMostRecentAuthenticationToken() != null) {
9399
doAuthenticate(context, listener);

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,14 @@ private BiConsumer<CustomAuthenticator, ActionListener<AuthenticationResult<Auth
9696
iteratingListener.onResponse(response);
9797
} else if (response.getStatus() == AuthenticationResult.Status.TERMINATE) {
9898
final Exception ex = response.getException();
99+
logger.debug(
100+
() -> format(
101+
"Authentication of token [%s] was terminated: %s (caused by: %s)",
102+
token.principal(),
103+
response.getMessage(),
104+
ex
105+
)
106+
);
99107
if (ex == null) {
100108
iteratingListener.onFailure(context.getRequest().authenticationFailed(token));
101109
} else {

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.io.IOException;
4141
import java.util.Collections;
4242
import java.util.List;
43+
import java.util.regex.Pattern;
4344

4445
import static org.elasticsearch.test.ActionListenerUtils.anyActionListener;
4546
import static org.hamcrest.Matchers.containsString;
@@ -321,7 +322,9 @@ public void testAuthenticateFallbackAndAnonymous() throws IOException {
321322
}
322323

323324
public void testContextWithDirectWrongTokenFailsAuthn() {
324-
final Authenticator.Context context = createAuthenticatorContext(mock(AuthenticationToken.class));
325+
final AuthenticationToken token = mock(AuthenticationToken.class);
326+
when(token.principal()).thenReturn("MOCK_USER");
327+
final Authenticator.Context context = createAuthenticatorContext(token);
325328
doCallRealMethod().when(serviceAccountAuthenticator).authenticate(eq(context), anyActionListener());
326329
doCallRealMethod().when(oAuth2TokenAuthenticator).authenticate(eq(context), anyActionListener());
327330
doCallRealMethod().when(apiKeyAuthenticator).authenticate(eq(context), anyActionListener());
@@ -335,6 +338,20 @@ public void testContextWithDirectWrongTokenFailsAuthn() {
335338
return null;
336339
}).when(realmsAuthenticator).authenticate(eq(context), any());
337340
final PlainActionFuture<Authentication> future = new PlainActionFuture<>();
341+
342+
Loggers.setLevel(LogManager.getLogger(AuthenticatorChain.class), Level.DEBUG);
343+
final MockLog mockLog = MockLog.capture(AuthenticatorChain.class);
344+
mockLog.addExpectation(
345+
new MockLog.PatternSeenEventExpectation(
346+
"debug-failure",
347+
AuthenticatorChain.class.getName(),
348+
Level.DEBUG,
349+
Pattern.quote("Authentication for context [Context{tokens=[")
350+
+ "AuthenticationToken\\$.*:MOCK_USER"
351+
+ Pattern.quote("], messages=[]}] failed")
352+
)
353+
);
354+
338355
authenticatorChain.authenticate(context, future);
339356
final ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, future::actionGet);
340357
assertThat(e.getMessage(), containsString("failed to authenticate"));
@@ -345,6 +362,8 @@ public void testContextWithDirectWrongTokenFailsAuthn() {
345362
verify(realmsAuthenticator, never()).extractCredentials(any());
346363
verifyNoMoreInteractions(authenticationContextSerializer);
347364
verifyNoMoreInteractions(operatorPrivilegesService);
365+
mockLog.assertAllExpectationsMatched();
366+
348367
// OR 2. realms fail the token
349368
doAnswer(invocationOnMock -> {
350369
@SuppressWarnings("unchecked")

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/PluggableAuthenticatorChainTests.java

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,32 @@
66
*/
77
package org.elasticsearch.xpack.security.authc;
88

9+
import org.apache.logging.log4j.Level;
10+
import org.apache.logging.log4j.LogManager;
11+
import org.elasticsearch.ElasticsearchSecurityException;
912
import org.elasticsearch.action.ActionListener;
13+
import org.elasticsearch.action.support.PlainActionFuture;
14+
import org.elasticsearch.common.logging.Loggers;
1015
import org.elasticsearch.common.settings.Settings;
1116
import org.elasticsearch.common.util.concurrent.ThreadContext;
1217
import org.elasticsearch.core.Nullable;
18+
import org.elasticsearch.rest.RestStatus;
1319
import org.elasticsearch.test.ESTestCase;
20+
import org.elasticsearch.test.MockLog;
1421
import org.elasticsearch.xpack.core.security.authc.Authentication;
1522
import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
1623
import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper;
1724
import org.elasticsearch.xpack.core.security.authc.AuthenticationToken;
1825
import org.elasticsearch.xpack.core.security.authc.CustomAuthenticator;
1926
import org.elasticsearch.xpack.core.security.user.User;
2027
import org.junit.Before;
28+
import org.mockito.Mockito;
2129

2230
import java.util.List;
2331
import java.util.concurrent.CountDownLatch;
2432
import java.util.concurrent.atomic.AtomicReference;
2533

34+
import static org.elasticsearch.test.TestMatchers.throwableWithMessage;
2635
import static org.hamcrest.Matchers.equalTo;
2736
import static org.hamcrest.Matchers.notNullValue;
2837

@@ -87,13 +96,15 @@ public String getValue() {
8796
public class TokenAAuthenticator implements CustomAuthenticator {
8897

8998
private final String id;
99+
private boolean succeed;
90100

91101
public TokenAAuthenticator() {
92-
id = "1";
102+
this("1", true);
93103
}
94104

95-
public TokenAAuthenticator(String id) {
105+
public TokenAAuthenticator(String id, boolean succeed) {
96106
this.id = id;
107+
this.succeed = succeed;
97108
}
98109

99110
@Override
@@ -109,9 +120,13 @@ public boolean supports(AuthenticationToken token) {
109120
@Override
110121
public void authenticate(@Nullable AuthenticationToken token, ActionListener<AuthenticationResult<Authentication>> listener) {
111122
if (token instanceof TestTokenA testToken) {
112-
User user = new User("token-a-auth-user-" + id + "-" + testToken.getValue());
113-
Authentication auth = AuthenticationTestHelper.builder().user(user).build(false);
114-
listener.onResponse(AuthenticationResult.success(auth));
123+
if (succeed) {
124+
User user = new User("token-a-auth-user-" + id + "-" + testToken.getValue());
125+
Authentication auth = AuthenticationTestHelper.builder().user(user).build(false);
126+
listener.onResponse(AuthenticationResult.success(auth));
127+
} else {
128+
listener.onResponse(AuthenticationResult.terminate("token-a-fail-" + id + "-" + testToken.getValue()));
129+
}
115130
} else {
116131
listener.onResponse(AuthenticationResult.notHandled());
117132
}
@@ -243,7 +258,7 @@ public void onFailure(Exception e) {
243258
public void testAuthenticateWhenTokenSupportedByBothAuthenticatorsInChain() throws Exception {
244259

245260
PluggableAuthenticatorChain chain = new PluggableAuthenticatorChain(
246-
List.of(new TokenAAuthenticator("foo"), new TokenAAuthenticator("bar"))
261+
List.of(new TokenAAuthenticator("foo", true), new TokenAAuthenticator("bar", true))
247262
);
248263
TestTokenA testToken = new TestTokenA("test-value");
249264

@@ -286,7 +301,7 @@ public void onFailure(Exception e) {
286301
public void testAuthenticateWhenTokenSupportedByNoAuthenticatorsInChain() throws Exception {
287302

288303
PluggableAuthenticatorChain chain = new PluggableAuthenticatorChain(
289-
List.of(new TokenAAuthenticator("foo"), new TokenAAuthenticator("bar"))
304+
List.of(new TokenAAuthenticator("foo", true), new TokenAAuthenticator("bar", true))
290305
);
291306
AuthenticationToken unknownToken = new AuthenticationToken() {
292307
@Override
@@ -338,7 +353,37 @@ public void onFailure(Exception e) {
338353
assertThat(result.getStatus(), equalTo(AuthenticationResult.Status.CONTINUE));
339354
}
340355

356+
public void testAuthenticationTermination() throws Exception {
357+
final PluggableAuthenticatorChain chain = new PluggableAuthenticatorChain(List.of(new TokenAAuthenticator("terminate", false)));
358+
final TestTokenA token = new TestTokenA("err");
359+
final Authenticator.Context context = createContext();
360+
context.addAuthenticationToken(token);
361+
362+
Loggers.setLevel(LogManager.getLogger(PluggableAuthenticatorChain.class), Level.DEBUG);
363+
try (MockLog mockLog = MockLog.capture(PluggableAuthenticatorChain.class)) {
364+
mockLog.addExpectation(
365+
new MockLog.SeenEventExpectation(
366+
"debug-auth-failure",
367+
PluggableAuthenticatorChain.class.getName(),
368+
Level.DEBUG,
369+
"Authentication of token [user-err] was terminated: token-a-fail-terminate-err (caused by: null)"
370+
)
371+
);
372+
373+
final PlainActionFuture<AuthenticationResult<Authentication>> future = new PlainActionFuture<>();
374+
chain.authenticate(context, future);
375+
mockLog.assertAllExpectationsMatched();
376+
377+
final ElasticsearchSecurityException ex = expectThrows(ElasticsearchSecurityException.class, () -> future.actionGet());
378+
assertThat(ex, throwableWithMessage("mock-request-failure"));
379+
assertThat(ex.status(), equalTo(RestStatus.UNAUTHORIZED));
380+
}
381+
}
382+
341383
private Authenticator.Context createContext() {
342-
return new Authenticator.Context(threadContext, null, null, true, null);
384+
final var request = Mockito.mock(AuthenticationService.AuditableRequest.class);
385+
Mockito.when(request.authenticationFailed(Mockito.any(AuthenticationToken.class)))
386+
.thenAnswer(inv -> new ElasticsearchSecurityException("mock-request-failure", RestStatus.UNAUTHORIZED));
387+
return new Authenticator.Context(threadContext, request, null, true, null);
343388
}
344389
}

0 commit comments

Comments
 (0)