Repository: boto/botocore
Pull request #464 (2015-02-18 00:55:43)

Prevent override of region when using endpoint_url
This ensures that user's region is not overwritten by the credentialScope if the user supplies an endpoint url. This is needed to be able to make signed calls to STS regionalized endpoints, without getting auth errors due to incorrect region . So for example, in the CLI, to make a GetSessionToken call to us-west-2 you would do:
```
$ aws sts get-session-token --region us-west-2 --endpoint-url https://sts.us-west-2.amazonaws.com
```

Before, this would cause a signing error if signed with sigv4 because the credentialScope was set to ``us-east-1`` and that overrode the region that you provided.

cc @jamesls @danielgtaylor  


Patch:
diff --git a/botocore/client.py b/botocore/client.py
index 0d76062b0..6adc03be8 100644
--- a/botocore/client.py
+++ b/botocore/client.py
@@ -208,9 +208,7 @@ class ClientCreator(object):
         endpoint_config = resolver.construct_endpoint(
                 service_model.endpoint_prefix,
                 region_name, scheme=scheme)
-        # Region name override from endpoint
-        region_name = endpoint_config.get('properties', {}).get(
-            'credentialScope', {}).get('region', region_name)
+
         # Signature version override from endpoint
         signature_version = service_model.signature_version
         if 'signatureVersion' in endpoint_config.get('properties', {}):
@@ -252,6 +250,10 @@ class ClientCreator(object):
             response_parser_factory=self._response_parser_factory)
         response_parser = botocore.parsers.create_parser(protocol)
 
+        # This is only temporary in the sense that we should remove any
+        # region_name logic from endpoints and put it into clients.
+        # But that can only happen once operation objects are deprecated.
+        region_name = endpoint.region_name
         signature_version, region_name = \
             self._get_signature_version_and_region(
                 service_model, region_name, is_secure, scoped_config)
diff --git a/botocore/endpoint.py b/botocore/endpoint.py
index 758a3176b..7473917e0 100644
--- a/botocore/endpoint.py
+++ b/botocore/endpoint.py
@@ -224,17 +224,15 @@ def _get_proxies(url):
 def get_endpoint(service, region_name, endpoint_url, verify=None):
     service_name = getattr(service, 'signing_name', service.endpoint_prefix)
     endpoint_prefix = service.endpoint_prefix
-    signature_version = getattr(service, 'signature_version', None)
     session = service.session
     event_emitter = session.get_component('event_emitter')
     user_agent = session.user_agent()
     return get_endpoint_complex(service_name, endpoint_prefix,
-                                signature_version,
                                 region_name, endpoint_url, verify, user_agent,
                                 event_emitter)
 
 
-def get_endpoint_complex(service_name, endpoint_prefix, signature_version,
+def get_endpoint_complex(service_name, endpoint_prefix,
                          region_name, endpoint_url, verify,
                          user_agent, event_emitter,
                          response_parser_factory=None):
@@ -292,39 +290,61 @@ class EndpointCreator(object):
                 endpoint = {'uri': endpoint_url, 'properties': {}}
             else:
                 raise
+
+        region_name = self._determine_region_name(endpoint, region_name,
+                                                  endpoint_url)
+        if endpoint_url is not None:
+            # If the user provides an endpoint url, we'll use that
+            # instead of what the heuristics rule gives us.
+            final_endpoint_url = endpoint_url
+        else:
+            final_endpoint_url = endpoint['uri']
+        return self._get_endpoint(service_model, region_name,
+                                  final_endpoint_url, verify,
+                                  response_parser_factory)
+
+    def _determine_region_name(self, endpoint_config, region_name=None,
+                               endpoint_url=None):
+        # This is a helper function to determine region name to use.
+        # It will take into account whether the user passes in a region
+        # name, whether their is a rule in the endpoint JSON, or
+        # an endpoint url was provided.
+
+        # TODO: Once we completely move to clients. We will remove region
+        # as public attribute from endpoints and as a result move this helper
+        # function to clients becuase region is really only important for
+        # signing.
+
         # We only support the credentialScope.region in the properties
         # bag right now, so if it's available, it will override the
         # provided region name.
-        region_name_override = endpoint['properties'].get(
+        region_name_override = endpoint_config['properties'].get(
             'credentialScope', {}).get('region')
-        if signature_version is NOT_SET:
-            signature_version = service_model.signature_version
-            if 'signatureVersion' in endpoint['properties']:
-                signature_version = endpoint['properties']['signatureVersion']
+
+        if endpoint_url is not None:
+            # If an endpoint_url is provided, do not use region name
+            # override if a region
+            # was provided by the user.
+            if region_name is not None:
+                region_name_override = None
+
         if region_name_override is not None:
             # Letting the heuristics rule override the region_name
             # allows for having a default region of something like us-west-2
             # for IAM, but we still will know to use us-east-1 for sigv4.
             region_name = region_name_override
-        if endpoint_url is not None:
-            # If the user provides an endpoint url, we'll use that
-            # instead of what the heuristics rule gives us.
-            final_endpoint_url = endpoint_url
-        else:
-            final_endpoint_url = endpoint['uri']
-        return self._get_endpoint(service_model, region_name,
-                                  signature_version, final_endpoint_url,
-                                  verify, response_parser_factory)
 
-    def _get_endpoint(self, service_model, region_name, signature_version,
-                      endpoint_url, verify, response_parser_factory):
+        return region_name
+
+
+    def _get_endpoint(self, service_model, region_name, endpoint_url,
+                      verify, response_parser_factory):
         service_name = service_model.signing_name
         endpoint_prefix = service_model.endpoint_prefix
         user_agent = self._user_agent
         event_emitter = self._event_emitter
         user_agent = self._user_agent
         return get_endpoint_complex(service_name, endpoint_prefix,
-                                    signature_version,
                                     region_name, endpoint_url,
                                     verify, user_agent, event_emitter,
                                     response_parser_factory)
diff --git a/botocore/operation.py b/botocore/operation.py
index 683b0b69d..6b87b02ae 100644
--- a/botocore/operation.py
+++ b/botocore/operation.py
@@ -76,9 +76,7 @@ class Operation(BotoCoreObject):
         endpoint_config = resolver.construct_endpoint(
                 service_model.endpoint_prefix,
                 endpoint.region_name, scheme=scheme)
-        # Region name override from endpoint
-        region_name = endpoint_config.get('properties', {}).get(
-            'credentialScope', {}).get('region', endpoint.region_name)
+
         # Signature version override from endpoint
         signature_version = self.service.signature_version
         if 'signatureVersion' in endpoint_config.get('properties', {}):
@@ -97,7 +95,7 @@ class Operation(BotoCoreObject):
                          service_model.endpoint_prefix, override)
                     signature_version = override
 
-        return signature_version, region_name
+        return signature_version, endpoint.region_name
 
     def call(self, endpoint, **kwargs):
         logger.debug("%s called with kwargs: %s", self, kwargs)


Test patch:
diff --git a/tests/integration/test_sts.py b/tests/integration/test_sts.py
new file mode 100644
index 000000000..af70c3a48
--- /dev/null
+++ b/tests/integration/test_sts.py
@@ -0,0 +1,62 @@
+# Copyright 2015 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+from tests import unittest
+
+import botocore.session
+from botocore.exceptions import ClientError
+
+class TestSTS(unittest.TestCase):
+    def setUp(self):
+        self.session = botocore.session.get_session()
+        credentials = self.session.get_credentials()
+        if credentials.token is not None:
+            self.skipTest('STS tests require long-term credentials')
+
+    def test_regionalized_endpoints(self):
+        sts = self.session.create_client('sts', region_name='ap-southeast-1')
+        response = sts.get_session_token()
+        # Do not want to be revealing any temporary keys if the assertion fails
+        self.assertIn('Credentials', response.keys())
+
+        # Since we have to activate STS regionalization, we will test
+        # that you can send an STS request to a regionalized endpoint
+        # by making a call with the explicitly wrong region name
+        sts = self.session.create_client(
+            'sts', region_name='ap-southeast-1',
+            endpoint_url='https://sts.us-west-2.amazonaws.com')
+        # Signing error will be thrown with the incorrect region name included.
+        with self.assertRaisesRegexp(ClientError, 'ap-southeast-1') as e:
+            sts.get_session_token()
+
+    def test_regionalized_endpoints_operation(self):
+        # TODO: Remove this test once service/operation objects are removed.
+        # This was added here because the CLI uses operation objects currently
+        # but this type of integration testing (non customized) should
+        # be done at the botocore level.
+        sts = self.session.get_service('sts')
+
+        endpoint = sts.get_endpoint(region_name='ap-southeast-1')
+        operation = sts.get_operation('GetSessionToken')
+        http, response = operation.call(endpoint)
+        # Do not want to be revealing any temporary keys if the assertion fails
+        self.assertIn('Credentials', response.keys())
+
+        endpoint = sts.get_endpoint(
+            region_name='ap-southeast-1',
+            endpoint_url='https://sts.us-west-2.amazonaws.com')
+        operation = sts.get_operation('GetSessionToken')
+        http, response = operation.call(endpoint)
+        self.assertIn('Error', response)
+        error = response['Error']
+        self.assertEqual('SignatureDoesNotMatch', error['Code'])
+        self.assertIn('ap-southeast-1', error['Message'])
diff --git a/tests/unit/test_endpoint.py b/tests/unit/test_endpoint.py
index 03b11fb0d..cc274a300 100644
--- a/tests/unit/test_endpoint.py
+++ b/tests/unit/test_endpoint.py
@@ -322,6 +322,40 @@ class TestEndpointCreator(unittest.TestCase):
         endpoint = creator.create_endpoint(self.service_model)
         self.assertEqual(endpoint.region_name, 'us-east-1')
 
+    def test_endpoint_resolver_no_uses_credential_scope_with_endpoint_url(self):
+        resolver = Mock()
+        resolver_region_override = 'us-east-1'
+        resolver.construct_endpoint.return_value = {
+            'uri': 'https://endpoint.url',
+            'properties': {
+                'credentialScope': {
+                    'region': resolver_region_override,
+                }
+            }
+        }
+        original_region_name = 'us-west-2'
+        creator = EndpointCreator(resolver, original_region_name,
+                                  Mock(), 'user-agent')
+        endpoint = creator.create_endpoint(self.service_model, endpoint_url='https://foo')
+        self.assertEqual(endpoint.region_name, 'us-west-2')
+
+    def test_endpoint_resolver_uses_credential_scope_with_endpoint_url_and_no_region(self):
+        resolver = Mock()
+        resolver_region_override = 'us-east-1'
+        resolver.construct_endpoint.return_value = {
+            'uri': 'https://endpoint.url',
+            'properties': {
+                'credentialScope': {
+                    'region': resolver_region_override,
+                }
+            }
+        }
+        original_region_name = None
+        creator = EndpointCreator(resolver, original_region_name,
+                                  Mock(), 'user-agent')
+        endpoint = creator.create_endpoint(self.service_model, endpoint_url='https://foo')
+        self.assertEqual(endpoint.region_name, resolver_region_override)
+
 
 class TestAWSSession(unittest.TestCase):
     def test_auth_header_preserved_from_s3_redirects(self):