diff --git a/engine/apps/phone_notifications/exceptions.py b/engine/apps/phone_notifications/exceptions.py index a605ac81..c7c38f12 100644 --- a/engine/apps/phone_notifications/exceptions.py +++ b/engine/apps/phone_notifications/exceptions.py @@ -48,3 +48,7 @@ class CallsLimitExceeded(Exception): class SMSLimitExceeded(Exception): pass + + +class PhoneNumberBanned(Exception): + pass diff --git a/engine/apps/phone_notifications/migrations/0002_bannedphonenumber.py b/engine/apps/phone_notifications/migrations/0002_bannedphonenumber.py new file mode 100644 index 00000000..025dbaf2 --- /dev/null +++ b/engine/apps/phone_notifications/migrations/0002_bannedphonenumber.py @@ -0,0 +1,22 @@ +# Generated by Django 4.2.11 on 2024-06-19 21:03 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('phone_notifications', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='BannedPhoneNumber', + fields=[ + ('phone_number', models.CharField(max_length=20, primary_key=True, serialize=False)), + ('created_at', models.DateTimeField(auto_now=True)), + ('reason', models.TextField(default=None, null=True)), + ('users', models.JSONField(default=None, null=True)), + ], + ), + ] diff --git a/engine/apps/phone_notifications/models/banned_phone_number.py b/engine/apps/phone_notifications/models/banned_phone_number.py new file mode 100644 index 00000000..a6d6511c --- /dev/null +++ b/engine/apps/phone_notifications/models/banned_phone_number.py @@ -0,0 +1,61 @@ +import json +import logging +from dataclasses import asdict, dataclass +from typing import List + +from django.db import models + +from apps.phone_notifications.exceptions import PhoneNumberBanned + +logger = logging.getLogger(__name__) + + +@dataclass +class BannedPhoneUserEntry: + user_id: int + user_name: str + org_id: int + stack_slug: str + org_slug: str + + +class BannedPhoneNumber(models.Model): + phone_number = models.CharField(primary_key=True, max_length=20) + created_at = models.DateTimeField(auto_now=True) + reason = models.TextField(null=True, default=None) + users = models.JSONField(null=True, default=None) + + def get_user_entries(self) -> List[BannedPhoneUserEntry]: + return [BannedPhoneUserEntry(**data) for data in json.loads(self.users)] + + +def ban_phone_number(phone_number: str, reason: str): + from apps.user_management.models import User + + banned_phone_number = BannedPhoneNumber(phone_number=phone_number) + users = User.objects.filter(_verified_phone_number=phone_number) + # Record instances of phone number use + user_entries = [ + asdict( + BannedPhoneUserEntry( + user_id=user.id, + user_name=user.username, + org_id=user.organization.org_id, + stack_slug=user.organization.stack_slug, + org_slug=user.organization.org_slug, + ) + ) + for user in users + ] + users.update(_verified_phone_number=None) + banned_phone_number.users = json.dumps(user_entries) + banned_phone_number.reason = reason + banned_phone_number.save() + + logger.info(f"ban_phone_number={phone_number}, in use by users={len(user_entries)}, reason={reason}") + + +def check_banned_phone_number(phone_number: str): + banned_entry = BannedPhoneNumber.objects.filter(phone_number=phone_number).first() + if banned_entry: + raise PhoneNumberBanned diff --git a/engine/apps/phone_notifications/phone_backend.py b/engine/apps/phone_notifications/phone_backend.py index 26b64399..a8cf50d6 100644 --- a/engine/apps/phone_notifications/phone_backend.py +++ b/engine/apps/phone_notifications/phone_backend.py @@ -21,6 +21,7 @@ from .exceptions import ( SMSLimitExceeded, ) from .models import PhoneCallRecord, ProviderPhoneCall, ProviderSMS, SMSRecord +from .models.banned_phone_number import check_banned_phone_number from .phone_provider import PhoneProvider, get_phone_provider logger = logging.getLogger(__name__) @@ -316,6 +317,7 @@ class PhoneBackend: if self._validate_user_number(user): logger.info(f"PhoneBackend.send_verification_sms: number already verified for user {user.id}") raise NumberAlreadyVerified + check_banned_phone_number(user.unverified_phone_number) self.phone_provider.send_verification_sms(user.unverified_phone_number) def make_verification_call(self, user): @@ -327,6 +329,7 @@ class PhoneBackend: if self._validate_user_number(user): logger.info(f"PhoneBackend.make_verification_call: number already verified user_id={user.id}") raise NumberAlreadyVerified + check_banned_phone_number(user.unverified_phone_number) self.phone_provider.make_verification_call(user.unverified_phone_number) def verify_phone_number(self, user, code) -> bool: diff --git a/engine/apps/phone_notifications/tests/test_banned_phone_number.py b/engine/apps/phone_notifications/tests/test_banned_phone_number.py new file mode 100644 index 00000000..8363a790 --- /dev/null +++ b/engine/apps/phone_notifications/tests/test_banned_phone_number.py @@ -0,0 +1,41 @@ +import pytest + +from apps.phone_notifications.models.banned_phone_number import BannedPhoneNumber, ban_phone_number + + +@pytest.mark.django_db +def test_ban_phone_number(make_organization, make_user_for_organization): + organization = make_organization() + banned_phone_number = "+1234567890" + unbanned_phone_number = "+0987654321" + banned_user_1 = make_user_for_organization( + organization=organization, + _verified_phone_number=banned_phone_number, + unverified_phone_number=banned_phone_number, + ) + banned_user_2 = make_user_for_organization( + organization=organization, + _verified_phone_number=banned_phone_number, + unverified_phone_number=banned_phone_number, + ) + unbanned_user = make_user_for_organization( + organization=organization, + _verified_phone_number=unbanned_phone_number, + unverified_phone_number=unbanned_phone_number, + ) + reason = "usage too high" + ban_phone_number(banned_phone_number, reason) + banned_user_1.refresh_from_db() + assert banned_user_1._verified_phone_number is None + assert banned_user_1.unverified_phone_number == banned_phone_number + banned_user_2.refresh_from_db() + assert banned_user_2._verified_phone_number is None + assert banned_user_2.unverified_phone_number == banned_phone_number + unbanned_user.refresh_from_db() + assert unbanned_user._verified_phone_number == unbanned_phone_number + assert unbanned_user.unverified_phone_number == unbanned_phone_number + ban_phone_number_entry = BannedPhoneNumber.objects.get(pk=banned_phone_number) + assert ban_phone_number_entry is not None + assert ban_phone_number_entry.reason == reason + user_entries = ban_phone_number_entry.get_user_entries() + assert len(user_entries) == 2 diff --git a/engine/apps/phone_notifications/tests/test_phone_backend_phone_verification.py b/engine/apps/phone_notifications/tests/test_phone_backend_phone_verification.py index cfb8996c..a12380cb 100644 --- a/engine/apps/phone_notifications/tests/test_phone_backend_phone_verification.py +++ b/engine/apps/phone_notifications/tests/test_phone_backend_phone_verification.py @@ -2,7 +2,8 @@ from unittest import mock import pytest -from apps.phone_notifications.exceptions import NumberAlreadyVerified +from apps.phone_notifications.exceptions import NumberAlreadyVerified, PhoneNumberBanned +from apps.phone_notifications.models.banned_phone_number import ban_phone_number from apps.phone_notifications.phone_backend import PhoneBackend from apps.phone_notifications.tests.mock_phone_provider import MockPhoneProvider @@ -67,3 +68,67 @@ def test_make_verification_call_raises_when_number_verified( user.save_verified_phone_number("+1234567890") with pytest.raises(NumberAlreadyVerified): phone_backend.make_verification_call(user) + + +@pytest.mark.django_db +@mock.patch("apps.phone_notifications.phone_backend.PhoneBackend._validate_user_number", return_value=False) +@mock.patch("apps.phone_notifications.tests.mock_phone_provider.MockPhoneProvider.send_verification_sms") +def test_send_verification_sms_banned_number( + mock_send_verification_sms, mock_validate_user_number, make_organization_and_user +): + _, user = make_organization_and_user() + phone_backend = PhoneBackend() + + number_to_verify = "+1234567890" + user.unverified_phone_number = "+1234567890" + ban_phone_number(number_to_verify, "usage too high") + with pytest.raises(PhoneNumberBanned): + phone_backend.send_verification_sms(user) + + +@pytest.mark.django_db +@mock.patch("apps.phone_notifications.phone_backend.PhoneBackend._validate_user_number", return_value=False) +@mock.patch("apps.phone_notifications.tests.mock_phone_provider.MockPhoneProvider.send_verification_sms") +def test_send_verification_sms_unaffected_by_ban( + mock_send_verification_sms, mock_validate_user_number, make_organization_and_user +): + _, user = make_organization_and_user() + phone_backend = PhoneBackend() + + number_to_verify = "+1234567890" + user.unverified_phone_number = "+1234567890" + ban_phone_number("+0987654321", "usage too high") + phone_backend.send_verification_sms(user) + mock_send_verification_sms.assert_called_once_with(number_to_verify) + + +@pytest.mark.django_db +@mock.patch("apps.phone_notifications.phone_backend.PhoneBackend._validate_user_number", return_value=False) +@mock.patch("apps.phone_notifications.tests.mock_phone_provider.MockPhoneProvider.make_verification_call") +def test_make_verification_call_banned_number( + mock_make_verification_call, mock_validate_user_number, make_organization_and_user +): + _, user = make_organization_and_user() + phone_backend = PhoneBackend() + + number_to_verify = "+1234567890" + user.unverified_phone_number = "+1234567890" + ban_phone_number(number_to_verify, "usage too high") + with pytest.raises(PhoneNumberBanned): + phone_backend.make_verification_call(user) + + +@pytest.mark.django_db +@mock.patch("apps.phone_notifications.phone_backend.PhoneBackend._validate_user_number", return_value=False) +@mock.patch("apps.phone_notifications.tests.mock_phone_provider.MockPhoneProvider.make_verification_call") +def test_make_verification_call_unaffected_by_ban( + mock_make_verification_call, mock_validate_user_number, make_organization_and_user +): + _, user = make_organization_and_user() + phone_backend = PhoneBackend() + + number_to_verify = "+1234567890" + user.unverified_phone_number = "+1234567890" + ban_phone_number("+0987654321", "usage too high") + phone_backend.make_verification_call(user) + mock_make_verification_call.assert_called_once_with(number_to_verify)