From 1dfb2394066518008c784341e9743e46e079ded0 Mon Sep 17 00:00:00 2001 From: Zachary Cutlip Date: Tue, 12 Mar 2024 20:27:43 -0700 Subject: [PATCH 01/12] in uses_biometric(), handle account list being empty - if empty, then uses_bio = False --- pyonepassword/_op_commands.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pyonepassword/_op_commands.py b/pyonepassword/_op_commands.py index 265f7807..03cfd115 100644 --- a/pyonepassword/_op_commands.py +++ b/pyonepassword/_op_commands.py @@ -183,12 +183,15 @@ 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): From 614cfcda55667f46f59b723af2ed1c7d9fb87b30 Mon Sep 17 00:00:00 2001 From: Zachary Cutlip Date: Tue, 12 Mar 2024 20:28:54 -0700 Subject: [PATCH 02/12] get _uses_bio before other facts that may depend on it --- pyonepassword/_op_commands.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyonepassword/_op_commands.py b/pyonepassword/_op_commands.py index 03cfd115..db8af90d 100644 --- a/pyonepassword/_op_commands.py +++ b/pyonepassword/_op_commands.py @@ -195,11 +195,11 @@ def uses_biometric(cls, op_path: str = "op", encoding: str = "utf-8", account_li 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._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() From 5c378f1858ae7d4f8b1415048861c76896bddbb4 Mon Sep 17 00:00:00 2001 From: Zachary Cutlip Date: Tue, 12 Mar 2024 20:30:01 -0700 Subject: [PATCH 03/12] handle missing op config if uses_bio == True, make config optional --- pyonepassword/_op_commands.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pyonepassword/_op_commands.py b/pyonepassword/_op_commands.py index db8af90d..1432f0cb 100644 --- a/pyonepassword/_op_commands.py +++ b/pyonepassword/_op_commands.py @@ -31,6 +31,7 @@ OPAuthenticationException, OPCmdFailedException, OPCmdMalformedSvcAcctTokenException, + OPConfigNotFoundException, OPDocumentDeleteException, OPDocumentEditException, OPDocumentGetException, @@ -197,6 +198,18 @@ def uses_biometric(cls, op_path: str = "op", encoding: str = "utf-8", account_li def _gather_facts(self): 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) From 52d11198836195e4fa525579c2e3251235c5875d Mon Sep 17 00:00:00 2001 From: Zachary Cutlip Date: Fri, 15 Mar 2024 19:14:39 -0700 Subject: [PATCH 04/12] add debug logging to pypi_password.py --- scripts/pypi_password.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/scripts/pypi_password.py b/scripts/pypi_password.py index 4e4b7933..85ac4970 100755 --- a/scripts/pypi_password.py +++ b/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) From e0fc3ed6cabab51ea00bcc256807fe0c77d74c27 Mon Sep 17 00:00:00 2001 From: Zachary Cutlip Date: Tue, 19 Mar 2024 20:48:13 -0700 Subject: [PATCH 05/12] process '*/error_output` files when santitizing --- scripts/sanitize.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/sanitize.py b/scripts/sanitize.py index 501a9b4f..2042708f 100755 --- a/scripts/sanitize.py +++ b/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): From c6838beaf75cef724892c214a6dc5faa452d01fd Mon Sep 17 00:00:00 2001 From: Zachary Cutlip Date: Tue, 19 Mar 2024 20:52:34 -0700 Subject: [PATCH 06/12] additional sanitization on mock-op response data --- .../responses-2/user-get-orig-user-name/error_output | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/config/mock-op/responses-user-edit/new-user-name/responses-2/user-get-orig-user-name/error_output b/tests/config/mock-op/responses-user-edit/new-user-name/responses-2/user-get-orig-user-name/error_output index 5e504f06..6329fe8b 100644 --- a/tests/config/mock-op/responses-user-edit/new-user-name/responses-2/user-get-orig-user-name/error_output +++ b/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. From 8bdda3b2536920cc7921665ed6859a3d5b9e35aa Mon Sep 17 00:00:00 2001 From: Zachary Cutlip Date: Tue, 19 Mar 2024 20:54:31 -0700 Subject: [PATCH 07/12] add query definition file for missing config/biometric disabled --- .../response-generation-no-conf-no-bio.cfg | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 tests/config/mock-op/response-generation/no-initial-auth-no-conf/response-generation-no-conf-no-bio.cfg diff --git a/tests/config/mock-op/response-generation/no-initial-auth-no-conf/response-generation-no-conf-no-bio.cfg b/tests/config/mock-op/response-generation/no-initial-auth-no-conf/response-generation-no-conf-no-bio.cfg new file mode 100644 index 00000000..efa63795 --- /dev/null +++ b/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 From ef74b35b8fcad2f8252484820d76e16b0778c369 Mon Sep 17 00:00:00 2001 From: Zachary Cutlip Date: Tue, 19 Mar 2024 20:55:30 -0700 Subject: [PATCH 08/12] add response definitions for no op config/biometric disabled --- .../no-conf-no-bio-response-directory.json | 37 +++++++++++++++++++ .../cli-version/error_output | 0 .../cli-version/output | 1 + .../list-signed-in-accounts/error_output | 0 .../list-signed-in-accounts/output | 1 + .../whoami-account-uuid/error_output | 1 + .../whoami-account-uuid/output | 0 .../whoami/error_output | 1 + .../responses-no-conf-no-bio/whoami/output | 0 9 files changed, 41 insertions(+) create mode 100644 tests/config/mock-op/no-conf-no-bio-response-directory.json create mode 100644 tests/config/mock-op/responses-no-conf-no-bio/cli-version/error_output create mode 100644 tests/config/mock-op/responses-no-conf-no-bio/cli-version/output create mode 100644 tests/config/mock-op/responses-no-conf-no-bio/list-signed-in-accounts/error_output create mode 100644 tests/config/mock-op/responses-no-conf-no-bio/list-signed-in-accounts/output create mode 100644 tests/config/mock-op/responses-no-conf-no-bio/whoami-account-uuid/error_output create mode 100644 tests/config/mock-op/responses-no-conf-no-bio/whoami-account-uuid/output create mode 100644 tests/config/mock-op/responses-no-conf-no-bio/whoami/error_output create mode 100644 tests/config/mock-op/responses-no-conf-no-bio/whoami/output diff --git a/tests/config/mock-op/no-conf-no-bio-response-directory.json b/tests/config/mock-op/no-conf-no-bio-response-directory.json new file mode 100644 index 00000000..c483b11e --- /dev/null +++ b/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": {} +} \ No newline at end of file diff --git a/tests/config/mock-op/responses-no-conf-no-bio/cli-version/error_output b/tests/config/mock-op/responses-no-conf-no-bio/cli-version/error_output new file mode 100644 index 00000000..e69de29b diff --git a/tests/config/mock-op/responses-no-conf-no-bio/cli-version/output b/tests/config/mock-op/responses-no-conf-no-bio/cli-version/output new file mode 100644 index 00000000..7a25c70f --- /dev/null +++ b/tests/config/mock-op/responses-no-conf-no-bio/cli-version/output @@ -0,0 +1 @@ +2.26.0 diff --git a/tests/config/mock-op/responses-no-conf-no-bio/list-signed-in-accounts/error_output b/tests/config/mock-op/responses-no-conf-no-bio/list-signed-in-accounts/error_output new file mode 100644 index 00000000..e69de29b diff --git a/tests/config/mock-op/responses-no-conf-no-bio/list-signed-in-accounts/output b/tests/config/mock-op/responses-no-conf-no-bio/list-signed-in-accounts/output new file mode 100644 index 00000000..fe51488c --- /dev/null +++ b/tests/config/mock-op/responses-no-conf-no-bio/list-signed-in-accounts/output @@ -0,0 +1 @@ +[] diff --git a/tests/config/mock-op/responses-no-conf-no-bio/whoami-account-uuid/error_output b/tests/config/mock-op/responses-no-conf-no-bio/whoami-account-uuid/error_output new file mode 100644 index 00000000..9cfe2b36 --- /dev/null +++ b/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 diff --git a/tests/config/mock-op/responses-no-conf-no-bio/whoami-account-uuid/output b/tests/config/mock-op/responses-no-conf-no-bio/whoami-account-uuid/output new file mode 100644 index 00000000..e69de29b diff --git a/tests/config/mock-op/responses-no-conf-no-bio/whoami/error_output b/tests/config/mock-op/responses-no-conf-no-bio/whoami/error_output new file mode 100644 index 00000000..f786c314 --- /dev/null +++ b/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 diff --git a/tests/config/mock-op/responses-no-conf-no-bio/whoami/output b/tests/config/mock-op/responses-no-conf-no-bio/whoami/output new file mode 100644 index 00000000..e69de29b From 3c2840c617f9bc20c0e7064271898d3ef93a6bda Mon Sep 17 00:00:00 2001 From: Zachary Cutlip Date: Tue, 26 Mar 2024 20:31:52 -0700 Subject: [PATCH 09/12] make `_OPCommandInterface._get_cli_version()` a class method --- pyonepassword/_op_commands.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pyonepassword/_op_commands.py b/pyonepassword/_op_commands.py index 1432f0cb..e47c6797 100644 --- a/pyonepassword/_op_commands.py +++ b/pyonepassword/_op_commands.py @@ -216,9 +216,10 @@ def _gather_facts(self): 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 From 3a60e1ba48045a1fea37e31bb4354516a8ebb814 Mon Sep 17 00:00:00 2001 From: Zachary Cutlip Date: Tue, 26 Mar 2024 20:46:43 -0700 Subject: [PATCH 10/12] add path constant for no-conf-no-bio response directory --- tests/fixtures/paths.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/fixtures/paths.py b/tests/fixtures/paths.py index a442960f..e5469ed3 100644 --- a/tests/fixtures/paths.py +++ b/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" ) From df568609e72d2ea50a887ad70cbec11089dda25e Mon Sep 17 00:00:00 2001 From: Zachary Cutlip Date: Tue, 26 Mar 2024 20:48:02 -0700 Subject: [PATCH 11/12] add fixture to setup environment with no-conf-no-bio responses --- tests/fixtures/op_fixtures.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/fixtures/op_fixtures.py b/tests/fixtures/op_fixtures.py index 6a2d4e5d..6189d121 100644 --- a/tests/fixtures/op_fixtures.py +++ b/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") From 07e263d334c3530f7bad67579aa323a6df960701 Mon Sep 17 00:00:00 2001 From: Zachary Cutlip Date: Tue, 26 Mar 2024 20:48:49 -0700 Subject: [PATCH 12/12] add test module to test missing op config --- .../test_missing_op_config.py | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 tests/test_auth_scenarios/test_missing_op_config.py diff --git a/tests/test_auth_scenarios/test_missing_op_config.py b/tests/test_auth_scenarios/test_missing_op_config.py new file mode 100644 index 00000000..19ce51e9 --- /dev/null +++ b/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 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)