IGNITE-23100 Improve semantic of authenticateAsync (#4306)

modules/security/src/main/java/org/apache/ignite/internal/security/authentication/AuthenticationManagerImpl.java

@@ -180,34 +180,27 @@ private CompletableFuture<UserDetails> authenticate(
             AuthenticationRequest<?, ?> authenticationRequest
     ) {
         try {
-            var fut = authenticator.authenticateAsync(authenticationRequest);
-
-            fut.thenAccept(userDetails -> {
-                if (userDetails != null) {
-                    logUserAuthenticated(userDetails);
-                }
-            });
-
-            return fut.handle((userDetails, throwable) -> {
-                if (throwable != null) {
-                    if (!(throwable instanceof InvalidCredentialsException
-                            || throwable instanceof UnsupportedAuthenticationTypeException)) {
-                        LOG.error("Unexpected exception during authentication", throwable);
-                    }
-
-                    logAuthenticationFailure(authenticationRequest);
-                    return null;
-                }
-
-                return userDetails;
-            });
-        } catch (InvalidCredentialsException | UnsupportedAuthenticationTypeException exception) {
-            logAuthenticationFailure(authenticationRequest);
-            return completedFuture(null);
+            return authenticator.authenticateAsync(authenticationRequest)
+                    .handle((userDetails, throwable) -> {
+                        if (throwable != null) {
+                            if (!(throwable instanceof InvalidCredentialsException
+                                    || throwable instanceof UnsupportedAuthenticationTypeException)) {
+                                LOG.error("Unexpected exception during authentication", throwable);
+                            }
+
+                            logAuthenticationFailure(authenticationRequest);
+                            return null;
+                        }
+
+                        if (userDetails != null) {
+                            logUserAuthenticated(userDetails);
+                        }
+                        return userDetails;
+                    });
         } catch (Exception e) {
             logAuthenticationFailure(authenticationRequest);
             LOG.error("Unexpected exception during authentication", e);
-            return completedFuture(null);
+            return nullCompletedFuture();
         }
     }
 

modules/security/src/main/java/org/apache/ignite/internal/security/authentication/basic/BasicAuthenticator.java

@@ -18,6 +18,7 @@
 package org.apache.ignite.internal.security.authentication.basic;
 
 import static java.util.concurrent.CompletableFuture.completedFuture;
+import static java.util.concurrent.CompletableFuture.failedFuture;
 import static java.util.function.Function.identity;
 
 import java.util.List;
@@ -51,9 +52,9 @@ public BasicAuthenticator(String providerName, List<BasicUser> users) {
     @Override
     public CompletableFuture<UserDetails> authenticateAsync(AuthenticationRequest<?, ?> authenticationRequest) {
         if (!(authenticationRequest instanceof UsernamePasswordRequest)) {
-            throw new UnsupportedAuthenticationTypeException(
+            return failedFuture(new UnsupportedAuthenticationTypeException(
                     "Unsupported authentication type: " + authenticationRequest.getClass().getName()
-            );
+            ));
         }
 
         Object requestUsername = authenticationRequest.getIdentity();
@@ -66,6 +67,6 @@ public CompletableFuture<UserDetails> authenticateAsync(AuthenticationRequest<?,
             }
         }
 
-        throw new InvalidCredentialsException("Invalid credentials");
+        return failedFuture(new InvalidCredentialsException("Invalid credentials"));
     }
 }

modules/security/src/test/java/org/apache/ignite/internal/security/authentication/AuthenticationManagerImplTest.java

@@ -17,16 +17,20 @@
 
 package org.apache.ignite.internal.security.authentication;
 
+import static java.util.concurrent.CompletableFuture.completedFuture;
+import static java.util.concurrent.CompletableFuture.failedFuture;
+import static org.apache.ignite.internal.security.authentication.UserDetailsMatcher.username;
 import static org.apache.ignite.internal.security.authentication.event.AuthenticationEvent.AUTHENTICATION_DISABLED;
 import static org.apache.ignite.internal.security.authentication.event.AuthenticationEvent.AUTHENTICATION_ENABLED;
 import static org.apache.ignite.internal.security.authentication.event.AuthenticationEvent.AUTHENTICATION_PROVIDER_REMOVED;
 import static org.apache.ignite.internal.security.authentication.event.AuthenticationEvent.AUTHENTICATION_PROVIDER_UPDATED;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureExceptionMatcher.willThrow;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe;
 import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
 import static org.apache.ignite.internal.util.CompletableFutures.falseCompletedFuture;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertInstanceOf;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
@@ -35,7 +39,6 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.CompletionException;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Consumer;
 import org.apache.ignite.configuration.annotation.ConfigurationType;
@@ -281,7 +284,7 @@ void shouldAuthenticateWithValidCredentials() {
 
         // then
         // successful authentication with valid credentials
-        assertEquals(USERNAME, manager.authenticateAsync(USERNAME_PASSWORD_REQUEST).join().username());
+        assertThat(manager.authenticateAsync(USERNAME_PASSWORD_REQUEST), willBe(username(is(USERNAME))));
     }
 
     @Test
@@ -291,10 +294,9 @@ void shouldThrowInvalidCredentialsExceptionForInvalidCredentials() {
 
         // then
         // failed authentication with invalid credentials
-        CompletionException ex = assertThrows(CompletionException.class,
-                () -> manager.authenticateAsync(new UsernamePasswordRequest(USERNAME, "invalid-password")).join());
-
-        assertInstanceOf(InvalidCredentialsException.class, ex.getCause());
+        assertThat(
+                manager.authenticateAsync(new UsernamePasswordRequest(USERNAME, "invalid-password")),
+                willThrow(InvalidCredentialsException.class));
     }
 
     @Test
@@ -303,29 +305,31 @@ void shouldReturnUnknownUserDetailsWhenAuthenticationIsDisabled() {
         disableAuthentication();
 
         // then
-        assertEquals(UserDetails.UNKNOWN, manager.authenticateAsync(USERNAME_PASSWORD_REQUEST).join());
+        assertThat(manager.authenticateAsync(USERNAME_PASSWORD_REQUEST), willBe(UserDetails.UNKNOWN));
     }
 
     @Test
     public void shouldAuthenticateWithFallbackOnSequentialAuthenticatorExceptions() {
         UsernamePasswordRequest credentials = new UsernamePasswordRequest("admin", "password");
 
         Authenticator authenticator1 = mock(Authenticator.class);
-        doThrow(new InvalidCredentialsException("Invalid credentials")).when(authenticator1).authenticateAsync(credentials);
+        doReturn(failedFuture(new InvalidCredentialsException("Invalid credentials")))
+                .when(authenticator1).authenticateAsync(credentials);
 
         Authenticator authenticator2 = mock(Authenticator.class);
-        doThrow(new UnsupportedAuthenticationTypeException("Unsupported type")).when(authenticator2).authenticateAsync(credentials);
+        doReturn(failedFuture(new UnsupportedAuthenticationTypeException("Unsupported type")))
+                .when(authenticator2).authenticateAsync(credentials);
 
         Authenticator authenticator3 = mock(Authenticator.class);
         doThrow(new RuntimeException("Test exception")).when(authenticator3).authenticateAsync(credentials);
 
         Authenticator authenticator4 = mock(Authenticator.class);
-        doReturn(CompletableFuture.completedFuture(new UserDetails("admin", "mock")))
+        doReturn(completedFuture(new UserDetails("admin", "mock")))
                 .when(authenticator4).authenticateAsync(credentials);
 
         manager.authenticators(List.of(authenticator1, authenticator2, authenticator3, authenticator4));
 
-        assertEquals("admin", manager.authenticateAsync(credentials).join().username());
+        assertThat(manager.authenticateAsync(credentials), willBe(username(is("admin"))));
 
         verify(authenticator1).authenticateAsync(credentials);
         verify(authenticator2).authenticateAsync(credentials);

modules/security/src/test/java/org/apache/ignite/internal/security/authentication/UserDetailsMatcher.java

@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.security.authentication;
+
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+
+/**
+ * Matchers for {@link UserDetails}.
+ */
+public class UserDetailsMatcher {
+
+    /**
+     * Constructs a matcher for {@link UserDetails} with matching username.
+     *
+     * @param subMatcher The matcher to apply to the username.
+     * @return Matcher for {@link UserDetails} with matching username.
+     */
+    public static FeatureMatcher<UserDetails, String> username(Matcher<? super String> subMatcher) {
+        return new FeatureMatcher<>(subMatcher, "UserDetails with username", "username") {
+
+            @Override
+            protected String featureValueOf(UserDetails actual) {
+                return actual.username();
+            }
+        };
+    }
+}

modules/security/src/test/java/org/apache/ignite/internal/security/authentication/basic/BasicAuthenticatorTest.java

@@ -17,12 +17,14 @@
 
 package org.apache.ignite.internal.security.authentication.basic;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.apache.ignite.internal.security.authentication.UserDetailsMatcher.username;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureExceptionMatcher.willThrow;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
 
 import java.util.List;
 import org.apache.ignite.internal.security.authentication.AnonymousRequest;
-import org.apache.ignite.internal.security.authentication.UserDetails;
 import org.apache.ignite.internal.security.authentication.UsernamePasswordRequest;
 import org.apache.ignite.security.exception.InvalidCredentialsException;
 import org.apache.ignite.security.exception.UnsupportedAuthenticationTypeException;
@@ -36,33 +38,28 @@ class BasicAuthenticatorTest {
 
     @Test
     void authenticate() {
-        // when
-        UserDetails userDetails = authenticator.authenticateAsync(new UsernamePasswordRequest("admin", "password")).join();
-
-        // then
-        assertEquals("admin", userDetails.username());
+        assertThat(
+                authenticator.authenticateAsync(new UsernamePasswordRequest("admin", "password")),
+                willBe(username(is("admin")))
+        );
     }
 
     @Test
     void authenticateInvalidCredentials() {
-        // when
-        InvalidCredentialsException exception = assertThrows(InvalidCredentialsException.class, () -> {
-            authenticator.authenticateAsync(new UsernamePasswordRequest("admin", "invalid"));
-        });
-
-        // then
-        assertEquals("Invalid credentials", exception.getMessage());
+        assertThat(
+                authenticator.authenticateAsync(new UsernamePasswordRequest("admin", "invalid")),
+                willThrow(InvalidCredentialsException.class, "Invalid credentials")
+        );
     }
 
     @Test
     void authenticateUnsupportedAuthenticationType() {
-        // when
-        UnsupportedAuthenticationTypeException exception = assertThrows(
-                UnsupportedAuthenticationTypeException.class,
-                () -> authenticator.authenticateAsync(new AnonymousRequest())
+        assertThat(
+                authenticator.authenticateAsync(new AnonymousRequest()),
+                willThrow(
+                        UnsupportedAuthenticationTypeException.class,
+                        "Unsupported authentication type: " + AnonymousRequest.class.getName()
+                )
         );
-
-        // then
-        assertEquals("Unsupported authentication type: " + AnonymousRequest.class.getName(), exception.getMessage());
     }
 }