diff --git a/backend/apps/accounts/tests/test_authentica_e2e_real.py b/backend/apps/accounts/tests/test_authentica_e2e_real.py deleted file mode 100644 index e6b908b..0000000 --- a/backend/apps/accounts/tests/test_authentica_e2e_real.py +++ /dev/null @@ -1,72 +0,0 @@ -"""Real Authentica E2E OTP flow. Requires live credentials and a phone receiving OTPs.""" - -import os -from datetime import timedelta - -import pytest -from django.test import override_settings -from django.urls import reverse - -from django.utils import timezone - -from apps.accounts.models import OtpChannel, OtpPurpose, PhoneOTP, User -from apps.accounts.services.phone import normalize_phone_number - - -@pytest.mark.django_db -@pytest.mark.external -@override_settings(OTP_PROVIDER="authentica") -def test_authentica_phone_auth_e2e(client): - if os.getenv("AUTHENTICA_E2E") != "1": - pytest.skip("AUTHENTICA_E2E=1 not set") - - api_key = os.getenv("AUTHENTICA_API_KEY") - phone_number = os.getenv("AUTHENTICA_E2E_PHONE") - if not api_key or not phone_number: - pytest.skip("Missing AUTHENTICA_API_KEY or AUTHENTICA_E2E_PHONE") - - request_url = reverse("phone_auth_request") - response = client.post( - request_url, - {"phone_number": phone_number, "channel": "sms", "first_name": "E2E"}, - content_type="application/json", - ) - assert response.status_code == 201 - request_id = response.json()["request_id"] - assert request_id - - code = os.getenv("AUTHENTICA_E2E_CODE") - if not code: - pytest.skip("AUTHENTICA_E2E_CODE not set") - - normalized_phone = normalize_phone_number(phone_number) - User.objects.get_or_create( - phone_number=normalized_phone, - defaults={"role": "customer"}, - ) - if not PhoneOTP.objects.filter(id=request_id).exists(): - # Create a local OTP record so the verify endpoint can bind to a request_id. - PhoneOTP.objects.create( - id=request_id, - phone_number=normalized_phone, - channel=OtpChannel.SMS, - purpose=OtpPurpose.AUTH, - provider="authentica", - code_hash="placeholder", - expires_at=timezone.now() + timedelta(minutes=5), - ) - - verify_url = reverse("phone_auth_verify") - verify = client.post( - verify_url, - {"request_id": request_id, "code": code}, - content_type="application/json", - ) - assert verify.status_code == 200 - data = verify.json() - assert "access" in data - assert "refresh" in data - - user = User.objects.filter(phone_number=normalized_phone).first() - assert user is not None - assert user.is_phone_verified is True diff --git a/backend/apps/accounts/tests/test_authentica_provider.py b/backend/apps/accounts/tests/test_authentica_provider.py index 7faa186..98265cb 100644 --- a/backend/apps/accounts/tests/test_authentica_provider.py +++ b/backend/apps/accounts/tests/test_authentica_provider.py @@ -83,6 +83,32 @@ def test_authentica_verify_otp_calls_api(mock_post): assert kwargs["json"] == {"phone": "+966512345678", "otp": "123456"} +@patch("requests.post") +def test_authentica_request_failure_raises(mock_post): + mock_response = MagicMock(ok=False) + mock_response.json.return_value = {"detail": "fail"} + mock_post.return_value = mock_response + + with patch.dict(os.environ, {"AUTHENTICA_API_KEY": "api-key"}): + provider = AuthenticaOtpProvider() + with pytest.raises(RuntimeError): + provider.send_otp("+966512345678", OtpChannel.SMS) + + +def test_authentica_send_sms_requires_sender_name(): + with patch.dict(os.environ, {"AUTHENTICA_API_KEY": "api-key"}): + provider = AuthenticaOtpProvider() + with pytest.raises(ValueError): + provider.send_sms("+966512345678", "Hello") + + +def test_authentica_send_otp_rejects_unknown_channel(): + with patch.dict(os.environ, {"AUTHENTICA_API_KEY": "api-key"}): + provider = AuthenticaOtpProvider() + with pytest.raises(ValueError): + provider.send_otp("+966512345678", "email") + + @pytest.mark.django_db def test_verify_otp_uses_provider_for_authentica(): otp = PhoneOTP.objects.create( diff --git a/backend/apps/accounts/tests/test_otp_api.py b/backend/apps/accounts/tests/test_otp_api.py new file mode 100644 index 0000000..2442354 --- /dev/null +++ b/backend/apps/accounts/tests/test_otp_api.py @@ -0,0 +1,53 @@ +from datetime import timedelta + +import pytest +from django.test import override_settings +from django.urls import reverse +from django.utils import timezone + +from apps.accounts.models import OtpChannel, OtpPurpose, PhoneOTP + + +@pytest.mark.django_db +@override_settings(OTP_PROVIDER="console") +def test_otp_request_whatsapp_ok(client): + response = client.post( + reverse("otp_request"), + {"phone_number": "0512345678", "channel": "whatsapp"}, + content_type="application/json", + ) + assert response.status_code == 201 + data = response.json() + assert "request_id" in data + assert "expires_at" in data + + +@pytest.mark.django_db +def test_otp_verify_rejects_expired(client): + otp = PhoneOTP.objects.create( + phone_number="+966512345678", + channel=OtpChannel.SMS, + purpose=OtpPurpose.VERIFY, + provider="console", + code_hash="unused", + expires_at=timezone.now() - timedelta(minutes=1), + ) + response = client.post( + reverse("otp_verify"), + {"request_id": str(otp.id), "code": "123456"}, + content_type="application/json", + ) + assert response.status_code == 400 + + +@pytest.mark.django_db +@override_settings(OTP_PROVIDER="console") +def test_otp_request_invalid_phone_localized_ar(client): + response = client.post( + reverse("otp_request"), + {"phone_number": "123", "channel": "sms"}, + content_type="application/json", + HTTP_ACCEPT_LANGUAGE="ar-sa", + ) + assert response.status_code == 400 + assert response.json()["phone_number"][0] == "يجب أن يكون رقم الهاتف بصيغة E.164 أو رقم جوال سعودي صالح" diff --git a/backend/apps/accounts/tests/test_otp_limits.py b/backend/apps/accounts/tests/test_otp_limits.py index 1158dca..d1f1db1 100644 --- a/backend/apps/accounts/tests/test_otp_limits.py +++ b/backend/apps/accounts/tests/test_otp_limits.py @@ -1,6 +1,10 @@ +from datetime import timedelta +from unittest.mock import patch + import pytest from django.contrib.auth.hashers import make_password from django.test import override_settings +from django.utils import timezone from apps.accounts.models import OtpChannel, OtpPurpose, PhoneOTP from apps.accounts.services.otp import OtpCooldownError, OtpRateLimitError, create_and_send_otp, verify_otp @@ -47,3 +51,76 @@ def test_otp_max_attempts_blocks_verification(): otp.refresh_from_db() assert otp.attempt_count == otp.max_attempts + 1 assert otp.verified_at is None + + +@pytest.mark.django_db +@override_settings(OTP_PROVIDER="console", OTP_RESEND_COOLDOWN_SECONDS=60) +def test_otp_cooldown_retry_after_seconds(): + fixed_now = timezone.now() + with patch("apps.accounts.services.otp.timezone.now", return_value=fixed_now): + result = create_and_send_otp("+966512345678", OtpChannel.SMS) + # Align created_at with fixed time for deterministic cooldown. + PhoneOTP.objects.filter(id=result.request_id).update(created_at=fixed_now) + with pytest.raises(OtpCooldownError) as excinfo: + create_and_send_otp("+966512345678", OtpChannel.SMS) + assert excinfo.value.retry_after_seconds == 60 + + +@pytest.mark.django_db +@override_settings(OTP_PROVIDER="console", OTP_MAX_PER_WINDOW=1, OTP_WINDOW_MINUTES=15, OTP_RESEND_COOLDOWN_SECONDS=0) +def test_otp_rate_limit_retry_after_seconds(): + fixed_now = timezone.now() + with patch("apps.accounts.services.otp.timezone.now", return_value=fixed_now): + result = create_and_send_otp("+966512345678", OtpChannel.SMS) + # Make the oldest OTP sit 10s before window expiry. + window_start = fixed_now - timedelta(minutes=15) + PhoneOTP.objects.filter(id=result.request_id).update(created_at=window_start + timedelta(seconds=10)) + with pytest.raises(OtpRateLimitError) as excinfo: + create_and_send_otp("+966512345678", OtpChannel.SMS) + assert excinfo.value.retry_after_seconds == 10 + + +@pytest.mark.django_db +@override_settings(OTP_PROVIDER="console", OTP_RESEND_COOLDOWN_SECONDS=1) +def test_otp_resend_after_cooldown_ok(): + result = create_and_send_otp("+966512345678", OtpChannel.SMS) + # Force cooldown to be elapsed. + PhoneOTP.objects.filter(id=result.request_id).update( + created_at=timezone.now() - timedelta(seconds=5) + ) + create_and_send_otp("+966512345678", OtpChannel.SMS) + assert PhoneOTP.objects.filter(phone_number="+966512345678").count() == 2 + + +@pytest.mark.django_db +def test_verify_otp_rejects_expired(): + otp = PhoneOTP.objects.create( + phone_number="+966512345678", + channel=OtpChannel.SMS, + purpose=OtpPurpose.AUTH, + provider="console", + code_hash=make_password("123456"), + expires_at=timezone.now() - timedelta(minutes=1), + ) + assert verify_otp(otp, "123456") is False + otp.refresh_from_db() + assert otp.attempt_count == 0 + assert otp.verified_at is None + + +@pytest.mark.django_db +def test_verify_otp_rejects_reuse_after_verified(): + otp = PhoneOTP.objects.create( + phone_number="+966512345678", + channel=OtpChannel.SMS, + purpose=OtpPurpose.AUTH, + provider="console", + code_hash=make_password("123456"), + expires_at=PhoneOTP.expiry_at(), + ) + assert verify_otp(otp, "123456") is True + otp.refresh_from_db() + assert otp.attempt_count == 1 + assert verify_otp(otp, "123456") is False + otp.refresh_from_db() + assert otp.attempt_count == 1 diff --git a/backend/apps/accounts/tests/test_twilio_provider.py b/backend/apps/accounts/tests/test_twilio_provider.py deleted file mode 100644 index 63a2907..0000000 --- a/backend/apps/accounts/tests/test_twilio_provider.py +++ /dev/null @@ -1,51 +0,0 @@ -"""Tests for Twilio OTP provider implementation.""" - -import pytest -from unittest.mock import MagicMock, patch - - -@pytest.mark.django_db -@patch("apps.accounts.services.otp.TwilioOtpProvider._get_client") -def test_twilio_send_sms_calls_client(mock_get_client): - from apps.accounts.services.otp import TwilioOtpProvider - - mock_client = MagicMock() - mock_get_client.return_value = mock_client - - with patch.dict("os.environ", { - "TWILIO_ACCOUNT_SID": "AC123", - "TWILIO_AUTH_TOKEN": "token", - "TWILIO_FROM_NUMBER": "+966500000000", - }): - provider = TwilioOtpProvider() - provider.send_sms("+966512345678", "Your code is 123456") - - mock_client.messages.create.assert_called_once_with( - body="Your code is 123456", - from_="+966500000000", - to="+966512345678", - ) - - -@pytest.mark.django_db -@patch("apps.accounts.services.otp.TwilioOtpProvider._get_client") -def test_twilio_send_whatsapp_calls_client(mock_get_client): - from apps.accounts.services.otp import TwilioOtpProvider - - mock_client = MagicMock() - mock_get_client.return_value = mock_client - - with patch.dict("os.environ", { - "TWILIO_ACCOUNT_SID": "AC123", - "TWILIO_AUTH_TOKEN": "token", - "TWILIO_FROM_NUMBER": "+966500000000", - "TWILIO_WHATSAPP_FROM": "14155238886", - }): - provider = TwilioOtpProvider() - provider.send_whatsapp("+966512345678", "Your code is 123456") - - mock_client.messages.create.assert_called_once_with( - body="Your code is 123456", - from_="whatsapp:14155238886", - to="whatsapp:+966512345678", - ) diff --git a/docs/adr/0003-authentica-otp-provider.md b/docs/adr/0003-authentica-otp-provider.md index 7351e55..7952d64 100644 --- a/docs/adr/0003-authentica-otp-provider.md +++ b/docs/adr/0003-authentica-otp-provider.md @@ -6,11 +6,11 @@ Accepted ## Context -The platform requires phone-first authentication with OTP delivery for KSA. The codebase includes multiple provider adapters (`console`, `twilio`, `unifonic`, `authentica`) but only Authentica is implemented for provider-managed OTP delivery (send/verify) and direct SMS messaging. Twilio and Unifonic adapters are partial or unimplemented; a console provider exists for local development. +The platform requires phone-first authentication with OTP delivery for KSA. The codebase includes provider adapters (`console`, `authentica`) and Authentica is implemented for provider-managed OTP delivery (send/verify) and direct SMS messaging. A console provider exists for local development. ## Decision -Use Authentica as the primary OTP provider for the MVP, with `OTP_PROVIDER=authentica` in production environments. Keep `console` for local development and tests, and retain Twilio/Unifonic adapters as scaffolds for future expansion. +Use Authentica as the primary OTP provider for the MVP, with `OTP_PROVIDER=authentica` in production environments. Keep `console` for local development and tests. ## Consequences @@ -20,8 +20,6 @@ Use Authentica as the primary OTP provider for the MVP, with `OTP_PROVIDER=authe ## Alternatives Considered -- Twilio as primary provider: not selected due to KSA-focused delivery needs and current adapter gaps. -- Unifonic as primary provider: deferred until the adapter is fully implemented and validated. ## Related diff --git a/docs/architecture.md b/docs/architecture.md index b0ab8d4..9338e2a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -67,7 +67,7 @@ The roadmap assumes a KSA-focused first launch (phone auth and Riyadh timezone d ### Backend Risks - **Incomplete provider implementations for production-critical flows** - - Twilio/Unifonic providers in `[backend/apps/accounts/services/otp.py](backend/apps/accounts/services/otp.py)` are stubs with `NotImplementedError` for send methods, yet they are the backbone for both OTP and booking notifications. + - Authentica is the only OTP/notification provider; ensure it is fully configured and exercised in production-like environments. - `MoyasarGateway` lacks `capture_payment` and `refund_payment` implementations, limiting payment lifecycle coverage. - **Risk**: Code reads “production ready” at the API level, but the underlying integrations are not, which could cause outages if deployed naively. - **Tight coupling between OTP and notifications** @@ -125,7 +125,7 @@ This roadmap assumes “MVP” is equivalent to **Phase 1: Core MVP Reliability* ### Phase 0 – Architecture & Production Readiness Hardening - **Finalize critical provider implementations** - - Implement at least one real SMS/WhatsApp provider (Twilio or Unifonic) end-to-end, behind the existing provider abstraction in `[backend/apps/accounts/services/otp.py](backend/apps/accounts/services/otp.py)`, and wire it into `[backend/apps/notifications/services.py](backend/apps/notifications/services.py)`. + - Ensure Authentica OTP/SMS is fully configured and validated end-to-end behind the provider abstraction in `[backend/apps/accounts/services/otp.py](backend/apps/accounts/services/otp.py)`, and wired into `[backend/apps/notifications/services.py](backend/apps/notifications/services.py)`. - Implement or deliberately fence off `capture_payment` and `refund_payment` in `[backend/apps/payments/services/gateway.py](backend/apps/payments/services/gateway.py)` so that the MVP either fully supports or explicitly does not support partial captures/refunds. - **Clarify and document boundaries** - Add a short architecture section in `README`/docs describing how `accounts`, `salons`, `bookings`, `payments`, and `notifications` interact, and what each service is responsible for. @@ -220,4 +220,3 @@ This diagram clarifies current coupling and highlights where future refactors (e - It identifies the highest-risk design/architecture issues that could impede MVP reliability or future evolution. - It provides a phased, concrete sequence of work packages that can be implemented via ExecPlans (e.g., booking notifications, Moyasar payments, phone auth UX). - Each major feature area (auth, bookings, payments, notifications, localization, tests) should have or adopt an ExecPlan under `docs/execplans/` in line with `PLANS.md` before implementation begins. - diff --git a/docs/risks.md b/docs/risks.md index 3661198..8367ce0 100644 --- a/docs/risks.md +++ b/docs/risks.md @@ -5,7 +5,7 @@ This file tracks known gaps and risks to address in future iterations. ## Security And Auth - Phone normalization is KSA-focused and minimal; broaden for multi-country use. - OTP protections are basic; add device fingerprinting and IP throttling if needed. -- Authentica OTP provider is implemented (SMS + WhatsApp via Authentica OTP); Unifonic remains a scaffold. +- Authentica OTP provider is implemented (SMS + WhatsApp via Authentica OTP). - Social login is a placeholder. - `USERNAME_FIELD` is now `"phone_number"`; `REQUIRED_FIELDS = []`; `create_superuser` accepts `phone_number`. Admin and `createsuperuser` work correctly for phone-only users.