Merge pull request #2402 from letsencrypt/revert-2393-webroot-map-and-flags

Revert "Allow webroot-map and --csr to exist together."

letsencrypt/cli.py

@@ -672,6 +672,11 @@ def run(config, plugins):  # pylint: disable=too-many-branches,too-many-locals
 def obtain_cert(config, plugins, lineage=None):
     """Implements "certonly": authenticate & obtain cert, but do not install it."""
 
+    if config.domains and config.csr is not None:
+        # TODO: --csr could have a priority, when --domains is
+        # supplied, check if CSR matches given domains?
+        return "--domains and --csr are mutually exclusive"
+
     try:
         # installers are used in auth mode to determine domain names
         installer, authenticator = choose_configurator_plugins(config, plugins, "certonly")
@@ -684,7 +689,8 @@ def obtain_cert(config, plugins, lineage=None):
     # This is a special case; cert and chain are simply saved
     if config.csr is not None:
         assert lineage is None, "Did not expect a CSR with a RenewableCert"
-        certr, chain = le_client.obtain_certificate_from_csr(_process_domain)
+        certr, chain = le_client.obtain_certificate_from_csr(le_util.CSR(
+            file=config.csr[0], data=config.csr[1], form="der"))
         if config.dry_run:
             logger.info(
                 "Dry run: skipping saving certificate to %s", config.cert_path)

letsencrypt/client.py

@@ -228,34 +228,21 @@ class Client(object):
             authzr)
         return certr, self.acme.fetch_chain(certr)
 
-    def obtain_certificate_from_csr(self, domain_callback):
+    def obtain_certificate_from_csr(self, csr):
         """Obtain certficiate from CSR.
 
-        :param function(config, domains) domain_callback: callback for each
+        :param .le_util.CSR csr: DER-encoded Certificate Signing
-        domain extracted from the CSR, to ensure that webroot-map and similar
+            Request.
-        housekeeping in cli.py is performed correctly
 
         :returns: `.CertificateResource` and certificate chain (as
             returned by `.fetch_chain`).
         :rtype: tuple
 
         """
-
+        return self._obtain_certificate(
-        #raise TypeError("About to call %r" % le_util.CSR)
+            # TODO: add CN to domains?
-        csr = le_util.CSR(file=self.config.csr[0], data=self.config.csr[1], form="der")
+            crypto_util.get_sans_from_csr(
-        # TODO: add CN to domains?
+                csr.data, OpenSSL.crypto.FILETYPE_ASN1), csr)
-        domains = crypto_util.get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1)
-        for d in domains:
-            domain_callback(self.config, d)
-
-        csr_domains, config_domains = set(domains), set(self.config.domains)
-        if csr_domains != config_domains:
-            raise errors.ConfigurationError(
-                "Inconsistent domain requests:\ncsr: {0}\ncli config: {1}"
-                .format(", ".join(csr_domains), ", ".join(config_domains))
-            )
-
-        return self._obtain_certificate(domains, csr)
 
     def obtain_certificate(self, domains):
         """Obtains a certificate from the ACME server.

letsencrypt/plugins/webroot.py

@@ -49,10 +49,8 @@ to serve all files under specified web root ({0})."""
         path_map = self.conf("map")
 
         if not path_map:
-            raise errors.PluginError(
+            raise errors.PluginError("--{0} must be set".format(
-                "Missing parts of webroot configuration; please set either "
+                self.option_name("path")))
-                "--webroot-path and --domains, or --webroot-map. Run with "
-                " --help webroot for examples.")
         for name, path in path_map.items():
             if not os.path.isdir(path):
                 raise errors.PluginError(path + " does not exist or is not a directory")

letsencrypt/tests/cli_test.py

@@ -228,7 +228,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
         args = ["certonly", "--webroot"]
         ret, _, _, _ = self._call(args)
-        self.assertTrue("please set either --webroot-path" in ret)
+        self.assertTrue("--webroot-path must be set" in ret)
 
         self._cli_missing_flag(["--standalone"], "With the standalone plugin, you probably")
 
@@ -323,6 +323,9 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertEqual(config.fullchain_path, os.path.abspath(fullchain))
 
     def test_certonly_bad_args(self):
+        ret, _, _, _ = self._call(['-d', 'foo.bar', 'certonly', '--csr', CSR])
+        self.assertEqual(ret, '--domains and --csr are mutually exclusive')
+
         ret, _, _, _ = self._call(['-a', 'bad_auth', 'certonly'])
         self.assertEqual(ret, 'The requested bad_auth plugin does not appear to be installed')
 

letsencrypt/tests/client_test.py

@@ -82,7 +82,6 @@ class ClientTest(unittest.TestCase):
             no_verify_ssl=False, config_dir="/etc/letsencrypt")
         # pylint: disable=star-args
         self.account = mock.MagicMock(**{"key.pem": KEY})
-        self.eg_domains = ["example.com", "www.example.com"]
 
         from letsencrypt.client import Client
         with mock.patch("letsencrypt.client.acme_client.Client") as acme:
@@ -102,7 +101,8 @@ class ClientTest(unittest.TestCase):
         self.acme.fetch_chain.return_value = mock.sentinel.chain
 
     def _check_obtain_certificate(self):
-        self.client.auth_handler.get_authorizations.assert_called_once_with(self.eg_domains)
+        self.client.auth_handler.get_authorizations.assert_called_once_with(
+            ["example.com", "www.example.com"])
         self.acme.request_issuance.assert_called_once_with(
             jose.ComparableX509(OpenSSL.crypto.load_certificate_request(
                 OpenSSL.crypto.FILETYPE_ASN1, CSR_SAN)),
@@ -111,21 +111,11 @@ class ClientTest(unittest.TestCase):
 
     def test_obtain_certificate_from_csr(self):
         self._mock_obtain_certificate()
-        mock_process_domain = mock.MagicMock()
+        self.assertEqual(
-        test_csr = le_util.CSR(form="der", file=None, data=CSR_SAN)
+            (mock.sentinel.certr, mock.sentinel.chain),
-        with mock.patch("letsencrypt.client.le_util.CSR") as mock_CSR:
+            self.client.obtain_certificate_from_csr(le_util.CSR(
-            mock_CSR.return_value = test_csr
+                form="der", file=None, data=CSR_SAN)))
-            self.client.config.domains = self.eg_domains
+        self._check_obtain_certificate()
-            self.assertEqual(
-                (mock.sentinel.certr, mock.sentinel.chain),
-                self.client.obtain_certificate_from_csr(mock_process_domain))
-
-            # make sure cli processing occurred
-            cli_processed = (call[0][1] for call in mock_process_domain.call_args_list)
-            self.assertEqual(set(cli_processed), set(("example.com", "www.example.com")))
-
-            # and that the cert was obtained correctly
-            self._check_obtain_certificate()
 
     @mock.patch("letsencrypt.client.crypto_util")
     def test_obtain_certificate(self, mock_crypto_util):