Skip to content
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
4ec016d
feat:added user otp model
nandkishorr Apr 27, 2026
667e54e
feat:added pydantic specs for apis
nandkishorr Apr 27, 2026
3dfdd03
feat:created send and confirm apis
nandkishorr Apr 27, 2026
8021e7a
feat:added reset password template
nandkishorr Apr 27, 2026
f666b68
feat:added routes and migrations
nandkishorr Apr 27, 2026
520e877
Merge branch 'develop' into ENG-32-create-otp-based-reset-for-password
nandkishorr Apr 27, 2026
8a9b522
fix:cleanup - review suggestions
nandkishorr Apr 27, 2026
164721e
Merge branch 'ENG-32-create-otp-based-reset-for-password' of https://…
nandkishorr Apr 27, 2026
54cdcda
Merge branch 'develop' into ENG-32-create-otp-based-reset-for-password
nandkishorr Apr 29, 2026
7454a65
Merge branch 'develop' into ENG-32-create-otp-based-reset-for-password
nandkishorr May 5, 2026
e15ae5a
refact:updated the api and otp sending method
nandkishorr May 6, 2026
afeb68c
cleanup
nandkishorr May 6, 2026
fafad78
refact:message contex files to env
nandkishorr May 6, 2026
ab6042d
Merge branch 'develop' into ENG-32-create-otp-based-reset-for-password
nandkishorr May 7, 2026
0a9937b
Merge branch 'develop' into ENG-32-create-otp-based-reset-for-password
nandkishorr May 17, 2026
fc1da34
Merge branch 'develop' into ENG-32-create-otp-based-reset-for-password
nandkishorr May 21, 2026
7215c1e
refact:updated the otp validation to api function
nandkishorr May 21, 2026
9347046
refact:review changes added
nandkishorr May 21, 2026
f532197
Merge branch 'develop' into ENG-32-create-otp-based-reset-for-password
nandkishorr May 25, 2026
447ce22
Merge branch 'develop' into ENG-32-create-otp-based-reset-for-password
nandkishorr May 26, 2026
6fe3ccd
fix:added review changes
nandkishorr May 26, 2026
97d1998
Merge branch 'develop' into ENG-32-create-otp-based-reset-for-password
nandkishorr May 28, 2026
1638abe
Merge branch 'develop' into ENG-32-create-otp-based-reset-for-password
nandkishorr May 28, 2026
7f8e3d9
Merge branch 'ENG-32-create-otp-based-reset-for-password' of https://…
nandkishorr May 28, 2026
02e5abc
Merge branch 'develop' into ENG-32-create-otp-based-reset-for-password
nandkishorr May 28, 2026
21dc945
Merge branch 'develop' into ENG-32-create-otp-based-reset-for-password
nandkishorr May 28, 2026
acfb02a
feat:added testcases for otp reset password api
nandkishorr May 28, 2026
8e03058
Merge branch 'ENG-32-create-otp-based-reset-for-password' of https://…
nandkishorr May 28, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 50 additions & 37 deletions care/emr/api/otp_viewsets/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,57 @@
from rest_framework.response import Response

from care.emr.api.viewsets.base import EMRBaseViewSet
from care.facility.models.patient import PatientMobileOTP
from care.facility.models.patient import MobileOTP
from care.utils import sms
from care.utils.models.validators import mobile_validator
from care.utils.sms.utils import get_sms_content
from config.patient_otp_token import PatientToken

logger = logging.getLogger(__name__)


class OTPType:
Comment thread
nandkishorr marked this conversation as resolved.
Outdated
def render_content(self, otp: str) -> str:
pass


class LoginOTP(OTPType):
def render_content(self, otp: str) -> str:
Comment thread
nandkishorr marked this conversation as resolved.
Outdated
return settings.OTP_SMS_LOGIN_CONTENT.format(otp=otp)


def rand_pass(size):
return "".join(secrets.choice(string.digits) for _ in range(size))


class OTPLoginRequestSpec(BaseModel):
def send_otp(phone_number, otp_type: OTPType):
sent_otps = MobileOTP.objects.filter(
created_date__gte=(timezone.now() - timedelta(settings.OTP_REPEAT_WINDOW)),
is_used=False,
phone_number=phone_number,
)
if sent_otps.count() >= settings.OTP_MAX_REPEATS_WINDOW:
raise ValueError("Max Retries has exceeded")

random_otp = ""
if settings.USE_SMS:
random_otp = rand_pass(settings.OTP_LENGTH)
try:
content = otp_type.render_content(random_otp)
sms.send_text_message(
content=content,
recipients=[phone_number],
)
except Exception as e:
raise Exception("Error while sending OTP. Contact admin.") from e
elif settings.IS_PRODUCTION:
random_otp = rand_pass(settings.OTP_LENGTH)
else:
random_otp = "45612"

MobileOTP.objects.create(phone_number=phone_number, otp=random_otp)


class OTPRequestBaseSpec(BaseModel):
phone_number: str

@field_validator("phone_number")
Expand All @@ -39,7 +76,7 @@
return value


class OTPLoginSpec(OTPLoginRequestSpec):
class OTPLoginSpec(OTPRequestBaseSpec):
otp: str = Field(min_length=settings.OTP_LENGTH, max_length=settings.OTP_LENGTH)


Expand All @@ -48,41 +85,17 @@
permission_classes = []

@extend_schema(
request=OTPLoginRequestSpec,
request=OTPRequestBaseSpec,
)
@action(detail=False, methods=["POST"])
def send(self, request):
data = OTPLoginRequestSpec(**request.data)
sent_otps = PatientMobileOTP.objects.filter(
created_date__gte=(timezone.now() - timedelta(settings.OTP_REPEAT_WINDOW)),
is_used=False,
phone_number=data.phone_number,
)
if sent_otps.count() >= settings.OTP_MAX_REPEATS_WINDOW:
raise ValidationError({"phone_number": "Max Retries has exceeded"})
random_otp = ""
if settings.USE_SMS:
random_otp = rand_pass(settings.OTP_LENGTH)
try:
content = get_sms_content(
settings.OTP_SMS_TEMPLATE_PATH, {"random_otp": random_otp}
)
sms.send_text_message(
content=content,
recipients=[data.phone_number],
)
except Exception as e:
logger.error(e)
return Response(
{"error": "Error while sending OTP. Contact admin."}, status=400
)
elif settings.IS_PRODUCTION:
random_otp = rand_pass(settings.OTP_LENGTH)
else:
random_otp = "45612"

otp_obj = PatientMobileOTP(phone_number=data.phone_number, otp=random_otp)
otp_obj.save()
data = OTPRequestBaseSpec(**request.data)
try:
send_otp(data.phone_number, otp_type=LoginOTP())
except ValueError as e:
raise ValidationError({"phone_number": str(e)}) from e

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
except Exception as e:
return Response({"error": str(e)}, status=400)

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
return Response({"otp": "generated"})
Comment thread
nandkishorr marked this conversation as resolved.

@extend_schema(
Expand All @@ -91,7 +104,7 @@
@action(detail=False, methods=["POST"])
def login(self, request):
data = OTPLoginSpec(**request.data)
otp_object = PatientMobileOTP.objects.filter(
otp_object = MobileOTP.objects.filter(
phone_number=data.phone_number, otp=data.otp, is_used=False
).first()
Comment on lines +108 to 110

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Expire login OTPs when validating them.

Lines 115-117 accept any unused row matching the phone number and OTP, with no created_date bound. That makes stale OTPs valid indefinitely until someone happens to use them, despite OTP_REPEAT_WINDOW being defined as the validity window.

Suggested fix
-        otp_object = MobileOTP.objects.filter(
-            phone_number=data.phone_number, otp=data.otp, is_used=False
-        ).first()
+        otp_object = (
+            MobileOTP.objects.filter(
+                phone_number=data.phone_number,
+                otp=data.otp,
+                is_used=False,
+                created_date__gte=(
+                    timezone.now() - timedelta(hours=settings.OTP_REPEAT_WINDOW)
+                ),
+            )
+            .order_by("-created_date")
+            .first()
+        )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@care/emr/api/otp_viewsets/login.py` around lines 115 - 117, The current
lookup for MobileOTP (using MobileOTP.objects.filter(...).first()) doesn't
enforce the OTP validity window; change the query to also filter created_date >=
timezone.now() - OTP_REPEAT_WINDOW (use Django timezone) so only recent OTPs are
accepted, and immediately expire the OTP upon successful validation by setting
otp_object.is_used = True and saving (or perform an atomic update like
MobileOTP.objects.filter(pk=otp_object.pk, is_used=False).update(is_used=True)
to avoid races). Reference MobileOTP, OTP_REPEAT_WINDOW, otp_object, and the
data.phone_number/data.otp fields when applying these changes.

if not otp_object:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 6.0 on 2026-05-05 17:41

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
('facility', '0484_remove_facility_discount_codes_and_more'),
]

operations = [
migrations.RenameModel(
old_name='PatientMobileOTP',
new_name='MobileOTP',
),
]
2 changes: 1 addition & 1 deletion care/facility/models/patient.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from care.utils.models.validators import mobile_or_landline_number_validator


class PatientMobileOTP(BaseModel):
class MobileOTP(BaseModel):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a periodic job to delete all OTP's that are older than the period they are active for.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its resolved in 3632

is_used = models.BooleanField(default=False)
phone_number = models.CharField(
max_length=14, validators=[mobile_or_landline_number_validator]
Expand Down
Empty file.
105 changes: 105 additions & 0 deletions care/users/api/otp_viewset/reset_password.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
from datetime import timedelta

from django.conf import settings
from django.contrib.auth.password_validation import (
get_password_validators,
validate_password,
)
from django.utils import timezone
from drf_spectacular.utils import extend_schema
from pydantic import Field
from rest_framework.decorators import action
from rest_framework.exceptions import ValidationError
from rest_framework.response import Response

from care.emr.api.otp_viewsets.login import OTPRequestBaseSpec, OTPType, send_otp
from care.emr.api.viewsets.base import EMRBaseViewSet
from care.facility.models.patient import MobileOTP
from care.users.models import User


class OTPResetSendSpec(OTPRequestBaseSpec):
pass


class ResetPasswordOTP(OTPType):
def render_content(self, otp: str) -> str:
return settings.OTP_SMS_RESET_PASSWORD_CONTENT.format(otp=otp)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated


class OTPResetConfirmSpec(OTPRequestBaseSpec):
otp: str = Field(min_length=settings.OTP_LENGTH, max_length=settings.OTP_LENGTH)
password: str = Field(min_length=8)
username: str | None = None


class OTPResetPasswordView(EMRBaseViewSet):
authentication_classes = []
permission_classes = []

@action(detail=False, methods=["POST"])
@extend_schema(request=OTPResetSendSpec)
def send(self, request):
data = OTPResetSendSpec(**request.data)

if not User.objects.filter(phone_number=data.phone_number).exists():
return Response({"otp": "generated"})

try:
send_otp(data.phone_number, otp_type=ResetPasswordOTP())
except ValueError as e:
raise ValidationError({"phone_number": str(e)}) from e

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
except Exception as e:
return Response({"error": str(e)}, status=400)

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
return Response({"otp": "generated"})

@action(detail=False, methods=["POST"])
@extend_schema(request=OTPResetConfirmSpec)
def confirm(self, request):
data = OTPResetConfirmSpec(**request.data)
otp_obj = (
MobileOTP.objects.filter(
phone_number=data.phone_number,
is_used=False,
created_date__gte=(
timezone.now() - timedelta(hours=settings.OTP_REPEAT_WINDOW)
),
)
.order_by("-created_date")
.first()
)
if not otp_obj or otp_obj.otp != data.otp:
raise ValidationError({"otp": "Invalid OTP"})

users = User.objects.filter(phone_number=data.phone_number)
if not users.exists():
raise ValidationError({"error": "No User linked to this phone number"})
if users.count() > 1:
Comment thread
nandkishorr marked this conversation as resolved.
Outdated
if data.username:
users = users.filter(username=data.username)
if not users.exists():
raise ValidationError(
{
"error": "No User with this username linked to this phone number"
}
)
else:
return Response(
{"error": "Multiple users linked to this phone number"},
status=409,
)
user = users.first()

validate_password(
data.password,
user=user,
password_validators=get_password_validators(
settings.AUTH_PASSWORD_VALIDATORS
),
)
user.set_password(data.password)
user.save()
Comment thread
nandkishorr marked this conversation as resolved.
MobileOTP.objects.filter(
phone_number=data.phone_number,
).delete()
Comment thread
nandkishorr marked this conversation as resolved.
return Response({"message": "Password reset successful"})
4 changes: 4 additions & 0 deletions config/api_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
from care.emr.api.viewsets.valueset import ValueSetViewSet
from care.security.api.viewsets.permissions import PermissionViewSet
from care.security.api.viewsets.roles import RoleViewSet
from care.users.api.otp_viewset.reset_password import OTPResetPasswordView
from care.users.api.viewsets.plug_config import PlugConfigViewset

router = DefaultRouter() if settings.DEBUG else SimpleRouter()
Expand All @@ -123,6 +124,9 @@
router.register("meta_artifacts", MetaArtifactViewSet, basename="meta_artifacts")

router.register("otp", OTPLoginView, basename="otp-login")
router.register(
"otp/password_reset", OTPResetPasswordView, basename="otp-password-reset"
)

router.register("otp/patient", PatientOTPView, basename="otp-patient")

Expand Down
10 changes: 9 additions & 1 deletion config/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,15 @@

SMS_BACKEND = "care.utils.sms.backend.sns.SnsBackend"

OTP_SMS_TEMPLATE_PATH = env("OTP_SMS_TEMPLATE", default="sms/otp_sms.txt")
OTP_SMS_LOGIN_CONTENT = env(
"OTP_SMS_LOGIN_CONTENT",
default="Care OTP for login is {otp}. Please do not share this with anyone.",
)

OTP_SMS_RESET_PASSWORD_CONTENT = env(
"OTP_SMS_RESET_PASSWORD_CONTENT",
default="Care OTP for password reset is {otp}. Please do not share this with anyone.",
)

USER_CREATE_PASSWORD_EMAIL_TEMPLATE_PATH = env(
"USER_CREATE_PASSWORD_TEMPLATE_PATH", default="email/user_create_password.html"
Expand Down
Loading