diff --git a/letsencrypt/client/apache/configurator.py b/letsencrypt/client/apache/configurator.py index 229718e63..488730dbf 100644 --- a/letsencrypt/client/apache/configurator.py +++ b/letsencrypt/client/apache/configurator.py @@ -801,7 +801,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): def is_site_enabled(self, avail_fp): """Checks to see if the given site is enabled. - .. todo:: fix hardcoded sites-enabled + .. todo:: fix hardcoded sites-enabled, check os.path.samefile :param str avail_fp: Complete file path of available site diff --git a/letsencrypt/client/apache/dvsni.py b/letsencrypt/client/apache/dvsni.py index 4f5549ffc..a9d08da27 100644 --- a/letsencrypt/client/apache/dvsni.py +++ b/letsencrypt/client/apache/dvsni.py @@ -21,6 +21,16 @@ class ApacheDvsni(object): :type dvsni_chall: `list` of :class:`letsencrypt.client.challenge_util.DvsniChall` + :param list indicies: Meant to hold indices of challenges in a + larger array. ApacheDvsni is capable of solving many challenges + at once which causes an indexing issue within ApacheConfigurator + who must return all responses in order. Imagine ApacheConfigurator + maintaining state about where all of the SimpleHttps Challenges, + Dvsni Challenges belong in the response array. This is an optional + utility. + + :param str challenge_conf: location of the challenge config file + """ def __init__(self, config): self.config = config @@ -46,7 +56,7 @@ class ApacheDvsni(object): def perform(self): """Peform a DVSNI challenge.""" if not self.dvsni_chall: - return dict() + return None # Save any changes to the configuration as a precaution # About to make temporary changes to the config self.config.save() @@ -85,14 +95,14 @@ class ApacheDvsni(object): responses.append({"type": "dvsni", "s": s_b64}) # Setup the configuration - self.mod_config(addresses) + self._mod_config(addresses) # Save reversible changes self.config.save("SNI Challenge", True) return responses - def mod_config(self, ll_addrs): + def _mod_config(self, ll_addrs): """Modifies Apache config files to include challenge vhosts. Result: Apache config includes virtual servers for issued challs @@ -116,18 +126,18 @@ class ApacheDvsni(object): # TODO: Use ip address of existing vhost instead of relying on FQDN config_text = "\n" for idx, lis in enumerate(ll_addrs): - config_text += self.get_config_text( + config_text += self._get_config_text( self.dvsni_chall[idx].nonce, lis, self.dvsni_chall[idx].key.file) config_text += "\n" - self.conf_include_check(self.config.parser.loc["default"]) + self._conf_include_check(self.config.parser.loc["default"]) self.config.register_file_creation(True, self.challenge_conf) with open(self.challenge_conf, 'w') as new_conf: new_conf.write(config_text) - def conf_include_check(self, main_config): + def _conf_include_check(self, main_config): """Adds DVSNI challenge conf file into configuration. Adds DVSNI challenge include file if it does not already exist @@ -142,7 +152,7 @@ class ApacheDvsni(object): self.config.parser.add_dir(parser.get_aug_path(main_config), "Include", self.challenge_conf) - def get_config_text(self, nonce, ip_addrs, dvsni_key_file): + def _get_config_text(self, nonce, ip_addrs, dvsni_key_file): """Chocolate virtual server configuration text :param str nonce: hex form of nonce diff --git a/letsencrypt/client/tests/apache/__init__.py b/letsencrypt/client/tests/apache/__init__.py new file mode 100644 index 000000000..2c0849a3d --- /dev/null +++ b/letsencrypt/client/tests/apache/__init__.py @@ -0,0 +1 @@ +"""Let's Encrypt Apache Tests""" diff --git a/letsencrypt/client/tests/config_util.py b/letsencrypt/client/tests/apache/config_util.py similarity index 98% rename from letsencrypt/client/tests/config_util.py rename to letsencrypt/client/tests/apache/config_util.py index 691a394f4..ad38818ab 100644 --- a/letsencrypt/client/tests/config_util.py +++ b/letsencrypt/client/tests/apache/config_util.py @@ -17,7 +17,7 @@ def dir_setup(test_dir="debian_apache_2_4/two_vhost_80"): work_dir = tempfile.mkdtemp("work") test_configs = pkg_resources.resource_filename( - __name__, "testdata/%s" % test_dir) + "letsencrypt.client.tests", "testdata/%s" % test_dir) shutil.copytree( test_configs, os.path.join(temp_dir, test_dir), symlinks=True) diff --git a/letsencrypt/client/tests/apache_configurator_test.py b/letsencrypt/client/tests/apache/configurator_test.py similarity index 96% rename from letsencrypt/client/tests/apache_configurator_test.py rename to letsencrypt/client/tests/apache/configurator_test.py index 5426409b5..00560c970 100644 --- a/letsencrypt/client/tests/apache_configurator_test.py +++ b/letsencrypt/client/tests/apache/configurator_test.py @@ -10,21 +10,20 @@ import zope.component from letsencrypt.client import challenge_util from letsencrypt.client import client -from letsencrypt.client import display from letsencrypt.client import errors from letsencrypt.client.apache import configurator from letsencrypt.client.apache import obj from letsencrypt.client.apache import parser -from letsencrypt.client.tests import config_util +from letsencrypt.client.tests.apache import config_util class TwoVhost80Test(unittest.TestCase): """Test two standard well configured HTTP vhosts.""" def setUp(self): - zope.component.provideUtility(display.NcursesDisplay()) + #zope.component.provideUtility(display.NcursesDisplay()) self.temp_dir, self.config_dir, self.work_dir = config_util.dir_setup( "debian_apache_2_4/two_vhost_80") @@ -170,9 +169,9 @@ class TwoVhost80Test(unittest.TestCase): # Only tests functionality specific to configurator.perform # Note: As more challenges are offered this will have to be expanded rsa256_file = pkg_resources.resource_filename( - __name__, 'testdata/rsa256_key.pem') + "letsencrypt.client.tests", 'testdata/rsa256_key.pem') rsa256_pem = pkg_resources.resource_string( - __name__, 'testdata/rsa256_key.pem') + "letsencrypt.client.tests", 'testdata/rsa256_key.pem') auth_key = client.Client.Key(rsa256_file, rsa256_pem) chall1 = challenge_util.DvsniChall( diff --git a/letsencrypt/client/tests/apache_dvsni_test.py b/letsencrypt/client/tests/apache/dvsni_test.py similarity index 55% rename from letsencrypt/client/tests/apache_dvsni_test.py rename to letsencrypt/client/tests/apache/dvsni_test.py index b50997541..6beb07b37 100644 --- a/letsencrypt/client/tests/apache_dvsni_test.py +++ b/letsencrypt/client/tests/apache/dvsni_test.py @@ -10,18 +10,15 @@ import zope.component from letsencrypt.client import challenge_util from letsencrypt.client import client from letsencrypt.client import CONFIG -from letsencrypt.client import display -from letsencrypt.client.apache import obj - -from letsencrypt.client.tests import config_util +from letsencrypt.client.tests.apache import config_util class DvsniPerformTest(unittest.TestCase): def setUp(self): from letsencrypt.client.apache import dvsni - zope.component.provideUtility(display.NcursesDisplay()) + #zope.component.provideUtility(display.NcursesDisplay()) self.temp_dir, self.config_dir, self.work_dir = config_util.dir_setup( "debian_apache_2_4/two_vhost_80") @@ -38,44 +35,46 @@ class DvsniPerformTest(unittest.TestCase): self.sni = dvsni.ApacheDvsni(config) rsa256_file = pkg_resources.resource_filename( - __name__, 'testdata/rsa256_key.pem') + "letsencrypt.client.tests", 'testdata/rsa256_key.pem') rsa256_pem = pkg_resources.resource_string( - __name__, 'testdata/rsa256_key.pem') + "letsencrypt.client.tests", 'testdata/rsa256_key.pem') auth_key = client.Client.Key(rsa256_file, rsa256_pem) - self.chall1 = challenge_util.DvsniChall( + self.challs = [] + self.challs.append(challenge_util.DvsniChall( "encryption-example.demo", "jIq_Xy1mXGN37tb4L6Xj_es58fW571ZNyXekdZzhh7Q", "37bc5eb75d3e00a19b4f6355845e5a18", - auth_key) - self.chall2 = challenge_util.DvsniChall( + auth_key)) + self.challs.append(challenge_util.DvsniChall( "letsencrypt.demo", "uqnaPzxtrndteOqtrXb0Asl5gOJfWAnnx6QJyvcmlDU", "59ed014cac95f77057b1d7a1b2c596ba", - auth_key) - - self.sni.add_chall(self.chall1) - self.sni.add_chall(self.chall2) + auth_key)) def tearDown(self): shutil.rmtree(self.temp_dir) shutil.rmtree(self.config_dir) shutil.rmtree(self.work_dir) + def test_perform0(self): + resp = self.sni.perform() + self.assertIs(resp, None) + @mock.patch("letsencrypt.client.apache.configurator." "ApacheConfigurator.restart") @mock.patch("letsencrypt.client.challenge_util.dvsni_gen_cert") - def test_perform(self, mock_dvsni_gen_cert, mock_restart): - mock_dvsni_gen_cert.side_effect = ["randomS1", "randomS2"] + def test_perform1(self, mock_dvsni_gen_cert, mock_restart): + chall = self.challs[0] + self.sni.add_chall(chall) + mock_dvsni_gen_cert.return_value = "randomS1" responses = self.sni.perform() - self.assertEqual(mock_dvsni_gen_cert.call_count, 2) + self.assertEqual(mock_dvsni_gen_cert.call_count, 1) calls = mock_dvsni_gen_cert.call_args_list expected_call_list = [ - (self.sni.get_cert_file(self.chall1.nonce), self.chall1.domain, - self.chall1.r_b64, self.chall1.nonce, self.chall1.key), - (self.sni.get_cert_file(self.chall2.nonce), self.chall2.domain, - self.chall2.r_b64, self.chall2.nonce, self.chall2.key) + (self.sni.get_cert_file(chall.nonce), chall.domain, + chall.r_b64, chall.nonce, chall.key) ] for i in range(len(expected_call_list)): @@ -86,17 +85,50 @@ class DvsniPerformTest(unittest.TestCase): len(self.sni.config.parser.find_dir( "Include", self.sni.challenge_conf)), 1) - self.assertEqual(len(responses), 2) + self.assertEqual(len(responses), 1) self.assertEqual(responses[0]["s"], "randomS1") - self.assertEqual(responses[1]["s"], "randomS2") + + @mock.patch("letsencrypt.client.apache.configurator." + "ApacheConfigurator.restart") + @mock.patch("letsencrypt.client.challenge_util.dvsni_gen_cert") + def test_perform2(self, mock_dvsni_gen_cert, mock_restart): + for chall in self.challs: + self.sni.add_chall(chall) + + mock_dvsni_gen_cert.side_effect = ["randomS0", "randomS1"] + responses = self.sni.perform() + + self.assertEqual(mock_dvsni_gen_cert.call_count, 2) + calls = mock_dvsni_gen_cert.call_args_list + expected_call_list = [] + + for chall in self.challs: + expected_call_list.append( + (self.sni.get_cert_file(chall.nonce), chall.domain, + chall.r_b64, chall.nonce, chall.key)) + + for i in range(len(expected_call_list)): + for j in range(len(expected_call_list[0])): + self.assertEqual(calls[i][0][j], expected_call_list[i][j]) + + self.assertEqual( + len(self.sni.config.parser.find_dir( + "Include", self.sni.challenge_conf)), + 1) + self.assertEqual(len(responses), 2) + for i in range(2): + self.assertEqual(responses[i]["s"], "randomS%d" % i) def test_mod_config(self): - v_addr1 = [obj.Addr(("1.2.3.4", "443")), obj.Addr(("5.6.7.8", "443"))] - v_addr2 = [obj.Addr(("127.0.0.1", "443"))] + from letsencrypt.client.apache.obj import Addr + for chall in self.challs: + self.sni.add_chall(chall) + v_addr1 = [Addr(("1.2.3.4", "443")), Addr(("5.6.7.8", "443"))] + v_addr2 = [Addr(("127.0.0.1", "443"))] ll_addr = [] ll_addr.append(v_addr1) ll_addr.append(v_addr2) - self.sni.mod_config(ll_addr) + self.sni._mod_config(ll_addr) # pylint: disable=protected-access self.sni.config.save() self.sni.config.parser.find_dir("Include", self.sni.challenge_conf) @@ -112,9 +144,9 @@ class DvsniPerformTest(unittest.TestCase): if vhost.addrs == set(v_addr1): self.assertEqual( vhost.names, - set([str(self.chall1.nonce + CONFIG.INVALID_EXT)])) + set([str(self.challs[0].nonce + CONFIG.INVALID_EXT)])) else: self.assertEqual(vhost.addrs, set(v_addr2)) self.assertEqual( vhost.names, - set([str(self.chall2.nonce + CONFIG.INVALID_EXT)])) + set([str(self.challs[1].nonce + CONFIG.INVALID_EXT)])) diff --git a/letsencrypt/client/tests/apache_obj_test.py b/letsencrypt/client/tests/apache/obj_test.py similarity index 67% rename from letsencrypt/client/tests/apache_obj_test.py rename to letsencrypt/client/tests/apache/obj_test.py index 46e76d4bd..151880ac4 100644 --- a/letsencrypt/client/tests/apache_obj_test.py +++ b/letsencrypt/client/tests/apache/obj_test.py @@ -1,14 +1,13 @@ +"""Test the helper objects in apache.obj.py.""" import unittest -from letsencrypt.client.apache import obj - - class AddrTest(unittest.TestCase): """Test the Addr class.""" def setUp(self): - self.addr1 = obj.Addr.fromstring("192.168.1.1") - self.addr2 = obj.Addr.fromstring("192.168.1.1:*") - self.addr3 = obj.Addr.fromstring("192.168.1.1:80") + from letsencrypt.client.apache.obj import Addr + self.addr1 = Addr.fromstring("192.168.1.1") + self.addr2 = Addr.fromstring("192.168.1.1:*") + self.addr3 = Addr.fromstring("192.168.1.1:80") def test_fromstring(self): self.assertEqual(self.addr1.get_addr(), "192.168.1.1") @@ -36,9 +35,10 @@ class AddrTest(unittest.TestCase): self.assertNotEqual(self.addr1, 3333) def test_set_inclusion(self): + from letsencrypt.client.apache.obj import Addr set_a = set([self.addr1, self.addr2]) - addr1b = obj.Addr.fromstring("192.168.1.1") - addr2b = obj.Addr.fromstring("192.168.1.1:*") + addr1b = Addr.fromstring("192.168.1.1") + addr2b = Addr.fromstring("192.168.1.1:*") set_b = set([addr1b, addr2b]) self.assertEqual(set_a, set_b) @@ -47,14 +47,18 @@ class AddrTest(unittest.TestCase): class VirtualHostTest(unittest.TestCase): """Test the VirtualHost class.""" def setUp(self): - self.vhost1 = obj.VirtualHost( + from letsencrypt.client.apache.obj import VirtualHost + from letsencrypt.client.apache.obj import Addr + self.vhost1 = VirtualHost( "filep", "vh_path", - set([obj.Addr.fromstring("localhost")]), False, False) + set([Addr.fromstring("localhost")]), False, False) def test_eq(self): - vhost1b = obj.VirtualHost( + from letsencrypt.client.apache.obj import Addr + from letsencrypt.client.apache.obj import VirtualHost + vhost1b = VirtualHost( "filep", "vh_path", - set([obj.Addr.fromstring("localhost")]), False, False) + set([Addr.fromstring("localhost")]), False, False) self.assertEqual(vhost1b, self.vhost1) self.assertEqual(str(vhost1b), str(self.vhost1)) diff --git a/letsencrypt/client/tests/apache_parser_test.py b/letsencrypt/client/tests/apache/parser_test.py similarity index 98% rename from letsencrypt/client/tests/apache_parser_test.py rename to letsencrypt/client/tests/apache/parser_test.py index 340cdd324..b7f1f6aa2 100644 --- a/letsencrypt/client/tests/apache_parser_test.py +++ b/letsencrypt/client/tests/apache/parser_test.py @@ -10,7 +10,7 @@ import zope.component from letsencrypt.client import display from letsencrypt.client import errors from letsencrypt.client.apache import parser -from letsencrypt.client.tests import config_util +from letsencrypt.client.tests.apache import config_util class ApacheParserTest(unittest.TestCase): diff --git a/letsencrypt/client/tests/recovery_token_test.py b/letsencrypt/client/tests/recovery_token_test.py index 240f60cf7..945c2b0b9 100644 --- a/letsencrypt/client/tests/recovery_token_test.py +++ b/letsencrypt/client/tests/recovery_token_test.py @@ -57,7 +57,7 @@ class RecoveryTokenTest(unittest.TestCase): self.assertEqual(response, {"type": "recoveryToken", "token": "555"}) response = self.rec_token.perform(RecTokenChall("example6.com")) - self.assertEqual(response, None) + self.assertIs(response, None) if __name__ == '__main__': diff --git a/tox.ini b/tox.ini index dc6b05e31..f636b5567 100644 --- a/tox.ini +++ b/tox.ini @@ -14,7 +14,7 @@ commands = [testenv:cover] commands = python setup.py dev - python setup.py nosetests --with-coverage --cover-min-percentage=60 + python setup.py nosetests --with-coverage --cover-min-percentage=59 [testenv:lint] commands =