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 4d034f44035 IGNITE-28303 Clients: do not retry authentication errors
(#7955)
4d034f44035 is described below
commit 4d034f440351b87357aa5e961517b86833c4e796
Author: Pavel Tupitsyn <[email protected]>
AuthorDate: Tue Apr 14 16:01:26 2026 +0300
IGNITE-28303 Clients: do not retry authentication errors (#7955)
Fix authn error handing in .NET and Java clients:
- Propagate authn exceptions correctly
- Do not invoke `RetryPolicy` for those errors
---
.../exception/handler/SqlExceptionHandler.java | 24 ++++++-----
.../client/ItThinClientAuthenticationTest.java | 46 +++++++++++++---------
.../internal/client/ItThinClientSqlTest.java | 2 +-
.../ignite/internal/client/ReliableChannel.java | 4 +-
.../ignite/internal/client/TcpClientChannel.java | 11 ++++++
.../ignite/client/ClientAuthenticationTest.java | 18 +++++++++
.../ignite/client/ClientKeyValueViewTest.java | 13 +++---
.../Apache.Ignite.Tests/BasicAuthenticatorTests.cs | 32 +++++++++++++++
.../dotnet/Apache.Ignite/IgniteException.cs | 2 +-
.../Apache.Ignite/Internal/ClientFailoverSocket.cs | 6 +++
.../dotnet/Apache.Ignite/Internal/ClientSocket.cs | 2 +-
11 files changed, 121 insertions(+), 39 deletions(-)
diff --git
a/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/SqlExceptionHandler.java
b/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/SqlExceptionHandler.java
index 4bdd8ae18d7..5419f91043d 100644
---
a/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/SqlExceptionHandler.java
+++
b/modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/SqlExceptionHandler.java
@@ -31,6 +31,7 @@ import
org.apache.ignite.internal.cli.core.style.component.ErrorUiComponent;
import org.apache.ignite.internal.logger.IgniteLogger;
import org.apache.ignite.internal.logger.Loggers;
import org.apache.ignite.internal.util.ExceptionUtils;
+import org.apache.ignite.lang.ErrorGroups.Authentication;
import org.apache.ignite.lang.ErrorGroups.Client;
import org.apache.ignite.lang.ErrorGroups.Sql;
import org.apache.ignite.lang.IgniteCheckedException;
@@ -54,6 +55,7 @@ public class SqlExceptionHandler implements
ExceptionHandler<SQLException> {
/** Default constructor. */
private SqlExceptionHandler() {
+
sqlExceptionMappers.put(Authentication.AUTHENTICATION_ERR_GROUP.groupCode(),
SqlExceptionHandler::authnErrUiComponent);
sqlExceptionMappers.put(Client.CLIENT_ERR_GROUP.groupCode(),
SqlExceptionHandler::connectionErrUiComponent);
sqlExceptionMappers.put(Sql.SQL_ERR_GROUP.groupCode(),
SqlExceptionHandler::sqlErrUiComponent);
}
@@ -66,15 +68,6 @@ public class SqlExceptionHandler implements
ExceptionHandler<SQLException> {
if (e.getCause() instanceof IgniteClientConnectionException) {
IgniteClientConnectionException cause =
(IgniteClientConnectionException) e.getCause();
- InvalidCredentialsException invalidCredentialsException =
findCause(cause, InvalidCredentialsException.class);
- if (invalidCredentialsException != null) {
- return ErrorUiComponent.builder()
- .header("Could not connect to node. Check
authentication configuration")
- .details(invalidCredentialsException.getMessage())
- .verbose(extractCauseMessage(cause.getMessage()))
- .build();
- }
-
SSLHandshakeException sslHandshakeException = findCause(cause,
SSLHandshakeException.class);
if (sslHandshakeException != null) {
return ErrorUiComponent.builder()
@@ -90,6 +83,19 @@ public class SqlExceptionHandler implements
ExceptionHandler<SQLException> {
return fromIgniteException(CLIENT_CONNECTION_FAILED_MESSAGE, e);
}
+ private static ErrorUiComponent authnErrUiComponent(IgniteException e) {
+ InvalidCredentialsException invalidCredentialsException = findCause(e,
InvalidCredentialsException.class);
+ if (invalidCredentialsException != null) {
+ return ErrorUiComponent.builder()
+ .header("Could not connect to node. Check authentication
configuration")
+ .details(invalidCredentialsException.getMessage())
+ .verbose(extractCauseMessage(e.getMessage()))
+ .build();
+ }
+
+ return fromIgniteException("Could not connect to node. Check
authentication configuration", e);
+ }
+
@Nullable
private static <T extends Throwable> T findCause(Throwable e, Class<T>
type) {
while (e != null) {
diff --git
a/modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientAuthenticationTest.java
b/modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientAuthenticationTest.java
index 4e8cc7b754e..59602cda976 100644
---
a/modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientAuthenticationTest.java
+++
b/modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientAuthenticationTest.java
@@ -35,6 +35,7 @@ import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import org.apache.ignite.client.BasicAuthenticator;
import org.apache.ignite.client.IgniteClient;
+import org.apache.ignite.client.IgniteClient.Builder;
import org.apache.ignite.internal.app.IgniteImpl;
import org.apache.ignite.internal.configuration.ConfigurationRegistry;
import
org.apache.ignite.internal.security.authentication.basic.BasicAuthenticationProviderChange;
@@ -98,12 +99,17 @@ public class ItThinClientAuthenticationTest extends
ItAbstractThinClientTest {
assertThat(enableAuthentication, willCompleteSuccessfully());
- clientWithAuth = IgniteClient.builder()
+ Builder builder = IgniteClient.builder()
.authenticator(basicAuthenticator)
- .addresses(getClientAddresses().toArray(new String[0]))
- .build();
+ .addresses(getClientAddresses().toArray(new String[0]));
+
+ await().untilAsserted(() -> {
+ try (IgniteClient c = builder.build()) {
+ assertThat(checkConnection(c), willCompleteSuccessfully());
+ }
+ });
- await().untilAsserted(() -> checkConnection(clientWithAuth));
+ clientWithAuth = builder.build();
}
@AfterEach
@@ -174,7 +180,7 @@ public class ItThinClientAuthenticationTest extends
ItAbstractThinClientTest {
}
@Test
- void renameBasicProviderAndThenChangeUserPassword() {
+ void renameBasicProviderAndThenChangeUserPassword() throws
InterruptedException {
updateClusterConfiguration("ignite {\n"
+ "security.authentication.providers.basic={\n"
+ "type=basic,\n"
@@ -182,23 +188,25 @@ public class ItThinClientAuthenticationTest extends
ItAbstractThinClientTest {
+ "security.authentication.providers.default=null\n"
+ "}");
- try (IgniteClient client = IgniteClient.builder()
-
.authenticator(BasicAuthenticator.builder().username("newuser").password("newpassword").build())
- .addresses(getClientAddresses().toArray(new String[0]))
- .build()) {
+ await().untilAsserted(() -> {
+ try (IgniteClient client = IgniteClient.builder()
+
.authenticator(BasicAuthenticator.builder().username("newuser").password("newpassword").build())
+ .addresses(getClientAddresses().toArray(new String[0]))
+ .build()) {
- checkConnection(client);
+ checkConnection(client);
- securityConfiguration.authentication().providers()
- .get("basic")
- .change(change -> {
- change.convert(BasicAuthenticationProviderChange.class)
- .changeUsers()
- .update("newuser", user ->
user.changePassword("newpassword-changed"));
- }).join();
+ securityConfiguration.authentication().providers()
+ .get("basic")
+ .change(change -> {
+
change.convert(BasicAuthenticationProviderChange.class)
+ .changeUsers()
+ .update("newuser", user ->
user.changePassword("newpassword-changed"));
+ }).join();
- await().until(() -> checkConnection(client),
willThrowWithCauseOrSuppressed(InvalidCredentialsException.class));
- }
+ await().until(() -> checkConnection(client),
willThrowWithCauseOrSuppressed(InvalidCredentialsException.class));
+ }
+ });
}
/**
diff --git
a/modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientSqlTest.java
b/modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientSqlTest.java
index 9115b6bc7a6..254e69ad6db 100644
---
a/modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientSqlTest.java
+++
b/modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientSqlTest.java
@@ -626,7 +626,7 @@ public class ItThinClientSqlTest extends
ItAbstractThinClientTest {
IgniteException.class,
() -> client().sql().execute((Transaction) null,
Mapper.of(Pojo.class), query));
- assertEquals("Failed to deserialize server response for op 50: No
mapped object field found for column 'FOO'", e.getMessage());
+ assertEquals("No mapped object field found for column 'FOO'",
e.getMessage());
}
@Test
diff --git
a/modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java
b/modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java
index f5774ea0b03..041f32997d4 100644
---
a/modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java
+++
b/modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java
@@ -73,6 +73,7 @@ import org.apache.ignite.internal.hlc.HybridTimestampTracker;
import org.apache.ignite.internal.logger.IgniteLogger;
import org.apache.ignite.internal.thread.NamedThreadFactory;
import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.lang.ErrorGroups.Authentication;
import org.apache.ignite.lang.IgniteException;
import org.apache.ignite.network.ClusterNode;
import org.jetbrains.annotations.Nullable;
@@ -779,7 +780,8 @@ public final class ReliableChannel implements AutoCloseable
{
return false;
}
- if (exception.code() == CLUSTER_ID_MISMATCH_ERR) {
+ if (exception.code() == CLUSTER_ID_MISMATCH_ERR
+ || exception.groupCode() ==
Authentication.AUTHENTICATION_ERR_GROUP.groupCode()) {
return false;
}
diff --git
a/modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java
b/modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java
index 5c198f1da98..6ec7fdac4c4 100644
---
a/modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java
+++
b/modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java
@@ -77,6 +77,7 @@ import org.apache.ignite.internal.tostring.S;
import org.apache.ignite.internal.util.ViewUtils;
import org.apache.ignite.lang.ErrorGroups.Table;
import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.TraceableException;
import org.apache.ignite.network.NetworkAddress;
import org.apache.ignite.sql.SqlBatchException;
import org.apache.ignite.tx.TransactionException;
@@ -533,6 +534,10 @@ class TcpClientChannel implements ClientChannel,
ClientMessageHandler, ClientCon
return null;
} catch (Throwable e) {
+ if (e instanceof TraceableException) {
+ throw sneakyThrow(e);
+ }
+
log.error("Failed to deserialize server response [remoteAddress="
+ cfg.getAddress() + ", opCode=" + opCode + "]: "
+ e.getMessage(), e);
@@ -754,7 +759,13 @@ class TcpClientChannel implements ClientChannel,
ClientMessageHandler, ClientCon
metrics.handshakesFailedTimeoutIncrement();
throw new IgniteClientConnectionException(CONNECTION_ERR,
"Handshake timeout", endpoint(), err);
}
+
metrics.handshakesFailedIncrement();
+
+ if (err instanceof TraceableException) {
+ throw new
IgniteClientConnectionException(((TraceableException) err).code(), "Handshake
error", endpoint(), err);
+ }
+
throw new IgniteClientConnectionException(CONNECTION_ERR,
"Handshake error", endpoint(), err);
});
}
diff --git
a/modules/client/src/test/java/org/apache/ignite/client/ClientAuthenticationTest.java
b/modules/client/src/test/java/org/apache/ignite/client/ClientAuthenticationTest.java
index 96704bffb46..2b71f45e4d4 100644
---
a/modules/client/src/test/java/org/apache/ignite/client/ClientAuthenticationTest.java
+++
b/modules/client/src/test/java/org/apache/ignite/client/ClientAuthenticationTest.java
@@ -23,6 +23,7 @@ import static
org.apache.ignite.internal.security.authentication.SecurityConfigu
import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
import static org.apache.ignite.internal.util.IgniteUtils.closeAll;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import java.util.UUID;
import org.apache.ignite.client.fakes.FakeIgnite;
@@ -93,6 +94,23 @@ public class ClientAuthenticationTest extends
BaseIgniteAbstractTest {
client =
startClient(BasicAuthenticator.builder().username(DEFAULT_USERNAME).password(DEFAULT_PASSWORD).build());
}
+ @Test
+ public void testBadCredentialsAreNotRetried() {
+ server = startServer(true);
+
+ BasicAuthenticator authenticator =
BasicAuthenticator.builder().username("u").password("wrong").build();
+
+ var builder = IgniteClient.builder()
+ .addresses("127.0.0.1:" + server.port())
+ .authenticator(authenticator)
+ .retryPolicy(new RetryLimitPolicy().retryLimit(5));
+
+ IgniteTestUtils.assertThrowsWithCause(builder::build,
InvalidCredentialsException.class, "Authentication failed");
+
+ // The server should have received exactly one connection attempt, no
retries.
+ assertEquals(1, server.metrics().sessionsRejected());
+ }
+
private IgniteClient startClient(@Nullable IgniteClientAuthenticator
authenticator) {
return IgniteClient.builder()
.addresses("127.0.0.1:" + server.port())
diff --git
a/modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java
b/modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java
index 72c68f3e3df..de04be723db 100644
---
a/modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java
+++
b/modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java
@@ -579,32 +579,31 @@ public class ClientKeyValueViewTest extends
AbstractClientTableTest {
@Test
public void testGetNullValueThrows() {
- testNullValueThrows(view -> view.get(null, DEFAULT_ID), "getNullable",
12);
+ testNullValueThrows(view -> view.get(null, DEFAULT_ID), "getNullable");
}
@Test
public void testGetAndPutNullValueThrows() {
- testNullValueThrows(view -> view.getAndPut(null, DEFAULT_ID,
DEFAULT_NAME), "getNullableAndPut", 16);
+ testNullValueThrows(view -> view.getAndPut(null, DEFAULT_ID,
DEFAULT_NAME), "getNullableAndPut");
}
@Test
public void testGetAndRemoveNullValueThrows() {
- testNullValueThrows(view -> view.getAndRemove(null, DEFAULT_ID),
"getNullableAndRemove", 32);
+ testNullValueThrows(view -> view.getAndRemove(null, DEFAULT_ID),
"getNullableAndRemove");
}
@Test
public void testGetAndReplaceNullValueThrows() {
- testNullValueThrows(view -> view.getAndReplace(null, DEFAULT_ID,
DEFAULT_NAME), "getNullableAndReplace", 26);
+ testNullValueThrows(view -> view.getAndReplace(null, DEFAULT_ID,
DEFAULT_NAME), "getNullableAndReplace");
}
- private void testNullValueThrows(Consumer<KeyValueView<Long, String>> run,
String methodName, int op) {
+ private void testNullValueThrows(Consumer<KeyValueView<Long, String>> run,
String methodName) {
KeyValueView<Long, String> primitiveView =
defaultTable().keyValueView(Mapper.of(Long.class), Mapper.of(String.class));
primitiveView.put(null, DEFAULT_ID, null);
var ex = assertThrowsWithCause(() -> run.accept(primitiveView),
UnexpectedNullValueException.class);
assertEquals(
- format("Failed to deserialize server response for op {}: Got
unexpected null value: use `{}` sibling method instead.",
- op, methodName),
+ format("Got unexpected null value: use `{}` sibling method
instead.", methodName),
ex.getMessage());
}
diff --git
a/modules/platforms/dotnet/Apache.Ignite.Tests/BasicAuthenticatorTests.cs
b/modules/platforms/dotnet/Apache.Ignite.Tests/BasicAuthenticatorTests.cs
index 711a1368e77..11a93ab75e0 100644
--- a/modules/platforms/dotnet/Apache.Ignite.Tests/BasicAuthenticatorTests.cs
+++ b/modules/platforms/dotnet/Apache.Ignite.Tests/BasicAuthenticatorTests.cs
@@ -64,6 +64,27 @@ public class BasicAuthenticatorTests : IgniteTestsBase
Assert.AreEqual(ErrorGroups.Authentication.InvalidCredentials,
invalidCredentialsException.Code);
}
+ [Test]
+ public async Task TestBadCredentialsAreNotRetried()
+ {
+ await EnableAuthn(true);
+
+ var retryPolicy = new TestRetryPolicy();
+ var cfg = new IgniteClientConfiguration(GetConfig())
+ {
+ RetryPolicy = retryPolicy,
+ Authenticator = new BasicAuthenticator
+ {
+ Username = "user-1",
+ Password = "wrong"
+ }
+ };
+
+ var ex = Assert.ThrowsAsync<IgniteClientConnectionException>(async ()
=> await IgniteClient.StartAsync(cfg));
+ Assert.IsInstanceOf<InvalidCredentialsException>(ex.InnerException);
+ Assert.AreEqual(0, retryPolicy.InvokeCount, "Retry policy should not
be invoked");
+ }
+
[Test]
public async Task TestAuthnOnClientAndServer()
{
@@ -150,4 +171,15 @@ public class BasicAuthenticatorTests : IgniteTestsBase
_authnEnabled = enable;
}
+
+ private class TestRetryPolicy : RetryLimitPolicy
+ {
+ public int InvokeCount { get; private set; }
+
+ public override bool ShouldRetry(IRetryPolicyContext context)
+ {
+ InvokeCount++;
+ return base.ShouldRetry(context);
+ }
+ }
}
diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteException.cs
b/modules/platforms/dotnet/Apache.Ignite/IgniteException.cs
index be517e3ea73..c1be493a438 100644
--- a/modules/platforms/dotnet/Apache.Ignite/IgniteException.cs
+++ b/modules/platforms/dotnet/Apache.Ignite/IgniteException.cs
@@ -40,7 +40,7 @@ namespace Apache.Ignite
public IgniteException(Guid traceId, int code, string? message,
Exception? innerException = null)
: base(message, innerException)
{
- TraceId = traceId;
+ TraceId = (innerException as IgniteException)?.TraceId ?? traceId;
Code = code;
}
diff --git
a/modules/platforms/dotnet/Apache.Ignite/Internal/ClientFailoverSocket.cs
b/modules/platforms/dotnet/Apache.Ignite/Internal/ClientFailoverSocket.cs
index cbeca14e367..1ba4ed359e5 100644
--- a/modules/platforms/dotnet/Apache.Ignite/Internal/ClientFailoverSocket.cs
+++ b/modules/platforms/dotnet/Apache.Ignite/Internal/ClientFailoverSocket.cs
@@ -881,6 +881,12 @@ namespace Apache.Ignite.Internal
return false;
}
+ if (e is IgniteException { GroupCode:
ErrorGroups.Authentication.GroupCode })
+ {
+ // Authentication errors should not be retried.
+ return false;
+ }
+
if (retryPolicy is null or RetryNonePolicy)
{
return false;
diff --git a/modules/platforms/dotnet/Apache.Ignite/Internal/ClientSocket.cs
b/modules/platforms/dotnet/Apache.Ignite/Internal/ClientSocket.cs
index e8b5e58f9ec..4fcf9d675e0 100644
--- a/modules/platforms/dotnet/Apache.Ignite/Internal/ClientSocket.cs
+++ b/modules/platforms/dotnet/Apache.Ignite/Internal/ClientSocket.cs
@@ -277,7 +277,7 @@ namespace Apache.Ignite.Internal
}
throw new IgniteClientConnectionException(
- ErrorGroups.Client.Connection,
+ (ex as IgniteException)?.Code ??
ErrorGroups.Client.Connection,
"Failed to connect to endpoint: " + endPoint.EndPoint,
ex);
}
