From 7b59f94dd3ce054444a3fb7a0b794a4832e88c9b Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Mon, 22 Aug 2016 17:00:02 +0200 Subject: [PATCH 1/3] Simplified pre-commit install --- .pre-commit-config.yaml | 3 ++- Makefile | 5 +++++ tox.ini | 5 +++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c4829e2e..dbbb154a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -3,12 +3,13 @@ sha: 5fe82b hooks: - id: autopep8-wrapper - args: ['-i', '--ignore=E309'] + args: ['-i', '--ignore=E309,E501'] - id: check-json - id: check-yaml - id: debug-statements - id: end-of-file-fixer - id: flake8 + args: [--max-line-length=131] - id: name-tests-test - id: trailing-whitespace - id: requirements-txt-fixer diff --git a/Makefile b/Makefile index 34dc7d53..0bf0435a 100644 --- a/Makefile +++ b/Makefile @@ -19,6 +19,11 @@ test: tests: test + +.PHONY: install-hooks +install-hooks: + tox -e pre-commit -- install -f --install-hooks + clean: @rm -rf .tox build dist docs/build *.egg-info find . -name '*.pyc' -delete diff --git a/tox.ini b/tox.ini index 75ea7f43..b3f19341 100644 --- a/tox.ini +++ b/tox.ini @@ -35,3 +35,8 @@ commands = sphinx-build -b html -d build/doctrees source build/html [flake8] exclude = .svn,CVS,.bzr,.hg,.git,__pycache__,.tox,docs,virtualenv_run max_line_length = 80 + +[testenv:pre-commit] +envdir = {toxworkdir}/pre-commit +deps = pre-commit>=0.2.10 +commands = pre-commit {posargs} From 6b888efd1df81558fc5b251b2f41e29c17f4afdf Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Mon, 22 Aug 2016 16:41:55 +0200 Subject: [PATCH 2/3] Security Objects handling NOTE: Support for: - Basic and ApiKey Only (no oauth2) - Security Objects in Operation Object Only ApiKey Support and Security Objects --- bravado_core/operation.py | 72 ++++++++++++++++++++++--- tests/request/unmarshal_request_test.py | 6 ++- tests/resource/conftest.py | 10 +--- 3 files changed, 71 insertions(+), 17 deletions(-) diff --git a/bravado_core/operation.py b/bravado_core/operation.py index 066a3781..fbe130e9 100644 --- a/bravado_core/operation.py +++ b/bravado_core/operation.py @@ -2,20 +2,22 @@ import logging import re +from bravado_core.exception import SwaggerSchemaError from bravado_core.param import Param log = logging.getLogger(__name__) class Operation(object): - """Swagger operation defined by a unique (http_method, path_name) pair. - :type swagger_spec: :class:`Spec` - :param path_name: path of the operation. e.g. /pet/{petId} - :param http_method: get/put/post/delete/etc - :param op_spec: operation specification in dict form - """ def __init__(self, swagger_spec, path_name, http_method, op_spec): + """Swagger operation defined by a unique (http_method, path_name) pair. + + :type swagger_spec: :class:`Spec` + :param path_name: path of the operation. e.g. /pet/{petId} + :param http_method: get/put/post/delete/etc + :param op_spec: operation specification in dict form + """ self.swagger_spec = swagger_spec self.path_name = path_name self.http_method = http_method @@ -26,6 +28,9 @@ def __init__(self, swagger_spec, path_name, http_method, op_spec): # in the Swagger 2.0 Spec. self._operation_id = None + # generated by @property to avoid multiple swagger validations + self._security_objects = None + # (key, value) = (param name, Param) self.params = {} @@ -44,6 +49,36 @@ def consumes(self): result = deref(self.swagger_spec.spec_dict.get('consumes', [])) return result + @property + def security_objects(self): + """Builds up the list of security options for this operation + + :returns: list of security definition associated to the operation + """ + if self._security_objects is None: + deref = self.swagger_spec.deref + op_spec = deref(self.op_spec) + spec_dict = deref(self.swagger_spec.spec_dict) + security_spec = deref(op_spec.get('security', [])) + if len(security_spec) == 0: + security_spec = spec_dict.get('security', []) + security_defs_dict = spec_dict.get('securityDefinitions', {}) + + self._security_objects = [] + for security_item in security_spec: + for security_name, security_scope in security_item.items(): + security_definition = security_defs_dict.get(security_name) + if security_definition is None: + raise SwaggerSchemaError( + '{security} not defined in {swagger_path}'.format( + swagger_path='/securityDefinitions', + security=security_name, + ) + ) + self._security_objects.append(security_definition) + + return self._security_objects + @property def produces(self): """Note that the operation can override the value defined globally @@ -111,6 +146,28 @@ def __repr__(self): return u"%s(%s)" % (self.__class__.__name__, self.operation_id) +def _build_params_from_security_objects(op): + """ + Generate the required parameters from the security object definition. + NOTE: the current implementation handles only basic and apiKey types. + + :type op: :class:`bravado_core.operation.Operation` + :return: list of well formatted parameters (JSON-like notation) + :rtype: list + """ + security_params_spec = [] + for security_definition in op.security_objects: + if security_definition['type'] == 'apiKey': + security_params_spec.append({ + 'required': True, + 'type': 'string', + 'description': security_definition.get('description', ''), + 'name': security_definition['name'], + 'in': security_definition['in'] + }) + return security_params_spec + + def build_params(op): """Builds up the list of this operation's parameters taking into account parameters that may be available for this operation's path component. @@ -127,11 +184,12 @@ def build_params(op): paths_spec = deref(spec_dict.get('paths', {})) path_spec = deref(paths_spec.get(op.path_name)) path_params_spec = deref(path_spec.get('parameters', [])) + security_params_spec = _build_params_from_security_objects(op) # Order of addition is *important* here. Since op_params are last in the # list, they will replace any previously defined path_params with the # same name when the final params dict is constructed in the loop below. - params_spec = path_params_spec + op_params_spec + params_spec = path_params_spec + security_params_spec + op_params_spec params = {} for param_spec in params_spec: diff --git a/tests/request/unmarshal_request_test.py b/tests/request/unmarshal_request_test.py index 3055de6e..c2448272 100644 --- a/tests/request/unmarshal_request_test.py +++ b/tests/request/unmarshal_request_test.py @@ -6,7 +6,11 @@ def test_request_with_path_parameter(petstore_spec): - request = Mock(spec=IncomingRequest, path={'petId': '1234'}) + request = Mock( + spec=IncomingRequest, + path={'petId': '1234'}, + headers={'api_key': 'key1'}, + ) # /pet/{pet_id} fits the bill op = petstore_spec.resources['pet'].operations['getPetById'] request_data = unmarshal_request(request, op) diff --git a/tests/resource/conftest.py b/tests/resource/conftest.py index 4b8b555d..69f64b26 100644 --- a/tests/resource/conftest.py +++ b/tests/resource/conftest.py @@ -44,15 +44,7 @@ def paths_spec(): "400": { "description": "Invalid status value" } - }, - "security": [ - { - "petstore_auth": [ - "write:pets", - "read:pets" - ] - } - ] + } } }, } From 31e4ad27310d263a2495b0fae93ec3b673d73908 Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Tue, 23 Aug 2016 15:49:43 +0200 Subject: [PATCH 3/3] Security Objects handling tests --- tests/operation/conftest.py | 148 ++++++++++++++++++++++++ tests/operation/security_object_test.py | 88 ++++++++++++++ tests/request/unmarshal_request_test.py | 1 + 3 files changed, 237 insertions(+) create mode 100644 tests/operation/conftest.py create mode 100644 tests/operation/security_object_test.py diff --git a/tests/operation/conftest.py b/tests/operation/conftest.py new file mode 100644 index 00000000..ba81e70f --- /dev/null +++ b/tests/operation/conftest.py @@ -0,0 +1,148 @@ +import pytest +from six import iteritems + +SECURITY_DEFINITIONS = { + 'basic': { + 'basic': { + 'type': 'basic', + }, + }, + 'apiKey': { + 'apiKey': { + 'type': 'apiKey', + 'name': 'api_key', + 'in': 'header', + }, + }, + 'oauth2': { + 'oauth2': { + 'type': 'oauth2', + 'authorizationUrl': 'http://petstore.swagger.io/api/oauth/dialog', + 'flow': 'implicit', + 'scopes': { + 'write:pets': 'modify pets in your account', + 'read:pets': 'read your pets', + }, + }, + } +} +SECURITY_OBJECTS = { + 'basic': [{'basic': []}], + 'apiKey': [{'apiKey': []}], + 'oauth2': [{'oauth2': []}], +} + + +def test_security_object_and_definition_constants(): + assert SECURITY_OBJECTS.keys() == SECURITY_DEFINITIONS.keys() + + +@pytest.fixture +def definitions_spec(): + return { + 'Pet': { + 'type': 'object', + 'required': ['name'], + 'properties': { + 'name': {'type': 'string'}, + 'age': {'type': 'integer'}, + 'breed': {'type': 'string'} + } + } + } + + +@pytest.fixture +def _paths_spec(): + # The '#/paths' dict from a spec + return { + '/pet/findByStatus': { + 'get': { + 'tags': [ + 'pet' + ], + 'summary': 'Finds Pets by status', + 'description': 'Multiple status values can be provided with comma seperated strings', # noqa + 'operationId': 'findPetsByStatus', + 'produces': [ + 'application/json', + 'application/xml' + ], + 'parameters': [ + { + 'name': 'status', + 'in': 'query', + 'description': 'Status values that need to be considered for filter', # noqa + 'required': False, + 'type': 'array', + 'items': { + 'type': 'string' + }, + 'collectionFormat': 'multi', + 'default': 'available' + } + ], + 'responses': { + '200': { + 'description': 'successful operation', + 'schema': { + 'type': 'array', + 'items': { + '$ref': '#/definitions/Pet' + } + } + }, + '400': { + 'description': 'Invalid status value' + } + }, + } + }, + } + + +@pytest.fixture( + params=SECURITY_OBJECTS.keys(), +) +def specs_with_security_obj_in_op_and_security_specs(request, _paths_spec, + definitions_spec): + security_object = SECURITY_OBJECTS[request.param] + + for path, path_item in iteritems(_paths_spec): + for http_method in path_item.keys(): + path_item[http_method]['security'] = security_object + + return { + 'paths': _paths_spec, + 'definitions': definitions_spec, + 'securityDefinitions': SECURITY_DEFINITIONS[request.param], + } + + +@pytest.fixture +def specs_with_security_obj_in_op_and_no_security_specs( + specs_with_security_obj_in_op_and_security_specs +): + del specs_with_security_obj_in_op_and_security_specs['securityDefinitions'] + return specs_with_security_obj_in_op_and_security_specs + + +@pytest.fixture( + params=SECURITY_OBJECTS.keys(), +) +def specs_with_security_obj_in_root_and_security_specs(request, _paths_spec, + definitions_spec): + return { + 'paths': _paths_spec, + 'definitions': definitions_spec, + 'security': SECURITY_OBJECTS[request.param], + 'securityDefinitions': SECURITY_DEFINITIONS[request.param], + } + + +@pytest.fixture +def specs_with_security_obj_in_root_and_no_security_specs( + specs_with_security_obj_in_root_and_security_specs +): + del specs_with_security_obj_in_root_and_security_specs['securityDefinitions'] # noqa + return specs_with_security_obj_in_root_and_security_specs diff --git a/tests/operation/security_object_test.py b/tests/operation/security_object_test.py new file mode 100644 index 00000000..e8dbd12d --- /dev/null +++ b/tests/operation/security_object_test.py @@ -0,0 +1,88 @@ +from mock import Mock +import pytest +from six import iteritems + +from bravado_core.exception import SwaggerSchemaError +from bravado_core.request import IncomingRequest, unmarshal_request +from bravado_core.resource import build_resources +from bravado_core.spec import Spec +from jsonschema import ValidationError + + +def test_op_with_security_in_op_without_security_defs( + specs_with_security_obj_in_op_and_no_security_specs, +): + with pytest.raises(SwaggerSchemaError): + build_resources(Spec( + specs_with_security_obj_in_op_and_no_security_specs, + )) + + +def test_op_with_security_in_root_without_security_defs( + specs_with_security_obj_in_root_and_no_security_specs, +): + with pytest.raises(SwaggerSchemaError): + build_resources(Spec( + specs_with_security_obj_in_root_and_no_security_specs, + )) + + +def _validate_resources(resources, security_definitions_spec): + resource = resources.get('pet') + assert resource is not None + for security_option, security_obj in iteritems(security_definitions_spec): + operation = getattr(resource, 'findPetsByStatus') + assert operation is not None + assert security_obj in operation.security_objects + if security_option == 'apiKey': + assert len(operation.params) == 2 + assert security_obj['name'] in operation.params + else: + assert len(operation.params) == 1 + + +def test_op_with_security_in_op_with_security_defs( + specs_with_security_obj_in_op_and_security_specs, +): + security_definitions_spec = \ + specs_with_security_obj_in_op_and_security_specs['securityDefinitions'] + _validate_resources( + resources=build_resources(Spec( + specs_with_security_obj_in_op_and_security_specs, + )), + security_definitions_spec=security_definitions_spec, + ) + + +def test_op_with_security_in_root_with_security_defs( + specs_with_security_obj_in_root_and_security_specs, +): + security_definitions_spec = \ + specs_with_security_obj_in_root_and_security_specs['securityDefinitions'] # noqa + _validate_resources( + resources=build_resources(Spec( + specs_with_security_obj_in_root_and_security_specs, + )), + security_definitions_spec=security_definitions_spec, + ) + + +def test_correct_request_with_apiKey_security(petstore_spec): + request = Mock( + spec=IncomingRequest, + path={'petId': '1234'}, + headers={'api_key': 'key1'}, + ) + op = petstore_spec.resources['pet'].operations['getPetById'] + unmarshal_request(request, op) + + +def test_wrong_request_with_apiKey_security(petstore_spec): + request = Mock( + spec=IncomingRequest, + path={'petId': '1234'}, + headers={}, + ) + op = petstore_spec.resources['pet'].operations['getPetById'] + with pytest.raises(ValidationError): + unmarshal_request(request, op) diff --git a/tests/request/unmarshal_request_test.py b/tests/request/unmarshal_request_test.py index c2448272..18611291 100644 --- a/tests/request/unmarshal_request_test.py +++ b/tests/request/unmarshal_request_test.py @@ -15,6 +15,7 @@ def test_request_with_path_parameter(petstore_spec): op = petstore_spec.resources['pet'].operations['getPetById'] request_data = unmarshal_request(request, op) assert request_data['petId'] == 1234 + assert request_data['api_key'] == 'key1' def test_request_with_no_parameters(petstore_spec):