From b8a87f1c571aee48aa2f168aefdca9509ac53915 Mon Sep 17 00:00:00 2001 From: Steven B <51370195+sdb9696@users.noreply.github.com> Date: Tue, 2 Jul 2024 13:43:37 +0100 Subject: [PATCH] Fix credential hash to return None on empty credentials (#1029) If discovery is triggered without credentials and discovers devices requiring authentication, blank credentials are used to initialise the protocols and no connection is actually made. In this instance we should not return the credentials_hash for blank credentials as it will be invalid. --- kasa/aestransport.py | 4 ++- kasa/klaptransport.py | 4 ++- kasa/protocol.py | 2 +- kasa/tests/fakeprotocol_iot.py | 4 +-- kasa/tests/test_protocol.py | 64 +++++++++++++++++++++++++++++++++- kasa/xortransport.py | 4 +-- 6 files changed, 74 insertions(+), 8 deletions(-) diff --git a/kasa/aestransport.py b/kasa/aestransport.py index cc373b19..c9cb83bd 100644 --- a/kasa/aestransport.py +++ b/kasa/aestransport.py @@ -117,8 +117,10 @@ class AesTransport(BaseTransport): return self.DEFAULT_PORT @property - def credentials_hash(self) -> str: + def credentials_hash(self) -> str | None: """The hashed credentials used by the transport.""" + if self._credentials == Credentials(): + return None return base64.b64encode(json_dumps(self._login_params).encode()).decode() def _get_login_params(self, credentials: Credentials) -> dict[str, str]: diff --git a/kasa/klaptransport.py b/kasa/klaptransport.py index 3a1eb336..dd90ffd2 100644 --- a/kasa/klaptransport.py +++ b/kasa/klaptransport.py @@ -132,8 +132,10 @@ class KlapTransport(BaseTransport): return self.DEFAULT_PORT @property - def credentials_hash(self) -> str: + def credentials_hash(self) -> str | None: """The hashed credentials used by the transport.""" + if self._credentials == Credentials(): + return None return base64.b64encode(self._local_auth_hash).decode() async def perform_handshake1(self) -> tuple[bytes, bytes, bytes]: diff --git a/kasa/protocol.py b/kasa/protocol.py index c7d505b8..7d717c5e 100755 --- a/kasa/protocol.py +++ b/kasa/protocol.py @@ -59,7 +59,7 @@ class BaseTransport(ABC): @property @abstractmethod - def credentials_hash(self) -> str: + def credentials_hash(self) -> str | None: """The hashed credentials used by the transport.""" @abstractmethod diff --git a/kasa/tests/fakeprotocol_iot.py b/kasa/tests/fakeprotocol_iot.py index 52320598..9c5f655c 100644 --- a/kasa/tests/fakeprotocol_iot.py +++ b/kasa/tests/fakeprotocol_iot.py @@ -234,8 +234,8 @@ class FakeIotTransport(BaseTransport): return 9999 @property - def credentials_hash(self) -> str: - return "" + def credentials_hash(self) -> None: + return None def set_alias(self, x, child_ids=None): if child_ids is None: diff --git a/kasa/tests/test_protocol.py b/kasa/tests/test_protocol.py index e0ddbbb4..1aeeedb2 100644 --- a/kasa/tests/test_protocol.py +++ b/kasa/tests/test_protocol.py @@ -13,6 +13,7 @@ import pytest from ..aestransport import AesTransport from ..credentials import Credentials +from ..device import Device from ..deviceconfig import DeviceConfig from ..exceptions import KasaException from ..iotprotocol import IotProtocol, _deprecated_TPLinkSmartHomeProtocol @@ -512,11 +513,72 @@ def test_transport_init_signature(class_name_obj): ) +@pytest.mark.parametrize( + ("transport_class", "login_version", "expected_hash"), + [ + pytest.param( + AesTransport, + 1, + "eyJwYXNzd29yZCI6IlFtRnkiLCJ1c2VybmFtZSI6Ik1qQXhZVFppTXpBMU0yTmpNVFF5TW1ReVl6TTJOekJpTmpJMk1UWXlNakZrTWpJNU1Ea3lPUT09In0=", + id="aes-lv-1", + ), + pytest.param( + AesTransport, + 2, + "eyJwYXNzd29yZDIiOiJaVFE1Tm1aa01qQXhNelprTkdKaU56Z3lPR1ZpWWpCaFlqa3lOV0l4WW1RNU56Y3lNRGhsTkE9PSIsInVzZXJuYW1lIjoiTWpBeFlUWmlNekExTTJOak1UUXlNbVF5WXpNMk56QmlOakkyTVRZeU1qRmtNakk1TURreU9RPT0ifQ==", + id="aes-lv-2", + ), + pytest.param(KlapTransport, 1, "xBhMRGYWStVCVk9aSD8/6Q==", id="klap-lv-1"), + pytest.param(KlapTransport, 2, "xBhMRGYWStVCVk9aSD8/6Q==", id="klap-lv-2"), + pytest.param( + KlapTransportV2, + 1, + "tEmiensOcZkP9twDEZKwU3JJl3asmseKCP7N9sfatVo=", + id="klapv2-lv-1", + ), + pytest.param( + KlapTransportV2, + 2, + "tEmiensOcZkP9twDEZKwU3JJl3asmseKCP7N9sfatVo=", + id="klapv2-lv-2", + ), + pytest.param(XorTransport, None, None, id="xor"), + ], +) +@pytest.mark.parametrize( + ("credentials", "expected_blank"), + [ + pytest.param(Credentials("Foo", "Bar"), False, id="credentials"), + pytest.param(None, True, id="no-credentials"), + pytest.param(Credentials(None, "Bar"), True, id="no-username"), # type: ignore[arg-type] + ], +) +async def test_transport_credentials_hash( + mocker, transport_class, login_version, expected_hash, credentials, expected_blank +): + """Test that the actual hashing doesn't break and empty credential returns an empty hash.""" + host = "127.0.0.1" + + params = Device.ConnectionParameters( + device_family=Device.Family.SmartTapoPlug, + encryption_type=Device.EncryptionType.Xor, + login_version=login_version, + ) + config = DeviceConfig(host, credentials=credentials, connection_type=params) + transport = transport_class(config=config) + + credentials_hash = transport.credentials_hash + + expected = None if expected_blank else expected_hash + assert credentials_hash == expected + + @pytest.mark.parametrize( "transport_class", [AesTransport, KlapTransport, KlapTransportV2, XorTransport, XorTransport], ) -async def test_transport_credentials_hash(mocker, transport_class): +async def test_transport_credentials_hash_from_config(mocker, transport_class): + """Test that credentials_hash provided via config sets correctly.""" host = "127.0.0.1" credentials = Credentials("Foo", "Bar") diff --git a/kasa/xortransport.py b/kasa/xortransport.py index e9686453..52fba3d3 100644 --- a/kasa/xortransport.py +++ b/kasa/xortransport.py @@ -54,9 +54,9 @@ class XorTransport(BaseTransport): return self.DEFAULT_PORT @property - def credentials_hash(self) -> str: + def credentials_hash(self) -> str | None: """The hashed credentials used by the transport.""" - return "" + return None async def _connect(self, timeout: int) -> None: """Try to connect or reconnect to the device."""