Change query_registration() to use _get_v2_account() (#9307)

* Change `query_registration()` to use `_get_v2_account()`

* Improve `_get_v2_account()`

Required for proper working of `certbot.main.update_registration()`. This
function updates the `regr.body` locally instead of passing the fields
which need to be updated to `acme.client.update_registration()` as a
separate argument in the `update` parameter.

* Revert "Improve `_get_v2_account()`"

This reverts commit e88a23ad76.

* Improve `_get_v2_account() (version 2)

Instead of e88a23a, this change should be more compatible with older
ACMEv1 accounts used through symlinking ACMEv2 account dirs to the
existing ACMEv1 account dirs.
It should also still be compatible with `certbot.main.update_registration`.

* Move and slightly update CHANGELOG entry

acme/acme/client.py

@@ -646,12 +646,8 @@ class ClientV2(ClientBase):
             Resource.
 
         """
-        self.net.account = regr  # See certbot/certbot#6258
-        # ACME v2 requires to use a POST-as-GET request (POST an empty JWS) here.
-        # This is done by passing None instead of an empty UpdateRegistration to _post().
-        response = self._post(regr.uri, None)
-        self.net.account = self._regr_from_response(response, uri=regr.uri,
-                                                    terms_of_service=regr.terms_of_service)
+        self.net.account = self._get_v2_account(regr, True)
+
         return self.net.account
 
     def update_registration(self, regr: messages.RegistrationResource,
@@ -671,12 +667,15 @@ class ClientV2(ClientBase):
         new_regr = self._get_v2_account(regr)
         return super().update_registration(new_regr, update)
 
-    def _get_v2_account(self, regr: messages.RegistrationResource) -> messages.RegistrationResource:
+    def _get_v2_account(self, regr: messages.RegistrationResource, update_body: bool = False
+                       ) -> messages.RegistrationResource:
         self.net.account = None
         only_existing_reg = regr.body.update(only_return_existing=True)
         response = self._post(self.directory['newAccount'], only_existing_reg)
         updated_uri = response.headers['Location']
-        new_regr = regr.update(uri=updated_uri)
+        new_regr = regr.update(body=messages.Registration.from_json(response.json())
+                               if update_body else regr.body,
+                               uri=updated_uri)
         self.net.account = new_regr
         return new_regr
 

acme/tests/client_test.py

@@ -140,6 +140,7 @@ class BackwardsCompatibleClientV2Test(ClientTestBase):
         self.response.json.return_value = DIRECTORY_V2.to_json()
         client = self._init()
         self.response.json.return_value = self.regr.body.to_json()
+        self.response.headers = {'Location': 'https://www.letsencrypt-demo.org/acme/reg/1'}
         self.assertEqual(self.regr, client.query_registration(self.regr))
 
     def test_forwarding(self):

certbot/CHANGELOG.md

@@ -14,7 +14,9 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Fixed
 
-*
+* The `show_account` subcommand now uses the "newAccount" ACME endpoint to fetch the account
+  data, so it doesn't rely on the locally stored account URL. This fixes situations where Certbot
+  would use old ACMEv1 registration info with non-functional account URLs.
 
 More details about these changes can be found on our GitHub repo.