From 6505054f622f2621af4be58760a96cdc0d1f6bd9 Mon Sep 17 00:00:00 2001 From: Alex Zorin Date: Mon, 23 Jan 2023 18:36:34 +1100 Subject: [PATCH 1/4] account: stop storing legacy new_authzr_uri --- certbot/certbot/_internal/account.py | 29 +++------------------------- certbot/tests/account_test.py | 11 +---------- 2 files changed, 4 insertions(+), 36 deletions(-) diff --git a/certbot/certbot/_internal/account.py b/certbot/certbot/_internal/account.py index eb2466f32..8dcf1de09 100644 --- a/certbot/certbot/_internal/account.py +++ b/certbot/certbot/_internal/account.py @@ -126,18 +126,6 @@ class AccountMemoryStorage(interfaces.AccountStorage): raise errors.AccountNotFound(account_id) -class RegistrationResourceWithNewAuthzrURI(messages.RegistrationResource): - """A backwards-compatible RegistrationResource with a new-authz URI. - - Hack: Certbot versions pre-0.11.1 expect to load - new_authzr_uri as part of the account. Because people - sometimes switch between old and new versions, we will - continue to write out this field for some time so older - clients don't crash in that scenario. - """ - new_authzr_uri: str = jose.field('new_authzr_uri') - - class AccountFileStorage(interfaces.AccountStorage): """Accounts file storage. @@ -360,20 +348,9 @@ class AccountFileStorage(interfaces.AccountStorage): def _update_regr(self, account: Account, acme: ClientV2, dir_path: str) -> None: with open(self._regr_path(dir_path), "w") as regr_file: - regr = account.regr - # If we have a value for new-authz, save it for forwards - # compatibility with older versions of Certbot. If we don't - # have a value for new-authz, this is an ACMEv2 directory where - # an older version of Certbot won't work anyway. - if hasattr(acme.directory, "new-authz"): - regr = RegistrationResourceWithNewAuthzrURI( - new_authzr_uri=acme.directory.new_authz, - body={}, - uri=regr.uri) - else: - regr = messages.RegistrationResource( - body={}, - uri=regr.uri) + regr = messages.RegistrationResource( + body={}, + uri=account.regr.uri) regr_file.write(regr.json_dumps()) def _update_meta(self, account: Account, dir_path: str) -> None: diff --git a/certbot/tests/account_test.py b/certbot/tests/account_test.py index 0037de31e..e0972f247 100644 --- a/certbot/tests/account_test.py +++ b/certbot/tests/account_test.py @@ -116,8 +116,7 @@ class AccountFileStorageTest(test_util.ConfigTestCase): 2021, 1, 5, 14, 4, 10, tzinfo=pytz.UTC)) self.acc = Account( regr=messages.RegistrationResource( - uri=None, body=messages.Registration(), - new_authzr_uri=new_authzr_uri), + uri=None, body=messages.Registration()), key=KEY, meta=meta) self.mock_client = mock.MagicMock() @@ -141,14 +140,6 @@ class AccountFileStorageTest(test_util.ConfigTestCase): loaded = self.storage.load(self.acc.id) self.assertEqual(self.acc, loaded) - def test_save_and_restore_old_version(self): - """Saved regr should include a new_authzr_uri for older Certbots""" - self.storage.save(self.acc, self.mock_client) - path = os.path.join(self.config.accounts_dir, self.acc.id, "regr.json") - with open(path, "r") as f: - regr = json.load(f) - self.assertIn("new_authzr_uri", regr) - def test_update_regr(self): self.storage.update_regr(self.acc, self.mock_client) account_path = os.path.join(self.config.accounts_dir, self.acc.id) From 554143e1870104974ee12b670cf1cbf4a41e513b Mon Sep 17 00:00:00 2001 From: Alex Zorin Date: Mon, 23 Jan 2023 19:43:34 +1100 Subject: [PATCH 2/4] fix lint --- certbot/CHANGELOG.md | 2 +- certbot/certbot/_internal/account.py | 8 ++++---- certbot/certbot/_internal/main.py | 2 +- certbot/tests/account_test.py | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 387373fff..cc7e5e78f 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -17,7 +17,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Fixed -* +* Fixed a crash when registering an account with BuyPass' ACME server. More details about these changes can be found on our GitHub repo. diff --git a/certbot/certbot/_internal/account.py b/certbot/certbot/_internal/account.py index 8dcf1de09..e289245b1 100644 --- a/certbot/certbot/_internal/account.py +++ b/certbot/certbot/_internal/account.py @@ -242,11 +242,11 @@ class AccountFileStorage(interfaces.AccountStorage): dir_path = self._prepare(account) self._create(account, dir_path) self._update_meta(account, dir_path) - self._update_regr(account, client, dir_path) + self._update_regr(account, dir_path) except IOError as error: raise errors.AccountStorageError(error) - def update_regr(self, account: Account, client: ClientV2) -> None: + def update_regr(self, account: Account) -> None: """Update the registration resource. :param Account account: account to update @@ -255,7 +255,7 @@ class AccountFileStorage(interfaces.AccountStorage): """ try: dir_path = self._prepare(account) - self._update_regr(account, client, dir_path) + self._update_regr(account, dir_path) except IOError as error: raise errors.AccountStorageError(error) @@ -346,7 +346,7 @@ class AccountFileStorage(interfaces.AccountStorage): with util.safe_open(self._key_path(dir_path), "w", chmod=0o400) as key_file: key_file.write(account.key.json_dumps()) - def _update_regr(self, account: Account, acme: ClientV2, dir_path: str) -> None: + def _update_regr(self, account: Account, dir_path: str) -> None: with open(self._regr_path(dir_path), "w") as regr_file: regr = messages.RegistrationResource( body={}, diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 0712f1962..316503c5d 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -948,7 +948,7 @@ def update_account(config: configuration.NamespaceConfig, # the v2 uri. Since it's the same object on disk, put it back to the v1 uri # 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) + account_storage.update_regr(acc) if not config.email: display_util.notify("Any contact information associated " diff --git a/certbot/tests/account_test.py b/certbot/tests/account_test.py index e0972f247..f00ac363d 100644 --- a/certbot/tests/account_test.py +++ b/certbot/tests/account_test.py @@ -141,7 +141,7 @@ class AccountFileStorageTest(test_util.ConfigTestCase): self.assertEqual(self.acc, loaded) def test_update_regr(self): - self.storage.update_regr(self.acc, self.mock_client) + self.storage.update_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"))) From 2e3cace73947d6b4f89e61571a0aff5d6e83a764 Mon Sep 17 00:00:00 2001 From: Alex Zorin Date: Fri, 27 Jan 2023 10:38:00 +1100 Subject: [PATCH 3/4] remove docstring for removed argument --- certbot/certbot/_internal/account.py | 1 - 1 file changed, 1 deletion(-) diff --git a/certbot/certbot/_internal/account.py b/certbot/certbot/_internal/account.py index e289245b1..9c8146b63 100644 --- a/certbot/certbot/_internal/account.py +++ b/certbot/certbot/_internal/account.py @@ -250,7 +250,6 @@ class AccountFileStorage(interfaces.AccountStorage): """Update the registration resource. :param Account account: account to update - :param ClientV2 client: ACME client associated to the account """ try: From 08e008ac54381b07bfeefc8dd72b11577639e758 Mon Sep 17 00:00:00 2001 From: Alex Zorin Date: Fri, 27 Jan 2023 10:41:45 +1100 Subject: [PATCH 4/4] remove unused attributes from test --- certbot/tests/account_test.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/certbot/tests/account_test.py b/certbot/tests/account_test.py index f00ac363d..3daac6498 100644 --- a/certbot/tests/account_test.py +++ b/certbot/tests/account_test.py @@ -109,7 +109,6 @@ class AccountFileStorageTest(test_util.ConfigTestCase): self.storage = AccountFileStorage(self.config) from certbot._internal.account import Account - new_authzr_uri = "hi" meta = Account.Meta( creation_host="test.example.org", creation_dt=datetime.datetime( @@ -120,7 +119,6 @@ class AccountFileStorageTest(test_util.ConfigTestCase): key=KEY, meta=meta) self.mock_client = mock.MagicMock() - self.mock_client.directory.new_authz = new_authzr_uri def test_init_creates_dir(self): self.assertTrue(os.path.isdir(