[ENG-6682] Add CCing and Reply-to features to Institutional Access User Messaging

CHANGELOG

@@ -2,6 +2,16 @@
 
 We follow the CalVer (https://calver.org/) versioning scheme: YY.MINOR.MICRO.
 
+24.10.0 (2024-12-05)
+====================
+
+- Migrate Preprint Affilations
+- Add OOPSpam and Akismet metrics to spam report
+- Add PrivateSpamMetricsReport
+- Update PrivateSpamMetricsReporter to work with refactored MonthlyReporter
+- Fix duplicate reports when run for past years
+- Fix counted-usage clobbers
+
 24.09.0 (2024-11-14)
 ====================
 

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/metrics/test_counted_usage.py

@@ -99,8 +99,8 @@ def test_by_client_session_id(self, app, mock_save, user):
         assert resp.status_code == 201
         assert_saved_with(
             mock_save,
-            # doc_id: sha256(b'http://example.foo/|http://example.foo/blahblah/blee|5b7c8b0a740a5b23712258a9d1164d2af008df02a8e3d339f16ead1d19595b34|1981-01-01|3').hexdigest()
-            expected_doc_id='55fffffdc0d674d15a5e8763d14e4ae90f658fbfb6fbf94f88a5d24978f02e72',
+            # doc_id: sha256(b'http://example.foo/|http://example.foo/blahblah/blee|5b7c8b0a740a5b23712258a9d1164d2af008df02a8e3d339f16ead1d19595b34|1981-01-01|3|api,view').hexdigest()
+            expected_doc_id='3239044c7462dd318edd0522a0ed7d84b9c6502ef16cb40dfcae6c1f456d57a2',
             expected_attrs={
                 'platform_iri': 'http://example.foo/',
                 'item_guid': 'zyxwv',
@@ -132,8 +132,8 @@ def test_by_client_session_id_anon(self, app, mock_save):
         assert resp.status_code == 201
         assert_saved_with(
             mock_save,
-            # doc_id: sha256(b'http://example.foo/|http://example.foo/bliz/|5b7c8b0a740a5b23712258a9d1164d2af008df02a8e3d339f16ead1d19595b34|1981-01-01|3').hexdigest()
-            expected_doc_id='e559ffbc4bd3e3e69252d34c273f0e771ec89ee455ec9b60fbbadf3944e4af4e',
+            # doc_id: sha256(b'http://example.foo/|http://example.foo/bliz/|5b7c8b0a740a5b23712258a9d1164d2af008df02a8e3d339f16ead1d19595b34|1981-01-01|3|view,web').hexdigest()
+            expected_doc_id='d01759e963893f9dc9b2ccf016a5ef29135673779802b5578f31449543677e82',
             expected_attrs={
                 'platform_iri': 'http://example.foo/',
                 'item_guid': 'zyxwv',
@@ -166,8 +166,8 @@ def test_by_user_auth(self, app, mock_save, user):
         assert resp.status_code == 201
         assert_saved_with(
             mock_save,
-            # doc_id: sha256(b'http://example.foo/|http://osf.io/mst3k|ec768abb16c3411570af99b9d635c2c32d1ca31d1b25eec8ee73759e7242e74a|1981-01-01|3').hexdigest()
-            expected_doc_id='743494d8a55079b91e202da1dbdfce5aea72e310c57a34b36df2c2af5ed4d362',
+            # doc_id: sha256(b'http://example.foo/|http://osf.io/mst3k|ec768abb16c3411570af99b9d635c2c32d1ca31d1b25eec8ee73759e7242e74a|1981-01-01|3|view,web').hexdigest()
+            expected_doc_id='7b8bc27c6d90fb45aa5bbd02deceba9f7384ed61b9a6e7253317c262020b94c2',
             expected_attrs={
                 'platform_iri': 'http://example.foo/',
                 'item_guid': 'yxwvu',
@@ -196,8 +196,8 @@ def test_by_useragent_header(self, app, mock_save):
         assert resp.status_code == 201
         assert_saved_with(
             mock_save,
-            # doc_id: sha256(b'http://example.foo/|yxwvu|97098dd3f7cd26053c0d0264d1c84eaeea8e08d2c55ca34017ffbe53c749ba5a|1981-01-01|3').hexdigest()
-            expected_doc_id='a50ac1b2dc1c918cdea7be50b005117fdb6ee00ea069ca3aa4aaf03c0f905fa0',
+            # doc_id: sha256(b'http://example.foo/|yxwvu|97098dd3f7cd26053c0d0264d1c84eaeea8e08d2c55ca34017ffbe53c749ba5a|1981-01-01|3|api,view').hexdigest()
+            expected_doc_id='d669528b30f443ffe506e183537af9624ef290090e90a200ecce7b7ca19c77f7',
             expected_attrs={
                 'platform_iri': 'http://example.foo/',
                 'item_guid': 'yxwvu',

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):