From 1de66b3d7d2f8b7681a28d4cd7ed60ad30a2c3f9 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 18 Feb 2016 16:02:07 -0800 Subject: [PATCH 1/2] Explicit error message for #2206 --- letsencrypt/cli.py | 5 +++++ letsencrypt/tests/cli_test.py | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 855c7a467..d245f096d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -649,6 +649,11 @@ def record_chosen_plugins(config, plugins, auth, inst): # Possible difficulties: config.csr was hacked into auth def run(config, plugins): # pylint: disable=too-many-branches,too-many-locals """Obtain a certificate and install.""" + if config.csr is not None: + raise errors.Error("Currently, the default 'run' verb cannot be used " + "when specifying a CSR file. Please try the " + "certonly command instead.") + try: installer, authenticator = choose_configurator_plugins(config, plugins, "run") except errors.PluginSelectionError as e: diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 77a4b5892..60fa3ebec 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -356,6 +356,15 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._call, ['-d', '204.11.231.35']) + def test_run_with_csr(self): + # This is an error because you can only use --csr with certonly + try: + self._call(['--csr', CSR]) + except errors.Error as e: + assert "Please try the certonly" in e.message + return + assert False, "Expected supplying --csr to fail with default verb" + def _get_argument_parser(self): plugins = disco.PluginsRegistry.find_all() return functools.partial(cli.prepare_and_parse_args, plugins) From 5eba011f8e57487b35cfab6fcc36a85233aee29a Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 18 Feb 2016 18:35:45 -0800 Subject: [PATCH 2/2] Generalize and move check inside handle_csr --- letsencrypt/cli.py | 15 ++++++--------- letsencrypt/tests/client_test.py | 3 +++ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index d245f096d..74084692d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -649,11 +649,6 @@ def record_chosen_plugins(config, plugins, auth, inst): # Possible difficulties: config.csr was hacked into auth def run(config, plugins): # pylint: disable=too-many-branches,too-many-locals """Obtain a certificate and install.""" - if config.csr is not None: - raise errors.Error("Currently, the default 'run' verb cannot be used " - "when specifying a CSR file. Please try the " - "certonly command instead.") - try: installer, authenticator = choose_configurator_plugins(config, plugins, "run") except errors.PluginSelectionError as e: @@ -988,10 +983,6 @@ def renew(config, unused_plugins): "renew specific certificates, use the certonly " "command. The renew verb may provide other options " "for selecting certificates to renew in the future.") - if config.csr is not None: - raise errors.Error("Currently, the renew verb cannot be used when " - "specifying a CSR file. Please try the certonly " - "command instead.") renewer_config = configuration.RenewerConfiguration(config) renew_successes = [] renew_failures = [] @@ -1249,6 +1240,12 @@ class HelpfulArgumentParser(object): Process a --csr flag. This needs to happen early enough that the webroot plugin can know about the calls to _process_domain """ + if parsed_args.verb != "certonly": + raise errors.Error("Currently, a CSR file may only be specified " + "when obtaining a new or replacement " + "via the certonly command. Please try the " + "certonly command instead.") + try: csr = le_util.CSR(file=parsed_args.csr[0], data=parsed_args.csr[1], form="der") typ = OpenSSL.crypto.FILETYPE_ASN1 diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index f712ea94c..daaea4f97 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -125,6 +125,9 @@ class ClientTest(unittest.TestCase): from letsencrypt import cli test_csr = le_util.CSR(form="der", file=None, data=CSR_SAN) mock_parsed_args = mock.MagicMock() + # The CLI should believe that this is a certonly request, because + # a CSR would not be allowed with other kinds of requests! + mock_parsed_args.verb = "certonly" with mock.patch("letsencrypt.client.le_util.CSR") as mock_CSR: mock_CSR.return_value = test_csr mock_parsed_args.domains = self.eg_domains[:]