From a5bfc5035f97841f1df00d42519e78d25aa64047 Mon Sep 17 00:00:00 2001 From: Nick Gheorghita Date: Mon, 10 Feb 2020 11:28:26 -0700 Subject: [PATCH] Split auth into multiple subparsers --- ethpm_cli/_utils/input.py | 13 ++++ ethpm_cli/commands/auth.py | 93 +++++++++++++++++++++-- ethpm_cli/commands/manifest.py | 13 +--- ethpm_cli/commands/package.py | 2 +- ethpm_cli/config.py | 10 +-- ethpm_cli/parser.py | 86 ++++++++++++++++------ tests/cli/test_auth_cli.py | 130 +++++++++++++++++++++++++++++++++ 7 files changed, 301 insertions(+), 46 deletions(-) create mode 100644 ethpm_cli/_utils/input.py create mode 100644 tests/cli/test_auth_cli.py diff --git a/ethpm_cli/_utils/input.py b/ethpm_cli/_utils/input.py new file mode 100644 index 0000000..913a8ef --- /dev/null +++ b/ethpm_cli/_utils/input.py @@ -0,0 +1,13 @@ +from ethpm_cli._utils.logger import cli_logger + + +def parse_bool_flag(question: str) -> bool: + while True: + response = input(f"{question} (y/n) ") + if response.lower() == "y": + return True + elif response.lower() == "n": + return False + else: + cli_logger.info(f"Invalid response: {response}.") + continue diff --git a/ethpm_cli/commands/auth.py b/ethpm_cli/commands/auth.py index a6e4455..d8dd3d8 100644 --- a/ethpm_cli/commands/auth.py +++ b/ethpm_cli/commands/auth.py @@ -1,23 +1,104 @@ +import json from pathlib import Path -import tempfile from typing import Any, Dict import eth_keyfile from eth_typing import Address -from eth_utils import to_bytes +from eth_utils import add_0x_prefix, to_bytes +from ethpm_cli._utils.filesystem import atomic_replace +from ethpm_cli._utils.input import parse_bool_flag +from ethpm_cli._utils.logger import cli_logger from ethpm_cli._utils.xdg import get_xdg_ethpmcli_root from ethpm_cli.constants import KEYFILE_PATH from ethpm_cli.exceptions import AuthorizationError +PRIVATE_KEY_WARNING = ( + "Please be careful when using your private key, it is a sensitive piece of information.\n", + "~ ~ ~ Not your keys, not your crypto. ~ ~ ~\n", + "ethPM doesn't save your private key, but it is best practice to re-use an already encrypted ", + "keyfile and link it with `ethpm auth link` rather than regenerating a new encrypted keyfile.", +) + + +def link_keyfile(keyfile_path: Path) -> None: + if valid_keyfile_exists(): + xdg_keyfile = get_keyfile_path() + cli_logger.info( + f"Keyfile detected at {xdg_keyfile}. Please use `ethpm auth unlink` to delete this " + "keyfile before linking a new one." + ) + else: + import_keyfile(keyfile_path) + address = get_authorized_address() + cli_logger.info( + f"Keyfile stored for address: {address}\n" + "It's now available for use when its password is passed in with the " + "`--keyfile-password` flag." + ) + + +def unlink_keyfile() -> None: + if not valid_keyfile_exists(): + cli_logger.info("Unable to unlink keyfile: empty keyfile found.") + else: + keyfile_path = get_keyfile_path() + address = get_authorized_address() + keyfile_path.write_text("") + cli_logger.info(f"Keyfile removed for address: {address}") + + +def init_keyfile() -> None: + if valid_keyfile_exists(): + cli_logger.info( + f"Keyfile detected. Please use `ethpm auth unlink` to delete this " + "keyfile before initializing a new one." + ) + return + + cli_logger.info(PRIVATE_KEY_WARNING) + agreement = parse_bool_flag( + "Are you sure you want to proceed with initializing a keyfile? " + ) + if not agreement: + cli_logger.info("Aborting keyfile initialization.") + return + + private_key = to_bytes(text=input("Please enter your 32-length private key: ")) + validate_private_key_length(private_key) + password = to_bytes( + text=input( + "Please enter a password to encrypt your keyfile with (Don't forget this password!): " + ) + ) + keyfile_json = eth_keyfile.create_keyfile_json(private_key, password) + ethpm_xdg_root = get_xdg_ethpmcli_root() + ethpm_cli_keyfile_path = ethpm_xdg_root / KEYFILE_PATH + with atomic_replace(ethpm_cli_keyfile_path) as file: + file.write(json.dumps(keyfile_json)) + address = get_authorized_address() + cli_logger.info(f"Encrypted keyfile saved for address: {address}") + + +def valid_keyfile_exists() -> bool: + try: + get_authorized_address() + except AuthorizationError: + return False + return True + + +def validate_private_key_length(private_key: str) -> None: + if len(private_key) != 32: + raise AuthorizationError(f"{private_key} is not 32 long") + def import_keyfile(keyfile_path: Path) -> None: validate_keyfile(keyfile_path) ethpm_xdg_root = get_xdg_ethpmcli_root() ethpm_cli_keyfile_path = ethpm_xdg_root / KEYFILE_PATH - tmp_keyfile = Path(tempfile.NamedTemporaryFile().name) - tmp_keyfile.write_text(keyfile_path.read_text()) - tmp_keyfile.replace(ethpm_cli_keyfile_path) + with atomic_replace(ethpm_cli_keyfile_path) as file: + file.write(keyfile_path.read_text()) def get_keyfile_path() -> Path: @@ -49,7 +130,7 @@ def get_authorized_address() -> Address: Returns the address associated with stored keyfile. No password required. """ keyfile = get_keyfile_data() - return keyfile["address"] + return add_0x_prefix(keyfile["address"]) def get_authorized_private_key(password: str) -> str: diff --git a/ethpm_cli/commands/manifest.py b/ethpm_cli/commands/manifest.py index 9eefdab..334b1fa 100644 --- a/ethpm_cli/commands/manifest.py +++ b/ethpm_cli/commands/manifest.py @@ -11,6 +11,7 @@ from ethpm.validation.package import validate_package_name from web3 import Web3 +from ethpm_cli._utils.input import parse_bool_flag from ethpm_cli._utils.logger import cli_logger from ethpm_cli._utils.shellart import bold_blue from ethpm_cli._utils.solc import ( @@ -450,18 +451,6 @@ def gen_links() -> Optional[Callable[..., Manifest]]: return None -def parse_bool_flag(question: str) -> bool: - while True: - response = input(f"{question} (y/n) ") - if response.lower() == "y": - return True - elif response.lower() == "n": - return False - else: - cli_logger.info(f"Invalid response: {response}.") - continue - - def write_manifest_to_disk(manifest: Manifest, project_dir: Path) -> None: while True: filename = input("Please enter a filename for your manifest. ") diff --git a/ethpm_cli/commands/package.py b/ethpm_cli/commands/package.py index dc1aa9c..1363f8f 100644 --- a/ethpm_cli/commands/package.py +++ b/ethpm_cli/commands/package.py @@ -93,7 +93,7 @@ def resolve_install_uri(args: Namespace) -> ResolvedInstallURI: ) registry_address = None elif registry_backend.can_translate_uri(args.uri): - registry_address, _, _, _, _ = parse_registry_uri(args.uri) + registry_address, _, _, _, _, _ = parse_registry_uri(args.uri) manifest_uri = registry_backend.fetch_uri_contents(args.uri) else: manifest_uri = args.uri diff --git a/ethpm_cli/config.py b/ethpm_cli/config.py index 28064ff..9f0be9e 100644 --- a/ethpm_cli/config.py +++ b/ethpm_cli/config.py @@ -16,7 +16,7 @@ from ethpm_cli._utils.ipfs import get_ipfs_backend from ethpm_cli._utils.logger import cli_logger from ethpm_cli._utils.xdg import get_xdg_ethpmcli_root -from ethpm_cli.commands.auth import get_authorized_private_key, import_keyfile +from ethpm_cli.commands.auth import get_authorized_private_key from ethpm_cli.constants import ( ETHPM_DIR_ENV_VAR, ETHPM_PACKAGES_DIR, @@ -63,16 +63,14 @@ def __init__(self, args: Namespace) -> None: else: chain_id = 1 - if "keyfile_path" in args and args.keyfile_path: - import_keyfile(args.keyfile_path) - + # setup auth if "keyfile_password" in args and args.keyfile_password: self.private_key = get_authorized_private_key(args.keyfile_password) self.w3 = setup_w3(chain_id, self.private_key) # Setup xdg ethpm dir self.xdg_ethpmcli_root = get_xdg_ethpmcli_root() - setup_xdg_ethpm_dir(self.xdg_ethpmcli_root, self.w3) + validate_xdg_ethpm_dir(self.xdg_ethpmcli_root, self.w3) # Setup projects dir if "project_dir" in args and args.project_dir: @@ -114,7 +112,7 @@ def setup_w3(chain_id: int, private_key: str = None) -> Web3: return w3 -def setup_xdg_ethpm_dir(xdg_ethpmcli_root: Path, w3: Web3) -> None: +def validate_xdg_ethpm_dir(xdg_ethpmcli_root: Path, w3: Web3) -> None: if not xdg_ethpmcli_root.is_dir(): initialize_xdg_ethpm_dir(xdg_ethpmcli_root, w3) diff --git a/ethpm_cli/parser.py b/ethpm_cli/parser.py index eef0ea2..9db5289 100644 --- a/ethpm_cli/parser.py +++ b/ethpm_cli/parser.py @@ -8,7 +8,12 @@ from ethpm_cli._utils.solc import compile_contracts, generate_solc_input from ethpm_cli._utils.xdg import get_xdg_ethpmcli_root from ethpm_cli.commands.activate import activate_package -from ethpm_cli.commands.auth import get_authorized_address +from ethpm_cli.commands.auth import ( + get_authorized_address, + init_keyfile, + link_keyfile, + unlink_keyfile, +) from ethpm_cli.commands.get import get_manifest from ethpm_cli.commands.install import ( install_package, @@ -118,7 +123,7 @@ def add_alias_arg_to_parser(parser: argparse.ArgumentParser) -> None: # todo: extend to pin & release a local manifest -def release_cmd(args: argparse.Namespace) -> None: +def release_action(args: argparse.Namespace) -> None: config = Config(args) release_package(args.package_name, args.package_version, args.manifest_uri, config) active_registry = get_active_registry(config.xdg_ethpmcli_root / REGISTRY_STORE) @@ -153,7 +158,7 @@ def release_cmd(args: argparse.Namespace) -> None: help="Content addressed URI at which the manifest for released package is located.", ) add_keyfile_password_arg_to_parser(release_parser) -release_parser.set_defaults(func=release_cmd) +release_parser.set_defaults(func=release_action) # @@ -165,19 +170,58 @@ def auth_action(args: argparse.Namespace) -> None: Config(args) try: authorized_address = get_authorized_address() - cli_logger.info(f"Keyfile stored for address: 0x{authorized_address}.") + cli_logger.info(f"Keyfile stored for address: {authorized_address}.") except AuthorizationError: cli_logger.info( - "No valid keyfile found. Use `ethpm auth --keyfile-path ` " - "to set your keyfile for use with ethPM CLI." + "No valid keyfile found. " + "If you already have a keyfile, use `ethpm auth link --keyfile-path [path_to_keyfile]` " + "to set your keyfile for use with ethPM CLI. \n" + "To generate an encrypted keyfile, use `ethpm auth init`" ) +def auth_link_action(args: argparse.Namespace) -> None: + Config(args) + if "keyfile_path" not in args or not args.keyfile_path: + cli_logger.info("Invalid --keyfile-path flag") + else: + link_keyfile(args.keyfile_path) + + +def auth_unlink_action(args: argparse.Namespace) -> None: + Config(args) + unlink_keyfile() + + +def auth_init_action(args: argparse.Namespace) -> None: + Config(args) + init_keyfile() + + auth_parser = ethpm_parser.add_parser( "auth", help="Authorization for automatically signing txs." ) -add_keyfile_path_arg_to_parser(auth_parser) auth_parser.set_defaults(func=auth_action) +auth_subparsers = auth_parser.add_subparsers(dest="auth") + +# ethpm auth link +auth_link_parser = auth_subparsers.add_parser( + "link", help="Link an encrypted keyfile.", +) +add_keyfile_path_arg_to_parser(auth_link_parser) +auth_link_parser.set_defaults(func=auth_link_action) + +# ethpm auth unlink +auth_unlink_parser = auth_subparsers.add_parser( + "unlink", help="Delete the encrypted keyfile used for private key auth.", +) +auth_unlink_parser.set_defaults(func=auth_unlink_action) + +# ethpm auth init +auth_init_parser = auth_subparsers.add_parser( + "init", help="Initialize and save an encrypted keyfile for private key auth.", +) +auth_init_parser.set_defaults(func=auth_init_action) # @@ -185,12 +229,12 @@ def auth_action(args: argparse.Namespace) -> None: # -def registry_list_cmd(args: argparse.Namespace) -> None: +def registry_list_action(args: argparse.Namespace) -> None: config = Config(args) list_registries(config) -def registry_add_cmd(args: argparse.Namespace) -> None: +def registry_add_action(args: argparse.Namespace) -> None: config = Config(args) add_registry(args.uri, args.alias, config) if args.alias: @@ -202,13 +246,13 @@ def registry_add_cmd(args: argparse.Namespace) -> None: cli_logger.info(log_msg) -def registry_activate_cmd(args: argparse.Namespace) -> None: +def registry_activate_action(args: argparse.Namespace) -> None: config = Config(args) activate_registry(args.uri_or_alias, config) cli_logger.info(f"Registry @ {args.uri_or_alias} activated.") -def registry_deploy_cmd(args: argparse.Namespace) -> None: +def registry_deploy_action(args: argparse.Namespace) -> None: config = Config(args) registry_address = deploy_registry(config, args.alias) chain_name = SUPPORTED_CHAIN_IDS[config.w3.eth.chainId] @@ -221,7 +265,7 @@ def registry_deploy_cmd(args: argparse.Namespace) -> None: ) -def registry_remove_cmd(args: argparse.Namespace) -> None: +def registry_remove_action(args: argparse.Namespace) -> None: config = Config(args) remove_registry(args.uri_or_alias, config) cli_logger.info(f"Registry: {args.uri_or_alias} removed from registry store.") @@ -238,13 +282,13 @@ def registry_remove_cmd(args: argparse.Namespace) -> None: add_alias_arg_to_parser(registry_deploy_parser) add_chain_id_arg_to_parser(registry_deploy_parser) add_keyfile_password_arg_to_parser(registry_deploy_parser) -registry_deploy_parser.set_defaults(func=registry_deploy_cmd) +registry_deploy_parser.set_defaults(func=registry_deploy_action) # ethpm registry list registry_list_parser = registry_subparsers.add_parser( "list", help="List all of the available registries in registry store." ) -registry_list_parser.set_defaults(func=registry_list_cmd) +registry_list_parser.set_defaults(func=registry_list_action) # ethpm registry add registry_add_parser = registry_subparsers.add_parser( @@ -254,7 +298,7 @@ def registry_remove_cmd(args: argparse.Namespace) -> None: "uri", action="store", type=str, help="Registry URI for target registry." ) add_alias_arg_to_parser(registry_add_parser) -registry_add_parser.set_defaults(func=registry_add_cmd) +registry_add_parser.set_defaults(func=registry_add_action) # ethpm registry remove registry_remove_parser = registry_subparsers.add_parser( @@ -263,7 +307,7 @@ def registry_remove_cmd(args: argparse.Namespace) -> None: registry_remove_parser.add_argument( "uri_or_alias", type=str, help="Registry URI or alias for registry to remove." ) -registry_remove_parser.set_defaults(func=registry_remove_cmd) +registry_remove_parser.set_defaults(func=registry_remove_action) # ethpm registry activate registry_activate_parser = registry_subparsers.add_parser( @@ -276,7 +320,7 @@ def registry_remove_cmd(args: argparse.Namespace) -> None: type=str, help="Registry URI or alias for target registry.", ) -registry_activate_parser.set_defaults(func=registry_activate_cmd) +registry_activate_parser.set_defaults(func=registry_activate_action) # @@ -284,13 +328,13 @@ def registry_remove_cmd(args: argparse.Namespace) -> None: # -def create_solc_input_cmd(args: argparse.Namespace) -> None: +def create_solc_input_action(args: argparse.Namespace) -> None: config = Config(args) validate_config_has_project_dir_attr(config) generate_solc_input(args.project_dir / "contracts") -def create_wizard_cmd(args: argparse.Namespace) -> None: +def create_wizard_action(args: argparse.Namespace) -> None: config = Config(args) if config.project_dir and not config.manifest_path: if not (config.project_dir / SOLC_OUTPUT).exists(): @@ -357,7 +401,7 @@ def create_basic_cmd(args: argparse.Namespace) -> None: help="Generate solidity compiler standard json input for given project directory.", ) add_project_dir_arg_to_parser(create_solc_input_parser) -create_solc_input_parser.set_defaults(func=create_solc_input_cmd) +create_solc_input_parser.set_defaults(func=create_solc_input_action) # ethpm create wizard create_wizard_parser = create_subparsers.add_parser( @@ -373,7 +417,7 @@ def create_basic_cmd(args: argparse.Namespace) -> None: help="Path of target manifest to amend.", ) add_project_dir_arg_to_parser(create_wizard_parser) -create_wizard_parser.set_defaults(func=create_wizard_cmd) +create_wizard_parser.set_defaults(func=create_wizard_action) # diff --git a/tests/cli/test_auth_cli.py b/tests/cli/test_auth_cli.py new file mode 100644 index 0000000..d4754a6 --- /dev/null +++ b/tests/cli/test_auth_cli.py @@ -0,0 +1,130 @@ +import json + +import pexpect +import pytest +from web3.auto.infura import w3 + +from ethpm_cli.config import initialize_xdg_ethpm_dir +from ethpm_cli.constants import KEYFILE_PATH +from ethpm_cli.main import ENTRY_DESCRIPTION + +# taken from eth-keyfile readme +PRIVATE_KEY = b"\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01" # noqa: E501 +PASSWORD = b"foo" +ENCRYPTED_KEYFILE_JSON = '{"address": "1a642f0e3c3af545e7acbd38b07251b3990914f1", "crypto": {"cipher": "aes-128-ctr", "cipherparams": {"iv": "6087dab2f9fdbbfaddc31a909735c1e6"}, "ciphertext": "5318b4d5bcd28de64ee5559e671353e16f075ecae9f99c7a79a38af5f869aa46", "kdf": "pbkdf2", "kdfparams": {"c": 262144, "dklen": 32, "prf": "hmac-sha256", "salt": "ae3cd4e7013836a3df6bd7241b12db061dbe2c6785853cce422d148a624ce0bd"}, "mac": "517ead924a9d0dc3124507e3393d175ce3ff7c1e96529c6c555ce9e51205e9b2"}, "id": "3198bc9c-6672-5ab3-d995-4942343ae5b6", "version": 3}' # noqa: E501 + + +@pytest.fixture +def tmp_xdg(tmp_path, monkeypatch): + tmp_xdg_dir = tmp_path / "xdg" + initialize_xdg_ethpm_dir(tmp_xdg_dir, w3) + + tmp_keyfile = tmp_path / "keyfile.json" + tmp_keyfile.write_text(ENCRYPTED_KEYFILE_JSON) + return tmp_xdg_dir, tmp_keyfile + + +def test_auth_without_keyfile(tmp_xdg): + child = pexpect.spawn("ethpm auth") + child.expect(ENTRY_DESCRIPTION) + child.expect("\r\n") + child.expect(f"No valid keyfile found.") + + +def test_auth_link_keyfile(tmp_xdg): + tmp_xdg_dir, tmp_keyfile = tmp_xdg + child = pexpect.spawn(f"ethpm auth link --keyfile-path {tmp_keyfile}") + child.expect("\r\n") + child.expect( + f"Keyfile stored for address: 0x1a642f0e3c3af545e7acbd38b07251b3990914f1" + ) + child.expect( + "It's now available for use when its password is passed in with the " + "`--keyfile-password` flag." + ) + assert (tmp_xdg_dir / KEYFILE_PATH).read_text() == tmp_keyfile.read_text() + + +def test_auth_link_requires_keyfile_path_flag(tmp_xdg): + child = pexpect.spawn(f"ethpm auth link") + child.expect("\r\n") + child.expect(f"Invalid --keyfile-path flag") + + +def test_auth_link_with_invalid_keyfile(tmp_xdg): + tmp_xdg_dir, _ = tmp_xdg + invalid_keyfile = tmp_xdg_dir / "invalid.json" + invalid_keyfile.write_text(json.dumps({"version": 4})) + child = pexpect.spawn(f"ethpm auth link --keyfile-path {invalid_keyfile}") + child.expect("\r\n") + child.expect( + f"Keyfile found at {invalid_keyfile} does not look like a supported eth-keyfile object." + ) + + +def test_auth_link_with_existing_keyfile(tmp_xdg): + tmp_xdg_dir, tmp_keyfile = tmp_xdg + (tmp_xdg_dir / KEYFILE_PATH).write_text(ENCRYPTED_KEYFILE_JSON) + child = pexpect.spawn(f"ethpm auth link --keyfile-path {tmp_keyfile}") + child.expect("\r\n") + child.expect( + f"Keyfile detected at {tmp_xdg_dir / KEYFILE_PATH}. Please use " + "`ethpm auth unlink` to delete this" + ) + + +def test_auth_unlink(tmp_xdg): + tmp_xdg_dir, tmp_keyfile = tmp_xdg + (tmp_xdg_dir / KEYFILE_PATH).write_text(ENCRYPTED_KEYFILE_JSON) + child = pexpect.spawn(f"ethpm auth unlink") + child.expect("\r\n") + child.expect( + "Keyfile removed for address: 0x1a642f0e3c3af545e7acbd38b07251b3990914f1" + ) + + +def test_auth_unlink_without_stored_keyfile(tmp_xdg): + tmp_xdg_dir, tmp_keyfile = tmp_xdg + (tmp_xdg_dir / KEYFILE_PATH).write_text("") + child = pexpect.spawn(f"ethpm auth unlink") + child.expect("\r\n") + child.expect("Unable to unlink keyfile: empty keyfile found.") + + +def test_auth_init(tmp_xdg): + tmp_xdg_dir, tmp_keyfile = tmp_xdg + (tmp_xdg_dir / KEYFILE_PATH).write_text("") + child = pexpect.spawn(f"ethpm auth init") + child.expect("\r\n") + child.expect( + "Please be careful when using your private key, it is a sensitive piece of information." + ) + child.expect("Are you sure you want to proceed with initializing a keyfile?") + child.sendline("Y") + child.expect("Please enter your 32-length private key:") + child.sendline(PRIVATE_KEY) + child.expect("Please enter a password to encrypt your keyfile with") + child.sendline(PASSWORD) + child.expect("Encrypted keyfile saved for address:") + actual_keyfile_json = json.loads((tmp_xdg_dir / KEYFILE_PATH).read_text()) + expected_keyfile_json = json.loads(ENCRYPTED_KEYFILE_JSON) + assert actual_keyfile_json["address"] == expected_keyfile_json["address"] + assert actual_keyfile_json["version"] == expected_keyfile_json["version"] + + +def test_auth_init_abort(tmp_xdg): + tmp_xdg_dir, tmp_keyfile = tmp_xdg + (tmp_xdg_dir / KEYFILE_PATH).write_text("") + child = pexpect.spawn(f"ethpm auth init") + child.expect("\r\n") + child.expect("Are you sure you want to proceed with initializing a keyfile?") + child.sendline("n") + child.expect("Aborting keyfile initialization") + + +def test_auth_init_with_existing_keyfile(tmp_xdg): + tmp_xdg_dir, tmp_keyfile = tmp_xdg + (tmp_xdg_dir / KEYFILE_PATH).write_text(ENCRYPTED_KEYFILE_JSON) + child = pexpect.spawn(f"ethpm auth init") + child.expect("\r\n") + child.expect("Keyfile detected.")