diff --git a/AUTHORS.md b/AUTHORS.md index 0cedcbd19..56fe2a3d8 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -61,6 +61,7 @@ Authors * [Daniel Albers](https://github.com/AID) * [Daniel Aleksandersen](https://github.com/da2x) * [Daniel Convissor](https://github.com/convissor) +* [Daniel "Drex" Drexler](https://github.com/aeturnum) * [Daniel Huang](https://github.com/dhuang) * [Dave Guarino](https://github.com/daguar) * [David cz](https://github.com/dave-cz) diff --git a/acme/acme/messages.py b/acme/acme/messages.py index 030e62d6c..8c9bf9701 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -315,6 +315,9 @@ class Registration(ResourceBody): # on new-reg key server ignores 'key' and populates it based on # JWS.signature.combined.jwk key = jose.Field('key', omitempty=True, decoder=jose.JWK.from_json) + # Contact field implements special behavior to allow messages that clear existing + # contacts while not expecting the `contact` field when loading from json. + # This is implemented in the constructor and *_json methods. contact = jose.Field('contact', omitempty=True, default=()) agreement = jose.Field('agreement', omitempty=True) status = jose.Field('status', omitempty=True) @@ -327,24 +330,73 @@ class Registration(ResourceBody): @classmethod def from_data(cls, phone=None, email=None, external_account_binding=None, **kwargs): - """Create registration resource from contact details.""" + """ + Create registration resource from contact details. + + The `contact` keyword being passed to a Registration object is meaningful, so + this function represents empty iterables in its kwargs by passing on an empty + `tuple`. + """ + + # Note if `contact` was in kwargs. + contact_provided = 'contact' in kwargs + + # Pop `contact` from kwargs and add formatted email or phone numbers details = list(kwargs.pop('contact', ())) if phone is not None: details.append(cls.phone_prefix + phone) if email is not None: details.extend([cls.email_prefix + mail for mail in email.split(',')]) - kwargs['contact'] = tuple(details) + + # Insert formatted contact information back into kwargs + # or insert an empty tuple if `contact` provided. + if details or contact_provided: + kwargs['contact'] = tuple(details) if external_account_binding: kwargs['external_account_binding'] = external_account_binding return cls(**kwargs) + def __init__(self, **kwargs): + """Note if the user provides a value for the `contact` member.""" + if 'contact' in kwargs: + # Avoid the __setattr__ used by jose.TypedJSONObjectWithFields + object.__setattr__(self, '_add_contact', True) + super(Registration, self).__init__(**kwargs) + def _filter_contact(self, prefix): return tuple( detail[len(prefix):] for detail in self.contact # pylint: disable=not-an-iterable if detail.startswith(prefix)) + def _add_contact_if_appropriate(self, jobj): + """ + The `contact` member of Registration objects should not be required when + de-serializing (as it would be if the Fields' `omitempty` flag were `False`), but + it should be included in serializations if it was provided. + + :param jobj: Dictionary containing this Registrations' data + :type jobj: dict + + :returns: Dictionary containing Registrations data to transmit to the server + :rtype: dict + """ + if getattr(self, '_add_contact', False): + jobj['contact'] = self.encode('contact') + + return jobj + + def to_partial_json(self): + """Modify josepy.JSONDeserializable.to_partial_json()""" + jobj = super(Registration, self).to_partial_json() + return self._add_contact_if_appropriate(jobj) + + def fields_to_partial_json(self): + """Modify josepy.JSONObjectWithFields.fields_to_partial_json()""" + jobj = super(Registration, self).fields_to_partial_json() + return self._add_contact_if_appropriate(jobj) + @property def phones(self): """All phones found in the ``contact`` field.""" diff --git a/acme/tests/messages_test.py b/acme/tests/messages_test.py index 890a5f413..3458105b2 100644 --- a/acme/tests/messages_test.py +++ b/acme/tests/messages_test.py @@ -254,6 +254,19 @@ class RegistrationTest(unittest.TestCase): from acme.messages import Registration hash(Registration.from_json(self.jobj_from)) + def test_default_not_transmitted(self): + from acme.messages import NewRegistration + empty_new_reg = NewRegistration() + new_reg_with_contact = NewRegistration(contact=()) + + self.assertEqual(empty_new_reg.contact, ()) + self.assertEqual(new_reg_with_contact.contact, ()) + + self.assertTrue('contact' not in empty_new_reg.to_partial_json()) + self.assertTrue('contact' not in empty_new_reg.fields_to_partial_json()) + self.assertTrue('contact' in new_reg_with_contact.to_partial_json()) + self.assertTrue('contact' in new_reg_with_contact.fields_to_partial_json()) + class UpdateRegistrationTest(unittest.TestCase): """Tests for acme.messages.UpdateRegistration.""" diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 1ec9f1699..d01d59069 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -27,6 +27,8 @@ More details about these changes can be found on our GitHub repo. this concerns the plugin name, CLI flags, and keys in credential files. The prefixed form is still supported but is deprecated, and will be removed in a future release. * Added `--nginx-sleep-seconds` (default `1`) for environments where nginx takes a long time to reload. +* Added the ability to remove email and phone contact information from an account + using `update_account --register-unsafely-without-email` ### Changed @@ -37,7 +39,8 @@ More details about these changes can be found on our GitHub repo. ### Fixed -* +* The `acme` library can now tell the ACME server to clear contact information by passing an empty + `tuple` to the `contact` field of a `Registration` message. More details about these changes can be found on our GitHub repo. diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 30f4dd0a2..d60e6edba 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -11,7 +11,7 @@ import josepy as jose import zope.component from acme import errors as acme_errors -from acme.magic_typing import Union +from acme.magic_typing import Union, Iterable, Optional # pylint: disable=unused-import import certbot from certbot import crypto_util from certbot import errors @@ -590,7 +590,7 @@ def _init_le_client(config, authenticator, installer): :type config: interfaces.IConfig :param authenticator: Acme authentication handler - :type authenticator: interfaces.IAuthenticator + :type authenticator: Optional[interfaces.IAuthenticator] :param installer: Installer object :type installer: interfaces.IInstaller @@ -703,17 +703,17 @@ def update_account(config, unused_plugins): if not accounts: 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.") + if config.email is None and not config.register_unsafely_without_email: config.email = display_ops.get_email(optional=False) acc, acme = _determine_account(config) cb_client = client.Client(config, acc, None, None, acme=acme) + # Empty list of contacts in case the user is removing all emails + + acc_contacts = () # type: Iterable[str] + if config.email: + acc_contacts = ['mailto:' + email for email in config.email.split(',')] # We rely on an exception to interrupt this process if it didn't work. - acc_contacts = ['mailto:' + email for email in config.email.split(',')] prev_regr_uri = acc.regr.uri acc.regr = cb_client.acme.update_registration(acc.regr.update( body=acc.regr.body.update(contact=acc_contacts))) @@ -722,8 +722,13 @@ def update_account(config, unused_plugins): # so that we can also continue to use the account object with acmev1. acc.regr = acc.regr.update(uri=prev_regr_uri) account_storage.update_regr(acc, cb_client.acme) - eff.prepare_subscription(config, acc) - add_msg("Your e-mail address was updated to {0}.".format(config.email)) + + if config.email is None: + add_msg("Any contact information associated with this account has been removed.") + else: + eff.prepare_subscription(config, acc) + add_msg("Your e-mail address was updated to {0}.".format(config.email)) + return None diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index d2d7ccf79..52d8c24d0 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -1404,6 +1404,43 @@ class MainTest(test_util.ConfigTestCase): "user@example.org"]) self.assertTrue("Could not find an existing account" in x[0]) + @mock.patch('certbot._internal.main._determine_account') + @mock.patch('certbot._internal.eff.prepare_subscription') + @mock.patch('certbot._internal.main.account') + def test_update_account_remove_email(self, mocked_account_module, mock_prepare, mock_det_acc): + # Mock account storage and the account object returned + mocked_storage = mock.MagicMock() + mocked_account = mock.MagicMock() + + mocked_account_module.AccountFileStorage.return_value = mocked_storage + mocked_storage.find_all.return_value = [mocked_account] + mock_det_acc.return_value = (mocked_account, "foo") + + # Mock registration body to verify calls are made + mock_regr_body = mock.MagicMock() + + # mocked_account.regr is overwritten in update, requiring an odd mock setup + mocked_account.regr.body = mock_regr_body + + x = self._call( + ["update_account", "--register-unsafely-without-email"]) + + + # When update succeeds, the return value of update_account() is None + self.assertTrue(x[0] is None) + # and we got supposedly did update the registration from + # the server + client_mock = x[3] + self.assertTrue(client_mock.Client().acme.update_registration.called) + + self.assertTrue(mock_regr_body.update.called) + self.assertTrue('contact' in mock_regr_body.update.call_args[1]) + self.assertEqual(mock_regr_body.update.call_args[1]['contact'], ()) + # and we saved the updated registration on disk + self.assertTrue(mocked_storage.update_regr.called) + # ensure we didn't try to subscribe (no email to subscribe with) + self.assertFalse(mock_prepare.called) + @mock.patch('certbot._internal.main.display_ops.get_email') @test_util.patch_get_utility() def test_update_account_with_email(self, mock_utility, mock_email):