Merge branch 'feature/institutional_access' of https://github.com/Cen…

…terForOpenScience/osf.io into institutional-access-node-request-improvements

* 'feature/institutional_access' of https://github.com/CenterForOpenScience/osf.io:
  change to bcc the sender instead of CC-ing them
  revert typo
  add code to allow cc-ing fellow institutional admins and put their own address as a reply_to

# Conflicts:
#	osf/migrations/0025_noderequest_requested_permissions_and_more.py

api/users/serializers.py

@@ -696,6 +696,15 @@ class UserMessageSerializer(JSONAPISerializer):
         related_view_kwargs={'institution_id': '<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(
@@ -756,7 +765,7 @@ def create(self, validated_data: Dict[str, Any]) -> UserMessage:
 
         if not recipient.is_affiliated_with_institution(institution):
             raise Conflict(
-                {'user': 'Can not send to recipient that is not affiliated with the provided institution.'},
+                {'user': 'Cannot send to a recipient that is not affiliated with the provided institution.'},
             )
 
         return UserMessage.objects.create(
@@ -765,4 +774,6 @@ def create(self, validated_data: Dict[str, Any]) -> UserMessage:
             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'],
         )

api_tests/users/views/test_user_message_institutional_access.py

@@ -6,6 +6,8 @@
     AuthUserFactory,
     InstitutionFactory
 )
+from website.mails import USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST
+
 
 @pytest.mark.django_db
 class TestUserMessageInstitutionalAccess:
@@ -151,5 +153,95 @@ def test_admin_cannot_message_user_outside_institution(
         """
         res = app.post_json_api(url_without_affiliation, payload, auth=institutional_admin.auth, expect_errors=True)
         assert res.status_code == 409
-        assert 'Can not send to recipient that is not affiliated with the provided institution.'\
-               in res.json['errors'][0]['detail']['user']
+        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,
+        )

framework/email/tasks.py

@@ -20,8 +20,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 +59,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 +72,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 +135,59 @@ def _send_with_sendgrid(
     categories=None,
     attachment_name: str = None,
     attachment_content=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=from_addr,
+        html_content=message,
+        to_emails=to_addr,
+        subject=subject
+    )
+
+    if reply_to:
+        mail.reply_to = [{'email': reply_to}]
+
+    if bcc_addr:
+        mail.add_bcc([{'email': email} for email in bcc_addr])
+
+    if categories:
+        mail.category = [Category(x) for x in categories]
+
+    if attachment_name and attachment_content:
+        attachment = Attachment(
+            file_content=_content_to_bytes(
+                FileContent(
+                    b64encode(attachment_content).decode()
+                )
+            ),
+            file_name=attachment_name
+        )
+        mail.add_attachment(attachment)
 
+    response = client.send(mail)
+    if response.status_code not in (200, 201):
+        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):

osf/migrations/0025_noderequest_requested_permissions_and_more.py

@@ -33,6 +33,8 @@ class Migration(migrations.Migration):
                 ('_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)),

osf/models/user_message.py

@@ -2,6 +2,7 @@
 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
 
@@ -70,14 +71,24 @@ class UserMessage(BaseModel, ObjectIDMixin):
         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,
-            mail=MessageTypes.get_template(MessageTypes.INSTITUTIONAL_REQUEST),
+            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,