From 99f00d21c414eb244d65e8020579d95eb07a0354 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 13 Oct 2017 22:25:33 +0300 Subject: [PATCH] Skip menu in webroot plugin when there's nothing to choose from (#5183) * Skip menu in webroot, when there's nothing to choose from * Added testcase --- certbot/plugins/webroot.py | 21 +++++++++++++++------ certbot/plugins/webroot_test.py | 31 ++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/certbot/plugins/webroot.py b/certbot/plugins/webroot.py index 4662b2aa6..714d83cce 100644 --- a/certbot/plugins/webroot.py +++ b/certbot/plugins/webroot.py @@ -102,10 +102,14 @@ to serve all files under specified web root ({0}).""" webroot = None while webroot is None: - webroot = self._prompt_with_webroot_list(domain, known_webroots) - - if webroot is None: - webroot = self._prompt_for_new_webroot(domain) + if known_webroots: + # Only show the menu if we have options for it + webroot = self._prompt_with_webroot_list(domain, known_webroots) + if webroot is None: + webroot = self._prompt_for_new_webroot(domain) + else: + # Allow prompt to raise PluginError instead of looping forever + webroot = self._prompt_for_new_webroot(domain, True) return webroot @@ -125,13 +129,18 @@ to serve all files under specified web root ({0}).""" else: # code == display_util.OK return None if index == 0 else known_webroots[index - 1] - def _prompt_for_new_webroot(self, domain): + def _prompt_for_new_webroot(self, domain, allowraise=False): code, webroot = ops.validated_directory( _validate_webroot, "Input the webroot for {0}:".format(domain), force_interactive=True) if code == display_util.CANCEL: - return None + if not allowraise: + return None + else: + raise errors.PluginError( + "Every requested domain must have a " + "webroot when using the webroot plugin.") else: # code == display_util.OK return _validate_webroot(webroot) diff --git a/certbot/plugins/webroot_test.py b/certbot/plugins/webroot_test.py index e202a0a6d..5a311716e 100644 --- a/certbot/plugins/webroot_test.py +++ b/certbot/plugins/webroot_test.py @@ -96,7 +96,7 @@ class AuthenticatorTest(unittest.TestCase): @test_util.patch_get_utility() def test_new_webroot(self, mock_get_utility): self.config.webroot_path = [] - self.config.webroot_map = {} + self.config.webroot_map = {"something.com": self.path} mock_display = mock_get_utility() mock_display.menu.return_value = (display_util.OK, 0,) @@ -108,6 +108,19 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual(self.config.webroot_map[self.achall.domain], self.path) + @test_util.patch_get_utility() + def test_new_webroot_empty_map_cancel(self, mock_get_utility): + self.config.webroot_path = [] + self.config.webroot_map = {} + + mock_display = mock_get_utility() + mock_display.menu.return_value = (display_util.OK, 0,) + with mock.patch('certbot.display.ops.validated_directory') as m: + m.return_value = (display_util.CANCEL, -1) + self.assertRaises(errors.PluginError, + self.auth.perform, + [self.achall]) + def test_perform_missing_root(self): self.config.webroot_path = None self.config.webroot_map = {} @@ -132,6 +145,22 @@ class AuthenticatorTest(unittest.TestCase): mock_chown.side_effect = OSError(errno.EACCES, "msg") self.auth.perform([self.achall]) # exception caught and logged + + @test_util.patch_get_utility() + def test_perform_new_webroot_not_in_map(self, mock_get_utility): + new_webroot = tempfile.mkdtemp() + self.config.webroot_path = [] + self.config.webroot_map = {"whatever.com": self.path} + mock_display = mock_get_utility() + mock_display.menu.side_effect = ((display_util.OK, 0), + (display_util.OK, new_webroot)) + achall = achallenges.KeyAuthorizationAnnotatedChallenge( + challb=acme_util.HTTP01_P, domain="something.com", account_key=KEY) + with mock.patch('certbot.display.ops.validated_directory') as m: + m.return_value = (display_util.OK, new_webroot,) + self.auth.perform([achall]) + self.assertEqual(self.config.webroot_map[achall.domain], new_webroot) + def test_perform_permissions(self): self.auth.prepare()