diff --git a/api/institutions/serializers.py b/api/institutions/serializers.py index 1d1e0761715..0262f9dddd7 100644 --- a/api/institutions/serializers.py +++ b/api/institutions/serializers.py @@ -41,6 +41,7 @@ class InstitutionSerializer(JSONAPISerializer): ser.CharField(read_only=True), permission='view_institutional_metrics', ) + institutional_request_access_enabled = ser.BooleanField(read_only=True) links = LinksField({ 'self': 'get_api_url', 'html': 'get_absolute_html_url', diff --git a/api/nodes/views.py b/api/nodes/views.py index 93879e2d40a..2dcf7aa541e 100644 --- a/api/nodes/views.py +++ b/api/nodes/views.py @@ -125,7 +125,7 @@ RegistrationSerializer, RegistrationCreateSerializer, ) -from api.requests.permissions import NodeRequestPermission +from api.requests.permissions import NodeRequestPermission, InstitutionalAdminRequestTypePermission from api.requests.serializers import NodeRequestSerializer, NodeRequestCreateSerializer from api.requests.views import NodeRequestMixin from api.resources import annotations as resource_annotations @@ -2239,6 +2239,7 @@ class NodeRequestListCreate(JSONAPIBaseView, generics.ListCreateAPIView, ListFil drf_permissions.IsAuthenticatedOrReadOnly, base_permissions.TokenHasScope, NodeRequestPermission, + InstitutionalAdminRequestTypePermission, ) required_read_scopes = [CoreScopes.NODE_REQUESTS_READ] diff --git a/api/requests/permissions.py b/api/requests/permissions.py index 09c97f012ba..9e4a240ac7d 100644 --- a/api/requests/permissions.py +++ b/api/requests/permissions.py @@ -1,11 +1,15 @@ -from rest_framework import permissions as drf_permissions +from rest_framework import exceptions, permissions as drf_permissions from api.base.utils import get_user_auth -from osf.models.action import NodeRequestAction, PreprintRequestAction +from osf.models import ( + Node, + NodeRequestAction, + PreprintRequestAction, + Preprint, + Institution, +) from osf.models.mixins import NodeRequestableMixin, PreprintRequestableMixin -from osf.models.node import Node -from osf.models.preprint import Preprint -from osf.utils.workflows import DefaultTriggers +from osf.utils.workflows import DefaultTriggers, NodeRequestTypes from osf.utils import permissions as osf_permissions @@ -32,7 +36,7 @@ def has_object_permission(self, request, view, obj): raise ValueError(f'Not a request-related model: {obj}') if not node.access_requests_enabled: - return False + raise exceptions.PermissionDenied(f'{node._id} does not have Access Requests enabled') is_requester = target is not None and target.creator == auth.user or trigger == DefaultTriggers.SUBMIT.value is_node_admin = node.has_permission(auth.user, osf_permissions.ADMIN) @@ -52,7 +56,35 @@ def has_object_permission(self, request, view, obj): # Requesters may not be contributors # Requesters may edit their comment or submit their request return is_requester and auth.user not in node.contributors - return False + + +class InstitutionalAdminRequestTypePermission(drf_permissions.BasePermission): + """ + Permission class for handling object permissions related to Node requests and actions. + """ + + def has_permission(self, request, view): + # Skip if not institutional_request request_type + request_type = request.data.get('request_type') + if request_type != NodeRequestTypes.INSTITUTIONAL_REQUEST.value: + return True + + institution_id = request.data.get('institution') + if not institution_id: + raise exceptions.ValidationError({'institution': 'Institution is required.'}) + + try: + institution = Institution.objects.get(_id=institution_id) + except Institution.DoesNotExist: + raise exceptions.ValidationError({'institution': 'Institution is does not exist.'}) + + if not institution.institutional_request_access_enabled: + raise exceptions.PermissionDenied({'institution': 'Institutional request access is not enabled.'}) + + if get_user_auth(request).user.is_institutional_admin_at(institution): + return True + else: + raise exceptions.PermissionDenied({'institution': 'You do not have permission to perform this action for this institution.'}) class PreprintRequestPermission(drf_permissions.BasePermission): diff --git a/api/requests/serializers.py b/api/requests/serializers.py index 71ef190b50d..8ad450936c0 100644 --- a/api/requests/serializers.py +++ b/api/requests/serializers.py @@ -4,10 +4,24 @@ from api.base.exceptions import Conflict from api.base.utils import absolute_reverse, get_user_auth -from api.base.serializers import JSONAPISerializer, LinksField, VersionedDateTimeField, RelationshipField -from osf.models import NodeRequest, PreprintRequest -from osf.utils.workflows import DefaultStates, RequestTypes +from api.base.serializers import ( + JSONAPISerializer, + LinksField, + VersionedDateTimeField, + RelationshipField, +) +from osf.models import ( + NodeRequest, + PreprintRequest, + Institution, + OSFUser, +) +from osf.utils.workflows import DefaultStates, RequestTypes, NodeRequestTypes from osf.utils import permissions as osf_permissions +from website import settings +from website.mails import send_mail, NODE_REQUEST_INSTITUTIONAL_ACCESS_REQUEST + +from rest_framework.exceptions import PermissionDenied, ValidationError class RequestSerializer(JSONAPISerializer): @@ -56,6 +70,8 @@ def create(self, validated_data): raise NotImplementedError() class NodeRequestSerializer(RequestSerializer): + request_type = ser.ChoiceField(read_only=True, required=False, choices=NodeRequestTypes.choices()) + class Meta: type_ = 'node-requests' @@ -66,6 +82,13 @@ class Meta: source='target__guids___id', ) + requested_permissions = ser.ChoiceField( + help_text='These are the default permission suggested when the Node admin sees users ' + 'listed in an `Request Access` list.', + choices=osf_permissions.API_CONTRIBUTOR_PERMISSIONS, + required=False, + ) + def get_target_url(self, obj): return absolute_reverse('nodes:node-detail', kwargs={'node_id': obj.target._id, 'version': self.context['request'].parser_context['kwargs']['version']}) @@ -89,40 +112,116 @@ def get_target_url(self, obj): }, ) -class NodeRequestCreateSerializer(NodeRequestSerializer): - request_type = ser.ChoiceField(required=True, choices=RequestTypes.choices()) - def create(self, validated_data): - auth = get_user_auth(self.context['request']) - if not auth.user: - raise exceptions.PermissionDenied +class NodeRequestCreateSerializer(NodeRequestSerializer): + request_type = ser.ChoiceField(read_only=False, required=False, choices=NodeRequestTypes.choices()) + message_recipient = RelationshipField( + help_text='An optional user who will receive an email explaining the nature of the request.', + required=False, + related_view='users:user-detail', + related_view_kwargs={'user_id': ''}, + ) + bcc_sender = ser.BooleanField( + required=False, + default=False, + help_text='If true, BCCs the sender, giving them a copy of the email message they sent.', + ) + reply_to = ser.BooleanField( + default=False, + help_text='Whether to set the sender\'s username as the `Reply-To` header in the email.', + ) + def to_internal_value(self, data): + """ + Retrieves the id value from `RelationshipField` fields + """ + instituion_id = data.pop('institution', None) + message_recipient_id = data.pop('message_recipient', None) + data = super().to_internal_value(data) + + if instituion_id: + data['institution'] = instituion_id + + if message_recipient_id: + data['message_recipient'] = message_recipient_id + return data + + def get_node_and_validate_non_contributor(self, auth): + """ + Ensures request user isn't already a contributor. + """ try: - node = self.context['view'].get_target() + return self.context['view'].get_target() except exceptions.PermissionDenied: node = self.context['view'].get_target(check_object_permissions=False) if auth.user in node.contributors: raise exceptions.PermissionDenied('You cannot request access to a node you contribute to.') raise - comment = validated_data.pop('comment', '') - request_type = validated_data.pop('request_type', None) + def create(self, validated_data) -> NodeRequest: + auth = get_user_auth(self.context['request']) + if not auth.user: + raise exceptions.PermissionDenied - if request_type != RequestTypes.ACCESS.value: - raise exceptions.ValidationError('You must specify a valid request_type.') + node = self.get_node_and_validate_non_contributor(auth) + + request_type = validated_data.get('request_type') + match request_type: + case NodeRequestTypes.ACCESS.value: + return self._create_node_request(node, validated_data) + case NodeRequestTypes.INSTITUTIONAL_REQUEST.value: + return self.make_node_institutional_access_request(node, validated_data) + case _: + raise ValidationError('You must specify a valid request_type.') + + def make_node_institutional_access_request(self, node, validated_data) -> NodeRequest: + sender = self.context['request'].user + node_request = self._create_node_request(node, validated_data) + node_request.is_institutional_request = True + node_request.save() + institution = Institution.objects.get(_id=validated_data['institution']) + recipient = OSFUser.load(validated_data.get('message_recipient')) + + if recipient: + if not recipient.is_affiliated_with_institution(institution): + raise PermissionDenied(f"User {recipient._id} is not affiliated with the institution.") + + if validated_data['comment']: + send_mail( + to_addr=recipient.username, + mail=NODE_REQUEST_INSTITUTIONAL_ACCESS_REQUEST, + user=recipient, + sender=sender, + bcc_addr=[sender.username] if validated_data['bcc_sender'] else None, + reply_to=sender.username if validated_data['reply_to'] else None, + recipient=recipient, + comment=validated_data['comment'], + institution=institution, + osf_url=settings.DOMAIN, + node=node_request.target, + ) + return node_request + + def _create_node_request(self, node, validated_data) -> NodeRequest: + creator = self.context['request'].user + request_type = validated_data['request_type'] + comment = validated_data.get('comment', '') + requested_permissions = validated_data.get('requested_permissions') try: node_request = NodeRequest.objects.create( target=node, - creator=auth.user, + creator=creator, comment=comment, machine_state=DefaultStates.INITIAL.value, request_type=request_type, + requested_permissions=requested_permissions, ) node_request.save() except IntegrityError: - raise Conflict(f'Users may not have more than one {request_type} request per node.') - node_request.run_submit(auth.user) + raise Conflict(f"Users may not have more than one {request_type} request per node.") + + node_request.run_submit(creator) return node_request class PreprintRequestSerializer(RequestSerializer): diff --git a/api/users/permissions.py b/api/users/permissions.py index a9186a4efd5..a8cc446be10 100644 --- a/api/users/permissions.py +++ b/api/users/permissions.py @@ -1,5 +1,7 @@ -from osf.models import OSFUser -from rest_framework import permissions +from rest_framework import permissions, exceptions + +from osf.models import OSFUser, Institution +from osf.models.user_message import MessageTypes class ReadOnlyOrCurrentUser(permissions.BasePermission): @@ -47,3 +49,36 @@ def has_permission(self, request, view): def has_object_permission(self, request, view, obj): assert isinstance(obj, OSFUser), f'obj must be a User, got {obj}' return not obj.is_registered + + +class UserMessagePermissions(permissions.BasePermission): + """ + Custom permission to allow only institutional admins to create certain types of UserMessages. + """ + def has_permission(self, request, view) -> bool: + """ + Validate if the user has permission to perform the requested action. + Args: + request: The HTTP request. + view: The view handling the request. + Returns: + bool: True if the user has the required permission, False otherwise. + """ + user = request.user + if not user or user.is_anonymous: + return False + + institution_id = request.data.get('institution') + if not institution_id: + raise exceptions.ValidationError({'institution': 'Institution is required.'}) + + try: + institution = Institution.objects.get(_id=institution_id) + except Institution.DoesNotExist: + raise exceptions.ValidationError({'institution': 'Specified institution does not exist.'}) + + message_type = request.data.get('message_type') + if message_type == MessageTypes.INSTITUTIONAL_REQUEST: + return user.is_institutional_admin_at(institution) and institution.institutional_request_access_enabled + else: + raise exceptions.ValidationError('Not valid message type.') diff --git a/api/users/serializers.py b/api/users/serializers.py index 5e8ca59d9cf..a3bd4d1cdb4 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -1,3 +1,5 @@ +from typing import Any, Dict + from django.utils import timezone from jsonschema import validate, Draft7Validator, ValidationError as JsonSchemaValidationError from rest_framework import exceptions @@ -19,16 +21,26 @@ JSONAPIListField, ShowIfCurrentUser, ) -from api.base.utils import absolute_reverse, default_node_list_queryset, get_user_auth, is_deprecated, hashids +from api.base.utils import ( + absolute_reverse, + default_node_list_queryset, + get_user_auth, + is_deprecated, + hashids, + get_object_or_error, +) + from api.base.versioning import get_kebab_snake_case_field from api.nodes.serializers import NodeSerializer, RegionRelationshipField from framework.auth.views import send_confirm_email_async from osf.exceptions import ValidationValueError, ValidationError, BlockedEmailError -from osf.models import Email, Node, OSFUser, Preprint, Registration +from osf.models import Email, Node, OSFUser, Preprint, Registration, UserMessage, Institution +from osf.models.user_message import MessageTypes from osf.models.provider import AbstractProviderGroupObjectPermission from osf.utils.requests import string_type_request_headers from website.profile.views import update_osf_help_mails_subscription, update_mailchimp_subscription from website.settings import MAILCHIMP_GENERAL_LIST, OSF_HELP_LIST, CONFIRM_REGISTRATIONS_BY_EMAIL +from website.util import api_v2_url class SocialField(ser.DictField): @@ -657,3 +669,111 @@ def update(self, instance, validated_data): class UserNodeSerializer(NodeSerializer): filterable_fields = NodeSerializer.filterable_fields | {'current_user_permissions'} + + +class UserMessageSerializer(JSONAPISerializer): + """ + Serializer for creating and managing `UserMessage` instances. + + Attributes: + message_text (CharField): The text content of the message. + message_type (ChoiceField): The type of message being sent, restricted to `MessageTypes`. + institution (RelationshipField): The institution related to the message. Required. + user (RelationshipField): The recipient of the message. + """ + id = IDField(read_only=True, source='_id') + message_text = ser.CharField( + required=True, + help_text='The content of the message to be sent.', + ) + message_type = ser.ChoiceField( + choices=MessageTypes.choices, + required=True, + help_text='The type of message being sent. Must match one of the defined `MessageTypes`.', + ) + institution = RelationshipField( + related_view='institutions:institution-detail', + related_view_kwargs={'institution_id': ''}, + help_text='The institution associated with this message.', + ) + bcc_sender = ser.BooleanField( + required=False, + default=False, + help_text='If true, BCCs the sender, giving them a copy of the email message they sent.', + ) + reply_to = ser.BooleanField( + default=False, + help_text='Whether to set the sender\'s username as the `Reply-To` header in the email.', + ) + + def get_absolute_url(self, obj: UserMessage) -> str: + return api_v2_url( + 'users:user-messages', + params={ + 'user_id': self.context['request'].parser_context['kwargs']['user_id'], + 'version': self.context['request'].parser_context['kwargs']['version'], + }, + ) + + def to_internal_value(self, data): + instituion_id = data.pop('institution', None) + data = super().to_internal_value(data) + if instituion_id: + data['institution'] = instituion_id + return data + + class Meta: + type_ = 'user-messages' + + def create(self, validated_data: Dict[str, Any]) -> UserMessage: + """ + Creates a `UserMessage` instance based on validated data. + + Args: + validated_data (Dict[str, Any]): The data validated by the serializer. + + Raises: + ValidationError: If required validations fail (e.g., sender not an institutional admin, + or recipient not affiliated with the institution). + + Returns: + UserMessage: The created `UserMessage` instance. + """ + request = self.context['request'] + sender = request.user + + recipient = get_object_or_error( + OSFUser, + self.context['view'].kwargs['user_id'], + request, + 'user', + ) + + institution_id = validated_data.get('institution') + if not institution_id: + raise exceptions.ValidationError({'institution': 'Institution is required.'}) + + institution = get_object_or_error( + Institution, + institution_id, + request, + 'institution', + ) + + if not sender.is_institutional_admin_at(institution): + raise Conflict({'sender': 'Only institutional administrators can create messages.'}) + + if not recipient.is_affiliated_with_institution(institution): + raise Conflict( + {'user': 'Cannot send to a recipient that is not affiliated with the provided institution.'}, + ) + + return UserMessage.objects.create( + sender=sender, + recipient=recipient, + institution=institution, + message_type=MessageTypes.INSTITUTIONAL_REQUEST, + message_text=validated_data['message_text'], + is_sender_BCCed=validated_data['bcc_sender'], + reply_to=validated_data['reply_to'], + ) diff --git a/api/users/urls.py b/api/users/urls.py index cf9bd0bb7b9..ef53094a121 100644 --- a/api/users/urls.py +++ b/api/users/urls.py @@ -19,6 +19,7 @@ re_path(r'^(?P\w+)/draft_preprints/$', views.UserDraftPreprints.as_view(), name=views.UserDraftPreprints.view_name), re_path(r'^(?P\w+)/registrations/$', views.UserRegistrations.as_view(), name=views.UserRegistrations.view_name), re_path(r'^(?P\w+)/settings/$', views.UserSettings.as_view(), name=views.UserSettings.view_name), + re_path(r'^(?P\w+)/messages/$', views.UserMessageView.as_view(), name=views.UserMessageView.view_name), re_path(r'^(?P\w+)/quickfiles/$', views.UserQuickFiles.as_view(), name=views.UserQuickFiles.view_name), re_path(r'^(?P\w+)/relationships/institutions/$', views.UserInstitutionsRelationship.as_view(), name=views.UserInstitutionsRelationship.view_name), re_path(r'^(?P\w+)/settings/emails/$', views.UserEmailsList.as_view(), name=views.UserEmailsList.view_name), diff --git a/api/users/views.py b/api/users/views.py index 927b5dc2f9b..c9063b1235e 100644 --- a/api/users/views.py +++ b/api/users/views.py @@ -7,6 +7,7 @@ from api.addons.views import AddonSettingsMixin from api.base import permissions as base_permissions +from api.users.permissions import UserMessagePermissions from api.base.waffle_decorators import require_flag from api.base.exceptions import Conflict, UserGone, Gone from api.base.filters import ListFilterMixin, PreprintFilterMixin @@ -55,6 +56,7 @@ UserAccountExportSerializer, ReadEmailUserDetailSerializer, UserChangePasswordSerializer, + UserMessageSerializer, ) from django.contrib.auth.models import AnonymousUser from django.http import JsonResponse @@ -957,3 +959,23 @@ def perform_destroy(self, instance): else: user.remove_unconfirmed_email(email) user.save() + + +class UserMessageView(JSONAPIBaseView, generics.CreateAPIView): + """ + List and create UserMessages for a user. + """ + permission_classes = ( + drf_permissions.IsAuthenticated, + base_permissions.TokenHasScope, + UserMessagePermissions, + ) + + required_read_scopes = [CoreScopes.USERS_MESSAGE_READ_EMAIL] + required_write_scopes = [CoreScopes.USERS_MESSAGE_WRITE_EMAIL] + parser_classes = (JSONAPIMultipleRelationshipsParser, JSONAPIMultipleRelationshipsParserForRegularJSON) + throttle_classes = [BurstRateThrottle, SendEmailThrottle] + serializer_class = UserMessageSerializer + + view_category = 'users' + view_name = 'user-messages' diff --git a/api_tests/nodes/views/test_node_contributor_insti_admin.py b/api_tests/nodes/views/test_node_contributor_insti_admin.py new file mode 100644 index 00000000000..8e95ddef658 --- /dev/null +++ b/api_tests/nodes/views/test_node_contributor_insti_admin.py @@ -0,0 +1,67 @@ +import pytest +from osf.models import Contributor +from osf_tests.factories import ( + AuthUserFactory, + ProjectFactory, + InstitutionFactory, +) +from api.base.settings.defaults import API_BASE + + +@pytest.mark.django_db +class TestChangeInstitutionalAdminContributor: + + @pytest.fixture() + def project_admin(self): + return AuthUserFactory() + + @pytest.fixture() + def project_admin2(self): + return AuthUserFactory() + + @pytest.fixture() + def institution(self): + return InstitutionFactory() + + @pytest.fixture() + def institutional_admin(self, institution): + admin_user = AuthUserFactory() + institution.get_group('institutional_admins').user_set.add(admin_user) + return admin_user + + @pytest.fixture() + def project(self, project_admin, project_admin2, institutional_admin): + project = ProjectFactory(creator=project_admin) + project.add_contributor(project_admin2, permissions='admin', visible=False) + project.add_contributor(institutional_admin, visible=False, make_curator=True) + return project + + @pytest.fixture() + def url_contrib(self, project, institutional_admin): + return f'/{API_BASE}nodes/{project._id}/contributors/{institutional_admin._id}/' + + def test_cannot_set_institutional_admin_contributor_bibliographic(self, app, project_admin, project, + institutional_admin, url_contrib): + res = app.put_json_api( + url_contrib, + { + 'data': { + 'id': f'{project._id}-{institutional_admin._id}', + 'type': 'contributors', + 'attributes': { + 'bibliographic': True, + } + } + }, + auth=project_admin.auth, + expect_errors=True + ) + + assert res.status_code == 409 + assert res.json['errors'][0]['detail'] == 'Curators cannot be made bibliographic contributors' + + contributor = Contributor.objects.get( + node=project, + user=institutional_admin + ) + assert not contributor.visible diff --git a/api_tests/registries_moderation/test_submissions.py b/api_tests/registries_moderation/test_submissions.py index a7e0b3dbb6c..532f71d93fe 100644 --- a/api_tests/registries_moderation/test_submissions.py +++ b/api_tests/registries_moderation/test_submissions.py @@ -4,7 +4,7 @@ from api.base.settings.defaults import API_BASE from api.providers.workflows import Workflows -from osf.utils.workflows import RequestTypes, RegistrationModerationTriggers, RegistrationModerationStates +from osf.utils.workflows import NodeRequestTypes, RegistrationModerationTriggers, RegistrationModerationStates from osf_tests.factories import ( @@ -66,7 +66,7 @@ def registration_with_withdraw_request(self, provider): registration = RegistrationFactory(provider=provider) NodeRequest.objects.create( - request_type=RequestTypes.WITHDRAWAL.value, + request_type=NodeRequestTypes.WITHDRAWAL.value, target=registration, creator=registration.creator ) @@ -75,7 +75,7 @@ def registration_with_withdraw_request(self, provider): @pytest.fixture() def access_request(self, provider): - request = NodeRequestFactory(request_type=RequestTypes.ACCESS.value) + request = NodeRequestFactory(request_type=NodeRequestTypes.ACCESS.value) request.target.provider = provider request.target.save() diff --git a/api_tests/requests/views/test_node_request_institutional_access.py b/api_tests/requests/views/test_node_request_institutional_access.py new file mode 100644 index 00000000000..c3fdc4e111b --- /dev/null +++ b/api_tests/requests/views/test_node_request_institutional_access.py @@ -0,0 +1,397 @@ +from unittest import mock +import pytest + +from api.base.settings.defaults import API_BASE +from api_tests.requests.mixins import NodeRequestTestMixin + +from osf_tests.factories import NodeFactory, InstitutionFactory, AuthUserFactory +from osf.utils.workflows import DefaultStates, NodeRequestTypes +from website.mails import NODE_REQUEST_INSTITUTIONAL_ACCESS_REQUEST + + +@pytest.mark.django_db +class TestNodeRequestListInstitutionalAccess(NodeRequestTestMixin): + + @pytest.fixture() + def url(self, project): + return f'/{API_BASE}nodes/{project._id}/requests/' + + @pytest.fixture() + def institution(self): + return InstitutionFactory(institutional_request_access_enabled=True) + + @pytest.fixture() + def institution_without_access(self): + return InstitutionFactory() + + @pytest.fixture() + def user_with_affiliation(self, institution): + user = AuthUserFactory() + user.add_or_update_affiliated_institution(institution) + return user + + @pytest.fixture() + def user_with_affiliation_on_institution_without_access(self, institution_without_access): + user = AuthUserFactory() + user.add_or_update_affiliated_institution(institution_without_access) + return user + + @pytest.fixture() + def user_without_affiliation(self): + return AuthUserFactory() + + @pytest.fixture() + def institutional_admin(self, institution): + admin_user = AuthUserFactory() + institution.get_group('institutional_admins').user_set.add(admin_user) + return admin_user + + @pytest.fixture() + def institutional_admin_on_institution_without_access(self, institution_without_access): + admin_user = AuthUserFactory() + institution_without_access.get_group('institutional_admins').user_set.add(admin_user) + return admin_user + + @pytest.fixture() + def create_payload(self, institution, user_with_affiliation): + return { + 'data': { + 'attributes': { + 'comment': 'Wanna Philly Philly?', + 'request_type': NodeRequestTypes.INSTITUTIONAL_REQUEST.value, + }, + 'relationships': { + 'institution': { + 'data': { + 'id': institution._id, + 'type': 'institutions' + } + }, + 'message_recipient': { + 'data': { + 'id': user_with_affiliation._id, + 'type': 'users' + } + } + }, + 'type': 'node-requests' + } + } + + @pytest.fixture() + def create_payload_on_institution_without_access(self, institution_without_access, user_with_affiliation_on_institution_without_access): + return { + 'data': { + 'attributes': { + 'comment': 'Wanna Philly Philly?', + 'request_type': NodeRequestTypes.INSTITUTIONAL_REQUEST.value, + }, + 'relationships': { + 'institution': { + 'data': { + 'id': institution_without_access._id, + 'type': 'institutions' + } + }, + 'message_recipient': { + 'data': { + 'id': user_with_affiliation_on_institution_without_access._id, + 'type': 'users' + } + } + }, + 'type': 'node-requests' + } + } + + @pytest.fixture() + def node_with_disabled_access_requests(self, institution): + node = NodeFactory() + node.access_requests_enabled = False + creator = node.creator + creator.add_or_update_affiliated_institution(institution) + creator.save() + node.add_affiliated_institution(institution, creator) + node.save() + return node + + def test_institutional_admin_can_make_institutional_request(self, app, project, institutional_admin, url, create_payload): + """ + Test that an institutional admin can make an institutional access request. + """ + res = app.post_json_api(url, create_payload, auth=institutional_admin.auth) + assert res.status_code == 201 + + # Verify the NodeRequest object is created + node_request = project.requests.get(creator=institutional_admin) + assert node_request.request_type == NodeRequestTypes.INSTITUTIONAL_REQUEST.value + assert node_request.comment == 'Wanna Philly Philly?' + assert node_request.machine_state == DefaultStates.PENDING.value + + def test_non_admin_cant_make_institutional_request(self, app, project, noncontrib, url, create_payload): + """ + Test that a non-institutional admin cannot make an institutional access request. + """ + res = app.post_json_api(url, create_payload, auth=noncontrib.auth, expect_errors=True) + assert res.status_code == 403 + assert 'You do not have permission to perform this action for this institution.' in res.json['errors'][0]['detail'] + + def test_institutional_admin_can_add_requested_permission(self, app, project, institutional_admin, url, create_payload): + """ + Test that an institutional admin can make an institutional access request with requested_permissions. + """ + create_payload['data']['attributes']['requested_permissions'] = 'admin' + + res = app.post_json_api(url, create_payload, auth=institutional_admin.auth) + assert res.status_code == 201 + + # Verify the NodeRequest object is created with the correct requested_permissions + node_request = project.requests.get(creator=institutional_admin) + assert node_request.request_type == NodeRequestTypes.INSTITUTIONAL_REQUEST.value + assert node_request.requested_permissions == 'admin' + + def test_institutional_admin_can_not_add_requested_permission(self, app, project, institutional_admin_on_institution_without_access, url, create_payload_on_institution_without_access): + """ + Test that an institutional admin can not make an institutional access request on institution with disabled access . + """ + create_payload_on_institution_without_access['data']['attributes']['requested_permissions'] = 'admin' + + res = app.post_json_api( + url, create_payload_on_institution_without_access, auth=institutional_admin_on_institution_without_access.auth, expect_errors=True + ) + + assert res.status_code == 403 + + def test_institutional_admin_needs_institution(self, app, project, institutional_admin, url, create_payload): + """ + Test that the payload needs the institution relationship and gives the correct error message. + """ + del create_payload['data']['relationships']['institution'] + + res = app.post_json_api(url, create_payload, auth=institutional_admin.auth, expect_errors=True) + assert res.status_code == 400 + assert 'Institution is required.' in res.json['errors'][0]['detail'] + + def test_institutional_admin_invalid_institution(self, app, project, institutional_admin, url, create_payload): + """ + Test that the payload validates the institution relationship and gives the correct error message when it's + invalid. + """ + create_payload['data']['relationships']['institution']['data']['id'] = 'invalid_id' + + res = app.post_json_api(url, create_payload, auth=institutional_admin.auth, expect_errors=True) + assert res.status_code == 400 + assert 'Institution is does not exist.' in res.json['errors'][0]['detail'] + + def test_institutional_admin_unauth_institution(self, app, project, institution_without_access, institutional_admin, url, create_payload): + """ + Test that the view authenticates the relationship between the institution and the user and gives the correct + error message when it's unauthorized + """ + create_payload['data']['relationships']['institution']['data']['id'] = institution_without_access._id + + res = app.post_json_api(url, create_payload, auth=institutional_admin.auth, expect_errors=True) + assert res.status_code == 403 + assert 'Institutional request access is not enabled.' in res.json['errors'][0]['detail'] + + @mock.patch('api.requests.serializers.send_mail') + @mock.patch('osf.utils.machines.mails.send_mail') + def test_email_send_institutional_request_specific_email( + self, + mock_send_mail_machines, + mock_send_mail_serializers, + user_with_affiliation, + app, + project, + url, + create_payload, + institutional_admin, + institution + ): + """ + Test that the institutional request triggers email notifications to appropriate recipients. + """ + # Set up mock behaviors + project.is_public = True + project.save() + + # Perform the action + res = app.post_json_api(url, create_payload, auth=institutional_admin.auth) + + # Ensure response is successful + assert res.status_code == 201 + + assert mock_send_mail_serializers.call_count == 1 + assert mock_send_mail_machines.call_count == 0 + + # Check calls for osf.utils.machines.mails.send_mail + mock_send_mail_serializers.assert_called_once_with( + to_addr=user_with_affiliation.username, + mail=NODE_REQUEST_INSTITUTIONAL_ACCESS_REQUEST, + user=user_with_affiliation, + bcc_addr=None, + reply_to=None, + **{ + 'sender': institutional_admin, + 'recipient': user_with_affiliation, + 'comment': create_payload['data']['attributes']['comment'], + 'institution': institution, + 'osf_url': mock.ANY, + 'node': project, + } + ) + + @mock.patch('api.requests.serializers.send_mail') + def test_email_not_sent_without_recipient(self, mock_mail, app, project, institutional_admin, url, + create_payload, institution): + """ + Test that an email is not sent when no recipient is listed when an institutional access request is made, + but the request is still made anyway without email. + """ + del create_payload['data']['relationships']['message_recipient'] + res = app.post_json_api(url, create_payload, auth=institutional_admin.auth) + assert res.status_code == 201 + + # Check that an email is sent + assert not mock_mail.called + + @mock.patch('api.requests.serializers.send_mail') + def test_email_not_sent_outside_institution(self, mock_mail, app, project, institutional_admin, url, + create_payload, user_without_affiliation, institution): + """ + Test that you are prevented from requesting a user with the correct institutional affiliation. + """ + create_payload['data']['relationships']['message_recipient']['data']['id'] = user_without_affiliation._id + res = app.post_json_api(url, create_payload, auth=institutional_admin.auth, expect_errors=True) + assert res.status_code == 403 + assert f'User {user_without_affiliation._id} is not affiliated with the institution.' in res.json['errors'][0]['detail'] + + # Check that an email is sent + assert not mock_mail.called + + @mock.patch('api.requests.serializers.send_mail') + def test_email_sent_on_creation( + self, + mock_mail, + app, + project, + institutional_admin, + url, + user_with_affiliation, + create_payload, + institution + ): + """ + Test that an email is sent to the appropriate recipients when an institutional access request is made. + """ + res = app.post_json_api(url, create_payload, auth=institutional_admin.auth) + assert res.status_code == 201 + + assert mock_mail.call_count == 1 + + mock_mail.assert_called_with( + to_addr=user_with_affiliation.username, + mail=NODE_REQUEST_INSTITUTIONAL_ACCESS_REQUEST, + user=user_with_affiliation, + bcc_addr=None, + reply_to=None, + **{ + 'sender': institutional_admin, + 'recipient': user_with_affiliation, + 'comment': create_payload['data']['attributes']['comment'], + 'institution': institution, + 'osf_url': mock.ANY, + 'node': project, + } + ) + + @mock.patch('api.requests.serializers.send_mail') + def test_bcc_institutional_admin( + self, + mock_mail, + app, + project, + institutional_admin, + url, + user_with_affiliation, + create_payload, + institution + ): + """ + Ensure BCC option works as expected, sending messages to sender giving them a copy for themselves. + """ + create_payload['data']['attributes']['bcc_sender'] = True + + res = app.post_json_api(url, create_payload, auth=institutional_admin.auth) + assert res.status_code == 201 + + assert mock_mail.call_count == 1 + + mock_mail.assert_called_with( + to_addr=user_with_affiliation.username, + mail=NODE_REQUEST_INSTITUTIONAL_ACCESS_REQUEST, + user=user_with_affiliation, + bcc_addr=[institutional_admin.username], + reply_to=None, + **{ + 'sender': institutional_admin, + 'recipient': user_with_affiliation, + 'comment': create_payload['data']['attributes']['comment'], + 'institution': institution, + 'osf_url': mock.ANY, + 'node': project, + } + ) + + @mock.patch('api.requests.serializers.send_mail') + def test_reply_to_institutional_admin( + self, + mock_mail, + app, + project, + institutional_admin, + url, + user_with_affiliation, + create_payload, + institution + ): + """ + Ensure reply-to option works as expected, allowing a reply to header be added to the email. + """ + create_payload['data']['attributes']['reply_to'] = True + + res = app.post_json_api(url, create_payload, auth=institutional_admin.auth) + assert res.status_code == 201 + + assert mock_mail.call_count == 1 + + mock_mail.assert_called_with( + to_addr=user_with_affiliation.username, + mail=NODE_REQUEST_INSTITUTIONAL_ACCESS_REQUEST, + user=user_with_affiliation, + bcc_addr=None, + reply_to=institutional_admin.username, + **{ + 'sender': institutional_admin, + 'recipient': user_with_affiliation, + 'comment': create_payload['data']['attributes']['comment'], + 'institution': institution, + 'osf_url': mock.ANY, + 'node': project, + } + ) + + def test_access_requests_disabled_raises_permission_denied( + self, app, node_with_disabled_access_requests, user_with_affiliation, institutional_admin, create_payload + ): + """ + Ensure that when `access_requests_enabled` is `False`, a PermissionDenied error is raised. + """ + res = app.post_json_api( + f'/{API_BASE}nodes/{node_with_disabled_access_requests._id}/requests/', + create_payload, + auth=institutional_admin.auth, + expect_errors=True + ) + assert res.status_code == 403 + assert f"{node_with_disabled_access_requests._id} does not have Access Requests enabled" in res.json['errors'][0]['detail'] diff --git a/api_tests/requests/views/test_request_list_create.py b/api_tests/requests/views/test_node_request_list.py similarity index 58% rename from api_tests/requests/views/test_request_list_create.py rename to api_tests/requests/views/test_node_request_list.py index fdd96b2cd02..f94d3b4acf2 100644 --- a/api_tests/requests/views/test_request_list_create.py +++ b/api_tests/requests/views/test_node_request_list.py @@ -1,10 +1,12 @@ from unittest import mock import pytest -from osf.utils import workflows from api.base.settings.defaults import API_BASE -from api_tests.requests.mixins import NodeRequestTestMixin, PreprintRequestTestMixin +from api_tests.requests.mixins import NodeRequestTestMixin + from osf_tests.factories import NodeFactory, NodeRequestFactory +from osf.utils.workflows import DefaultStates, NodeRequestTypes + @pytest.mark.django_db class TestNodeRequestListCreate(NodeRequestTestMixin): @@ -18,7 +20,7 @@ def create_payload(self): 'data': { 'attributes': { 'comment': 'ASDFG', - 'request_type': 'access' + 'request_type': NodeRequestTypes.ACCESS.value }, 'type': 'node-requests' } @@ -112,8 +114,8 @@ def test_filter_by_machine_state(self, app, project, noncontrib, url, admin, nod initial_node_request = NodeRequestFactory( creator=noncontrib, target=project, - request_type=workflows.RequestTypes.ACCESS.value, - machine_state=workflows.DefaultStates.INITIAL.value + request_type=NodeRequestTypes.ACCESS.value, + machine_state=DefaultStates.INITIAL.value ) filtered_url = f'{url}?filter[machine_state]=pending' res = app.get(filtered_url, auth=admin.auth) @@ -121,69 +123,3 @@ def test_filter_by_machine_state(self, app, project, noncontrib, url, admin, nod ids = [result['id'] for result in res.json['data']] assert initial_node_request._id not in ids assert node_request.machine_state == 'pending' and node_request._id in ids - -@pytest.mark.django_db -class TestPreprintRequestListCreate(PreprintRequestTestMixin): - def url(self, preprint): - return f'/{API_BASE}preprints/{preprint._id}/requests/' - - @pytest.fixture() - def create_payload(self): - return { - 'data': { - 'attributes': { - 'comment': 'ASDFG', - 'request_type': 'withdrawal' - }, - 'type': 'preprint-requests' - } - } - - def test_noncontrib_cannot_submit(self, app, noncontrib, create_payload, pre_mod_preprint, post_mod_preprint, none_mod_preprint): - for preprint in [pre_mod_preprint, post_mod_preprint, none_mod_preprint]: - res = app.post_json_api(self.url(preprint), create_payload, auth=noncontrib.auth, expect_errors=True) - assert res.status_code == 403 - - def test_unauth_cannot_submit(self, app, create_payload, pre_mod_preprint, post_mod_preprint, none_mod_preprint): - for preprint in [pre_mod_preprint, post_mod_preprint, none_mod_preprint]: - res = app.post_json_api(self.url(preprint), create_payload, expect_errors=True) - assert res.status_code == 401 - - def test_write_contributor_cannot_submit(self, app, write_contrib, create_payload, pre_mod_preprint, post_mod_preprint, none_mod_preprint): - for preprint in [pre_mod_preprint, post_mod_preprint, none_mod_preprint]: - res = app.post_json_api(self.url(preprint), create_payload, auth=write_contrib.auth, expect_errors=True) - assert res.status_code == 403 - - def test_admin_can_submit(self, app, admin, create_payload, pre_mod_preprint, post_mod_preprint, none_mod_preprint): - for preprint in [pre_mod_preprint, post_mod_preprint, none_mod_preprint]: - res = app.post_json_api(self.url(preprint), create_payload, auth=admin.auth) - assert res.status_code == 201 - - def test_admin_can_view_requests(self, app, admin, pre_request, post_request, none_request): - for request in [pre_request, post_request, none_request]: - res = app.get(self.url(request.target), auth=admin.auth) - assert res.status_code == 200 - assert res.json['data'][0]['id'] == request._id - - def test_noncontrib_and_write_contrib_cannot_view_requests(self, app, noncontrib, write_contrib, pre_request, post_request, none_request): - for request in [pre_request, post_request, none_request]: - for user in [noncontrib, write_contrib]: - res = app.get(self.url(request.target), auth=user.auth, expect_errors=True) - assert res.status_code == 403 - - def test_unauth_cannot_view_requests(self, app, noncontrib, write_contrib, pre_request, post_request, none_request): - for request in [pre_request, post_request, none_request]: - res = app.get(self.url(request.target), expect_errors=True) - assert res.status_code == 401 - - def test_requester_cannot_submit_again(self, app, admin, create_payload, pre_mod_preprint, pre_request): - res = app.post_json_api(self.url(pre_mod_preprint), create_payload, auth=admin.auth, expect_errors=True) - assert res.status_code == 409 - assert res.json['errors'][0]['detail'] == 'Users may not have more than one withdrawal request per preprint.' - - @pytest.mark.skip('TODO: IN-284 -- add emails') - @mock.patch('website.reviews.listeners.mails.send_mail') - def test_email_sent_to_moderators_on_submit(self, mock_mail, app, admin, create_payload, moderator, post_mod_preprint): - res = app.post_json_api(self.url(post_mod_preprint), create_payload, auth=admin.auth) - assert res.status_code == 201 - assert mock_mail.call_count == 1 diff --git a/api_tests/requests/views/test_preprint_request_list.py b/api_tests/requests/views/test_preprint_request_list.py new file mode 100644 index 00000000000..d23736aa312 --- /dev/null +++ b/api_tests/requests/views/test_preprint_request_list.py @@ -0,0 +1,72 @@ +from unittest import mock +import pytest + +from api.base.settings.defaults import API_BASE +from api_tests.requests.mixins import PreprintRequestTestMixin + + +@pytest.mark.django_db +class TestPreprintRequestListCreate(PreprintRequestTestMixin): + def url(self, preprint): + return f'/{API_BASE}preprints/{preprint._id}/requests/' + + @pytest.fixture() + def create_payload(self): + return { + 'data': { + 'attributes': { + 'comment': 'ASDFG', + 'request_type': 'withdrawal' + }, + 'type': 'preprint-requests' + } + } + + def test_noncontrib_cannot_submit(self, app, noncontrib, create_payload, pre_mod_preprint, post_mod_preprint, none_mod_preprint): + for preprint in [pre_mod_preprint, post_mod_preprint, none_mod_preprint]: + res = app.post_json_api(self.url(preprint), create_payload, auth=noncontrib.auth, expect_errors=True) + assert res.status_code == 403 + + def test_unauth_cannot_submit(self, app, create_payload, pre_mod_preprint, post_mod_preprint, none_mod_preprint): + for preprint in [pre_mod_preprint, post_mod_preprint, none_mod_preprint]: + res = app.post_json_api(self.url(preprint), create_payload, expect_errors=True) + assert res.status_code == 401 + + def test_write_contributor_cannot_submit(self, app, write_contrib, create_payload, pre_mod_preprint, post_mod_preprint, none_mod_preprint): + for preprint in [pre_mod_preprint, post_mod_preprint, none_mod_preprint]: + res = app.post_json_api(self.url(preprint), create_payload, auth=write_contrib.auth, expect_errors=True) + assert res.status_code == 403 + + def test_admin_can_submit(self, app, admin, create_payload, pre_mod_preprint, post_mod_preprint, none_mod_preprint): + for preprint in [pre_mod_preprint, post_mod_preprint, none_mod_preprint]: + res = app.post_json_api(self.url(preprint), create_payload, auth=admin.auth) + assert res.status_code == 201 + + def test_admin_can_view_requests(self, app, admin, pre_request, post_request, none_request): + for request in [pre_request, post_request, none_request]: + res = app.get(self.url(request.target), auth=admin.auth) + assert res.status_code == 200 + assert res.json['data'][0]['id'] == request._id + + def test_noncontrib_and_write_contrib_cannot_view_requests(self, app, noncontrib, write_contrib, pre_request, post_request, none_request): + for request in [pre_request, post_request, none_request]: + for user in [noncontrib, write_contrib]: + res = app.get(self.url(request.target), auth=user.auth, expect_errors=True) + assert res.status_code == 403 + + def test_unauth_cannot_view_requests(self, app, noncontrib, write_contrib, pre_request, post_request, none_request): + for request in [pre_request, post_request, none_request]: + res = app.get(self.url(request.target), expect_errors=True) + assert res.status_code == 401 + + def test_requester_cannot_submit_again(self, app, admin, create_payload, pre_mod_preprint, pre_request): + res = app.post_json_api(self.url(pre_mod_preprint), create_payload, auth=admin.auth, expect_errors=True) + assert res.status_code == 409 + assert res.json['errors'][0]['detail'] == 'Users may not have more than one withdrawal request per preprint.' + + @pytest.mark.skip('TODO: IN-284 -- add emails') + @mock.patch('website.reviews.listeners.mails.send_mail') + def test_email_sent_to_moderators_on_submit(self, mock_mail, app, admin, create_payload, moderator, post_mod_preprint): + res = app.post_json_api(self.url(post_mod_preprint), create_payload, auth=admin.auth) + assert res.status_code == 201 + assert mock_mail.call_count == 1 diff --git a/api_tests/users/views/test_user_message_institutional_access.py b/api_tests/users/views/test_user_message_institutional_access.py new file mode 100644 index 00000000000..36f2a59e252 --- /dev/null +++ b/api_tests/users/views/test_user_message_institutional_access.py @@ -0,0 +1,288 @@ +from unittest import mock +import pytest +from osf.models.user_message import MessageTypes, UserMessage +from api.base.settings.defaults import API_BASE +from osf_tests.factories import ( + AuthUserFactory, + InstitutionFactory +) +from website.mails import USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST +from webtest import AppError + + +@pytest.mark.django_db +class TestUserMessageInstitutionalAccess: + """ + Tests for `UserMessage`. + """ + + @pytest.fixture() + def institution(self): + return InstitutionFactory(institutional_request_access_enabled=True) + + @pytest.fixture() + def institution_without_access(self): + return InstitutionFactory() + + @pytest.fixture() + def user(self): + return AuthUserFactory() + + @pytest.fixture() + def noncontrib(self): + return AuthUserFactory() + + @pytest.fixture() + def user_with_affiliation(self, institution): + user = AuthUserFactory() + user.add_or_update_affiliated_institution(institution) + return user + + @pytest.fixture() + def user_with_affiliation_on_institution_without_access(self, institution_without_access): + user = AuthUserFactory() + user.add_or_update_affiliated_institution(institution_without_access) + return user + + @pytest.fixture() + def institutional_admin(self, institution): + admin_user = AuthUserFactory() + institution.get_group('institutional_admins').user_set.add(admin_user) + return admin_user + + @pytest.fixture() + def institutional_admin_on_institution_without_access(self, institution_without_access): + admin_user = AuthUserFactory() + institution_without_access.get_group('institutional_admins').user_set.add(admin_user) + return admin_user + + @pytest.fixture() + def url_with_affiliation(self, user_with_affiliation): + return f'/{API_BASE}users/{user_with_affiliation._id}/messages/' + + @pytest.fixture() + def url_with_affiliation_on_institution_without_access(self, user_with_affiliation_on_institution_without_access): + return f'/{API_BASE}users/{user_with_affiliation_on_institution_without_access._id}/messages/' + + @pytest.fixture() + def url_without_affiliation(self, user): + return f'/{API_BASE}users/{user._id}/messages/' + + @pytest.fixture() + def payload(self, institution, user): + return { + 'data': { + 'attributes': { + 'message_text': 'Requesting user access for collaboration', + 'message_type': MessageTypes.INSTITUTIONAL_REQUEST.value, + }, + 'relationships': { + 'institution': { + 'data': {'id': institution._id, 'type': 'institutions'}, + }, + }, + 'type': 'user-message' + } + } + + @mock.patch('osf.models.user_message.send_mail') + def test_institutional_admin_can_create_message(self, mock_send_mail, app, institutional_admin, institution, url_with_affiliation, payload): + """ + Ensure an institutional admin can create a `UserMessage` with a `message` and `institution`. + """ + mock_send_mail.return_value = mock.MagicMock() + + res = app.post_json_api( + url_with_affiliation, + payload, + auth=institutional_admin.auth + ) + assert res.status_code == 201 + data = res.json['data'] + + user_message = UserMessage.objects.get(sender=institutional_admin) + + assert user_message.message_text == payload['data']['attributes']['message_text'] + assert user_message.institution == institution + + mock_send_mail.assert_called_once() + assert mock_send_mail.call_args[1]['to_addr'] == user_message.recipient.username + assert 'Requesting user access for collaboration' in mock_send_mail.call_args[1]['message_text'] + assert user_message._id == data['id'] + + @mock.patch('osf.models.user_message.send_mail') + def test_institutional_admin_can_not_create_message(self, mock_send_mail, app, institutional_admin_on_institution_without_access, + institution_without_access, url_with_affiliation_on_institution_without_access, + payload): + """ + Ensure an institutional admin cannot create a `UserMessage` with a `message` and `institution` witch has 'institutional_request_access_enabled' as False + """ + mock_send_mail.return_value = mock.MagicMock() + + # Use pytest.raises to explicitly expect the 403 error + with pytest.raises(AppError) as exc_info: + app.post_json_api( + url_with_affiliation_on_institution_without_access, + payload, + auth=institutional_admin_on_institution_without_access.auth + ) + + # Assert that the raised error contains the 403 Forbidden status + assert '403 Forbidden' in str(exc_info.value) + + def test_unauthenticated_user_cannot_create_message(self, app, user, url_with_affiliation, payload): + """ + Ensure that unauthenticated users cannot create a `UserMessage`. + """ + res = app.post_json_api(url_with_affiliation, payload, expect_errors=True) + assert res.status_code == 401 + assert 'Authentication credentials were not provided' in res.json['errors'][0]['detail'] + + def test_non_institutional_admin_cannot_create_message(self, app, noncontrib, user, url_with_affiliation, payload): + """ + Ensure a non-institutional admin cannot create a `UserMessage`, even with valid data. + """ + res = app.post_json_api(url_with_affiliation, payload, auth=noncontrib.auth, expect_errors=True) + assert res.status_code == 403 + + def test_request_without_institution(self, app, institutional_admin, user, url_with_affiliation, payload): + """ + Test that a `UserMessage` can be created without specifying an institution, and `institution` is None. + """ + del payload['data']['relationships']['institution'] + + res = app.post_json_api(url_with_affiliation, payload, auth=institutional_admin.auth, expect_errors=True) + assert res.status_code == 400 + error = res.json['errors'] + assert error == [ + { + 'source': { + 'pointer': '/data/relationships/institution' + }, + 'detail': 'Institution is required.' + } + ] + + def test_missing_message_fails(self, app, institutional_admin, user, url_with_affiliation, payload): + """ + Ensure a `UserMessage` cannot be created without a `message` attribute. + """ + del payload['data']['attributes']['message_text'] + + res = app.post_json_api(url_with_affiliation, payload, auth=institutional_admin.auth, expect_errors=True) + assert res.status_code == 400 + error = res.json['errors'] + assert error == [ + { + 'source': { + 'pointer': '/data/attributes/message_text' + }, + 'detail': 'This field is required.', + } + ] + + def test_admin_cannot_message_user_outside_institution( + self, + app, + institutional_admin, + url_without_affiliation, + payload, + user + ): + """ + Ensure that an institutional admin cannot create a `UserMessage` for a user who is not affiliated with their institution. + """ + res = app.post_json_api(url_without_affiliation, payload, auth=institutional_admin.auth, expect_errors=True) + assert res.status_code == 409 + assert ('Cannot send to a recipient that is not affiliated with the provided institution.' + in res.json['errors'][0]['detail']['user']) + + @mock.patch('osf.models.user_message.send_mail') + def test_cc_institutional_admin( + self, + mock_send_mail, + app, + institutional_admin, + institution, + url_with_affiliation, + user_with_affiliation, + payload + ): + """ + Ensure CC option works as expected, sending messages to all institutional admins except the sender. + """ + mock_send_mail.return_value = mock.MagicMock() + + # Enable CC in the payload + payload['data']['attributes']['bcc_sender'] = True + + # Perform the API request + res = app.post_json_api( + url_with_affiliation, + payload, + auth=institutional_admin.auth, + ) + assert res.status_code == 201 + user_message = UserMessage.objects.get() + + assert user_message.is_sender_BCCed + # Two emails are sent during the CC but this is how the mock works `send_email` is called once. + mock_send_mail.assert_called_once_with( + to_addr=user_with_affiliation.username, + bcc_addr=[institutional_admin.username], + reply_to=None, + message_text='Requesting user access for collaboration', + mail=USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST, + user=user_with_affiliation, + sender=institutional_admin, + recipient=user_with_affiliation, + institution=institution, + ) + + @mock.patch('osf.models.user_message.send_mail') + def test_cc_field_defaults_to_false(self, mock_send_mail, app, institutional_admin, url_with_affiliation, user_with_affiliation, institution, payload): + """ + Ensure the `cc` field defaults to `false` when not provided in the payload. + """ + res = app.post_json_api(url_with_affiliation, payload, auth=institutional_admin.auth) + assert res.status_code == 201 + + user_message = UserMessage.objects.get(sender=institutional_admin) + assert user_message.message_text == payload['data']['attributes']['message_text'] + mock_send_mail.assert_called_once_with( + to_addr=user_with_affiliation.username, + bcc_addr=None, + reply_to=None, + message_text='Requesting user access for collaboration', + mail=USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST, + user=user_with_affiliation, + sender=institutional_admin, + recipient=user_with_affiliation, + institution=institution, + ) + + @mock.patch('osf.models.user_message.send_mail') + def test_reply_to_header_set(self, mock_send_mail, app, institutional_admin, user_with_affiliation, institution, url_with_affiliation, payload): + """ + Ensure that the 'Reply-To' header is correctly set to the sender's email address. + """ + payload['data']['attributes']['reply_to'] = True + + res = app.post_json_api( + url_with_affiliation, + payload, + auth=institutional_admin.auth, + ) + assert res.status_code == 201 + + mock_send_mail.assert_called_once_with( + to_addr=user_with_affiliation.username, + bcc_addr=None, + reply_to=institutional_admin.username, + message_text='Requesting user access for collaboration', + mail=USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST, + user=user_with_affiliation, + sender=institutional_admin, + recipient=user_with_affiliation, + institution=institution, + ) diff --git a/framework/auth/oauth_scopes.py b/framework/auth/oauth_scopes.py index c18084fbb9a..67644050383 100644 --- a/framework/auth/oauth_scopes.py +++ b/framework/auth/oauth_scopes.py @@ -29,6 +29,8 @@ class CoreScopes: USERS_READ = 'users_read' USERS_WRITE = 'users_write' + USERS_MESSAGE_READ_EMAIL = 'users_message_read_email' + USERS_MESSAGE_WRITE_EMAIL = 'users_message_write_email' USERS_CREATE = 'users_create' USER_SETTINGS_READ = 'user.settings_read' @@ -323,7 +325,8 @@ class ComposedScopes: CoreScopes.MEETINGS_READ, CoreScopes.INSTITUTION_READ, CoreScopes.SEARCH, - CoreScopes.SCOPES_READ + CoreScopes.SCOPES_READ, + CoreScopes.USERS_MESSAGE_READ_EMAIL )\ + ( CoreScopes.READ_COLLECTION_SUBMISSION, @@ -342,7 +345,8 @@ class ComposedScopes: + ( CoreScopes.CEDAR_METADATA_RECORD_WRITE, CoreScopes.WRITE_COLLECTION_SUBMISSION_ACTION, - CoreScopes.WRITE_COLLECTION_SUBMISSION + CoreScopes.WRITE_COLLECTION_SUBMISSION, + CoreScopes.USERS_MESSAGE_WRITE_EMAIL ) # Admin permissions- includes functionality not intended for third-party use diff --git a/framework/email/tasks.py b/framework/email/tasks.py index bb2528fd1d3..cf43395222e 100644 --- a/framework/email/tasks.py +++ b/framework/email/tasks.py @@ -5,7 +5,21 @@ from io import BytesIO from sendgrid import SendGridAPIClient -from sendgrid.helpers.mail import Attachment, Mail, FileContent, Category +from sendgrid.helpers.mail import ( + Mail, + Bcc, + ReplyTo, + Category, + Attachment, + FileContent, + Email, + To, + Personalization, + Cc, + FileName, + Disposition, +) + from framework import sentry from framework.celery_tasks import app @@ -20,8 +34,10 @@ def send_email( to_addr: str, subject: str, message: str, + reply_to: bool = False, ttls: bool = True, login: bool = True, + bcc_addr: [] = None, username: str = None, password: str = None, categories=None, @@ -57,6 +73,8 @@ def send_email( categories=categories, attachment_name=attachment_name, attachment_content=attachment_content, + reply_to=reply_to, + bcc_addr=bcc_addr, ) else: return _send_with_smtp( @@ -68,35 +86,58 @@ def send_email( login=login, username=username, password=password, + reply_to=reply_to, + bcc_addr=bcc_addr, ) -def _send_with_smtp(from_addr, to_addr, subject, message, ttls=True, login=True, username=None, password=None): +def _send_with_smtp( + from_addr, + to_addr, + subject, + message, + ttls=True, + login=True, + username=None, + password=None, + bcc_addr=None, + reply_to=None, +): username = username or settings.MAIL_USERNAME password = password or settings.MAIL_PASSWORD if login and (username is None or password is None): logger.error('Mail username and password not set; skipping send.') - return + return False - msg = MIMEText(message, 'html', _charset='utf-8') + msg = MIMEText( + message, + 'html', + _charset='utf-8', + ) msg['Subject'] = subject msg['From'] = from_addr msg['To'] = to_addr - s = smtplib.SMTP(settings.MAIL_SERVER) - s.ehlo() - if ttls: - s.starttls() - s.ehlo() - if login: - s.login(username, password) - s.sendmail( - from_addr=from_addr, - to_addrs=[to_addr], - msg=msg.as_string(), - ) - s.quit() + if reply_to: + msg['Reply-To'] = reply_to + + # Combine recipients for SMTP + recipients = [to_addr] + (bcc_addr or []) + + # Establish SMTP connection and send the email + with smtplib.SMTP(settings.MAIL_SERVER) as server: + server.ehlo() + if ttls: + server.starttls() + server.ehlo() + if login: + server.login(username, password) + server.sendmail( + from_addr=from_addr, + to_addrs=recipients, + msg=msg.as_string() + ) return True @@ -108,39 +149,72 @@ def _send_with_sendgrid( categories=None, attachment_name: str = None, attachment_content=None, + cc_addr=None, + bcc_addr=None, + reply_to=None, client=None, ): - if ( - settings.SENDGRID_WHITELIST_MODE and to_addr in settings.SENDGRID_EMAIL_WHITELIST - ) or settings.SENDGRID_WHITELIST_MODE is False: - client = client or SendGridAPIClient(settings.SENDGRID_API_KEY) - mail = Mail(from_email=from_addr, html_content=message, to_emails=to_addr, subject=subject) - if categories: - mail.category = [Category(x) for x in categories] - if attachment_name and attachment_content: - content_bytes = _content_to_bytes(attachment_content) - content_bytes = FileContent(b64encode(content_bytes).decode()) - attachment = Attachment(file_content=content_bytes, file_name=attachment_name) - mail.add_attachment(attachment) - - response = client.send(mail) - if response.status_code >= 400: - sentry.log_message( - f'{response.status_code} error response from sendgrid.' - f'from_addr: {from_addr}\n' - f'to_addr: {to_addr}\n' - f'subject: {subject}\n' - 'mimetype: html\n' - f'message: {response.body[:30]}\n' - f'categories: {categories}\n' - f'attachment_name: {attachment_name}\n' - ) - return response.status_code < 400 - else: + in_allowed_list = to_addr in settings.SENDGRID_EMAIL_WHITELIST + if settings.SENDGRID_WHITELIST_MODE and not in_allowed_list: sentry.log_message( f'SENDGRID_WHITELIST_MODE is True. Failed to send emails to non-whitelisted recipient {to_addr}.' ) + return False + + client = client or SendGridAPIClient(settings.SENDGRID_API_KEY) + mail = Mail( + from_email=Email(from_addr), + html_content=message, + subject=subject, + ) + + # Personalization to handle To, CC, and BCC sendgrid client concept + personalization = Personalization() + + personalization.add_to(To(to_addr)) + + if cc_addr: + if isinstance(cc_addr, str): + cc_addr = [cc_addr] + for email in cc_addr: + personalization.add_cc(Cc(email)) + + if bcc_addr: + if isinstance(bcc_addr, str): + bcc_addr = [bcc_addr] + for email in bcc_addr: + personalization.add_bcc(Bcc(email)) + + if reply_to: + mail.reply_to = ReplyTo(reply_to) + + mail.add_personalization(personalization) + + if categories: + mail.add_category([Category(x) for x in categories]) + + if attachment_name and attachment_content: + attachment = Attachment( + file_content=FileContent(b64encode(attachment_content).decode()), + file_name=FileName(attachment_name), + disposition=Disposition('attachment') + ) + mail.add_attachment(attachment) + response = client.send(mail) + if response.status_code not in (200, 201, 202): + sentry.log_message( + f'{response.status_code} error response from sendgrid.' + f'from_addr: {from_addr}\n' + f'to_addr: {to_addr}\n' + f'subject: {subject}\n' + 'mimetype: html\n' + f'message: {response.body[:30]}\n' + f'categories: {categories}\n' + f'attachment_name: {attachment_name}\n' + ) + else: + return True def _content_to_bytes(attachment_content: BytesIO | str | bytes) -> bytes: if isinstance(attachment_content, bytes): diff --git a/osf/migrations/0025_institutional_request_access_enabled_and_more.py b/osf/migrations/0025_institutional_request_access_enabled_and_more.py new file mode 100644 index 00000000000..0d72e062b99 --- /dev/null +++ b/osf/migrations/0025_institutional_request_access_enabled_and_more.py @@ -0,0 +1,52 @@ +# Generated by Django 4.2.13 on 2025-01-09 19:19 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django_extensions.db.fields +import osf.models.base + + +class Migration(migrations.Migration): + + dependencies = [ + ('osf', '0024_institution_link_to_external_reports_archive'), + ] + + operations = [ + migrations.AddField( + model_name='institution', + name='institutional_request_access_enabled', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='noderequest', + name='requested_permissions', + field=models.CharField(blank=True, choices=[('read', 'read'), ('write', 'write'), ('admin', 'admin')], help_text='The permissions being requested for the node (e.g., read, write, admin).', max_length=31, null=True), + ), + migrations.AlterField( + model_name='noderequest', + name='request_type', + field=models.CharField(choices=[('access', 'Access'), ('withdrawal', 'Withdrawal'), ('institutional_request', 'Institutional_Request')], help_text='The specific type of node request (e.g., access request).', max_length=31), + ), + migrations.CreateModel( + name='UserMessage', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', django_extensions.db.fields.CreationDateTimeField(auto_now_add=True, verbose_name='created')), + ('modified', django_extensions.db.fields.ModificationDateTimeField(auto_now=True, verbose_name='modified')), + ('_id', models.CharField(db_index=True, default=osf.models.base.generate_object_id, max_length=24, unique=True)), + ('message_text', models.TextField(help_text='The content of the message. The custom text of a formatted email.')), + ('message_type', models.CharField(choices=[('institutional_request', 'INSTITUTIONAL_REQUEST')], help_text='The type of message being sent, as defined in MessageTypes.', max_length=50)), + ('is_sender_BCCed', models.BooleanField(default=False, help_text='The boolean value that indicates whether other institutional admins were BCCed')), + ('reply_to', models.BooleanField(default=False, help_text="Whether to set the sender's username as the `Reply-To` header in the email.")), + ('institution', models.ForeignKey(help_text='The institution associated with this message.', on_delete=django.db.models.deletion.CASCADE, to='osf.institution')), + ('recipient', models.ForeignKey(help_text='The recipient of this message.', on_delete=django.db.models.deletion.CASCADE, related_name='received_user_messages', to=settings.AUTH_USER_MODEL)), + ('sender', models.ForeignKey(help_text='The user who sent this message.', on_delete=django.db.models.deletion.CASCADE, related_name='sent_user_messages', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'abstract': False, + }, + bases=(models.Model, osf.models.base.QuerySetExplainMixin), + ), + ] diff --git a/osf/migrations/0026_add_is_institutional_request_is_curator.py b/osf/migrations/0026_add_is_institutional_request_is_curator.py new file mode 100644 index 00000000000..51da69ada2c --- /dev/null +++ b/osf/migrations/0026_add_is_institutional_request_is_curator.py @@ -0,0 +1,28 @@ +# Generated by Django 4.2.13 on 2025-01-10 19:27 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('osf', '0025_institutional_request_access_enabled_and_more'), + ] + + operations = [ + migrations.AddField( + model_name='contributor', + name='is_curator', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='noderequest', + name='is_institutional_request', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='preprintrequest', + name='is_institutional_request', + field=models.BooleanField(default=False), + ), + ] diff --git a/osf/models/__init__.py b/osf/models/__init__.py index cad31ea323f..4dbcb4d42ff 100644 --- a/osf/models/__init__.py +++ b/osf/models/__init__.py @@ -108,3 +108,5 @@ Email, OSFUser, ) +from .user_message import UserMessage + diff --git a/osf/models/contributor.py b/osf/models/contributor.py index 4c5807f3ee2..a427a7e50f6 100644 --- a/osf/models/contributor.py +++ b/osf/models/contributor.py @@ -1,4 +1,4 @@ -from django.db import models +from django.db import models, IntegrityError from osf.utils.fields import NonNaiveDateTimeField from osf.utils import permissions @@ -30,6 +30,7 @@ def permission(self): class Contributor(AbstractBaseContributor): node = models.ForeignKey('AbstractNode', on_delete=models.CASCADE) + is_curator = models.BooleanField(default=False) @property def _id(self): @@ -41,6 +42,12 @@ class Meta: # NOTE: Adds an _order column order_with_respect_to = 'node' + def save(self, *args, **kwargs): + if self.is_curator and self.visible: + raise IntegrityError('Curators cannot be made bibliographic contributors') + + return super().save(*args, **kwargs) + class PreprintContributor(AbstractBaseContributor): preprint = models.ForeignKey('Preprint', on_delete=models.CASCADE) diff --git a/osf/models/institution.py b/osf/models/institution.py index d0ce38eacf4..5dce3c1df36 100644 --- a/osf/models/institution.py +++ b/osf/models/institution.py @@ -103,6 +103,7 @@ class Institution(DirtyFieldsMixin, Loggable, ObjectIDMixin, BaseModel, Guardian related_name='institutions' ) + institutional_request_access_enabled = models.BooleanField(default=False) is_deleted = models.BooleanField(default=False, db_index=True) deleted = NonNaiveDateTimeField(null=True, blank=True) deactivated = NonNaiveDateTimeField(null=True, blank=True) diff --git a/osf/models/mixins.py b/osf/models/mixins.py index 17c35db04a6..54e945a9f47 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -1327,7 +1327,7 @@ def _get_admin_contributors_query(self, users, require_active=True): return qs def add_contributor(self, contributor, permissions=None, visible=True, - send_email=None, auth=None, log=True, save=False): + send_email=None, auth=None, log=True, save=False, make_curator=False): """Add a contributor to the project. :param User contributor: The contributor to be added @@ -1338,6 +1338,7 @@ def add_contributor(self, contributor, permissions=None, visible=True, :param Auth auth: All the auth information including user, API key :param bool log: Add log to self :param bool save: Save after adding contributor + :param bool make_curator incicates whether the user should be an institituional curator :returns: Whether contributor was added """ send_email = send_email or self.contributor_email_template @@ -1393,6 +1394,11 @@ def add_contributor(self, contributor, permissions=None, visible=True, if getattr(self, 'get_identifier_value', None) and self.get_identifier_value('doi'): request, user_id = get_request_and_user_id() self.update_or_enqueue_on_resource_updated(user_id, first_save=False, saved_fields=['contributors']) + + if make_curator: + contributor_obj.is_curator = True + contributor_obj.save() + return contrib_to_add def add_contributors(self, contributors, auth=None, log=True, save=False): @@ -1839,7 +1845,11 @@ def set_visible(self, user, visible, log=True, auth=None, save=False): if visible and not self.contributor_class.objects.filter(**kwargs).exists(): set_visible_kwargs = kwargs set_visible_kwargs['visible'] = False - self.contributor_class.objects.filter(**set_visible_kwargs).update(visible=True) + contribs = self.contributor_class.objects.filter(**set_visible_kwargs) + if self.guardian_object_type == 'node' and contribs.filter(is_curator=True).exists(): + raise ValueError('Curators cannot be made bibliographic contributors') + contribs.update(visible=True) + elif not visible and self.contributor_class.objects.filter(**kwargs).exists(): num_visible_kwargs = self.contributor_kwargs num_visible_kwargs['visible'] = True diff --git a/osf/models/node.py b/osf/models/node.py index 62925966e2e..6aff5126abe 100644 --- a/osf/models/node.py +++ b/osf/models/node.py @@ -15,7 +15,7 @@ from django.core.exceptions import ImproperlyConfigured from django.core.paginator import Paginator from django.urls import reverse -from django.db import models, connection +from django.db import models, connection, IntegrityError from django.db.models.signals import post_save from django.dispatch import receiver from django.utils import timezone @@ -77,6 +77,7 @@ from website.util.metrics import OsfSourceTags, CampaignSourceTags from website.util import api_url_for, api_v2_url, web_url_for from .base import BaseModel, GuidMixin, GuidMixinQuerySet +from api.base.exceptions import Conflict from api.caching.tasks import update_storage_usage from api.caching import settings as cache_settings from api.caching.utils import storage_usage_cache @@ -1079,7 +1080,18 @@ def set_visible(self, user, visible, log=True, auth=None, save=False): if not self.is_contributor(user): raise ValueError(f'User {user} not in contributors') if visible and not Contributor.objects.filter(node=self, user=user, visible=True).exists(): - Contributor.objects.filter(node=self, user=user, visible=False).update(visible=True) + contributor = Contributor.objects.get( + node=self, + user=user, + visible=False + ) + try: + contributor.visible = True + contributor.save() + except IntegrityError as e: + if 'Curators cannot be made bibliographic contributors' in str(e): + raise Conflict(str(e)) from e + raise e elif not visible and Contributor.objects.filter(node=self, user=user, visible=True).exists(): if Contributor.objects.filter(node=self, visible=True).count() == 1: raise ValueError('Must have at least one visible contributor') diff --git a/osf/models/request.py b/osf/models/request.py index d91837cbc7c..cab7469b724 100644 --- a/osf/models/request.py +++ b/osf/models/request.py @@ -1,8 +1,8 @@ from django.db import models - from .base import BaseModel, ObjectIDMixin -from osf.utils.workflows import RequestTypes +from osf.utils.workflows import RequestTypes, NodeRequestTypes from .mixins import NodeRequestableMixin, PreprintRequestableMixin +from osf.utils.permissions import API_CONTRIBUTOR_PERMISSIONS class AbstractRequest(BaseModel, ObjectIDMixin): @@ -12,6 +12,7 @@ class Meta: request_type = models.CharField(max_length=31, choices=RequestTypes.choices()) creator = models.ForeignKey('OSFUser', related_name='submitted_%(class)s', on_delete=models.CASCADE) comment = models.TextField(null=True, blank=True) + is_institutional_request = models.BooleanField(default=False) @property def target(self): @@ -22,6 +23,18 @@ class NodeRequest(AbstractRequest, NodeRequestableMixin): """ Request for Node Access """ target = models.ForeignKey('AbstractNode', related_name='requests', on_delete=models.CASCADE) + request_type = models.CharField( + max_length=31, + choices=NodeRequestTypes.choices(), + help_text='The specific type of node request (e.g., access request).' + ) + requested_permissions = models.CharField( + max_length=31, + choices=((perm.lower(), perm) for perm in API_CONTRIBUTOR_PERMISSIONS), + null=True, + blank=True, + help_text='The permissions being requested for the node (e.g., read, write, admin).' + ) class PreprintRequest(AbstractRequest, PreprintRequestableMixin): diff --git a/osf/models/user.py b/osf/models/user.py index bb0f97f91a9..26cfd93b8dc 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -644,6 +644,34 @@ def osf_groups(self): OSFGroup = apps.get_model('osf.OSFGroup') return get_objects_for_user(self, 'member_group', OSFGroup, with_superuser=False) + def is_institutional_admin_at(self, institution): + """ + Checks if user is admin of a specific institution. + """ + return self.has_perms( + institution.groups['institutional_admins'], + institution + ) + + def is_institutional_admin(self): + """ + Checks if user is admin of any institution. + """ + return self.groups.filter( + name__startswith='institution_', + name__endswith='_institutional_admins' + ).exists() + + def is_institutional_curator(self, node): + """ + Checks if user is user has curator permissions for a node. + """ + return Contributor.objects.filter( + node=node, + user=self, + is_curator=True, + ).exists() + def group_role(self, group): """ For the given OSFGroup, return the user's role - either member or manager diff --git a/osf/models/user_message.py b/osf/models/user_message.py new file mode 100644 index 00000000000..ac77cefe629 --- /dev/null +++ b/osf/models/user_message.py @@ -0,0 +1,122 @@ +from typing import Type +from django.db import models +from django.db.models.signals import post_save +from django.dispatch import receiver + +from .base import BaseModel, ObjectIDMixin +from website.mails import send_mail, USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST + + +class MessageTypes(models.TextChoices): + """ + Enumeration of the different user-to-user message types supported by UserMessage. + + Notes: + Message types should be limited to direct communication between two users. + These may include cases where the sender represents an organization or group, + but they must not involve bulk messaging or group-wide notifications. + """ + # Admin-to-member communication within an institution. + INSTITUTIONAL_REQUEST = ('institutional_request', 'INSTITUTIONAL_REQUEST') + + @classmethod + def get_template(cls: Type['MessageTypes'], message_type: str) -> str: + """ + Retrieve the email template associated with a specific message type. + + Args: + message_type (str): The type of the message. + + Returns: + str: The email template string for the specified message type. + """ + return { + cls.INSTITUTIONAL_REQUEST: USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST + }[message_type] + + +class UserMessage(BaseModel, ObjectIDMixin): + """ + Represents a user-to-user message, potentially sent on behalf of an organization or group. + + Attributes: + sender (OSFUser): The user who initiated the message. + recipient (OSFUser): The intended recipient of the message. + message_text (str): The content of the message being sent. + message_type (str): The type of message, e.g., 'institutional_request'. + institution (Institution): The institution linked to the message, if applicable. + """ + sender = models.ForeignKey( + 'OSFUser', + on_delete=models.CASCADE, + related_name='sent_user_messages', + help_text='The user who sent this message.' + ) + recipient = models.ForeignKey( + 'OSFUser', + on_delete=models.CASCADE, + related_name='received_user_messages', + help_text='The recipient of this message.' + ) + message_text = models.TextField( + help_text='The content of the message. The custom text of a formatted email.' + ) + message_type = models.CharField( + max_length=50, + choices=MessageTypes.choices, + help_text='The type of message being sent, as defined in MessageTypes.' + ) + institution = models.ForeignKey( + 'Institution', + on_delete=models.CASCADE, + help_text='The institution associated with this message.' + ) + is_sender_BCCed = models.BooleanField( + default=False, + help_text='The boolean value that indicates whether other institutional admins were BCCed', + ) + reply_to = models.BooleanField( + default=False, + help_text='Whether to set the sender\'s username as the `Reply-To` header in the email.' + ) + + def send_institution_request(self) -> None: + """ + Sends an institutional access request email to the recipient of the message. + """ + send_mail( + mail=MessageTypes.get_template(self.message_type), + to_addr=self.recipient.username, + bcc_addr=[self.sender.username] if self.is_sender_BCCed else None, + reply_to=self.sender.username if self.reply_to else None, + user=self.recipient, + **{ + 'sender': self.sender, + 'recipient': self.recipient, + 'message_text': self.message_text, + 'institution': self.institution, + }, + ) + + +@receiver(post_save, sender=UserMessage) +def user_message_created(sender: Type[UserMessage], instance: UserMessage, created: bool, **kwargs) -> None: + """ + Signal handler executed after a UserMessage instance is saved. + + Args: + sender (Type[UserMessage]): The UserMessage model class. + instance (UserMessage): The newly created instance of the UserMessage. + created (bool): Whether this is the first save of the instance. + + Notes: + If the message type is 'INSTITUTIONAL_REQUEST', it triggers sending an + institutional request email. Raises an error for unsupported message types. + """ + if not created: + return # Ignore subsequent saves. + + if instance.message_type == MessageTypes.INSTITUTIONAL_REQUEST: + instance.send_institution_request() + else: + raise NotImplementedError(f'Unsupported message type: {instance.message_type}') diff --git a/osf/utils/machines.py b/osf/utils/machines.py index 8e4b8f07338..ac63b1b7894 100644 --- a/osf/utils/machines.py +++ b/osf/utils/machines.py @@ -1,4 +1,5 @@ from django.utils import timezone +from django.db import IntegrityError from transitions import Machine, MachineError from api.providers.workflows import Workflows @@ -7,7 +8,6 @@ from osf.exceptions import InvalidTransitionError from osf.models.preprintlog import PreprintLog from osf.models.action import ReviewAction, NodeRequestAction, PreprintRequestAction - from osf.utils import permissions from osf.utils.workflows import ( DefaultStates, @@ -19,6 +19,7 @@ APPROVAL_TRANSITIONS, CollectionSubmissionStates, COLLECTION_SUBMISSION_TRANSITIONS, + NodeRequestTypes ) from website.mails import mails from website.reviews import signals as reviews_signals @@ -26,6 +27,8 @@ from osf.utils import notifications as notify +from api.base.exceptions import Conflict + class BaseMachine(Machine): action = None @@ -199,12 +202,19 @@ def save_changes(self, ev): if ev.event.name == DefaultTriggers.ACCEPT.value: if not self.machineable.target.is_contributor(self.machineable.creator): contributor_permissions = ev.kwargs.get('permissions', permissions.READ) - self.machineable.target.add_contributor( - self.machineable.creator, - auth=Auth(ev.kwargs['user']), - permissions=contributor_permissions, - visible=ev.kwargs.get('visible', True), - send_email=f'{self.machineable.request_type}_request') + try: + self.machineable.target.add_contributor( + self.machineable.creator, + auth=Auth(ev.kwargs['user']), + permissions=contributor_permissions, + visible=ev.kwargs.get('visible', True), + send_email=f'{self.machineable.request_type}_request', + make_curator=self.machineable.request_type == NodeRequestTypes.INSTITUTIONAL_REQUEST.value, + ) + except IntegrityError as e: + if 'Curators cannot be made bibliographic contributors' in str(e): + raise Conflict(str(e)) from e + raise e def resubmission_allowed(self, ev): # TODO: [PRODUCT-395] @@ -216,15 +226,15 @@ def notify_submit(self, ev): context = self.get_context() context['contributors_url'] = f'{self.machineable.target.absolute_url}contributors/' context['project_settings_url'] = f'{self.machineable.target.absolute_url}settings/' - - for admin in self.machineable.target.get_users_with_perm(permissions.ADMIN): - mails.send_mail( - admin.username, - mails.ACCESS_REQUEST_SUBMITTED, - admin=admin, - osf_contact_email=OSF_CONTACT_EMAIL, - **context - ) + if not self.machineable.request_type == NodeRequestTypes.INSTITUTIONAL_REQUEST.value: + for admin in self.machineable.target.get_users_with_perm(permissions.ADMIN): + mails.send_mail( + admin.username, + mails.ACCESS_REQUEST_SUBMITTED, + admin=admin, + osf_contact_email=OSF_CONTACT_EMAIL, + **context + ) def notify_resubmit(self, ev): """ Notify admins that someone is requesting access again diff --git a/osf/utils/workflows.py b/osf/utils/workflows.py index 43d21659bde..b054de25452 100644 --- a/osf/utils/workflows.py +++ b/osf/utils/workflows.py @@ -515,3 +515,9 @@ def db_name(self): class RequestTypes(ChoiceEnum): ACCESS = 'access' WITHDRAWAL = 'withdrawal' + +@unique +class NodeRequestTypes(ChoiceEnum): + ACCESS = 'access' + WITHDRAWAL = 'withdrawal' + INSTITUTIONAL_REQUEST = 'institutional_request' diff --git a/osf_tests/factories.py b/osf_tests/factories.py index 0bd1664977d..7df749c0f72 100644 --- a/osf_tests/factories.py +++ b/osf_tests/factories.py @@ -257,6 +257,7 @@ class InstitutionFactory(DjangoModelFactory): email_domains = FakeList('domain_name', n=1) orcid_record_verified_source = '' delegation_protocol = '' + institutional_request_access_enabled = False class Meta: model = models.Institution @@ -1021,6 +1022,13 @@ class Meta: comment = factory.Faker('text') + +class UserMessageFactory(DjangoModelFactory): + class Meta: + model = models.UserMessage + + comment = factory.Faker('text') + osfstorage_settings = apps.get_app_config('addons_osfstorage') diff --git a/osf_tests/test_institutional_admin_contributors.py b/osf_tests/test_institutional_admin_contributors.py new file mode 100644 index 00000000000..7cfd9044ce2 --- /dev/null +++ b/osf_tests/test_institutional_admin_contributors.py @@ -0,0 +1,116 @@ +import pytest + +from osf.models import Contributor +from osf_tests.factories import ( + AuthUserFactory, + ProjectFactory, + InstitutionFactory, + NodeRequestFactory +) +from django.db.utils import IntegrityError + +@pytest.mark.django_db +class TestContributorModel: + + @pytest.fixture() + def user(self): + return AuthUserFactory() + + @pytest.fixture() + def user_with_institutional_request(self, project): + user = AuthUserFactory() + NodeRequestFactory( + target=project, + creator=user, + is_institutional_request=True, + ) + return user + + @pytest.fixture() + def user_with_non_institutional_request(self, project): + user = AuthUserFactory() + NodeRequestFactory( + target=project, + creator=user, + is_institutional_request=False, + ) + return user + + @pytest.fixture() + def project(self): + return ProjectFactory() + + @pytest.fixture() + def institution(self): + return InstitutionFactory() + + @pytest.fixture() + def institutional_admin(self, institution): + admin_user = AuthUserFactory() + institution.get_group('institutional_admins').user_set.add(admin_user) + return admin_user + + @pytest.fixture() + def curator(self, institutional_admin, project): + return Contributor( + user=institutional_admin, + node=project, + visible=False, + is_curator=True + ) + + def test_contributor_with_visible_and_pending_request_raises_error(self, user_with_institutional_request, project, institution): + user_with_institutional_request.save() + user_with_institutional_request.visible = True + user_with_institutional_request.refresh_from_db() + assert user_with_institutional_request.visible + + try: + project.add_contributor(user_with_institutional_request, make_curator=True) + except IntegrityError as e: + assert e.args == ('Curators cannot be made bibliographic contributors',) + + def test_contributor_with_visible_and_valid_request(self, user_with_non_institutional_request, project, institution): + user_with_non_institutional_request.save() + user_with_non_institutional_request.visible = True + user_with_non_institutional_request.save() + + user_with_non_institutional_request.refresh_from_db() + assert user_with_non_institutional_request.visible + + def test_contributor_with_visible_and_institutional_admin_raises_error(self, curator, project, institution): + curator.save() + curator.visible = True + with pytest.raises(IntegrityError, match='Curators cannot be made bibliographic contributors'): + curator.save() + + assert curator.visible + curator.refresh_from_db() + assert not curator.visible + + # save completes when valid + curator.visible = False + curator.save() + + def test_regular_visible_contributor_is_saved(self, user, project): + contributor = Contributor( + user=user, + node=project, + visible=True, + is_curator=False + ) + contributor.save() + saved_contributor = Contributor.objects.get(pk=contributor.pk) + assert saved_contributor.user == user + assert saved_contributor.node == project + assert saved_contributor.visible is True + assert saved_contributor.is_curator is False + + def test_invisible_curator_is_saved(self, institutional_admin, curator, project): + curator.save() + saved_curator = Contributor.objects.get(pk=curator.pk) + assert curator == saved_curator + assert saved_curator.user == institutional_admin + assert saved_curator.node == project + assert saved_curator.visible is False + assert saved_curator.is_curator is True diff --git a/tests/framework_tests/test_email.py b/tests/framework_tests/test_email.py index 1dde2ada9ad..c19596b7ed8 100644 --- a/tests/framework_tests/test_email.py +++ b/tests/framework_tests/test_email.py @@ -6,7 +6,7 @@ import sendgrid from sendgrid import SendGridAPIClient -from sendgrid.helpers.mail import Category +from sendgrid.helpers.mail import Mail, Email, To, Category from framework.email.tasks import send_email, _send_with_sendgrid from website import settings @@ -55,17 +55,26 @@ def test_send_with_sendgrid_success(self, mock_mail: MagicMock): categories=(category1, category2) ) assert ret - mock_mail.assert_called_once_with( - from_email=from_addr, - to_emails=to_addr, - subject=subject, - html_content=message, - ) - assert len(mock_mail.return_value.category) == 2 - assert mock_mail.return_value.category[0].get() == category1 - assert mock_mail.return_value.category[1].get() == category2 - mock_client.send.assert_called_once_with(mock_mail.return_value) + # Check Mail object arguments + mock_mail.assert_called_once() + kwargs = mock_mail.call_args.kwargs + assert kwargs['from_email'].email == from_addr + assert kwargs['subject'] == subject + assert kwargs['html_content'] == message + + mock_mail.return_value.add_personalization.assert_called_once() + + # Capture the categories added via add_category + mock_mail.return_value.add_category.assert_called_once() + added_categories = mock_mail.return_value.add_category.call_args.args[0] + assert len(added_categories) == 2 + assert isinstance(added_categories[0], Category) + assert isinstance(added_categories[1], Category) + assert added_categories[0].get() == category1 + assert added_categories[1].get() == category2 + + mock_client.send.assert_called_once_with(mock_mail.return_value) @mock.patch(f'{_send_with_sendgrid.__module__}.sentry.log_message', autospec=True) @mock.patch(f'{_send_with_sendgrid.__module__}.Mail', autospec=True) @@ -84,12 +93,14 @@ def test_send_with_sendgrid_failure_returns_false(self, mock_mail, sentry_mock): ) assert not ret sentry_mock.assert_called_once() - mock_mail.assert_called_once_with( - from_email=from_addr, - to_emails=to_addr, - subject=subject, - html_content=message, - ) + + # Check Mail object arguments + mock_mail.assert_called_once() + kwargs = mock_mail.call_args.kwargs + assert kwargs['from_email'].email == from_addr + assert kwargs['subject'] == subject + assert kwargs['html_content'] == message + mock_client.send.assert_called_once_with(mock_mail.return_value) diff --git a/website/mails/mails.py b/website/mails/mails.py index afca9e78f03..1a9d675c06d 100644 --- a/website/mails/mails.py +++ b/website/mails/mails.py @@ -76,17 +76,29 @@ def render_message(tpl_name, **context): def send_mail( - to_addr, mail, from_addr=None, mailer=None, celery=True, - username=None, password=None, callback=None, attachment_name=None, - attachment_content=None, **context): - """Send an email from the OSF. - Example: :: - + to_addr, + mail, + from_addr=None, + bcc_addr=None, + reply_to=None, + mailer=None, + celery=True, + username=None, + password=None, + callback=None, + attachment_name=None, + attachment_content=None, + **context): + """ + Send an email from the OSF. + Example: from website import mails mails.send_email('foo@bar.com', mails.TEST, name="Foo") :param str to_addr: The recipient's email address + :param str bcc_addr: The BCC senders's email address (or list of addresses) + :param str reply_to: The sender's email address will appear in the reply-to header :param Mail mail: The mail object :param str mimetype: Either 'plain' or 'html' :param function callback: celery task to execute after send_mail completes @@ -119,6 +131,8 @@ def send_mail( categories=mail.categories, attachment_name=attachment_name, attachment_content=attachment_content, + bcc_addr=bcc_addr, + reply_to=reply_to, ) logger.debug('Preparing to send...') @@ -595,3 +609,13 @@ def get_english_article(word): 'addons_boa_job_failure', subject='Your Boa job has failed' ) + +NODE_REQUEST_INSTITUTIONAL_ACCESS_REQUEST = Mail( + 'node_request_institutional_access_request', + subject='Institutional Access Project Request' +) + +USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST = Mail( + 'user_message_institutional_access_request', + subject='Message from Institutional Admin' +) diff --git a/website/profile/utils.py b/website/profile/utils.py index 3ccff69a164..5b1e22e8916 100644 --- a/website/profile/utils.py +++ b/website/profile/utils.py @@ -51,7 +51,8 @@ def serialize_user(user, node=None, admin=False, full=False, is_profile=False, i is_contributor_obj = isinstance(contrib, Contributor) flags = { 'visible': contrib.visible if is_contributor_obj else node.contributor_set.filter(user=user, visible=True).exists(), - 'permission': contrib.permission if is_contributor_obj else None + 'permission': contrib.permission if is_contributor_obj else None, + 'is_curator': contrib.is_curator, } ret.update(flags) if user.is_registered: @@ -195,10 +196,15 @@ def serialize_access_requests(node): return [ { 'user': serialize_user(access_request.creator), + 'is_institutional_request': access_request.is_institutional_request, 'comment': access_request.comment, + 'requested_permissions': access_request.requested_permissions, 'id': access_request._id } for access_request in node.requests.filter( - request_type=workflows.RequestTypes.ACCESS.value, + request_type__in=[ + workflows.NodeRequestTypes.ACCESS.value, + workflows.NodeRequestTypes.INSTITUTIONAL_REQUEST.value + ], machine_state=workflows.DefaultStates.PENDING.value ).select_related('creator') ] diff --git a/website/project/views/contributor.py b/website/project/views/contributor.py index 56b6802d2a6..0b36b494c34 100644 --- a/website/project/views/contributor.py +++ b/website/project/views/contributor.py @@ -3,6 +3,7 @@ from flask import request from django.core.exceptions import ValidationError from django.utils import timezone +from django.db import IntegrityError from framework import forms, status from framework.auth import cas @@ -287,6 +288,13 @@ def project_manage_contributors(auth, node, **kwargs): node.manage_contributors(contributors, auth=auth, save=True) except (ValueError, NodeStateError) as error: raise HTTPError(http_status.HTTP_400_BAD_REQUEST, data={'message_long': error.args[0]}) + except IntegrityError as error: + status.push_status_message( + 'You can not make an institutional curator a bibliographic contributor.', + kind='error', + trust=False + ) + raise HTTPError(http_status.HTTP_409_CONFLICT, data={'message_long': error.args[0]}) # If user has removed herself from project, alert; redirect to # node summary if node is public, else to user's dashboard page diff --git a/website/static/js/accessRequestManager.js b/website/static/js/accessRequestManager.js index 04616df41a8..862f86ca597 100644 --- a/website/static/js/accessRequestManager.js +++ b/website/static/js/accessRequestManager.js @@ -22,8 +22,9 @@ var AccessRequestModel = function(accessRequest, pageOwner, isRegistration, isPa self.permissionText = ko.observable(self.options.permissionMap[self.permission()]); - self.visible = ko.observable(true); - + self.is_curator = ko.observable(accessRequest.user.is_curator || false); + self.is_institutional_request = ko.observable(accessRequest.is_institutional_request || false); + self.visible = ko.observable(!accessRequest.is_institutional_request); self.pageOwner = pageOwner; self.expanded = ko.observable(false); diff --git a/website/static/js/contribManager.js b/website/static/js/contribManager.js index a93174c07cd..b0537a1d9ef 100644 --- a/website/static/js/contribManager.js +++ b/website/static/js/contribManager.js @@ -59,6 +59,8 @@ var ContributorModel = function(contributor, currentUserCanEdit, pageOwner, isRe self.permission = ko.observable(contributor.permission); + self.is_curator = ko.observable(contributor.is_curator || false); + self.permissionText = ko.observable(self.options.permissionMap[self.permission()]); self.visible = ko.observable(contributor.visible); diff --git a/website/static/js/project.js b/website/static/js/project.js index b32cc77592c..91453024c1b 100644 --- a/website/static/js/project.js +++ b/website/static/js/project.js @@ -217,12 +217,18 @@ $(document).ready(function() { 'in the Contributors list and in project citations. Non-bibliographic contributors ' + 'can read and modify the project as normal.'; - $('.visibility-info').attr( + $('.visibility-info-contrib').attr( 'data-content', bibliographicContribInfoHtml ).popover({ trigger: 'hover' }); + $('.visibility-info-curator').attr( + 'data-content', 'An administrator designated by your affiliated institution to curate your project.' + ).popover({ + trigger: 'hover' + }); + //////////////////// // Event Handlers // //////////////////// diff --git a/website/templates/emails/node_request_institutional_access_request.html.mako b/website/templates/emails/node_request_institutional_access_request.html.mako new file mode 100644 index 00000000000..d49b2811ea1 --- /dev/null +++ b/website/templates/emails/node_request_institutional_access_request.html.mako @@ -0,0 +1,35 @@ +<%inherit file="notify_base.mako" /> + +<%def name="content()"> + + + <%!from website import settings%> + Hello ${recipient.fullname}, +

+ ${sender.fullname} has requested access to ${node.title}. +

+ % if comment: +

+ ${comment} +

+ % endif +

+ To review the request, click here to allow or deny access and configure permissions. +

+

+ This request is being sent to you because your project has the “Request Access” feature enabled. + This allows potential collaborators to request to be added to your project or to disable this feature, click + here +

+ +

+ Want more information? Visit ${settings.DOMAIN} to learn about OSF, or + https://cos.io/ for information about its supporting organization, the Center + for Open Science. +

+

+ Questions? Email ${settings.OSF_CONTACT_EMAIL} +

+ + + diff --git a/website/templates/emails/user_message_institutional_access_request.html.mako b/website/templates/emails/user_message_institutional_access_request.html.mako new file mode 100644 index 00000000000..1e314f91e4e --- /dev/null +++ b/website/templates/emails/user_message_institutional_access_request.html.mako @@ -0,0 +1,26 @@ +<%inherit file="notify_base.mako" /> + +<%def name="content()"> + + + <%!from website import settings%> + Hello ${recipient.fullname}, +

+ This message is coming from an Institutional administrator within your Institution. +

+ % if message_text: +

+ ${message_text} +

+ % endif +

+ Want more information? Visit ${settings.DOMAIN} to learn about OSF, or + https://cos.io/ for information about its supporting organization, the Center + for Open Science. +

+

+ Questions? Email ${settings.OSF_CONTACT_EMAIL} +

+ + + diff --git a/website/templates/project/contributors.mako b/website/templates/project/contributors.mako index 65954d3aced..c3284ebb99d 100644 --- a/website/templates/project/contributors.mako +++ b/website/templates/project/contributors.mako @@ -188,7 +188,7 @@ Bibliographic Contributor - + + Curator + + @@ -242,6 +252,17 @@ data-html="true" > + + Curator + + + @@ -307,12 +328,30 @@ -
-
+
+
+
+ +
+
+
+ type='checkbox' + aria-label='Curator Disabled Bibliographic User Checkbox' + style='background-color: lightgray;' + disabled + > +
+ + +
+ +
+
+
@@ -356,7 +395,7 @@
@@ -366,11 +405,26 @@
+
+
+ +
+ + +
+
+
+ +
+
+ +
+