Hook more events into presign functionality

.changes/next-release/bugfix-Presign-62691.json

@@ -0,0 +1,5 @@
+{
+  "category": "Presign", 
+  "type": "bugfix", 
+  "description": "Fix issue where some events were not fired during the presigning of a request thus not including a variety of customizations (`#1340 <https://github.com/boto/botocore/issues/1340>`__)"
+}

botocore/client.py

@@ -139,7 +139,7 @@ def _register_s3_events(self, client, endpoint_bridge, endpoint_url,
             # Also make sure that the hostname gets switched to
             # s3-accelerate.amazonaws.com
             client.meta.events.register_first(
-                'request-created.s3', switch_host_s3_accelerate)
+                'before-sign.s3', switch_host_s3_accelerate)
 
         self._set_s3_presign_signature_version(
             client.meta, client_config, scoped_config)
@@ -618,6 +618,16 @@ def _make_api_call(self, operation_name, api_params):
 
     def _convert_to_request_dict(self, api_params, operation_model,
                                  context=None):
+        api_params = self._emit_api_params(
+            api_params, operation_model, context)
+        request_dict = self._serializer.serialize_to_request(
+            api_params, operation_model)
+        prepare_request_dict(request_dict, endpoint_url=self._endpoint.host,
+                             user_agent=self._client_config.user_agent,
+                             context=context)
+        return request_dict
+
+    def _emit_api_params(self, api_params, operation_model, context):
         # Given the API params provided by the user and the operation_model
         # we can serialize the request to a request_dict.
         operation_name = operation_model.name
@@ -639,13 +649,7 @@ def _convert_to_request_dict(self, api_params, operation_model,
                 endpoint_prefix=self._service_model.endpoint_prefix,
                 operation_name=operation_name),
             params=api_params, model=operation_model, context=context)
-
-        request_dict = self._serializer.serialize_to_request(
-            api_params, operation_model)
-        prepare_request_dict(request_dict, endpoint_url=self._endpoint.host,
-                             user_agent=self._client_config.user_agent,
-                             context=context)
-        return request_dict
+        return api_params
 
     def get_paginator(self, operation_name):
         """Create a paginator for an operation.

botocore/signers.py

@@ -131,7 +131,9 @@ def sign(self, operation_name, request, region_name=None,
             'before-sign.{0}.{1}'.format(self._service_name, operation_name),
             request=request, signing_name=signing_name,
             region_name=self._region_name,
-            signature_version=signature_version, request_signer=self)
+            signature_version=signature_version, request_signer=self,
+            operation_name=operation_name
+        )
 
         if signature_version != botocore.UNSIGNED:
             kwargs = {
@@ -551,8 +553,13 @@ def generate_presigned_url(self, ClientMethod, Params=None, ExpiresIn=3600,
     """
     client_method = ClientMethod
     params = Params
+    if params is None:
+        params = {}
     expires_in = ExpiresIn
     http_method = HttpMethod
+    context = {
+        'is_presign_request': True
+    }
 
     request_signer = self._request_signer
     serializer = self._serializer
@@ -565,6 +572,8 @@ def generate_presigned_url(self, ClientMethod, Params=None, ExpiresIn=3600,
     operation_model = self.meta.service_model.operation_model(
         operation_name)
 
+    params = self._emit_api_params(params, operation_model, context)
+
     # Create a request dict based on the params to serialize.
     request_dict = serializer.serialize_to_request(
         params, operation_model)
@@ -575,8 +584,7 @@ def generate_presigned_url(self, ClientMethod, Params=None, ExpiresIn=3600,
 
     # Prepare the request dict by including the client's endpoint url.
     prepare_request_dict(
-        request_dict, endpoint_url=self.meta.endpoint_url
-    )
+        request_dict, endpoint_url=self.meta.endpoint_url, context=context)
 
     # Generate the presigned url.
     return request_signer.generate_presigned_url(

botocore/stub.py

@@ -331,12 +331,14 @@ def _assert_expected_call_order(self, model, params):
                 operation_name=model.name,
                 reason='Operation mismatch: found response for %s.' % name)
 
-    def _get_response_handler(self, model, params, **kwargs):
+    def _get_response_handler(self, model, params, context, **kwargs):
         self._assert_expected_call_order(model, params)
         # Pop off the entire response once everything has been validated
         return self._queue.popleft()['response']
 
-    def _assert_expected_params(self, model, params, **kwargs):
+    def _assert_expected_params(self, model, params, context, **kwargs):
+        if self._should_not_stub(context):
+            return
         self._assert_expected_call_order(model, params)
         expected_params = self._queue[0]['expected_params']
         if expected_params is None:
@@ -357,6 +359,13 @@ def _assert_expected_params(self, model, params, **kwargs):
                 reason='Expected parameters:\n%s,\nbut received:\n%s' % (
                     pformat(expected_params), pformat(params)))
 
+    def _should_not_stub(self, context):
+        # Do not include presign requests when processing stubbed client calls
+        # as a presign request will never have an HTTP request sent over the
+        # wire for it and therefore not receive a response back.
+        if context and context.get('is_presign_request'):
+            return True
+
     def _validate_response(self, operation_name, service_response):
         service_model = self.client.meta.service_model
         operation_model = service_model.operation_model(operation_name)

tests/functional/test_s3.py

@@ -350,6 +350,49 @@ def test_presign_unsigned(self):
         self.assertEqual(
             'https://s3.us-east-2.amazonaws.com/', url)
 
+    def test_presign_url_with_ssec(self):
+        config = Config(signature_version='s3')
+        client = self.session.create_client('s3', 'us-east-1', config=config)
+        url = client.generate_presigned_url(
+            ClientMethod='get_object',
+            Params={
+                'Bucket': 'mybucket',
+                'Key': 'mykey',
+                'SSECustomerKey': 'a' * 32,
+                'SSECustomerAlgorithm': 'AES256'
+            }
+        )
+        # The md5 of the sse-c key will be injected when parameters are
+        # built so it should show up in the presigned url as well.
+        self.assertIn(
+            'x-amz-server-side-encryption-customer-key-md5=', url
+        )
+
+    def test_presign_s3_accelerate(self):
+        config = Config(signature_version=botocore.UNSIGNED,
+                        s3={'use_accelerate_endpoint': True})
+        client = self.session.create_client('s3', 'us-east-1', config=config)
+        url = client.generate_presigned_url(
+            ClientMethod='get_object',
+            Params={'Bucket': 'mybucket', 'Key': 'mykey'}
+        )
+        # The url should be the accelerate endpoint
+        self.assertEqual(
+            'https://mybucket.s3-accelerate.amazonaws.com/mykey', url)
+
+    def test_presign_post_s3_accelerate(self):
+        config = Config(signature_version=botocore.UNSIGNED,
+                        s3={'use_accelerate_endpoint': True})
+        client = self.session.create_client('s3', 'us-east-1', config=config)
+        parts = client.generate_presigned_post(
+            Bucket='mybucket', Key='mykey')
+        # The url should be the accelerate endpoint
+        expected = {
+            'fields': {'key': 'mykey'},
+            'url': 'https://mybucket.s3-accelerate.amazonaws.com/'
+        }
+        self.assertEqual(parts, expected)
+
 
 def test_correct_url_used_for_s3():
     # Test that given various sets of config options and bucket names,

tests/functional/test_stub.py

@@ -28,9 +28,12 @@
 class TestStubber(unittest.TestCase):
     def setUp(self):
         session = botocore.session.get_session()
-        config = botocore.config.Config(signature_version=botocore.UNSIGNED)
-        self.client = session.create_client('s3', config=config)
-
+        config = botocore.config.Config(
+            signature_version=botocore.UNSIGNED,
+            s3={'addressing_style': 'path'}
+        )
+        self.client = session.create_client(
+            's3', region_name='us-east-1', config=config)
         self.stubber = Stubber(self.client)
 
     def test_stubber_returns_response(self):
@@ -270,3 +273,43 @@ def test_many_expected_params(self):
         except StubAssertionError:
             self.fail(
                 "Stubber inappropriately raised error for same parameters.")
+
+    def test_no_stub_for_presign_url(self):
+        try:
+            with self.stubber:
+                url = self.client.generate_presigned_url(
+                    ClientMethod='get_object',
+                    Params={
+                        'Bucket': 'mybucket',
+                        'Key': 'mykey'
+                    }
+                )
+                self.assertEqual(
+                    url, 'https://s3.amazonaws.com/mybucket/mykey')
+        except StubResponseError:
+            self.fail(
+                'Stubbed responses should not be required for generating '
+                'presigned requests'
+            )
+
+    def test_can_stub_with_presign_url_mixed_in(self):
+        desired_response = {}
+        expected_params = {
+            'Bucket': 'mybucket',
+            'Prefix': 'myprefix',
+        }
+        self.stubber.add_response(
+            'list_objects', desired_response, expected_params)
+        with self.stubber:
+            url = self.client.generate_presigned_url(
+                ClientMethod='get_object',
+                Params={
+                    'Bucket': 'myotherbucket',
+                    'Key': 'myotherkey'
+                }
+            )
+            self.assertEqual(
+                    url, 'https://s3.amazonaws.com/myotherbucket/myotherkey')
+            actual_response = self.client.list_objects(**expected_params)
+            self.assertEqual(desired_response, actual_response)
+        self.stubber.assert_no_pending_responses()

tests/unit/test_signers.py

@@ -22,6 +22,7 @@
 from botocore.config import Config
 from botocore.credentials import Credentials
 from botocore.credentials import ReadOnlyCredentials
+from botocore.hooks import HierarchicalEmitter
 from botocore.exceptions import NoRegionError, UnknownSignatureVersionError
 from botocore.exceptions import UnknownClientMethodError, ParamValidationError
 from botocore.exceptions import UnsupportedSignatureVersionError
@@ -128,7 +129,7 @@ def test_emits_before_sign(self):
             'before-sign.service_name.operation_name',
             request=mock.ANY, signing_name='signing_name',
             region_name='region_name', signature_version='v4',
-            request_signer=self.signer)
+            request_signer=self.signer, operation_name='operation_name')
 
     def test_disable_signing(self):
         # Returning botocore.UNSIGNED from choose-signer disables signing!
@@ -727,7 +728,11 @@ def test_generate_presigned_url(self):
             'query_string': {},
             'url_path': u'/mybucket/mykey',
             'method': u'GET',
-            'context': {}}
+            # mock.ANY is used because client parameter related events
+            # inject values into the context. So using the context's exact
+            # value for these tests will be a maintenance burden if
+            # anymore customizations are added that inject into the context.
+            'context': mock.ANY}
         self.generate_url_mock.assert_called_with(
             request_dict=ref_request_dict, expires_in=3600,
             operation_name='GetObject')
@@ -748,7 +753,7 @@ def test_generate_presigned_url_with_query_string(self):
             'query_string': {u'response-content-disposition': disposition},
             'url_path': u'/mybucket/mykey',
             'method': u'GET',
-            'context': {}}
+            'context': mock.ANY}
         self.generate_url_mock.assert_called_with(
             request_dict=ref_request_dict, expires_in=3600,
             operation_name='GetObject')
@@ -772,7 +777,7 @@ def test_generate_presigned_url_expires(self):
             'query_string': {},
             'url_path': u'/mybucket/mykey',
             'method': u'GET',
-            'context': {}}
+            'context': mock.ANY}
         self.generate_url_mock.assert_called_with(
             request_dict=ref_request_dict, expires_in=20,
             operation_name='GetObject')
@@ -788,11 +793,44 @@ def test_generate_presigned_url_override_http_method(self):
             'query_string': {},
             'url_path': u'/mybucket/mykey',
             'method': u'PUT',
-            'context': {}}
+            'context': mock.ANY}
         self.generate_url_mock.assert_called_with(
             request_dict=ref_request_dict, expires_in=3600,
             operation_name='GetObject')
 
+    def test_generate_presigned_url_emits_param_events(self):
+        emitter = mock.Mock(HierarchicalEmitter)
+        emitter.emit.return_value = []
+        self.client.meta.events = emitter
+        self.client.generate_presigned_url(
+            'get_object', Params={'Bucket': self.bucket, 'Key': self.key})
+        events_emitted = [
+            emit_call[0][0] for emit_call in emitter.emit.call_args_list
+        ]
+        self.assertEqual(
+            events_emitted,
+            [
+                'provide-client-params.s3.GetObject',
+                'before-parameter-build.s3.GetObject'
+            ]
+        )
+
+    def test_generate_presign_url_emits_is_presign_in_context(self):
+        emitter = mock.Mock(HierarchicalEmitter)
+        emitter.emit.return_value = []
+        self.client.meta.events = emitter
+        self.client.generate_presigned_url(
+            'get_object', Params={'Bucket': self.bucket, 'Key': self.key})
+        kwargs_emitted = [
+            emit_call[1] for emit_call in emitter.emit.call_args_list
+        ]
+        for kwargs in kwargs_emitted:
+            self.assertTrue(
+                kwargs.get('context', {}).get('is_presign_request'),
+                'The context did not have is_presign_request set to True for '
+                'the following kwargs emitted: %s' % kwargs
+            )
+
 
 class TestGeneratePresignedPost(unittest.TestCase):
     def setUp(self):