From 228619261940fd0eeb59adf935f70930ee9850c8 Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Mon, 7 Dec 2020 20:19:30 +1100
Subject: [PATCH 01/15] cli: redesign output of failed authz reporting

---
 certbot/certbot/_internal/auth_handler.py | 55 ++++++-----------------
 certbot/tests/auth_handler_test.py        | 42 +++++++++++++----
 2 files changed, 48 insertions(+), 49 deletions(-)

diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py
index 7ea2a1de8..d8b51a153 100644
--- a/certbot/certbot/_internal/auth_handler.py
+++ b/certbot/certbot/_internal/auth_handler.py
@@ -4,6 +4,7 @@ import logging
 import time
 
 import zope.component
+from josepy.jwk import JWK # pylint: disable=unused-import
 
 from acme import challenges
 from acme import errors as acme_errors
@@ -15,6 +16,7 @@ from certbot import achallenges
 from certbot import errors
 from certbot import interfaces
 from certbot._internal import error_handler
+from certbot.display import util as display_util
 
 logger = logging.getLogger(__name__)
 
@@ -394,39 +396,8 @@ def _report_no_chall_path(challbs):
     raise errors.AuthorizationError(msg)
 
 
-_ERROR_HELP_COMMON = (
-    "To fix these errors, please make sure that your domain name was entered "
-    "correctly and the DNS A/AAAA record(s) for that domain contain(s) the "
-    "right IP address.")
-
-
-_ERROR_HELP = {
-    "connection":
-        _ERROR_HELP_COMMON + " Additionally, please check that your computer "
-        "has a publicly routable IP address and that no firewalls are preventing "
-        "the server from communicating with the client. If you're using the "
-        "webroot plugin, you should also verify that you are serving files "
-        "from the webroot path you provided.",
-    "dnssec":
-        _ERROR_HELP_COMMON + " Additionally, if you have DNSSEC enabled for "
-        "your domain, please ensure that the signature is valid.",
-    "malformed":
-        "To fix these errors, please make sure that you did not provide any "
-        "invalid information to the client, and try running Certbot "
-        "again.",
-    "serverInternal":
-        "Unfortunately, an error on the ACME server prevented you from completing "
-        "authorization. Please try again later.",
-    "tls":
-        _ERROR_HELP_COMMON + " Additionally, please check that you have an "
-        "up-to-date TLS configuration that allows the server to communicate "
-        "with the Certbot client.",
-    "unauthorized": _ERROR_HELP_COMMON,
-    "unknownHost": _ERROR_HELP_COMMON,
-}
-
-
 def _report_failed_authzrs(failed_authzrs, account_key):
+    # type: (List[messages.AuthorizationResource], JWK) -> None
     """Notifies the user about failed authorizations."""
     problems = {}  # type: Dict[str, List[achallenges.AnnotatedChallenge]]
     failed_achalls = [challb_to_achall(challb, account_key, authzr.body.identifier.value)
@@ -436,18 +407,24 @@ def _report_failed_authzrs(failed_authzrs, account_key):
     for achall in failed_achalls:
         problems.setdefault(achall.error.typ, []).append(achall)
 
-    reporter = zope.component.getUtility(interfaces.IReporter)
+    config = zope.component.getUtility(interfaces.IConfig)
+    msg = ["\nCertbot encountered errors for these domains "
+           "while requesting the certificate (using the {} plugin):"
+           .format(config.authenticator)]
+
     for achalls in problems.values():
-        reporter.add_message(_generate_failed_chall_msg(achalls), reporter.MEDIUM_PRIORITY)
+        msg.append(_generate_failed_chall_msg(achalls))
+
+    display_util.notify("".join(msg))
 
 
 def _generate_failed_chall_msg(failed_achalls):
+    # type: (List[achallenges.AnnotatedChallenge]) -> str
     """Creates a user friendly error message about failed challenges.
 
     :param list failed_achalls: A list of failed
         :class:`certbot.achallenges.AnnotatedChallenge` with the same error
         type.
-
     :returns: A formatted error message for the client.
     :rtype: str
 
@@ -456,14 +433,10 @@ def _generate_failed_chall_msg(failed_achalls):
     typ = error.typ
     if messages.is_acme_error(error):
         typ = error.code
-    msg = ["The following errors were reported by the server:"]
+    msg = []
 
     for achall in failed_achalls:
-        msg.append("\n\nDomain: %s\nType:   %s\nDetail: %s" % (
+        msg.append("\n  Domain: %s\n  Type:   %s\n  Detail: %s\n" % (
             achall.domain, typ, achall.error.detail))
 
-    if typ in _ERROR_HELP:
-        msg.append("\n\n")
-        msg.append(_ERROR_HELP[typ])
-
     return "".join(msg)
diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py
index 8eb5d7702..d1bf09ae0 100644
--- a/certbot/tests/auth_handler_test.py
+++ b/certbot/tests/auth_handler_test.py
@@ -504,21 +504,47 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
         self.authzr2.body.identifier.value = 'foo.bar'
         self.authzr2.body.challenges = [http_01_diff]
 
+    @mock.patch('certbot._internal.auth_handler.display_util.notify')
     @test_util.patch_get_utility()
-    def test_same_error_and_domain(self, mock_zope):
+    def test_same_error_and_domain(self, mock_util, mock_notify):
         from certbot._internal import auth_handler
-
+        mock_util().authenticator = "foobaz"
         auth_handler._report_failed_authzrs([self.authzr1], 'key')
-        call_list = mock_zope().add_message.call_args_list
-        self.assertEqual(len(call_list), 1)
-        self.assertTrue("Domain: example.com\nType:   tls\nDetail: detail" in call_list[0][0][0])
+        mock_notify.assert_called_with(
+            '\n'
+            'Certbot encountered errors for these domains while requesting the '
+            'certificate (using the foobaz plugin):\n'
+            '  Domain: example.com\n'
+            '  Type:   tls\n'
+            '  Detail: detail\n'
+            '\n'
+            '  Domain: example.com\n'
+            '  Type:   tls\n'
+            '  Detail: detail\n'
+        )
 
+    @mock.patch('certbot._internal.auth_handler.display_util.notify')
     @test_util.patch_get_utility()
-    def test_different_errors_and_domains(self, mock_zope):
+    def test_different_errors_and_domains(self, mock_util, mock_notify):
         from certbot._internal import auth_handler
-
+        mock_util().authenticator = "quux"
         auth_handler._report_failed_authzrs([self.authzr1, self.authzr2], 'key')
-        self.assertEqual(mock_zope().add_message.call_count, 2)
+        mock_notify.assert_called_with(
+            '\n'
+            'Certbot encountered errors for these domains while requesting the '
+            'certificate (using the quux plugin):\n'
+            '  Domain: example.com\n'
+            '  Type:   tls\n'
+            '  Detail: detail\n'
+            '\n'
+            '  Domain: example.com\n'
+            '  Type:   tls\n'
+            '  Detail: detail\n'
+            '\n'
+            '  Domain: foo.bar\n'
+            '  Type:   dnssec\n'
+            '  Detail: detail\n'
+        )
 
 
 def gen_auth_resp(chall_list):

From 3eeb0fad0358e10fbb9de22cafaf3086ea783d48 Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Mon, 7 Dec 2020 22:45:52 +1100
Subject: [PATCH 02/15] fix problem sorting to be stable between py2 & 3

---
 certbot/certbot/_internal/auth_handler.py |  2 +-
 certbot/tests/auth_handler_test.py        | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py
index d8b51a153..56618977a 100644
--- a/certbot/certbot/_internal/auth_handler.py
+++ b/certbot/certbot/_internal/auth_handler.py
@@ -412,7 +412,7 @@ def _report_failed_authzrs(failed_authzrs, account_key):
            "while requesting the certificate (using the {} plugin):"
            .format(config.authenticator)]
 
-    for achalls in problems.values():
+    for _, achalls in sorted(problems.items(), key=lambda item: item[0]):
         msg.append(_generate_failed_chall_msg(achalls))
 
     display_util.notify("".join(msg))
diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py
index d1bf09ae0..9a6857bc9 100644
--- a/certbot/tests/auth_handler_test.py
+++ b/certbot/tests/auth_handler_test.py
@@ -533,17 +533,17 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
             '\n'
             'Certbot encountered errors for these domains while requesting the '
             'certificate (using the quux plugin):\n'
-            '  Domain: example.com\n'
-            '  Type:   tls\n'
-            '  Detail: detail\n'
-            '\n'
-            '  Domain: example.com\n'
-            '  Type:   tls\n'
-            '  Detail: detail\n'
-            '\n'
             '  Domain: foo.bar\n'
             '  Type:   dnssec\n'
             '  Detail: detail\n'
+            '\n'
+            '  Domain: example.com\n'
+            '  Type:   tls\n'
+            '  Detail: detail\n'
+            '\n'
+            '  Domain: example.com\n'
+            '  Type:   tls\n'
+            '  Detail: detail\n'
         )
 
 

From 0626f6704100f40b1911a0ac073703104beb4447 Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Wed, 23 Dec 2020 18:10:29 +1100
Subject: [PATCH 03/15] add some catch-all error text

---
 certbot/certbot/_internal/auth_handler.py | 11 ++++++++---
 certbot/tests/auth_handler_test.py        | 14 ++++++++++----
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py
index 56618977a..e14412c02 100644
--- a/certbot/certbot/_internal/auth_handler.py
+++ b/certbot/certbot/_internal/auth_handler.py
@@ -408,13 +408,18 @@ def _report_failed_authzrs(failed_authzrs, account_key):
         problems.setdefault(achall.error.typ, []).append(achall)
 
     config = zope.component.getUtility(interfaces.IConfig)
-    msg = ["\nCertbot encountered errors for these domains "
-           "while requesting the certificate (using the {} plugin):"
-           .format(config.authenticator)]
+    msg = ["\nCertbot failed to authenticate some domains (using the {} plugin)."
+           " The ACME server reported these problems:".format(config.authenticator)]
 
     for _, achalls in sorted(problems.items(), key=lambda item: item[0]):
         msg.append(_generate_failed_chall_msg(achalls))
 
+    msg.append(
+        '\nHint: the Certificate Authority externally verifies the local changes that '
+        'Certbot makes. Ensure the above domains are configured correctly and that changes '
+        'made by the {} plugin are accessible from the internet.\n'
+        .format(config.authenticator)
+    )
     display_util.notify("".join(msg))
 
 
diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py
index 9a6857bc9..dd7fd002b 100644
--- a/certbot/tests/auth_handler_test.py
+++ b/certbot/tests/auth_handler_test.py
@@ -512,8 +512,8 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
         auth_handler._report_failed_authzrs([self.authzr1], 'key')
         mock_notify.assert_called_with(
             '\n'
-            'Certbot encountered errors for these domains while requesting the '
-            'certificate (using the foobaz plugin):\n'
+            'Certbot failed to authenticate some domains (using the foobaz plugin). '
+            'The ACME server reported these problems:\n'
             '  Domain: example.com\n'
             '  Type:   tls\n'
             '  Detail: detail\n'
@@ -521,6 +521,9 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
             '  Domain: example.com\n'
             '  Type:   tls\n'
             '  Detail: detail\n'
+            '\nHint: the Certificate Authority externally verifies the local changes that '
+            'Certbot makes. Ensure the above domains are configured correctly and that '
+            'changes made by the foobaz plugin are accessible from the internet.\n'
         )
 
     @mock.patch('certbot._internal.auth_handler.display_util.notify')
@@ -531,8 +534,8 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
         auth_handler._report_failed_authzrs([self.authzr1, self.authzr2], 'key')
         mock_notify.assert_called_with(
             '\n'
-            'Certbot encountered errors for these domains while requesting the '
-            'certificate (using the quux plugin):\n'
+            'Certbot failed to authenticate some domains (using the quux plugin). '
+            'The ACME server reported these problems:\n'
             '  Domain: foo.bar\n'
             '  Type:   dnssec\n'
             '  Detail: detail\n'
@@ -544,6 +547,9 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
             '  Domain: example.com\n'
             '  Type:   tls\n'
             '  Detail: detail\n'
+            '\nHint: the Certificate Authority externally verifies the local changes that '
+            'Certbot makes. Ensure the above domains are configured correctly and that '
+            'changes made by the quux plugin are accessible from the internet.\n'
         )
 
 

From 0a0c6817345f564328b00fde81efed08de75c0cf Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Wed, 20 Jan 2021 21:42:19 +1100
Subject: [PATCH 04/15] add per-authenticator hints

---
 .../certbot_apache/_internal/configurator.py  |  5 ++
 .../certbot_nginx/_internal/configurator.py   |  7 +++
 certbot/certbot/_internal/auth_handler.py     | 54 +++++++++----------
 certbot/certbot/_internal/plugins/manual.py   | 28 ++++++++++
 certbot/certbot/_internal/plugins/webroot.py  |  6 +++
 certbot/certbot/plugins/common.py             | 20 +++++++
 certbot/certbot/plugins/dns_common.py         |  7 +++
 certbot/tests/auth_handler_test.py            | 43 +++++++--------
 8 files changed, 121 insertions(+), 49 deletions(-)

diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py
index c20a4fdd6..b4e4ccd07 100644
--- a/certbot-apache/certbot_apache/_internal/configurator.py
+++ b/certbot-apache/certbot_apache/_internal/configurator.py
@@ -2467,6 +2467,11 @@ class ApacheConfigurator(common.Installer):
                 version=".".join(str(i) for i in self.version))
         )
 
+    def auth_hint(self, chall_type):
+        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.")
+
     ###########################################################################
     # Challenges Section
     ###########################################################################
diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py
index 87afedd38..5958c1df6 100644
--- a/certbot-nginx/certbot_nginx/_internal/configurator.py
+++ b/certbot-nginx/certbot_nginx/_internal/configurator.py
@@ -1029,6 +1029,13 @@ class NginxConfigurator(common.Installer):
                 version=".".join(str(i) for i in self.version))
         )
 
+    def auth_hint(self, chall_type):
+        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 "
+            "that it is accessible from the internet, or try increase --nginx-sleep-seconds."
+        )
+
     ###################################################
     # Wrapper functions for Reverter class (IInstaller)
     ###################################################
diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py
index e14412c02..45ebd10d4 100644
--- a/certbot/certbot/_internal/auth_handler.py
+++ b/certbot/certbot/_internal/auth_handler.py
@@ -176,7 +176,7 @@ class AuthHandler(object):
 
         # In case of failed authzrs, create a report to the user.
         if authzrs_failed_to_report:
-            _report_failed_authzrs(authzrs_failed_to_report, self.account.key)
+            self._report_failed_authzrs(authzrs_failed_to_report)
             if not best_effort:
                 # Without best effort, having failed authzrs is critical and fail the process.
                 raise errors.AuthorizationError('Some challenges have failed.')
@@ -267,6 +267,31 @@ class AuthHandler(object):
 
         return achalls
 
+    def _report_failed_authzrs(self, failed_authzrs):
+        # type: (List[messages.AuthorizationResource]) -> None
+        """Notifies the user about failed authorizations."""
+        problems = {}  # type: Dict[str, List[achallenges.AnnotatedChallenge]]
+        failed_achalls = [challb_to_achall(challb, self.account.key, authzr.body.identifier.value)
+                        for authzr in failed_authzrs for challb in authzr.body.challenges
+                        if challb.error]
+
+        for achall in failed_achalls:
+            problems.setdefault(achall.error.typ, []).append(achall)
+
+        msg = ["\nCertbot failed to authenticate some domains (using the {} plugin)."
+            " The Certificate Authority reported these problems:".format(self.auth.name)]
+
+        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)))
+
+        display_util.notify("".join(msg))
+
 
 def challb_to_achall(challb, account_key, domain):
     """Converts a ChallengeBody object to an AnnotatedChallenge.
@@ -396,33 +421,6 @@ def _report_no_chall_path(challbs):
     raise errors.AuthorizationError(msg)
 
 
-def _report_failed_authzrs(failed_authzrs, account_key):
-    # type: (List[messages.AuthorizationResource], JWK) -> None
-    """Notifies the user about failed authorizations."""
-    problems = {}  # type: Dict[str, List[achallenges.AnnotatedChallenge]]
-    failed_achalls = [challb_to_achall(challb, account_key, authzr.body.identifier.value)
-                      for authzr in failed_authzrs for challb in authzr.body.challenges
-                      if challb.error]
-
-    for achall in failed_achalls:
-        problems.setdefault(achall.error.typ, []).append(achall)
-
-    config = zope.component.getUtility(interfaces.IConfig)
-    msg = ["\nCertbot failed to authenticate some domains (using the {} plugin)."
-           " The ACME server reported these problems:".format(config.authenticator)]
-
-    for _, achalls in sorted(problems.items(), key=lambda item: item[0]):
-        msg.append(_generate_failed_chall_msg(achalls))
-
-    msg.append(
-        '\nHint: the Certificate Authority externally verifies the local changes that '
-        'Certbot makes. Ensure the above domains are configured correctly and that changes '
-        'made by the {} plugin are accessible from the internet.\n'
-        .format(config.authenticator)
-    )
-    display_util.notify("".join(msg))
-
-
 def _generate_failed_chall_msg(failed_achalls):
     # type: (List[achallenges.AnnotatedChallenge]) -> str
     """Creates a user friendly error message about failed challenges.
diff --git a/certbot/certbot/_internal/plugins/manual.py b/certbot/certbot/_internal/plugins/manual.py
index a2e4bb28e..37c16382b 100644
--- a/certbot/certbot/_internal/plugins/manual.py
+++ b/certbot/certbot/_internal/plugins/manual.py
@@ -108,6 +108,34 @@ 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'
+        if self.conf('auth-hook'):
+            return (
+                'The Certificate Authority failed to verify the changes made by the '
+                '--manual-auth-hook. Ensure that this hook is functioning correctly{dns_hint}. '
+                'Refer to {certbot} --help manual.'
+                .format(
+                    certbot='certbot',
+                    dns_hint=(
+                        ' and that it waits a sufficient duration '
+                        'of time after creating a DNS TXT record'
+                    ) if is_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}.'
+                .format(
+                    verify='verify' if is_dns else 'download',
+                    resources='DNS TXT records' if is_dns else 'challenge files',
+                    dns_hint=(
+                        ' or try waiting longer for DNS propagation on the next attempt'
+                     ) if is_dns else ''
+                )
+            )
+
     def get_chall_pref(self, domain):
         # pylint: disable=unused-argument,missing-function-docstring
         return [challenges.HTTP01, challenges.DNS01]
diff --git a/certbot/certbot/_internal/plugins/webroot.py b/certbot/certbot/_internal/plugins/webroot.py
index 484d209d6..2ad1b9457 100644
--- a/certbot/certbot/_internal/plugins/webroot.py
+++ b/certbot/certbot/_internal/plugins/webroot.py
@@ -62,6 +62,12 @@ 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):
+        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 "
+                "can be downloaded from the internet.")
+
     def get_chall_pref(self, domain):  # pragma: no cover
         # pylint: disable=unused-argument,missing-function-docstring
         return [challenges.HTTP01]
diff --git a/certbot/certbot/plugins/common.py b/certbot/certbot/plugins/common.py
index d3fb5b7dc..a5d61eda2 100644
--- a/certbot/certbot/plugins/common.py
+++ b/certbot/certbot/plugins/common.py
@@ -97,6 +97,26 @@ 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
+        """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.
+
+        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)
+
+        :rtype str:
+        """
+        # This is a fallback hint. Authenticators should implement their own auth_hint that
+        # addresses the specific mechanics of that authenticator.
+        return ("The Certificate Authority couldn't exterally verify that the {name} plugin "
+                "completed the required {chall_type} 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))
+
 
 class Installer(Plugin):
     """An installer base class with reverter and ssl_dhparam methods defined.
diff --git a/certbot/certbot/plugins/dns_common.py b/certbot/certbot/plugins/dns_common.py
index 245b7dc05..c6b881816 100644
--- a/certbot/certbot/plugins/dns_common.py
+++ b/certbot/certbot/plugins/dns_common.py
@@ -37,6 +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):
+        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)
+        )
+
     def get_chall_pref(self, unused_domain):  # pylint: disable=missing-function-docstring
         return [challenges.DNS01]
 
diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py
index dd7fd002b..8e927f0ae 100644
--- a/certbot/tests/auth_handler_test.py
+++ b/certbot/tests/auth_handler_test.py
@@ -327,7 +327,8 @@ class HandleAuthorizationsTest(unittest.TestCase):
 
         mock_order = mock.MagicMock(authorizations=authzrs)
 
-        with mock.patch('certbot._internal.auth_handler._report_failed_authzrs') as mock_report:
+        with mock.patch('certbot._internal.auth_handler.AuthHandler._report_failed_authzrs') \
+            as mock_report:
             valid_authzr = self.handler.handle_authorizations(mock_order, True)
 
         # Because best_effort=True, we did not blow up. Instead ...
@@ -474,10 +475,18 @@ class GenChallengePathTest(unittest.TestCase):
 
 
 class ReportFailedAuthzrsTest(unittest.TestCase):
-    """Tests for certbot._internal.auth_handler._report_failed_authzrs."""
+    """Tests for certbot._internal.auth_handler.AuthHandler._report_failed_authzrs."""
     # pylint: disable=protected-access
 
+
     def setUp(self):
+        from certbot._internal.auth_handler import AuthHandler
+
+        self.mock_auth = mock.MagicMock(name="buzz")
+        self.mock_auth.name = "buzz"
+        self.mock_auth.auth_hint.return_value = "the buzz hint"
+        self.handler = AuthHandler(self.mock_auth, mock.MagicMock(), mock.MagicMock(), [])
+
         kwargs = {
             "chall": acme_util.HTTP01,
             "uri": "uri",
@@ -505,15 +514,12 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
         self.authzr2.body.challenges = [http_01_diff]
 
     @mock.patch('certbot._internal.auth_handler.display_util.notify')
-    @test_util.patch_get_utility()
-    def test_same_error_and_domain(self, mock_util, mock_notify):
-        from certbot._internal import auth_handler
-        mock_util().authenticator = "foobaz"
-        auth_handler._report_failed_authzrs([self.authzr1], 'key')
+    def test_same_error_and_domain(self, mock_notify):
+        self.handler._report_failed_authzrs([self.authzr1])
         mock_notify.assert_called_with(
             '\n'
-            'Certbot failed to authenticate some domains (using the foobaz plugin). '
-            'The ACME server reported these problems:\n'
+            'Certbot failed to authenticate some domains (using the buzz plugin). '
+            'The Certificate Authority reported these problems:\n'
             '  Domain: example.com\n'
             '  Type:   tls\n'
             '  Detail: detail\n'
@@ -521,21 +527,18 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
             '  Domain: example.com\n'
             '  Type:   tls\n'
             '  Detail: detail\n'
-            '\nHint: the Certificate Authority externally verifies the local changes that '
-            'Certbot makes. Ensure the above domains are configured correctly and that '
-            'changes made by the foobaz plugin are accessible from the internet.\n'
+            '\nHint: the buzz hint\n'
         )
 
     @mock.patch('certbot._internal.auth_handler.display_util.notify')
-    @test_util.patch_get_utility()
-    def test_different_errors_and_domains(self, mock_util, mock_notify):
-        from certbot._internal import auth_handler
-        mock_util().authenticator = "quux"
-        auth_handler._report_failed_authzrs([self.authzr1, self.authzr2], 'key')
+    def test_different_errors_and_domains(self, mock_notify):
+        self.mock_auth.name = "quux"
+        self.mock_auth.auth_hint.return_value = "quuuuuux"
+        self.handler._report_failed_authzrs([self.authzr1, self.authzr2])
         mock_notify.assert_called_with(
             '\n'
             'Certbot failed to authenticate some domains (using the quux plugin). '
-            'The ACME server reported these problems:\n'
+            'The Certificate Authority reported these problems:\n'
             '  Domain: foo.bar\n'
             '  Type:   dnssec\n'
             '  Detail: detail\n'
@@ -547,9 +550,7 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
             '  Domain: example.com\n'
             '  Type:   tls\n'
             '  Detail: detail\n'
-            '\nHint: the Certificate Authority externally verifies the local changes that '
-            'Certbot makes. Ensure the above domains are configured correctly and that '
-            'changes made by the quux plugin are accessible from the internet.\n'
+            '\nHint: quuuuuux\n'
         )
 
 

From b9a05f97b34f3140456b9855a17d2e5261b90575 Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Sat, 23 Jan 2021 21:48:46 +1100
Subject: [PATCH 05/15] pass achalls to auth_hint, write some tests

---
 .../certbot_apache/_internal/configurator.py  |  2 +-
 .../certbot_nginx/_internal/configurator.py   |  2 +-
 certbot/certbot/_internal/auth_handler.py     |  5 +--
 certbot/certbot/_internal/plugins/manual.py   | 37 ++++++++++++-------
 certbot/certbot/_internal/plugins/webroot.py  |  2 +-
 certbot/certbot/plugins/common.py             | 14 ++++---
 certbot/certbot/plugins/dns_common.py         |  8 ++--
 certbot/tests/plugins/common_test.py          |  7 ++++
 certbot/tests/plugins/dns_common_test.py      |  6 +++
 certbot/tests/plugins/manual_test.py          | 29 +++++++++++++++
 10 files changed, 82 insertions(+), 30 deletions(-)

diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py
index b4e4ccd07..fc90caecc 100644
--- a/certbot-apache/certbot_apache/_internal/configurator.py
+++ b/certbot-apache/certbot_apache/_internal/configurator.py
@@ -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.")
diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py
index 5958c1df6..be465fbae 100644
--- a/certbot-nginx/certbot_nginx/_internal/configurator.py
+++ b/certbot-nginx/certbot_nginx/_internal/configurator.py
@@ -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 "
diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py
index 45ebd10d4..8eae98752 100644
--- a/certbot/certbot/_internal/auth_handler.py
+++ b/certbot/certbot/_internal/auth_handler.py
@@ -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))
 
diff --git a/certbot/certbot/_internal/plugins/manual.py b/certbot/certbot/_internal/plugins/manual.py
index 37c16382b..2c632e8b1 100644
--- a/certbot/certbot/_internal/plugins/manual.py
+++ b/certbot/certbot/_internal/plugins/manual.py
@@ -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 ''
                 )
             )
 
diff --git a/certbot/certbot/_internal/plugins/webroot.py b/certbot/certbot/_internal/plugins/webroot.py
index 2ad1b9457..51c551821 100644
--- a/certbot/certbot/_internal/plugins/webroot.py
+++ b/certbot/certbot/_internal/plugins/webroot.py
@@ -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 "
diff --git a/certbot/certbot/plugins/common.py b/certbot/certbot/plugins/common.py
index a5d61eda2..eeefcf47f 100644
--- a/certbot/certbot/plugins/common.py
+++ b/certbot/certbot/plugins/common.py
@@ -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):
diff --git a/certbot/certbot/plugins/dns_common.py b/certbot/certbot/plugins/dns_common.py
index c6b881816..03f43f1cc 100644
--- a/certbot/certbot/plugins/dns_common.py
+++ b/certbot/certbot/plugins/dns_common.py
@@ -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
diff --git a/certbot/tests/plugins/common_test.py b/certbot/tests/plugins/common_test.py
index bcd190e9b..65067ae5e 100644
--- a/certbot/tests/plugins/common_test.py
+++ b/certbot/tests/plugins/common_test.py
@@ -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."""
diff --git a/certbot/tests/plugins/dns_common_test.py b/certbot/tests/plugins/dns_common_test.py
index 993f3b461..4b898bfe1 100644
--- a/certbot/tests/plugins/dns_common_test.py
+++ b/certbot/tests/plugins/dns_common_test.py
@@ -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):
diff --git a/certbot/tests/plugins/manual_test.py b/certbot/tests/plugins/manual_test.py
index 7318783fd..d1b7b768c 100644
--- a/certbot/tests/plugins/manual_test.py
+++ b/certbot/tests/plugins/manual_test.py
@@ -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

From 53151133afdbae76fdef0f319e2fea8d3c5c0b08 Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Sat, 23 Jan 2021 22:07:50 +1100
Subject: [PATCH 06/15] exclude static auth hints from coverage

---
 certbot-apache/certbot_apache/_internal/configurator.py | 2 +-
 certbot-nginx/certbot_nginx/_internal/configurator.py   | 2 +-
 certbot/certbot/_internal/plugins/webroot.py            | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py
index c1d2c7417..f5e029fbc 100644
--- a/certbot-apache/certbot_apache/_internal/configurator.py
+++ b/certbot-apache/certbot_apache/_internal/configurator.py
@@ -2470,7 +2470,7 @@ class ApacheConfigurator(common.Installer):
                 version=".".join(str(i) for i in self.version))
         )
 
-    def auth_hint(self, failed_achalls):
+    def auth_hint(self, failed_achalls): # pragma: no cover
         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.")
diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py
index e0f72f2d4..d095ff31e 100644
--- a/certbot-nginx/certbot_nginx/_internal/configurator.py
+++ b/certbot-nginx/certbot_nginx/_internal/configurator.py
@@ -1029,7 +1029,7 @@ class NginxConfigurator(common.Installer):
                 version=".".join(str(i) for i in self.version))
         )
 
-    def auth_hint(self, failed_achalls):
+    def auth_hint(self, failed_achalls): # pragma: no cover
         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 "
diff --git a/certbot/certbot/_internal/plugins/webroot.py b/certbot/certbot/_internal/plugins/webroot.py
index 0f5ab5565..cbb3cacfc 100644
--- a/certbot/certbot/_internal/plugins/webroot.py
+++ b/certbot/certbot/_internal/plugins/webroot.py
@@ -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, failed_achalls):
+    def auth_hint(self, failed_achalls): # pragma: no cover
         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 "

From c5c53fff965960fb1226aee4ad7d7e68e2ba56b9 Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Tue, 26 Jan 2021 17:10:23 +1100
Subject: [PATCH 07/15] dont call auth_hint unless derived from .Plugin

---
 certbot/certbot/_internal/auth_handler.py |  3 ++-
 certbot/tests/auth_handler_test.py        | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py
index 8eae98752..9f55274ce 100644
--- a/certbot/certbot/_internal/auth_handler.py
+++ b/certbot/certbot/_internal/auth_handler.py
@@ -17,6 +17,7 @@ from certbot import errors
 from certbot import interfaces
 from certbot._internal import error_handler
 from certbot.display import util as display_util
+from certbot.plugins import common as plugin_common
 
 logger = logging.getLogger(__name__)
 
@@ -284,7 +285,7 @@ class AuthHandler(object):
         for _, achalls in sorted(problems.items(), key=lambda item: item[0]):
             msg.append(_generate_failed_chall_msg(achalls))
 
-        if failed_achalls:
+        if failed_achalls and isinstance(self.auth, plugin_common.Plugin):
             msg.append('\nHint: {}\n'.format(self.auth.auth_hint(failed_achalls)))
 
         display_util.notify("".join(msg))
diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py
index 8e927f0ae..b74fbfd3b 100644
--- a/certbot/tests/auth_handler_test.py
+++ b/certbot/tests/auth_handler_test.py
@@ -17,6 +17,7 @@ from certbot import achallenges
 from certbot import errors
 from certbot import interfaces
 from certbot import util
+from certbot.plugins import common as plugin_common
 from certbot.tests import acme_util
 from certbot.tests import util as test_util
 
@@ -482,7 +483,7 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
     def setUp(self):
         from certbot._internal.auth_handler import AuthHandler
 
-        self.mock_auth = mock.MagicMock(name="buzz")
+        self.mock_auth = mock.MagicMock(spec=plugin_common.Plugin, name="buzz")
         self.mock_auth.name = "buzz"
         self.mock_auth.auth_hint.return_value = "the buzz hint"
         self.handler = AuthHandler(self.mock_auth, mock.MagicMock(), mock.MagicMock(), [])
@@ -553,6 +554,18 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
             '\nHint: quuuuuux\n'
         )
 
+    @mock.patch('certbot._internal.auth_handler.display_util.notify')
+    def test_non_subclassed_authenticator(self, mock_notify):
+        """If authenticator not derived from common.Plugin, we shouldn't call .auth_hint"""
+        from certbot._internal.auth_handler import AuthHandler
+
+        self.mock_auth = mock.MagicMock(name="quuz")
+        self.mock_auth.name = "quuz"
+        self.mock_auth.auth_hint.side_effect = Exception
+        self.handler = AuthHandler(self.mock_auth, mock.MagicMock(), mock.MagicMock(), [])
+        self.handler._report_failed_authzrs([self.authzr1])
+        self.assertEqual(mock_notify.call_count, 1)
+
 
 def gen_auth_resp(chall_list):
     """Generate a dummy authorization response."""

From 9f5474526fa54c9ba5d4550dd3352f7f48c76b89 Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Tue, 26 Jan 2021 17:24:56 +1100
Subject: [PATCH 08/15] dns fallback hint: dont assume --dns-blah works

--dns-blah won't work for third-party plugins, they need to be specified
using --authenticator dns-blah.
---
 certbot/certbot/plugins/dns_common.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/certbot/certbot/plugins/dns_common.py b/certbot/certbot/plugins/dns_common.py
index 03f43f1cc..25c2b55e0 100644
--- a/certbot/certbot/plugins/dns_common.py
+++ b/certbot/certbot/plugins/dns_common.py
@@ -41,7 +41,7 @@ class DNSAuthenticator(common.Plugin):
         delay = self.conf('propagation-seconds')
         return (
             'The Certificate Authority failed to verify the DNS TXT records created by the '
-            '--{name} plugin. Ensure the above domains are hosted by this DNS provider, '
+            '{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 '')
         )

From b75b70fbb28ca5aa0c37429af3445e55ce99100c Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Tue, 26 Jan 2021 17:32:58 +1100
Subject: [PATCH 09/15] add code comments about the auth_hint interface

---
 certbot/certbot/_internal/auth_handler.py | 2 ++
 certbot/certbot/plugins/common.py         | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py
index 9f55274ce..7681c2896 100644
--- a/certbot/certbot/_internal/auth_handler.py
+++ b/certbot/certbot/_internal/auth_handler.py
@@ -285,6 +285,8 @@ class AuthHandler(object):
         for _, achalls in sorted(problems.items(), key=lambda item: item[0]):
             msg.append(_generate_failed_chall_msg(achalls))
 
+        # auth_hint can currently only be implemented by authenticators that subclass
+        # plugin_common.Plugin. Refer to comment on that function.
         if failed_achalls and isinstance(self.auth, plugin_common.Plugin):
             msg.append('\nHint: {}\n'.format(self.auth.auth_hint(failed_achalls)))
 
diff --git a/certbot/certbot/plugins/common.py b/certbot/certbot/plugins/common.py
index eeefcf47f..178658126 100644
--- a/certbot/certbot/plugins/common.py
+++ b/certbot/certbot/plugins/common.py
@@ -106,6 +106,10 @@ 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".
 
+        TODO: auth_hint belongs in IAuthenticator but can't be added until the next major
+        version of Certbot. For now, it lives in .Plugin and auth_handler will only call it
+        on authenticators that subclass .Plugin.
+
         :param list failed_achalls: List of one or more failed challenges
                                     (:class:`achallenges.AnnotatedChallenge` subclasses).
 

From 7ad723ba16386515faa2146da724ee8935853fd1 Mon Sep 17 00:00:00 2001
From: alexzorin <alex@zor.io>
Date: Thu, 11 Feb 2021 15:43:58 +1100
Subject: [PATCH 10/15] update code comment

Co-authored-by: ohemorange <ebportnoy@gmail.com>
---
 certbot/certbot/_internal/auth_handler.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py
index 7681c2896..0da7eb0b5 100644
--- a/certbot/certbot/_internal/auth_handler.py
+++ b/certbot/certbot/_internal/auth_handler.py
@@ -285,7 +285,7 @@ class AuthHandler(object):
         for _, achalls in sorted(problems.items(), key=lambda item: item[0]):
             msg.append(_generate_failed_chall_msg(achalls))
 
-        # auth_hint can currently only be implemented by authenticators that subclass
+        # auth_hint will only be called on authenticators that subclass
         # plugin_common.Plugin. Refer to comment on that function.
         if failed_achalls and isinstance(self.auth, plugin_common.Plugin):
             msg.append('\nHint: {}\n'.format(self.auth.auth_hint(failed_achalls)))

From 9705d712b5792a384e077731fd77529a22703b11 Mon Sep 17 00:00:00 2001
From: alexzorin <alex@zor.io>
Date: Thu, 11 Feb 2021 15:44:13 +1100
Subject: [PATCH 11/15] update code comment

Co-authored-by: ohemorange <ebportnoy@gmail.com>
---
 certbot/certbot/plugins/common.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/certbot/certbot/plugins/common.py b/certbot/certbot/plugins/common.py
index 178658126..ea7ec95e1 100644
--- a/certbot/certbot/plugins/common.py
+++ b/certbot/certbot/plugins/common.py
@@ -108,7 +108,7 @@ class Plugin(object):
 
         TODO: auth_hint belongs in IAuthenticator but can't be added until the next major
         version of Certbot. For now, it lives in .Plugin and auth_handler will only call it
-        on authenticators that subclass .Plugin.
+        on authenticators that subclass .Plugin. For now, inherit from `.Plugin` to implement and/or override the method.
 
         :param list failed_achalls: List of one or more failed challenges
                                     (:class:`achallenges.AnnotatedChallenge` subclasses).

From 18d865f0a2d49b953a42a71f14045b7a5d16788e Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Thu, 11 Feb 2021 18:45:54 +1100
Subject: [PATCH 12/15] fix lint

---
 certbot/certbot/plugins/common.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/certbot/certbot/plugins/common.py b/certbot/certbot/plugins/common.py
index ea7ec95e1..d3fa11f6a 100644
--- a/certbot/certbot/plugins/common.py
+++ b/certbot/certbot/plugins/common.py
@@ -108,7 +108,8 @@ class Plugin(object):
 
         TODO: auth_hint belongs in IAuthenticator but can't be added until the next major
         version of Certbot. For now, it lives in .Plugin and auth_handler will only call it
-        on authenticators that subclass .Plugin. For now, inherit from `.Plugin` to implement and/or override the method.
+        on authenticators that subclass .Plugin. For now, inherit from `.Plugin` to implement
+        and/or override the method.
 
         :param list failed_achalls: List of one or more failed challenges
                                     (:class:`achallenges.AnnotatedChallenge` subclasses).

From b353dc1832485381ab193893873692c6417a0100 Mon Sep 17 00:00:00 2001
From: alexzorin <alex@zor.io>
Date: Mon, 15 Feb 2021 17:23:04 +1100
Subject: [PATCH 13/15] text change in nginx hint

Co-authored-by: ohemorange <ebportnoy@gmail.com>
---
 certbot-nginx/certbot_nginx/_internal/configurator.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py
index d095ff31e..b2a66515e 100644
--- a/certbot-nginx/certbot_nginx/_internal/configurator.py
+++ b/certbot-nginx/certbot_nginx/_internal/configurator.py
@@ -1033,7 +1033,7 @@ class NginxConfigurator(common.Installer):
         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 "
-            "that it is accessible from the internet, or try increase --nginx-sleep-seconds."
+            "that it is accessible from the internet, or try increasing --nginx-sleep-seconds."
         )
 
     ###################################################

From 651cc7cd071b1551729a421cb3c2bb234b8dba90 Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Mon, 22 Feb 2021 16:43:42 +1100
Subject: [PATCH 14/15] type annotations

---
 certbot/certbot/_internal/auth_handler.py | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py
index 0da7eb0b5..9be04c82a 100644
--- a/certbot/certbot/_internal/auth_handler.py
+++ b/certbot/certbot/_internal/auth_handler.py
@@ -4,7 +4,6 @@ import logging
 import time
 
 import zope.component
-from josepy.jwk import JWK # pylint: disable=unused-import
 
 from acme import challenges
 from acme import errors as acme_errors
@@ -268,10 +267,9 @@ class AuthHandler(object):
 
         return achalls
 
-    def _report_failed_authzrs(self, failed_authzrs):
-        # type: (List[messages.AuthorizationResource]) -> None
+    def _report_failed_authzrs(self, failed_authzrs: List[messages.AuthorizationResource]) -> None:
         """Notifies the user about failed authorizations."""
-        problems = {}  # type: Dict[str, List[achallenges.AnnotatedChallenge]]
+        problems: Dict[str, List[achallenges.AnnotatedChallenge]] = {}
         failed_achalls = [challb_to_achall(challb, self.account.key, authzr.body.identifier.value)
                         for authzr in failed_authzrs for challb in authzr.body.challenges
                         if challb.error]

From a942c8112e405e8050191c88209476d03ab613f3 Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Thu, 4 Mar 2021 22:26:24 +1100
Subject: [PATCH 15/15] copy: avoid "plugin" where possible

---
 certbot-apache/certbot_apache/_internal/configurator.py | 4 ++--
 certbot-nginx/certbot_nginx/_internal/configurator.py   | 4 ++--
 certbot/certbot/_internal/auth_handler.py               | 6 +++---
 certbot/certbot/_internal/plugins/manual.py             | 2 +-
 certbot/certbot/_internal/plugins/webroot.py            | 6 +++---
 certbot/certbot/plugins/dns_common.py                   | 6 +++---
 certbot/tests/auth_handler_test.py                      | 4 ++--
 certbot/tests/plugins/manual_test.py                    | 4 ++--
 8 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py
index f5e029fbc..538707b46 100644
--- a/certbot-apache/certbot_apache/_internal/configurator.py
+++ b/certbot-apache/certbot_apache/_internal/configurator.py
@@ -2472,8 +2472,8 @@ class ApacheConfigurator(common.Installer):
 
     def auth_hint(self, failed_achalls): # pragma: no cover
         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.")
+                "changes made by Certbot. Ensure that the listed domains point to this Apache "
+                "server and that it is accessible from the internet.")
 
     ###########################################################################
     # Challenges Section
diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py
index b2a66515e..108a9770f 100644
--- a/certbot-nginx/certbot_nginx/_internal/configurator.py
+++ b/certbot-nginx/certbot_nginx/_internal/configurator.py
@@ -1032,8 +1032,8 @@ class NginxConfigurator(common.Installer):
     def auth_hint(self, failed_achalls): # pragma: no cover
         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 "
-            "that it is accessible from the internet, or try increasing --nginx-sleep-seconds."
+            "made by Certbot. Ensure the listed domains point to this nginx server and that it is "
+            "accessible from the internet."
         )
 
     ###################################################
diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py
index 9be04c82a..fcfd66d47 100644
--- a/certbot/certbot/_internal/auth_handler.py
+++ b/certbot/certbot/_internal/auth_handler.py
@@ -277,8 +277,8 @@ class AuthHandler(object):
         for achall in failed_achalls:
             problems.setdefault(achall.error.typ, []).append(achall)
 
-        msg = ["\nCertbot failed to authenticate some domains (using the {} plugin)."
-            " The Certificate Authority reported these problems:".format(self.auth.name)]
+        msg = [f"\nCertbot failed to authenticate some domains (authenticator: {self.auth.name})."
+            " The Certificate Authority reported these problems:"]
 
         for _, achalls in sorted(problems.items(), key=lambda item: item[0]):
             msg.append(_generate_failed_chall_msg(achalls))
@@ -286,7 +286,7 @@ class AuthHandler(object):
         # auth_hint will only be called on authenticators that subclass
         # plugin_common.Plugin. Refer to comment on that function.
         if failed_achalls and isinstance(self.auth, plugin_common.Plugin):
-            msg.append('\nHint: {}\n'.format(self.auth.auth_hint(failed_achalls)))
+            msg.append(f"\nHint: {self.auth.auth_hint(failed_achalls)}\n")
 
         display_util.notify("".join(msg))
 
diff --git a/certbot/certbot/_internal/plugins/manual.py b/certbot/certbot/_internal/plugins/manual.py
index 2c632e8b1..7e6aa6fba 100644
--- a/certbot/certbot/_internal/plugins/manual.py
+++ b/certbot/certbot/_internal/plugins/manual.py
@@ -124,7 +124,7 @@ permitted by DNS standards.)
             return (
                 '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" and the Certbot User Guide.'
                 .format(
                     certbot=cli_constants.cli_command,
                     resources=resources,
diff --git a/certbot/certbot/_internal/plugins/webroot.py b/certbot/certbot/_internal/plugins/webroot.py
index cbb3cacfc..f383ca3bb 100644
--- a/certbot/certbot/_internal/plugins/webroot.py
+++ b/certbot/certbot/_internal/plugins/webroot.py
@@ -64,9 +64,9 @@ to serve all files under specified web root ({0})."""
 
     def auth_hint(self, failed_achalls): # pragma: no cover
         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 "
-                "can be downloaded from the internet.")
+                "created by Certbot. Ensure that the listed domains serve their content from "
+                "the provided --webroot-path/-w and that files created there can be downloaded "
+                "from the internet.")
 
     def get_chall_pref(self, domain):  # pragma: no cover
         # pylint: disable=unused-argument,missing-function-docstring
diff --git a/certbot/certbot/plugins/dns_common.py b/certbot/certbot/plugins/dns_common.py
index 25c2b55e0..7cbe121b3 100644
--- a/certbot/certbot/plugins/dns_common.py
+++ b/certbot/certbot/plugins/dns_common.py
@@ -40,9 +40,9 @@ class DNSAuthenticator(common.Plugin):
     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 the above domains are hosted by this DNS provider, '
-            'or try increasing --{name}-propagation-seconds (currently {secs} second{suffix}).'
+            'The Certificate Authority failed to verify the DNS TXT records created by --{name}. '
+            '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 '')
         )
 
diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py
index b74fbfd3b..a207f4dc9 100644
--- a/certbot/tests/auth_handler_test.py
+++ b/certbot/tests/auth_handler_test.py
@@ -519,7 +519,7 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
         self.handler._report_failed_authzrs([self.authzr1])
         mock_notify.assert_called_with(
             '\n'
-            'Certbot failed to authenticate some domains (using the buzz plugin). '
+            'Certbot failed to authenticate some domains (authenticator: buzz). '
             'The Certificate Authority reported these problems:\n'
             '  Domain: example.com\n'
             '  Type:   tls\n'
@@ -538,7 +538,7 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
         self.handler._report_failed_authzrs([self.authzr1, self.authzr2])
         mock_notify.assert_called_with(
             '\n'
-            'Certbot failed to authenticate some domains (using the quux plugin). '
+            'Certbot failed to authenticate some domains (authenticator: quux). '
             'The Certificate Authority reported these problems:\n'
             '  Domain: foo.bar\n'
             '  Type:   dnssec\n'
diff --git a/certbot/tests/plugins/manual_test.py b/certbot/tests/plugins/manual_test.py
index d1b7b768c..7c00daaf3 100644
--- a/certbot/tests/plugins/manual_test.py
+++ b/certbot/tests/plugins/manual_test.py
@@ -129,13 +129,13 @@ class AuthenticatorTest(test_util.TempDirTestCase):
             '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".'
+            'Refer to "certbot --help manual" and the Certbot User Guide.'
         )
         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".'
+            '"certbot --help manual" and the Certbot User Guide.'
         )
 
     def test_auth_hint_no_hook(self):