From 526bc5cf84aeaca0669d87ada3ced32615efe8e9 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 6 Apr 2016 17:35:29 -0700
Subject: [PATCH] Refactor CSR importing from cli -> crypto_util

More specifically:
  HelpfulArgumentParser.handle_csr -> crypto_util.import_csr_file
---
 letsencrypt/cli.py               | 18 ++----------------
 letsencrypt/crypto_util.py       | 30 ++++++++++++++++++++++++++++++
 letsencrypt/tests/client_test.py |  6 ++++--
 3 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 74c51f5e3..61fc57777 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -6,10 +6,8 @@ import logging
 import logging.handlers
 import os
 import sys
-import traceback
 
 import configargparse
-import OpenSSL
 import six
 
 import letsencrypt
@@ -361,20 +359,8 @@ class HelpfulArgumentParser(object):
         if parsed_args.allow_subset_of_names:
             raise errors.Error("--allow-subset-of-names cannot be used with --csr")
 
-        try:
-            csr = le_util.CSR(file=parsed_args.csr[0], data=parsed_args.csr[1], form="der")
-            typ = OpenSSL.crypto.FILETYPE_ASN1
-            domains = crypto_util.get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1)
-        except OpenSSL.crypto.Error:
-            try:
-                e1 = traceback.format_exc()
-                typ = OpenSSL.crypto.FILETYPE_PEM
-                csr = le_util.CSR(file=parsed_args.csr[0], data=parsed_args.csr[1], form="pem")
-                domains = crypto_util.get_sans_from_csr(csr.data, typ)
-            except OpenSSL.crypto.Error:
-                logger.debug("DER CSR parse error %s", e1)
-                logger.debug("PEM CSR parse error %s", traceback.format_exc())
-                raise errors.Error("Failed to parse CSR file: {0}".format(parsed_args.csr[0]))
+        csrfile, contents = parsed_args.csr[0:2]
+        typ, csr, domains = crypto_util.import_csr_file(csrfile, contents)
 
         # This is not necessary for webroot to work, however,
         # obtain_certificate_from_csr requires parsed_args.domains to be set
diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py
index 5fdcba843..2ca43f76f 100644
--- a/letsencrypt/crypto_util.py
+++ b/letsencrypt/crypto_util.py
@@ -6,6 +6,7 @@
 """
 import logging
 import os
+import traceback
 
 import OpenSSL
 import pyrfc3339
@@ -171,6 +172,35 @@ def csr_matches_pubkey(csr, privkey):
         return False
 
 
+def import_csr_file(csrfile, contents):
+    """Import a CSR file, which can be either PEM or DER.
+
+    :param str csrfile: CSR filename
+    :param str contents: contens of the CSR file
+
+    :rtype: tuple
+
+    :returns: (le_util.CSR object representing the CSR,
+               OpenSSL FILETYPE_ representing DER or PEM,
+               list of domains requested in the CSR)
+    """
+    try:
+        csr = le_util.CSR(file=csrfile, data=contents, form="der")
+        typ = OpenSSL.crypto.FILETYPE_ASN1
+        domains = get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1)
+    except OpenSSL.crypto.Error:
+        try:
+            e1 = traceback.format_exc()
+            typ = OpenSSL.crypto.FILETYPE_PEM
+            csr = le_util.CSR(file=csrfile, data=contents, form="pem")
+            domains = get_sans_from_csr(csr.data, typ)
+        except OpenSSL.crypto.Error:
+            logger.debug("DER CSR parse error %s", e1)
+            logger.debug("PEM CSR parse error %s", traceback.format_exc())
+            raise errors.Error("Failed to parse CSR file: {0}".format(csrfile))
+    return typ, csr, domains
+
+
 def make_key(bits):
     """Generate PEM encoded RSA key.
 
diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py
index cd6b11158..c3d5b322f 100644
--- a/letsencrypt/tests/client_test.py
+++ b/letsencrypt/tests/client_test.py
@@ -134,7 +134,7 @@ class ClientTest(unittest.TestCase):
 
         self.acme.fetch_chain.assert_called_once_with(mock.sentinel.certr)
 
-    # FIXME move parts of this to test_cli.py...
+    # FIXME move parts of this to crypto_util tests...
     @mock.patch("letsencrypt.client.logger")
     def test_obtain_certificate_from_csr(self, mock_logger):
         self._mock_obtain_certificate()
@@ -144,9 +144,11 @@ class ClientTest(unittest.TestCase):
         # 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:
+        with mock.patch("letsencrypt.cli.crypto_util.le_util.CSR") as mock_CSR:
             mock_CSR.return_value = test_csr
             mock_parsed_args.domains = self.eg_domains[:]
+            mock_parsed_args.allow_subset_of_names = False
+            mock_parsed_args.csr = (mock.MagicMock(), mock.MagicMock())
             mock_parser = mock.MagicMock(cli.HelpfulArgumentParser)
             cli.HelpfulArgumentParser.handle_csr(mock_parser, mock_parsed_args)