From 34c2f802e72862ee8940846cee0e7cfff5ceed79 Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Thu, 7 Mar 2024 14:39:48 -0500 Subject: [PATCH] pre-commit pass --- .../authorized_storage_account/serializers.py | 7 ++- .../authorized_storage_account/views.py | 1 - addon_service/common/permissions.py | 8 +-- addon_service/common/serializer_fields.py | 5 +- addon_service/tests/_helpers.py | 55 ++++++++++--------- .../test_authorized_storage_account.py | 15 ++--- .../test_configured_storage_addon.py | 24 ++++---- .../test_by_type/test_resource_reference.py | 29 +++++++--- .../tests/test_by_type/test_user_reference.py | 14 ++--- app/authentication.py | 6 +- 10 files changed, 92 insertions(+), 72 deletions(-) diff --git a/addon_service/authorized_storage_account/serializers.py b/addon_service/authorized_storage_account/serializers.py index 2211e13e..80895728 100644 --- a/addon_service/authorized_storage_account/serializers.py +++ b/addon_service/authorized_storage_account/serializers.py @@ -1,4 +1,3 @@ -from rest_framework.serializers import ReadOnlyField from rest_framework_json_api import serializers from rest_framework_json_api.relations import ( HyperlinkedRelatedField, @@ -61,8 +60,10 @@ def __init__(self, *args, **kwargs): } def create(self, validate_data): - session_user_uri = self.context['request'].session.get('user_reference_uri') - account_owner, _ = UserReference.objects.get_or_create(user_uri=session_user_uri) + session_user_uri = self.context["request"].session.get("user_reference_uri") + account_owner, _ = UserReference.objects.get_or_create( + user_uri=session_user_uri + ) external_storage_service = validate_data["external_storage_service"] # TODO(ENG-5189): Update this once credentials format is finalized credentials, _ = ExternalCredentials.objects.get_or_create( diff --git a/addon_service/authorized_storage_account/views.py b/addon_service/authorized_storage_account/views.py index f138b37a..30c996cd 100644 --- a/addon_service/authorized_storage_account/views.py +++ b/addon_service/authorized_storage_account/views.py @@ -1,4 +1,3 @@ -from rest_framework.exceptions import MethodNotAllowed from addon_service.common.permissions import ( IsAuthenticated, SessionUserIsOwner, diff --git a/addon_service/common/permissions.py b/addon_service/common/permissions.py index 4a477540..0a7013a4 100644 --- a/addon_service/common/permissions.py +++ b/addon_service/common/permissions.py @@ -8,13 +8,11 @@ class IsAuthenticated(permissions.BasePermission): - def has_permission(self, request, view): return request.session.get("user_reference_uri") is not None class SessionUserIsOwner(permissions.BasePermission): - def has_object_permission(self, request, view, obj): session_user_uri = request.session.get("user_reference_uri") if session_user_uri: @@ -31,11 +29,13 @@ class SessionUserIsReferencedResourceAdmin(permissions.BasePermission): def has_permission(self, request, view): resource_uri = None try: - resource_uri = ResourceReference.objects.get( + resource_uri = ResourceReference.objects.get( id=request.data.get("authoirized_resource", {}).get("id") ).resource_uri except ResourceReference.DoesNotExist: - resource_uri = request.data.get("authorized_resource", {}).get("resource_uri") + resource_uri = request.data.get("authorized_resource", {}).get( + "resource_uri" + ) if resource_uri is None: raise exceptions.ParseError diff --git a/addon_service/common/serializer_fields.py b/addon_service/common/serializer_fields.py index 5c447905..219deae3 100644 --- a/addon_service/common/serializer_fields.py +++ b/addon_service/common/serializer_fields.py @@ -1,5 +1,8 @@ from rest_framework import serializers as drf_serializers from rest_framework_json_api import serializers as json_api_serializers -class ReadOnlyResourceRelatedField(json_api_serializers.ResourceRelatedField, drf_serializers.ReadOnlyField): + +class ReadOnlyResourceRelatedField( + json_api_serializers.ResourceRelatedField, drf_serializers.ReadOnlyField +): pass diff --git a/addon_service/tests/_helpers.py b/addon_service/tests/_helpers.py index 3d8ee3f1..785bc2a6 100644 --- a/addon_service/tests/_helpers.py +++ b/addon_service/tests/_helpers.py @@ -1,6 +1,8 @@ -import json from collections import defaultdict -from functools import partial, wraps +from functools import ( + partial, + wraps, +) from http import HTTPStatus from unittest.mock import patch @@ -21,10 +23,10 @@ def get_test_request(user=None, method="get", path="", cookies=None): _request.COOKIES[name] = value return _request -class MockOSF: +class MockOSF: def __init__(self, permissions=None): - '''A lightweight, configurable mock of OSF for testing remote permissions. + """A lightweight, configurable mock of OSF for testing remote permissions. Accepts a mapping of arbitrary resource_uris to user permissiosn and `public` status { @@ -37,17 +39,21 @@ def __init__(self, permissions=None): Users of the mock can either explicitly tell the Mock which user to assume a call is from, or they can include a cookie with the 'user_uri' in their GET request, and MockOSF will honor that user - ''' + """ self._permissions = defaultdict(lambda: defaultdict(dict)) if permissions: self._permissions.update(permissions) self._configured_caller_uri = None - self._mock_auth_request = patch('app.authentication.make_auth_request', side_effect=self._mock_user_check) - self._mock_resource_check = patch('addon_service.common.permissions.authenticate_resource', side_effect=self._mock_resource_check) + self._mock_auth_request = patch( + "app.authentication.make_auth_request", side_effect=self._mock_user_check + ) + self._mock_resource_check = patch( + "addon_service.common.permissions.authenticate_resource", + side_effect=self._mock_resource_check, + ) self._mock_auth_request.start() self._mock_resource_check.start() - def stop(self): self._mock_auth_request.stop() self._mock_resource_check.stop() @@ -59,7 +65,7 @@ def configure_user_role(self, user_uri, resource_uri, role): self._permissions[resource_uri][user_uri] = role def configure_resource_visibility(self, resource_uri, *, public=True): - self._permissions[resource_uri]['public'] = public + self._permissions[resource_uri]["public"] = public def _get_assumed_caller(self, cookies=None): if self._configured_caller_uri: @@ -71,18 +77,18 @@ def _get_assumed_caller(self, cookies=None): def _get_user_permissions(self, user_uri, resource_uri): # Use of defaultdict means this will always have some value role = self._permissions[resource_uri][user_uri] - if role == 'read': - return ['read'] - if role == 'write': - return ['read', 'write'] - if role == 'admin': - return ['read', 'write', 'admin'] - if self._permissions[resource_uri]['public']: - return ['read'] + if role == "read": + return ["read"] + if role == "write": + return ["read", "write"] + if role == "admin": + return ["read", "write", "admin"] + if self._permissions[resource_uri]["public"]: + return ["read"] return [] def _mock_user_check(self, *args, **kwargs): - caller_uri = self._get_assumed_caller(cookies=kwargs.get('cookies')) + caller_uri = self._get_assumed_caller(cookies=kwargs.get("cookies")) return {"data": {"links": {"iri": caller_uri}}} def _mock_resource_check(self, request, uri, required_permission, *args, **kwargs): @@ -93,20 +99,17 @@ def _mock_resource_check(self, request, uri, required_permission, *args, **kwarg return uri # mimicking behavior from the check being mocked - def with_mocked_httpx_get(response_status=HTTPStatus.OK, user_uri=None): - - def decorator(func): @wraps(func) def wrapper(testcase, *args, **kwargs): mock_side_effect = partial( _mock_httpx_response, response_status=response_status, - user_uri=user_uri or testcase._user.user_uri + user_uri=user_uri or testcase._user.user_uri, ) - patcher = patch('httpx.Client.get', side_effect=mock_side_effect) - client = patcher.start() + patcher = patch("httpx.Client.get", side_effect=mock_side_effect) + patcher.start() test_result = func(testcase, *args, **kwargs) patcher.stop() return test_result @@ -127,9 +130,7 @@ def _mock_httpx_response(response_status, user_uri, url, *args, **kwargs): guid = url.rstrip("/").split("/")[-1] payload = { "data": { - "attributes": { - "current_user_permissions": ["read", "write", "admin"] - }, + "attributes": {"current_user_permissions": ["read", "write", "admin"]}, "links": {"iri": f"{settings.URI_ID}{guid}"}, } } diff --git a/addon_service/tests/test_by_type/test_authorized_storage_account.py b/addon_service/tests/test_by_type/test_authorized_storage_account.py index 71837c90..aa011f24 100644 --- a/addon_service/tests/test_by_type/test_authorized_storage_account.py +++ b/addon_service/tests/test_by_type/test_authorized_storage_account.py @@ -11,8 +11,8 @@ ) from addon_service.tests import _factories from addon_service.tests._helpers import ( + MockOSF, get_test_request, - MockOSF ) from app import settings @@ -82,8 +82,12 @@ def test_post(self): reverse("authorized-storage-accounts-list"), payload, format="vnd.api+json" ) self.assertEqual(_resp.status_code, 201) - created_account_id = int(_resp.data['url'].rstrip('/').split('/')[-1]) - self.assertTrue(external_service.authorized_storage_accounts.filter(id=created_account_id).exists()) + created_account_id = int(_resp.data["url"].rstrip("/").split("/")[-1]) + self.assertTrue( + external_service.authorized_storage_accounts.filter( + id=created_account_id + ).exists() + ) def test_methods_not_allowed(self): _methods_not_allowed = { @@ -138,7 +142,6 @@ def setUpTestData(cls): cls._user = cls._asa.external_account.owner cls._view = AuthorizedStorageAccountViewSet.as_view({"get": "retrieve"}) - def setUp(self): super().setUp() self._mock_osf = MockOSF() @@ -172,9 +175,7 @@ def test_unauthorized(self): def test_wrong_user(self): _resp = self._view( - get_test_request( - cookies={settings.USER_REFERENCE_COOKIE: "wrong/10"} - ), + get_test_request(cookies={settings.USER_REFERENCE_COOKIE: "wrong/10"}), pk=self._asa.pk, ) self.assertEqual(_resp.status_code, HTTPStatus.FORBIDDEN) diff --git a/addon_service/tests/test_by_type/test_configured_storage_addon.py b/addon_service/tests/test_by_type/test_configured_storage_addon.py index 850b8403..5e3328ec 100644 --- a/addon_service/tests/test_by_type/test_configured_storage_addon.py +++ b/addon_service/tests/test_by_type/test_configured_storage_addon.py @@ -5,7 +5,10 @@ from django.urls import reverse from rest_framework.test import APITestCase -from addon_service.models import ConfiguredStorageAddon, ResourceReference +from addon_service.models import ( + ConfiguredStorageAddon, + ResourceReference, +) from addon_service.tests import _factories as test_factories from addon_service.tests._helpers import MockOSF from app import settings @@ -33,7 +36,7 @@ def setUp(self): self._mock_osf = MockOSF() self.addCleanup(self._mock_osf.stop) self._mock_osf.configure_user_role( - self._user.user_uri, self._configured_storage_addon.resource_uri, 'admin' + self._user.user_uri, self._configured_storage_addon.resource_uri, "admin" ) self.client.cookies[settings.USER_REFERENCE_COOKIE] = self._user.user_uri @@ -57,12 +60,12 @@ def related_url(self, related_field): class ConfiguredStorageAddonAPITests(BaseAPITest): - def test_get_detail(self): response = self.client.get(self.detail_url()) self.assertEqual(response.status_code, HTTPStatus.OK) self.assertEqual( - response.json()['data']['attributes']["root_folder"], self._configured_storage_addon.root_folder + response.json()["data"]["attributes"]["root_folder"], + self._configured_storage_addon.root_folder, ) def test_methods_not_allowed(self): @@ -90,7 +93,6 @@ def test_model_loading(self): class ConfiguredStorageAddonViewSetTests(BaseAPITest): - def test_viewset_retrieve(self): response = self.client.get(self.detail_url()) self.assertEqual(response.status_code, HTTPStatus.OK) @@ -109,9 +111,7 @@ class ConfiguredStorageAddonPOSTTests(BaseAPITest): "base_account": { "data": {"type": "authorized-storage-accounts", "id": ""} }, - "authorized_resource": { - "data": {"type": "resource-references"} - }, + "authorized_resource": {"data": {"type": "resource-references"}}, }, } } @@ -124,8 +124,12 @@ def setUp(self): def test_post_with_new_resource(self): new_resource_uri = "http://example.com/new_resource/" - self._mock_osf.configure_user_role(self._user.user_uri, new_resource_uri, 'admin') - self.assertFalse(ResourceReference.objects.filter(resource_uri=new_resource_uri).exists()) + self._mock_osf.configure_user_role( + self._user.user_uri, new_resource_uri, "admin" + ) + self.assertFalse( + ResourceReference.objects.filter(resource_uri=new_resource_uri).exists() + ) self.default_payload["data"]["relationships"]["authorized_resource"]["data"][ "resource_uri" ] = new_resource_uri diff --git a/addon_service/tests/test_by_type/test_resource_reference.py b/addon_service/tests/test_by_type/test_resource_reference.py index 1cd18278..97bfd657 100644 --- a/addon_service/tests/test_by_type/test_resource_reference.py +++ b/addon_service/tests/test_by_type/test_resource_reference.py @@ -10,8 +10,8 @@ from addon_service.resource_reference.views import ResourceReferenceViewSet from addon_service.tests import _factories from addon_service.tests._helpers import ( - get_test_request, MockOSF, + get_test_request, with_mocked_httpx_get, ) from app import settings @@ -28,11 +28,14 @@ def setUpTestData(cls): def setUp(self): super().setUp() self._mock_osf = MockOSF() - self._mock_osf.configure_user_role(user_uri=self._user.user_uri, resource_uri=self._resource.resource_uri, role='admin') + self._mock_osf.configure_user_role( + user_uri=self._user.user_uri, + resource_uri=self._resource.resource_uri, + role="admin", + ) self.addCleanup(self._mock_osf.stop) self.client.cookies[settings.USER_REFERENCE_COOKIE] = self._user.user_uri - @property def _detail_path(self): return reverse("resource-references-detail", kwargs={"pk": self._resource.pk}) @@ -116,7 +119,9 @@ def setUpTestData(cls): def setUp(self): self._mock_osf = MockOSF() self.addCleanup(self._mock_osf.stop) - self._mock_osf.configure_user_role(self._user.user_uri, self._resource.resource_uri, 'admin') + self._mock_osf.configure_user_role( + self._user.user_uri, self._resource.resource_uri, "admin" + ) def test_get(self): _resp = self._view( @@ -139,17 +144,23 @@ def test_get(self): ) def test_unauthorized__private_resource(self): - self._mock_osf.configure_resource_visibility(self._resource.resource_uri, public=False) + self._mock_osf.configure_resource_visibility( + self._resource.resource_uri, public=False + ) _anon_resp = self._view(get_test_request(), pk=self._resource.pk) self.assertEqual(_anon_resp.status_code, HTTPStatus.FORBIDDEN) def test_unauthorized__public_resource(self): - self._mock_osf.configure_resource_visibility(self._resource.resource_uri, public=True) + self._mock_osf.configure_resource_visibility( + self._resource.resource_uri, public=True + ) _anon_resp = self._view(get_test_request(), pk=self._resource.pk) self.assertEqual(_anon_resp.status_code, HTTPStatus.OK) def test_wrong_user__pivate_resource(self): - self._mock_osf.configure_resource_visibility(self._resource.resource_uri, public=False) + self._mock_osf.configure_resource_visibility( + self._resource.resource_uri, public=False + ) _resp = self._view( get_test_request( cookies={settings.USER_REFERENCE_COOKIE: "this is wrong user auth"} @@ -159,7 +170,9 @@ def test_wrong_user__pivate_resource(self): self.assertEqual(_resp.status_code, HTTPStatus.FORBIDDEN) def test_wrong_user__public_resource(self): - self._mock_osf.configure_resource_visibility(self._resource.resource_uri, public=True) + self._mock_osf.configure_resource_visibility( + self._resource.resource_uri, public=True + ) _resp = self._view( get_test_request( cookies={settings.USER_REFERENCE_COOKIE: "this is wrong user auth"} diff --git a/addon_service/tests/test_by_type/test_user_reference.py b/addon_service/tests/test_by_type/test_user_reference.py index e91bb593..e6f732be 100644 --- a/addon_service/tests/test_by_type/test_user_reference.py +++ b/addon_service/tests/test_by_type/test_user_reference.py @@ -10,9 +10,8 @@ from addon_service import models as db from addon_service.tests import _factories from addon_service.tests._helpers import ( + MockOSF, get_test_request, - with_mocked_httpx_get, - MockOSF ) from addon_service.user_reference.views import UserReferenceViewSet from app import settings @@ -124,9 +123,7 @@ def setUp(self): def test_get(self): _resp = self._view( get_test_request( - cookies={ - settings.USER_REFERENCE_COOKIE: self._user.user_uri - }, + cookies={settings.USER_REFERENCE_COOKIE: self._user.user_uri}, ), pk=self._user.pk, ) @@ -146,9 +143,11 @@ def test_get(self): ) def test_wrong_user(self): - self._mock_osf.configure_assumed_caller('wrong') + self._mock_osf.configure_assumed_caller("wrong") _resp = self._view( - get_test_request(cookies={settings.USER_REFERENCE_COOKIE: "this is the wrong cookie"}), + get_test_request( + cookies={settings.USER_REFERENCE_COOKIE: "this is the wrong cookie"} + ), pk=self._user.pk, ) self.assertEqual(_resp.status_code, HTTPStatus.FORBIDDEN) @@ -173,7 +172,6 @@ def setUp(self): cookies={settings.USER_REFERENCE_COOKIE: self._user.user_uri}, ) - def test_get_related__empty(self): _resp = self._related_view( self.request, diff --git a/app/authentication.py b/app/authentication.py index ef563883..915af79f 100644 --- a/app/authentication.py +++ b/app/authentication.py @@ -1,11 +1,11 @@ import base64 import binascii -import httpx from urllib.parse import ( urlparse, urlunparse, ) +import httpx from django.conf import settings from django.contrib.auth.models import AnonymousUser from rest_framework import ( @@ -14,7 +14,7 @@ ) -#TODO: Improve dockerization of OSF so that we don't need this +# TODO: Improve dockerization of OSF so that we don't need this def handle_redirects(response): """Redirect fix for localhost during local development.""" if settings.DEBUG and response.status_code in {301, 302, 303, 307, 308}: @@ -33,7 +33,7 @@ class SkipAuthMethod(exceptions.APIException): def authenticate_resource(request, uri, required_permission): - print('WHAT?!!!!\n\n\n') + print("WHAT?!!!!\n\n\n") resource_url = settings.RESOURCE_REFERENCE_LOOKUP_URL.format( uri.replace(settings.URI_ID, "").rstrip("/") )