From d32ef2e80cac654dea8fe26b0c75cd5d4ae62766 Mon Sep 17 00:00:00 2001 From: Vadim Pakhnushev <8614891+valepakh@users.noreply.github.com> Date: Thu, 29 Aug 2024 14:11:46 +0300 Subject: [PATCH] IGNITE-23100 Improve semantic of authenticateAsync (#4306) --- .../AuthenticationManagerImpl.java | 43 ++++++++----------- .../basic/BasicAuthenticator.java | 7 +-- .../AuthenticationManagerImplTest.java | 30 +++++++------ .../authentication/UserDetailsMatcher.java | 43 +++++++++++++++++++ .../basic/BasicAuthenticatorTest.java | 41 ++++++++---------- 5 files changed, 101 insertions(+), 63 deletions(-) create mode 100644 modules/security/src/test/java/org/apache/ignite/internal/security/authentication/UserDetailsMatcher.java diff --git a/modules/security/src/main/java/org/apache/ignite/internal/security/authentication/AuthenticationManagerImpl.java b/modules/security/src/main/java/org/apache/ignite/internal/security/authentication/AuthenticationManagerImpl.java index bbb6141d898..153d1a9ea98 100644 --- a/modules/security/src/main/java/org/apache/ignite/internal/security/authentication/AuthenticationManagerImpl.java +++ b/modules/security/src/main/java/org/apache/ignite/internal/security/authentication/AuthenticationManagerImpl.java @@ -180,34 +180,27 @@ private CompletableFuture 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(); } } diff --git a/modules/security/src/main/java/org/apache/ignite/internal/security/authentication/basic/BasicAuthenticator.java b/modules/security/src/main/java/org/apache/ignite/internal/security/authentication/basic/BasicAuthenticator.java index 0e43fcd35be..677a3eaf513 100644 --- a/modules/security/src/main/java/org/apache/ignite/internal/security/authentication/basic/BasicAuthenticator.java +++ b/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 users) { @Override public CompletableFuture 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 authenticateAsync(AuthenticationRequest 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,7 +305,7 @@ void shouldReturnUnknownUserDetailsWhenAuthenticationIsDisabled() { disableAuthentication(); // then - assertEquals(UserDetails.UNKNOWN, manager.authenticateAsync(USERNAME_PASSWORD_REQUEST).join()); + assertThat(manager.authenticateAsync(USERNAME_PASSWORD_REQUEST), willBe(UserDetails.UNKNOWN)); } @Test @@ -311,21 +313,23 @@ 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); diff --git a/modules/security/src/test/java/org/apache/ignite/internal/security/authentication/UserDetailsMatcher.java b/modules/security/src/test/java/org/apache/ignite/internal/security/authentication/UserDetailsMatcher.java new file mode 100644 index 00000000000..2733608039e --- /dev/null +++ b/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 username(Matcher subMatcher) { + return new FeatureMatcher<>(subMatcher, "UserDetails with username", "username") { + + @Override + protected String featureValueOf(UserDetails actual) { + return actual.username(); + } + }; + } +} diff --git a/modules/security/src/test/java/org/apache/ignite/internal/security/authentication/basic/BasicAuthenticatorTest.java b/modules/security/src/test/java/org/apache/ignite/internal/security/authentication/basic/BasicAuthenticatorTest.java index 46fd945362e..d7e10286fb5 100644 --- a/modules/security/src/test/java/org/apache/ignite/internal/security/authentication/basic/BasicAuthenticatorTest.java +++ b/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()); } }