pass achalls to auth_hint, write some tests

This commit is contained in:
Alex Zorin 2021-01-23 21:48:46 +11:00
parent 0a0c681734
commit b9a05f97b3
10 changed files with 82 additions and 30 deletions

View file

@ -2467,7 +2467,7 @@ class ApacheConfigurator(common.Installer):
version=".".join(str(i) for i in self.version))
)
def auth_hint(self, chall_type):
def auth_hint(self, failed_achalls):
return ("The Certificate Authority failed to verify the temporary Apache configuration "
"changes made by the --apache plugin. Ensure that the above domains point to "
"this Apache server and that it is accessible from the internet.")

View file

@ -1029,7 +1029,7 @@ class NginxConfigurator(common.Installer):
version=".".join(str(i) for i in self.version))
)
def auth_hint(self, chall_type):
def auth_hint(self, failed_achalls):
return (
"The Certificate Authority failed to verify the temporary nginx configuration changes "
"made by the --nginx plugin. Ensure the above domains point to this nginx server and "

View file

@ -284,11 +284,8 @@ class AuthHandler(object):
for _, achalls in sorted(problems.items(), key=lambda item: item[0]):
msg.append(_generate_failed_chall_msg(achalls))
# Show a hint for whatever the first failed challenge was. Every failed challenge will
# almost certainly be the same type, except for very exotic --manual setups.
if failed_achalls:
chall_type = failed_achalls[0].typ
msg.append('\nHint: {}\n'.format(self.auth.auth_hint(chall_type)))
msg.append('\nHint: {}\n'.format(self.auth.auth_hint(failed_achalls)))
display_util.notify("".join(msg))

View file

@ -9,6 +9,7 @@ from certbot import errors
from certbot import interfaces
from certbot import reverter
from certbot import util
from certbot._internal.cli import cli_constants
from certbot._internal import hooks
from certbot.compat import misc
from certbot.compat import os
@ -108,31 +109,39 @@ permitted by DNS standards.)
'validation challenges either through shell scripts provided by '
'the user or by performing the setup manually.')
def auth_hint(self, chall_type):
is_dns = chall_type == 'dns-01'
def auth_hint(self, failed_achalls):
has_chall = lambda cls: any(isinstance(achall.chall, cls) for achall in failed_achalls)
has_dns = has_chall(challenges.DNS01)
resource_names = {
challenges.DNS01: 'DNS TXT records',
challenges.HTTP01: 'challenge files',
challenges.TLSALPN01: 'TLS-ALPN certificates'
}
resources = ' and '.join(sorted([v for k, v in resource_names.items() if has_chall(k)]))
if self.conf('auth-hook'):
return (
'The Certificate Authority failed to verify the changes made by the '
'The Certificate Authority failed to verify the {resources} created by the '
'--manual-auth-hook. Ensure that this hook is functioning correctly{dns_hint}. '
'Refer to {certbot} --help manual.'
'Refer to "{certbot} --help manual".'
.format(
certbot='certbot',
certbot=cli_constants.cli_command,
resources=resources,
dns_hint=(
' and that it waits a sufficient duration '
'of time after creating a DNS TXT record'
) if is_dns else ''
' and that it waits a sufficient duration of time for DNS propagation'
) if has_dns else ''
)
)
else:
return (
'The Certificate Authority failed to {verify} the manually created {resources}. '
'Ensure that you created the {resources} in the correct location{dns_hint}.'
'The Certificate Authority failed to verify the manually created {resources}. '
'Ensure that you created these in the correct location{dns_hint}.'
.format(
verify='verify' if is_dns else 'download',
resources='DNS TXT records' if is_dns else 'challenge files',
resources=resources,
dns_hint=(
' or try waiting longer for DNS propagation on the next attempt'
) if is_dns else ''
', or try waiting longer for DNS propagation on the next attempt'
) if has_dns else ''
)
)

View file

@ -62,7 +62,7 @@ to serve all files under specified web root ({0})."""
"file, it needs to be on a single line, like: webroot-map = "
'{"example.com":"/var/www"}.')
def auth_hint(self, chall_type):
def auth_hint(self, failed_achalls):
return ("The Certificate Authority failed to download the temporary challenge files "
"created by the --webroot plugin. Ensure that the above domains serve their "
"content from the provided --webroot-path/-w and that files created there "

View file

@ -9,7 +9,7 @@ import pkg_resources
import zope.interface
from acme.magic_typing import List
from certbot import achallenges # pylint: disable=unused-import
from certbot import achallenges # pylint: disable=unused-import
from certbot import crypto_util
from certbot import errors
from certbot import interfaces
@ -97,8 +97,8 @@ class Plugin(object):
"""Find a configuration value for variable ``var``."""
return getattr(self.config, self.dest(var))
def auth_hint(self, chall_type):
# type: (str) -> str
def auth_hint(self, failed_achalls):
# type: (List[achallenges.AnnotatedChallenge]) -> str
"""Human-readable string to help the user troubleshoot the authenticator.
Shown to the user if one or more of the attempted challenges were not a success.
@ -106,16 +106,18 @@ class Plugin(object):
Should describe, in simple language, what the authenticator tried to do, what went
wrong and what the user should try as their "next steps".
:param str chall_type: The challenge type (label of ACME Validation Method)
:param list failed_achalls: List of one or more failed challenges
(:class:`achallenges.AnnotatedChallenge` subclasses).
:rtype str:
"""
# This is a fallback hint. Authenticators should implement their own auth_hint that
# addresses the specific mechanics of that authenticator.
challs = " and ".join(sorted({achall.typ for achall in failed_achalls}))
return ("The Certificate Authority couldn't exterally verify that the {name} plugin "
"completed the required {chall_type} challenges. Ensure the plugin is configured "
"completed the required {challs} challenges. Ensure the plugin is configured "
"correctly and that the changes it makes are accessible from the internet."
.format(name=self.name, chall_type=chall_type))
.format(name=self.name, challs=challs))
class Installer(Plugin):

View file

@ -37,11 +37,13 @@ class DNSAuthenticator(common.Plugin):
help='The number of seconds to wait for DNS to propagate before asking the ACME server '
'to verify the DNS record.')
def auth_hint(self, chall_type):
def auth_hint(self, failed_achalls):
delay = self.conf('propagation-seconds')
return (
'The Certificate Authority failed to verify the DNS TXT records created by the '
'--{name} plugin. Ensure that the above domains are managed by that DNS provider, '
'or try increasing --{name}-propagation-seconds.'.format(name=self.name)
'--{name} plugin. Ensure the above domains are hosted by this DNS provider, '
'or try increasing --{name}-propagation-seconds (currently {secs} second{suffix}).'
.format(name=self.name, secs=delay, suffix='s' if delay != 1 else '')
)
def get_chall_pref(self, unused_domain): # pylint: disable=missing-function-docstring

View file

@ -83,6 +83,13 @@ class PluginTest(unittest.TestCase):
parser.add_argument.assert_called_once_with(
"--mock-foo-bar", dest="different_to_foo_bar", x=1, y=None)
def test_fallback_auth_hint(self):
self.assertIn("the mock plugin completed the required dns-01 challenges",
self.plugin.auth_hint([acme_util.DNS01_A, acme_util.DNS01_A]))
self.assertIn("the mock plugin completed the required dns-01 and http-01 challenges",
self.plugin.auth_hint([acme_util.DNS01_A, acme_util.HTTP01_A,
acme_util.DNS01_A]))
class InstallerTest(test_util.ConfigTestCase):
"""Tests for certbot.plugins.common.Installer."""

View file

@ -119,6 +119,12 @@ class DNSAuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthen
credentials = self.auth._configure_credentials("credentials", "", {"test": ""})
self.assertEqual(credentials.conf("test"), "value")
def test_auth_hint(self):
self.assertIn(
'try increasing --fake-propagation-seconds (currently 0 seconds).',
self.auth.auth_hint([mock.MagicMock()])
)
class CredentialsConfigurationTest(test_util.TempDirTestCase):
class _MockLoggingHandler(logging.Handler):

View file

@ -122,6 +122,35 @@ class AuthenticatorTest(test_util.TempDirTestCase):
else:
self.assertFalse('CERTBOT_TOKEN' in os.environ)
def test_auth_hint_hook(self):
self.config.manual_auth_hook = '/bin/true'
self.assertEqual(
self.auth.auth_hint([acme_util.DNS01_A, acme_util.HTTP01_A]),
'The Certificate Authority failed to verify the DNS TXT records and challenge '
'files created by the --manual-auth-hook. Ensure that this hook is functioning '
'correctly and that it waits a sufficient duration of time for DNS propagation. '
'Refer to "certbot --help manual".'
)
self.assertEqual(
self.auth.auth_hint([acme_util.HTTP01_A]),
'The Certificate Authority failed to verify the challenge files created by the '
'--manual-auth-hook. Ensure that this hook is functioning correctly. Refer to '
'"certbot --help manual".'
)
def test_auth_hint_no_hook(self):
self.assertEqual(
self.auth.auth_hint([acme_util.DNS01_A, acme_util.HTTP01_A]),
'The Certificate Authority failed to verify the manually created DNS TXT records '
'and challenge files. Ensure that you created these in the correct location, or '
'try waiting longer for DNS propagation on the next attempt.'
)
self.assertEqual(
self.auth.auth_hint([acme_util.HTTP01_A, acme_util.HTTP01_A, acme_util.HTTP01_A]),
'The Certificate Authority failed to verify the manually created challenge files. '
'Ensure that you created these in the correct location.'
)
if __name__ == '__main__':
unittest.main() # pragma: no cover