Revert "Implement POST-as-GET requests (#6522)"

This reverts commit 0b5468e992.

acme/acme/client.py

@@ -200,6 +200,22 @@ class ClientBase(object):  # pylint: disable=too-many-instance-attributes
 
         return datetime.datetime.now() + datetime.timedelta(seconds=seconds)
 
+    def poll(self, authzr):
+        """Poll Authorization Resource for status.
+
+        :param authzr: Authorization Resource
+        :type authzr: `.AuthorizationResource`
+
+        :returns: Updated Authorization Resource and HTTP response.
+
+        :rtype: (`.AuthorizationResource`, `requests.Response`)
+
+        """
+        response = self.net.get(authzr.uri)
+        updated_authzr = self._authzr_from_response(
+            response, authzr.body.identifier, authzr.uri)
+        return updated_authzr, response
+
     def _revoke(self, cert, rsn, url):
         """Revoke certificate.
 
@@ -221,7 +237,6 @@ class ClientBase(object):  # pylint: disable=too-many-instance-attributes
             raise errors.ClientError(
                 'Successful revocation must return HTTP OK status')
 
-
 class Client(ClientBase):
     """ACME client for a v1 API.
 
@@ -374,22 +389,6 @@ class Client(ClientBase):
             body=jose.ComparableX509(OpenSSL.crypto.load_certificate(
                 OpenSSL.crypto.FILETYPE_ASN1, response.content)))
 
-    def poll(self, authzr):
-        """Poll Authorization Resource for status.
-
-        :param authzr: Authorization Resource
-        :type authzr: `.AuthorizationResource`
-
-        :returns: Updated Authorization Resource and HTTP response.
-
-        :rtype: (`.AuthorizationResource`, `requests.Response`)
-
-        """
-        response = self.net.get(authzr.uri)
-        updated_authzr = self._authzr_from_response(
-            response, authzr.body.identifier, authzr.uri)
-        return updated_authzr, response
-
     def poll_and_request_issuance(
             self, csr, authzrs, mintime=5, max_attempts=10):
         """Poll and request issuance.
@@ -653,29 +652,13 @@ class ClientV2(ClientBase):
         body = messages.Order.from_json(response.json())
         authorizations = []
         for url in body.authorizations:
-            authorizations.append(self._authzr_from_response(self._post_as_get(url), uri=url))
+            authorizations.append(self._authzr_from_response(self.net.get(url), uri=url))
         return messages.OrderResource(
             body=body,
             uri=response.headers.get('Location'),
             authorizations=authorizations,
             csr_pem=csr_pem)
 
-    def poll(self, authzr):
-        """Poll Authorization Resource for status.
-
-        :param authzr: Authorization Resource
-        :type authzr: `.AuthorizationResource`
-
-        :returns: Updated Authorization Resource and HTTP response.
-
-        :rtype: (`.AuthorizationResource`, `requests.Response`)
-
-        """
-        response = self._post_as_get(authzr.uri)
-        updated_authzr = self._authzr_from_response(
-            response, authzr.body.identifier, authzr.uri)
-        return updated_authzr, response
-
     def poll_and_finalize(self, orderr, deadline=None):
         """Poll authorizations and finalize the order.
 
@@ -699,7 +682,7 @@ class ClientV2(ClientBase):
         responses = []
         for url in orderr.body.authorizations:
             while datetime.datetime.now() < deadline:
-                authzr = self._authzr_from_response(self._post_as_get(url), uri=url)
+                authzr = self._authzr_from_response(self.net.get(url), uri=url)
                 if authzr.body.status != messages.STATUS_PENDING:
                     responses.append(authzr)
                     break
@@ -734,12 +717,12 @@ class ClientV2(ClientBase):
         self._post(orderr.body.finalize, wrapped_csr)
         while datetime.datetime.now() < deadline:
             time.sleep(1)
-            response = self._post_as_get(orderr.uri)
+            response = self.net.get(orderr.uri)
             body = messages.Order.from_json(response.json())
             if body.error is not None:
                 raise errors.IssuanceError(body.error)
             if body.certificate is not None:
-                certificate_response = self._post_as_get(body.certificate,
+                certificate_response = self.net.get(body.certificate,
                                                     content_type=DER_CONTENT_TYPE).text
                 return orderr.update(body=body, fullchain_pem=certificate_response)
         raise errors.TimeoutError()
@@ -757,32 +740,6 @@ class ClientV2(ClientBase):
         """
         return self._revoke(cert, rsn, self.directory['revokeCert'])
 
-    def _post_as_get(self, *args, **kwargs):
-        """
-        Send GET request using the POST-as-GET protocol if needed.
-        The request will be first issued using POST-as-GET for ACME v2. If the ACME CA servers do
-        not support this yet and return an error, request will be retried using GET.
-        For ACME v1, only GET request will be tried, as POST-as-GET is not supported.
-        :param args:
-        :param kwargs:
-        :return:
-        """
-        if self.acme_version >= 2:
-            # We add an empty payload for POST-as-GET requests
-            new_args = args[:1] + (None,) + args[1:]
-            try:
-                return self._post(*new_args, **kwargs)  # pylint: disable=star-args
-            except messages.Error as error:
-                if error.code == 'malformed':
-                    logger.debug('Error during a POST-as-GET request, '
-                                 'your ACME CA may not support it:\n%s', error)
-                    logger.debug('Retrying request with GET.')
-                else:  # pragma: no cover
-                    raise
-
-        # If POST-as-GET is not supported yet, we use a GET instead.
-        return self.net.get(*args, **kwargs)
-
 
 class BackwardsCompatibleClientV2(object):
     """ACME client wrapper that tends towards V2-style calls, but
@@ -812,7 +769,12 @@ class BackwardsCompatibleClientV2(object):
             self.client = ClientV2(directory, net=net)
 
     def __getattr__(self, name):
+        if name in vars(self.client):
             return getattr(self.client, name)
+        elif name in dir(ClientBase):
+            return getattr(self.client, name)
+        else:
+            raise AttributeError()
 
     def new_account_and_tos(self, regr, check_tos_cb=None):
         """Combined register and agree_tos for V1, new_account for V2
@@ -982,7 +944,7 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
         :rtype: `josepy.JWS`
 
         """
-        jobj = obj.json_dumps(indent=2).encode() if obj else b''
+        jobj = obj.json_dumps(indent=2).encode()
         logger.debug('JWS payload:\n%s', jobj)
         kwargs = {
             "alg": self.alg,

acme/acme/client_test.py

@@ -1,5 +1,4 @@
 """Tests for acme.client."""
-# pylint: disable=too-many-lines
 import copy
 import datetime
 import json
@@ -731,9 +730,8 @@ class ClientV2Test(ClientTestBase):
         authz_response2 = self.response
         authz_response2.json.return_value = self.authz2.to_json()
         authz_response2.headers['Location'] = self.authzr2.uri
+        self.net.get.side_effect = (authz_response, authz_response2)
 
-        with mock.patch('acme.client.ClientV2._post_as_get') as mock_post_as_get:
-            mock_post_as_get.side_effect = (authz_response, authz_response2)
         self.assertEqual(self.client.new_order(CSR_SAN_PEM), self.orderr)
 
     @mock.patch('acme.client.datetime')
@@ -823,30 +821,6 @@ class ClientV2Test(ClientTestBase):
         self.response.json.return_value = self.regr.body.update(
             contact=()).to_json()
 
-    def test_post_as_get(self):
-        with mock.patch('acme.client.ClientV2._authzr_from_response') as mock_client:
-            mock_client.return_value = self.authzr2
-
-            self.client.poll(self.authzr2)  # pylint: disable=protected-access
-
-            self.client.net.post.assert_called_once_with(
-                self.authzr2.uri, None, acme_version=2,
-                new_nonce_url='https://www.letsencrypt-demo.org/acme/new-nonce')
-            self.client.net.get.assert_not_called()
-
-            class FakeError(messages.Error):  # pylint: disable=too-many-ancestors
-                """Fake error to reproduce a malformed request ACME error"""
-                def __init__(self):  # pylint: disable=super-init-not-called
-                    pass
-                @property
-                def code(self):
-                    return 'malformed'
-            self.client.net.post.side_effect = FakeError()
-
-            self.client.poll(self.authzr2)  # pylint: disable=protected-access
-
-            self.client.net.get.assert_called_once_with(self.authzr2.uri)
-
 
 class MockJSONDeSerializable(jose.JSONDeSerializable):
     # pylint: disable=missing-docstring

tests/certbot-pebble-integration.sh

@@ -1,16 +0,0 @@
-#!/bin/bash
-# Simple integration test. Make sure to activate virtualenv beforehand
-# (source venv/bin/activate) and that you are running Pebble test
-# instance (see ./pebble-fetch.sh).
-
-cleanup_and_exit() {
-    EXIT_STATUS=$?
-    unset SERVER
-    exit $EXIT_STATUS
-}
-
-trap cleanup_and_exit EXIT
-
-export SERVER=https://localhost:14000/dir
-
-./tests/certbot-boulder-integration.sh

tests/pebble-fetch.sh

@@ -1,41 +0,0 @@
-#!/bin/bash
-# Download and run Pebble instance for integration testing
-set -xe
-
-PEBBLE_VERSION=2018-11-02
-
-# We reuse the same GOPATH-style directory than for Boulder.
-# Pebble does not need it, but it will make the installation consistent with Boulder's one.
-export GOPATH=${GOPATH:-$HOME/gopath}
-PEBBLEPATH=${PEBBLEPATH:-$GOPATH/src/github.com/letsencrypt/pebble}
-
-mkdir -p ${PEBBLEPATH}
-
-cat << UNLIKELY_EOF > "$PEBBLEPATH/docker-compose.yml"
-version: '3'
-
-services:
- pebble:
-  image: letsencrypt/pebble:${PEBBLE_VERSION}
-  command: pebble -strict ${PEBBLE_STRICT:-false} -dnsserver 10.77.77.1
-  ports:
-    - 14000:14000
-  environment:
-    - PEBBLE_VA_NOSLEEP=1
-UNLIKELY_EOF
-
-docker-compose -f "$PEBBLEPATH/docker-compose.yml" up -d pebble
-
-set +x  # reduce verbosity while waiting for boulder
-for n in `seq 1 150` ; do
-  if curl -k https://localhost:14000/dir 2>/dev/null; then
-    break
-  else
-    sleep 1
-  fi
-done
-
-if ! curl -k https://localhost:14000/dir 2>/dev/null; then
-  echo "timed out waiting for pebble to start"
-  exit 1
-fi