Merge remote-tracking branch 'alexzorin/7979_restore_v1_as_v2' into upgrade-acmev1

certbot/certbot/_internal/constants.py

@@ -120,6 +120,8 @@ CLI_DEFAULTS = dict(
 )
 STAGING_URI = "https://acme-staging-v02.api.letsencrypt.org/directory"
 
+V1_URI = "https://acme-v01.api.letsencrypt.org/directory"
+
 # The set of reasons for revoking a certificate is defined in RFC 5280 in
 # section 5.3.1. The reasons that users are allowed to submit are restricted to
 # those accepted by the ACME server implementation. They are listed in

certbot/certbot/_internal/renewal.py

@@ -19,6 +19,7 @@ from certbot import errors
 from certbot import interfaces
 from certbot import util
 from certbot._internal import cli
+from certbot._internal import constants
 from certbot._internal import hooks
 from certbot._internal import storage
 from certbot._internal import updater
@@ -243,16 +244,28 @@ def _restore_int(name, value):
         raise errors.Error("Expected a numeric value for {0}".format(name))
 
 
-def _restore_str(unused_name, value):
+def _restore_str(name, value):
     """Restores a string key-value pair from a renewal config file.
 
-    :param str unused_name: option name
+    :param str name: option name
     :param str value: option value
 
     :returns: converted option value to be stored in the runtime config
     :rtype: str or None
 
     """
+    # Previous to v0.5.0, Certbot always stored the `server` URL in the renewal config,
+    # resulting in configs which explicitly use the deprecated ACMEv1 URL, today
+    # preventing an automatic transition to the default modern ACME URL.
+    # (https://github.com/certbot/certbot/issues/7978#issuecomment-625442870)
+    # As a mitigation, this function reinterprets the value of the `server` parameter if
+    # necessary, replacing the ACMEv1 URL with the default ACME URL. It is still possible
+    # to override this choice with the explicit `--server` CLI flag.
+    if name == "server" and value == constants.V1_URI:
+        logger.info("Using server %s instead of legacy %s",
+                    constants.CLI_DEFAULTS["server"], value)
+        return constants.CLI_DEFAULTS["server"]
+
     return None if value == "None" else value
 
 

certbot/tests/renewal_test.py

@@ -110,6 +110,14 @@ class RestoreRequiredConfigElementsTest(test_util.ConfigTestCase):
         self.assertRaises(
             errors.Error, self._call, self.config, renewalparams)
 
+    @mock.patch('certbot._internal.renewal.cli.set_by_cli')
+    def test_ancient_server_renewal_conf(self, mock_set_by_cli):
+        from certbot._internal import constants
+        self.config.server = None
+        mock_set_by_cli.return_value = False
+        self._call(self.config, {'server': constants.V1_URI})
+        self.assertEqual(self.config.server, constants.CLI_DEFAULTS['server'])
+
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover