issue #24: implement better testing for establishing the fallback strategy when we fail to extract the key

Author: parawanderer

python/README.md

@@ -106,7 +106,7 @@ pyinstaller \
 
 Zip up result (MacOS):
 ```shell
-APP_VERSION=1.0.3
+APP_VERSION=1.0.4
 cd ./dist
 zip -r OpenTagViewer-ExportWizardMacOS-$APP_VERSION.zip OpenTagViewer.app/ OpenTagViewer
 ```

python/main/airtag_decryptor.py

@@ -1,3 +1,4 @@
+from abc import ABC, abstractmethod
 import os
 import re
 import shlex
@@ -37,25 +38,42 @@ class KeyStoreKeyNotFoundException(Exception):
     pass
 
 
-def get_key(label: str) -> bytearray:
+class AbstractSubprocessRunner(ABC):
+    """
+    Wrapper class to enable easier unit testing
+    """
+
+    @abstractmethod
+    def run(self, command: list[str]) -> str:
+        ...
+
+
+class SubprocessRunner(AbstractSubprocessRunner):
+    """
+    Runs generic `subprocess` implementation
+    """
+
+    def run(self, command: list[str]) -> str:
+        return subprocess.run(command, capture_output=True, text=True).stdout
+
+
+def get_key(label: str, runner: AbstractSubprocessRunner = SubprocessRunner()) -> bytearray:
     """
     Tries to extract the key via the command `security find-generic-password -l <label> -w`.
 
     :throws KeyStoreKeyNotFoundException: when not found or unable to extract
     """
     try:
-        result = subprocess.run(
-            [
-                "security",
-                "find-generic-password",
-                "-l",
-                label,
-                "-w"
-            ],
-            capture_output=True,
-            text=True
-        )
-        key_in_hex_format: str = result.stdout
+        key_in_hex_format: str = runner.run([
+            "security",
+            "find-generic-password",
+            "-l",
+            label,
+            "-w"
+        ])
+
+        if len(key_in_hex_format.strip()) == 0:
+            raise KeyStoreKeyNotFoundException("security command output with flag -w returned empty")
 
         return extract_key(key_in_hex_format)
 
@@ -72,7 +90,7 @@ def extract_key(key_in_hex_format: str) -> bytearray:
     return key
 
 
-def get_key_from_full_output(label: str):
+def get_key_from_full_output(label: str, runner: AbstractSubprocessRunner = SubprocessRunner()):
     """
     Tries to extract the key via the command `security find-generic-password -l <label>`.
 
@@ -81,17 +99,16 @@ def get_key_from_full_output(label: str):
     :throws KeyStoreKeyNotFoundException: when not found in output or unable to extract
     """
     try:
-        result = subprocess.run(
-            [
-                "security",
-                "find-generic-password",
-                "-l",
-                label
-            ],
-            capture_output=True,
-            text=True
-        )
-        full_key_info: str = result.stdout
+        full_key_info: str = runner.run([
+            "security",
+            "find-generic-password",
+            "-l",
+            label
+        ])
+
+        if len(full_key_info.strip()) == 0:
+            raise KeyStoreKeyNotFoundException("security command full output returned empty")
+
         return extract_gena_key(full_key_info)
     except Exception as e:
         raise KeyStoreKeyNotFoundException("Failed to retrieve keystore value") from e
@@ -137,6 +154,14 @@ def extract_gena_key(output: str):
     return key
 
 
+def get_key_fallback(label: str, runner: AbstractSubprocessRunner = SubprocessRunner()) -> bytearray:
+    try:
+        return get_key(label, runner)
+    except KeyStoreKeyNotFoundException:
+        # Fallback to alternate solution (see issue #13)
+        return get_key_from_full_output(label, runner)
+
+
 def decrypt_plist(in_file_path: str, key: bytearray) -> dict:
     """
     Given an encrypted plist file at path `in_file_path`, decrypt it using `key` and AES-GMC and return

python/main/wizard.py

@@ -16,18 +16,16 @@
     KEYCHAIN_LABEL,
     INPUT_PATH,
     WHITELISTED_DIRS,
-    KeyStoreKeyNotFoundException,
-    get_key,
     decrypt_plist,
-    get_key_from_full_output,
+    get_key_fallback,
     make_output_path,
     dump_plist
 )
 from main.utils import MACOS_VER
 
 # Wrapper around the main decryptor implementation that allows to filter which beacon files get exported/zipped
 
-VERSION = "1.0.3"
+VERSION = "1.0.4"
 
 APP_TITLE = f"OpenTagViewer AirTag Exporter {VERSION}"
 
@@ -131,13 +129,6 @@ def _retrieve_beacon_data(self) -> dict[str, BeaconData]:
             self.quit()
             raise Exception("User does not want to give password access to keystore!")
 
-    def _retrieve_key(self):
-        try:
-            return get_key(KEYCHAIN_LABEL)
-        except KeyStoreKeyNotFoundException:
-            # Fallback to alternate solution (see issue #13)
-            return get_key_from_full_output(KEYCHAIN_LABEL)
-
     def _read_all_plists(self, beacon_store_key: bytearray) -> tuple[list[PListFileInfo], list[PListFileInfo]]:
         owned_beacons: list[PListFileInfo]
         beacon_naming_records: list[PListFileInfo]
@@ -162,7 +153,7 @@ def _read_all_plists(self, beacon_store_key: bytearray) -> tuple[list[PListFileI
 
     def _create_beacon_data_map(self) -> dict[str, BeaconData]:
         # get key: prompts password entry
-        beacon_store_key: bytearray = self._retrieve_key()
+        beacon_store_key: bytearray = get_key_fallback(KEYCHAIN_LABEL)
 
         if not beacon_store_key:
             messagebox.showerror(