diff --git a/certbot/account.py b/certbot/account.py index 2ef3629e2..798f8664e 100644 --- a/certbot/account.py +++ b/certbot/account.py @@ -186,16 +186,29 @@ class AccountFileStorage(interfaces.AccountStorage): return acc def save(self, account): + self._save(account, regr_only=False) + + def save_regr(self, account): + """Save the registration resource. + + :param Account account: account whose regr should be saved + + """ + self._save(account, regr_only=True) + + def _save(self, account, regr_only): account_dir_path = self._account_dir_path(account.id) util.make_or_verify_dir(account_dir_path, 0o700, os.geteuid(), - self.config.strict_permissions) + self.config.strict_permissions) try: with open(self._regr_path(account_dir_path), "w") as regr_file: regr_file.write(account.regr.json_dumps()) - with util.safe_open(self._key_path(account_dir_path), - "w", chmod=0o400) as key_file: - key_file.write(account.key.json_dumps()) - with open(self._metadata_path(account_dir_path), "w") as metadata_file: - metadata_file.write(account.meta.json_dumps()) + if not regr_only: + with util.safe_open(self._key_path(account_dir_path), + "w", chmod=0o400) as key_file: + key_file.write(account.key.json_dumps()) + with open(self._metadata_path( + account_dir_path), "w") as metadata_file: + metadata_file.write(account.meta.json_dumps()) except IOError as error: raise errors.AccountStorageError(error) diff --git a/certbot/cli.py b/certbot/cli.py index bcb7785c5..643602a44 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -61,6 +61,7 @@ cert. Major SUBCOMMANDS are: install Install a previously obtained cert in a server renew Renew previously obtained certs that are near expiry revoke Revoke a previously obtained certificate + register Perform tasks related to registering with the CA rollback Rollback server configuration changes made during install config_changes Show changes made to server config during installation plugins Display information about installed plugins @@ -86,7 +87,8 @@ More detailed help: the available topics are: all, automation, paths, security, testing, or any of the subcommands or - plugins (certonly, install, nginx, apache, standalone, webroot, etc) + plugins (certonly, install, register, nginx, apache, standalone, webroot, + etc.) """ @@ -285,8 +287,9 @@ class HelpfulArgumentParser(object): self.VERBS = {"auth": main.obtain_cert, "certonly": main.obtain_cert, "config_changes": main.config_changes, "run": main.run, "install": main.install, "plugins": main.plugins_cmd, - "renew": main.renew, "revoke": main.revoke, - "rollback": main.rollback, "everything": main.run} + "register": main.register, "renew": main.renew, + "revoke": main.revoke, "rollback": main.rollback, + "everything": main.run} # List of topics for which additional help can be provided HELP_TOPICS = ["all", "security", "paths", "automation", "testing"] + list(self.VERBS) @@ -570,7 +573,7 @@ class HelpfulArgumentParser(object): return dict([(t, t == chosen_topic) for t in self.help_topics]) -def prepare_and_parse_args(plugins, args, detect_defaults=False): +def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: disable=too-many-statements """Returns parsed command line arguments. :param .PluginsRegistry plugins: available plugins @@ -625,6 +628,11 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): "certificates. Updates to the Subscriber Agreement will still " "affect you, and will be effective 14 days after posting an " "update to the web site.") + helpful.add( + "register", "--update-registration", action="store_true", + help="With the register verb, indicates that details associated " + "with an existing registration, such as the e-mail address, " + "should be updated, rather than registering a new account.") helpful.add(None, "-m", "--email", help=config_help("email")) # positional arg shadows --domains, instead of appending, and # --domains is useful, because it can be stored in config diff --git a/certbot/client.py b/certbot/client.py index 8c178408c..1dec6a1a9 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -150,7 +150,7 @@ def perform_registration(acme, config): return acme.register(messages.NewRegistration.from_data(email=config.email)) except messages.Error as e: if e.typ == "urn:acme:error:invalidEmail": - config.namespace.email = display_ops.get_email(more=True, invalid=True) + config.namespace.email = display_ops.get_email(invalid=True) return perform_registration(acme, config) else: raise diff --git a/certbot/display/ops.py b/certbot/display/ops.py index a8f2283fc..c7e566256 100644 --- a/certbot/display/ops.py +++ b/certbot/display/ops.py @@ -15,41 +15,56 @@ logger = logging.getLogger(__name__) z_util = zope.component.getUtility -def get_email(more=False, invalid=False): +def get_email(invalid=False, optional=True): """Prompt for valid email address. - :param bool more: explain why the email is strongly advisable, but how to - skip it - :param bool invalid: true if the user just typed something, but it wasn't - a valid-looking email + :param bool invalid: True if an invalid address was provided by the user + :param bool optional: True if the user can use + --register-unsafely-without-email to avoid providing an e-mail - :returns: Email or ``None`` if cancelled by user. + :returns: e-mail address :rtype: str - """ - msg = "Enter email address (used for urgent notices and lost key recovery)" - if invalid: - msg = "There seem to be problems with that address. " + msg - if more: - msg += ('\n\nIf you really want to skip this, you can run the client with ' - '--register-unsafely-without-email but make sure you backup your ' - 'account key from /etc/letsencrypt/accounts\n\n') - try: - code, email = zope.component.getUtility(interfaces.IDisplay).input(msg) - except errors.MissingCommandlineFlag: - msg = ("You should register before running non-interactively, or provide --agree-tos" - " and --email flags") - raise errors.MissingCommandlineFlag(msg) + :raises errors.Error: if the user cancels - if code == display_util.OK: - if util.safe_email(email): - return email + """ + invalid_prefix = "There seem to be problems with that address. " + msg = "Enter email address (used for urgent notices and lost key recovery)" + unsafe_suggestion = ("\n\nIf you really want to skip this, you can run " + "the client with --register-unsafely-without-email " + "but make sure you then backup your account key from " + "/etc/letsencrypt/accounts\n\n") + if optional: + if invalid: + msg += unsafe_suggestion else: - # TODO catch the server's ACME invalid email address error, and - # make a similar call when that happens - return get_email(more=True, invalid=(email != "")) + suggest_unsafe = True else: - return None + suggest_unsafe = False + + while True: + try: + code, email = z_util(interfaces.IDisplay).input( + invalid_prefix + msg if invalid else msg) + except errors.MissingCommandlineFlag: + msg = ("You should register before running non-interactively, " + "or provide --agree-tos and --email flags") + raise errors.MissingCommandlineFlag(msg) + + if code != display_util.OK: + if optional: + raise errors.Error( + "An e-mail address or " + "--register-unsafely-without-email must be provided.") + else: + raise errors.Error("An e-mail address must be provided.") + elif util.safe_email(email): + return email + elif suggest_unsafe: + msg += unsafe_suggestion + suggest_unsafe = False # add this message at most once + + invalid = bool(email) def choose_account(accounts): diff --git a/certbot/main.py b/certbot/main.py index 96c0221a2..fa14bbf99 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -366,6 +366,48 @@ def _init_le_client(config, authenticator, installer): return client.Client(config, acc, authenticator, installer, acme=acme) +def register(config, unused_plugins): + """Create or modify accounts on the server.""" + + # Portion of _determine_account logic to see whether accounts already + # exist or not. + account_storage = account.AccountFileStorage(config) + accounts = account_storage.find_all() + + # registering a new account + if not config.update_registration: + if len(accounts) > 0: + # TODO: add a flag to register a duplicate account (this will + # also require extending _determine_account's behavior + # or else extracting the registration code from there) + return ("There is an existing account; registration of a " + "duplicate account with this command is currently " + "unsupported.") + # _determine_account will register an account + _determine_account(config) + return + + # --update-registration + if len(accounts) == 0: + return "Could not find an existing account to update." + if config.email is None: + if config.register_unsafely_without_email: + return ("--register-unsafely-without-email provided, however, a " + "new e-mail address must\ncurrently be provided when " + "updating a registration.") + config.namespace.email = display_ops.get_email(optional=False) + + acc, acme = _determine_account(config) + acme_client = client.Client(config, acc, None, None, acme=acme) + # We rely on an exception to interrupt this process if it didn't work. + acc.regr = acme_client.acme.update_registration(acc.regr.update( + body=acc.regr.body.update(contact=('mailto:' + config.email,)))) + account_storage.save_regr(acc) + reporter_util = zope.component.getUtility(interfaces.IReporter) + msg = "Your e-mail address was updated to {0}.".format(config.email) + reporter_util.add_message(msg, reporter_util.MEDIUM_PRIORITY) + + def install(config, plugins): """Install a previously obtained cert in a server.""" # XXX: Update for renewer/RenewableCert diff --git a/certbot/tests/account_test.py b/certbot/tests/account_test.py index a96e57507..4cd2bfebf 100644 --- a/certbot/tests/account_test.py +++ b/certbot/tests/account_test.py @@ -137,6 +137,16 @@ class AccountFileStorageTest(unittest.TestCase): # restore self.assertEqual(self.acc, self.storage.load(self.acc.id)) + def test_save_regr(self): + self.storage.save_regr(self.acc) + account_path = os.path.join(self.config.accounts_dir, self.acc.id) + self.assertTrue(os.path.exists(account_path)) + self.assertTrue(os.path.exists(os.path.join( + account_path, "regr.json"))) + for file_name in "meta.json", "private_key.json": + self.assertFalse(os.path.exists( + os.path.join(account_path, file_name))) + def test_find_all(self): self.storage.save(self.acc) self.assertEqual([self.acc], self.storage.find_all()) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 671da16f0..b40b98988 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -915,6 +915,74 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._call(['-c', test_util.vector_path('cli.ini')]) self.assertTrue(mocked_run.called) + def test_register(self): + with mock.patch('certbot.main.client') as mocked_client: + acc = mock.MagicMock() + acc.id = "imaginary_account" + mocked_client.register.return_value = (acc, "worked") + self._call_no_clientmock(["register", "--email", "user@example.org"]) + # TODO: It would be more correct to explicitly check that + # _determine_account() gets called in the above case, + # but coverage statistics should also show that it did. + with mock.patch('certbot.main.account') as mocked_account: + mocked_storage = mock.MagicMock() + mocked_account.AccountFileStorage.return_value = mocked_storage + mocked_storage.find_all.return_value = ["an account"] + x = self._call_no_clientmock(["register", "--email", "user@example.org"]) + self.assertTrue("There is an existing account" in x[0]) + + def test_update_registration_no_existing_accounts(self): + # with mock.patch('certbot.main.client') as mocked_client: + with mock.patch('certbot.main.account') as mocked_account: + mocked_storage = mock.MagicMock() + mocked_account.AccountFileStorage.return_value = mocked_storage + mocked_storage.find_all.return_value = [] + x = self._call_no_clientmock( + ["register", "--update-registration", "--email", + "user@example.org"]) + self.assertTrue("Could not find an existing account" in x[0]) + + def test_update_registration_unsafely(self): + # This test will become obsolete when register --update-registration + # supports removing an e-mail address from the account + with mock.patch('certbot.main.account') as mocked_account: + mocked_storage = mock.MagicMock() + mocked_account.AccountFileStorage.return_value = mocked_storage + mocked_storage.find_all.return_value = ["an account"] + x = self._call_no_clientmock( + "register --update-registration " + "--register-unsafely-without-email".split()) + self.assertTrue("--register-unsafely-without-email" in x[0]) + + @mock.patch('certbot.main.display_ops.get_email') + @mock.patch('certbot.main.zope.component.getUtility') + def test_update_registration_with_email(self, mock_utility, mock_email): + email = "user@example.com" + mock_email.return_value = email + with mock.patch('certbot.main.client') as mocked_client: + with mock.patch('certbot.main.account') as mocked_account: + with mock.patch('certbot.main._determine_account') as mocked_det: + with mock.patch('certbot.main.client') as mocked_client: + mocked_storage = mock.MagicMock() + mocked_account.AccountFileStorage.return_value = mocked_storage + mocked_storage.find_all.return_value = ["an account"] + mocked_det.return_value = (mock.MagicMock(), "foo") + acme_client = mock.MagicMock() + mocked_client.Client.return_value = acme_client + x = self._call_no_clientmock( + ["register", "--update-registration"]) + # When registration change succeeds, the return value + # of register() is None + self.assertTrue(x[0] is None) + # and we got supposedly did update the registration from + # the server + self.assertTrue( + acme_client.acme.update_registration.called) + # and we saved the updated registration on disk + self.assertTrue(mocked_storage.save_regr.called) + self.assertTrue( + email in mock_utility().add_message.call_args[0][0]) + def test_conflicting_args(self): args = ['renew', '--dialog', '--text'] self.assertRaises(errors.Error, self._call, args) diff --git a/certbot/tests/display/ops_test.py b/certbot/tests/display/ops_test.py index 874a9cc9e..3aff37d86 100644 --- a/certbot/tests/display/ops_test.py +++ b/certbot/tests/display/ops_test.py @@ -12,6 +12,7 @@ from acme import jose from acme import messages from certbot import account +from certbot import errors from certbot import interfaces from certbot.display import util as display_util @@ -37,7 +38,8 @@ class GetEmailTest(unittest.TestCase): def test_cancel_none(self): self.input.return_value = (display_util.CANCEL, "foo@bar.baz") - self.assertTrue(self._call() is None) + self.assertRaises(errors.Error, self._call) + self.assertRaises(errors.Error, self._call, optional=False) def test_ok_safe(self): self.input.return_value = (display_util.OK, "foo@bar.baz") @@ -51,27 +53,24 @@ class GetEmailTest(unittest.TestCase): mock_safe_email.side_effect = [False, True] self.assertTrue(self._call() is "foo@bar.baz") - def test_more_and_invalid_flags(self): - more_txt = "--register-unsafely-without-email" + def test_invalid_flag(self): invalid_txt = "There seem to be problems" - base_txt = "Enter email" self.input.return_value = (display_util.OK, "foo@bar.baz") with mock.patch("certbot.display.ops.util.safe_email") as mock_safe_email: mock_safe_email.return_value = True self._call() - msg = self.input.call_args[0][0] - self.assertTrue(more_txt not in msg) - self.assertTrue(invalid_txt not in msg) - self.assertTrue(base_txt in msg) - self._call(more=True) - msg = self.input.call_args[0][0] - self.assertTrue(more_txt in msg) - self.assertTrue(invalid_txt not in msg) - self._call(more=True, invalid=True) - msg = self.input.call_args[0][0] - self.assertTrue(more_txt in msg) - self.assertTrue(invalid_txt in msg) - self.assertTrue(base_txt in msg) + self.assertTrue(invalid_txt not in self.input.call_args[0][0]) + self._call(invalid=True) + self.assertTrue(invalid_txt in self.input.call_args[0][0]) + + def test_optional_flag(self): + self.input.return_value = (display_util.OK, "foo@bar.baz") + with mock.patch("certbot.display.ops.util.safe_email") as mock_safe_email: + mock_safe_email.side_effect = [False, True] + self._call(optional=False) + for call in self.input.call_args_list: + self.assertTrue( + "--register-unsafely-without-email" not in call[0][0]) class ChooseAccountTest(unittest.TestCase):