Merge pull request #2967 from certbot/update_registration

Add a register verb, with a way to update registrations

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)

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

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

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 <email_address> 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 <email_address> 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):

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

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())

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)

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):