From 7c5b10b37eb140cc9d3c5918deb8f4663158ae31 Mon Sep 17 00:00:00 2001 From: Mike Shultz Date: Mon, 29 Jan 2024 15:03:16 -0700 Subject: [PATCH] feat: adds account generation and import API functions (#1887) * feat: adds ape_accounts.generate_account for programattic account creation * refactor: review feedback; code hygiene * fix: missing file * feat: warn on simple passwords * fix: make prompt order the same as it was originally * fix(test): handle prompt difference in CLI integration account tests * test: use correct test account, fix passwords and output parsing * style: doulbe spaces are for typewriters and old people * docs: document generate_account use in accounts user guide * feat: adds import_account_from_mnemonic and import_account_from_private_key * fix: remove unused arg from import_account_from_private_key() * docs: document use of import_account_from_mnemonic() and import_account_from_private_key() * style: review feedback, docstrings and cleanup * chore: include space in special char validator * docs: clarity fix Co-authored-by: antazoey * docs: use class def notation for docstring Co-authored-by: antazoey * style(lint): line too long * fix(docs): incorrect import in example Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com> * docs: adds ape_accounts autodoc, improves accounts userguide additions * docs: wording review feedback * test: use eth_tester_provider fixture in test_default_sender_account * docs: link to ape_accounts autodoc page from index ToC * test: use different alias in test_import_account_from_mnemonic * test: adds delete_account_after fixture, providing context manager that deletes account files after closing --------- Co-authored-by: antazoey Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com> --- docs/index.md | 1 + docs/methoddocs/ape_accounts.md | 6 + docs/userguides/accounts.md | 55 ++++- src/ape/cli/arguments.py | 14 +- src/ape/utils/validators.py | 49 +++++ src/ape_accounts/__init__.py | 17 +- src/ape_accounts/_cli.py | 60 +++--- src/ape_accounts/accounts.py | 91 +++++++- tests/conftest.py | 18 +- tests/functional/conftest.py | 13 ++ tests/functional/test_accounts.py | 195 ++++++++++++++++-- .../test_default_sender_context_manager.py | 7 +- tests/integration/cli/test_accounts.py | 37 +++- 13 files changed, 486 insertions(+), 77 deletions(-) create mode 100644 docs/methoddocs/ape_accounts.md create mode 100644 src/ape/utils/validators.py diff --git a/docs/index.md b/docs/index.md index 6154bff3e5..f475f90bfa 100644 --- a/docs/index.md +++ b/docs/index.md @@ -48,6 +48,7 @@ :maxdepth: 1 methoddocs/ape.md + methoddocs/ape_accounts.md methoddocs/api.md methoddocs/cli.md methoddocs/contracts.md diff --git a/docs/methoddocs/ape_accounts.md b/docs/methoddocs/ape_accounts.md new file mode 100644 index 0000000000..c767fdda8d --- /dev/null +++ b/docs/methoddocs/ape_accounts.md @@ -0,0 +1,6 @@ +# ape-accounts + +```{eval-rst} +.. automodule:: ape_accounts + :members: +``` diff --git a/docs/userguides/accounts.md b/docs/userguides/accounts.md index 973972e144..586ec62aad 100644 --- a/docs/userguides/accounts.md +++ b/docs/userguides/accounts.md @@ -95,11 +95,14 @@ Under-the-hood, this structure comes from the [eth-keyfile library](https://gith When Ape creates the keyfile, either from import or account-generation (described below!), it prompts you for a passphrase to use for encrypting the keyfile, similarly to how you would use a password in browser-based wallets. The keyfile stores the private key in an encrypted-at-rest state, which maximizes security of the locally-stored key material. -The `ape-accounts` plugin lets you use keyfile-based account to sign messages and transactions. +The `ape-accounts` core plugin lets you use keyfile-based account to sign messages and transactions. When signing a message or transaction using an account from `ape-accounts`, you will be prompted to enter the passphrase you specified when importing or generating that account. All the available CLI commands for this account's plugin can be found [here](../commands/accounts.html). -For example, you can [generate](../commands/accounts.html#accounts-generate) an account: + +#### Generating New Accounts + +You can [generate](../commands/accounts.html#accounts-generate) an account: ```bash ape accounts generate @@ -134,6 +137,22 @@ ape accounts generate --word-count If you do not use the `--word-count` option, Ape will use the default word count of 12. You can use all of these together or separately to control the way Ape creates and displays your account information. + +This same functionality is also scriptable with the same inputs as the `generate` command: + +```python +from ape_accounts import generate_account + +account, mnemonic = generate_account("my-account", "mySecureP@ssphrase") + +print(f'Save your mnemonic: {mnemonic}') +print(f'Your new account address is: {account.address}') +``` + +See the [documentation for `generate_account()`](../methoddocs/ape_accounts.html#ape_accounts.generate_account) for more options. + +#### Importing Existing Accounts + If you already have an account and wish to import it into Ape (say, from Metamask), you can use the [import command](../commands/accounts.html#accounts-import): ```bash @@ -158,6 +177,38 @@ ape accounts import --use-mnemonic --hd-path If you use the `--hd-path` option, you will need to pass the [HDPath](https://help.myetherwallet.com/en/articles/5867305-hd-wallets-and-derivation-paths) you'd like to use as an argument in the command. If you do not use the `--hd-path` option, Ape will use the default HDPath of (Ethereum network, first account). + +You can import an account programatically using a seed phrase [using `import_account_from_mnemonic()`](../methoddocs/ape_accounts.html#ape_accounts.import_account_from_mnemonic): + +```python +from ape_acounts import import_account_from_mnemonic + +alias = "my-account" +passphrase = "my$ecurePassphrase" +mnemonic = "test test test test test test test test test test test junk" + +account = import_account_from_mnemonic(alias, passphrase, mnemonic) + +print(f'Your imported account address is: {account.address}') +``` + +Or using a raw private key [using `import_account_from_private_key()`](../methoddocs/ape_accounts.html#ape_accounts.import_account_from_private_key): + +```python +import os +from ape_acounts import import_account_from_private_key + +alias = "my-account" +passphrase = "my SecurePassphrase" +private_key = os.urandom(32).hex() + +account = import_account_from_private_key(alias, passphrase, private_key) + +print(f'Your imported account address is: {account.address}') +``` + +#### Exporting Accounts + You can also [export](../commands/accounts.html#accounts-export) the private key of an account: ```bash diff --git a/src/ape/cli/arguments.py b/src/ape/cli/arguments.py index a2be011d94..5b1e99be58 100644 --- a/src/ape/cli/arguments.py +++ b/src/ape/cli/arguments.py @@ -1,27 +1,17 @@ from itertools import chain import click -from eth_utils import is_hex from ape.cli.choices import _ACCOUNT_TYPE_FILTER, Alias from ape.cli.paramtype import AllFilePaths -from ape.exceptions import AccountsError, AliasAlreadyInUseError from ape.utils.basemodel import ManagerAccessMixin +from ape.utils.validators import _validate_account_alias _flatten = chain.from_iterable def _alias_callback(ctx, param, value): - if value in ManagerAccessMixin.account_manager.aliases: - # Alias cannot be used. - raise AliasAlreadyInUseError(value) - - elif not isinstance(value, str): - raise AccountsError(f"Alias must be a str, not '{type(value)}'.") - elif is_hex(value) and len(value) >= 42: - raise AccountsError("Longer aliases cannot be hex strings.") - - return value + return _validate_account_alias(value) def existing_alias_argument(account_type: _ACCOUNT_TYPE_FILTER = None, **kwargs): diff --git a/src/ape/utils/validators.py b/src/ape/utils/validators.py new file mode 100644 index 0000000000..f7e1d72833 --- /dev/null +++ b/src/ape/utils/validators.py @@ -0,0 +1,49 @@ +"""Base non-pydantic validator utils""" +import re +from warnings import warn + +from eth_utils import is_hex + +from ape.exceptions import AccountsError, AliasAlreadyInUseError +from ape.utils.basemodel import ManagerAccessMixin + +MIN_PASSPHRASE_LENGTH = 6 + + +def _has_num(val: str): + return re.search(r"\d{1}", val) is not None + + +def _has_special(val: str): + return re.search(r"[\!@#$%\^&\*\(\) ]{1}", val) is not None + + +def _validate_account_alias(alias: str) -> str: + """Validate an account alias""" + if alias in ManagerAccessMixin.account_manager.aliases: + raise AliasAlreadyInUseError(alias) + elif not isinstance(alias, str): + raise AccountsError(f"Alias must be a str, not '{type(alias)}'.") + elif is_hex(alias) and len(alias) >= 42: + # Prevents private keys from accidentally being stored in plaintext + # Ref: https://github.com/ApeWorX/ape/issues/1525 + raise AccountsError("Longer aliases cannot be hex strings.") + + return alias + + +def _validate_account_passphrase(passphrase: str) -> str: + """Make sure given passphrase is valid for account encryption""" + if not passphrase or not isinstance(passphrase, str): + raise AccountsError("Account file encryption passphrase must be provided.") + + if len(passphrase) < MIN_PASSPHRASE_LENGTH: + warn("Passphrase length is extremely short. Consider using something longer.") + + if not (_has_num(passphrase) or _has_special(passphrase)): + warn("Passphrase complexity is simple. Consider using numbers and special characters.") + + return passphrase + + +__all__ = ["_validate_account_alias", "_validate_account_passphrase"] diff --git a/src/ape_accounts/__init__.py b/src/ape_accounts/__init__.py index 9db73b137a..862b78819b 100644 --- a/src/ape_accounts/__init__.py +++ b/src/ape_accounts/__init__.py @@ -1,8 +1,23 @@ from ape import plugins -from .accounts import AccountContainer, KeyfileAccount +from .accounts import ( + AccountContainer, + KeyfileAccount, + generate_account, + import_account_from_mnemonic, + import_account_from_private_key, +) @plugins.register(plugins.AccountPlugin) def account_types(): return AccountContainer, KeyfileAccount + + +__all__ = [ + "AccountContainer", + "KeyfileAccount", + "generate_account", + "import_account_from_mnemonic", + "import_account_from_private_key", +] diff --git a/src/ape_accounts/_cli.py b/src/ape_accounts/_cli.py index f1586cac98..0374c2217a 100644 --- a/src/ape_accounts/_cli.py +++ b/src/ape_accounts/_cli.py @@ -1,13 +1,20 @@ import json +from typing import Optional import click from eth_account import Account as EthAccount from eth_account.hdaccount import ETHEREUM_DEFAULT_PATH -from eth_utils import to_bytes, to_checksum_address +from eth_utils import to_checksum_address from ape.cli import ape_cli_context, existing_alias_argument, non_existing_alias_argument from ape.utils.basemodel import ManagerAccessMixin -from ape_accounts import AccountContainer, KeyfileAccount +from ape_accounts import ( + AccountContainer, + KeyfileAccount, + generate_account, + import_account_from_mnemonic, + import_account_from_private_key, +) def _get_container() -> AccountContainer: @@ -88,26 +95,24 @@ def _list(cli_ctx, show_all_plugins): @non_existing_alias_argument() @ape_cli_context() def generate(cli_ctx, alias, hide_mnemonic, word_count, custom_hd_path): - path = _get_container().data_folder.joinpath(f"{alias}.json") - EthAccount.enable_unaudited_hdwallet_features() - # os.urandom (used internally for this method) requires a certain amount of entropy. - # Adding entropy increases os.urandom randomness output - # Despite not being used in create_with_mnemonic click.prompt( "Enhance the security of your account by adding additional random input", hide_input=True, ) - account, mnemonic = EthAccount.create_with_mnemonic( - num_words=word_count, account_path=custom_hd_path - ) - if not hide_mnemonic and click.confirm("Show mnemonic?", default=True): - cli_ctx.logger.info(f"Newly generated mnemonic is: {click.style(mnemonic, bold=True)}") + + show_mnemonic = not hide_mnemonic and click.confirm("Show mnemonic?", default=True) + passphrase = click.prompt( "Create Passphrase to encrypt account", hide_input=True, confirmation_prompt=True, ) - path.write_text(json.dumps(EthAccount.encrypt(account.key, passphrase))) + + account, mnemonic = generate_account(alias, passphrase, custom_hd_path, word_count) + + if show_mnemonic: + cli_ctx.logger.info(f"Newly generated mnemonic is: {click.style(mnemonic, bold=True)}") + cli_ctx.logger.success( f"A new account '{account.address}' with " + f"HDPath {custom_hd_path} has been added with the id '{alias}'" @@ -129,31 +134,36 @@ def generate(cli_ctx, alias, hide_mnemonic, word_count, custom_hd_path): @non_existing_alias_argument() @ape_cli_context() def _import(cli_ctx, alias, import_from_mnemonic, custom_hd_path): - path = _get_container().data_folder.joinpath(f"{alias}.json") + account: Optional[KeyfileAccount] = None + + def ask_for_passphrase(): + return click.prompt( + "Create Passphrase to encrypt account", + hide_input=True, + confirmation_prompt=True, + ) + if import_from_mnemonic: mnemonic = click.prompt("Enter mnemonic seed phrase", hide_input=True) EthAccount.enable_unaudited_hdwallet_features() try: - account = EthAccount.from_mnemonic(mnemonic=mnemonic, account_path=custom_hd_path) + passphrase = ask_for_passphrase() + account = import_account_from_mnemonic(alias, passphrase, mnemonic, custom_hd_path) except Exception as error: cli_ctx.abort(f"Seed phrase can't be imported: {error}") else: key = click.prompt("Enter Private Key", hide_input=True) try: - account = EthAccount.from_key(to_bytes(hexstr=key)) + passphrase = ask_for_passphrase() + account = import_account_from_private_key(alias, passphrase, key) except Exception as error: cli_ctx.abort(f"Key can't be imported: {error}") - passphrase = click.prompt( - "Create Passphrase to encrypt account", - hide_input=True, - confirmation_prompt=True, - ) - path.write_text(json.dumps(EthAccount.encrypt(account.key, passphrase))) - cli_ctx.logger.success( - f"A new account '{account.address}' has been added with the id '{alias}'" - ) + if account: + cli_ctx.logger.success( + f"A new account '{account.address}' has been added with the id '{alias}'" + ) @cli.command(short_help="Export an account private key") diff --git a/src/ape_accounts/accounts.py b/src/ape_accounts/accounts.py index 8e653cedb4..e0e2451b4b 100644 --- a/src/ape_accounts/accounts.py +++ b/src/ape_accounts/accounts.py @@ -1,12 +1,14 @@ import json from os import environ from pathlib import Path -from typing import Any, Dict, Iterator, Optional +from typing import Any, Dict, Iterator, Optional, Tuple import click from eip712.messages import EIP712Message from eth_account import Account as EthAccount +from eth_account.hdaccount import ETHEREUM_DEFAULT_PATH from eth_account.messages import encode_defunct +from eth_account.signers.local import LocalAccount from eth_keys import keys # type: ignore from eth_pydantic_types import HexBytes from eth_utils import to_bytes @@ -15,6 +17,8 @@ from ape.exceptions import AccountsError from ape.logging import logger from ape.types import AddressType, MessageSignature, SignableMessage, TransactionSignature +from ape.utils.basemodel import ManagerAccessMixin +from ape.utils.validators import _validate_account_alias, _validate_account_passphrase class InvalidPasswordError(AccountsError): @@ -257,3 +261,88 @@ def __decrypt_keyfile(self, passphrase: str) -> HexBytes: return EthAccount.decrypt(self.keyfile, passphrase) except ValueError as err: raise InvalidPasswordError() from err + + +def _write_and_return_account(alias: str, passphrase: str, account: LocalAccount) -> KeyfileAccount: + """Write an account to disk and return an Ape KeyfileAccount""" + path = ManagerAccessMixin.account_manager.containers["accounts"].data_folder.joinpath( + f"{alias}.json" + ) + path.write_text(json.dumps(EthAccount.encrypt(account.key, passphrase))) + + return KeyfileAccount(keyfile_path=path) + + +def generate_account( + alias: str, passphrase: str, hd_path: str = ETHEREUM_DEFAULT_PATH, word_count: int = 12 +) -> Tuple[KeyfileAccount, str]: + """ + Generate a new account. + + Args: + alias (str): The alias name of the account. + passphrase (str): Passphrase used to encrypt the account storage file. + hd_path (str): The hierarchal deterministic path to use when generating the account. + Defaults to `m/44'/60'/0'/0/0`. + word_count (int): The amount of words to use in the generated mnemonic. + + Returns: + Tuple of :class:`~ape_accounts.accounts.KeyfileAccount` and mnemonic for the generated + account. + """ + EthAccount.enable_unaudited_hdwallet_features() + + alias = _validate_account_alias(alias) + passphrase = _validate_account_passphrase(passphrase) + + account, mnemonic = EthAccount.create_with_mnemonic(num_words=word_count, account_path=hd_path) + ape_account = _write_and_return_account(alias, passphrase, account) + return ape_account, mnemonic + + +def import_account_from_mnemonic( + alias: str, passphrase: str, mnemonic: str, hd_path: str = ETHEREUM_DEFAULT_PATH +) -> KeyfileAccount: + """ + Import a new account from a mnemonic seed phrase. + + Args: + alias (str): The alias name of the account. + passphrase (str): Passphrase used to encrypt the account storage file. + mnemonic (str): List of space-separated words representing the mnemonic seed phrase. + hd_path (str): The hierarchal deterministic path to use when generating the account. + Defaults to `m/44'/60'/0'/0/0`. + + Returns: + Tuple of AccountAPI and mnemonic for the generated account. + """ + EthAccount.enable_unaudited_hdwallet_features() + + alias = _validate_account_alias(alias) + passphrase = _validate_account_passphrase(passphrase) + + account = EthAccount.from_mnemonic(mnemonic, account_path=hd_path) + + return _write_and_return_account(alias, passphrase, account) + + +def import_account_from_private_key( + alias: str, passphrase: str, private_key: str +) -> KeyfileAccount: + """ + Import a new account from a mnemonic seed phrase. + + Args: + alias (str): The alias name of the account. + passphrase (str): Passphrase used to encrypt the account storage file. + private_key (str): Hex string private key to import. + + Returns: + Tuple of AccountAPI and mnemonic for the generated account. + """ + alias = _validate_account_alias(alias) + passphrase = _validate_account_passphrase(passphrase) + + account = EthAccount.from_key(to_bytes(hexstr=private_key)) + + return _write_and_return_account(alias, passphrase, account) diff --git a/tests/conftest.py b/tests/conftest.py index 32db51221f..c7229780fa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -150,24 +150,24 @@ def dependency_manager(project): @pytest.fixture(scope="session") def keyparams(): - # NOTE: password is 'a' + # NOTE: password is 'asdf1234' return { - "address": "f39fd6e51aad88f6f4ce6ab8827279cfffb92266", + "address": "f39Fd6e51aad88F6F4ce6aB8827279cffFb92266", "crypto": { "cipher": "aes-128-ctr", - "cipherparams": {"iv": "fe11b8a2576bacd917b02c065f764369"}, - "ciphertext": "80a7a3a901cbfd4720ca77b81b14e4db87da84e0de9d2257dfd7427108c0f573", + "cipherparams": {"iv": "229df0a8949798c192caf21531b64a01"}, + "ciphertext": "c68e03a33ab139a4822f578d76452658c13a0ea370f3c997651613dea8925483", "kdf": "scrypt", "kdfparams": { "dklen": 32, "n": 262144, - "r": 1, - "p": 8, - "salt": "62c6413db938987b0be436335e38b4ae", + "r": 8, + "p": 1, + "salt": "e1f01ece5afa9819e0ff6c1761737c68", }, - "mac": "43935d1a983e13c083e724ebc9d02d5467e0accfa6df8db1fc41f9870f5e776e", + "mac": "10ee5db98f1a653c9bda7657f3b3b8bd55dd2fec93936e6b1783af912f9167c2", }, - "id": "87c9b6e3-5ce4-4758-ab5b-444074e594fe", + "id": "1af390c5-c4cf-46d0-9341-5374e1a84959", "version": 3, } diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py index 3b82ec437b..b2593840bd 100644 --- a/tests/functional/conftest.py +++ b/tests/functional/conftest.py @@ -1,5 +1,6 @@ import threading import time +from contextlib import contextmanager from distutils.dir_util import copy_tree from pathlib import Path from typing import Optional, cast @@ -749,3 +750,15 @@ def fake_partial(*args, **kwargs): if actual: ethereum.sepolia_fork.__dict__["providers"] = actual + + +@pytest.fixture +def delete_account_after(project_path): + @contextmanager + def delete_account_context(alias: str): + yield + account_path = ape.config.DATA_FOLDER / "accounts" / f"{alias}.json" + if account_path.exists(): + account_path.unlink() + + return delete_account_context diff --git a/tests/functional/test_accounts.py b/tests/functional/test_accounts.py index b6bb7bd53b..9ca0f71a84 100644 --- a/tests/functional/test_accounts.py +++ b/tests/functional/test_accounts.py @@ -7,10 +7,22 @@ import ape from ape.api import ImpersonatedAccount -from ape.exceptions import AccountsError, NetworkError, ProjectError, SignatureError +from ape.exceptions import ( + AccountsError, + AliasAlreadyInUseError, + NetworkError, + ProjectError, + SignatureError, +) from ape.types import AutoGasLimit from ape.types.signatures import recover_signer from ape.utils.testing import DEFAULT_NUMBER_OF_TEST_ACCOUNTS +from ape_accounts import ( + KeyfileAccount, + generate_account, + import_account_from_mnemonic, + import_account_from_private_key, +) from ape_ethereum.ecosystem import ProxyType from ape_ethereum.transactions import TransactionType from ape_test.accounts import TestAccount @@ -25,8 +37,10 @@ APE_TEST_PATH = "ape_test.accounts.TestAccount" APE_ACCOUNTS_PATH = "ape_accounts.accounts.KeyfileAccount" -PASSPHRASE = "a" +PASSPHRASE = "asdf1234" INVALID_PASSPHRASE = "incorrect passphrase" +MNEMONIC = "test test test test test test test test test test test junk" +PRIVATE_KEY = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" @pytest.fixture(params=(APE_TEST_PATH, APE_ACCOUNTS_PATH)) @@ -86,7 +100,7 @@ def test_sign_eip712_message(signer): def test_sign_message_with_prompts(runner, keyfile_account, message): # "y\na\ny": yes sign, password, yes keep unlocked start_nonce = keyfile_account.nonce - with runner.isolation(input="y\na\ny"): + with runner.isolation(input=f"y\n{PASSPHRASE}\ny"): signature = keyfile_account.sign_message(message) assert keyfile_account.check_signature(message, signature) @@ -179,7 +193,7 @@ def test_transfer_with_value_send_everything_true(sender, receiver): def test_transfer_with_prompts(runner, receiver, keyfile_account): # "y\na\ny": yes sign, password, yes keep unlocked - with runner.isolation("y\na\ny"): + with runner.isolation(f"y\n{PASSPHRASE}\ny"): receipt = keyfile_account.transfer(receiver, "1 gwei") assert receipt.receiver == receiver @@ -354,24 +368,24 @@ def test_accounts_contains(accounts, owner): def test_autosign_messages(runner, keyfile_account, message): - keyfile_account.set_autosign(True, passphrase="a") + keyfile_account.set_autosign(True, passphrase=PASSPHRASE) signature = keyfile_account.sign_message(message) assert keyfile_account.check_signature(message, signature) # Re-enable prompted signing keyfile_account.set_autosign(False) - with runner.isolation(input="y\na\n"): + with runner.isolation(input=f"y\n{PASSPHRASE}\n"): signature = keyfile_account.sign_message(message) assert keyfile_account.check_signature(message, signature) def test_autosign_transactions(runner, keyfile_account, receiver): - keyfile_account.set_autosign(True, passphrase="a") + keyfile_account.set_autosign(True, passphrase=PASSPHRASE) assert keyfile_account.transfer(receiver, "1 gwei") # Re-enable prompted signing keyfile_account.set_autosign(False) - with runner.isolation(input="y\na\n"): + with runner.isolation(input=f"y\n{PASSPHRASE}\n"): assert keyfile_account.transfer(receiver, "1 gwei") @@ -412,7 +426,7 @@ def test_contract_as_sender_non_fork_network(contract_instance): def test_unlock_with_passphrase_and_sign_message(runner, keyfile_account, message): - keyfile_account.unlock(passphrase="a") + keyfile_account.unlock(passphrase=PASSPHRASE) # y: yes, sign (note: unlocking makes the key available but is not the same as autosign). with runner.isolation(input="y\n"): @@ -422,7 +436,7 @@ def test_unlock_with_passphrase_and_sign_message(runner, keyfile_account, messag def test_unlock_from_prompt_and_sign_message(runner, keyfile_account, message): # a = password - with runner.isolation(input="a\n"): + with runner.isolation(input=f"{PASSPHRASE}\n"): keyfile_account.unlock() # yes, sign the message @@ -432,7 +446,7 @@ def test_unlock_from_prompt_and_sign_message(runner, keyfile_account, message): def test_unlock_with_passphrase_and_sign_transaction(runner, keyfile_account, receiver): - keyfile_account.unlock(passphrase="a") + keyfile_account.unlock(passphrase=PASSPHRASE) # y: yes, sign (note: unlocking makes the key available but is not the same as autosign). with runner.isolation(input="y\n"): receipt = keyfile_account.transfer(receiver, "1 gwei") @@ -441,7 +455,7 @@ def test_unlock_with_passphrase_and_sign_transaction(runner, keyfile_account, re def test_unlock_from_prompt_and_sign_transaction(runner, keyfile_account, receiver): # a = password - with runner.isolation(input="a\n"): + with runner.isolation(input=f"{PASSPHRASE}\n"): keyfile_account.unlock() # yes, sign the transaction @@ -487,7 +501,7 @@ def test_unlock_and_reload(runner, accounts, keyfile_account, message): Tests against a condition where reloading after unlocking would not honor unlocked state. """ - keyfile_account.unlock(passphrase="a") + keyfile_account.unlock(passphrase=PASSPHRASE) reloaded_account = accounts.load(keyfile_account.alias) # y: yes, sign (note: unlocking makes the key available but is not the same as autosign). @@ -662,12 +676,12 @@ def test_prepare_transaction_and_call_using_max_gas(tx_type, ethereum, sender, e def test_public_key(runner, keyfile_account): - with runner.isolation(input="a\ny\n"): + with runner.isolation(input=f"{PASSPHRASE}\ny\n"): assert isinstance(keyfile_account.public_key, HexBytes) def test_load_public_key_from_keyfile(runner, keyfile_account): - with runner.isolation(input="a\ny\n"): + with runner.isolation(input=f"{PASSPHRASE}\ny\n"): assert isinstance(keyfile_account.public_key, HexBytes) assert ( @@ -676,3 +690,154 @@ def test_load_public_key_from_keyfile(runner, keyfile_account): ) # no need for password when loading from the keyfile assert keyfile_account.public_key + + +def test_generate_account(delete_account_after): + alias = "gentester" + with delete_account_after(alias): + account, mnemonic = generate_account(alias, PASSPHRASE) + assert len(mnemonic.split(" ")) == 12 + assert isinstance(account, KeyfileAccount) + assert account.alias == alias + assert account.locked is True + account.unlock(PASSPHRASE) + assert account.locked is False + + +def test_generate_account_invalid_alias(delete_account_after): + with pytest.raises(AccountsError, match="Longer aliases cannot be hex strings."): + generate_account( + "3fbc0ce3e71421b94f7ff4e753849c540dec9ade57bad60ebbc521adcbcbc024", "asdf1234" + ) + + with pytest.raises(AccountsError, match="Alias must be a str"): + # Testing an invalid type as arg, so ignoring + generate_account(b"imma-bytestr", "asdf1234") # type: ignore + + used_alias = "used" + with delete_account_after(used_alias): + generate_account(used_alias, "qwerty1") + with pytest.raises(AliasAlreadyInUseError): + generate_account(used_alias, "asdf1234") + + +def test_generate_account_invalid_passphrase(): + with pytest.raises(AccountsError, match="Account file encryption passphrase must be provided."): + generate_account("invalid-passphrase", "") + + with pytest.raises(AccountsError, match="Account file encryption passphrase must be provided."): + generate_account("invalid-passphrase", b"bytestring") # type: ignore + + +def test_generate_account_insecure_passphrase(delete_account_after): + short_alias = "shortaccount" + with delete_account_after(short_alias): + with pytest.warns(UserWarning, match="short"): + generate_account(short_alias, "short") + + simple_alias = "simpleaccount" + with delete_account_after(simple_alias): + with pytest.warns(UserWarning, match="simple"): + generate_account(simple_alias, "simple") + + +def test_import_account_from_mnemonic(delete_account_after): + alias = "iafmtester" + with delete_account_after(alias): + account = import_account_from_mnemonic(alias, PASSPHRASE, MNEMONIC) + assert isinstance(account, KeyfileAccount) + assert account.alias == alias + assert account.address == "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" + assert account.locked is True + account.unlock(PASSPHRASE) + assert account.locked is False + + +def test_import_account_from_mnemonic_invalid_alias(delete_account_after): + with pytest.raises(AccountsError, match="Longer aliases cannot be hex strings."): + import_account_from_mnemonic( + "3fbc0ce3e71421b94f7ff4e753849c540dec9ade57bad60ebbc521adcbcbc024", "asdf1234", MNEMONIC + ) + + with pytest.raises(AccountsError, match="Alias must be a str"): + # Testing an invalid type as arg, so ignoring + import_account_from_mnemonic(b"imma-bytestr", "asdf1234", MNEMONIC) # type: ignore + + used_alias = "iamfused" + with delete_account_after(used_alias): + import_account_from_mnemonic(used_alias, "qwerty1", MNEMONIC) + with pytest.raises(AliasAlreadyInUseError): + import_account_from_mnemonic(used_alias, "asdf1234", MNEMONIC) + + +def test_import_account_from_mnemonic_invalid_passphrase(): + with pytest.raises(AccountsError, match="Account file encryption passphrase must be provided."): + import_account_from_mnemonic("invalid-passphrase", "", MNEMONIC) + + with pytest.raises(AccountsError, match="Account file encryption passphrase must be provided."): + import_account_from_mnemonic("invalid-passphrase", b"bytestring", MNEMONIC) # type: ignore + + +def test_import_account_from_mnemonic_insecure_passphrase(delete_account_after): + short_alias = "iafmshortaccount" + with delete_account_after(short_alias): + with pytest.warns(UserWarning, match="short"): + import_account_from_mnemonic(short_alias, "short", MNEMONIC) + + simple_alias = "iafmsimpleaccount" + with delete_account_after(simple_alias): + with pytest.warns(UserWarning, match="simple"): + import_account_from_mnemonic(simple_alias, "simple", MNEMONIC) + + +def test_import_account_from_private_key(delete_account_after): + alias = "iafpktester" + with delete_account_after(alias): + account = import_account_from_private_key(alias, PASSPHRASE, PRIVATE_KEY) + assert isinstance(account, KeyfileAccount) + assert account.alias == alias + assert account.address == "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" + assert account.locked is True + account.unlock(PASSPHRASE) + assert account.locked is False + + +def test_import_account_from_private_key_invalid_alias(delete_account_after): + with pytest.raises(AccountsError, match="Longer aliases cannot be hex strings."): + import_account_from_private_key( + "3fbc0ce3e71421b94f7ff4e753849c540dec9ade57bad60ebbc521adcbcbc024", + "asdf1234", + PRIVATE_KEY, + ) + + with pytest.raises(AccountsError, match="Alias must be a str"): + # Testing an invalid type as arg, so ignoring + import_account_from_private_key(b"imma-bytestr", "asdf1234", PRIVATE_KEY) # type: ignore + + used_alias = "iafpkused" + with delete_account_after(used_alias): + import_account_from_private_key(used_alias, "qwerty1", PRIVATE_KEY) + with pytest.raises(AliasAlreadyInUseError): + import_account_from_private_key(used_alias, "asdf1234", PRIVATE_KEY) + + +def test_import_account_from_private_key_invalid_passphrase(): + with pytest.raises(AccountsError, match="Account file encryption passphrase must be provided."): + import_account_from_private_key("invalid-passphrase", "", PRIVATE_KEY) + + with pytest.raises(AccountsError, match="Account file encryption passphrase must be provided."): + import_account_from_private_key( + "invalid-passphrase", b"bytestring", PRIVATE_KEY # type: ignore + ) + + +def test_import_account_from_private_key_insecure_passphrase(delete_account_after): + short_alias = "iafpkshortaccount" + with delete_account_after(short_alias): + with pytest.warns(UserWarning, match="short"): + import_account_from_private_key(short_alias, "short", PRIVATE_KEY) + + simple_alias = "iafpksimpleaccount" + with delete_account_after(simple_alias): + with pytest.warns(UserWarning, match="simple"): + import_account_from_private_key(simple_alias, "simple", PRIVATE_KEY) diff --git a/tests/functional/test_default_sender_context_manager.py b/tests/functional/test_default_sender_context_manager.py index f4288df6ae..e908f8c8da 100644 --- a/tests/functional/test_default_sender_context_manager.py +++ b/tests/functional/test_default_sender_context_manager.py @@ -21,10 +21,13 @@ def test_default_sender_test_account(solidity_contract_instance, owner, test_acc def test_default_sender_account( - solidity_contract_container, networks_connected_to_tester, accounts, keyfile_account + solidity_contract_container, + networks_connected_to_tester, + accounts, + keyfile_account, ): owner = accounts[0] - passphrase = "a" + passphrase = "asdf1234" with accounts.use_sender(owner) as acct: acct.set_autosign(True, passphrase) diff --git a/tests/integration/cli/test_accounts.py b/tests/integration/cli/test_accounts.py index 92f52f1455..f8e5371eec 100644 --- a/tests/integration/cli/test_accounts.py +++ b/tests/integration/cli/test_accounts.py @@ -1,4 +1,6 @@ import json +import re +from typing import List, Optional import pytest from eth_account import Account @@ -7,13 +9,24 @@ from tests.integration.cli.utils import assert_failure, run_once ALIAS = "test" -PASSWORD = "a" +PASSWORD = "asdf1234" PRIVATE_KEY = "0000000000000000000000000000000000000000000000000000000000000001" MNEMONIC = "test test test test test test test test test test test junk" INVALID_MNEMONIC = "test test" CUSTOM_HDPATH = "m/44'/61'/0'/0/0" # Ethereum Classic ($ETC) HDPath +def extract_mnemonic(output: str) -> Optional[List[str]]: + found = re.search(r"Newly generated mnemonic is: ([a-z ]+)", output) + if found: + try: + mnemonic_string = found.group(1) + return mnemonic_string.split(" ") + except IndexError: + pass + return None + + @pytest.fixture(autouse=True) def temp_keyfile_path(temp_accounts_path): test_keyfile_path = temp_accounts_path / f"{ALIAS}.json" @@ -213,8 +226,9 @@ def test_generate_default(ape_cli, runner, temp_keyfile_path): input="\n".join(["random entropy", show_mnemonic, PASSWORD, PASSWORD]), ) assert result.exit_code == 0, result.output - assert "Newly generated mnemonic is" in result.output - mnemonic_length = len(result.output.split(":")[4].split("\n")[0].split()) + mnemonic = extract_mnemonic(result.output) + assert mnemonic is not None + mnemonic_length = len(mnemonic) assert mnemonic_length == 12 assert ETHEREUM_DEFAULT_PATH in result.output assert ALIAS in result.output @@ -266,8 +280,9 @@ def test_generate_24_words(ape_cli, runner, temp_keyfile_path): input="\n".join(["random entropy", show_mnemonic, PASSWORD, PASSWORD]), ) assert result.exit_code == 0, result.output - assert "Newly generated mnemonic is" in result.output - mnemonic_length = len(result.output.split(":")[4].split("\n")[0].split()) + mnemonic = extract_mnemonic(result.output) + assert mnemonic is not None + mnemonic_length = len(mnemonic) assert mnemonic_length == word_count assert ETHEREUM_DEFAULT_PATH in result.output assert ALIAS in result.output @@ -285,8 +300,9 @@ def test_generate_custom_hdpath(ape_cli, runner, temp_keyfile_path): input="\n".join(["random entropy", show_mnemonic, PASSWORD, PASSWORD]), ) assert result.exit_code == 0, result.output - assert "Newly generated mnemonic is" in result.output - mnemonic_length = len(result.output.split(":")[4].split("\n")[0].split()) + mnemonic = extract_mnemonic(result.output) + assert mnemonic is not None + mnemonic_length = len(mnemonic) assert mnemonic_length == 12 assert CUSTOM_HDPATH in result.output assert ALIAS in result.output @@ -305,8 +321,9 @@ def test_generate_24_words_and_custom_hdpath(ape_cli, runner, temp_keyfile_path) input="\n".join(["random entropy", show_mnemonic, PASSWORD, PASSWORD]), ) assert result.exit_code == 0, result.output - assert "Newly generated mnemonic is" in result.output - mnemonic_length = len(result.output.split(":")[4].split("\n")[0].split()) + mnemonic = extract_mnemonic(result.output) + assert mnemonic is not None + mnemonic_length = len(mnemonic) assert mnemonic_length == word_count assert CUSTOM_HDPATH in result.output assert ALIAS in result.output @@ -347,7 +364,7 @@ def test_list_all(ape_cli, runner, keyfile_account): def test_change_password(ape_cli, runner, temp_keyfile): assert temp_keyfile.is_file() # Delete Account (`N` for "Leave unlocked?") - valid_input = [PASSWORD, "N", "b", "b"] + valid_input = [PASSWORD, "N", "password2", "password2"] result = runner.invoke( ape_cli, ["accounts", "change-password", ALIAS],