addressed comments

Signed-off-by: Lior Sventitzky <[email protected]>

Author: liorsve

glide-core/redis-rs/redis/src/cluster_async/mod.rs

@@ -2283,13 +2283,14 @@ where
                     Ok(Response::Single(Value::Okay))
                 }
                 Operation::GetUsername => {
-                    let username = core
+                    let username = match core
                         .get_cluster_param(|params| params.username.clone())
-                        .expect(MUTEX_WRITE_ERR);
-                    Ok(Response::Single(match username {
-                        Some(u) => Value::SimpleString(u),
+                        .expect(MUTEX_READ_ERR)
+                    {
+                        Some(username) => Value::SimpleString(username),
                         None => Value::Nil,
-                    }))
+                    };
+                    Ok(Response::Single(username))
                 }
             },
         }

glide-core/src/client/mod.rs

@@ -525,23 +525,28 @@ impl Client {
                     Some(ResponsePolicy::AllSucceeded),
                 ));
                 let mut cmd = redis::cmd("AUTH");
-                match self.internal_client {
-                    ClientWrapper::Cluster { ref mut client } => {
-                        if let Ok(Value::SimpleString(username)) = client.get_username().await {
-                            cmd.arg(username);
-                        }
-                    }
-                    ClientWrapper::Standalone(ref client) => {
-                        if let Some(username) = client.get_username().await {
-                            cmd.arg(username);
-                        }
-                    }
+                if let Some(username) = self.get_username().await {
+                    cmd.arg(username);
                 }
                 cmd.arg(password);
                 self.send_command(&cmd, Some(routing)).await
             }
         }
     }
+
+    async fn get_username(&mut self) -> Option<String> {
+        match &mut self.internal_client {
+            ClientWrapper::Cluster { client } => {
+                if let Ok(Value::SimpleString(username)) = client.get_username().await {
+                    return Some(username);
+                }
+            }
+            ClientWrapper::Standalone(client) => {
+                return client.get_username().await;
+            }
+        }
+        None
+    }
 }
 
 fn load_cmd(code: &[u8]) -> Cmd {

glide-core/src/client/reconnecting_connection.rs

@@ -120,7 +120,11 @@ async fn create_connection(
     discover_az: bool,
     connection_timeout: Duration,
 ) -> Result<ReconnectingConnection, (ReconnectingConnection, RedisError)> {
-    let client = connection_backend.connection_info.read().unwrap();
+    let client = {
+        let guard = connection_backend.connection_info.read().unwrap();
+        guard.clone()
+    };
+
     let connection_options = GlideConnectionOptions {
         push_sender,
         disconnect_notifier: Some::<Box<dyn DisconnectNotifier>>(Box::new(
@@ -129,6 +133,7 @@ async fn create_connection(
         discover_az,
         connection_timeout: Some(connection_timeout),
     };
+
     let action = || async {
         get_multiplexed_connection(&client, &connection_options)
             .await

node/tests/AuthTest.test.ts

@@ -80,7 +80,7 @@ describe("Auth tests", () => {
         username: string,
         password: string,
     ) {
-        await client.customCommand([
+        const result = await client.customCommand([
             "ACL",
             "SETUSER",
             username,
@@ -90,13 +90,15 @@ describe("Auth tests", () => {
             "&*",
             "+@all",
         ]);
+        expect(result).toEqual("OK");
     }
 
     async function deleteAclUsernameAndPassword(
         client: BaseClient,
         username: string,
     ) {
-        return await client.customCommand(["ACL", "DELUSER", username]);
+        const result = await client.customCommand(["ACL", "DELUSER", username]);
+        expect(result).toEqual(1);
     }
 
     afterEach(async () => {
@@ -227,10 +229,6 @@ describe("Auth tests", () => {
                 async () => {
                     await runTest(
                         async (client: BaseClient) => {
-                            // if (client instanceof GlideClient) {
-                            //     return;
-                            // }
-
                             // Update password without re-authentication
                             const result =
                                 await client.updateConnectionPassword(
@@ -257,6 +255,11 @@ describe("Auth tests", () => {
                                 "normal",
                             ]);
 
+                            // Sleep to ensure disconnection
+                            await new Promise((resolve) =>
+                                setTimeout(resolve, 1000),
+                            );
+
                             // Verify client auto-reconnects with new password
                             await client.set("test_key2", "test_value2");
                             const value2 = await client.get("test_key2");
@@ -405,8 +408,14 @@ describe("Auth tests", () => {
                             "TYPE",
                             "normal",
                         ]);
+                        // Sleep to ensure disconnection
+                        await new Promise((resolve) =>
+                            setTimeout(resolve, 1000),
+                        );
+
                         // Try updating client password without immediate re-auth and with - non immediate should succeed,
                         // immediate auth should fail (failing to reconnect)
+
                         const result = await client.updateConnectionPassword(
                             NEW_PASSWORD,
                             false,
@@ -463,6 +472,11 @@ describe("Auth tests", () => {
                                 NEW_PASSWORD,
                             );
 
+                            // Sleep to ensure disconnection
+                            await new Promise((resolve) =>
+                                setTimeout(resolve, 1000),
+                            );
+
                             // Verify client auto-reconnects with new password
                             await client.set("test_key2", "test_value2");
                             const value2 = await client.get("test_key2");
@@ -557,6 +571,11 @@ describe("Auth tests", () => {
                             NEW_PASSWORD,
                         );
 
+                        // Sleep to ensure disconnection
+                        await new Promise((resolve) =>
+                            setTimeout(resolve, 1000),
+                        );
+
                         // Try updating client password without immediate re-auth and with - non immediate should succeed,
                         // immediate auth should fail (failing to reconnect)
                         expect(

python/python/tests/conftest.py

@@ -366,18 +366,20 @@ async def config_set_new_password(client: TGlideClient, password):
 
 async def set_new_acl_username_with_password(client: TGlideClient, username, password):
     """
-    Sets a new password for the given TGlideClient server connected.
-    This function updates the server to require a new password.
+    Sets a new ACL user with the provided password
     """
-    if isinstance(client, GlideClient):
-        await client.custom_command(
-            ["ACL", "SETUSER", username, "ON", f">{password}", "~*", "&*", "+@all"]
-        )
-    elif isinstance(client, GlideClusterClient):
-        await client.custom_command(
-            ["ACL", "SETUSER", username, "ON", f">{password}", "~*", "&*", "+@all"],
-            route=AllNodes(),
-        )
+    try:
+        if isinstance(client, GlideClient):
+            await client.custom_command(
+                ["ACL", "SETUSER", username, "ON", f">{password}", "~*", "&*", "+@all"]
+            )
+        elif isinstance(client, GlideClusterClient):
+            await client.custom_command(
+                ["ACL", "SETUSER", username, "ON", f">{password}", "~*", "&*", "+@all"],
+                route=AllNodes(),
+            )
+    except Exception as e:
+        raise RuntimeError(f"Failed to set ACL user: {e}")
 
 
 async def delete_acl_username_and_password(client: TGlideClient, username):
@@ -386,6 +388,7 @@ async def delete_acl_username_and_password(client: TGlideClient, username):
     """
     if isinstance(client, GlideClient):
         return await client.custom_command(["ACL", "DELUSER", username])
+
     elif isinstance(client, GlideClusterClient):
         return await client.custom_command(
             ["ACL", "DELUSER", username], route=AllNodes()

python/python/tests/test_auth.py

@@ -41,6 +41,46 @@ async def cleanup(self, request, management_client: TGlideClient):
         except RequestError:
             pass
 
+    @pytest.mark.parametrize("cluster_mode", [True, False])
+    @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3])
+    async def test_update_connection_password(
+        self, glide_client: TGlideClient, management_client: TGlideClient
+    ):
+        """
+        Test replacing the connection password without immediate re-authentication.
+        Verifies that:
+        1. The client can update its internal password
+        2. The client remains connected with current auth
+        3. The client can reconnect using the new password after server password change
+        This test is only for cluster mode, as standalone mode does not have a connection available handler
+        """
+        result = await glide_client.update_connection_password(
+            NEW_PASSWORD, immediate_auth=False
+        )
+        assert result == OK
+        # Verify that the client is still authenticated
+        assert await glide_client.set("test_key", "test_value") == OK
+        value = await glide_client.get("test_key")
+        assert value == b"test_value"
+        await config_set_new_password(glide_client, NEW_PASSWORD)
+        await kill_connections(management_client)
+        # Add a short delay to allow the server to apply the new password
+        # without this delay, command may or may not time out while the client reconnect
+        # ending up with a flaky test
+        await asyncio.sleep(2)
+        # Verify that the client is able to reconnect with the new password,
+        value = await glide_client.get("test_key")
+        assert value == b"test_value"
+        await kill_connections(management_client)
+        await asyncio.sleep(2)
+        # Verify that the client is able to immediateAuth with the new password after client is killed
+        result = await glide_client.update_connection_password(
+            NEW_PASSWORD, immediate_auth=True
+        )
+        assert result == OK
+        # Verify that the client is still authenticated
+        assert await glide_client.set("test_key", "test_value") == OK
+
     @pytest.mark.parametrize("cluster_mode", [False])
     @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3])
     async def test_update_connection_password_connection_lost_before_password_update(
@@ -54,7 +94,7 @@ async def test_update_connection_password_connection_lost_before_password_update
         await glide_client.set("test_key", "test_value")
         await config_set_new_password(glide_client, NEW_PASSWORD)
         await kill_connections(management_client)
-        await asyncio.sleep(1)
+        await asyncio.sleep(2)
         result = await glide_client.update_connection_password(
             NEW_PASSWORD, immediate_auth=False
         )
@@ -176,6 +216,9 @@ async def test_update_connection_password_with_acl_user(
             management_client, USERNAME, NEW_PASSWORD
         )
 
+        # Sleep to allow enough time for reconnecting
+        await asyncio.sleep(2)
+
         # The client should now reconnect with the new password automatically
         # Verify that the client is still able to perform operations
         value = await acl_glide_client.get("test_key")
@@ -208,6 +251,9 @@ async def test_update_connection_password_reconnection_with_immediate_auth_with_
             management_client, USERNAME, NEW_PASSWORD
         )
 
+        # Sleep to allow enough time for reconnecting
+        await asyncio.sleep(2)
+
         result = await acl_glide_client.update_connection_password(
             NEW_PASSWORD, immediate_auth=True
         )