From d6c6a7efa7fc4e5823cddd181ba4dc962b6cafc1 Mon Sep 17 00:00:00 2001 From: Stefan Kairinos Date: Wed, 24 Jan 2024 10:41:20 +0000 Subject: [PATCH] fix: new permissions and fix test cases (#61) * hi florian * import allow none * remove spaces * feedback --- .vscode/workspace.code-snippets | 51 +++++++++++++++++++ codeforlife/permissions/__init__.py | 8 +++ codeforlife/permissions/allow_none.py | 18 +++++++ .../is_cron_request_from_google.py | 9 ++-- codeforlife/serializers/base.py | 4 ++ codeforlife/tests/model_view_set.py | 35 +++++++++---- 6 files changed, 113 insertions(+), 12 deletions(-) create mode 100644 .vscode/workspace.code-snippets create mode 100644 codeforlife/permissions/allow_none.py diff --git a/.vscode/workspace.code-snippets b/.vscode/workspace.code-snippets new file mode 100644 index 0000000..8fdbf4f --- /dev/null +++ b/.vscode/workspace.code-snippets @@ -0,0 +1,51 @@ +{ + "module.docstring": { + "prefix": [ + "module.docstring", + "\"\"\"", + "'''" + ], + "scope": "python", + "body": [ + "\"\"\"", + "© Ocado Group", + "Created on $CURRENT_DATE/$CURRENT_MONTH/$CURRENT_YEAR at $CURRENT_HOUR:$CURRENT_MINUTE:$CURRENT_SECOND($CURRENT_TIMEZONE_OFFSET)." + "", + "${1:__description__}", + "\"\"\"" + ] + }, + "module.doccomment": { + "prefix": [ + "module.doccomment", + "/" + ], + "scope": "javascript,typescript,javascriptreact,typescriptreact", + "body": [ + "/**", + " * © Ocado Group", + " * Created on $CURRENT_DATE/$CURRENT_MONTH/$CURRENT_YEAR at $CURRENT_HOUR:$CURRENT_MINUTE:$CURRENT_SECOND($CURRENT_TIMEZONE_OFFSET)." + " *", + " * ${1:__description__}", + " */" + ] + }, + "pylint.disable-next": { + "prefix": [ + "# pylint" + ], + "scope": "python", + "body": [ + "# pylint: disable-next=${1:__code_name__}" + ] + }, + "mypy.ignore": { + "prefix": [ + "# type" + ], + "scope": "python", + "body": [ + "# type: ignore[${1:__code_name__}]" + ] + } +} \ No newline at end of file diff --git a/codeforlife/permissions/__init__.py b/codeforlife/permissions/__init__.py index cc21e16..ddb7bf5 100644 --- a/codeforlife/permissions/__init__.py +++ b/codeforlife/permissions/__init__.py @@ -1 +1,9 @@ +""" +© Ocado Group +Created on 23/01/2024 at 16:38:07(+00:00). + +Reusable DRF permissions. +""" + +from .allow_none import AllowNone from .is_cron_request_from_google import IsCronRequestFromGoogle diff --git a/codeforlife/permissions/allow_none.py b/codeforlife/permissions/allow_none.py new file mode 100644 index 0000000..6bc2173 --- /dev/null +++ b/codeforlife/permissions/allow_none.py @@ -0,0 +1,18 @@ +""" +© Ocado Group +Created on 23/01/2024 at 14:46:23(+00:00). +""" + +from rest_framework.permissions import BasePermission + + +class AllowNone(BasePermission): + """ + Blocks all incoming requests. + + This is the opposite of DRF's AllowAny permission: + https://www.django-rest-framework.org/api-guide/permissions/#allowany + """ + + def has_permission(self, request, view): + return False diff --git a/codeforlife/permissions/is_cron_request_from_google.py b/codeforlife/permissions/is_cron_request_from_google.py index cf98e5c..b8f49f7 100644 --- a/codeforlife/permissions/is_cron_request_from_google.py +++ b/codeforlife/permissions/is_cron_request_from_google.py @@ -1,7 +1,10 @@ +""" +© Ocado Group +Created on 23/01/2024 at 14:45:07(+00:00). +""" + from django.conf import settings from rest_framework.permissions import BasePermission -from rest_framework.request import Request -from rest_framework.views import View class IsCronRequestFromGoogle(BasePermission): @@ -11,7 +14,7 @@ class IsCronRequestFromGoogle(BasePermission): https://cloud.google.com/appengine/docs/flexible/scheduling-jobs-with-cron-yaml#securing_urls_for_cron """ - def has_permission(self, request: Request, view: View): + def has_permission(self, request, view): return ( settings.DEBUG or request.META.get("HTTP_X_APPENGINE_CRON") == "true" diff --git a/codeforlife/serializers/base.py b/codeforlife/serializers/base.py index 8f3ea63..951692e 100644 --- a/codeforlife/serializers/base.py +++ b/codeforlife/serializers/base.py @@ -19,3 +19,7 @@ class ModelSerializer(_ModelSerializer[AnyModel], t.Generic[AnyModel]): # pylint: disable-next=useless-parent-delegation def update(self, instance, validated_data: t.Dict[str, t.Any]): return super().update(instance, validated_data) + + # pylint: disable-next=useless-parent-delegation + def create(self, validated_data: t.Dict[str, t.Any]): + return super().create(validated_data) diff --git a/codeforlife/tests/model_view_set.py b/codeforlife/tests/model_view_set.py index 5bce60e..6140585 100644 --- a/codeforlife/tests/model_view_set.py +++ b/codeforlife/tests/model_view_set.py @@ -15,9 +15,10 @@ from django.utils import timezone from django.utils.http import urlencode from pyotp import TOTP +from rest_framework import status from rest_framework.response import Response from rest_framework.serializers import ModelSerializer -from rest_framework.test import APIClient, APITestCase +from rest_framework.test import APIClient, APIRequestFactory, APITestCase from rest_framework.viewsets import ModelViewSet from ..user.models import AuthFactor, User @@ -36,7 +37,12 @@ class ModelViewSetClient( responses. """ - Data = t.Dict[str, t.Any] + def __init__(self, enforce_csrf_checks: bool = False, **defaults): + super().__init__(enforce_csrf_checks, **defaults) + self.request_factory = APIRequestFactory( + enforce_csrf_checks, + **defaults, + ) _test_case: "ModelViewSetTestCase[AnyModelViewSet, AnyModelSerializer, AnyModel]" @@ -61,6 +67,7 @@ def _model_view_set_class(self): # pylint: disable-next=no-member return self._test_case.get_model_view_set_class() + Data = t.Dict[str, t.Any] StatusCodeAssertion = t.Optional[t.Union[int, t.Callable[[int], bool]]] ListFilters = t.Optional[t.Dict[str, str]] @@ -204,7 +211,7 @@ def generic( def create( self, data: Data, - status_code_assertion: StatusCodeAssertion = None, + status_code_assertion: StatusCodeAssertion = status.HTTP_201_CREATED, **kwargs, ): """Create a model. @@ -219,6 +226,7 @@ def create( response: Response = self.post( self.reverse("list"), + data=data, status_code_assertion=status_code_assertion, **kwargs, ) @@ -235,7 +243,7 @@ def create( def retrieve( self, model: AnyModel, - status_code_assertion: StatusCodeAssertion = None, + status_code_assertion: StatusCodeAssertion = status.HTTP_200_OK, **kwargs, ): """Retrieve a model. @@ -265,7 +273,7 @@ def retrieve( def list( self, models: t.Iterable[AnyModel], - status_code_assertion: StatusCodeAssertion = None, + status_code_assertion: StatusCodeAssertion = status.HTTP_200_OK, filters: ListFilters = None, **kwargs, ): @@ -302,7 +310,7 @@ def partial_update( self, model: AnyModel, data: Data, - status_code_assertion: StatusCodeAssertion = None, + status_code_assertion: StatusCodeAssertion = status.HTTP_200_OK, **kwargs, ): """Partially update a model. @@ -336,7 +344,8 @@ def partial_update( def destroy( self, model: AnyModel, - status_code_assertion: StatusCodeAssertion = None, + status_code_assertion: StatusCodeAssertion = status.HTTP_204_NO_CONTENT, + anonymized: bool = False, **kwargs, ): """Destroy a model. @@ -344,6 +353,7 @@ def destroy( Args: model: The model to destroy. status_code_assertion: The expected status code. + anonymized: Whether or not the data is anonymized. Returns: The HTTP response. @@ -355,7 +365,10 @@ def destroy( **kwargs, ) - # TODO: add standard post-destroy assertions. + if not anonymized and self.status_code_is_ok(response.status_code): + # pylint: disable-next=no-member + with self._test_case.assertRaises(model.DoesNotExist): + model.refresh_from_db() return response @@ -369,11 +382,15 @@ def login(self, **credentials): if user.session.session_auth_factors.filter( auth_factor__type=AuthFactor.Type.OTP ).exists(): + request = self.request_factory.request() + request.user = user + now = timezone.now() otp = TOTP(user.otp_secret).at(now) with patch.object(timezone, "now", return_value=now): assert super().login( - otp=otp + request=request, + otp=otp, ), f'Failed to login with OTP "{otp}" at {now}.' assert user.is_authenticated, "Failed to authenticate user."