diff --git a/letsencrypt/client/auth_handler.py b/letsencrypt/client/auth_handler.py index 6f0ece535..984862f46 100644 --- a/letsencrypt/client/auth_handler.py +++ b/letsencrypt/client/auth_handler.py @@ -129,6 +129,7 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes .. todo:: It might be worth it to try different challenges to find one that doesn't throw an exception + .. todo:: separate into more functions """ logging.info("Performing the following challenges:") @@ -145,14 +146,19 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes # Order is important here as we will not expose the outside # Authenticator to our own indices. flat_client = [] - flat_auth = [] + flat_dv = [] + for dom in self.domains: flat_client.extend(ichall.chall for ichall in self.client_c[dom]) - flat_auth.extend(ichall.chall for ichall in self.dv_c[dom]) + flat_dv.extend(ichall.chall for ichall in self.dv_c[dom]) + client_resp = [] + dv_resp = [] try: - client_resp = self.client_auth.perform(flat_client) - dv_resp = self.dv_auth.perform(flat_auth) + if flat_client: + client_resp = self.client_auth.perform(flat_client) + if flat_dv: + dv_resp = self.dv_auth.perform(flat_dv) # This will catch both specific types of errors. except errors.LetsEncryptAuthHandlerError as err: logging.critical("Failure in setting up challenges:") @@ -167,8 +173,10 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes logging.info("Ready for verification...") # Assemble Responses - self._assign_responses(client_resp, self.client_c) - self._assign_responses(dv_resp, self.dv_c) + if client_resp: + self._assign_responses(client_resp, self.client_c) + if dv_resp: + self._assign_responses(dv_resp, self.dv_c) def _assign_responses(self, flat_list, ichall_dict): """Assign responses from flat_list back to the IndexedChall dicts. @@ -212,9 +220,13 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes # These are indexed challenges... give just the challenges to the auth # Chose to make these lists instead of a generator to make it easier to # work with... - self.dv_auth.cleanup([ichall.chall for ichall in self.dv_c[domain]]) - self.client_auth.cleanup( - [ichall.chall for ichall in self.client_c[domain]]) + dv_list = [ichall.chall for ichall in self.dv_c[domain]] + client_list = [ichall.chall for ichall in self.client_c[domain]] + if dv_list: + self.dv_auth.cleanup(dv_list) + if client_list: + self.client_auth.cleanup(client_list) + def _cleanup_state(self, delete_list): """Cleanup state after an authorization is received. diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py index 9fcd95c6a..04c7d35e7 100644 --- a/letsencrypt/client/interfaces.py +++ b/letsencrypt/client/interfaces.py @@ -30,6 +30,9 @@ class IAuthenticator(zope.interface.Interface): :param list chall_list: List of namedtuple types defined in :mod:`letsencrypt.client.challenge_util` (``DvsniChall``, etc.). + - chall_list will never be empty + - chall_list will only contain types found within + :func:`get_chall_pref` :returns: ACME Challenge responses or if it cannot be completed then: @@ -43,20 +46,16 @@ class IAuthenticator(zope.interface.Interface): """ def cleanup(chall_list): - """Revert changes and shutdown after challenges complete.""" + """Revert changes and shutdown after challenges complete. + :param list chall_list: List of namedtuple types defined in + :mod:`letsencrypt.client.challenge_util` (``DvsniChall``, etc.) -class IChallenge(zope.interface.Interface): - """Let's Encrypt challenge.""" + - Only challenges given previously in the perform function will be + found in chall_list. + - chall_list will never be empty - def perform(): - """Perform the challenge.""" - - def generate_response(): - """Generate response.""" - - def cleanup(): - """Cleanup.""" + """ class IConfig(zope.interface.Interface): diff --git a/letsencrypt/client/tests/acme_util.py b/letsencrypt/client/tests/acme_util.py index aa142af8e..08a7e44bd 100644 --- a/letsencrypt/client/tests/acme_util.py +++ b/letsencrypt/client/tests/acme_util.py @@ -26,7 +26,7 @@ CHALLENGES = { "successURL": "https://example.ca/confirmrecovery/bb1b9928932", "contact": "c********n@example.com" }, - "recoveryTokent": + "recoveryToken": { "type": "recoveryToken" }, diff --git a/letsencrypt/client/tests/auth_handler_test.py b/letsencrypt/client/tests/auth_handler_test.py index 137b1627e..945141f4e 100644 --- a/letsencrypt/client/tests/auth_handler_test.py +++ b/letsencrypt/client/tests/auth_handler_test.py @@ -25,8 +25,8 @@ class SatisfyChallengesTest(unittest.TestCase): def setUp(self): from letsencrypt.client.auth_handler import AuthHandler - self.mock_dv_auth = mock.MagicMock(name='ApacheConfigurator') - self.mock_client_auth = mock.MagicMock(name='ClientAuthenticator') + self.mock_dv_auth = mock.MagicMock(name="ApacheConfigurator") + self.mock_client_auth = mock.MagicMock(name="ClientAuthenticator") self.mock_dv_auth.get_chall_pref.return_value = ["dvsni"] self.mock_client_auth.get_chall_pref.return_value = ["recoveryToken"] @@ -59,6 +59,29 @@ class SatisfyChallengesTest(unittest.TestCase): self.assertEqual(len(self.handler.dv_c[dom]), 1) self.assertEqual(len(self.handler.client_c[dom]), 0) + def test_name1_rectok1(self): + dom = "0" + challenge = [acme_util.CHALLENGES["recoveryToken"]] + msg = acme_util.get_chall_msg(dom, "nonce0", challenge) + self.handler.add_chall_msg(dom, msg, "dummy_key") + + self.handler._satisfy_challenges() # pylint: disable=protected-access + + self.assertEqual(len(self.handler.responses), 1) + self.assertEqual(len(self.handler.responses[dom]), 1) + + # Test if statement for dv_auth perform + self.assertEqual(self.mock_client_auth.perform.call_count, 1) + self.assertEqual(self.mock_dv_auth.perform.call_count, 0) + + self.assertEqual("RecTokenChall0", self.handler.responses[dom][0]) + # Assert 1 domain + self.assertEqual(len(self.handler.dv_c), 1) + self.assertEqual(len(self.handler.client_c), 1) + # Assert 1 auth challenge, 0 dv + self.assertEqual(len(self.handler.dv_c[dom]), 0) + self.assertEqual(len(self.handler.client_c[dom]), 1) + def test_name5_dvsni5(self): challenge = [acme_util.CHALLENGES["dvsni"]] for i in xrange(5): @@ -74,6 +97,10 @@ class SatisfyChallengesTest(unittest.TestCase): self.assertEqual(len(self.handler.client_c), 5) # Each message contains 1 auth, 0 client + # Test proper call count for methods + self.assertEqual(self.mock_client_auth.perform.call_count, 0) + self.assertEqual(self.mock_dv_auth.perform.call_count, 1) + for i in xrange(5): dom = str(i) self.assertEqual(len(self.handler.responses[dom]), 1) @@ -103,6 +130,10 @@ class SatisfyChallengesTest(unittest.TestCase): self.assertEqual(len(self.handler.dv_c), 1) self.assertEqual(len(self.handler.client_c), 1) + # Test if statement for client_auth perform + self.assertEqual(self.mock_client_auth.perform.call_count, 0) + self.assertEqual(self.mock_dv_auth.perform.call_count, 1) + self.assertEqual( self.handler.responses[dom], self._get_exp_response(dom, path, challenges)) @@ -251,33 +282,38 @@ class SatisfyChallengesTest(unittest.TestCase): str(i), "nonce%d" % i, challenges, combos), "dummy_key") - mock_chall_path.return_value = gen_path( - ["dvsni", "proofOfPossession"], challenges) + mock_chall_path.side_effect = [ + gen_path(["dvsni", "proofOfPossession"], challenges), + gen_path(["proofOfPossession"], challenges), + gen_path(["dvsni"], challenges), + ] # This may change in the future... but for now catch the error self.assertRaises(errors.LetsEncryptAuthHandlerError, self.handler._satisfy_challenges) # Verify cleanup is actually run correctly - self.assertEqual(self.mock_dv_auth.cleanup.call_count, 3) - self.assertEqual(self.mock_client_auth.cleanup.call_count, 3) + self.assertEqual(self.mock_dv_auth.cleanup.call_count, 2) + self.assertEqual(self.mock_client_auth.cleanup.call_count, 2) + + + dv_cleanup_args = self.mock_dv_auth.cleanup.call_args_list + client_cleanup_args = self.mock_client_auth.cleanup.call_args_list # Check DV cleanup - mock_cleanup_args = self.mock_dv_auth.cleanup.call_args_list - for i in xrange(3): - # Assert length of arg list was 1 - arg_chall_list = mock_cleanup_args[i][0][0] - self.assertEqual(len(arg_chall_list), 1) - self.assertTrue(isinstance(arg_chall_list[0], - challenge_util.DvsniChall)) + for i in xrange(2): + dv_chall_list = dv_cleanup_args[i][0][0] + self.assertEqual(len(dv_chall_list), 1) + self.assertTrue( + isinstance(dv_chall_list[0], challenge_util.DvsniChall)) + # Check Auth cleanup - mock_cleanup_args = self.mock_client_auth.cleanup.call_args_list - for i in xrange(3): - arg_chall_list = mock_cleanup_args[i][0][0] - self.assertEqual(len(arg_chall_list), 1) - self.assertTrue(isinstance(arg_chall_list[0], - challenge_util.PopChall)) + for i in xrange(2): + client_chall_list = client_cleanup_args[i][0][0] + self.assertEqual(len(client_chall_list), 1) + self.assertTrue( + isinstance(client_chall_list[0], challenge_util.PopChall)) def _get_exp_response(self, domain, path, challenges): # pylint: disable=no-self-use @@ -293,8 +329,8 @@ class GetAuthorizationsTest(unittest.TestCase): def setUp(self): from letsencrypt.client.auth_handler import AuthHandler - self.mock_dv_auth = mock.MagicMock(name='ApacheConfigurator') - self.mock_client_auth = mock.MagicMock(name='ClientAuthenticator') + self.mock_dv_auth = mock.MagicMock(name="ApacheConfigurator") + self.mock_client_auth = mock.MagicMock(name="ClientAuthenticator") self.mock_sat_chall = mock.MagicMock(name="_satisfy_challenges") self.mock_acme_auth = mock.MagicMock(name="acme_authorization") @@ -484,5 +520,5 @@ def gen_path(str_list, challenges): return path -if __name__ == '__main__': +if __name__ == "__main__": unittest.main()