From 4dacf23405e6fdf453e0a40d9337e7e580cb2bf1 Mon Sep 17 00:00:00 2001 From: Behoston Date: Thu, 14 Mar 2019 15:39:24 +0100 Subject: [PATCH 1/3] Add permission checker interface. --- flask_jsonapi/permissions.py | 131 ++++++++++++++++++++++++++++++ tests/test_permissions.py | 153 +++++++++++++++++++++++++++++++++++ 2 files changed, 284 insertions(+) create mode 100644 flask_jsonapi/permissions.py create mode 100644 tests/test_permissions.py diff --git a/flask_jsonapi/permissions.py b/flask_jsonapi/permissions.py new file mode 100644 index 0000000..6f819c6 --- /dev/null +++ b/flask_jsonapi/permissions.py @@ -0,0 +1,131 @@ +import abc +import http +import logging +import typing + +from flask_jsonapi import exceptions + +READ_ACTION = 'read' +DESTROY_ACTION = 'destroy' +UPDATE_ACTION = 'update' +LIST_ACTION = 'list' +CREATE_ACTION = 'create' + +logger = logging.getLogger(__name__) + + +class PermissionException(exceptions.JsonApiException): + status = http.HTTPStatus.FORBIDDEN.value + title = http.HTTPStatus.FORBIDDEN.phrase + + def __init__(self, source=None, detail=None, *args, title=None, status=None, **kwargs): + super().__init__(source, detail, *args, title=title, status=status, **kwargs) + + +Resource = typing.TypeVar('Resource') + + +class PermissionChecker(abc.ABC): + @abc.abstractmethod + def check_read_permission(self, *, resource: Resource) -> Resource: + """ + :param resource: resource for which we want to check permissions + :return: resource if user has permission, exception in other case + """ + pass + + @abc.abstractmethod + def check_destroy_permission(self, *, resource: Resource) -> Resource: + """ + :param resource: resource for which we want to check permissions + :return: resource if user has permission, exception in other case + """ + pass + + @abc.abstractmethod + def check_update_permission(self, *, resource: Resource, data: dict) -> typing.Tuple[Resource, dict]: + """ + :param resource: resource for which we want to check permissions + :param data: new data + :return: new data if user has permission, exception in other case + """ + pass + + def check_list_permission(self, *, resources: typing.List[Resource]) -> typing.List[Resource]: + """ + :param resources: resources list for which we want to check permissions + :return: filtered resources list + """ + result = [] + for resource in resources: + try: + result.append(self.check_read_permission(resource=resource)) + except PermissionException: + pass + return result + + @abc.abstractmethod + def check_create_permission(self, *, data: dict) -> dict: + """ + :param data: data for which we want to check permissions + :return: data if user has permission, exception in other case + """ + pass + + +class ObjectLevelPermissionChecker(PermissionChecker): + class ObjectIdNotFoundInData(Exception): + pass + + @property + @abc.abstractmethod + def object_id_attribute(self) -> str: + pass + + def check_create_permission(self, *, data: dict) -> dict: + return self._check_data_permission(data=data, action=CREATE_ACTION) + + def check_read_permission(self, *, resource: Resource) -> Resource: + return self._check_resource_permission(resource=resource, action=READ_ACTION) + + def check_update_permission(self, *, resource: Resource, data: dict) -> typing.Tuple[Resource, dict]: + self._check_resource_permission(resource=resource, action=UPDATE_ACTION) + try: + self._check_data_permission(data=data, action=UPDATE_ACTION) + except self.ObjectIdNotFoundInData: + pass + return resource, data + + def check_destroy_permission(self, *, resource) -> Resource: + return self._check_resource_permission(resource=resource, action=DESTROY_ACTION) + + def _check_resource_permission(self, *, resource: Resource, action: str) -> Resource: + object_id = self.get_object_id_from_resource(resource) + self.check_or_raise(object_id=object_id, action=action) + return resource + + def _check_data_permission(self, *, data: dict, action: str): + object_id = self.get_object_id_from_data(data) + self.check_or_raise(object_id=object_id, action=action) + return data + + def get_object_id_from_resource(self, resource: Resource): + return getattr(resource, self.object_id_attribute) + + def get_object_id_from_data(self, data: dict): + try: + return data[self.object_id_attribute] + except KeyError: + raise self.ObjectIdNotFoundInData + + def check_or_raise(self, *, object_id, action: str): + has_permission = self.has_permission( + object_id=object_id, + action=action, + ) + if not has_permission: + raise PermissionException + + @abc.abstractmethod + def has_permission(self, object_id, action) -> bool: + pass diff --git a/tests/test_permissions.py b/tests/test_permissions.py new file mode 100644 index 0000000..6170376 --- /dev/null +++ b/tests/test_permissions.py @@ -0,0 +1,153 @@ +import uuid + +from dataclasses import dataclass +from unittest import mock + +import pytest + +from flask_jsonapi import permissions + + +class PermissionCheckerForTests(permissions.ObjectLevelPermissionChecker): + object_id_attribute = 'object_id' + + def has_permission(self, object_id, action) -> bool: + return False + + +@dataclass +class ResourceForTests: + object_id: uuid.UUID + + +class TestObjectLevelPermissionChecker: + def test_check_read_permission(self): + checker = PermissionCheckerForTests() + checker.has_permission = mock.MagicMock() + resource = ResourceForTests(uuid.uuid4()) + + checker.check_read_permission(resource=resource) + + checker.has_permission.assert_called_with(action=permissions.READ_ACTION, object_id=resource.object_id) + + def test_check_list_permission(self): + checker = PermissionCheckerForTests() + checker.has_permission = mock.MagicMock() + resource0 = ResourceForTests(uuid.uuid4()) + resource1 = ResourceForTests(uuid.uuid4()) + resource2 = ResourceForTests(uuid.uuid4()) + resource3 = ResourceForTests(uuid.uuid4()) + resource4 = ResourceForTests(uuid.uuid4()) + + checker.check_list_permission(resources=[resource0, resource1, resource2, resource3, resource4]) + + checker.has_permission.assert_has_calls( + [ + mock.call(action=permissions.READ_ACTION, object_id=resource0.object_id), + mock.call(action=permissions.READ_ACTION, object_id=resource1.object_id), + mock.call(action=permissions.READ_ACTION, object_id=resource2.object_id), + mock.call(action=permissions.READ_ACTION, object_id=resource3.object_id), + mock.call(action=permissions.READ_ACTION, object_id=resource4.object_id), + ], + any_order=True, + ) + + def test_check_destroy_permission(self): + checker = PermissionCheckerForTests() + checker.has_permission = mock.MagicMock() + resource = ResourceForTests(uuid.uuid4()) + + checker.check_destroy_permission(resource=resource) + + checker.has_permission.assert_called_with(action=permissions.DESTROY_ACTION, object_id=resource.object_id) + + def test_check_create_permission(self): + checker = PermissionCheckerForTests() + checker.has_permission = mock.MagicMock() + data = {'object_id': str(uuid.uuid4())} + + checker.check_create_permission(data=data) + + checker.has_permission.assert_called_with(action=permissions.CREATE_ACTION, object_id=data['object_id']) + + def test_check_update_permission(self): + checker = PermissionCheckerForTests() + checker.has_permission = mock.MagicMock() + resource_id = uuid.uuid4() + resource = ResourceForTests(resource_id) + data = {'object_id': str(resource_id)} + + checker.check_update_permission(data=data, resource=resource) + + checker.has_permission.assert_has_calls( + [ + mock.call(action=permissions.UPDATE_ACTION, object_id=resource_id), + mock.call(action=permissions.UPDATE_ACTION, object_id=str(resource_id)), + ], + any_order=True, + ) + + def test_exception_raised_when_not_permission_to_read(self): + checker = PermissionCheckerForTests() + resource_id = uuid.uuid4() + resource = ResourceForTests(resource_id) + + with pytest.raises(permissions.PermissionException): + checker.check_read_permission(resource=resource) + + def test_exception_raised_when_not_permission_to_destroy(self): + checker = PermissionCheckerForTests() + resource_id = uuid.uuid4() + resource = ResourceForTests(resource_id) + + with pytest.raises(permissions.PermissionException): + checker.check_destroy_permission(resource=resource) + + def test_exception_raised_when_not_permission_to_update(self): + checker = PermissionCheckerForTests() + resource_id = uuid.uuid4() + data = {'object_id': resource_id} + resource = ResourceForTests(resource_id) + + with pytest.raises(permissions.PermissionException): + checker.check_update_permission(resource=resource, data=data) + + def test_exception_raised_when_not_permission_to_create(self): + checker = PermissionCheckerForTests() + resource_id = uuid.uuid4() + data = {'object_id': resource_id} + + with pytest.raises(permissions.PermissionException): + checker.check_create_permission(data=data) + + def test_exception_not_raised_when_not_permission_to_list(self): + checker = PermissionCheckerForTests() + resource0 = ResourceForTests(uuid.uuid4()) + resource1 = ResourceForTests(uuid.uuid4()) + resource2 = ResourceForTests(uuid.uuid4()) + resource3 = ResourceForTests(uuid.uuid4()) + resource4 = ResourceForTests(uuid.uuid4()) + + result = checker.check_list_permission(resources=[resource0, resource1, resource2, resource3, resource4]) + + assert result == [] + + def test_check_list_permissions_should_filter_resources(self): + checker = PermissionCheckerForTests() + checker.has_permission = mock.MagicMock() + checker.has_permission.side_effect = [ + True, + False, + False, + False, + True, + ] + resource0 = ResourceForTests(uuid.uuid4()) + resource1 = ResourceForTests(uuid.uuid4()) + resource2 = ResourceForTests(uuid.uuid4()) + resource3 = ResourceForTests(uuid.uuid4()) + resource4 = ResourceForTests(uuid.uuid4()) + + result = checker.check_list_permission(resources=[resource0, resource1, resource2, resource3, resource4]) + + assert result == [resource0, resource4] From c65da6a1d0de9ac90648a347660faf44577493c2 Mon Sep 17 00:00:00 2001 From: Behoston Date: Thu, 14 Mar 2019 17:38:31 +0100 Subject: [PATCH 2/3] Refactor: move permissions to package. --- flask_jsonapi/permissions/__init__.py | 19 ++++++++++++++++++ flask_jsonapi/permissions/actions.py | 5 +++++ .../checkers.py} | 20 ++++++------------- 3 files changed, 30 insertions(+), 14 deletions(-) create mode 100644 flask_jsonapi/permissions/__init__.py create mode 100644 flask_jsonapi/permissions/actions.py rename flask_jsonapi/{permissions.py => permissions/checkers.py} (91%) diff --git a/flask_jsonapi/permissions/__init__.py b/flask_jsonapi/permissions/__init__.py new file mode 100644 index 0000000..77269fe --- /dev/null +++ b/flask_jsonapi/permissions/__init__.py @@ -0,0 +1,19 @@ +from .actions import CREATE_ACTION +from .actions import DESTROY_ACTION +from .actions import LIST_ACTION +from .actions import READ_ACTION +from .actions import UPDATE_ACTION +from .checkers import ObjectLevelPermissionChecker +from .checkers import PermissionChecker +from .checkers import PermissionException + +__all__ = [ + CREATE_ACTION, + READ_ACTION, + UPDATE_ACTION, + DESTROY_ACTION, + LIST_ACTION, + PermissionChecker, + ObjectLevelPermissionChecker, + PermissionException, +] diff --git a/flask_jsonapi/permissions/actions.py b/flask_jsonapi/permissions/actions.py new file mode 100644 index 0000000..64d1d7f --- /dev/null +++ b/flask_jsonapi/permissions/actions.py @@ -0,0 +1,5 @@ +READ_ACTION = 'read' +DESTROY_ACTION = 'destroy' +UPDATE_ACTION = 'update' +LIST_ACTION = 'list' +CREATE_ACTION = 'create' diff --git a/flask_jsonapi/permissions.py b/flask_jsonapi/permissions/checkers.py similarity index 91% rename from flask_jsonapi/permissions.py rename to flask_jsonapi/permissions/checkers.py index 6f819c6..174b1f3 100644 --- a/flask_jsonapi/permissions.py +++ b/flask_jsonapi/permissions/checkers.py @@ -1,17 +1,9 @@ import abc import http -import logging import typing from flask_jsonapi import exceptions - -READ_ACTION = 'read' -DESTROY_ACTION = 'destroy' -UPDATE_ACTION = 'update' -LIST_ACTION = 'list' -CREATE_ACTION = 'create' - -logger = logging.getLogger(__name__) +from flask_jsonapi.permissions import actions class PermissionException(exceptions.JsonApiException): @@ -83,21 +75,21 @@ def object_id_attribute(self) -> str: pass def check_create_permission(self, *, data: dict) -> dict: - return self._check_data_permission(data=data, action=CREATE_ACTION) + return self._check_data_permission(data=data, action=actions.CREATE_ACTION) def check_read_permission(self, *, resource: Resource) -> Resource: - return self._check_resource_permission(resource=resource, action=READ_ACTION) + return self._check_resource_permission(resource=resource, action=actions.READ_ACTION) def check_update_permission(self, *, resource: Resource, data: dict) -> typing.Tuple[Resource, dict]: - self._check_resource_permission(resource=resource, action=UPDATE_ACTION) + self._check_resource_permission(resource=resource, action=actions.UPDATE_ACTION) try: - self._check_data_permission(data=data, action=UPDATE_ACTION) + self._check_data_permission(data=data, action=actions.UPDATE_ACTION) except self.ObjectIdNotFoundInData: pass return resource, data def check_destroy_permission(self, *, resource) -> Resource: - return self._check_resource_permission(resource=resource, action=DESTROY_ACTION) + return self._check_resource_permission(resource=resource, action=actions.DESTROY_ACTION) def _check_resource_permission(self, *, resource: Resource, action: str) -> Resource: object_id = self.get_object_id_from_resource(resource) From b9376d96b4fcc0556ff708139a65803ff9959f2a Mon Sep 17 00:00:00 2001 From: Behoston Date: Fri, 15 Mar 2019 10:27:08 +0100 Subject: [PATCH 3/3] Add protected views. --- flask_jsonapi/permissions/__init__.py | 6 ++ flask_jsonapi/permissions/views.py | 79 +++++++++++++++++++ tests/conftest.py | 13 +++ tests/permissions/__init__.py | 0 .../test_chckers.py} | 0 5 files changed, 98 insertions(+) create mode 100644 flask_jsonapi/permissions/views.py create mode 100644 tests/permissions/__init__.py rename tests/{test_permissions.py => permissions/test_chckers.py} (100%) diff --git a/flask_jsonapi/permissions/__init__.py b/flask_jsonapi/permissions/__init__.py index 77269fe..d4277c4 100644 --- a/flask_jsonapi/permissions/__init__.py +++ b/flask_jsonapi/permissions/__init__.py @@ -6,6 +6,9 @@ from .checkers import ObjectLevelPermissionChecker from .checkers import PermissionChecker from .checkers import PermissionException +from .views import ProtectedDetailView +from .views import ProtectedListView +from .views import ProtectedViewSet __all__ = [ CREATE_ACTION, @@ -16,4 +19,7 @@ PermissionChecker, ObjectLevelPermissionChecker, PermissionException, + ProtectedDetailView, + ProtectedListView, + ProtectedViewSet, ] diff --git a/flask_jsonapi/permissions/views.py b/flask_jsonapi/permissions/views.py new file mode 100644 index 0000000..0ebc159 --- /dev/null +++ b/flask_jsonapi/permissions/views.py @@ -0,0 +1,79 @@ +import abc +import logging + +from flask_jsonapi import ResourceRepositoryDetailView +from flask_jsonapi import ResourceRepositoryListView +from flask_jsonapi import ResourceRepositoryViewSet +from flask_jsonapi import descriptors +from flask_jsonapi import exceptions + +from . import checkers + +logger = logging.getLogger(__name__) + + +class ProtectedDetailView(ResourceRepositoryDetailView): + def __init__(self, permission_checker: checkers.PermissionChecker, **kwargs): + super().__init__(**kwargs) + self.permission_checker = permission_checker + + def read(self, id): + resource = super().read(id) + resource = self.permission_checker.check_read_permission(resource=resource) + return resource + + def destroy(self, id): + resource = super().read(id) + resource = self.permission_checker.check_destroy_permission(resource=resource) + return super().destroy(resource.id) + + def update(self, id, data, **kwargs): + resource = super().read(id) + resource, data = self.permission_checker.check_update_permission(resource=resource, data=data) + return super().update(id, data, **kwargs) + + +class ProtectedListView(ResourceRepositoryListView, abc.ABC): + def __init__(self, permission_checker: checkers.PermissionChecker, **kwargs): + super().__init__(**kwargs) + self.permission_checker = permission_checker + + def get(self, *args, **kwargs): + try: + return super().get(*args, **kwargs) + except checkers.PermissionException: + raise exceptions.ForbiddenError("You don't have permissions") + + def post(self, *args, **kwargs): + try: + return super().post(*args, **kwargs) + except checkers.PermissionException: + raise exceptions.ForbiddenError("You don't have permissions") + + def read_many(self, filters: dict, **kwargs): + filters = self._apply_permission_filter(filters) + resources = super().read_many(filters, **kwargs) + length_before_permission = len(resources) + resources = self.permission_checker.check_list_permission(resources=resources) + if length_before_permission != len(resources): + logger.warning('No permission for some items!', extra=kwargs) + return resources + + @abc.abstractmethod + def _apply_permission_filter(self, filters: dict) -> dict: + pass + + def create(self, data: dict, **kwargs): + data = self.permission_checker.check_create_permission(data=data) + return super().create(data, **kwargs) + + +class ProtectedViewSet(ResourceRepositoryViewSet): + detail_view_cls = ProtectedDetailView + list_view_cls: ProtectedListView + permission_checker: checkers.PermissionChecker = descriptors.NotImplementedProperty('permission_checker') + + def get_views_kwargs(self): + kwargs = super().get_views_kwargs() + kwargs['permission_checker'] = self.permission_checker + return kwargs diff --git a/tests/conftest.py b/tests/conftest.py index 1d176f6..2bf7bbf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,3 +6,16 @@ def app(): application = flask.Flask(__name__) return application + + +@pytest.fixture +def api(app): + from flask_jsonapi import api + application_api = api.Api(app) + return application_api + + +@pytest.fixture +def test_client(app): + with app.test_client() as client: + yield client diff --git a/tests/permissions/__init__.py b/tests/permissions/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_permissions.py b/tests/permissions/test_chckers.py similarity index 100% rename from tests/test_permissions.py rename to tests/permissions/test_chckers.py