You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by pt...@apache.org on 2024/08/22 13:26:14 UTC
(ignite-3) branch main updated: IGNITE-23000 Thin client: authenticate asynchronously to avoid blocking Netty thread (#4263)
This is an automated email from the ASF dual-hosted git repository.
ptupitsyn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 29d28d03eb IGNITE-23000 Thin client: authenticate asynchronously to avoid blocking Netty thread (#4263)
29d28d03eb is described below
commit 29d28d03eb027a8c2c71fde2d2e03e414bf6c6a8
Author: Pavel Tupitsyn <pt...@apache.org>
AuthorDate: Thu Aug 22 16:26:08 2024 +0300
IGNITE-23000 Thin client: authenticate asynchronously to avoid blocking Netty thread (#4263)
* Change `Authenticator.authenticate` to `authenticateAsync`
* Providers are supposed to implement this method in a non-blocking fashion
* Refactor `ClientInboundMessageHandler`
* Remove locks - use Netty executor to handle authn events, no need for synchronization
* Perform authn asynchronously
---
.../handler/ClientInboundMessageHandler.java | 204 ++++++++++++---------
.../client/handler/DummyAuthenticationManager.java | 5 +-
.../IgniteAuthenticationProvider.java | 15 +-
.../security/authentication/Authenticator.java | 6 +-
.../authentication/AuthenticationManagerImpl.java | 103 ++++++-----
.../authentication/basic/BasicAuthenticator.java | 6 +-
.../AuthenticationManagerImplTest.java | 32 ++--
.../basic/BasicAuthenticatorTest.java | 6 +-
8 files changed, 216 insertions(+), 161 deletions(-)
diff --git a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java
index 8d1fcd6e76..772253c877 100644
--- a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java
+++ b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java
@@ -37,8 +37,6 @@ import java.util.Set;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.function.Consumer;
import javax.net.ssl.SSLException;
import org.apache.ignite.client.handler.configuration.ClientConnectorView;
@@ -121,7 +119,6 @@ import org.apache.ignite.internal.schema.SchemaVersionMismatchException;
import org.apache.ignite.internal.security.authentication.AnonymousRequest;
import org.apache.ignite.internal.security.authentication.AuthenticationManager;
import org.apache.ignite.internal.security.authentication.AuthenticationRequest;
-import org.apache.ignite.internal.security.authentication.UserDetails;
import org.apache.ignite.internal.security.authentication.UsernamePasswordRequest;
import org.apache.ignite.internal.security.authentication.event.AuthenticationEvent;
import org.apache.ignite.internal.security.authentication.event.AuthenticationEventParameters;
@@ -144,12 +141,20 @@ import org.jetbrains.annotations.Nullable;
/**
* Handles messages from thin clients.
+ *
+ * <p>All message handling is sequential, {@link #channelRead} and other handlers are invoked on a single thread.</p>
*/
@SuppressWarnings({"rawtypes", "unchecked"})
public class ClientInboundMessageHandler extends ChannelInboundHandlerAdapter implements EventListener<AuthenticationEventParameters> {
/** The logger. */
private static final IgniteLogger LOG = Loggers.forClass(ClientInboundMessageHandler.class);
+ private static final byte STATE_BEFORE_HANDSHAKE = 0;
+
+ private static final byte STATE_HANDSHAKE_REQUESTED = 1;
+
+ private static final byte STATE_HANDSHAKE_RESPONSE_SENT = 2;
+
/** Ignite tables API. */
private final IgniteTablesInternal igniteTables;
@@ -188,8 +193,8 @@ public class ClientInboundMessageHandler extends ChannelInboundHandlerAdapter im
/** Context. */
private ClientContext clientContext;
- /** Read-write lock. Protects {@link #clientContext}. */
- private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
+ /** Current state. */
+ private byte state = STATE_BEFORE_HANDSHAKE;
/** Chanel handler context. */
private volatile ChannelHandlerContext channelHandlerContext;
@@ -278,16 +283,12 @@ public class ClientInboundMessageHandler extends ChannelInboundHandlerAdapter im
@Override
public void handlerAdded(ChannelHandlerContext ctx) {
- authenticationEventsToSubscribe().forEach(event -> {
- authenticationManager.listen(event, this);
- });
+ authenticationEventsToSubscribe().forEach(event -> authenticationManager.listen(event, this));
}
@Override
public void handlerRemoved(ChannelHandlerContext ctx) {
- authenticationEventsToSubscribe().forEach(event -> {
- authenticationManager.removeListener(event, this);
- });
+ authenticationEventsToSubscribe().forEach(event -> authenticationManager.removeListener(event, this));
}
@Override
@@ -312,11 +313,26 @@ public class ClientInboundMessageHandler extends ChannelInboundHandlerAdapter im
// Packer buffer is released by Netty on send, or by inner exception handlers below.
var packer = getPacker(ctx.alloc());
- if (clientContext == null) {
- metrics.bytesReceivedAdd(ClientMessageCommon.MAGIC_BYTES.length);
- handshake(ctx, unpacker, packer);
- } else {
- processOperation(ctx, unpacker, packer);
+ switch (state) {
+ case STATE_BEFORE_HANDSHAKE:
+ state = STATE_HANDSHAKE_REQUESTED;
+ metrics.bytesReceivedAdd(ClientMessageCommon.MAGIC_BYTES.length);
+ handshake(ctx, unpacker, packer);
+
+ break;
+
+ case STATE_HANDSHAKE_REQUESTED:
+ // Handshake is in progress, any messages are not allowed.
+ throw new IgniteException(PROTOCOL_ERR, "Unexpected message received before handshake completion");
+
+ case STATE_HANDSHAKE_RESPONSE_SENT:
+ assert clientContext != null : "Client context != null";
+ processOperation(ctx, unpacker, packer);
+
+ break;
+
+ default:
+ throw new IllegalStateException("Unexpected state: " + state);
}
}
@@ -348,81 +364,89 @@ public class ClientInboundMessageHandler extends ChannelInboundHandlerAdapter im
Map<HandshakeExtension, Object> extensions = extractExtensions(unpacker);
- // It's necessary to perform authentication and update the client context while holding a write lock.
- // This prevents a race condition where authentication succeeds but the context isn't updated in time.
- // In such a scenario, we might receive an authentication event and attempt to close the connection,
- // but fail because the context is still null.
- readWriteLock.writeLock().lock();
- try {
- AuthenticationRequest<?, ?> authenticationRequest = createAuthenticationRequest(extensions);
- UserDetails userDetails = authenticationManager.authenticate(authenticationRequest);
- clientContext = new ClientContext(clientVer, clientCode, features, userDetails);
- } finally {
- readWriteLock.writeLock().unlock();
- }
+ authenticationManager
+ .authenticateAsync(createAuthenticationRequest(extensions))
+ .handleAsync((user, err) -> {
+ if (err != null) {
+ handshakeError(ctx, packer, err);
+ } else {
+ clientContext = new ClientContext(clientVer, clientCode, features, user);
- if (LOG.isDebugEnabled()) {
- LOG.debug("Handshake [connectionId=" + connectionId + ", remoteAddress=" + ctx.channel().remoteAddress() + "]: "
- + clientContext);
- }
+ sendHandshakeResponse(ctx, packer);
+ }
- // Response.
- ProtocolVersion.LATEST_VER.pack(packer);
- packer.packNil(); // No error.
+ return null;
+ }, ctx.executor());
+ } catch (Throwable t) {
+ handshakeError(ctx, packer, t);
+ } finally {
+ unpacker.close();
+ }
+ }
- packer.packLong(configuration.idleTimeout());
+ private void handshakeError(ChannelHandlerContext ctx, ClientMessagePacker packer, Throwable t) {
+ LOG.warn("Handshake failed [connectionId=" + connectionId + ", remoteAddress=" + ctx.channel().remoteAddress() + "]: "
+ + t.getMessage(), t);
- ClusterNode localMember = clusterService.topologyService().localMember();
- packer.packString(localMember.id());
- packer.packString(localMember.name());
+ packer.close();
- ClusterTag tag = clusterTag.join();
- packer.packUuid(tag.clusterId());
- packer.packString(tag.clusterName());
+ var errPacker = getPacker(ctx.alloc());
- packer.packLong(observableTimestamp(null));
+ try {
+ ProtocolVersion.LATEST_VER.pack(errPacker);
- // Pack current version
- packer.packByte(IgniteProductVersion.CURRENT_VERSION.major());
- packer.packByte(IgniteProductVersion.CURRENT_VERSION.minor());
- packer.packByte(IgniteProductVersion.CURRENT_VERSION.maintenance());
- packer.packByteNullable(IgniteProductVersion.CURRENT_VERSION.patch());
- packer.packStringNullable(IgniteProductVersion.CURRENT_VERSION.preRelease());
+ writeErrorCore(t, errPacker);
- packer.packBinaryHeader(0); // Features.
- packer.packInt(0); // Extensions.
+ write(errPacker, ctx);
+ } catch (Throwable t2) {
+ LOG.warn("Handshake failed [connectionId=" + connectionId + ", remoteAddress=" + ctx.channel().remoteAddress() + "]: "
+ + t2.getMessage(), t2);
- write(packer, ctx);
+ errPacker.close();
+ exceptionCaught(ctx, t2);
+ }
- metrics.sessionsAcceptedIncrement();
- metrics.sessionsActiveIncrement();
+ metrics.sessionsRejectedIncrement();
+ }
- ctx.channel().closeFuture().addListener(f -> metrics.sessionsActiveDecrement());
- } catch (Throwable t) {
- LOG.warn("Handshake failed [connectionId=" + connectionId + ", remoteAddress=" + ctx.channel().remoteAddress() + "]: "
- + t.getMessage(), t);
+ private void sendHandshakeResponse(ChannelHandlerContext ctx, ClientMessagePacker packer) {
+ ProtocolVersion.LATEST_VER.pack(packer);
+ packer.packNil(); // No error.
- packer.close();
+ packer.packLong(configuration.idleTimeout());
+
+ ClusterNode localMember = clusterService.topologyService().localMember();
+ packer.packString(localMember.id());
+ packer.packString(localMember.name());
- var errPacker = getPacker(ctx.alloc());
+ ClusterTag tag = clusterTag.join();
+ packer.packUuid(tag.clusterId());
+ packer.packString(tag.clusterName());
- try {
- ProtocolVersion.LATEST_VER.pack(errPacker);
+ packer.packLong(observableTimestamp(null));
- writeErrorCore(t, errPacker);
+ // Pack current version
+ packer.packByte(IgniteProductVersion.CURRENT_VERSION.major());
+ packer.packByte(IgniteProductVersion.CURRENT_VERSION.minor());
+ packer.packByte(IgniteProductVersion.CURRENT_VERSION.maintenance());
+ packer.packByteNullable(IgniteProductVersion.CURRENT_VERSION.patch());
+ packer.packStringNullable(IgniteProductVersion.CURRENT_VERSION.preRelease());
- write(errPacker, ctx);
- } catch (Throwable t2) {
- LOG.warn("Handshake failed [connectionId=" + connectionId + ", remoteAddress=" + ctx.channel().remoteAddress() + "]: "
- + t2.getMessage(), t2);
+ packer.packBinaryHeader(0); // Features.
+ packer.packInt(0); // Extensions.
- errPacker.close();
- exceptionCaught(ctx, t2);
- }
+ write(packer, ctx);
- metrics.sessionsRejectedIncrement();
- } finally {
- unpacker.close();
+ state = STATE_HANDSHAKE_RESPONSE_SENT;
+
+ metrics.sessionsAcceptedIncrement();
+ metrics.sessionsActiveIncrement();
+
+ ctx.channel().closeFuture().addListener(f -> metrics.sessionsActiveDecrement());
+
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Handshake [connectionId=" + connectionId + ", remoteAddress=" + ctx.channel().remoteAddress() + "]: "
+ + clientContext);
}
}
@@ -919,11 +943,24 @@ public class ClientInboundMessageHandler extends ChannelInboundHandlerAdapter im
@Override
public CompletableFuture<Boolean> notify(AuthenticationEventParameters parameters) {
- if (shouldCloseConnection(parameters)) {
- LOG.warn("Closing connection due to authentication event [connectionId=" + connectionId + ", remoteAddress="
- + channelHandlerContext.channel().remoteAddress() + ", event=" + parameters.type() + ']');
- closeConnection();
+ var channelCtx = channelHandlerContext;
+
+ if (channelCtx == null) {
+ // Not connected yet.
+ return falseCompletedFuture();
}
+
+ // Use Netty executor (single thread) to process the event sequentially with network operations - no need to synchronize.
+ channelCtx.executor().submit(() -> {
+ if (shouldCloseConnection(parameters)) {
+ LOG.warn("Closing connection due to authentication event [connectionId=" + connectionId + ", remoteAddress="
+ + channelHandlerContext.channel().remoteAddress() + ", event=" + parameters.type() + ']');
+
+ closeConnection();
+ }
+ });
+
+ // No need to wait for the event processing to complete, return false to continue listening.
return falseCompletedFuture();
}
@@ -943,12 +980,7 @@ public class ClientInboundMessageHandler extends ChannelInboundHandlerAdapter im
}
private boolean currentUserAffected(AuthenticationProviderEventParameters parameters) {
- readWriteLock.readLock().lock();
- try {
- return clientContext != null && clientContext.userDetails().providerName().equals(parameters.name());
- } finally {
- readWriteLock.readLock().unlock();
- }
+ return clientContext != null && clientContext.userDetails().providerName().equals(parameters.name());
}
private boolean currentUserAffected(UserEventParameters parameters) {
@@ -958,8 +990,10 @@ public class ClientInboundMessageHandler extends ChannelInboundHandlerAdapter im
}
private void closeConnection() {
- if (channelHandlerContext != null) {
- channelHandlerContext.close();
+ ChannelHandlerContext ctx = channelHandlerContext;
+
+ if (ctx != null) {
+ ctx.close();
}
}
diff --git a/modules/client-handler/src/testFixtures/java/org/apache/ignite/client/handler/DummyAuthenticationManager.java b/modules/client-handler/src/testFixtures/java/org/apache/ignite/client/handler/DummyAuthenticationManager.java
index 1fc0bc055c..6c1045c857 100644
--- a/modules/client-handler/src/testFixtures/java/org/apache/ignite/client/handler/DummyAuthenticationManager.java
+++ b/modules/client-handler/src/testFixtures/java/org/apache/ignite/client/handler/DummyAuthenticationManager.java
@@ -17,6 +17,7 @@
package org.apache.ignite.client.handler;
+import static java.util.concurrent.CompletableFuture.completedFuture;
import static org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
import java.util.concurrent.CompletableFuture;
@@ -58,7 +59,7 @@ public class DummyAuthenticationManager implements AuthenticationManager {
}
@Override
- public UserDetails authenticate(AuthenticationRequest<?, ?> authenticationRequest) {
- return UserDetails.UNKNOWN;
+ public CompletableFuture<UserDetails> authenticateAsync(AuthenticationRequest<?, ?> authenticationRequest) {
+ return completedFuture(UserDetails.UNKNOWN);
}
}
diff --git a/modules/rest/src/main/java/org/apache/ignite/internal/rest/authentication/IgniteAuthenticationProvider.java b/modules/rest/src/main/java/org/apache/ignite/internal/rest/authentication/IgniteAuthenticationProvider.java
index 1f57b1acc6..e48b834219 100644
--- a/modules/rest/src/main/java/org/apache/ignite/internal/rest/authentication/IgniteAuthenticationProvider.java
+++ b/modules/rest/src/main/java/org/apache/ignite/internal/rest/authentication/IgniteAuthenticationProvider.java
@@ -24,7 +24,6 @@ import io.micronaut.security.authentication.AuthenticationResponse;
import io.micronaut.security.authentication.UsernamePasswordCredentials;
import org.apache.ignite.internal.rest.ResourceHolder;
import org.apache.ignite.internal.security.authentication.AuthenticationManager;
-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.reactivestreams.Publisher;
@@ -49,9 +48,17 @@ public class IgniteAuthenticationProvider implements AuthenticationProvider, Res
public Publisher<AuthenticationResponse> authenticate(HttpRequest<?> httpRequest, AuthenticationRequest<?, ?> authenticationRequest) {
return Flux.create(emitter -> {
try {
- UserDetails userDetails = authenticationManager.authenticate(toIgniteAuthenticationRequest(authenticationRequest));
- emitter.next(AuthenticationResponse.success(userDetails.username()));
- emitter.complete();
+ authenticationManager.authenticateAsync(toIgniteAuthenticationRequest(authenticationRequest))
+ .handle((userDetails, throwable) -> {
+ if (throwable != null) {
+ emitter.error(AuthenticationResponse.exception(throwable.getMessage()));
+ } else {
+ emitter.next(AuthenticationResponse.success(userDetails.username()));
+ emitter.complete();
+ }
+
+ return null;
+ });
} catch (InvalidCredentialsException ex) {
emitter.error(AuthenticationResponse.exception(ex.getMessage()));
}
diff --git a/modules/security-api/src/main/java/org/apache/ignite/internal/security/authentication/Authenticator.java b/modules/security-api/src/main/java/org/apache/ignite/internal/security/authentication/Authenticator.java
index 269f70e494..ce021ffaa8 100644
--- a/modules/security-api/src/main/java/org/apache/ignite/internal/security/authentication/Authenticator.java
+++ b/modules/security-api/src/main/java/org/apache/ignite/internal/security/authentication/Authenticator.java
@@ -17,21 +17,25 @@
package org.apache.ignite.internal.security.authentication;
+import java.util.concurrent.CompletableFuture;
import org.apache.ignite.security.exception.InvalidCredentialsException;
import org.apache.ignite.security.exception.UnsupportedAuthenticationTypeException;
/**
* General interface for all authenticators.
*/
+@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
public interface Authenticator {
/**
* Authenticates a user with the given request. Returns the user details if the authentication was successful. Throws an exception
* otherwise.
*
+ * <p>Implementations should be non-blocking, this method can be called from an IO thread.
+ *
* @param authenticationRequest The authentication request.
* @return The user details.
* @throws InvalidCredentialsException If the authentication failed.
* @throws UnsupportedAuthenticationTypeException If the authentication type is not supported.
*/
- UserDetails authenticate(AuthenticationRequest<?, ?> authenticationRequest);
+ CompletableFuture<UserDetails> authenticateAsync(AuthenticationRequest<?, ?> authenticationRequest);
}
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 0c751c7175..bbb6141d89 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
@@ -17,17 +17,16 @@
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.AuthenticationUtils.findBasicProviderName;
import static org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
-import java.util.ArrayList;
import java.util.Comparator;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import java.util.Objects;
import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.stream.Collectors;
import org.apache.ignite.configuration.NamedListView;
import org.apache.ignite.configuration.notifications.ConfigurationListener;
@@ -87,19 +86,9 @@ public class AuthenticationManagerImpl
private final AuthenticationProviderEventFactory providerEventFactory;
/**
- * Read-write lock for the list of authenticators and the authentication enabled flag.
+ * List of authenticators. Null when authentication is disabled.
*/
- private final ReadWriteLock rwLock = new ReentrantReadWriteLock();
-
- /**
- * List of authenticators.
- */
- private List<Authenticator> authenticators = new ArrayList<>();
-
- /**
- * Authentication enabled flag.
- */
- private boolean authEnabled = false;
+ private volatile @Nullable List<Authenticator> authenticators;
private final EventLog eventLog;
@@ -161,41 +150,64 @@ public class AuthenticationManagerImpl
* {@inheritDoc}
*/
@Override
- public UserDetails authenticate(AuthenticationRequest<?, ?> authenticationRequest) {
- rwLock.readLock().lock();
- try {
- if (authEnabled) {
- return authenticators.stream()
- .map(authenticator -> authenticate(authenticator, authenticationRequest))
- .filter(Objects::nonNull)
- .findFirst()
- .orElseThrow(() -> new InvalidCredentialsException("Authentication failed"));
- } else {
- return UserDetails.UNKNOWN;
- }
- } finally {
- rwLock.readLock().unlock();
+ public CompletableFuture<UserDetails> authenticateAsync(AuthenticationRequest<?, ?> authenticationRequest) {
+ var providers = authenticators;
+
+ if (providers != null) {
+ return authenticate(providers.iterator(), authenticationRequest);
+ } else {
+ return completedFuture(UserDetails.UNKNOWN);
}
}
- @Nullable
- private UserDetails authenticate(
+ private CompletableFuture<UserDetails> authenticate(Iterator<Authenticator> iter, AuthenticationRequest<?, ?> authenticationRequest) {
+ if (!iter.hasNext()) {
+ return failedFuture(new InvalidCredentialsException("Authentication failed"));
+ }
+
+ return authenticate(iter.next(), authenticationRequest)
+ .thenCompose(userDetails -> {
+ if (userDetails != null) {
+ return completedFuture(userDetails);
+ }
+
+ return authenticate(iter, authenticationRequest);
+ });
+ }
+
+ private CompletableFuture<UserDetails> authenticate(
Authenticator authenticator,
AuthenticationRequest<?, ?> authenticationRequest
) {
try {
- var userDetails = authenticator.authenticate(authenticationRequest);
- if (userDetails != null) {
- logUserAuthenticated(userDetails);
- }
- return userDetails;
+ 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 null;
+ return completedFuture(null);
} catch (Exception e) {
logAuthenticationFailure(authenticationRequest);
LOG.error("Unexpected exception during authentication", e);
- return null;
+ return completedFuture(null);
}
}
@@ -223,18 +235,14 @@ public class AuthenticationManagerImpl
}
private void refreshProviders(@Nullable SecurityView view) {
- rwLock.writeLock().lock();
try {
if (view == null || !view.enabled()) {
- authEnabled = false;
+ authenticators = null;
} else {
authenticators = providersFromAuthView(view.authentication());
- authEnabled = true;
}
} catch (Exception exception) {
LOG.error("Couldn't refresh authentication providers. Leaving the old settings", exception);
- } finally {
- rwLock.writeLock().unlock();
}
}
@@ -254,12 +262,7 @@ public class AuthenticationManagerImpl
@Override
public boolean authenticationEnabled() {
- return authEnabled;
- }
-
- @TestOnly
- public void authEnabled(boolean authEnabled) {
- this.authEnabled = authEnabled;
+ return authenticators != null;
}
@TestOnly
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 f004b8d1ee..0e43fcd35b 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
@@ -17,10 +17,12 @@
package org.apache.ignite.internal.security.authentication.basic;
+import static java.util.concurrent.CompletableFuture.completedFuture;
import static java.util.function.Function.identity;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.CompletableFuture;
import java.util.stream.Collectors;
import org.apache.ignite.internal.security.authentication.AuthenticationRequest;
import org.apache.ignite.internal.security.authentication.Authenticator;
@@ -47,7 +49,7 @@ public class BasicAuthenticator implements Authenticator {
}
@Override
- public UserDetails authenticate(AuthenticationRequest<?, ?> authenticationRequest) {
+ public CompletableFuture<UserDetails> authenticateAsync(AuthenticationRequest<?, ?> authenticationRequest) {
if (!(authenticationRequest instanceof UsernamePasswordRequest)) {
throw new UnsupportedAuthenticationTypeException(
"Unsupported authentication type: " + authenticationRequest.getClass().getName()
@@ -60,7 +62,7 @@ public class BasicAuthenticator implements Authenticator {
BasicUser basicUser = users.get(requestUsername);
if (basicUser != null) {
if (basicUser.password().equals(requestPassword)) {
- return new UserDetails(basicUser.name(), providerName);
+ return completedFuture(new UserDetails(basicUser.name(), providerName));
}
}
diff --git a/modules/security/src/test/java/org/apache/ignite/internal/security/authentication/AuthenticationManagerImplTest.java b/modules/security/src/test/java/org/apache/ignite/internal/security/authentication/AuthenticationManagerImplTest.java
index b0ff9568a8..a3d0cd6b08 100644
--- a/modules/security/src/test/java/org/apache/ignite/internal/security/authentication/AuthenticationManagerImplTest.java
+++ b/modules/security/src/test/java/org/apache/ignite/internal/security/authentication/AuthenticationManagerImplTest.java
@@ -25,6 +25,7 @@ import static org.apache.ignite.internal.testframework.matchers.CompletableFutur
import static org.apache.ignite.internal.util.CompletableFutures.falseCompletedFuture;
import static org.hamcrest.MatcherAssert.assertThat;
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;
@@ -34,6 +35,7 @@ import static org.mockito.Mockito.verify;
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;
@@ -279,7 +281,7 @@ class AuthenticationManagerImplTest extends BaseIgniteAbstractTest {
// then
// successful authentication with valid credentials
- assertEquals(USERNAME, manager.authenticate(USERNAME_PASSWORD_REQUEST).username());
+ assertEquals(USERNAME, manager.authenticateAsync(USERNAME_PASSWORD_REQUEST).join().username());
}
@Test
@@ -289,8 +291,10 @@ class AuthenticationManagerImplTest extends BaseIgniteAbstractTest {
// then
// failed authentication with invalid credentials
- assertThrows(InvalidCredentialsException.class,
- () -> manager.authenticate(new UsernamePasswordRequest(USERNAME, "invalid-password")));
+ CompletionException ex = assertThrows(CompletionException.class,
+ () -> manager.authenticateAsync(new UsernamePasswordRequest(USERNAME, "invalid-password")).join());
+
+ assertInstanceOf(InvalidCredentialsException.class, ex.getCause());
}
@Test
@@ -299,7 +303,7 @@ class AuthenticationManagerImplTest extends BaseIgniteAbstractTest {
disableAuthentication();
// then
- assertEquals(UserDetails.UNKNOWN, manager.authenticate(USERNAME_PASSWORD_REQUEST));
+ assertEquals(UserDetails.UNKNOWN, manager.authenticateAsync(USERNAME_PASSWORD_REQUEST).join());
}
@Test
@@ -307,26 +311,26 @@ class AuthenticationManagerImplTest extends BaseIgniteAbstractTest {
UsernamePasswordRequest credentials = new UsernamePasswordRequest("admin", "password");
Authenticator authenticator1 = mock(Authenticator.class);
- doThrow(new InvalidCredentialsException("Invalid credentials")).when(authenticator1).authenticate(credentials);
+ doThrow(new InvalidCredentialsException("Invalid credentials")).when(authenticator1).authenticateAsync(credentials);
Authenticator authenticator2 = mock(Authenticator.class);
- doThrow(new UnsupportedAuthenticationTypeException("Unsupported type")).when(authenticator2).authenticate(credentials);
+ doThrow(new UnsupportedAuthenticationTypeException("Unsupported type")).when(authenticator2).authenticateAsync(credentials);
Authenticator authenticator3 = mock(Authenticator.class);
- doThrow(new RuntimeException("Test exception")).when(authenticator3).authenticate(credentials);
+ doThrow(new RuntimeException("Test exception")).when(authenticator3).authenticateAsync(credentials);
Authenticator authenticator4 = mock(Authenticator.class);
- doReturn(new UserDetails("admin", "mock")).when(authenticator4).authenticate(credentials);
+ doReturn(CompletableFuture.completedFuture(new UserDetails("admin", "mock")))
+ .when(authenticator4).authenticateAsync(credentials);
- manager.authEnabled(true);
manager.authenticators(List.of(authenticator1, authenticator2, authenticator3, authenticator4));
- assertEquals("admin", manager.authenticate(credentials).username());
+ assertEquals("admin", manager.authenticateAsync(credentials).join().username());
- verify(authenticator1).authenticate(credentials);
- verify(authenticator2).authenticate(credentials);
- verify(authenticator3).authenticate(credentials);
- verify(authenticator4).authenticate(credentials);
+ verify(authenticator1).authenticateAsync(credentials);
+ verify(authenticator2).authenticateAsync(credentials);
+ verify(authenticator3).authenticateAsync(credentials);
+ verify(authenticator4).authenticateAsync(credentials);
}
private void enableAuthentication() {
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 2f4bce91c5..46fd945362 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
@@ -37,7 +37,7 @@ class BasicAuthenticatorTest {
@Test
void authenticate() {
// when
- UserDetails userDetails = authenticator.authenticate(new UsernamePasswordRequest("admin", "password"));
+ UserDetails userDetails = authenticator.authenticateAsync(new UsernamePasswordRequest("admin", "password")).join();
// then
assertEquals("admin", userDetails.username());
@@ -47,7 +47,7 @@ class BasicAuthenticatorTest {
void authenticateInvalidCredentials() {
// when
InvalidCredentialsException exception = assertThrows(InvalidCredentialsException.class, () -> {
- authenticator.authenticate(new UsernamePasswordRequest("admin", "invalid"));
+ authenticator.authenticateAsync(new UsernamePasswordRequest("admin", "invalid"));
});
// then
@@ -59,7 +59,7 @@ class BasicAuthenticatorTest {
// when
UnsupportedAuthenticationTypeException exception = assertThrows(
UnsupportedAuthenticationTypeException.class,
- () -> authenticator.authenticate(new AnonymousRequest())
+ () -> authenticator.authenticateAsync(new AnonymousRequest())
);
// then