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 35daa6a17a2 IGNITE-28292 Improve authentication exception detection
and logging in client and REST handlers (#7817)
35daa6a17a2 is described below
commit 35daa6a17a2cb01370b1521896a8a7808d3ca42d
Author: Vadim Pakhnushev <[email protected]>
AuthorDate: Fri Mar 20 12:09:39 2026 +0300
IGNITE-28292 Improve authentication exception detection and logging in
client and REST handlers (#7817)
---
.../handler/ClientInboundMessageHandler.java | 30 +++++++----
...nspector.java => AccumulatingLogInspector.java} | 17 +++---
.../testframework/log4j2/EventLogInspector.java | 26 ++--------
.../ignite/internal/eventlog/ItEventLogTest.java | 60 +++++++++++++++++++++-
.../IgniteAuthenticationProvider.java | 16 ++++++
.../authentication/AuthenticationManagerImpl.java | 8 +--
6 files changed, 115 insertions(+), 42 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 e1cb98429e8..16b3a90fbc0 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
@@ -186,6 +186,7 @@ import org.apache.ignite.lang.IgniteException;
import org.apache.ignite.lang.TraceableException;
import org.apache.ignite.network.IgniteCluster;
import org.apache.ignite.security.AuthenticationType;
+import org.apache.ignite.security.exception.InvalidCredentialsException;
import
org.apache.ignite.security.exception.UnsupportedAuthenticationTypeException;
import org.apache.ignite.sql.SqlBatchException;
import org.apache.ignite.tx.RetriableTransactionException;
@@ -516,6 +517,11 @@ public class ClientInboundMessageHandler
.thenCompose(unused ->
authenticationManager.authenticateAsync(authReq))
.whenCompleteAsync((user, err) -> {
if (err != null) {
+ if (isAuthenticationException(err)) {
+ LOG.warn("Client authentication failed
[connectionId={}, remoteAddress={}, identity={}]: {}",
+ connectionId,
ctx.channel().remoteAddress(), authReq.getIdentity(), err.getMessage());
+ }
+
handshakeError(ctx, err);
} else {
handshakeSuccess(ctx, user, clientFeatures,
clientVer, clientCode);
@@ -562,8 +568,10 @@ public class ClientInboundMessageHandler
}
private void handshakeError(ChannelHandlerContext ctx, Throwable t) {
- LOG.warn("Handshake failed [connectionId=" + connectionId + ",
remoteAddress=" + ctx.channel().remoteAddress() + "]: "
- + t.getMessage(), t);
+ // Authentication failures are already logged by the caller with more
details (e.g. username).
+ if (!isAuthenticationException(t)) {
+ LOG.warn("Handshake failed [connectionId={}, remoteAddress={}]",
t, connectionId, ctx.channel().remoteAddress());
+ }
var errPacker = getPacker(ctx.alloc());
@@ -698,11 +706,11 @@ public class ClientInboundMessageHandler
private void writeError(long requestId, int opCode, Throwable err,
ChannelHandlerContext ctx, boolean isNotification) {
if (LOG.isDebugEnabled() && shouldLogError(err)) {
if (isNotification) {
- LOG.debug("Error processing client notification
[connectionId=" + connectionId + ", id=" + requestId
- + ", remoteAddress=" + ctx.channel().remoteAddress() +
"]:" + err.getMessage(), err);
+ LOG.debug("Error processing client notification
[connectionId={}, id={}, remoteAddress={}]",
+ err, connectionId, requestId,
ctx.channel().remoteAddress());
} else {
- LOG.debug("Error processing client request [connectionId=" +
connectionId + ", id=" + requestId + ", op=" + opCode
- + ", remoteAddress=" + ctx.channel().remoteAddress() +
"]:" + err.getMessage(), err);
+ LOG.debug("Error processing client request [connectionId={},
id={}, op={}, remoteAddress={}]",
+ err, connectionId, requestId, opCode,
ctx.channel().remoteAddress());
}
}
@@ -1421,9 +1429,8 @@ public class ClientInboundMessageHandler
fut.completeExceptionally(err);
}
} catch (Throwable t) {
- LOG.warn("Unexpected error while processing SERVER_OP_RESPONSE
[id=" + requestId
- + ", connectionId=" + connectionId + ", remoteAddress=" +
channelHandlerContext.channel().remoteAddress()
- + ", message=" + t.getMessage() + ']', t);
+ LOG.warn("Unexpected error while processing SERVER_OP_RESPONSE
[id={}, connectionId={}, remoteAddress={}]",
+ t, requestId, connectionId,
channelHandlerContext.channel().remoteAddress());
}
}
@@ -1501,6 +1508,11 @@ public class ClientInboundMessageHandler
);
}
+ private static boolean isAuthenticationException(Throwable t) {
+ Throwable cause = ExceptionUtils.unwrapCause(t);
+ return cause instanceof InvalidCredentialsException || cause
instanceof UnsupportedAuthenticationTypeException;
+ }
+
private class ComputeConnection implements PlatformComputeConnection {
@Override
public CompletableFuture<ComputeJobDataHolder> executeJobAsync(
diff --git
a/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/EventLogInspector.java
b/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/AccumulatingLogInspector.java
similarity index 71%
copy from
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/EventLogInspector.java
copy to
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/AccumulatingLogInspector.java
index 1dbd91419a9..f8c52f4338b 100644
---
a/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/EventLogInspector.java
+++
b/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/AccumulatingLogInspector.java
@@ -22,15 +22,20 @@ import java.util.concurrent.CopyOnWriteArrayList;
import org.apache.ignite.internal.testframework.log4j2.LogInspector.Handler;
/**
- * Helper class that listens for the messages from the EventLog logger and
puts them into the list.
+ * Helper class that listens for the messages from the specified logger and
puts them into the list.
*/
-public class EventLogInspector {
+public class AccumulatingLogInspector {
private final List<String> events = new CopyOnWriteArrayList<>();
- private final LogInspector logInspector = new LogInspector(
- "EventLog", // From LogSinkConfigurationSchema.criteria default
value
- new Handler(event -> true, event ->
events.add(event.getMessage().getFormattedMessage()))
- );
+ private final LogInspector logInspector;
+
+ /** Constructor. */
+ public AccumulatingLogInspector(String loggerName) {
+ logInspector = new LogInspector(
+ loggerName, // From LogSinkConfigurationSchema.criteria
default value
+ new Handler(event -> true, event ->
events.add(event.getMessage().getFormattedMessage()))
+ );
+ }
public void start() {
events.clear();
diff --git
a/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/EventLogInspector.java
b/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/EventLogInspector.java
index 1dbd91419a9..b751d81064b 100644
---
a/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/EventLogInspector.java
+++
b/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/EventLogInspector.java
@@ -17,31 +17,11 @@
package org.apache.ignite.internal.testframework.log4j2;
-import java.util.List;
-import java.util.concurrent.CopyOnWriteArrayList;
-import org.apache.ignite.internal.testframework.log4j2.LogInspector.Handler;
-
/**
* Helper class that listens for the messages from the EventLog logger and
puts them into the list.
*/
-public class EventLogInspector {
- private final List<String> events = new CopyOnWriteArrayList<>();
-
- private final LogInspector logInspector = new LogInspector(
- "EventLog", // From LogSinkConfigurationSchema.criteria default
value
- new Handler(event -> true, event ->
events.add(event.getMessage().getFormattedMessage()))
- );
-
- public void start() {
- events.clear();
- logInspector.start();
- }
-
- public void stop() {
- logInspector.stop();
- }
-
- public List<String> events() {
- return events;
+public class EventLogInspector extends AccumulatingLogInspector {
+ public EventLogInspector() {
+ super("EventLog");
}
}
diff --git
a/modules/eventlog/src/integrationTest/java/org/apache/ignite/internal/eventlog/ItEventLogTest.java
b/modules/eventlog/src/integrationTest/java/org/apache/ignite/internal/eventlog/ItEventLogTest.java
index df9f53fa8d5..608474f3932 100644
---
a/modules/eventlog/src/integrationTest/java/org/apache/ignite/internal/eventlog/ItEventLogTest.java
+++
b/modules/eventlog/src/integrationTest/java/org/apache/ignite/internal/eventlog/ItEventLogTest.java
@@ -17,9 +17,12 @@
package org.apache.ignite.internal.eventlog;
+import static
org.apache.ignite.internal.testframework.matchers.HttpResponseMatcher.hasStatusCode;
import static org.awaitility.Awaitility.await;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasSize;
@@ -28,19 +31,30 @@ import static org.hamcrest.Matchers.matchesRegex;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.io.IOException;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse.BodyHandlers;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.util.Base64;
import java.util.List;
import org.apache.ignite.InitParametersBuilder;
import org.apache.ignite.client.BasicAuthenticator;
import org.apache.ignite.client.IgniteClient;
import org.apache.ignite.client.IgniteClientConnectionException;
+import org.apache.ignite.internal.ClusterConfiguration;
import org.apache.ignite.internal.ClusterPerClassIntegrationTest;
import org.apache.ignite.internal.properties.IgniteProductVersion;
+import
org.apache.ignite.internal.testframework.log4j2.AccumulatingLogInspector;
+import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
class ItEventLogTest extends ClusterPerClassIntegrationTest {
+ private static final String NODE_URL = "http://localhost:" +
ClusterConfiguration.DEFAULT_BASE_HTTP_PORT;
+
private static final String PROVIDER_NAME = "basic";
private static final String USERNAME = "admin";
@@ -49,12 +63,30 @@ class ItEventLogTest extends ClusterPerClassIntegrationTest
{
private static Path eventlogPath;
+ private final AccumulatingLogInspector clientLogInspector = new
AccumulatingLogInspector(
+ "org.apache.ignite.client.handler.ClientInboundMessageHandler");
+
+ private final AccumulatingLogInspector restLogInspector = new
AccumulatingLogInspector(
+
"org.apache.ignite.internal.rest.authentication.IgniteAuthenticationProvider");
+
@BeforeAll
static void captureEventLogPath() {
String buildDirPath = System.getProperty("buildDirPath");
eventlogPath = Path.of(buildDirPath).resolve("event.log");
}
+ @BeforeEach
+ void startInspectors() {
+ clientLogInspector.start();
+ restLogInspector.start();
+ }
+
+ @AfterEach
+ void stopInspectors() {
+ clientLogInspector.stop();
+ restLogInspector.stop();
+ }
+
@Override
protected int initialNodes() {
return 1;
@@ -101,7 +133,7 @@ class ItEventLogTest extends ClusterPerClassIntegrationTest
{
assertThat(readEventLog(),
contains(matchesRegex(expectedEventJsonPattern)));
- // When try to authenticate with invalid credentials.
+ // When try to authenticate with invalid credentials via thin client.
BasicAuthenticator invalidAuthenticator =
BasicAuthenticator.builder().username("UNKNOWN").password("SECRET").build();
assertThrows(
IgniteClientConnectionException.class,
@@ -121,6 +153,32 @@ class ItEventLogTest extends
ClusterPerClassIntegrationTest {
+ "}";
assertThat(readEventLog(),
hasItem(matchesRegex(expectedEventJsonPatternAfterInvalidAuth)));
+
+ // And the client handler log contains authentication failure details.
+ assertThat(clientLogInspector.events(), hasItem(allOf(
+ containsString("Client authentication failed"),
+ containsString("remoteAddress="),
+ containsString("identity=UNKNOWN]")
+ )));
+
+ // When try to authenticate with invalid credentials via REST API.
+ HttpClient httpClient = HttpClient.newBuilder().build();
+ String endpoint = "/management/v1/configuration/cluster/";
+ HttpRequest request = HttpRequest.newBuilder(URI.create(NODE_URL +
endpoint))
+ .header("Authorization", basicAuthHeader("UNKNOWN", "SECRET"))
+ .build();
+
+ assertThat(httpClient.send(request, BodyHandlers.ofString()),
hasStatusCode(401));
+
+ // Then the REST log contains authentication failure details.
+ assertThat(restLogInspector.events(), hasItem(allOf(
+ containsString("REST authentication failed [uri=" + endpoint +
", remoteAddress="),
+ containsString("identity=UNKNOWN]")
+ )));
+ }
+
+ private static String basicAuthHeader(String username, String password) {
+ return "Basic " + Base64.getEncoder().encodeToString((username + ":" +
password).getBytes());
}
private static List<String> readEventLog() throws IOException {
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 e48b834219c..de916a4403e 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
@@ -22,6 +22,8 @@ import
io.micronaut.security.authentication.AuthenticationProvider;
import io.micronaut.security.authentication.AuthenticationRequest;
import io.micronaut.security.authentication.AuthenticationResponse;
import io.micronaut.security.authentication.UsernamePasswordCredentials;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
import org.apache.ignite.internal.rest.ResourceHolder;
import
org.apache.ignite.internal.security.authentication.AuthenticationManager;
import
org.apache.ignite.internal.security.authentication.UsernamePasswordRequest;
@@ -34,6 +36,8 @@ import reactor.core.publisher.FluxSink;
* Implementation of {@link AuthenticationProvider}. Delegates authentication
to {@link AuthenticationManager}.
*/
public class IgniteAuthenticationProvider implements AuthenticationProvider,
ResourceHolder {
+ private static final IgniteLogger LOG =
Loggers.forClass(IgniteAuthenticationProvider.class);
+
private AuthenticationManager authenticationManager;
IgniteAuthenticationProvider(AuthenticationManager authenticationManager) {
@@ -51,6 +55,7 @@ public class IgniteAuthenticationProvider implements
AuthenticationProvider, Res
authenticationManager.authenticateAsync(toIgniteAuthenticationRequest(authenticationRequest))
.handle((userDetails, throwable) -> {
if (throwable != null) {
+ logAuthenticationFailure(httpRequest,
authenticationRequest, throwable);
emitter.error(AuthenticationResponse.exception(throwable.getMessage()));
} else {
emitter.next(AuthenticationResponse.success(userDetails.username()));
@@ -60,11 +65,22 @@ public class IgniteAuthenticationProvider implements
AuthenticationProvider, Res
return null;
});
} catch (InvalidCredentialsException ex) {
+ logAuthenticationFailure(httpRequest, authenticationRequest,
ex);
emitter.error(AuthenticationResponse.exception(ex.getMessage()));
}
}, FluxSink.OverflowStrategy.ERROR);
}
+ private static void logAuthenticationFailure(
+ HttpRequest<?> httpRequest,
+ AuthenticationRequest<?, ?> authenticationRequest,
+ Throwable throwable
+ ) {
+ LOG.warn("REST authentication failed [uri={}, remoteAddress={},
identity={}]: {}",
+ httpRequest.getUri(), httpRequest.getRemoteAddress(),
authenticationRequest.getIdentity(),
+ throwable.getMessage());
+ }
+
private static
org.apache.ignite.internal.security.authentication.AuthenticationRequest<?, ?>
toIgniteAuthenticationRequest(
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 543e9ee19c6..beff6a290aa 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
@@ -21,6 +21,7 @@ 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 static org.apache.ignite.internal.util.ExceptionUtils.unwrapCause;
import java.util.Comparator;
import java.util.Iterator;
@@ -185,9 +186,10 @@ public class AuthenticationManagerImpl
return authenticator.authenticateAsync(authenticationRequest)
.handle((userDetails, throwable) -> {
if (throwable != null) {
- if (!(throwable instanceof
InvalidCredentialsException
- || throwable instanceof
UnsupportedAuthenticationTypeException)) {
- LOG.error("Unexpected exception during
authentication", throwable);
+ Throwable cause = unwrapCause(throwable);
+ if (!(cause instanceof InvalidCredentialsException
+ || cause instanceof
UnsupportedAuthenticationTypeException)) {
+ LOG.error("Unexpected exception during
authentication", cause);
}
logAuthenticationFailure(authenticationRequest);