From 4c0230d5373c0192e71bee833c37312de6679a4a Mon Sep 17 00:00:00 2001 From: lebaudantoine Date: Fri, 20 Dec 2024 15:40:34 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20post=20email=20to=20market?= =?UTF-8?q?ing=20tools=20while=20signing=20up=20new=20users?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Submitting new users to the marketing service is currently handled during signup and is performed only once. This is a pragmatic first implementation, yet imperfect. In the future, this should be improved by delegating the call to a Celery worker or an async task. --- src/backend/core/authentication/backends.py | 31 +++- .../tests/authentication/test_backends.py | 141 +++++++++++++++++- src/backend/meet/settings.py | 9 ++ 3 files changed, 179 insertions(+), 2 deletions(-) diff --git a/src/backend/core/authentication/backends.py b/src/backend/core/authentication/backends.py index 202a5293..46fa366f 100644 --- a/src/backend/core/authentication/backends.py +++ b/src/backend/core/authentication/backends.py @@ -1,7 +1,7 @@ """Authentication Backends for the Meet core app.""" from django.conf import settings -from django.core.exceptions import SuspiciousOperation +from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation from django.utils.translation import gettext_lazy as _ import requests @@ -10,6 +10,11 @@ from mozilla_django_oidc.auth import ( ) from core.models import User +from core.services.marketing_service import ( + ContactCreationError, + ContactData, + get_marketing_service, +) class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend): @@ -86,6 +91,10 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend): password="!", # noqa: S106 **claims, ) + + if settings.SIGNUP_NEW_USER_TO_MARKETING_EMAIL: + self.signup_to_marketing_email(email) + elif not user: return None @@ -96,6 +105,26 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend): return user + @staticmethod + def signup_to_marketing_email(email): + """Pragmatic approach to newsletter signup during authentication flow. + + Details: + 1. Uses a very short timeout (1s) to prevent blocking the auth process + 2. Silently fails if the marketing service is down/slow to prioritize user experience + 3. Trade-off: May miss some signups but ensures auth flow remains fast + + Note: For a more robust solution, consider using Async task processing (Celery/Django-Q) + """ + try: + marketing_service = get_marketing_service() + contact_data = ContactData( + email=email, attributes={"VISIO_SOURCE": ["SIGNIN"]} + ) + marketing_service.create_contact(contact_data, timeout=1) + except (ContactCreationError, ImproperlyConfigured, ImportError): + pass + def get_existing_user(self, sub, email): """Fetch existing user by sub or email.""" try: diff --git a/src/backend/core/tests/authentication/test_backends.py b/src/backend/core/tests/authentication/test_backends.py index 280caa37..750b46c0 100644 --- a/src/backend/core/tests/authentication/test_backends.py +++ b/src/backend/core/tests/authentication/test_backends.py @@ -1,12 +1,15 @@ """Unit tests for the Authentication Backends.""" -from django.core.exceptions import SuspiciousOperation +from unittest import mock + +from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation import pytest from core import models from core.authentication.backends import OIDCAuthenticationBackend from core.factories import UserFactory +from core.services import marketing_service pytestmark = pytest.mark.django_db @@ -412,3 +415,139 @@ def test_update_user_when_no_update_needed(django_assert_num_queries, claims): user.refresh_from_db() assert user.email == "john.doe@example.com" + + +@mock.patch.object(OIDCAuthenticationBackend, "signup_to_marketing_email") +def test_marketing_signup_new_user_enabled(mock_signup, monkeypatch, settings): + """Test marketing signup for new user with settings enabled.""" + settings.SIGNUP_NEW_USER_TO_MARKETING_EMAIL = True + + klass = OIDCAuthenticationBackend() + email = "test@example.com" + + def get_userinfo_mocked(*args): + return {"sub": "123", "email": email} + + monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked) + + user = klass.get_or_create_user("test-token", None, None) + + assert user.email == email + mock_signup.assert_called_once_with(email) + + +@mock.patch.object(OIDCAuthenticationBackend, "signup_to_marketing_email") +def test_marketing_signup_new_user_disabled(mock_signup, monkeypatch, settings): + """Test no marketing signup for new user with settings disabled.""" + settings.SIGNUP_NEW_USER_TO_MARKETING_EMAIL = False + + klass = OIDCAuthenticationBackend() + email = "test@example.com" + + def get_userinfo_mocked(*args): + return {"sub": "123", "email": email} + + monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked) + + user = klass.get_or_create_user("test-token", None, None) + + assert user.email == email + mock_signup.assert_not_called() + + +@mock.patch.object(OIDCAuthenticationBackend, "signup_to_marketing_email") +def test_marketing_signup_new_user_default_disabled(mock_signup, monkeypatch): + """Test no marketing signup for new user with settings by default disabled.""" + + klass = OIDCAuthenticationBackend() + email = "test@example.com" + + def get_userinfo_mocked(*args): + return {"sub": "123", "email": email} + + monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked) + + user = klass.get_or_create_user("test-token", None, None) + + assert user.email == email + mock_signup.assert_not_called() + + +@pytest.mark.parametrize( + "is_signup_enabled", + [True, False], +) +@mock.patch.object(OIDCAuthenticationBackend, "signup_to_marketing_email") +def test_marketing_signup_existing_user( + mock_signup, monkeypatch, settings, is_signup_enabled +): + """Test no marketing signup for existing user regardless of settings.""" + + settings.SIGNUP_NEW_USER_TO_MARKETING_EMAIL = is_signup_enabled + + klass = OIDCAuthenticationBackend() + db_user = UserFactory(email="test@example.com") + + def get_userinfo_mocked(*args): + return {"sub": db_user.sub, "email": db_user.email} + + monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked) + + user = klass.get_or_create_user("test-token", None, None) + assert user == db_user + mock_signup.assert_not_called() + + +@mock.patch("core.authentication.backends.get_marketing_service") +def test_signup_to_marketing_email_success(mock_marketing): + """Test successful marketing signup.""" + + email = "test@example.com" + + # Call the method + OIDCAuthenticationBackend.signup_to_marketing_email(email) + + # Verify service interaction + mock_service = mock_marketing.return_value + mock_service.create_contact.assert_called_once() + + +@pytest.mark.parametrize( + "error", + [ + ImportError, + ImproperlyConfigured, + ], +) +@mock.patch("core.authentication.backends.get_marketing_service") +def test_marketing_signup_handles_service_initialization_errors( + mock_marketing, error, settings +): + """Tests errors that occur when trying to get/initialize the marketing service.""" + settings.SIGNUP_NEW_USER_TO_MARKETING_EMAIL = True + + mock_marketing.side_effect = error + + # Should not raise any exception + OIDCAuthenticationBackend.signup_to_marketing_email("test@example.com") + + +@pytest.mark.parametrize( + "error", + [ + marketing_service.ContactCreationError, + ImproperlyConfigured, + ImportError, + ], +) +@mock.patch("core.authentication.backends.get_marketing_service") +def test_marketing_signup_handles_contact_creation_errors( + mock_marketing, error, settings +): + """Tests errors that occur during the contact creation process.""" + + settings.SIGNUP_NEW_USER_TO_MARKETING_EMAIL = True + mock_marketing.return_value.create_contact.side_effect = error + + # Should not raise any exception + OIDCAuthenticationBackend.signup_to_marketing_email("test@example.com") diff --git a/src/backend/meet/settings.py b/src/backend/meet/settings.py index 6dd52171..9b30d2de 100755 --- a/src/backend/meet/settings.py +++ b/src/backend/meet/settings.py @@ -458,6 +458,15 @@ class Base(Configuration): ) # Marketing and communication settings + SIGNUP_NEW_USER_TO_MARKETING_EMAIL = values.BooleanValue( + False, + environ_name="SIGNUP_NEW_USERS_TO_NEWSLETTER", + environ_prefix=None, + help_text=( + "When enabled, new users are automatically added to mailing list " + "for product updates, marketing communications, and customized emails. " + ), + ) MARKETING_SERVICE_CLASS = values.Value( "core.services.marketing_service.BrevoMarketingService", environ_name="MARKETING_SERVICE_CLASS",