[Windows|Unix] Avoid to re-execute challenges already validated
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 72482035667..07d00e3face 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,7 +6,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Added
 
-*
+* Avoid to process again challenges that are already validated
+  when a certificate is issued.
 
 ### Changed
 
@@ -22,6 +23,7 @@ all Certbot components during releases for the time being, however, the only
 package with changes other than its version number was:
 
 * acme
+* certbot
 
 More details about these changes can be found on our GitHub repo.
 
diff --git a/certbot-nginx/tests/boulder-integration.conf.sh b/certbot-nginx/tests/boulder-integration.conf.sh
index 4374f9094d6..470eab28e26 100755
--- a/certbot-nginx/tests/boulder-integration.conf.sh
+++ b/certbot-nginx/tests/boulder-integration.conf.sh
@@ -1,3 +1,4 @@
+#!/usr/bin/env bash
 # Based on
 # https://www.exratione.com/2014/03/running-nginx-as-a-non-root-user/
 # https://github.com/exratione/non-root-nginx/blob/9a77f62e5d5cb9c9026fd62eece76b9514011019/nginx.conf
@@ -52,7 +53,7 @@ http {
     listen 5002 $default_server;
     # IPv6.
     listen [::]:5002 $default_server;
-    server_name nginx.wtf nginx2.wtf;
+    server_name nginx.wtf nginx-tls.wtf nginx2.wtf;
 
     root $root/webroot;
 
diff --git a/certbot-nginx/tests/boulder-integration.sh b/certbot-nginx/tests/boulder-integration.sh
index 2a24e645fba..03425734d03 100755
--- a/certbot-nginx/tests/boulder-integration.sh
+++ b/certbot-nginx/tests/boulder-integration.sh
@@ -39,8 +39,8 @@ nginx -v
 reload_nginx
 certbot_test_nginx --domains nginx.wtf run
 test_deployment_and_rollback nginx.wtf
-certbot_test_nginx --domains nginx.wtf run --preferred-challenges tls-sni
-test_deployment_and_rollback nginx.wtf
+certbot_test_nginx --domains nginx-tls.wtf run --preferred-challenges tls-sni
+test_deployment_and_rollback nginx-tls.wtf
 certbot_test_nginx --domains nginx2.wtf --preferred-challenges http
 test_deployment_and_rollback nginx2.wtf
 # Overlapping location block and server-block-level return 301
diff --git a/certbot/auth_handler.py b/certbot/auth_handler.py
index efee4914363..3dfaaf26f4e 100644
--- a/certbot/auth_handler.py
+++ b/certbot/auth_handler.py
@@ -31,7 +31,7 @@ class AuthHandler(object):
         :class:`~acme.challenges.Challenge` types
     :type auth: :class:`certbot.interfaces.IAuthenticator`
 
-    :ivar acme.client.BackwardsCompatibleClientV2 acme: ACME client API.
+    :ivar acme.client.BackwardsCompatibleClientV2 acme_client: ACME client API.
 
     :ivar account: Client's Account
     :type account: :class:`certbot.account.Account`
@@ -40,9 +40,9 @@ class AuthHandler(object):
         type strings with the most preferred challenge listed first
 
     """
-    def __init__(self, auth, acme, account, pref_challs):
+    def __init__(self, auth, acme_client, account, pref_challs):
         self.auth = auth
-        self.acme = acme
+        self.acme = acme_client
 
         self.account = account
         self.pref_challs = pref_challs
@@ -85,19 +85,26 @@ def handle_authorizations(self, orderr, best_effort=False):
         self.verify_authzr_complete(aauthzrs)
 
         # Only return valid authorizations
-        retVal = [aauthzr.authzr for aauthzr in aauthzrs
-                  if aauthzr.authzr.body.status == messages.STATUS_VALID]
+        ret_val = [aauthzr.authzr for aauthzr in aauthzrs
+                   if aauthzr.authzr.body.status == messages.STATUS_VALID]
 
-        if not retVal:
+        if not ret_val:
             raise errors.AuthorizationError(
                 "Challenges failed for all domains")
 
-        return retVal
+        return ret_val
 
     def _choose_challenges(self, aauthzrs):
-        """Retrieve necessary challenges to satisfy server."""
-        logger.info("Performing the following challenges:")
-        for aauthzr in aauthzrs:
+        """
+        Retrieve necessary and pending challenges to satisfy server.
+        NB: Necessary and already validated challenges are not retrieved,
+        as they can be reused for a certificate issuance.
+        """
+        pending_authzrs = [aauthzr for aauthzr in aauthzrs
+                           if aauthzr.authzr.body.status != messages.STATUS_VALID]
+        if pending_authzrs:
+            logger.info("Performing the following challenges:")
+        for aauthzr in pending_authzrs:
             aauthzr_challenges = aauthzr.authzr.body.challenges
             if self.acme.acme_version == 1:
                 combinations = aauthzr.authzr.body.combinations
@@ -125,7 +132,7 @@ def _has_challenges(self, aauthzrs):
 
     def _solve_challenges(self, aauthzrs):
         """Get Responses for challenges from authenticators."""
-        resp = []  # type: Collection[acme.challenges.ChallengeResponse]
+        resp = []  # type: Collection[challenges.ChallengeResponse]
         all_achalls = self._get_all_achalls(aauthzrs)
         try:
             if all_achalls:
@@ -531,7 +538,7 @@ def _report_failed_challs(failed_achalls):
 
     """
     problems = collections.defaultdict(list)\
-    # type: DefaultDict[str, List[achallenges.KeyAuthorizationAnnotatedChallenge]]
+        # type: DefaultDict[str, List[achallenges.KeyAuthorizationAnnotatedChallenge]]
     for achall in failed_achalls:
         if achall.error:
             problems[achall.error.typ].append(achall)
diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py
index e1319b6146e..fe0ece12e9f 100644
--- a/certbot/tests/auth_handler_test.py
+++ b/certbot/tests/auth_handler_test.py
@@ -57,7 +57,7 @@ def test_unrecognized(self):
              errors.Error, self.handler._challenge_factory, authzr, [0])
 
 
-class HandleAuthorizationsTest(unittest.TestCase):
+class HandleAuthorizationsTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
     """handle_authorizations test.
 
     This tests everything except for all functions under _poll_challenges.
@@ -316,6 +316,24 @@ def test_incomplete_authzr_error(self, mock_verify, mock_poll):
         self.assertEqual(
             self.mock_auth.cleanup.call_args[0][0][0].typ, "tls-sni-01")
 
+    def test_validated_challenge_not_rerun(self):
+        # With pending challenge, we expect the challenge to be tried, and fail.
+        authzr = acme_util.gen_authzr(
+                messages.STATUS_PENDING, "0",
+                [acme_util.HTTP01],
+                [messages.STATUS_PENDING], False)
+        mock_order = mock.MagicMock(authorizations=[authzr])
+        self.assertRaises(
+            errors.AuthorizationError, self.handler.handle_authorizations, mock_order)
+
+        # With validated challenge; we expect the challenge not be tried again, and succeed.
+        authzr = acme_util.gen_authzr(
+                messages.STATUS_VALID, "0",
+                [acme_util.HTTP01],
+                [messages.STATUS_VALID], False)
+        mock_order = mock.MagicMock(authorizations=[authzr])
+        self.handler.handle_authorizations(mock_order)
+
     def _validate_all(self, aauthzrs, unused_1, unused_2):
         for i, aauthzr in enumerate(aauthzrs):
             azr = aauthzr.authzr

In response to #5342.

Currently, certbot will execute the operations necessary to validate a challenge even if the challenge has already been validated before against the acme ca server. This can occur for instance if a certificate is asked and issue correctly, then deleted locally, then asked again.

It is a corner case, but it will lead to some heavy operations (like updating a DNS zone, or creating an HTTP server) that are not needed.

This PR corrects this behavior by not executing challenges already validated, and use them directly instead to issue the certificate.

Fixes #5342