IGNITE-28303 Clients: do not retry authentication errors

modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/SqlExceptionHandler.java

@@ -31,6 +31,7 @@
 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 @@ private static ErrorUiComponent connectionErrUiComponent(IgniteException e) {
         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 @@ private static ErrorUiComponent connectionErrUiComponent(IgniteException e) {
         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) {

modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientAuthenticationTest.java

@@ -35,6 +35,7 @@
 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 @@ void setUp() {
 
         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,31 +180,33 @@ void connectionIsClosedIfUserRemoved() {
     }
 
     @Test
-    void renameBasicProviderAndThenChangeUserPassword() {
+    void renameBasicProviderAndThenChangeUserPassword() throws InterruptedException {
         updateClusterConfiguration("ignite {\n"
                 + "security.authentication.providers.basic={\n"
                 + "type=basic,\n"
                 + "users=[{username=newuser,password=newpassword}]},"
                 + "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));
+            }
+        });
     }
 
     /**

modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientSqlTest.java

@@ -626,7 +626,7 @@ void testResultSetMappingColumnNameMismatch() {
                 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

modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java

@@ -73,6 +73,7 @@
 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 @@ private boolean shouldRetry(
             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;
         }
 

modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java

@@ -77,6 +77,7 @@
 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 @@ private <T> void completeAsync(
 
             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 @@ private CompletableFuture<Object> handshakeAsync(ProtocolVersion ver) throws Ign
                 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);
         });
     }

modules/client/src/test/java/org/apache/ignite/client/ClientAuthenticationTest.java

@@ -23,6 +23,7 @@
 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 void testAuthnOnClientAuthnOnServer() {
         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())

modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java

@@ -579,32 +579,31 @@ public void testNonNullableColumnWithDefaultValueSetNullThrowsException() {
 
     @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());
     }
 

modules/platforms/dotnet/Apache.Ignite.Tests/BasicAuthenticatorTests.cs

@@ -64,6 +64,27 @@ public async Task TestAuthnOnServerNoAuthnOnClient()
         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 @@ await TestUtils.WaitForConditionAsync(async () =>
 
         _authnEnabled = enable;
     }
+
+    private class TestRetryPolicy : RetryLimitPolicy
+    {
+        public int InvokeCount { get; private set; }
+
+        public override bool ShouldRetry(IRetryPolicyContext context)
+        {
+            InvokeCount++;
+            return base.ShouldRetry(context);
+        }
+    }
 }

modules/platforms/dotnet/Apache.Ignite/IgniteException.cs

@@ -40,7 +40,7 @@ public class IgniteException : Exception
         public IgniteException(Guid traceId, int code, string? message, Exception? innerException = null)
             : base(message, innerException)
         {
-            TraceId = traceId;
+            TraceId = (innerException as IgniteException)?.TraceId ?? traceId;
             Code = code;
         }
 

modules/platforms/dotnet/Apache.Ignite/Internal/ClientFailoverSocket.cs

@@ -881,6 +881,12 @@ private bool ShouldRetry(Exception exception, ClientOp op, int attempt, IRetryPo
                 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;

modules/platforms/dotnet/Apache.Ignite/Internal/ClientSocket.cs

@@ -277,7 +277,7 @@ await sslStreamFactory.CreateAsync(stream, endPoint.Host, cts.Token)
                 }
 
                 throw new IgniteClientConnectionException(
-                    ErrorGroups.Client.Connection,
+                    (ex as IgniteException)?.Code ?? ErrorGroups.Client.Connection,
                     "Failed to connect to endpoint: " + endPoint.EndPoint,
                     ex);
             }