Skip to content

Commit 4954365

Browse files
authored
fix(notifications): deliver @mention notifications and match full usernames (#15196)
* fix(notifications): deliver @mention notifications and match full usernames * add regression tests for @mention delivery and username matching * ruff lint and format
1 parent a286dcb commit 4954365

2 files changed

Lines changed: 84 additions & 4 deletions

File tree

dojo/notifications/helper.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -889,9 +889,14 @@ def _process_notifications(
889889

890890

891891
def process_tag_notifications(request, note, parent_url, parent_title):
892-
regex = re.compile(r"(?:\A|\s)@(\w+)\b")
892+
# Django usernames may contain "@ . + - _" in addition to word characters (see
893+
# Django's username validators), so capture that whole set instead of only \w.
894+
# The leading (?:\A|\s)@ anchor keeps email addresses written in prose from
895+
# triggering, and trailing dots are stripped so a mention that ends a sentence
896+
# ("thanks @alice.") still resolves to the username.
897+
regex = re.compile(r"(?:\A|\s)@([\w.@+-]+)")
893898

894-
usernames_to_check = set(un.lower() for un in regex.findall(note.entry)) # noqa: C401
899+
usernames_to_check = {un.lower().rstrip(".") for un in regex.findall(note.entry)}
895900

896901
users_to_notify = [
897902
Dojo_User.objects.filter(username=username).get()
@@ -910,7 +915,7 @@ def process_tag_notifications(request, note, parent_url, parent_title):
910915
title=f"{request.user} jotted a note",
911916
url=parent_url,
912917
icon="commenting",
913-
recipients=users_to_notify,
918+
recipients=[user.username for user in users_to_notify],
914919
requested_by=crum.get_current_user())
915920

916921

unittests/test_notifications.py

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import pghistory
77
from auditlog.context import set_actor
88
from crum import impersonate
9-
from django.test import override_settings
9+
from django.test import RequestFactory, override_settings
1010
from django.urls import reverse
1111
from django.utils import timezone
1212
from rest_framework.authtoken.models import Token
@@ -23,6 +23,7 @@
2323
Engagement,
2424
Finding,
2525
Finding_Group,
26+
Notes,
2627
Notification_Webhooks,
2728
Notifications,
2829
Product,
@@ -38,6 +39,7 @@
3839
WebhookNotificationManger,
3940
async_create_notification,
4041
create_notification,
42+
process_tag_notifications,
4143
webhook_status_cleanup,
4244
)
4345
from dojo.url.models import URL
@@ -1301,3 +1303,76 @@ def test_ping_with_owner_assigned(self, mock):
13011303
},
13021304
},
13031305
)
1306+
1307+
1308+
@versioned_fixtures
1309+
class TestProcessTagNotifications(DojoTestCase):
1310+
1311+
"""
1312+
Regression tests for dojo.notifications.helper.process_tag_notifications.
1313+
1314+
@mention notifications were silently never delivered: the helper passed
1315+
Dojo_User objects as ``recipients`` where usernames are expected, so
1316+
``_process_recipients`` (which filters ``user__username__in``) matched
1317+
nothing. Usernames containing ``.``, ``-``, ``@`` or ``+`` were also
1318+
truncated by the old word-character-only mention regex.
1319+
"""
1320+
1321+
fixtures = ["dojo_testdata.json"]
1322+
1323+
def _enable_mention_alerts(self, user):
1324+
# dojo_testdata.json ships only the system (user=None) Notifications row;
1325+
# the recipient lookup needs a per-user row with product=None.
1326+
notifications, _ = Notifications.objects.get_or_create(user=user, product=None)
1327+
notifications.user_mentioned = ["alert"]
1328+
notifications.save()
1329+
1330+
def _mention(self, author, entry):
1331+
note = Notes.objects.create(entry=entry, author=author)
1332+
request = RequestFactory().get("/")
1333+
request.user = author
1334+
with impersonate(author):
1335+
process_tag_notifications(
1336+
request,
1337+
note,
1338+
parent_url="/finding/1",
1339+
parent_title="Finding: Example",
1340+
)
1341+
1342+
def test_mentioned_user_receives_alert(self):
1343+
author = Dojo_User.objects.get(username="admin")
1344+
mentioned = Dojo_User.objects.create(username="mentionee", is_active=True)
1345+
self._enable_mention_alerts(mentioned)
1346+
1347+
self._mention(author, f"@{mentioned.username} please take a look")
1348+
1349+
self.assertEqual(
1350+
Alerts.objects.filter(user_id=mentioned, source="User Mentioned").count(),
1351+
1,
1352+
"an @mention should create exactly one in-app alert for the mentioned user",
1353+
)
1354+
1355+
def test_username_with_dot_is_matched(self):
1356+
author = Dojo_User.objects.get(username="admin")
1357+
mentioned = Dojo_User.objects.create(username="jane.doe", is_active=True)
1358+
self._enable_mention_alerts(mentioned)
1359+
1360+
self._mention(author, "thanks @jane.doe")
1361+
1362+
self.assertEqual(
1363+
Alerts.objects.filter(user_id=mentioned).count(),
1364+
1,
1365+
"a username containing '.' must be matched in full, not truncated to 'jane'",
1366+
)
1367+
1368+
def test_email_address_in_prose_is_not_a_mention(self):
1369+
author = Dojo_User.objects.get(username="admin")
1370+
mentioned = Dojo_User.objects.create(username="ops", is_active=True)
1371+
self._enable_mention_alerts(mentioned)
1372+
1373+
self._mention(author, "email me at someone@ops")
1374+
1375+
self.assertFalse(
1376+
Alerts.objects.filter(user_id=mentioned).exists(),
1377+
"an email-like token in prose (no whitespace before '@') must not notify",
1378+
)

0 commit comments

Comments
 (0)