diff --git a/api/base/serializers.py b/api/base/serializers.py index 60afedc1876..5f430fbf7e6 100644 --- a/api/base/serializers.py +++ b/api/base/serializers.py @@ -1183,6 +1183,13 @@ def to_representation(self, obj): if hasattr(obj, 'get_absolute_info_url'): ret['info'] = self._extend_url_with_vol_key(obj.get_absolute_info_url()) + request = self.context['request'] + referer = request.headers.get('Referer', '') + if 'html' in ret and 'legacy' in referer: + parsed_html_url = urlparse(ret['html']) + legacy_url = urlparse(referer) + ret['html'] = parsed_html_url._replace(scheme=legacy_url.scheme, netloc=legacy_url.netloc).geturl() + return ret diff --git a/api/files/views.py b/api/files/views.py index bd2eb9979cd..197c1dd3cdb 100644 --- a/api/files/views.py +++ b/api/files/views.py @@ -30,7 +30,6 @@ FileDetailSerializer, FileVersionSerializer, ) -from osf.utils.permissions import ADMIN class FileMixin: @@ -87,11 +86,11 @@ def get_target(self): # overrides RetrieveAPIView def get_object(self): - user = utils.get_user_auth(self.request).user file = self.get_file() if self.request.GET.get('create_guid', False): - if (self.get_target().has_permission(user, ADMIN) and utils.has_admin_scope(self.request)): + auth = utils.get_user_auth(self.request) + if self.get_target().can_view(auth): file.get_guid(create=True) # We normally would pass this through `get_file` as an annotation, but the `select_for_update` feature prevents diff --git a/api/nodes/serializers.py b/api/nodes/serializers.py index f3323d356cb..016a1e76c55 100644 --- a/api/nodes/serializers.py +++ b/api/nodes/serializers.py @@ -787,7 +787,7 @@ def create(self, validated_data): template_node = Node.load(template_from) if template_node is None: raise exceptions.NotFound - if not template_node.has_permission(user, osf_permissions.READ, check_parent=False): + if not template_node.is_public and not template_node.is_contributor(user): raise exceptions.PermissionDenied validated_data.pop('creator') changed_data = {template_from: validated_data} diff --git a/api/preprints/serializers.py b/api/preprints/serializers.py index ee109f852a6..d47cc9098dc 100644 --- a/api/preprints/serializers.py +++ b/api/preprints/serializers.py @@ -52,6 +52,7 @@ ) from osf.utils import permissions as osf_permissions from osf.utils.workflows import DefaultStates +from osf.models.contributor import get_user_permission class PrimaryFileRelationshipField(RelationshipField): @@ -653,6 +654,7 @@ def validate_permission(self, value): user # if user is None then probably we're trying to make bulk update and this validation is not relevant and preprint.machine_state == DefaultStates.INITIAL.value and preprint.creator_id == user.id + and get_user_permission(user, preprint) != value ): raise ValidationError( 'You cannot change your permission setting at this time. ' diff --git a/api/providers/permissions.py b/api/providers/permissions.py index cc2c9e4632c..52d40e75dc9 100644 --- a/api/providers/permissions.py +++ b/api/providers/permissions.py @@ -1,4 +1,3 @@ -from guardian.shortcuts import get_perms from rest_framework import permissions as drf_permissions from api.base.utils import get_user_auth @@ -36,4 +35,7 @@ def has_permission(self, request, view): class MustBeModerator(drf_permissions.BasePermission): def has_permission(self, request, view): auth = get_user_auth(request) - return bool(get_perms(auth.user, view.get_provider())) + provider = view.get_provider() + is_admin = provider.get_group('admin').user_set.filter(id=auth.user.id).exists() + is_moderator = provider.get_group('moderator').user_set.filter(id=auth.user.id).exists() + return is_moderator or is_admin diff --git a/api/registrations/permissions.py b/api/registrations/permissions.py index de4452ba6a3..526dbb4d702 100644 --- a/api/registrations/permissions.py +++ b/api/registrations/permissions.py @@ -7,10 +7,10 @@ class ContributorOrModerator(permissions.BasePermission): def has_object_permission(self, request, view, obj): auth = get_user_auth(request) + is_admin = obj.provider.get_group('admin').user_set.filter(id=auth.user.id).exists() + is_moderator = obj.provider.get_group('moderator').user_set.filter(id=auth.user.id).exists() - # If a user has perms on the provider, they must be a moderator or admin - is_moderator = bool(get_perms(auth.user, obj.provider)) - return obj.is_admin_contributor(auth.user) or is_moderator + return obj.is_admin_contributor(auth.user) or is_moderator or is_admin class ContributorOrModeratorOrPublic(permissions.BasePermission): diff --git a/api_tests/nodes/views/test_node_list.py b/api_tests/nodes/views/test_node_list.py index 15398613ea3..bbfa7870050 100644 --- a/api_tests/nodes/views/test_node_list.py +++ b/api_tests/nodes/views/test_node_list.py @@ -259,6 +259,16 @@ def test_current_user_permissions(self, app, user, url, public_project, non_cont res = app.get(url_public, auth=superuser.auth) assert permissions.READ not in res.json['data'][0]['attributes']['current_user_permissions'] + def test_legacy_host_for_htmls(self, app, url, public_project): + settings.DOMAIN = 'https://staging3.osf.io' + current_domain_response = app.get(url).json['data'] + assert current_domain_response[-1]['links']['html'].startswith(settings.DOMAIN) + + # mock request from legacy OSF domain to staging3 backend + # so that backend uses it to generate html links instead of current domain + legacy_domain_response = app.get(url, headers={'Referer': 'http://legacy.osf.io'}).json['data'] + assert legacy_domain_response[-1]['links']['html'].startswith('http://legacy.osf.io') + @pytest.mark.django_db @pytest.mark.enable_bookmark_creation @@ -1467,25 +1477,6 @@ def test_create_from_template_errors(self, app, user_one, user_two, url): expect_errors=True) assert res.status_code == 404 - # test_403_on_create_from_template_of_unauthorized_project - template_from = ProjectFactory(creator=user_two, is_public=True) - templated_project_data = { - 'data': { - 'type': 'nodes', - 'attributes': - { - 'title': 'No permission', - 'category': 'project', - 'template_from': template_from._id, - } - } - } - res = app.post_json_api( - url, templated_project_data, - auth=user_one.auth, - expect_errors=True) - assert res.status_code == 403 - def test_creates_project_from_template(self, app, user_one, category, url): template_from = ProjectFactory(creator=user_one, is_public=True) template_component = ProjectFactory( @@ -1517,6 +1508,97 @@ def test_creates_project_from_template(self, app, user_one, category, url): assert len(new_project.nodes) == len(template_from.nodes) assert new_project.nodes[0].title == template_component.title + def test_non_contributor_create_project_from_public_template_success(self, app, user_one, category, url): + template_from = ProjectFactory(creator=user_one, is_public=True) + user_without_permissions = AuthUserFactory() + templated_project_data = { + 'data': { + 'type': 'nodes', + 'attributes': + { + 'title': 'template from project', + 'category': category, + 'template_from': template_from._id, + } + } + } + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth + ) + assert res.status_code == 201 + + def test_non_contributor_create_project_from_private_template_no_permission_fails(self, app, user_one, category, url): + template_from = ProjectFactory(creator=user_one, is_public=False) + user_without_permissions = AuthUserFactory() + templated_project_data = { + 'data': { + 'type': 'nodes', + 'attributes': + { + 'title': 'template from project', + 'category': category, + 'template_from': template_from._id, + } + } + } + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth, + expect_errors=True + ) + assert res.status_code == 403 + + def test_contributor_create_project_from_private_template_with_permission_success(self, app, user_one, category, url): + template_from = ProjectFactory(creator=user_one, is_public=False) + user_without_permissions = AuthUserFactory() + template_from.add_contributor(user_without_permissions, permissions=permissions.READ, auth=Auth(user_one), save=True) + templated_project_data = { + 'data': { + 'type': 'nodes', + 'attributes': + { + 'title': 'template from project', + 'category': category, + 'template_from': template_from._id, + } + } + } + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth + ) + assert res.status_code == 201 + assert template_from.has_permission(user_without_permissions, permissions.READ) + + template_from.update_contributor( + user_without_permissions, + permission=permissions.WRITE, + auth=Auth(user_one), + save=True, + visible=True + ) + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth + ) + assert res.status_code == 201 + assert template_from.has_permission(user_without_permissions, permissions.WRITE) + + template_from.update_contributor( + user_without_permissions, + permission=permissions.ADMIN, + auth=Auth(user_one), + save=True, + visible=True + ) + res = app.post_json_api( + url, templated_project_data, + auth=user_without_permissions.auth + ) + assert res.status_code == 201 + assert template_from.has_permission(user_without_permissions, permissions.ADMIN) + def test_creates_project_creates_project_and_sanitizes_html( self, app, user_one, category, url): title = 'Cool Project' diff --git a/api_tests/providers/collections/views/test_permissions.py b/api_tests/providers/collections/views/test_permissions.py new file mode 100644 index 00000000000..ed4f76efcde --- /dev/null +++ b/api_tests/providers/collections/views/test_permissions.py @@ -0,0 +1,22 @@ +import pytest + +from api.base.settings.defaults import API_BASE +from api_tests.providers.mixins import OnlyModeratorOrAdminPermissionsMixin + +from osf_tests.factories import CollectionProviderFactory + + +@pytest.mark.django_db +class TestOnlyModeratorOrAdmin(OnlyModeratorOrAdminPermissionsMixin): + + @pytest.fixture() + def urls(self, provider, moderator, admin): + return [ + f'/{API_BASE}providers/collections/{provider._id}/moderators/', + f'/{API_BASE}providers/collections/{provider._id}/moderators/{moderator._id}/', + f'/{API_BASE}providers/collections/{provider._id}/moderators/{admin._id}/', + ] + + @pytest.fixture() + def provider(self): + return CollectionProviderFactory() diff --git a/api_tests/providers/mixins.py b/api_tests/providers/mixins.py index d6e28678059..86f18c19718 100644 --- a/api_tests/providers/mixins.py +++ b/api_tests/providers/mixins.py @@ -1038,3 +1038,42 @@ def test_provider_has_both_acceptable_and_default_licenses(self, app, provider, assert license_one._id in license_ids assert license_three._id in license_ids assert license_two._id not in license_ids + + +@pytest.mark.django_db +class OnlyModeratorOrAdminPermissionsMixin: + + @pytest.fixture() + def provider(self): + raise NotImplementedError + + @pytest.fixture() + def user(self): + return AuthUserFactory() + + @pytest.fixture() + def moderator(self, provider): + mod = AuthUserFactory() + provider.get_group('moderator').user_set.add(mod) + return mod + + @pytest.fixture() + def admin(self, provider): + adm = AuthUserFactory() + provider.get_group('admin').user_set.add(adm) + return adm + + @pytest.fixture() + def urls(self): + raise NotImplementedError + + def test_moderator_or_admin_have_access_to_provider(self, app, provider, user, moderator, admin, urls): + for url in urls: + user_res = app.get(url, auth=user.auth, expect_errors=True) + assert user_res.status_code == 403 + + moderator_res = app.get(url, auth=moderator.auth) + assert moderator_res.status_code == 200 + + admin_res = app.get(url, auth=admin.auth) + assert admin_res.status_code == 200 diff --git a/api_tests/providers/preprints/views/test_permissions.py b/api_tests/providers/preprints/views/test_permissions.py new file mode 100644 index 00000000000..09b726a991f --- /dev/null +++ b/api_tests/providers/preprints/views/test_permissions.py @@ -0,0 +1,23 @@ +import pytest + +from api.base.settings.defaults import API_BASE +from api_tests.providers.mixins import OnlyModeratorOrAdminPermissionsMixin + +from osf_tests.factories import PreprintProviderFactory + + +@pytest.mark.django_db +class TestOnlyModeratorOrAdmin(OnlyModeratorOrAdminPermissionsMixin): + + @pytest.fixture() + def urls(self, provider, moderator, admin): + return [ + f'/{API_BASE}providers/preprints/{provider._id}/withdraw_requests/', + f'/{API_BASE}providers/preprints/{provider._id}/moderators/', + f'/{API_BASE}providers/preprints/{provider._id}/moderators/{moderator._id}/', + f'/{API_BASE}providers/preprints/{provider._id}/moderators/{admin._id}/', + ] + + @pytest.fixture() + def provider(self): + return PreprintProviderFactory() diff --git a/api_tests/providers/registrations/views/test_permissions.py b/api_tests/providers/registrations/views/test_permissions.py new file mode 100644 index 00000000000..6efd4d2808d --- /dev/null +++ b/api_tests/providers/registrations/views/test_permissions.py @@ -0,0 +1,25 @@ +import pytest + +from api.base.settings.defaults import API_BASE +from api_tests.providers.mixins import OnlyModeratorOrAdminPermissionsMixin + +from osf_tests.factories import RegistrationProviderFactory + + +@pytest.mark.django_db +class TestOnlyModeratorOrAdmin(OnlyModeratorOrAdminPermissionsMixin): + + @pytest.fixture() + def urls(self, provider, moderator, admin): + return [ + f'/{API_BASE}providers/registrations/{provider._id}/requests/', + f'/{API_BASE}providers/registrations/{provider._id}/registrations/', + f'/{API_BASE}providers/registrations/{provider._id}/actions/', + f'/{API_BASE}providers/registrations/{provider._id}/moderators/', + f'/{API_BASE}providers/registrations/{provider._id}/moderators/{moderator._id}/', + f'/{API_BASE}providers/registrations/{provider._id}/moderators/{admin._id}/', + ] + + @pytest.fixture() + def provider(self): + return RegistrationProviderFactory() diff --git a/osf/models/contributor.py b/osf/models/contributor.py index a427a7e50f6..42944161a03 100644 --- a/osf/models/contributor.py +++ b/osf/models/contributor.py @@ -103,11 +103,15 @@ def get_contributor_permission(contributor, resource): """ Returns a contributor's permissions - perms through contributorship only. No permissions through osf group membership. """ + return get_user_permission(contributor.user, resource) + + +def get_user_permission(user, resource): read = resource.format_group(permissions.READ) write = resource.format_group(permissions.WRITE) admin = resource.format_group(permissions.ADMIN) # Checking for django group membership allows you to also get the intended permissions of unregistered contributors - user_groups = contributor.user.groups.filter(name__in=[read, write, admin]).values_list('name', flat=True) + user_groups = user.groups.filter(name__in=[read, write, admin]).values_list('name', flat=True) if admin in user_groups: return permissions.ADMIN elif write in user_groups: diff --git a/osf/models/user.py b/osf/models/user.py index 79e11f34ebd..a989e5b9fd6 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -61,7 +61,7 @@ from website.project import new_bookmark_collection from website.util.metrics import OsfSourceTags, unregistered_created_source_tag from importlib import import_module -from osf.utils.requests import get_headers_from_request +from osf.utils.requests import string_type_request_headers SessionStore = import_module(settings.SESSION_ENGINE).SessionStore @@ -1026,7 +1026,7 @@ def save(self, *args, **kwargs): ret = super().save(*args, **kwargs) # must save BEFORE spam check, as user needs guid. if set(self.SPAM_USER_PROFILE_FIELDS.keys()).intersection(dirty_fields): request = get_current_request() - headers = get_headers_from_request(request) + headers = string_type_request_headers(request) self.check_spam(dirty_fields, request_headers=headers) dirty_fields = set(dirty_fields)