From 987fabc99ce2ad10a292bc2f016496e9c5c34d4c Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Mon, 9 Dec 2024 15:55:21 +0000 Subject: [PATCH 1/4] Catch the integrity error --- .../lead_tracking/hubspot/services.py | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/api/integrations/lead_tracking/hubspot/services.py b/api/integrations/lead_tracking/hubspot/services.py index 324af4f8bfcd..adc1d0fa1147 100644 --- a/api/integrations/lead_tracking/hubspot/services.py +++ b/api/integrations/lead_tracking/hubspot/services.py @@ -1,5 +1,6 @@ import logging +from django.db import IntegrityError from rest_framework.request import Request from integrations.lead_tracking.hubspot.constants import HUBSPOT_COOKIE_NAME @@ -17,13 +18,18 @@ def register_hubspot_tracker(request: Request) -> None: ) return - logger.info( - f"Creating HubspotTracker instance for user {request.user.email} with cookie {hubspot_cookie}" - ) - - HubspotTracker.objects.update_or_create( - user=request.user, - defaults={ - "hubspot_cookie": hubspot_cookie, - }, - ) + try: + HubspotTracker.objects.update_or_create( + user=request.user, + defaults={ + "hubspot_cookie": hubspot_cookie, + }, + ) + logger.info( + f"Created HubspotTracker instance for user {request.user.email} with cookie {hubspot_cookie}" + ) + except IntegrityError: + logger.info( + f"HubspotTracker could not be created for user {request.user.email}" + f" due to cookie conflict with cookie {hubspot_cookie}" + ) From ced59023fde9b00d4b184da5a5f74b01fec230f7 Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Mon, 9 Dec 2024 15:55:56 +0000 Subject: [PATCH 2/4] Test that the hubspot cookie collision doesn't raise an error --- .../test_unit_organisations_views.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/api/tests/unit/organisations/test_unit_organisations_views.py b/api/tests/unit/organisations/test_unit_organisations_views.py index 2f0fd2cdd123..ea78fc01a40a 100644 --- a/api/tests/unit/organisations/test_unit_organisations_views.py +++ b/api/tests/unit/organisations/test_unit_organisations_views.py @@ -135,6 +135,40 @@ def test_non_superuser_can_create_new_organisation_by_default( assert HubspotTracker.objects.filter(user=staff_user).exists() +def test_colliding_hubspot_cookies_are_ignored( + staff_client: APIClient, + staff_user: FFAdminUser, + admin_user: FFAdminUser, +) -> None: + # Given + org_name = "Test create org" + webhook_notification_email = "test@email.com" + url = reverse("api-v1:organisations:organisation-list") + colliding_cookie = "test_cookie_tracker" + HubspotTracker.objects.create( + user=admin_user, + hubspot_cookie=colliding_cookie, + ) + data = { + "name": org_name, + "webhook_notification_email": webhook_notification_email, + HUBSPOT_COOKIE_NAME: colliding_cookie, + } + + assert not HubspotTracker.objects.filter(user=staff_user).exists() + + # When + response = staff_client.post(url, data=data) + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert ( + Organisation.objects.get(name=org_name).webhook_notification_email + == webhook_notification_email + ) + assert not HubspotTracker.objects.filter(user=staff_user).exists() + + @override_settings(RESTRICT_ORG_CREATE_TO_SUPERUSERS=True) def test_create_new_orgnisation_returns_403_with_non_superuser( staff_client: APIClient, From 02a0246c18764fb7d3715d519b07b0430683294d Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Mon, 9 Dec 2024 19:41:01 +0000 Subject: [PATCH 3/4] Switch to inclusion check --- .../lead_tracking/hubspot/services.py | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/api/integrations/lead_tracking/hubspot/services.py b/api/integrations/lead_tracking/hubspot/services.py index adc1d0fa1147..4821dbad5489 100644 --- a/api/integrations/lead_tracking/hubspot/services.py +++ b/api/integrations/lead_tracking/hubspot/services.py @@ -1,6 +1,5 @@ import logging -from django.db import IntegrityError from rest_framework.request import Request from integrations.lead_tracking.hubspot.constants import HUBSPOT_COOKIE_NAME @@ -18,18 +17,23 @@ def register_hubspot_tracker(request: Request) -> None: ) return - try: - HubspotTracker.objects.update_or_create( - user=request.user, - defaults={ - "hubspot_cookie": hubspot_cookie, - }, - ) - logger.info( - f"Created HubspotTracker instance for user {request.user.email} with cookie {hubspot_cookie}" - ) - except IntegrityError: + if ( + HubspotTracker.objects.filter(hubspot_cookie=hubspot_cookie) + .exclude(user=request.user) + .exists() + ): logger.info( f"HubspotTracker could not be created for user {request.user.email}" f" due to cookie conflict with cookie {hubspot_cookie}" ) + return + + HubspotTracker.objects.update_or_create( + user=request.user, + defaults={ + "hubspot_cookie": hubspot_cookie, + }, + ) + logger.info( + f"Created HubspotTracker instance for user {request.user.email} with cookie {hubspot_cookie}" + ) From bcd903746cab9d1023f12e068de1f0e1d0135159 Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Tue, 10 Dec 2024 14:00:19 +0000 Subject: [PATCH 4/4] Switch to user ids from emails --- api/integrations/lead_tracking/hubspot/services.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/api/integrations/lead_tracking/hubspot/services.py b/api/integrations/lead_tracking/hubspot/services.py index 4821dbad5489..7926b8d59b21 100644 --- a/api/integrations/lead_tracking/hubspot/services.py +++ b/api/integrations/lead_tracking/hubspot/services.py @@ -12,9 +12,7 @@ def register_hubspot_tracker(request: Request) -> None: hubspot_cookie = request.data.get(HUBSPOT_COOKIE_NAME) if not hubspot_cookie: - logger.info( - f"Request did not included Hubspot data for user {request.user.email}" - ) + logger.info(f"Request did not included Hubspot data for user {request.user.id}") return if ( @@ -23,7 +21,7 @@ def register_hubspot_tracker(request: Request) -> None: .exists() ): logger.info( - f"HubspotTracker could not be created for user {request.user.email}" + f"HubspotTracker could not be created for user {request.user.id}" f" due to cookie conflict with cookie {hubspot_cookie}" ) return @@ -35,5 +33,5 @@ def register_hubspot_tracker(request: Request) -> None: }, ) logger.info( - f"Created HubspotTracker instance for user {request.user.email} with cookie {hubspot_cookie}" + f"Created HubspotTracker instance for user {request.user.id} with cookie {hubspot_cookie}" )