Changed Connection class members username and password to properties to enable backward compatibility with changing the members value on existing connection.

Author: barshaul

redis/connection.py

@@ -8,6 +8,7 @@
 from itertools import chain
 from queue import Empty, Full, LifoQueue
 from time import time
+from typing import Optional
 from urllib.parse import parse_qs, unquote, urlparse
 
 from redis.backoff import NoBackoff
@@ -526,8 +527,6 @@ def __init__(
             )
 
         self.credential_provider = credential_provider
-        self.username = username
-        self.password = password
         if username or password:
             # Keep backward compatibility by creating a static credential provider
             # for the passed username and password
@@ -564,6 +563,38 @@ def __init__(
         self._connect_callbacks = []
         self._buffer_cutoff = 6000
 
+    @property
+    def password(self) -> Optional[str]:
+        if self.credential_provider is not None:
+            return self.credential_provider.password
+        else:
+            return None
+
+    @password.setter
+    def password(self, value: Optional[str]):
+        if value is None:
+            # Delete the credential provider
+            self.credential_provider = None
+            return
+        if self.credential_provider is not None:
+            self.credential_provider.password = value
+        else:
+            self.credential_provider = StaticCredentialProvider(password=value)
+
+    @property
+    def username(self) -> Optional[str]:
+        if self.credential_provider is not None:
+            return self.credential_provider.username
+        else:
+            return None
+
+    @username.setter
+    def username(self, value: Optional[str]):
+        if self.credential_provider is not None:
+            self.credential_provider.username = value
+        else:
+            self.credential_provider = StaticCredentialProvider(username=value)
+
     def __repr__(self):
         repr_args = ",".join([f"{k}={v}" for k, v in self.repr_pieces()])
         return f"{self.__class__.__name__}<{repr_args}>"
@@ -1087,8 +1118,6 @@ def __init__(
                 "2. 'credential_provider'"
             )
         self.credential_provider = credential_provider
-        self.username = username
-        self.password = password
         if username or password:
             # Keep backward compatibility by creating a static credential provider
             # for the passed username and password
@@ -1121,6 +1150,38 @@ def __init__(
         self._connect_callbacks = []
         self._buffer_cutoff = 6000
 
+    @property
+    def password(self) -> Optional[str]:
+        if self.credential_provider is not None:
+            return self.credential_provider.password
+        else:
+            return None
+
+    @password.setter
+    def password(self, value: Optional[str]):
+        if value is None:
+            # Delete the credential provider
+            self.credential_provider = None
+            return
+        if self.credential_provider is not None:
+            self.credential_provider.password = value
+        else:
+            self.credential_provider = StaticCredentialProvider(password=value)
+
+    @property
+    def username(self) -> Optional[str]:
+        if self.credential_provider is not None:
+            return self.credential_provider.username
+        else:
+            return None
+
+    @username.setter
+    def username(self, value: Optional[str]):
+        if self.credential_provider is not None:
+            self.credential_provider.username = value
+        else:
+            self.credential_provider = StaticCredentialProvider(username=value)
+
     def repr_pieces(self):
         pieces = [("path", self.path), ("db", self.db)]
         if self.client_name:

redis/credentials.py

@@ -18,8 +18,8 @@ def supplier(self, arg1, arg2, ...) -> (username, password)
         :param supplier_args: arguments to pass to the supplier function
         :param supplier_kwargs: keyword arguments to pass to the supplier function
         """
-        self._username_ = "" if username is None else username
-        self._password_ = "" if password is None else password
+        self._username_ = username if username is not None else ""
+        self._password_ = password if password is not None else ""
         self.supplier = supplier
         self.supplier_args = supplier_args
         self.supplier_kwargs = supplier_kwargs
@@ -46,7 +46,7 @@ def password(self):
 
     @password.setter
     def password(self, value):
-        self._password_ = value
+        self._password_ = value if value is not None else ""
 
     @property
     def username(self):
@@ -58,7 +58,7 @@ def username(self):
 
     @username.setter
     def username(self, value):
-        self._username_ = value
+        self._username_ = value if value is not None else ""
 
 
 class StaticCredentialProvider(CredentialProvider):
@@ -70,8 +70,4 @@ class StaticCredentialProvider(CredentialProvider):
     def __init__(
         self, username: Union[str, None] = "", password: Union[str, None] = ""
     ):
-        super().__init__(
-            username=username,
-            password=password,
-            credential_provider=lambda self: (self.username, self.password),
-        )
+        super().__init__(username=username, password=password)

tests/test_credentials.py

@@ -6,6 +6,7 @@
 import redis
 from redis import AuthenticationError, DataError, ResponseError
 from redis.credentials import CredentialProvider, StaticCredentialProvider
+from redis.utils import str_if_bytes
 from tests.conftest import _get_client, skip_if_redis_enterprise
 
 
@@ -179,6 +180,34 @@ def test_password_and_username_together_with_cred_provider_raise_error(
             "'credential_provider'."
         )
 
+    @pytest.mark.onlynoncluster
+    def test_change_username_password_on_existing_connection(self, r, request):
+        username = "origin_username"
+        password = "origin_password"
+        new_username = "new_username"
+        new_password = "new_password"
+        init_acl_user(r, request, username, password)
+        r2 = _get_client(
+            redis.Redis, request, flushdb=False, username=username, password=password
+        )
+        assert r2.ping()
+        conn = r2.connection_pool.get_connection("_")
+        conn.send_command("PING")
+        assert str_if_bytes(conn.read_response()) == "PONG"
+        assert conn.username == username
+        assert conn.password == password
+        init_acl_user(r, request, new_username, new_password)
+        conn.password = new_password
+        conn.username = new_username
+        assert conn.credential_provider.password == new_password
+        assert conn.credential_provider.username == new_username
+        conn.send_command("PING")
+        assert str_if_bytes(conn.read_response()) == "PONG"
+        conn.username = None
+        assert conn.credential_provider.username == ""
+        conn.password = None
+        assert conn.credential_provider is None
+
 
 class TestStaticCredentialProvider:
     def test_static_credential_provider_acl_user_and_pass(self, r, request):