From f2af193bb8b05e59d15ed3f729ff152e6ce131eb Mon Sep 17 00:00:00 2001 From: Mathieu Imfeld Date: Sun, 26 Dec 2021 18:40:42 +0100 Subject: [PATCH 01/39] Ignoring /.coverage --- .gitignore | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index a7b6698..ba552da 100644 --- a/.gitignore +++ b/.gitignore @@ -246,5 +246,5 @@ dmypy.json # End of https://www.toptal.com/developers/gitignore/api/jetbrains,python -var/terraform/.terraform/ -var/terraform/.terraform.lock.hcl + +.coverage From fe4bf3afdfcd9ab9bfb2c5367b16a5efb05c2ccf Mon Sep 17 00:00:00 2001 From: Mathieu Imfeld Date: Sun, 26 Dec 2021 18:49:47 +0100 Subject: [PATCH 02/39] Cosmetic change --- mrmat_python_api_flask/apis/greeting/v1/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mrmat_python_api_flask/apis/greeting/v1/model.py b/mrmat_python_api_flask/apis/greeting/v1/model.py index a20ddd0..cd77038 100644 --- a/mrmat_python_api_flask/apis/greeting/v1/model.py +++ b/mrmat_python_api_flask/apis/greeting/v1/model.py @@ -35,7 +35,7 @@ class Meta: required=True, dump_only=True, metadata={ - 'description': 'The message returned' + 'description': 'A greeting message' } ) From 3ec814d49c57b102e41ca3c12132fa6bd57c5cb7 Mon Sep 17 00:00:00 2001 From: Mathieu Imfeld Date: Sun, 26 Dec 2021 18:50:19 +0100 Subject: [PATCH 03/39] Cosmetic change --- mrmat_python_api_flask/apis/greeting/v2/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mrmat_python_api_flask/apis/greeting/v2/model.py b/mrmat_python_api_flask/apis/greeting/v2/model.py index 146908f..a8f8365 100644 --- a/mrmat_python_api_flask/apis/greeting/v2/model.py +++ b/mrmat_python_api_flask/apis/greeting/v2/model.py @@ -49,7 +49,7 @@ class Meta: required=True, dump_only=True, metadata={ - 'description': 'The message returned' + 'description': 'A greeting message' } ) From 36391fd84ef334583b60337d7dbeba906e8d259b Mon Sep 17 00:00:00 2001 From: Mathieu Imfeld Date: Thu, 30 Dec 2021 11:14:24 +0100 Subject: [PATCH 04/39] Interim --- .idea/mrmat-python-api-flask.iml | 1 + mrmat_python_api_flask/apis/__init__.py | 30 +++++++++++++ .../apis/healthz/__init__.py | 1 - mrmat_python_api_flask/apis/healthz/api.py | 20 ++++----- mrmat_python_api_flask/apis/healthz/model.py | 42 ----------------- .../apis/resource/v1/__init__.py | 4 +- .../apis/resource/v1/api.py | 18 ++++---- .../apis/resource/v1/model.py | 13 +----- tests/test_healthz.py | 5 ++- tests/test_resource_v1.py | 45 ++++++++++++++++++- 10 files changed, 101 insertions(+), 78 deletions(-) delete mode 100644 mrmat_python_api_flask/apis/healthz/model.py diff --git a/.idea/mrmat-python-api-flask.iml b/.idea/mrmat-python-api-flask.iml index a5ec002..6334ba1 100644 --- a/.idea/mrmat-python-api-flask.iml +++ b/.idea/mrmat-python-api-flask.iml @@ -16,6 +16,7 @@ + diff --git a/mrmat_python_api_flask/apis/__init__.py b/mrmat_python_api_flask/apis/__init__.py index c386ce8..d9e9c8f 100644 --- a/mrmat_python_api_flask/apis/__init__.py +++ b/mrmat_python_api_flask/apis/__init__.py @@ -19,3 +19,33 @@ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. + +""" +Code that can be re-used by all APIs +""" + +from marshmallow import fields + +from mrmat_python_api_flask import ma + + +class StatusOutputSchema(ma.Schema): + class Meta: + fields = ('code', 'message') + + code = fields.Int( + default=0, + metadata={ + 'description': 'An integer status code which will typically match the HTTP status code' + } + ) + message = fields.Str( + required=True, + dump_only=True, + metadata={ + 'description': 'A human-readable message' + } + ) + + +status_output = StatusOutputSchema() diff --git a/mrmat_python_api_flask/apis/healthz/__init__.py b/mrmat_python_api_flask/apis/healthz/__init__.py index 9fd9dbd..60b7b26 100644 --- a/mrmat_python_api_flask/apis/healthz/__init__.py +++ b/mrmat_python_api_flask/apis/healthz/__init__.py @@ -23,5 +23,4 @@ """Pluggable blueprint of the Health API """ -from .model import HealthzOutput # noqa: F401 from .api import bp as api_healthz # noqa: F401 diff --git a/mrmat_python_api_flask/apis/healthz/api.py b/mrmat_python_api_flask/apis/healthz/api.py index f65781f..3f7be71 100644 --- a/mrmat_python_api_flask/apis/healthz/api.py +++ b/mrmat_python_api_flask/apis/healthz/api.py @@ -23,18 +23,16 @@ """Blueprint for the Healthz API """ -from flask.views import MethodView -from flask_smorest import Blueprint -from .model import HealthzOutput, healthz_output +#from flask_smorest import Blueprint +from flask import Blueprint -bp = Blueprint('healthz', - __name__, - description='Health API') +from mrmat_python_api_flask.apis import status_output +#bp = Blueprint('healthz', __name__, description='Health API') +bp = Blueprint('healthz', __name__) -@bp.route('/') -class Healthz(MethodView): - @bp.response(200, HealthzOutput) - def get(self): - return healthz_output.dump({'status': 'OK'}), 200 +@bp.route('/') +#@bp.response(200, StatusOutputSchema) +def get(): + return status_output.dump(status=200, message='OK'), 200 diff --git a/mrmat_python_api_flask/apis/healthz/model.py b/mrmat_python_api_flask/apis/healthz/model.py deleted file mode 100644 index d3c305b..0000000 --- a/mrmat_python_api_flask/apis/healthz/model.py +++ /dev/null @@ -1,42 +0,0 @@ -# MIT License -# -# Copyright (c) 2021 MrMat -# -# Permission is hereby granted, free of charge, to any person obtaining a copy -# of this software and associated documentation files (the "Software"), to deal -# in the Software without restriction, including without limitation the rights -# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -# copies of the Software, and to permit persons to whom the Software is -# furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice shall be included in -# all copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -# SOFTWARE. - -"""Healthz API Model""" - -from marshmallow import fields - -from mrmat_python_api_flask import ma - - -class HealthzOutput(ma.Schema): - class Meta: - fields = ('status',) - - status = fields.Str( - required=True, - dump_only=True, - metadata={ - 'description': 'An indication of application health' - }) - - -healthz_output = HealthzOutput() diff --git a/mrmat_python_api_flask/apis/resource/v1/__init__.py b/mrmat_python_api_flask/apis/resource/v1/__init__.py index d2ca02f..52bde28 100644 --- a/mrmat_python_api_flask/apis/resource/v1/__init__.py +++ b/mrmat_python_api_flask/apis/resource/v1/__init__.py @@ -23,5 +23,5 @@ """Pluggable blueprint of the Resource API v1 """ -from .api import bp as api_resource_v1 # noqa: F401 -from .model import Owner, Resource, OwnerSchema, ResourceSchema # noqa: F401 +from .api import bp as api_resource_v1 # noqa: F401 +from .model import Owner, Resource, OwnerSchema, ResourceSchema # noqa: F401 diff --git a/mrmat_python_api_flask/apis/resource/v1/api.py b/mrmat_python_api_flask/apis/resource/v1/api.py index 7fb3247..0413d64 100644 --- a/mrmat_python_api_flask/apis/resource/v1/api.py +++ b/mrmat_python_api_flask/apis/resource/v1/api.py @@ -30,6 +30,7 @@ from marshmallow import ValidationError from mrmat_python_api_flask import db, oidc +from mrmat_python_api_flask.apis import status_output from .model import Owner, Resource, resource_schema, resources_schema bp = Blueprint('resource_v1', __name__) @@ -57,7 +58,7 @@ def get_one(i: int): logger.info(f'Called by {client_id} for {name}') resource = Resource.query.filter(Resource.id == i).one_or_none() if resource is None: - return {'status': 404, 'message': f'Unable to find entry with identifier {i} in database'}, 404 + return status_output.dump(code=404, message='Unable to find a resource with this id'), 404 return resource_schema.dump(resource), 200 @@ -69,7 +70,7 @@ def create(): try: json_body = request.get_json() if not json_body: - return {'message': 'No input data provided'}, 400 + return status_output.dump(code=400, message='Missing input data'), 400 body = resource_schema.load(request.get_json()) except ValidationError as ve: return ve.messages, 422 @@ -81,8 +82,8 @@ def create(): .filter(Resource.name == body['name'] and Resource.owner.client_id == client_id)\ .one_or_none() if resource is not None: - return {'status': 409, - 'message': f'A resource with the same name and owner already exists with id {resource.id}'}, 409 + # TODO: Allow turning this off because it can be used as an enumeration attack + return status_output.dump(code=409, message='This resource already exists'), 409 # # Look up the owner and create one if necessary @@ -107,9 +108,9 @@ def modify(i: int): resource = Resource.query.filter(Resource.id == i).one_or_none() if resource is None: - return {'status': 404, 'message': 'Unable to find requested resource'}, 404 + return status_output.dump(code=404, message='Unable to find a resource with this id'), 404 if resource.owner.client_id != client_id: - return {'status': 401, 'message': 'You do not own this resource'}, 401 + return status_output.dump(code=401, message='You are not authorised to modify this resource'), 401 resource.name = body['name'] db.session.add(resource) @@ -125,9 +126,10 @@ def remove(i: int): resource = Resource.query.filter(Resource.id == i).one_or_none() if resource is None: - return {'status': 410, 'message': 'The requested resource is permanently deleted'}, 410 + # TODO: Allow turning this off because it can be used as an enumeration attack + return status_output.dump(code=410, message='The resource is gone'), 410 if resource.owner.client_id != client_id: - return {'status': 401, 'message': 'You do not own this resource'}, 401 + return status_output.dump(code=401, message='You are not authorised to remove this resource'), 401 db.session.delete(resource) db.session.commit() diff --git a/mrmat_python_api_flask/apis/resource/v1/model.py b/mrmat_python_api_flask/apis/resource/v1/model.py index d3daaf0..a767473 100644 --- a/mrmat_python_api_flask/apis/resource/v1/model.py +++ b/mrmat_python_api_flask/apis/resource/v1/model.py @@ -48,24 +48,15 @@ class Resource(db.Model): UniqueConstraint('owner_id', 'name', name='no_duplicate_names_per_owner') -class OwnerSchema(ma.Schema): +class OwnerSchema(ma.SQLAlchemyAutoSchema): class Meta: fields = ('id', 'client_id', 'name') - id = fields.Int(dump_only=True) - client_id = fields.Str(dump_only=True) - name = fields.Str() - -class ResourceSchema(ma.Schema): +class ResourceSchema(ma.SQLAlchemyAutoSchema): class Meta: fields = ('id', 'owner', 'name') - id = fields.Int() - owner_id = fields.Int(load_only=True) - owner = fields.Nested(lambda: OwnerSchema(), dump_only=True) # pylint: disable=W0108 - name = fields.Str() - owner_schema = OwnerSchema() owners_schema = OwnerSchema(many=True) diff --git a/tests/test_healthz.py b/tests/test_healthz.py index 96de54d..53f7ec9 100644 --- a/tests/test_healthz.py +++ b/tests/test_healthz.py @@ -32,5 +32,6 @@ def test_healthz(self, no_test_infrastructure): with no_test_infrastructure.app_client() as client: rv: Response = client.get('/healthz/') json_body = rv.get_json() - assert 'status' in json_body - assert json_body['status'] == 'OK' + assert rv.status_code == 200 + assert json_body['code'] == 200 + assert json_body['message'] == 'OK' diff --git a/tests/test_resource_v1.py b/tests/test_resource_v1.py index 64028a6..f99ebe4 100644 --- a/tests/test_resource_v1.py +++ b/tests/test_resource_v1.py @@ -63,8 +63,8 @@ def test_resource_lifecycle(self, tmpdir, local_test_infrastructure): (resp, resp_body) = rac.remove(resource_id) assert resp.status_code == 410 + assert resp_body['code'] == 410 assert resp_body['message'] == 'The requested resource is permanently deleted' - assert resp_body['status'] == 410 def test_insufficient_scope(self, tmpdir, local_test_infrastructure): with local_test_infrastructure.app_client(tmpdir) as client: @@ -81,3 +81,46 @@ def test_insufficient_scope(self, tmpdir, local_test_infrastructure): (resp, resp_body) = rac.remove(1) assert resp.status_code == 401 assert resp_body is None + + def test_duplicate_creation_fails(self, tmpdir, local_test_infrastructure): + with local_test_infrastructure.app_client(tmpdir) as client: + with local_test_infrastructure.user_token(scopes=['mpaf-read', 'mpaf-write']) as user_token: + rac = ResourceAPIClient(client, token=user_token['access_token']) + + resource_name = f'Test Resource {datetime.utcnow()}' + (resp, resp_body) = rac.create(name=resource_name) + assert resp.status_code == 201 + assert resp_body['id'] is not None + assert resp_body['name'] == resource_name + + (resp, resp_body) = rac.create(name=resource_name) + assert resp.status_code == 409 + assert resp_body['code'] == 409 + assert resp_body['message'] == 'This resource already exists' + + def test_ownership_is_maintained(self, tmpdir, local_test_infrastructure): + with local_test_infrastructure.app_client(tmpdir) as client, \ + local_test_infrastructure.user_token(user_id='test-user1', scopes=['mpaf-read', 'mpaf-write']) as token1, \ + local_test_infrastructure.user_token(user_id='test-user2', scopes=['mpaf-read', 'mpaf-write']) as token2: + + rac1 = ResourceAPIClient(client, token=token1['access_token']) + rac2 = ResourceAPIClient(client, token=token2['access_token']) + + resource_name = f'Test Resource {datetime.utcnow()}' + (resp, resp_body) = rac1.create(name=resource_name) + assert resp.status_code == 201 + + (resp, resp_body) = rac2.create(name=resource_name) + assert resp.status_code == 201 + user2_resource = resp_body['id'] + + (resp, resp_body) = rac1.modify(user2_resource, name='Test') + assert resp.status_code == 401 + assert resp_body['code'] == 401 + assert resp_body['message'] == 'You are not authorised to modify this resource' + + (resp, resp_body) = rac1.remove(user2_resource) + assert resp.status_code == 401 + assert resp_body['code'] == 401 + assert resp_body['message'] == 'You are not authorised to remove this resource' + From 6cb4704063ba64d102863f3dc1fdb5c69a888105 Mon Sep 17 00:00:00 2001 From: Mathieu Imfeld Date: Fri, 31 Dec 2021 09:22:43 +0100 Subject: [PATCH 05/39] Changed from FLASK_CONFIG to APP_CONFIG --- .idea/runConfigurations/build.xml | 6 +++--- .idea/runConfigurations/mrmat_python_api_flask__prod_.xml | 2 +- .idea/runConfigurations/test__resources_.xml | 2 +- .idea/runConfigurations/tests.xml | 2 +- .idea/runConfigurations/tests__no_infrastructure_.xml | 2 +- README.md | 6 +++--- mrmat_python_api_flask/__init__.py | 5 +++-- 7 files changed, 13 insertions(+), 12 deletions(-) diff --git a/.idea/runConfigurations/build.xml b/.idea/runConfigurations/build.xml index 65ea552..da868f9 100644 --- a/.idea/runConfigurations/build.xml +++ b/.idea/runConfigurations/build.xml @@ -6,14 +6,14 @@ -