Compare commits
3 Commits
0c992404ea
...
main
| Author | SHA1 | Date | |
|---|---|---|---|
| 560460dd84 | |||
| c212acc504 | |||
| 15ed5036d1 |
@@ -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
|
|
||||||
@@ -83,6 +83,32 @@ def test_authentica_verify_otp_calls_api(mock_post):
|
|||||||
assert kwargs["json"] == {"phone": "+966512345678", "otp": "123456"}
|
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
|
@pytest.mark.django_db
|
||||||
def test_verify_otp_uses_provider_for_authentica():
|
def test_verify_otp_uses_provider_for_authentica():
|
||||||
otp = PhoneOTP.objects.create(
|
otp = PhoneOTP.objects.create(
|
||||||
|
|||||||
@@ -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 أو رقم جوال سعودي صالح"
|
||||||
@@ -1,6 +1,10 @@
|
|||||||
|
from datetime import timedelta
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from django.contrib.auth.hashers import make_password
|
from django.contrib.auth.hashers import make_password
|
||||||
from django.test import override_settings
|
from django.test import override_settings
|
||||||
|
from django.utils import timezone
|
||||||
|
|
||||||
from apps.accounts.models import OtpChannel, OtpPurpose, PhoneOTP
|
from apps.accounts.models import OtpChannel, OtpPurpose, PhoneOTP
|
||||||
from apps.accounts.services.otp import OtpCooldownError, OtpRateLimitError, create_and_send_otp, verify_otp
|
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()
|
otp.refresh_from_db()
|
||||||
assert otp.attempt_count == otp.max_attempts + 1
|
assert otp.attempt_count == otp.max_attempts + 1
|
||||||
assert otp.verified_at is None
|
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
|
||||||
|
|||||||
@@ -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",
|
|
||||||
)
|
|
||||||
@@ -6,11 +6,11 @@ Accepted
|
|||||||
|
|
||||||
## Context
|
## 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
|
## 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
|
## Consequences
|
||||||
|
|
||||||
@@ -20,8 +20,6 @@ Use Authentica as the primary OTP provider for the MVP, with `OTP_PROVIDER=authe
|
|||||||
|
|
||||||
## Alternatives Considered
|
## 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
|
## Related
|
||||||
|
|
||||||
|
|||||||
@@ -67,7 +67,7 @@ The roadmap assumes a KSA-focused first launch (phone auth and Riyadh timezone d
|
|||||||
### Backend Risks
|
### Backend Risks
|
||||||
|
|
||||||
- **Incomplete provider implementations for production-critical flows**
|
- **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.
|
- `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.
|
- **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**
|
- **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
|
### Phase 0 – Architecture & Production Readiness Hardening
|
||||||
|
|
||||||
- **Finalize critical provider implementations**
|
- **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.
|
- 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**
|
- **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.
|
- 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 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).
|
- 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.
|
- 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.
|
||||||
|
|
||||||
|
|||||||
+1
-1
@@ -5,7 +5,7 @@ This file tracks known gaps and risks to address in future iterations.
|
|||||||
## Security And Auth
|
## Security And Auth
|
||||||
- Phone normalization is KSA-focused and minimal; broaden for multi-country use.
|
- Phone normalization is KSA-focused and minimal; broaden for multi-country use.
|
||||||
- OTP protections are basic; add device fingerprinting and IP throttling if needed.
|
- 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.
|
- 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.
|
- `USERNAME_FIELD` is now `"phone_number"`; `REQUIRED_FIELDS = []`; `create_superuser` accepts `phone_number`. Admin and `createsuperuser` work correctly for phone-only users.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user