diff --git a/.github/workflows/testing-linting.yml b/.github/workflows/testing-linting.yml index 5a8d6983..65e3fc05 100644 --- a/.github/workflows/testing-linting.yml +++ b/.github/workflows/testing-linting.yml @@ -1,7 +1,7 @@ # This workflow will install Python dependencies, run tests and lint with a variety of Python versions # For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions -name: testing and linting +name: Testing and Linting on: push: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4ccb8a14..5610d84e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,11 +7,11 @@ repos: - id: check-yaml - id: check-merge-conflict - repo: https://github.com/PyCQA/flake8 - rev: 7.0.0 + rev: 7.1.0 hooks: - id: flake8 - repo: https://github.com/hhatto/autopep8 - rev: 'v2.1.0' + rev: 'v2.3.1' hooks: - id: autopep8 - repo: https://github.com/pycqa/isort diff --git a/CHANGELOG.md b/CHANGELOG.md index 9dbfd104..71cdcd2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,28 @@ All notable changes to this project will be documented in this file. +## [5.0.1] 2024-08-03 + +### Fixed + +- Create new items via stdin to `op` command rather than reading from temp file (gh-172) + +### `op` CLI Version Support + +*Deprecated support* + +- `op` versions < 2.26.0 and >= 2.21.0 (gh-200) + +*Unsupported* + +- `op` versions < 2.21.0 (gh-200) + + ## [5.0.0] 2024-05-24 ### Added -- Version checking for the `op` CLI tool at run-time +- Version checking for the `op` CLI tool at run-time (gh-162) - `opversion` command - Export `OPCLIVersion` as API - `OPCLIVersionSupportException` class @@ -21,7 +38,7 @@ All notable changes to this project will be documented in this file. ### Misc -- Remove `op` version checks for special behaviors where the version is no longer supported +- Remove `op` version checks for special behaviors where the version is no longer supported (gh-193) ## [4.3.0] 2024-03-28 diff --git a/README.md b/README.md index 4f3b4b64..18353ba6 100644 --- a/README.md +++ b/README.md @@ -14,9 +14,9 @@ A Python API to sign into and query a 1Password account using the `op` command. ## Requirements - Python >= 3.9 -- 1Password command-line tool >= 2.24.0 - - Versions >= 2.19.0, < 2.24.0 supported but deprecated - - Versions < 2.19.0 are unsupported and an exception will be raised +- 1Password command-line tool >= 2.26.0 + - Versions >= 2.21.0, < 2.26.0 supported but deprecated + - Versions < 2.21.0 are unsupported and an exception will be raised - See [1Password Developer Documentation](https://developer.1password.com/docs/cli) - Internet connectivity to 1Password.com - The `op` command queries your online account, not your local vault @@ -354,14 +354,14 @@ def main(): For details on creating new items in a 1Password vault, see [item-creation.md](docs/item-creation.md) -Also see the examles in [examples/item_creation](examples/item_creation/) +Also see the examples in [examples/item_creation](examples/item_creation/) ### Item Editing For details on editing existing items in a 1Password vault, see [item-editing.md](docs/item-editing.md) -Also see the examles in [examples/item_editing](examples/item_editing/) +Also see the examples in [examples/item_editing](examples/item_editing/) ### Document Editing diff --git a/_readme_template.md b/_readme_template.md index da4f1791..518fd863 100644 --- a/_readme_template.md +++ b/_readme_template.md @@ -354,14 +354,14 @@ def main(): For details on creating new items in a 1Password vault, see [item-creation.md](docs/item-creation.md) -Also see the examles in [examples/item_creation](examples/item_creation/) +Also see the examples in [examples/item_creation](examples/item_creation/) ### Item Editing For details on editing existing items in a 1Password vault, see [item-editing.md](docs/item-editing.md) -Also see the examles in [examples/item_editing](examples/item_editing/) +Also see the examples in [examples/item_editing](examples/item_editing/) ### Document Editing diff --git a/docker_testing/build.sh b/docker_testing/build.sh index 7e6f5e8c..62c0b5c7 100755 --- a/docker_testing/build.sh +++ b/docker_testing/build.sh @@ -2,7 +2,12 @@ SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +echo "Building..." + docker buildx build "$SCRIPT_DIR"/docker/ -f "$SCRIPT_DIR"/docker/py39.Dockerfile -t docker_py39 || exit docker buildx build "$SCRIPT_DIR"/docker/ -f "$SCRIPT_DIR"/docker/py310.Dockerfile -t docker_py310 || exit docker buildx build "$SCRIPT_DIR"/docker/ -f "$SCRIPT_DIR"/docker/py311.Dockerfile -t docker_py311 || exit docker buildx build "$SCRIPT_DIR"/docker/ -f "$SCRIPT_DIR"/docker/py312.Dockerfile -t docker_py312 || exit + +echo "...done" +echo "" diff --git a/docker_testing/clean.sh b/docker_testing/clean.sh index 64787cfa..d6fd02f2 100755 --- a/docker_testing/clean.sh +++ b/docker_testing/clean.sh @@ -2,5 +2,10 @@ # keep unused docker image names so we can ensure they get removed # this should not be an error +echo "Cleaning..." +echo "removing docker images" docker image rm docker_py38 docker_py39 docker_py310 docker_py311 docker_py312 +echo "removing .tox-docker" rm -rf .tox-docker +echo "...done" +echo "" diff --git a/docker_testing/run.sh b/docker_testing/run.sh index a744abe6..3a75fe98 100755 --- a/docker_testing/run.sh +++ b/docker_testing/run.sh @@ -22,6 +22,7 @@ trap handle_sig INT ret=0 +echo "Running docker tests..." docker run --rm -it -v "$(pwd):/usr/src/testdir" docker_py39 /test.sh "$@" ret="$(($?+ret))" docker run --rm -it -v "$(pwd):/usr/src/testdir" docker_py310 /test.sh "$@" @@ -31,4 +32,6 @@ ret="$(($?+ret))" docker run --rm -it -v "$(pwd):/usr/src/testdir" docker_py312 /test.sh "$@" ret="$(($?+ret))" +echo "...done" +echo "" exit "$ret" diff --git a/examples/item_creation/example-create-item.py b/examples/item_creation/example-create-item.py index 5bfb2dba..372bde79 100644 --- a/examples/item_creation/example-create-item.py +++ b/examples/item_creation/example-create-item.py @@ -1,12 +1,10 @@ -from pyonepassword import OP +from pyonepassword import OP, logging from pyonepassword.api.object_types import OPLoginItem -from ..do_signin import do_signin - if __name__ == "__main__": # see README.md for sign-in process - op: OP = do_signin() - + logger = logging.console_debug_logger("example-create-item") + op: OP = OP(logger=logger) title = "Example Login Item" username = "test_username" great_password = "really-great-password" diff --git a/ipython_snippets/item_creation.py b/ipython_snippets/item_creation.py index fc310eac..7fd166ad 100644 --- a/ipython_snippets/item_creation.py +++ b/ipython_snippets/item_creation.py @@ -1,4 +1,4 @@ -from typing import List +from typing import List, Optional from pyonepassword import OP # noqa: F401 from pyonepassword.api.object_types import ( @@ -23,8 +23,8 @@ class OPNewSecureNoteItem(OPNewItemMixin, OPSecureNoteItem): def __init__(self, title: str, note_text: str, - fields: List[OPItemField] = [], - sections: List[OPSection] = []): + fields: Optional[List[OPItemField]] = None, + sections: Optional[List[OPSection]] = None): if sections is None: # pragma: no coverage sections = [] diff --git a/pyonepassword/__about__.py b/pyonepassword/__about__.py index 54b5a137..bce3551c 100644 --- a/pyonepassword/__about__.py +++ b/pyonepassword/__about__.py @@ -1,5 +1,5 @@ __title__ = "pyonepassword" -__version__ = "5.0.0" +__version__ = "5.0.1" __summary__ = "A python API to query a 1Password account using the 'op' command-line tool" """ diff --git a/pyonepassword/_op_cli_argv.py b/pyonepassword/_op_cli_argv.py index 0a4e536b..621863eb 100644 --- a/pyonepassword/_op_cli_argv.py +++ b/pyonepassword/_op_cli_argv.py @@ -3,7 +3,6 @@ from ._field_assignment import FIELD_TYPE_MAP, OPFieldTypeEnum from ._svc_account import OPSvcAcctSupportCode, OPSvcAcctSupportRegistry -from .op_items._new_item import OPNewItemMixin from .op_items.password_recipe import OPPasswordRecipe from .string import RedactedString @@ -127,7 +126,8 @@ def document_delete_argv(cls, def item_generic_argv(cls, op_exe: str, subcommands: Optional[Union[str, List[str]]], - sub_cmd_args: Optional[List[str]] = None): + sub_cmd_args: Optional[List[str]] = None, + encoding="utf-8"): args = [] global_args = ["--format", "json"] if sub_cmd_args: @@ -136,7 +136,8 @@ def item_generic_argv(cls, "item", args, subcommands=subcommands, - global_args=global_args) + global_args=global_args, + encoding=encoding) return argv @classmethod @@ -374,25 +375,20 @@ def account_forget_argv(cls, op_exe, account): # pragma: no coverage @classmethod def item_create_argv(cls, op_exe, - item: OPNewItemMixin, password_recipe: Optional[OPPasswordRecipe] = None, vault: Optional[str] = None, encoding="utf-8"): """ op item create --template ./new_item.json --vault "Test Data" --generate-password=20,letters,digits --dry-run --format json """ - template_filename = item.secure_tempfile( - encoding=encoding) - - item_create_args = ["--template", template_filename] - + item_create_args = [] if password_recipe: # '--password-recipe' requires an '=' unlike other option/argument pairs item_create_args.append(f"--generate-password={password_recipe}") if vault: item_create_args.extend(["--vault", vault]) argv = cls.item_generic_argv( - op_exe, "create", sub_cmd_args=item_create_args) + op_exe, "create", sub_cmd_args=item_create_args, encoding=encoding) return argv @classmethod diff --git a/pyonepassword/_op_commands.py b/pyonepassword/_op_commands.py index 48cabfe3..79e28bc0 100644 --- a/pyonepassword/_op_commands.py +++ b/pyonepassword/_op_commands.py @@ -24,6 +24,7 @@ OPSvcAcctCommandNotSupportedException ) from .account import OPAccount, OPAccountList +from .op_items._new_item import OPNewItemMixin from .op_items.password_recipe import OPPasswordRecipe from .py_op_exceptions import ( OPAuthenticationException, @@ -131,7 +132,7 @@ def __init__(self, # if a password is passed in but existing_auth is required, caller may be confused: # - intentionally passed in incompatible options # - possibly has OP_SERVICE_ACCOUNT_TOKEN set accidentally - msg = f"Password argument passed but EXISTING_AUTH_REQD flag is set. flag source: {auth_pref_source}" # nopep8 + msg = f"Password argument passed but EXISTING_AUTH_REQD flag is set. flag source: {auth_pref_source}" self.logger.error(msg) raise OPAuthenticationException(msg) @@ -796,11 +797,12 @@ def _item_list(self, categories=[], include_archive=False, tags=[], vault=None, raise OPItemListException.from_opexception(e) return output - def _item_create(self, item, vault, password_recipe, decode="utf-8"): - argv = self._item_create_argv(item, password_recipe, vault) + def _item_create(self, item: OPNewItemMixin, vault, password_recipe, decode="utf-8"): + item_json = item.serialize(indent=2) + argv = self._item_create_argv(password_recipe, vault) try: output = self._run_with_auth_check( - self.op_path, self._account_identifier, argv, capture_stdout=True, decode=decode) + self.op_path, self._account_identifier, argv, capture_stdout=True, input=item_json, decode=decode) except OPCmdFailedException as e: raise OPItemCreateException.from_opexception(e) @@ -984,10 +986,10 @@ def _vault_list_argv(self, group_name_or_id=None, user_name_or_id=None): self.op_path, group_name_or_id=group_name_or_id, user_name_or_id=user_name_or_id) return vault_list_argv - def _item_create_argv(self, item, password_recipe, vault): + def _item_create_argv(self, password_recipe, vault): vault_arg = vault if vault else self.vault item_create_argv = _OPArgv.item_create_argv( - self.op_path, item, password_recipe=password_recipe, vault=vault_arg + self.op_path, password_recipe=password_recipe, vault=vault_arg ) return item_create_argv diff --git a/pyonepassword/_py_op_deprecation.py b/pyonepassword/_py_op_deprecation.py index 6be9d473..a94b6dd5 100644 --- a/pyonepassword/_py_op_deprecation.py +++ b/pyonepassword/_py_op_deprecation.py @@ -94,12 +94,12 @@ def _rename_kwargs(func_name: str, kwargs: Dict[str, Any], kwarg_aliases: Dict[s if old_kwarg in kwargs: if new_kwarg in kwargs: raise TypeError( - f"{func_name} received both {old_kwarg} and {new_kwarg} as arguments!" # nopep8 + f"{func_name} received both {old_kwarg} and {new_kwarg} as arguments!" f" {old_kwarg} is deprecated, use {new_kwarg} instead." ) warnings.warn( message=( - f"`{old_kwarg}` is deprecated as an argument to `{func_name}`; use" # nopep8 + f"`{old_kwarg}` is deprecated as an argument to `{func_name}`; use" f" `{new_kwarg}` instead." ), category=DeprecationWarning, diff --git a/pyonepassword/_svc_account.py b/pyonepassword/_svc_account.py index 61fd679e..55a3152c 100644 --- a/pyonepassword/_svc_account.py +++ b/pyonepassword/_svc_account.py @@ -179,7 +179,7 @@ def command_supported(self, _argv: List[str]) -> OPSvcAcctSupportCode: # then command is not supported _cmd_not_found = command is not None and cmd_spec is None if (cmd_spec and not cmd_dict) or _cmd_not_found: - _support_msg = f"Command or subcommand not supported: [{command} {' '.join(subcommands)}]" # nopep8 + _support_msg = f"Command or subcommand not supported: [{command} {' '.join(subcommands)}]" _support_code = SVC_ACCT_CMD_NOT_SUPPORTED if cmd_dict and _support_code == _SVC_ACCT_CMD_NOT_VALIDATED: @@ -220,10 +220,10 @@ def command_supported(self, _argv: List[str]) -> OPSvcAcctSupportCode: if _support_code == SVC_ACCT_INCOMPAT_OPTIONS: _support_msg = "" if not required_opt_satified: - _support_msg += f"Required options not provided: [{','.join(list(reqd_opt_diff))}]" # nopep8 + _support_msg += f"Required options not provided: [{','.join(list(reqd_opt_diff))}]" if not prohibited_opt_satisfied: - _support_msg += f" Prohibited options found: [{','.join(list(prohib_opt_diff))}]" # nopep8 + _support_msg += f" Prohibited options found: [{','.join(list(prohib_opt_diff))}]" _support_msg = _support_msg.lstrip() diff --git a/pyonepassword/data/op_versions/version_support.json b/pyonepassword/data/op_versions/version_support.json index d1c51ef3..bd725711 100644 --- a/pyonepassword/data/op_versions/version_support.json +++ b/pyonepassword/data/op_versions/version_support.json @@ -3,8 +3,8 @@ "version": 1 }, "version-support":{ - "supported": "2.24.0", - "minimum": "2.19.0" + "supported": "2.26.0", + "minimum": "2.21.0" }, "feature-support": { "example-feature-service-accounts": "2.18.0" diff --git a/pyonepassword/op_items/_item_list.py b/pyonepassword/op_items/_item_list.py index 6e8f3378..7c01fff7 100644 --- a/pyonepassword/op_items/_item_list.py +++ b/pyonepassword/op_items/_item_list.py @@ -25,7 +25,7 @@ def serialize(self, indent=None) -> str: json_str = json.dumps(self, indent=indent) return json_str - def sort(self): + def sort(self): # type: ignore[override] # we really want to be sorted by title, since that's the most # human friendly (mainly for troubleshooting) # but in the case if title collisions, this is still non-deterministic diff --git a/pyonepassword/op_items/_new_item.py b/pyonepassword/op_items/_new_item.py index fa93d78b..0f68b625 100644 --- a/pyonepassword/op_items/_new_item.py +++ b/pyonepassword/op_items/_new_item.py @@ -1,8 +1,4 @@ import json -import os -import shutil -import tempfile -from pathlib import Path from typing import Dict, List, Optional from ..py_op_exceptions import OPInvalidItemException @@ -217,35 +213,10 @@ def __init__(self, title: str, fields: List[OPItemField] = [], sections: List[OP # Satisfy mypy: Too many arguments for "__init__" of "object" args = [template_dict] super().__init__(*args) - self._temp_files: List[str] = [] - def secure_tempfile(self, encoding="utf8") -> str: - """ - Serialize this item object to a secure temp file for use during 'op item create' - - The temporary file is deleted when this object goes out of scope - - Parameters - ---------- - encoding: str, optional - Encoding to use when serializing to file. Defaults to "utf-8" - - Returns - ------- - temp.name: str - The name of the temporary file - """ - temp = tempfile.NamedTemporaryFile( - mode="w", delete=False, encoding=encoding) - self._temp_files.append(temp.name) - json.dump(self, temp) - temp.close() - template_dest_dir = os.environ.get(TEMPLATE_COPY_DST_ENV_VAR, None) - if template_dest_dir: - # _copy_template() does no error handling - # only call if we were given a template copy destination - self._copy_template(temp.name, template_dest_dir) - return temp.name + def serialize(self, indent=None) -> str: + json_str = json.dumps(self, indent=indent) + return json_str def supports_passwords(self) -> bool: """ @@ -256,29 +227,3 @@ def supports_passwords(self) -> bool: password_supported: bool """ return self.PASSWORDS_SUPPORTED - - def _copy_template(self, template_src, template_dest_dir): - """ - Optionally make a backup copy of the new item template - if PYOP_NEW_ITEM_TEMPLATE_CP_DST environment variable is set - for debugging/troubleshooting - - This method intentionally does minimal error handling. It should only be called - for troublehooting purposes and any errors creating the destination or copying the source - should be fatal errors - """ - template_dest_dir = Path(template_dest_dir) - template_dest_dir.mkdir(parents=True, exist_ok=True) - shutil.copy2(template_src, template_dest_dir) - - def __del__(self): - - # if we blow up during object initialization - # _temp_files may not exist, so check first - if hasattr(self, "_temp_files"): - while self._temp_files: - t = self._temp_files.pop() - try: - os.unlink(t) - except FileNotFoundError: - continue diff --git a/pyonepassword/pyonepassword.py b/pyonepassword/pyonepassword.py index d9d43382..840651fa 100644 --- a/pyonepassword/pyonepassword.py +++ b/pyonepassword/pyonepassword.py @@ -2099,7 +2099,7 @@ def _validate_item_field_exists(self, msg = f"No field found '{field_label}' that lacks a section" raise OPFieldNotFoundException(msg) else: - msg = f"Section '{section_label}', field '{field_label}' not found" # nopep8 + msg = f"Section '{section_label}', field '{field_label}' not found" raise OPFieldNotFoundException(msg) return field diff --git a/scripts/update_readme.py b/scripts/update_readme.py index 2f70a5cc..8f5cd7e7 100755 --- a/scripts/update_readme.py +++ b/scripts/update_readme.py @@ -45,11 +45,14 @@ def main(): readme_text = generate_readme_text(README_TEMPLATE) needs_update = check_readme(README, readme_text) if args.check: + if needs_update: + print("README needs updating") return int(needs_update) if needs_update: with open(README, "w") as f: f.write(readme_text) + print("README updated") if __name__ == "__main__": diff --git a/setup.py b/setup.py index d247eb86..65328e4c 100644 --- a/setup.py +++ b/setup.py @@ -43,7 +43,7 @@ def validate_readme(): # links on PyPI should have absolute URLs # this awful regex looks for [any text](any url), making sure there's no 'http:' # in the url part -# it then inserts https://github.com/zcutlip/pyonepassword/blobl/main/ +# it then inserts https://github.com/zcutlip/pyonepassword/blob/main/ # after between the '(' and the relative URL # believe it or not this also works with directories such as examples/item_editing/ # Github redirects from blob/main/ to tree/main/ in this case diff --git a/tests/new_item/test_new_database_item.py b/tests/new_item/test_new_database_item.py index d1653d4a..5b330e99 100644 --- a/tests/new_item/test_new_database_item.py +++ b/tests/new_item/test_new_database_item.py @@ -1,6 +1,5 @@ from __future__ import annotations -import os from typing import TYPE_CHECKING, Any, Dict, List from pyonepassword.api.object_types import OPDatabaseItemTemplate @@ -607,53 +606,6 @@ def test_new_database_item_100(valid_data: ValidData): assert database_template.options is None -def test_new_database_item_temp_file_010(): - """ - Create: - - An OPDatabaseItemTemplate object - - request a secure temp file with the object serialized as JSON - - release the new object - Verify: - - the temp file exists - - the temp file has been deleted when the object is released - """ - username = "test_username" - title = "Test Database Item" - - database_template = OPDatabaseItemTemplate(title, username) - temp_file_path = database_template.secure_tempfile() - assert os.path.isfile(temp_file_path) - - database_template = None - assert not os.path.exists(temp_file_path) - - -def test_new_database_item_temp_file_020(): - """ - Create: - - An OPDatabaseItemTemplate object - - request a secure temp file with the object serialized as JSON - - delete the temp file from under the object - - release the object - Verify: - - the temp file exists - - no exception is raised when object is released - """ - username = "test_username" - title = "Test Database Item" - - database_template = OPDatabaseItemTemplate(title, username) - temp_file_path = database_template.secure_tempfile() - assert os.path.isfile(temp_file_path) - - # dirty trick; this shouldn't happen. But we need to be robust against it anyway - os.remove(temp_file_path) - try: - database_template = None - except Exception as e: - assert False, f"new_login = None raised an exception {e}" - - def test_new_database_item_tags_010(valid_data: ValidData): """ Create: diff --git a/tests/new_item/test_new_login_item.py b/tests/new_item/test_new_login_item.py index e9e55657..1d3596f1 100644 --- a/tests/new_item/test_new_login_item.py +++ b/tests/new_item/test_new_login_item.py @@ -1,6 +1,5 @@ from __future__ import annotations -import os from typing import TYPE_CHECKING import pytest @@ -390,52 +389,6 @@ def test_new_login_item_12(valid_data: ValidData): OPLoginItemTemplate(title, username, sections=sections) -def test_new_login_item_13(): - """ - Create: - - An OPLoginItemTemplate object - - request a secure temp file with the object serialized as JSON - - release the new object - Verify: - - the temp file exists - - the temp file has been deleted when the object is released - """ - username = "test_username" - title = "Test Login Item" - - new_login = OPLoginItemTemplate(title, username) - temp_file_path = new_login.secure_tempfile() - assert os.path.isfile(temp_file_path) - new_login = None - assert not os.path.exists(temp_file_path) - - -def test_new_login_item_14(): - """ - Create: - - An OPLoginItemTemplate object - - request a secure temp file with the object serialized as JSON - - delete the temp file from under the object - - release the object - Verify: - - the temp file exists - - no exception is raised when object is released - """ - username = "test_username" - title = "Test Login Item" - - new_login = OPLoginItemTemplate(title, username) - temp_file_path = new_login.secure_tempfile() - assert os.path.isfile(temp_file_path) - - # dirty trick; this shouldn't happen. But we need to be robust against it anyway - os.remove(temp_file_path) - try: - new_login = None - except Exception as e: - assert False, f"new_login = None raised an exception {e}" - - def test_new_login_item_15(): """ Create: