Skip to content

Commit c71060f

Browse files
committed
Test: fix race in auth result propagation test
This commit fixes a race condition in a test introduced by #36900 that verifies concurrent authentications get a result propagated from the first thread that attempts to authenticate. Previously, a thread may be in a state where it had not attempted to authenticate when the first thread that authenticates finishes the authentication, which would cause the test to fail as there would be an additional authentication attempt. This change adds additional latches to ensure all threads have attempted to authenticate before a result gets returned in the thread that is performing authentication.
1 parent 722b850 commit c71060f

File tree

1 file changed

+20
-2
lines changed

1 file changed

+20
-2
lines changed

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -484,13 +484,27 @@ public void testUnauthenticatedResultPropagatesWithSameCreds() throws Exception
484484

485485
final int numberOfProcessors = Runtime.getRuntime().availableProcessors();
486486
final int numberOfThreads = scaledRandomIntBetween((numberOfProcessors + 1) / 2, numberOfProcessors * 3);
487-
final CountDownLatch latch = new CountDownLatch(1 + numberOfThreads);
488487
List<Thread> threads = new ArrayList<>(numberOfThreads);
489488
final SecureString credsToUse = new SecureString(randomAlphaOfLength(12).toCharArray());
489+
490+
// we use a bunch of different latches here, the first `latch` is used to ensure all threads have been started
491+
// before they start to execute. The `authWaitLatch` is there to ensure we have all threads waiting on the
492+
// listener before we auth otherwise we may run into a race condition where we auth and one of the threads is
493+
// not waiting on auth yet. Finally, the completedLatch is used to signal that each thread received a response!
494+
final CountDownLatch latch = new CountDownLatch(1 + numberOfThreads);
495+
final CountDownLatch authWaitLatch = new CountDownLatch(numberOfThreads);
496+
final CountDownLatch completedLatch = new CountDownLatch(numberOfThreads);
490497
final CachingUsernamePasswordRealm realm = new CachingUsernamePasswordRealm(config, threadPool) {
491498
@Override
492499
protected void doAuthenticate(UsernamePasswordToken token, ActionListener<AuthenticationResult> listener) {
493500
authCounter.incrementAndGet();
501+
authWaitLatch.countDown();
502+
try {
503+
authWaitLatch.await();
504+
} catch (InterruptedException e) {
505+
logger.info("authentication was interrupted", e);
506+
Thread.currentThread().interrupt();
507+
}
494508
// do something slow
495509
if (pwdHasher.verify(token.credentials(), passwordHash.toCharArray())) {
496510
listener.onFailure(new IllegalStateException("password auth should never succeed"));
@@ -513,14 +527,17 @@ protected void doLookupUser(String username, ActionListener<User> listener) {
513527

514528
realm.authenticate(token, ActionListener.wrap((result) -> {
515529
if (result.isAuthenticated()) {
530+
completedLatch.countDown();
516531
throw new IllegalStateException("invalid password led to an authenticated result: " + result);
517532
}
518533
assertThat(result.getMessage(), containsString("password verification failed"));
534+
completedLatch.countDown();
519535
}, (e) -> {
520536
logger.error("caught exception", e);
537+
completedLatch.countDown();
521538
fail("unexpected exception - " + e);
522539
}));
523-
540+
authWaitLatch.countDown();
524541
} catch (InterruptedException e) {
525542
logger.error("thread was interrupted", e);
526543
Thread.currentThread().interrupt();
@@ -535,6 +552,7 @@ protected void doLookupUser(String username, ActionListener<User> listener) {
535552
for (Thread thread : threads) {
536553
thread.join();
537554
}
555+
completedLatch.await();
538556
assertEquals(1, authCounter.get());
539557
}
540558

0 commit comments

Comments
 (0)