Fix gen_chall_path, add unittests

letsencrypt/client/auth_handler.py

@@ -315,24 +315,23 @@ class AuthHandler(object):  # pylint: disable=too-many-instance-attributes
 def gen_challenge_path(challs, preferences, combinations):
     """Generate a plan to get authority over the identity.
 
-    .. todo:: Make sure that the challenges are feasible...
-        Example: Do you have the recovery key?
+    .. todo:: This can be possibly be rewritten to use resolved_combinations.
 
-    :param list challs: A list of challenges
+    :param tuple challs: A tuple of challenges
         (:class:`letsencrypt.acme.challenges.Challenge`) from
         :class:`letsencrypt.acme.messages.Challenge` server message to
         be fulfilled by the client in order to prove possession of the
         identifier.
 
     :param list preferences: List of challenge preferences for domain
-        (:class:`letsencrypt.acme.challenges.Challege` subclasses)
+        (:class:`letsencrypt.acme.challenges.Challenge` subclasses)
 
-    :param list combinations: A collection of sets of challenges from
+    :param tuple combinations: A collection of sets of challenges from
         :class:`letsencrypt.acme.messages.Challenge`, each of which would
         be sufficient to prove possession of the identifier.
 
-    :returns: List of indices from ``challenges``.
-    :rtype: list
+    :returns: tuple of indices from ``challenges``.
+    :rtype: tuple
 
     """
     if combinations:
@@ -349,29 +348,34 @@ def _find_smart_path(challs, preferences, combinations):
 
     """
     chall_cost = {}
-    max_cost = 0
+    max_cost = 1
     for i, chall_cls in enumerate(preferences):
         chall_cost[chall_cls] = i
         max_cost += i
 
+    # max_cost is now equal to sum(indices) + 1
+
     best_combo = []
     # Set above completing all of the available challenges
-    best_combo_cost = max_cost + 1
+    best_combo_cost = max_cost
 
     combo_total = 0
     for combo in combinations:
         for challenge_index in combo:
             combo_total += chall_cost.get(challs[
                 challenge_index].__class__, max_cost)
+
         if combo_total < best_combo_cost:
             best_combo = combo
             best_combo_cost = combo_total
-            combo_total = 0
+
+        combo_total = 0
 
     if not best_combo:
-        logging.fatal("Client does not support any combination of "
-                      "challenges to satisfy ACME server")
-        sys.exit(22)
+        msg = ("Client does not support any combination of challenges that "
+               "will satisfy the CA.")
+        logging.fatal(msg)
+        raise errors.LetsEncryptAuthHandlerError(msg)
 
     return best_combo
 
@@ -387,13 +391,14 @@ def _find_dumb_path(challs, preferences):
     assert len(preferences) == len(set(preferences))
 
     path = []
-    satisfied = set()
+    # This cannot be a set() because POP challenge is not currently hashable
+    satisfied = []
     for pref_c in preferences:
         for i, offered_chall in enumerate(challs):
             if (isinstance(offered_chall, pref_c) and
                     is_preferred(offered_chall, satisfied)):
                 path.append(i)
-                satisfied.add(offered_chall)
+                satisfied.append(offered_chall)
     return path
 
 

letsencrypt/client/tests/acme_util.py

@@ -27,19 +27,19 @@ POP = challenges.ProofOfPossession(
     alg="RS256", nonce="xD\xf9\xb9\xdbU\xed\xaa\x17\xf1y|\x81\x88\x99 ",
     hints=challenges.ProofOfPossession.Hints(
         jwk=jose.JWKRSA(key=KEY.publickey()),
-        cert_fingerprints=[
+        cert_fingerprints=(
             "93416768eb85e33adc4277f4c9acd63e7418fcfe",
             "16d95b7b63f1972b980b14c20291f3c0d1855d95",
             "48b46570d9fc6358108af43ad1649484def0debf"
-        ],
-        certs=[], # TODO
-        subject_key_identifiers=["d0083162dcc4c8a23ecb8aecbd86120e56fd24e5"],
-        serial_numbers=[34234239832, 23993939911, 17],
-        issuers=[
+        ),
+        certs=(), # TODO
+        subject_key_identifiers=("d0083162dcc4c8a23ecb8aecbd86120e56fd24e5"),
+        serial_numbers=(34234239832, 23993939911, 17),
+        issuers=(
             "C=US, O=SuperT LLC, CN=SuperTrustworthy Public CA",
             "O=LessTrustworthy CA Inc, CN=LessTrustworthy But StillSecure",
-        ],
-        authorized_for=["www.example.com", "example.net"],
+        ),
+        authorized_for=("www.example.com", "example.net"),
     )
 )
 
@@ -61,6 +61,6 @@ def gen_combos(challs):
         else:
             renewal_chall.append(i)
 
-    # Gen combos for 1 of each type
-    return [[i, j] for i in xrange(len(dv_chall))
-            for j in xrange(len(renewal_chall))]
+    # Gen combos for 1 of each type, lowest index first (makes testing easier)
+    return tuple((i, j) if i < j else (j, i)
+                 for i in dv_chall for j in renewal_chall)

letsencrypt/client/tests/auth_handler_test.py

@@ -513,6 +513,78 @@ class PathSatisfiedTest(unittest.TestCase):
             self.assertFalse(self.handler._path_satisfied(dom[i]))
 
 
+class GenChallengePathTest(unittest.TestCase):
+    """Tests for letsencrypt.client.auth_handler.gen_challenge_path.
+
+    .. todo:: Add more tests for dumb_path... depending on what we want to do.
+
+    """
+    def setUp(self):
+        logging.disable(logging.fatal)
+
+    def tearDown(self):
+        logging.disable(logging.NOTSET)
+
+    @classmethod
+    def _call(cls, challs, preferences, combinations):
+        from letsencrypt.client.auth_handler import gen_challenge_path
+        return gen_challenge_path(challs, preferences, combinations)
+
+    def test_common_case(self):
+        """Given DVSNI and SimpleHTTPS with appropriate combos."""
+        challs = (acme_util.DVSNI, acme_util.SIMPLE_HTTPS)
+        prefs = [challenges.DVSNI]
+        combos = ((0,), (1,))
+
+        # Smart then trivial dumb path test
+        self.assertEqual(self._call(challs, prefs, combos), (0,))
+        self.assertTrue(self._call(challs, prefs, None))
+        # Rearrange order...
+        self.assertEqual(self._call(challs[::-1], prefs, combos), (1,))
+        self.assertTrue(self._call(challs[::-1], prefs, None))
+
+    def test_common_case_with_continuity(self):
+        challs = (acme_util.RECOVERY_TOKEN,
+                  acme_util.RECOVERY_CONTACT,
+                  acme_util.DVSNI,
+                  acme_util.SIMPLE_HTTPS)
+        prefs = [challenges.RecoveryToken, challenges.DVSNI]
+        combos = acme_util.gen_combos(challs)
+        self.assertEqual(self._call(challs, prefs, combos), (0, 2))
+
+         # dumb_path() trivial test
+        self.assertTrue(self._call(challs, prefs, None))
+
+    def test_full_client_server(self):
+        challs = (acme_util.RECOVERY_TOKEN,
+                  acme_util.RECOVERY_CONTACT,
+                  acme_util.POP,
+                  acme_util.DVSNI,
+                  acme_util.SIMPLE_HTTPS,
+                  acme_util.DNS)
+        # Typical webserver client that can do everything except DNS
+        # Attempted to make the order realistic
+        prefs = [challenges.RecoveryToken,
+                 challenges.ProofOfPossession,
+                 challenges.SimpleHTTPS,
+                 challenges.DVSNI,
+                 challenges.RecoveryContact]
+        combos = acme_util.gen_combos(challs)
+        self.assertEqual(self._call(challs, prefs, combos), (0, 4))
+
+        # Dumb path trivial test
+        self.assertTrue(self._call(challs, prefs, None))
+
+    def test_not_supported(self):
+        challs = (acme_util.POP, acme_util.DVSNI)
+        prefs = [challenges.DVSNI]
+        combos = ((0, 1),)
+
+        self.assertRaises(errors.LetsEncryptAuthHandlerError,
+                          self._call,
+                          challs, prefs, combos)
+
+
 class MutuallyExclusiveTest(unittest.TestCase):
     """Tests for letsencrypt.client.auth_handler.mutually_exclusive."""
 

tox.ini

@@ -19,7 +19,7 @@ setenv =
 basepython = python2.7
 commands =
     pip install -e .[testing]
-    python setup.py nosetests --with-coverage --cover-min-percentage=86
+    python setup.py nosetests --with-coverage --cover-min-percentage=87
 
 [testenv:lint]
 # recent versions of pylint do not support Python 2.6 (#97, #187)