diff --git a/addon_service/authorized_storage_account/models.py b/addon_service/authorized_storage_account/models.py index 43766e6f..b257a0dc 100644 --- a/addon_service/authorized_storage_account/models.py +++ b/addon_service/authorized_storage_account/models.py @@ -33,4 +33,4 @@ def account_owner(self): @property def owner_reference(self): - return self.external_account.owner.user_uri + return self.account_owner.user_uri diff --git a/addon_service/authorized_storage_account/serializers.py b/addon_service/authorized_storage_account/serializers.py index 519a86a2..2211e13e 100644 --- a/addon_service/authorized_storage_account/serializers.py +++ b/addon_service/authorized_storage_account/serializers.py @@ -1,3 +1,4 @@ +from rest_framework.serializers import ReadOnlyField from rest_framework_json_api import serializers from rest_framework_json_api.relations import ( HyperlinkedRelatedField, @@ -5,6 +6,7 @@ ) from rest_framework_json_api.utils import get_resource_type_from_model +from addon_service.common.serializer_fields import ReadOnlyResourceRelatedField from addon_service.models import ( AuthorizedStorageAccount, ConfiguredStorageAddon, @@ -18,19 +20,6 @@ RESOURCE_NAME = get_resource_type_from_model(AuthorizedStorageAccount) -class AccountOwnerField(ResourceRelatedField): - def to_internal_value(self, data): - return UserReference.objects.get(user_uri=data["id"]) - - -class ExternalStorageServiceField(ResourceRelatedField): - def to_internal_value(self, data): - external_storage_service, _ = ExternalStorageService.objects.get_or_create( - auth_uri=data["id"], - ) - return external_storage_service - - class AuthorizedStorageAccountSerializer(serializers.HyperlinkedModelSerializer): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -42,12 +31,12 @@ def __init__(self, *args, **kwargs): url = serializers.HyperlinkedIdentityField( view_name=f"{RESOURCE_NAME}-detail", required=False ) - account_owner = AccountOwnerField( + account_owner = ReadOnlyResourceRelatedField( many=False, queryset=UserReference.objects.all(), related_link_view_name=f"{RESOURCE_NAME}-related", ) - external_storage_service = ExternalStorageServiceField( + external_storage_service = ResourceRelatedField( queryset=ExternalStorageService.objects.all(), many=False, related_link_view_name=f"{RESOURCE_NAME}-related", @@ -72,15 +61,16 @@ def __init__(self, *args, **kwargs): } def create(self, validate_data): - account_owner = validate_data["account_owner"] + 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, created = ExternalCredentials.objects.get_or_create( + credentials, _ = ExternalCredentials.objects.get_or_create( oauth_key=validate_data["username"], oauth_secret=validate_data["password"], ) - external_account, created = ExternalAccount.objects.get_or_create( + external_account, _ = ExternalAccount.objects.get_or_create( owner=account_owner, credentials=credentials, credentials_issuer=external_storage_service.credentials_issuer, diff --git a/addon_service/authorized_storage_account/views.py b/addon_service/authorized_storage_account/views.py index 3dddce97..f138b37a 100644 --- a/addon_service/authorized_storage_account/views.py +++ b/addon_service/authorized_storage_account/views.py @@ -1,5 +1,6 @@ +from rest_framework.exceptions import MethodNotAllowed from addon_service.common.permissions import ( - CanCreateASA, + IsAuthenticated, SessionUserIsOwner, ) from addon_service.common.viewsets import RetrieveWriteViewSet @@ -18,7 +19,4 @@ def get_permissions(self): if self.action in ["retrieve", "retrieve_related", "update", "destroy"]: return [SessionUserIsOwner()] - elif self.action == "create": - return [CanCreateASA()] - else: - raise NotImplementedError("view action permission not implemented") + return [IsAuthenticated()] diff --git a/addon_service/common/permissions.py b/addon_service/common/permissions.py index 09c7746a..4a477540 100644 --- a/addon_service/common/permissions.py +++ b/addon_service/common/permissions.py @@ -3,14 +3,17 @@ permissions, ) -from addon_service.models import UserReference +from addon_service.models import ResourceReference from app.authentication import authenticate_resource +class IsAuthenticated(permissions.BasePermission): + + def has_permission(self, request, view): + return request.session.get("user_reference_uri") is not None + + class SessionUserIsOwner(permissions.BasePermission): - """ - Decorator to fetch 'user_reference_uri' from the session and pass it to the permission check function. - """ def has_object_permission(self, request, view, obj): session_user_uri = request.session.get("user_reference_uri") @@ -19,30 +22,22 @@ def has_object_permission(self, request, view, obj): return False -class SessionUserIsResourceReferenceOwner(permissions.BasePermission): +class SessionUserCanViewReferencedResource(permissions.BasePermission): def has_object_permission(self, request, view, obj): - resource_uri = authenticate_resource(request, obj.resource_uri, "read") - return obj.resource_uri == resource_uri + return authenticate_resource(request, obj.resource_uri, "read") -class CanCreateCSA(permissions.BasePermission): +class SessionUserIsReferencedResourceAdmin(permissions.BasePermission): def has_permission(self, request, view): - authorized_resource_id = request.data.get("authorized_resource", {}).get("id") - if authenticate_resource(request, authorized_resource_id, "admin"): - return True - return False + resource_uri = None + try: + 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") + if resource_uri is None: + raise exceptions.ParseError -class CanCreateASA(permissions.BasePermission): - def has_permission(self, request, view): - session_user_uri = request.session.get("user_reference_uri") - request_user_uri = request.data.get("account_owner", {}).get("id") - if not session_user_uri == request_user_uri: - raise exceptions.NotAuthenticated( - "Account owner ID is missing in the request." - ) - try: - UserReference.objects.get(user_uri=request_user_uri) - return True - except UserReference.DoesNotExist: - raise exceptions.NotAuthenticated("User does not exist.") + return authenticate_resource(request, resource_uri, "admin") diff --git a/addon_service/common/serializer_fields.py b/addon_service/common/serializer_fields.py new file mode 100644 index 00000000..5c447905 --- /dev/null +++ b/addon_service/common/serializer_fields.py @@ -0,0 +1,5 @@ +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): + pass diff --git a/addon_service/configured_storage_addon/models.py b/addon_service/configured_storage_addon/models.py index 290c82de..15469c80 100644 --- a/addon_service/configured_storage_addon/models.py +++ b/addon_service/configured_storage_addon/models.py @@ -32,3 +32,7 @@ def account_owner(self): @property def owner_reference(self): return self.base_account.external_account.owner.user_uri + + @property + def resource_uri(self): + return self.authorized_resource.resource_uri diff --git a/addon_service/configured_storage_addon/serializers.py b/addon_service/configured_storage_addon/serializers.py index 68d5744d..b39a8fc5 100644 --- a/addon_service/configured_storage_addon/serializers.py +++ b/addon_service/configured_storage_addon/serializers.py @@ -15,7 +15,7 @@ class AuthorizedResourceField(ResourceRelatedField): def to_internal_value(self, data): resource_reference, _ = ResourceReference.objects.get_or_create( - resource_uri=data["id"] + resource_uri=data["resource_uri"] ) return resource_reference diff --git a/addon_service/configured_storage_addon/views.py b/addon_service/configured_storage_addon/views.py index 1236c5dc..fe1b7202 100644 --- a/addon_service/configured_storage_addon/views.py +++ b/addon_service/configured_storage_addon/views.py @@ -1,6 +1,6 @@ from addon_service.common.permissions import ( - CanCreateCSA, - SessionUserIsOwner, + SessionUserCanViewReferencedResource, + SessionUserIsReferencedResourceAdmin, ) from addon_service.common.viewsets import RetrieveWriteViewSet @@ -17,8 +17,8 @@ def get_permissions(self): return super().get_permissions() if self.action in ["retrieve", "retrieve_related", "update", "destroy"]: - return [SessionUserIsOwner()] + return [SessionUserCanViewReferencedResource()] elif self.action == "create": - return [CanCreateCSA()] + return [SessionUserIsReferencedResourceAdmin()] else: raise NotImplementedError("view action permission not implemented") diff --git a/addon_service/resource_reference/views.py b/addon_service/resource_reference/views.py index 8afaad30..9398bb14 100644 --- a/addon_service/resource_reference/views.py +++ b/addon_service/resource_reference/views.py @@ -1,4 +1,4 @@ -from addon_service.common.permissions import SessionUserIsResourceReferenceOwner +from addon_service.common.permissions import SessionUserCanViewReferencedResource from addon_service.common.viewsets import RetrieveOnlyViewSet from addon_service.serializers import ResourceReferenceSerializer @@ -8,4 +8,4 @@ class ResourceReferenceViewSet(RetrieveOnlyViewSet): queryset = ResourceReference.objects.all() serializer_class = ResourceReferenceSerializer - permission_classes = [SessionUserIsResourceReferenceOwner] + permission_classes = [SessionUserCanViewReferencedResource] diff --git a/addon_service/tests/_helpers.py b/addon_service/tests/_helpers.py index 125184dd..ce95b205 100644 --- a/addon_service/tests/_helpers.py +++ b/addon_service/tests/_helpers.py @@ -1,8 +1,12 @@ -from functools import wraps +import json +from collections import defaultdict +from functools import partial, wraps +from http import HTTPStatus from unittest.mock import patch import httpx from django.contrib.sessions.backends.db import SessionStore +from rest_framework import exceptions as drf_exceptions from rest_framework.test import APIRequestFactory from app import settings @@ -17,41 +21,112 @@ def get_test_request(user=None, method="get", path="", cookies=None): _request.COOKIES[name] = value return _request +class MockOSF: + + def __init__(self, permissions=None): + '''A lightweight, configurable mock of OSF for testing remote permissions. + + Accepts a mapping of arbitrary user_uris to their roles on arbitrary resource_uris. + i.e. + { + 'osf.io/abcde': {'osf.io/bcdef': 'write', 'osf.io/cdefg': 'admin'}, + 'osf.io/zyxwv': {'osf.io/yxwvut': 'read'} + } + Intercepts 'get' requests and uses the request url and this mapping to generate a minimal + response required for testing GravyValet's behavior. + + 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.start() + self._mock_resource_check.start() + + + def stop(self): + self._mock_auth_request.stop() + self._mock_resource_check.stop() + + def configure_assumed_caller(self, caller_uri): + self._configured_caller_uri = caller_uri + + def configure_user_role(self, user_uri, resource_uri, role): + self._permissions[user_uri][resource_uri] = role + + def _get_assumed_caller(self, cookies=None): + if self._configured_caller_uri: + return self._configured_caller_uri + if cookies is not None: + return cookies.get(settings.USER_REFERENCE_COOKIE) + raise ValueError('MockOSF cannot handle requests without configuring a user') + + def _get_user_permissions(self, user_uri, resource_uri): + role = self._permissions[user_uri][resource_uri] + if not role: + return [] + if role == 'read': + return ['read'] + if role == 'write': + return ['read', 'write'] + if role == 'admin': + return ['read', 'write', 'admin'] + + def _mock_user_check(self, *args, **kwargs): + 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): + caller = self._get_assumed_caller(cookies=request.COOKIES) + permissions = self._get_user_permissions(user_uri=caller, resource_uri=uri) + if required_permission.lower() not in permissions: + raise drf_exceptions.PermissionDenied + return uri # mimicking behavior from the check being mocked + + + +def with_mocked_httpx_get(response_status=HTTPStatus.OK, user_uri=None): -def with_mocked_httpx_get(response_status=200): - """Decorator to mock httpx.Client get requests with a customizable response status.""" def decorator(func): @wraps(func) - def wrapper(self, *args, **kwargs): - with patch( - "httpx.Client.get", - new=lambda *args, **kwargs: mock_httpx_response( - *args, response_status=response_status - ), - ): - return func(self, *args, **kwargs) + 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 + ) + patcher = patch('httpx.Client.get', side_effect=mock_side_effect) + client = patcher.start() + test_result = func(testcase, *args, **kwargs) + patcher.stop() + return test_result return wrapper return decorator -def mock_httpx_response(url, current_user, response_status, *args, **kwargs): +def _mock_httpx_response(response_status, user_uri, url, *args, **kwargs): """Generates mock httpx.Response based on the requested URL and response status.""" - if response_status == 200: - if url == settings.USER_REFERENCE_LOOKUP_URL: - payload = {"data": {"links": {"iri": current_user.user_uri}}} - else: - guid = url.rstrip("/").split("/")[-1] - payload = { - "data": { - "attributes": { - "current_user_permissions": ["read", "write", "admin"] - }, - "links": {"iri": f"{settings.URI_ID}{guid}"}, - } - } - return httpx.Response(status_code=200, json=payload) - else: # Handles 403 and other statuses explicitly + if not response_status.is_success: return httpx.Response(status_code=response_status) + + if url == settings.USER_REFERENCE_LOOKUP_URL: + payload = {"data": {"links": {"iri": user_uri}}} + else: + guid = url.rstrip("/").split("/")[-1] + payload = { + "data": { + "attributes": { + "current_user_permissions": ["read", "write", "admin"] + }, + "links": {"iri": f"{settings.URI_ID}{guid}"}, + } + } + return httpx.Response(status_code=response_status, json=payload) 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 eaf69add..ed653552 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 @@ -12,7 +12,7 @@ from addon_service.tests import _factories from addon_service.tests._helpers import ( get_test_request, - with_mocked_httpx_get, + MockOSF ) from app import settings @@ -21,13 +21,13 @@ class TestAuthorizedStorageAccountAPI(APITestCase): @classmethod def setUpTestData(cls): cls._asa = _factories.AuthorizedStorageAccountFactory() - cls._user = cls._asa.external_account.owner + cls._user = cls._asa.account_owner def setUp(self): super().setUp() - self.client.cookies[settings.USER_REFERENCE_COOKIE] = [ - "Some form of auth is necessary or POSTS are ignored." - ] + self._mock_osf = MockOSF() + self.addCleanup(self._mock_osf.stop) + self.client.cookies[settings.USER_REFERENCE_COOKIE] = self._user.user_uri @property def _detail_path(self): @@ -49,7 +49,6 @@ def _related_path(self, related_field): }, ) - @with_mocked_httpx_get def test_get(self): _resp = self.client.get(self._detail_path) self.assertEqual(_resp.status_code, HTTPStatus.OK) @@ -58,7 +57,34 @@ def test_get(self): self._asa.default_root_folder, ) - @with_mocked_httpx_get + def test_post(self): + external_service = _factories.ExternalStorageServiceFactory() + self.assertFalse(external_service.authorized_storage_accounts.exists()) + payload = { + "data": { + "type": "authorized-storage-accounts", + "attributes": { + "username": "", + "password": "", + }, + "relationships": { + "external_storage_service": { + "data": { + "type": "external-storage-services", + "id": external_service.id, + } + }, + }, + } + } + + _resp = self.client.post( + 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()) + def test_methods_not_allowed(self): _methods_not_allowed = { self._detail_path: {"post"}, @@ -112,10 +138,15 @@ def setUpTestData(cls): cls._user = cls._asa.external_account.owner cls._view = AuthorizedStorageAccountViewSet.as_view({"get": "retrieve"}) - @with_mocked_httpx_get + + def setUp(self): + super().setUp() + self._mock_osf = MockOSF() + self.addCleanup(self._mock_osf.stop) + def test_get(self): _resp = self._view( - get_test_request(cookies={"osf": "This is my chosen form of auth"}), + get_test_request(cookies={"osf": self._user.user_uri}), pk=self._asa.pk, ) self.assertEqual(_resp.status_code, HTTPStatus.OK) @@ -139,11 +170,11 @@ def test_unauthorized(self): _anon_resp = self._view(get_test_request(), pk=self._asa.pk) self.assertEqual(_anon_resp.status_code, HTTPStatus.UNAUTHORIZED) - @with_mocked_httpx_get(response_status=403) def test_wrong_user(self): + self._mock_osf.configure_assumed_caller('wrong/10') _resp = self._view( get_test_request( - cookies={settings.USER_REFERENCE_COOKIE: "this is wrong user auth"} + cookies={settings.USER_REFERENCE_COOKIE: "wrong/10"} ), pk=self._user.pk, ) @@ -159,7 +190,12 @@ def setUpTestData(cls): {"get": "retrieve_related"}, ) - @with_mocked_httpx_get + def setUp(self): + super().setUp() + self._mock_osf = MockOSF() + self._mock_osf.configure_assumed_caller(self._user.user_uri) + self.addCleanup(self._mock_osf.stop) + def test_get_related__empty(self): _resp = self._related_view( get_test_request(cookies={"osf": "This is my chosen form of auth"}), @@ -169,7 +205,6 @@ def test_get_related__empty(self): self.assertEqual(_resp.status_code, HTTPStatus.OK) self.assertEqual(_resp.data, []) - @with_mocked_httpx_get def test_get_related__several(self): _addons = _factories.ConfiguredStorageAddonFactory.create_batch( size=5, @@ -186,51 +221,3 @@ def test_get_related__several(self): {_datum["id"] for _datum in _content["data"]}, {str(_addon.pk) for _addon in _addons}, ) - - -class TestAuthorizedStorageAccountPOSTAPI(APITestCase): - @classmethod - def setUpTestData(cls): - cls._ess = _factories.ExternalStorageServiceFactory() - cls._ea = _factories.ExternalAccountFactory() - cls._user = cls._ea.owner - - def setUp(self): - super().setUp() - self.client.cookies[settings.USER_REFERENCE_COOKIE] = [ - "Some form of auth is necessary or POSTS are ignored." - ] - - @with_mocked_httpx_get - def test_post(self): - assert not self._ess.authorized_storage_accounts.all() # sanity/factory check - - payload = { - "data": { - "type": "authorized-storage-accounts", - "attributes": { - "username": "", - "password": "", - }, - "relationships": { - "external_storage_service": { - "data": { - "type": "external-storage-services", - "id": self._ess.auth_uri, - } - }, - "account_owner": { - "data": { - "id": self._user.user_uri, - "type": "user-references", - } - }, - }, - } - } - - response = self.client.post( - reverse("authorized-storage-accounts-list"), payload, format="vnd.api+json" - ) - self.assertEqual(response.status_code, 201) - assert self._ess.authorized_storage_accounts.all() 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 a09a919f..850b8403 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,9 +5,9 @@ from django.urls import reverse from rest_framework.test import APITestCase -from addon_service.models import ConfiguredStorageAddon +from addon_service.models import ConfiguredStorageAddon, ResourceReference from addon_service.tests import _factories as test_factories -from addon_service.tests._helpers import with_mocked_httpx_get +from addon_service.tests._helpers import MockOSF from app import settings @@ -26,13 +26,21 @@ def set_auth_header(self, auth_type): @classmethod def setUpTestData(cls): - cls.configured_storage_addon = test_factories.ConfiguredStorageAddonFactory() - cls.user = cls.configured_storage_addon.base_account.external_account.owner + cls._configured_storage_addon = test_factories.ConfiguredStorageAddonFactory() + cls._user = cls._configured_storage_addon.base_account.external_account.owner + + 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.client.cookies[settings.USER_REFERENCE_COOKIE] = self._user.user_uri def detail_url(self): return reverse( "configured-storage-addons-detail", - kwargs={"pk": self.configured_storage_addon.pk}, + kwargs={"pk": self._configured_storage_addon.pk}, ) def list_url(self): @@ -42,22 +50,21 @@ def related_url(self, related_field): return reverse( "configured-storage-addons-related", kwargs={ - "pk": self.configured_storage_addon.pk, + "pk": self._configured_storage_addon.pk, "related_field": related_field, }, ) class ConfiguredStorageAddonAPITests(BaseAPITest): - @with_mocked_httpx_get + def test_get_detail(self): response = self.client.get(self.detail_url()) self.assertEqual(response.status_code, HTTPStatus.OK) self.assertEqual( - response.json()["root_folder"], self.configured_storage_addon.root_folder + response.json()['data']['attributes']["root_folder"], self._configured_storage_addon.root_folder ) - @with_mocked_httpx_get def test_methods_not_allowed(self): not_allowed_methods = { self.detail_url(): ["post"], @@ -73,22 +80,21 @@ def test_methods_not_allowed(self): class ConfiguredStorageAddonModelTests(TestCase): @classmethod def setUpTestData(cls): - cls.configured_storage_addon = test_factories.ConfiguredStorageAddonFactory() + cls._configured_storage_addon = test_factories.ConfiguredStorageAddonFactory() def test_model_loading(self): loaded_addon = ConfiguredStorageAddon.objects.get( - id=self.configured_storage_addon.id + id=self._configured_storage_addon.id ) - self.assertEqual(self.configured_storage_addon.pk, loaded_addon.pk) + self.assertEqual(self._configured_storage_addon.pk, loaded_addon.pk) class ConfiguredStorageAddonViewSetTests(BaseAPITest): - @with_mocked_httpx_get + def test_viewset_retrieve(self): response = self.client.get(self.detail_url()) self.assertEqual(response.status_code, HTTPStatus.OK) - @with_mocked_httpx_get(response_status=403) def test_unauthorized_user(self): self.set_auth_header("session") response = self.client.get(self.related_url("base_account")) @@ -104,7 +110,7 @@ class ConfiguredStorageAddonPOSTTests(BaseAPITest): "data": {"type": "authorized-storage-accounts", "id": ""} }, "authorized_resource": { - "data": {"type": "resource-references", "id": ""} + "data": {"type": "resource-references"} }, }, } @@ -114,14 +120,14 @@ def setUp(self): super().setUp() self.default_payload["data"]["relationships"]["base_account"]["data"][ "id" - ] = str(self.configured_storage_addon.base_account_id) + ] = self._configured_storage_addon.base_account_id - @with_mocked_httpx_get def test_post_with_new_resource(self): - self.assertFalse(ConfiguredStorageAddon.objects.exists()) 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.default_payload["data"]["relationships"]["authorized_resource"]["data"][ - "id" + "resource_uri" ] = new_resource_uri response = self.client.post( @@ -133,9 +139,3 @@ def test_post_with_new_resource(self): authorized_resource__resource_uri=new_resource_uri ).exists() ) - - def test_post_various_auth_methods(self): - for auth_type in ["oauth", "basic", "no_auth", "session"]: - with self.subTest(auth_type=auth_type): - self.set_auth_header(auth_type) - self.test_post_with_new_resource() 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 4aca218c..2e4741e0 100644 --- a/addon_service/tests/test_by_type/test_resource_reference.py +++ b/addon_service/tests/test_by_type/test_resource_reference.py @@ -11,6 +11,7 @@ from addon_service.tests import _factories from addon_service.tests._helpers import ( get_test_request, + MockOSF, with_mocked_httpx_get, ) from app import settings @@ -26,9 +27,10 @@ def setUpTestData(cls): def setUp(self): super().setUp() - self.client.cookies[settings.USER_REFERENCE_COOKIE] = [ - "Some form of auth is necessary or POSTS are ignored." - ] + self._mock_osf = MockOSF(permissions={self._user.user_uri: {self._resource.resource_uri: 'admin'}}) + self.addCleanup(self._mock_osf.stop) + self.client.cookies[settings.USER_REFERENCE_COOKIE] = self._user.user_uri + @property def _detail_path(self): @@ -48,13 +50,11 @@ def _related_configured_storage_addons_path(self): }, ) - @with_mocked_httpx_get def test_get(self): _resp = self.client.get(self._detail_path) self.assertEqual(_resp.status_code, HTTPStatus.OK) self.assertEqual(_resp.data["resource_uri"], self._resource.resource_uri) - @with_mocked_httpx_get def test_methods_not_allowed(self): _methods_not_allowed = { self._detail_path: {"patch", "put", "post"}, @@ -112,10 +112,14 @@ def setUpTestData(cls): # _user magically becomes the current requester cls._user = cls._csa.base_account.external_account.owner - @with_mocked_httpx_get + 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') + def test_get(self): _resp = self._view( - get_test_request(cookies={"osf": "This is my chosen form of auth"}), + get_test_request(cookies={"osf": self._user.user_uri}), pk=self._resource.pk, ) self.assertEqual(_resp.status_code, HTTPStatus.OK) @@ -133,12 +137,10 @@ def test_get(self): }, ) - @with_mocked_httpx_get def test_unauthorized(self): _anon_resp = self._view(get_test_request(), pk=self._resource.pk) self.assertEqual(_anon_resp.status_code, HTTPStatus.UNAUTHORIZED) - @with_mocked_httpx_get(response_status=403) def test_wrong_user(self): _resp = self._view( get_test_request( 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 7aa23c6b..e91bb593 100644 --- a/addon_service/tests/test_by_type/test_user_reference.py +++ b/addon_service/tests/test_by_type/test_user_reference.py @@ -12,6 +12,7 @@ from addon_service.tests._helpers import ( get_test_request, with_mocked_httpx_get, + MockOSF ) from addon_service.user_reference.views import UserReferenceViewSet from app import settings @@ -24,9 +25,9 @@ def setUpTestData(cls): def setUp(self): super().setUp() - self.client.cookies[settings.USER_REFERENCE_COOKIE] = [ - "Some form of auth is necessary to confirm the user reference." - ] + self._mock_osf = MockOSF() + self.addCleanup(self._mock_osf.stop) + self.client.cookies[settings.USER_REFERENCE_COOKIE] = self._user.user_uri @property def _detail_path(self): @@ -46,7 +47,6 @@ def _related_accounts_path(self): }, ) - @with_mocked_httpx_get def test_get(self): _resp = self.client.get(self._detail_path) self.assertEqual(_resp.status_code, HTTPStatus.OK) @@ -64,7 +64,6 @@ def test_get(self): }, ) - @with_mocked_httpx_get def test_methods_not_allowed(self): _methods_not_allowed = { self._detail_path: {"patch", "put", "post"}, @@ -117,12 +116,16 @@ def setUpTestData(cls): cls._user = _factories.UserReferenceFactory() cls._view = UserReferenceViewSet.as_view({"get": "retrieve"}) - @with_mocked_httpx_get + def setUp(self): + super().setUp() + self._mock_osf = MockOSF() + self.addCleanup(self._mock_osf.stop) + def test_get(self): _resp = self._view( get_test_request( cookies={ - settings.USER_REFERENCE_COOKIE: "Some form of auth is necessary." + settings.USER_REFERENCE_COOKIE: self._user.user_uri }, ), pk=self._user.pk, @@ -142,21 +145,20 @@ def test_get(self): }, ) - @with_mocked_httpx_get(response_status=403) def test_wrong_user(self): + self._mock_osf.configure_assumed_caller('wrong') _resp = self._view( - get_test_request(cookies={"osf": "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) - @with_mocked_httpx_get def test_unauthorized(self): _anon_resp = self._view(get_test_request(), pk=self._user.pk) self.assertEqual(_anon_resp.status_code, HTTPStatus.UNAUTHORIZED) -class TestUserReferenceRelatedView(TestCase): +class TestUserReferenceRelatedView(APITestCase): @classmethod def setUpTestData(cls): cls._user = _factories.UserReferenceFactory() @@ -164,12 +166,14 @@ def setUpTestData(cls): def setUp(self): super().setUp() + self._mock_osf = MockOSF() + self.addCleanup(self._mock_osf.stop) self.request = get_test_request( user=self._user, - cookies={settings.USER_REFERENCE_COOKIE: "Some form of auth is necessary."}, + cookies={settings.USER_REFERENCE_COOKIE: self._user.user_uri}, ) - @with_mocked_httpx_get + def test_get_related__empty(self): _resp = self._related_view( self.request, @@ -179,7 +183,6 @@ def test_get_related__empty(self): self.assertEqual(_resp.status_code, HTTPStatus.OK) self.assertEqual(_resp.data, []) - @with_mocked_httpx_get def test_get_related__several(self): _accounts = _factories.AuthorizedStorageAccountFactory.create_batch( size=5, diff --git a/app/authentication.py b/app/authentication.py index 15923954..ef563883 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,6 +33,7 @@ class SkipAuthMethod(exceptions.APIException): def authenticate_resource(request, uri, required_permission): + print('WHAT?!!!!\n\n\n') resource_url = settings.RESOURCE_REFERENCE_LOOKUP_URL.format( uri.replace(settings.URI_ID, "").rstrip("/") ) @@ -84,7 +85,7 @@ def make_auth_request(url, **kwargs): auth=kwargs.pop("auth", None), event_hooks={"response": [handle_redirects]}, ) as client: - response = client.get(url, **kwargs) + response = client.get(url=url, **kwargs) exceptions_map = { 400: exceptions.ValidationError("Invalid request."), 401: exceptions.AuthenticationFailed("Invalid credentials."),