PR feedback implemented

botocore/auth.py

@@ -1136,6 +1136,18 @@ def add_auth(self, request):
         request.headers['Authorization'] = auth_header
 
 
+def resolve_auth_type(auth_trait):
+    for auth_type in auth_trait:
+        if auth_type == 'smithy.api#noAuth':
+            return AUTH_TYPE_TO_SIGNATURE_VERSION[auth_type]
+        elif auth_type in AUTH_TYPE_TO_SIGNATURE_VERSION:
+            signature_version = AUTH_TYPE_TO_SIGNATURE_VERSION[auth_type]
+            if signature_version in AUTH_TYPE_MAPS:
+                return signature_version
+        else:
+            raise UnknownSignatureVersionError(signature_version=auth_type)
+
+
 AUTH_TYPE_MAPS = {
     'v2': SigV2Auth,
     'v3': SigV3Auth,
@@ -1171,15 +1183,3 @@ def add_auth(self, request):
     'smithy.api#httpBearerAuth': 'bearer',
     'smithy.api#noAuth': 'none',
 }
-
-
-def resolve_auth_type(auth_trait):
-    for auth_type in auth_trait:
-        if auth_type == 'smithy.api#noAuth':
-            return AUTH_TYPE_TO_SIGNATURE_VERSION[auth_type]
-        elif auth_type in AUTH_TYPE_TO_SIGNATURE_VERSION:
-            signature_version = AUTH_TYPE_TO_SIGNATURE_VERSION[auth_type]
-            if signature_version in AUTH_TYPE_MAPS:
-                return signature_version
-        else:
-            raise UnknownSignatureVersionError(signature_version=auth_type)

botocore/client.py

@@ -958,12 +958,10 @@ def _make_api_call(self, operation_name, api_params):
             'client_region': self.meta.region_name,
             'client_config': self.meta.config,
             'has_streaming_input': operation_model.has_streaming_input,
-            'auth_type': operation_model.auth_type,
+            'auth_type': operation_model.resolved_auth_type,
+            'unsigned_payload': operation_model.unsigned_payload,
         }
 
-        if operation_model.unsigned_payload:
-            request_context['payload_signing_enabled'] = False
-
         api_params = self._emit_api_params(
             api_params=api_params,
             operation_model=operation_model,

botocore/handlers.py

@@ -203,6 +203,11 @@ def set_operation_specific_signer(context, signing_name, **kwargs):
     if auth_type == 'bearer':
         return 'bearer'
 
+    # If the operation needs an unsigned body, we set additional context
+    # allowing the signer to be aware of this.
+    if context.get('unsigned_payload') or auth_type == 'v4-unsigned-body':
+        context['payload_signing_enabled'] = False
+
     if auth_type.startswith('v4'):
         if auth_type == 'v4-s3express':
             return auth_type
@@ -220,11 +225,6 @@ def set_operation_specific_signer(context, signing_name, **kwargs):
         else:
             signature_version = 'v4'
 
-        # If the operation needs an unsigned body, we set additional context
-        # allowing the signer to be aware of this.
-        if auth_type == 'v4-unsigned-body':
-            context['payload_signing_enabled'] = False
-
         # Signing names used by s3 and s3-control use customized signers "s3v4"
         # and "s3v4a".
         if signing_name in S3_SIGNING_NAMES:

botocore/model.py

@@ -630,9 +630,13 @@ def auth(self):
 
     @CachedProperty
     def auth_type(self):
+        return self._operation_model.get('authtype')
+
+    @CachedProperty
+    def resolved_auth_type(self):
         if self.auth:
             return resolve_auth_type(self.auth)
-        return self._operation_model.get('authtype')
+        return self.auth_type
 
     @CachedProperty
     def unsigned_payload(self):

tests/functional/test_auth_config.py

@@ -18,11 +18,15 @@
 # signature may fail and others may succeed. e.g. a service may want to use bearer
 # auth but fall back to sigv4 if a token isn't available. There's currently no way to do
 # this in botocore, so this test ensures we handle this gracefully when the need arises.
+
+
+# The dictionary's value here needs to be hashable to be added to the set below; any
+# new auth types with multiple requirements should be added in a comma-separated list
 AUTH_TYPE_REQUIREMENTS = {
-    'aws.auth#sigv4': ['credentials'],
-    'aws.auth#sigv4a': ['credentials'],
-    'smithy.api#httpBearerAuth': ['bearer_token'],
-    'smithy.api#noAuth': [],
+    'aws.auth#sigv4': 'credentials',
+    'aws.auth#sigv4a': 'credentials',
+    'smithy.api#httpBearerAuth': 'bearer_token',
+    'smithy.api#noAuth': 'none',
 }
 
 
@@ -67,10 +71,7 @@ def test_all_requirements_match_for_operation(auth_service, operation_model):
 
 
 def assert_all_requirements_match(auth_config, message):
-    if len(auth_config) > 1:
-        first_auth = auth_config.pop()
-        first_auth_reqs = AUTH_TYPE_REQUIREMENTS[first_auth]
-        assert all(
-            first_auth_reqs == AUTH_TYPE_REQUIREMENTS[req]
-            for req in auth_config
-        ), message
+    auth_requirements = set()
+    for auth_type in auth_config:
+        auth_requirements.add(AUTH_TYPE_REQUIREMENTS[auth_type])
+    assert len(auth_requirements) == 1

tests/unit/auth/test_auth_trait.py

@@ -11,12 +11,7 @@
 # ANY KIND, either express or implied. See the License for the specific
 # language governing permissions and limitations under the License.
 
-from botocore.auth import (
-    AUTH_TYPE_MAPS,
-    AUTH_TYPE_TO_SIGNATURE_VERSION,
-    BaseSigner,
-    resolve_auth_type,
-)
+from botocore.auth import BaseSigner, resolve_auth_type
 from botocore.exceptions import UnknownSignatureVersionError
 from tests import mock, unittest
 

tests/unit/test_client.py

@@ -1713,17 +1713,6 @@ def test_client_internal_credential_shim(self):
         )
         self.assertEqual(service_client._get_credentials(), self.credentials)
 
-    def test_unsigned_payload_disables_signing(self):
-        creator = self.create_client_creator()
-        service_client = creator.create_client('myservice', 'us-west-2')
-        test_operation = self.service_description['operations'][
-            'TestOperation'
-        ]
-        test_operation['unsignedPayload'] = True
-        service_client.test_operation(Foo='one')
-        context = self.endpoint.make_request.call_args[0][1]['context']
-        self.assertFalse(context['payload_signing_enabled'])
-
 
 class TestClientErrors(TestAutoGeneratedClient):
     def add_error_response(self, error_response):

tests/unit/test_handlers.py

@@ -1036,13 +1036,11 @@ def test_set_operation_specific_signer_v4a_existing_signing_context(self):
         signing_name = 'myservice'
         context = {
             'auth_type': 'v4a',
-            'signing': {'foo': 'bar', 'region': 'abc'},
+            'signing': {'foo': 'bar'},
         }
         handlers.set_operation_specific_signer(
             context=context, signing_name=signing_name
         )
-        # region has been updated
-        self.assertEqual(context['signing']['region'], 'abc')
         # signing_name has been added
         self.assertEqual(context['signing']['signing_name'], signing_name)
         # foo remained untouched
@@ -1090,12 +1088,44 @@ def test_set_operation_specific_signer_prefers_client_config(self):
             'client_config': Config(
                 sigv4a_signing_region_set="region_1,region_2"
             ),
+            'signing': {
+                'region': 'abc',
+            },
         }
         handlers.set_operation_specific_signer(
             context=context, signing_name=signing_name
         )
         self.assertEqual(context['signing']['region'], 'region_1,region_2')
 
+    def test_payload_signing_disabled_sets_proper_key(self):
+        signing_name = 'myservice'
+        context = {
+            'auth_type': 'v4',
+            'signing': {
+                'foo': 'bar',
+                'region': 'abc',
+            },
+            'unsigned_payload': True,
+        }
+        handlers.set_operation_specific_signer(
+            context=context, signing_name=signing_name
+        )
+        self.assertEqual(context.get('payload_signing_enabled'), False)
+
+    def test_no_payload_signing_disabled_does_not_set_key(self):
+        signing_name = 'myservice'
+        context = {
+            'auth_type': 'v4',
+            'signing': {
+                'foo': 'bar',
+                'region': 'abc',
+            },
+        }
+        handlers.set_operation_specific_signer(
+            context=context, signing_name=signing_name
+        )
+        self.assertNotIn('payload_signing_enabled', context)
+
 
 @pytest.mark.parametrize(
     'auth_type, expected_response', [('v4', 's3v4'), ('v4a', 's3v4a')]

tests/unit/test_model.py

@@ -182,6 +182,7 @@ def setUp(self):
                     'errors': [{'shape': 'NoSuchResourceException'}],
                     'documentation': 'Docs for OperationName',
                     'authtype': 'v4',
+                    'auth': ['aws.auth#sigv4'],
                 },
                 'OperationTwo': {
                     'http': {
@@ -396,6 +397,22 @@ def test_error_shapes(self):
             operation.error_shapes[0].name, 'NoSuchResourceException'
         )
 
+    def test_has_auth(self):
+        operation = self.service_model.operation_model('OperationName')
+        self.assertEqual(operation.auth, ["aws.auth#sigv4"])
+
+    def test_auth_not_set(self):
+        operation = self.service_model.operation_model('OperationTwo')
+        self.assertIsNone(operation.auth)
+
+    def test_has_resolved_auth_type(self):
+        operation = self.service_model.operation_model('OperationName')
+        self.assertEqual(operation.resolved_auth_type, 'v4')
+
+    def test_resolved_auth_type_not_set(self):
+        operation = self.service_model.operation_model('OperationTwo')
+        self.assertIsNone(operation.resolved_auth_type)
+
     def test_has_auth_type(self):
         operation = self.service_model.operation_model('OperationName')
         self.assertEqual(operation.auth_type, 'v4')