Merge branch 'master' into webroot-map-and-flags

letsencrypt/auth_handler.py

@@ -57,7 +57,7 @@ class AuthHandler(object):
     def get_authorizations(self, domains, best_effort=False):
         """Retrieve all authorizations for challenges.
 
-        :param set domains: Domains for authorization
+        :param list domains: Domains for authorization
         :param bool best_effort: Whether or not all authorizations are
              required (this is useful in renewal)
 
@@ -480,6 +480,9 @@ def is_preferred(offered_challb, satisfied,
     return True
 
 
+_ACME_PREFIX = "urn:acme:error:"
+
+
 _ERROR_HELP_COMMON = (
     "To fix these errors, please make sure that your domain name was entered "
     "correctly and the DNS A record(s) for that domain contain(s) the "
@@ -490,7 +493,9 @@ _ERROR_HELP = {
     "connection":
         _ERROR_HELP_COMMON + " Additionally, please check that your computer "
         "has a publicly routable IP address and that no firewalls are preventing "
-        "the server from communicating with the client.",
+        "the server from communicating with the client. If you're using the "
+        "webroot plugin, you should also verify that you are serving files "
+        "from the webroot path you provided.",
     "dnssec":
         _ERROR_HELP_COMMON + " Additionally, if you have DNSSEC enabled for "
         "your domain, please ensure that the signature is valid.",
@@ -540,11 +545,13 @@ def _generate_failed_chall_msg(failed_achalls):
 
     """
     typ = failed_achalls[0].error.typ
+    if typ.startswith(_ACME_PREFIX):
+        typ = typ[len(_ACME_PREFIX):]
     msg = ["The following errors were reported by the server:"]
 
     for achall in failed_achalls:
         msg.append("\n\nDomain: %s\nType:   %s\nDetail: %s" % (
-            achall.domain, achall.error.typ, achall.error.detail))
+            achall.domain, typ, achall.error.detail))
 
     if typ in _ERROR_HELP:
         msg.append("\n\n")

letsencrypt/cli.py

@@ -850,10 +850,10 @@ def _reconstitute(config, full_path):
     try:
         domains = [le_util.enforce_domain_sanity(x) for x in
                    renewal_candidate.names()]
-    except (UnicodeError, ValueError):
+    except errors.ConfigurationError as error:
         logger.warning("Renewal configuration file %s references a cert "
-                       "that mentions a domain name that we regarded as "
-                       "invalid. Skipping.", full_path)
+                       "that contains an invalid domain name. The problem "
+                       "was: %s. Skipping.", full_path, error)
         return None
 
     setattr(config.namespace, "domains", domains)

letsencrypt/tests/auth_handler_test.py

@@ -437,9 +437,12 @@ class ReportFailedChallsTest(unittest.TestCase):
             "chall": acme_util.HTTP01,
             "uri": "uri",
             "status": messages.STATUS_INVALID,
-            "error": messages.Error(typ="tls", detail="detail"),
+            "error": messages.Error(typ="urn:acme:error:tls", detail="detail"),
         }
 
+        # Prevent future regressions if the error type changes
+        self.assertTrue(kwargs["error"].description is not None)
+
         self.http01 = achallenges.KeyAuthorizationAnnotatedChallenge(
             # pylint: disable=star-args
             challb=messages.ChallengeBody(**kwargs),

letsencrypt/tests/cli_test.py

@@ -599,19 +599,101 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             print "Logs:"
             print lf.read()
 
-
-    def test_renewal_verb(self):
+    def test_renew_verb(self):
         with open(test_util.vector_path('sample-renewal.conf')) as src:
             # put the correct path for cert.pem, chain.pem etc in the renewal conf
             renewal_conf = src.read().replace("MAGICDIR", test_util.vector_path())
         rd = os.path.join(self.config_dir, "renewal")
-        os.makedirs(rd)
+        if not os.path.exists(rd):
+            os.makedirs(rd)
         rc = os.path.join(rd, "sample-renewal.conf")
         with open(rc, "w") as dest:
             dest.write(renewal_conf)
         args = ["renew", "--dry-run", "-tvv"]
         self._test_renewal_common(True, [], args=args, renew=True)
 
+    def test_renew_verb_empty_config(self):
+        renewer_configs_dir = os.path.join(self.config_dir, 'renewal')
+        os.makedirs(renewer_configs_dir)
+        with open(os.path.join(renewer_configs_dir, 'empty.conf'), 'w'):
+            pass  # leave the file empty
+        self.test_renew_verb()
+
+    def _make_dummy_renewal_config(self):
+        renewer_configs_dir = os.path.join(self.config_dir, 'renewal')
+        os.makedirs(renewer_configs_dir)
+        with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f:
+            f.write("My contents don't matter")
+
+    def _test_renew_common(self, renewalparams=None,
+                           names=None, assert_oc_called=None):
+        self._make_dummy_renewal_config()
+        with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc:
+            mock_lineage = mock.MagicMock()
+            if renewalparams is not None:
+                mock_lineage.configuration = {'renewalparams': renewalparams}
+            if names is not None:
+                mock_lineage.names.return_value = names
+            mock_rc.return_value = mock_lineage
+            with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert:
+                self._test_renewal_common(True, None,
+                                          args=['renew'], renew=False)
+            if assert_oc_called is not None:
+                if assert_oc_called:
+                    self.assertTrue(mock_obtain_cert.called)
+                else:
+                    self.assertFalse(mock_obtain_cert.called)
+
+    def test_renew_no_renewalparams(self):
+        self._test_renew_common(assert_oc_called=False)
+
+    def test_renew_no_authenticator(self):
+        self._test_renew_common(renewalparams={}, assert_oc_called=False)
+
+    def test_renew_with_bad_int(self):
+        renewalparams = {'authenticator': 'webroot',
+                         'rsa_key_size': 'over 9000'}
+        self._test_renew_common(renewalparams=renewalparams,
+                                assert_oc_called=False)
+
+    def test_renew_with_bad_domain(self):
+        renewalparams = {'authenticator': 'webroot'}
+        names = ['*.example.com']
+        self._test_renew_common(renewalparams=renewalparams,
+                                names=names, assert_oc_called=False)
+
+    def test_renew_plugin_config_restoration(self):
+        renewalparams = {'authenticator': 'webroot',
+                         'webroot_path': 'None',
+                         'webroot_imaginary_flag': '42'}
+        self._test_renew_common(renewalparams=renewalparams,
+                                assert_oc_called=True)
+
+    def test_renew_reconstitute_error(self):
+        # pylint: disable=protected-access
+        with mock.patch('letsencrypt.cli._reconstitute') as mock_reconstitute:
+            mock_reconstitute.side_effect = Exception
+            self._test_renew_common(assert_oc_called=False)
+
+    def test_renew_obtain_cert_error(self):
+        self._make_dummy_renewal_config()
+        with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc:
+            mock_lineage = mock.MagicMock()
+            mock_rc.return_value = mock_lineage
+            mock_lineage.configuration = {
+                'renewalparams': {'authenticator': 'webroot'}}
+            with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert:
+                mock_obtain_cert.side_effect = Exception
+                self._test_renewal_common(True, None,
+                                          args=['renew'], renew=False)
+
+    def test_renew_with_bad_cli_args(self):
+        self.assertRaises(errors.Error, self._test_renewal_common, True, None,
+                          args='renew -d example.com'.split(), renew=False)
+        self.assertRaises(errors.Error, self._test_renewal_common, True, None,
+                          args='renew --csr {0}'.format(CSR).split(),
+                          renew=False)
+
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
     @mock.patch('letsencrypt.cli._treat_as_renewal')
     @mock.patch('letsencrypt.cli._init_le_client')