Present dialog only once per domain, added tests

This commit is contained in:
Joona Hoikkala 2018-02-23 17:31:00 +02:00
parent 20af164bf1
commit 1b55f94e73
No known key found for this signature in database
GPG key ID: 1708DAE66E87A524
6 changed files with 145 additions and 15 deletions

View file

@ -153,9 +153,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
self.assoc = dict()
# Outstanding challenges
self._chall_out = set()
# List of vhosts configured for wildcard certificates on this run.
# List of vhosts configured per wildcard domain on this run.
# used by deploy_cert() and enhance()
self.wildcard_vhosts = list()
self.wildcard_vhosts = dict()
# Maps enhancements to vhosts we've enabled the enhancement for
self._enhanced_vhosts = defaultdict(set)
@ -306,7 +306,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
for vhost in deploy_vhosts:
self._deploy_cert(vhost, cert_path, key_path,
chain_path, fullchain_path)
self.wildcard_vhosts.append(vhost)
if domain not in self.wildcard_vhosts.keys():
self.wildcard_vhosts[domain] = list()
self.wildcard_vhosts[domain].append(vhost)
else:
vhost = self.choose_vhost(domain)
self._deploy_cert(vhost, cert_path, key_path, chain_path, fullchain_path)
@ -1476,10 +1478,16 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
raise errors.PluginError(
"Unsupported enhancement: {0}".format(enhancement))
try:
# Handle virtualhosts configured for a wildcard certificate
if self.wildcard_domain(domain) and self.wildcard_vhosts:
for vhost in self.wildcard_vhosts:
func(vhost, options)
if self.wildcard_domain(domain):
# Handle virtualhosts configured for a wildcard domain
if domain in self.wildcard_vhosts.keys():
# Vhosts for a wildcard domain were already selected
for vhost in self.wildcard_vhosts[domain]:
func(vhost, options)
else:
# Ask which Vhosts to enhance for wildcard domain
for vhost in self.choose_vhosts_wildcard(domain, create_ssl=False):
func(vhost, options)
else:
func(self.choose_vhost(domain), options)
except errors.PluginError:

View file

@ -168,7 +168,17 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods
modmacro="Yes" if self.modmacro else "No"))
def display_repr(self):
return str(self)
"""Return a representation of VHost to be used in dialog"""
return (
"File: {filename}\n"
"Addresses: {addrs}\n"
"Names: {names}\n"
"HTTPS: {https}\n\n".format(
filename=self.filep,
addrs=", ".join(str(addr) for addr in self.addrs),
names=", ".join(self.get_names()),
https="Yes" if self.ssl else "No"))
def __eq__(self, other):
if isinstance(other, self.__class__):

View file

@ -1337,6 +1337,92 @@ class MultipleVhostsTest(util.ApacheTest):
self.config.enable_mod,
"whatever")
def test_wildcard_domain(self):
cases = {u"*.example.org": True, b"*.x.example.org": True,
u"a.example.org": False, b"a.x.example.org": False}
for key in cases.keys():
self.assertEqual(self.config.wildcard_domain(key), cases[key])
def test_choose_vhosts_wildcard(self):
mock_path = "certbot_apache.display_ops.select_vhost_multiple"
with mock.patch(mock_path) as mock_select_vhs:
mock_select_vhs.return_value = [self.vh_truth[3]]
vhs = self.config.choose_vhosts_wildcard("*.certbot.demo",
create_ssl=True)
# Check that the dialog was called with one vh: certbot.demo
self.assertEquals(mock_select_vhs.call_args[0][0][0], self.vh_truth[3])
self.assertEquals(len(mock_select_vhs.call_args_list), 1)
# And the actual returned values
self.assertEquals(len(vhs), 1)
self.assertTrue(vhs[0].name == "certbot.demo")
self.assertTrue(vhs[0].ssl)
self.assertFalse(vhs[0] == self.vh_truth[3])
@mock.patch("certbot_apache.configurator.ApacheConfigurator.make_vhost_ssl")
def test_choose_vhosts_wildcard_no_ssl(self, mock_makessl):
mock_path = "certbot_apache.display_ops.select_vhost_multiple"
with mock.patch(mock_path) as mock_select_vhs:
mock_select_vhs.return_value = [self.vh_truth[3]]
vhs = self.config.choose_vhosts_wildcard("*.certbot.demo",
create_ssl=False)
self.assertFalse(mock_makessl.called)
self.assertEquals(vhs[0], self.vh_truth[3])
@mock.patch("certbot_apache.configurator.ApacheConfigurator.vhosts_for_wildcard")
@mock.patch("certbot_apache.configurator.ApacheConfigurator.make_vhost_ssl")
def test_choose_vhosts_wildcard_already_ssl(self, mock_makessl, mock_vh_for_w):
# Already SSL vhost
mock_vh_for_w.return_value = [self.vh_truth[7]]
mock_path = "certbot_apache.display_ops.select_vhost_multiple"
with mock.patch(mock_path) as mock_select_vhs:
mock_select_vhs.return_value = [self.vh_truth[7]]
vhs = self.config.choose_vhosts_wildcard("whatever",
create_ssl=True)
self.assertEquals(mock_select_vhs.call_args[0][0][0], self.vh_truth[7])
self.assertEquals(len(mock_select_vhs.call_args_list), 1)
# Ensure that make_vhost_ssl was not called, vhost.ssl == true
self.assertFalse(mock_makessl.called)
# And the actual returned values
self.assertEquals(len(vhs), 1)
self.assertTrue(vhs[0].ssl)
self.assertEquals(vhs[0], self.vh_truth[7])
def test_deploy_cert_wildcard(self):
mock_choose_vhosts = mock.MagicMock()
mock_choose_vhosts.return_value = [self.vh_truth[7]]
self.config.choose_vhosts_wildcard = mock_choose_vhosts
mock_d = "certbot_apache.configurator.ApacheConfigurator._deploy_cert"
with mock.patch(mock_d):
self.config.deploy_cert("*.wildcard.example.org", "/tmp/path",
"/tmp/path", "/tmp/path", "/tmp/path")
self.assertTrue("*.wildcard.example.org" in
self.config.wildcard_vhosts.keys())
self.assertTrue(self.vh_truth[7] in
self.config.wildcard_vhosts["*.wildcard.example.org"])
@mock.patch("certbot_apache.configurator.ApacheConfigurator.choose_vhosts_wildcard")
def test_enhance_wildcard_after_install(self, mock_choose):
self.config.parser.modules.add("mod_ssl.c")
self.config.parser.modules.add("headers_module")
self.config.wildcard_vhosts["*.certbot.demo"] = [self.vh_truth[3]]
self.config.enhance("*.certbot.demo", "ensure-http-header",
"Upgrade-Insecure-Requests")
self.assertFalse(mock_choose.called)
@mock.patch("certbot_apache.configurator.ApacheConfigurator.choose_vhosts_wildcard")
def test_enhance_wildcard_no_install(self, mock_choose):
mock_choose.return_value = [self.vh_truth[3]]
self.config.parser.modules.add("mod_ssl.c")
self.config.parser.modules.add("headers_module")
self.config.enhance("*.certbot.demo", "ensure-http-header",
"Upgrade-Insecure-Requests")
self.assertTrue(mock_choose.called)
class AugeasVhostsTest(util.ApacheTest):
"""Test vhosts with illegal names dependent on augeas version."""
# pylint: disable=protected-access

View file

@ -11,9 +11,39 @@ from certbot.tests import util as certbot_util
from certbot_apache import obj
from certbot_apache.display_ops import select_vhost_multiple
from certbot_apache.tests import util
class SelectVhostMultiTest(unittest.TestCase):
"""Tests for certbot_apache.display_ops.select_vhost_multiple."""
def setUp(self):
self.base_dir = "/example_path"
self.vhosts = util.get_vh_truth(
self.base_dir, "debian_apache_2_4/multiple_vhosts")
def test_select_no_input(self):
self.assertFalse(select_vhost_multiple([]))
@certbot_util.patch_get_utility()
def test_select_correct(self, mock_util):
mock_util().checklist.return_value = (
display_util.OK, [self.vhosts[3].display_repr(),
self.vhosts[2].display_repr()])
vhs = select_vhost_multiple([self.vhosts[3],
self.vhosts[2],
self.vhosts[1]])
self.assertTrue(self.vhosts[2] in vhs)
self.assertTrue(self.vhosts[3] in vhs)
self.assertFalse(self.vhosts[1] in vhs)
@certbot_util.patch_get_utility()
def test_select_cancel(self, mock_util):
mock_util().checklist.return_value = (display_util.CANCEL, "whatever")
vhs = select_vhost_multiple([self.vhosts[2], self.vhosts[3]])
self.assertFalse(vhs)
class SelectVhostTest(unittest.TestCase):
"""Tests for certbot_apache.display_ops.select_vhost."""

View file

@ -301,7 +301,7 @@ class ChooseNamesTest(unittest.TestCase):
all_valid = ["example.com", "second.example.com",
"also.example.com", "under_score.example.com",
"justtld"]
all_invalid = ["öóòps.net", "*.wildcard.com", "uniçodé.com"]
all_invalid = ["öóòps.net", "uniçodé.com"]
two_valid = ["example.com", "úniçøde.com", "also.example.com"]
self.assertEqual(get_valid_domains(all_valid), all_valid)
self.assertEqual(get_valid_domains(all_invalid), [])

View file

@ -1,3 +1,4 @@
# coding=utf-8
"""Tests for certbot.main."""
# pylint: disable=too-many-lines
from __future__ import print_function
@ -939,11 +940,6 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met
self.assertRaises(errors.ConfigurationError,
self._call,
['-d', (('a' * 50) + '.') * 10])
# Wildcard
self.assertRaises(errors.ConfigurationError,
self._call,
['-d', '*.wildcard.tld'])
# Bare IP address (this is actually a different error message now)
self.assertRaises(errors.ConfigurationError,
self._call,
@ -1232,7 +1228,7 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met
def test_renew_with_bad_domain(self):
renewalparams = {'authenticator': 'webroot'}
names = ['*.example.com']
names = ['exámple.com']
self._test_renew_common(renewalparams=renewalparams, error_expected=True,
names=names, assert_oc_called=False)