Merge pull request #189 from zcutlip/dev/187-handle-missing-op-config…

…-gracefully

issue #187 handle missing op config gracefully

pyonepassword/_op_commands.py

@@ -31,6 +31,7 @@
     OPAuthenticationException,
     OPCmdFailedException,
     OPCmdMalformedSvcAcctTokenException,
+    OPConfigNotFoundException,
     OPDocumentDeleteException,
     OPDocumentEditException,
     OPDocumentGetException,
@@ -183,26 +184,42 @@ def uses_biometric(cls, op_path: str = "op", encoding: str = "utf-8", account_li
         if account_list is None:
             account_list = cls._get_account_list(op_path, decode=encoding)
         acct: OPAccount
-        for acct in account_list:
-            if not acct.shorthand:
-                continue
-            # There is at least one account_shorthand found in `op account list`
+        if not account_list:
             uses_bio = False
-            break
+        else:
+            for acct in account_list:
+                if not acct.shorthand:
+                    continue
+                # There is at least one account_shorthand found in `op account list`
+                uses_bio = False
+                break
         return uses_bio
 
     def _gather_facts(self):
-        self._op_config = OPCLIConfig()
-        self._cli_version = self._get_cli_version(self.op_path)
-        self._account_list = self._get_account_list(self.op_path)
         self._uses_bio = self.uses_biometric(
             op_path=self.op_path, account_list=self._account_list)
+        self.logger.debug(f"uses bio: {self._uses_bio}")
+        try:
+            # if we haven't done a sign-in via the CLI/without desktop app integration
+            # there won't be a config
+            self._op_config = OPCLIConfig()
+        except OPConfigNotFoundException:
+            if self._uses_bio:
+                # set this to None. We should only use it if biometric is diabled,
+                # in which case this should be a hard error
+                self._op_config = None
+            else:
+                raise
+        self._cli_version = self._get_cli_version(self.op_path)
+        self._account_list = self._get_account_list(self.op_path)
+
         self._account_identifier = self._normalize_account_id()
         self._sess_var = self._compute_session_var_name()
 
-    def _get_cli_version(self, op_path: str) -> OPCLIVersion:
+    @classmethod
+    def _get_cli_version(cls, op_path: str) -> OPCLIVersion:
         argv = _OPArgv.cli_version_argv(op_path)
-        output = self._run(argv, capture_stdout=True, decode="utf-8")
+        output = cls._run(argv, capture_stdout=True, decode="utf-8")
         output = output.rstrip()
         cli_version = OPCLIVersion(output)
         return cli_version

scripts/pypi_password.py

@@ -5,6 +5,8 @@
 import sys
 from argparse import ArgumentParser
 
+from pyonepassword.logging import console_debug_logger
+
 # isort: split
 parent_path = os.path.dirname(
     os.path.dirname(
@@ -27,7 +29,7 @@
 )
 
 
-def do_signin(vault=None, op_path="op", use_existing_session=False, account=None):
+def do_signin(vault=None, op_path="op", use_existing_session=False, account=None, logger=None):
     auth = EXISTING_AUTH_IGNORE
     if use_existing_session:
         auth = EXISTING_AUTH_AVAIL
@@ -36,6 +38,7 @@ def do_signin(vault=None, op_path="op", use_existing_session=False, account=None
     uses_biometric = OP.uses_biometric(op_path=op_path)
     try:
         op = OP(vault=vault, op_path=op_path,
+                logger=logger,
                 existing_auth=auth, password_prompt=False, account=account)
     except OPAuthenticationException as e:
         if uses_biometric:
@@ -60,6 +63,9 @@ def pypi_parse_args():
                         help="Attempt to use an existing 'op' session. If unsuccessful master password will be requested.", action='store_true')
     parser.add_argument("--account", "-A",
                         help="1Password account to use. (See op signin --help, for valid identifiers")
+    parser.add_argument("--debug",
+                        help="Enable debug logging to the console",
+                        action='store_true')
     parsed = parser.parse_args()
     return parsed
 
@@ -68,10 +74,14 @@ def main():
     op: OP
     print(f"pyonepassword version {OP.version()}", file=sys.stderr)
     parsed = pypi_parse_args()
+    logger = None
+    if parsed.debug:
+        logger = console_debug_logger("pypi_password")
     pypi_item_name = parsed.pypi_item_name
     try:
         op = do_signin(use_existing_session=parsed.use_session,
-                       account=parsed.account)
+                       account=parsed.account,
+                       logger=logger)
     except OPSigninException as e:
         print("sign-in failed", file=sys.stderr)
         print(e.err_output, file=sys.stderr)

scripts/sanitize.py

@@ -11,7 +11,8 @@
 from pathlib import Path
 from typing import Dict
 
-WHITELIST = ["*/output", "*.txt", "*.json", "*.py", "*/input/**"]
+WHITELIST = ["*/output", "*/error_output",
+             "*.txt", "*.json", "*.py", "*/input/**"]
 
 
 def digest_file(file_path):

tests/config/mock-op/no-conf-no-bio-response-directory.json

@@ -0,0 +1,37 @@
+{
+  "meta": {
+    "response_dir": "tests/config/mock-op/responses-no-conf-no-bio",
+    "input_dir": "input"
+  },
+  "commands": {
+    "--account|5GHHPJK5HZC5BAT7WDUXW57G44|--format|json|whoami": {
+      "exit_status": 1,
+      "stdout": "output",
+      "stderr": "error_output",
+      "name": "whoami-account-uuid",
+      "changes_state": false
+    },
+    "--version": {
+      "exit_status": 0,
+      "stdout": "output",
+      "stderr": "error_output",
+      "name": "cli-version",
+      "changes_state": false
+    },
+    "--format|json|account|list": {
+      "exit_status": 0,
+      "stdout": "output",
+      "stderr": "error_output",
+      "name": "list-signed-in-accounts",
+      "changes_state": false
+    },
+    "--format|json|whoami": {
+      "exit_status": 1,
+      "stdout": "output",
+      "stderr": "error_output",
+      "name": "whoami",
+      "changes_state": false
+    }
+  },
+  "commands_with_input": {}
+}

tests/config/mock-op/response-generation/no-initial-auth-no-conf/response-generation-no-conf-no-bio.cfg

@@ -0,0 +1,25 @@
+[MAIN]
+config-path = ./tests/config/mock-op
+response-path = responses-no-conf-no-bio
+response-dir-file = no-conf-no-bio-response-directory.json
+ignore-signin-fail = true
+skip-global-signin = True
+
+
+# be sure to do the following before generating responses
+# - remove ~/.config/op
+# - disable TouchID (or other biometric) in 1Password.app developer settings
+[cli-version]
+type = cli-version
+create-instance = False
+
+[list-signed-in-accounts]
+type = account-list
+create-instance = False
+# account_list returns an empty list, but succeeds
+expected-return = 0
+
+[whoami]
+type = whoami
+create-instance = False
+expected-return = 1

tests/config/mock-op/responses-no-conf-no-bio/cli-version/output

@@ -0,0 +1 @@
+2.26.0

tests/config/mock-op/responses-no-conf-no-bio/list-signed-in-accounts/output

@@ -0,0 +1 @@
+[]

tests/config/mock-op/responses-no-conf-no-bio/whoami-account-uuid/error_output

@@ -0,0 +1 @@
+[ERROR] 2024/03/19 19:33:50 no account found for filter 5GHHPJK5HZC5BAT7WDUXW57G44

tests/config/mock-op/responses-no-conf-no-bio/whoami/error_output

@@ -0,0 +1 @@
+[ERROR] 2024/03/19 19:34:06 no account found for filter 

tests/config/mock-op/responses-user-edit/new-user-name/responses-2/user-get-orig-user-name/error_output

@@ -1 +1 @@
-[ERROR] 2023/12/15 21:26:48 "Zachary Cutlip" isn't a user in this account. Specify the user with their UUID, email address, or name.
+[ERROR] 2023/12/15 21:26:48 "Example User" isn't a user in this account. Specify the user with their UUID, email address, or name.

tests/fixtures/op_fixtures.py

@@ -45,6 +45,7 @@
     ITEM_DELETE_MULTIPLE_STATE_CONFIG_PATH,
     ITEM_DELETE_MULTIPLE_TITLE_GLOB_STATE_CONFIG_PATH,
     ITEM_EDIT_STATE_CONFIG_PATH,
+    NO_CONF_NO_BIO_RESP_DIRECTORY_PATH,
     RESP_DIRECTORY_PATH,
     SVC_ACCT_CORRUPT_RESP_DIRECTORY_PATH,
     SVC_ACCT_NOT_YET_AUTH_STATE_CONFIG_PATH,
@@ -126,6 +127,14 @@ def _setup_alt_env():
     os.environ["LOG_OP_ERR"] = "1"
 
 
+def _setup_no_conf_no_bio_env():
+    os.environ["MOCK_OP_RESPONSE_DIRECTORY"] = str(
+        NO_CONF_NO_BIO_RESP_DIRECTORY_PATH)
+    # os.environ["MOCK_OP_SIGNIN_SUCCEED"] = "1"
+    # os.environ["MOCK_OP_SIGNIN_USES_BIO"] = "1"
+    os.environ["LOG_OP_ERR"] = "1"
+
+
 def _setup_no_bio_normal_env():
     _setup_normal_env()
     os.environ["MOCK_OP_SIGNIN_USES_BIO"] = "0"
@@ -392,6 +401,11 @@ def setup_svc_account_revoked_env():
     _setup_svc_acct_env(resp_dir_path=SVC_ACCT_REVOKED_RESP_DIRECTORY_PATH)
 
 
+@fixture
+def setup_no_conf_no_bio_env():
+    _setup_no_conf_no_bio_env()
+
+
 @fixture
 def setup_normal_op_env_signin_failure():
     _setup_normal_env(signin_success="0")

tests/fixtures/paths.py

@@ -56,6 +56,11 @@
 SVC_ACCT_REVOKED_RESP_DIRECTORY_PATH = Path(
     MOCK_OP_CONFIG_PATH, "svc-acct-revoked-token-directory.json"
 )
+
+NO_CONF_NO_BIO_RESP_DIRECTORY_PATH = Path(
+    MOCK_OP_CONFIG_PATH, "no-conf-no-bio-response-directory.json"
+)
+
 SVC_ACCT_NOT_YET_AUTH_RESP_PATH = Path(
     MOCK_OP_CONFIG_PATH, "responses-svc-acct-not-yet-auth"
 )

tests/test_auth_scenarios/test_missing_op_config.py

@@ -0,0 +1,81 @@
+import pytest
+
+from pyonepassword import OP
+from pyonepassword.api.exceptions import OPConfigNotFoundException
+from pyonepassword.op_cli_version import OPCLIVersion
+from pyonepassword.py_op_exceptions import OPWhoAmiException
+
+
+@pytest.mark.usefixtures("invalid_op_cli_config_missing")
+@pytest.mark.usefixtures("setup_no_conf_no_bio_env")
+def test_missing_op_config_010(console_logger):
+
+    with pytest.raises(OPConfigNotFoundException):
+        OP(op_path="mock-op", logger=console_logger)
+
+
+@pytest.mark.usefixtures("invalid_op_cli_config_missing")
+@pytest.mark.usefixtures("setup_no_conf_no_bio_env")
+def test_missing_op_config_020(console_logger):
+    """
+    Simulate:
+    - running op --format json account list with no ~/.config/op/
+    - Integration with the desktop app, and hence biometric, is disabled
+
+    Verify:
+        an empty list is returned
+    """
+    OP.set_logger(console_logger)
+    accounts = OP._get_account_list(op_path="mock-op")
+    assert len(accounts) == 0
+
+
+@pytest.mark.usefixtures("invalid_op_cli_config_missing")
+@pytest.mark.usefixtures("setup_no_conf_no_bio_env")
+def test_missing_op_config_030(console_logger):
+    """
+    Simulate:
+    - running op --format json whoami with no ~/.config/op/
+    - Integration with the desktop app, and hence biometric, is disabled
+
+    Verify:
+        OPWhoAmiException is raised
+    """
+    OP.set_logger(console_logger)
+    with pytest.raises(OPWhoAmiException):
+        OP._whoami(op_path="mock-op")
+
+
+@pytest.mark.usefixtures("invalid_op_cli_config_missing")
+@pytest.mark.usefixtures("setup_no_conf_no_bio_env")
+def test_missing_op_config_040(console_logger):
+    """
+    Simulate:
+    - running op --version with no ~/.config/op/
+    - Integration with the desktop app, and hence biometric, is disabled
+
+    Verify:
+        a version is returned
+        version is instance of OPCLIVersion
+    """
+    OP.set_logger(console_logger)
+    version = OP._get_cli_version(op_path="mock-op")
+    assert version
+    assert isinstance(version, OPCLIVersion)
+
+
+@pytest.mark.usefixtures("invalid_op_cli_config_missing")
+@pytest.mark.usefixtures("setup_no_conf_no_bio_env")
+def test_missing_op_config_050(console_logger):
+    """
+    Simulate:
+    - running op --format json whoami <account_uuid> with no ~/.config/op/
+    - Integration with the desktop app, and hence biometric, is disabled
+
+    Verify:
+        OPWhoAmiException is raised
+    """
+    account_uuid = "5GHHPJK5HZC5BAT7WDUXW57G44"
+    OP.set_logger(console_logger)
+    with pytest.raises(OPWhoAmiException):
+        OP._whoami(op_path="mock-op", account=account_uuid)