Merge pull request #38 from zalando/release/v0.8

Better error messages

connexion/decorators/security.py

@@ -13,12 +13,14 @@
 
 # Authentication and authorization related decorators
 
-import logging
+from flask import request
 import functools
+import logging
+import requests
 import types
 
-from flask import abort, request
-import requests
+from connexion.problem import problem
+
 
 logger = logging.getLogger('connexion.api.security')
 
@@ -38,22 +40,22 @@ def wrapper(*args, **kwargs):
         authorization = request.headers.get('Authorization')
         if authorization is None:
             logger.error("... No auth provided. Aborting with 401.")
-            raise abort(401)
+            return problem(401, 'Unauthorized', "No authorization token provided")
         else:
             _, token = authorization.split()
             logger.debug("... Getting token '%s' from %s", token, token_info_url)
             token_request = requests.get(token_info_url, params={'access_token': token})
             logger.debug("... Token info (%d): %s", token_request.status_code, token_request.text)
             if not token_request.ok:
-                raise abort(401)
+                return problem(401, 'Unauthorized', "Provided oauth token is not valid")
             token_info = token_request.json()
             user_scopes = set(token_info['scope'])
             scopes_intersection = user_scopes & allowed_scopes
             logger.debug("... Scope intersection: %s", scopes_intersection)
             if not scopes_intersection:
                 logger.error("... User scopes (%s) don't include one of the allowed scopes (%s). Aborting with 401.",
                              user_scopes, allowed_scopes)
-                raise abort(401)
+                return problem(403, 'Forbidden', "Provided token doesn't have the required scope")
             logger.info("... Token authenticated.")
         return function(*args, **kwargs)
 

connexion/decorators/validation.py

@@ -12,13 +12,12 @@
 """
 
 import flask
-import logging
 import functools
+import logging
 import numbers
 import types
 
-from flask import abort
-
+from connexion.problem import problem
 
 logger = logging.getLogger('connexion.decorators.parameters')
 
@@ -30,48 +29,66 @@
             'boolean': bool}  # map of swagger types to python types
 
 
-def validate_schema(data, schema):
-    schema_type = schema.get('type')
-    if schema_type == 'array':
-        if not isinstance(data, list):
-            raise abort(400)
-        for item in data:
-            validate_schema(item, schema.get('items'))
-
-    if schema_type == 'object':
-        if not isinstance(data, dict):
-            raise abort(400)
-
-        # verify if required keys are present
-        required_keys = schema.get('required', [])
-        logger.debug('... required keys: %s', required_keys)
-        for required_key in schema.get('required', required_keys):
-            if required_key not in data:
-                logger.debug("... '%s' missing", required_key)
-                raise abort(400)
-
-        # verify if value types are correct
-        for key in data.keys():
-            key_properties = schema['properties'].get(key)
-            if key_properties:
-                expected_type = TYPE_MAP.get(key_properties['type'])
-                if expected_type and not isinstance(data[key], expected_type):
-                    logger.debug("... '%s' is not a '%s'", key, expected_type)
-                    raise abort(400)
-
-
 class RequestBodyValidator:
-
     def __init__(self, schema):
         self.schema = schema
 
     def __call__(self, function: types.FunctionType) -> types.FunctionType:
         @functools.wraps(function)
         def wrapper(*args, **kwargs):
             data = flask.request.json
+
             logger.debug("%s validating schema...", flask.request.url)
-            validate_schema(data, self.schema)
+            error = self.validate_schema(data, self.schema)
+            if error:
+                return error
+
             response = function(*args, **kwargs)
             return response
 
         return wrapper
+
+    def validate_schema(self, data, schema) -> flask.Response:
+        schema_type = schema.get('type')
+        log_extra = {'url': flask.request.url, 'schema_type': schema_type}
+
+        if schema_type == 'array':
+            if not isinstance(data, list):
+                actual_type_name = type(data).__name__
+                logger.error("Wrong data type, expected 'list' got '%s'", actual_type_name, extra=log_extra)
+                return problem(400, 'Bad Request', "Wrong type, expected 'array' got '{}'".format(actual_type_name))
+            for item in data:
+                error = self.validate_schema(item, schema.get('items'))
+                if error:
+                    return error
+        elif schema_type == 'object':
+            if not isinstance(data, dict):
+                actual_type_name = type(data).__name__
+                logger.error("Wrong data type, expected 'dict' got '%s'", actual_type_name, extra=log_extra)
+                return problem(400, 'Bad Request', "Wrong type, expected 'object' got '{}'".format(actual_type_name))
+
+            # verify if required keys are present
+            required_keys = schema.get('required', [])
+            logger.debug('... required keys: %s', required_keys)
+            log_extra['required_keys'] = required_keys
+            for required_key in schema.get('required', required_keys):
+                if required_key not in data:
+                    logger.error("Missing parameter '%s'", required_key, extra=log_extra)
+                    return problem(400, 'Bad Request', "Missing parameter '{}'".format(required_key))
+
+            # verify if value types are correct
+            for key in data.keys():
+                key_properties = schema['properties'].get(key)
+                if key_properties:
+                    error = self.validate_schema(data[key], key_properties)
+                    if error:
+                        return error
+        else:
+            expected_type = TYPE_MAP.get(schema_type)  # type: type
+            actual_type = type(data)  # type: type
+            if expected_type and not isinstance(data, expected_type):
+                expected_type_name = expected_type.__name__
+                actual_type_name = actual_type.__name__
+                logger.error("'%s' is not a '%s'", data, expected_type_name)
+                return problem(400, 'Bad Request',
+                               "Wrong type, expected '{}' got '{}'".format(schema_type, actual_type_name))

tests/fakeapi/api.yaml

@@ -195,6 +195,26 @@ paths:
           description: goodbye response
           schema:
             type: string
+  /test_schema_list:
+    post:
+      summary: Returns empty response
+      description: Returns empty response
+      operationId: fakeapi.hello.schema_list
+      parameters:
+        - name: new_stack
+          required: true
+          in: body
+          schema:
+            type: array
+            items:
+              type: string
+      produces:
+        - application/json
+      responses:
+        200:
+          description: goodbye response
+          schema:
+            type: string
   /test_not_implemented:
     post:
       summary: Returns empty response

tests/fakeapi/hello.py

@@ -58,3 +58,7 @@ def empty():
 
 def schema():
     return ''
+
+
+def schema_list():
+    return ''

tests/test_app.py

@@ -193,6 +193,10 @@ def test_security(oauth_requests):
     app_client = app1.app.test_client()
     get_bye_no_auth = app_client.get('/v1.0/byesecure/jsantos')  # type: flask.Response
     assert get_bye_no_auth.status_code == 401
+    assert get_bye_no_auth.content_type == 'application/problem+json'
+    get_bye_no_auth_reponse = json.loads(get_bye_no_auth.data.decode())  # type: dict
+    assert get_bye_no_auth_reponse['title'] == 'Unauthorized'
+    assert get_bye_no_auth_reponse['detail'] == "No authorization token provided"
 
     headers = {"Authorization": "Bearer 100"}
     get_bye_good_auth = app_client.get('/v1.0/byesecure/jsantos', headers=headers)  # type: flask.Response
@@ -202,12 +206,20 @@ def test_security(oauth_requests):
     app_client = app1.app.test_client()
     headers = {"Authorization": "Bearer 200"}
     get_bye_wrong_scope = app_client.get('/v1.0/byesecure/jsantos', headers=headers)  # type: flask.Response
-    assert get_bye_wrong_scope.status_code == 401
+    assert get_bye_wrong_scope.status_code == 403
+    assert get_bye_wrong_scope.content_type == 'application/problem+json'
+    get_bye_wrong_scope_reponse = json.loads(get_bye_wrong_scope.data.decode())  # type: dict
+    assert get_bye_wrong_scope_reponse['title'] == 'Forbidden'
+    assert get_bye_wrong_scope_reponse['detail'] == "Provided token doesn't have the required scope"
 
     app_client = app1.app.test_client()
     headers = {"Authorization": "Bearer 300"}
     get_bye_bad_token = app_client.get('/v1.0/byesecure/jsantos', headers=headers)  # type: flask.Response
     assert get_bye_bad_token.status_code == 401
+    assert get_bye_bad_token.content_type == 'application/problem+json'
+    get_bye_bad_token_reponse = json.loads(get_bye_bad_token.data.decode())  # type: dict
+    assert get_bye_bad_token_reponse['title'] == 'Unauthorized'
+    assert get_bye_bad_token_reponse['detail'] == "Provided oauth token is not valid"
 
 
 def test_empty(app):
@@ -222,13 +234,48 @@ def test_schema(app):
     app_client = app.app.test_client()
     headers = {'Content-type': 'application/json'}
 
-    empty_request = app_client.post('/v1.0/test_schema', headers=headers, data={})  # type: flask.Response
+    empty_request = app_client.post('/v1.0/test_schema', headers=headers, data=json.dumps({}))  # type: flask.Response
     assert empty_request.status_code == 400
+    assert empty_request.content_type == 'application/problem+json'
+    empty_request_response = json.loads(empty_request.data.decode())  # type: dict
+    assert empty_request_response['title'] == 'Bad Request'
+    assert empty_request_response['detail'] == "Missing parameter 'image_version'"
 
     bad_type = app_client.post('/v1.0/test_schema', headers=headers,
                                data=json.dumps({'image_version': 22}))  # type: flask.Response
     assert bad_type.status_code == 400
+    assert bad_type.content_type == 'application/problem+json'
+    bad_type_response = json.loads(bad_type.data.decode())  # type: dict
+    assert bad_type_response['title'] == 'Bad Request'
+    assert bad_type_response['detail'] == "Wrong type, expected 'string' got 'int'"
 
     good_request = app_client.post('/v1.0/test_schema', headers=headers,
                                    data=json.dumps({'image_version': 'version'}))  # type: flask.Response
     assert good_request.status_code == 200
+
+    wrong_type = app_client.post('/v1.0/test_schema', headers=headers, data=json.dumps(42))  # type: flask.Response
+    assert wrong_type.status_code == 400
+    assert wrong_type.content_type == 'application/problem+json'
+    wrong_type_response = json.loads(wrong_type.data.decode())  # type: dict
+    assert wrong_type_response['title'] == 'Bad Request'
+    assert wrong_type_response['detail'] == "Wrong type, expected 'object' got 'int'"
+
+
+def test_schema_list(app):
+    app_client = app.app.test_client()
+    headers = {'Content-type': 'application/json'}
+
+    wrong_type = app_client.post('/v1.0/test_schema_list', headers=headers, data=json.dumps(42))  # type: flask.Response
+    assert wrong_type.status_code == 400
+    assert wrong_type.content_type == 'application/problem+json'
+    wrong_type_response = json.loads(wrong_type.data.decode())  # type: dict
+    assert wrong_type_response['title'] == 'Bad Request'
+    assert wrong_type_response['detail'] == "Wrong type, expected 'array' got 'int'"
+
+    wrong_items = app_client.post('/v1.0/test_schema_list', headers=headers,
+                                  data=json.dumps([42]))  # type: flask.Response
+    assert wrong_items.status_code == 400
+    assert wrong_items.content_type == 'application/problem+json'
+    wrong_items_response = json.loads(wrong_items.data.decode())  # type: dict
+    assert wrong_items_response['title'] == 'Bad Request'
+    assert wrong_items_response['detail'] == "Wrong type, expected 'string' got 'int'"

tests/test_operation.py

@@ -1,7 +1,10 @@
 import pathlib
 import pytest
+import types
+
 from connexion.exceptions import InvalidSpecification
 from connexion.operation import Operation
+from connexion.decorators.security import security_passthrough, verify_oauth
 
 TEST_FOLDER = pathlib.Path(__file__).parent
 
@@ -89,15 +92,30 @@
               'security': [{'oauth': ['uid']}],
               'summary': 'Create new stack'}
 
+SECURITY_DEFINITIONS = {'oauth': {'type': 'oauth2',
+                                  'flow': 'password',
+                                  'x-tokenInfoUrl': 'https://ouath.example/token_info',
+                                  'scopes': {'myscope': 'can do stuff'}}}
+
+SECURITY_DEFINITIONS_WO_INFO = {'oauth': {'type': 'oauth2',
+                                          'flow': 'password',
+                                          'scopes': {'myscope': 'can do stuff'}}}
+
 
 def test_operation():
     operation = Operation(method='GET',
                           path='endpoint',
                           operation=OPERATION1,
                           app_produces=['application/json'],
                           app_security=[],
-                          security_definitions={},
+                          security_definitions=SECURITY_DEFINITIONS,
                           definitions=DEFINITIONS)
+    assert isinstance(operation.function, types.FunctionType)
+    # security decorator should be a partial with verify_oauth as the function and token url and scopes as arguments.
+    # See https://docs.python.org/2/library/functools.html#partial-objects
+    assert operation._Operation__security_decorator.func is verify_oauth
+    assert operation._Operation__security_decorator.args == ('https://ouath.example/token_info', set(['uid']))
+
     assert operation.method == 'GET'
     assert operation.produces == ['application/json']
     assert operation.security == [{'oauth': ['uid']}]
@@ -150,3 +168,20 @@ def test_invalid_reference():
     exception = exc_info.value
     assert str(exception) == "<InvalidSpecification: GET endpoint  '$ref' needs to to point to definitions>"
     assert repr(exception) == "<InvalidSpecification: GET endpoint  '$ref' needs to to point to definitions>"
+
+
+def test_no_token_info():
+    operation = Operation(method='GET',
+                          path='endpoint',
+                          operation=OPERATION1,
+                          app_produces=['application/json'],
+                          app_security=SECURITY_DEFINITIONS_WO_INFO,
+                          security_definitions=SECURITY_DEFINITIONS_WO_INFO,
+                          definitions=DEFINITIONS)
+    assert isinstance(operation.function, types.FunctionType)
+    assert operation._Operation__security_decorator is security_passthrough
+
+    assert operation.method == 'GET'
+    assert operation.produces == ['application/json']
+    assert operation.security == [{'oauth': ['uid']}]
+    assert operation.body_schema == DEFINITIONS['new_stack']