stages/authenticator_sms: fix error when phone number from context already exists (#7264)
This commit is contained in:
parent
6f67c1a277
commit
c0fe99714f
|
@ -12,7 +12,6 @@ from authentik.flows.challenge import (
|
|||
Challenge,
|
||||
ChallengeResponse,
|
||||
ChallengeTypes,
|
||||
ErrorDetailSerializer,
|
||||
WithUserInfoChallenge,
|
||||
)
|
||||
from authentik.flows.stage import ChallengeStageView
|
||||
|
@ -24,6 +23,7 @@ from authentik.stages.authenticator_sms.models import (
|
|||
from authentik.stages.prompt.stage import PLAN_CONTEXT_PROMPT
|
||||
|
||||
SESSION_KEY_SMS_DEVICE = "authentik/stages/authenticator_sms/sms_device"
|
||||
PLAN_CONTEXT_PHONE = "phone"
|
||||
|
||||
|
||||
class AuthenticatorSMSChallenge(WithUserInfoChallenge):
|
||||
|
@ -48,6 +48,8 @@ class AuthenticatorSMSChallengeResponse(ChallengeResponse):
|
|||
def validate(self, attrs: dict) -> dict:
|
||||
"""Check"""
|
||||
if "code" not in attrs:
|
||||
if "phone_number" not in attrs:
|
||||
raise ValidationError("phone_number required")
|
||||
self.device.phone_number = attrs["phone_number"]
|
||||
self.stage.validate_and_send(attrs["phone_number"])
|
||||
return super().validate(attrs)
|
||||
|
@ -67,6 +69,7 @@ class AuthenticatorSMSStageView(ChallengeStageView):
|
|||
stage: AuthenticatorSMSStage = self.executor.current_stage
|
||||
hashed_number = hash_phone_number(phone_number)
|
||||
query = Q(phone_number=hashed_number) | Q(phone_number=phone_number)
|
||||
print(SMSDevice.objects.filter(query, stage=stage.pk))
|
||||
if SMSDevice.objects.filter(query, stage=stage.pk).exists():
|
||||
raise ValidationError(_("Invalid phone number"))
|
||||
# No code yet, but we have a phone number, so send a verification message
|
||||
|
@ -75,9 +78,9 @@ class AuthenticatorSMSStageView(ChallengeStageView):
|
|||
|
||||
def _has_phone_number(self) -> Optional[str]:
|
||||
context = self.executor.plan.context
|
||||
if "phone" in context.get(PLAN_CONTEXT_PROMPT, {}):
|
||||
if PLAN_CONTEXT_PHONE in context.get(PLAN_CONTEXT_PROMPT, {}):
|
||||
self.logger.debug("got phone number from plan context")
|
||||
return context.get(PLAN_CONTEXT_PROMPT, {}).get("phone")
|
||||
return context.get(PLAN_CONTEXT_PROMPT, {}).get(PLAN_CONTEXT_PHONE)
|
||||
if SESSION_KEY_SMS_DEVICE in self.request.session:
|
||||
self.logger.debug("got phone number from device in session")
|
||||
device: SMSDevice = self.request.session[SESSION_KEY_SMS_DEVICE]
|
||||
|
@ -113,10 +116,17 @@ class AuthenticatorSMSStageView(ChallengeStageView):
|
|||
try:
|
||||
self.validate_and_send(phone_number)
|
||||
except ValidationError as exc:
|
||||
response = AuthenticatorSMSChallengeResponse()
|
||||
response._errors.setdefault("phone_number", [])
|
||||
response._errors["phone_number"].append(ErrorDetailSerializer(exc.detail))
|
||||
return self.challenge_invalid(response)
|
||||
# We had a phone number given already (at this point only possible from flow
|
||||
# context), but an error occurred while sending a number (most likely)
|
||||
# due to a duplicate device, so delete the number we got given, reset the state
|
||||
# (ish) and retry
|
||||
device.phone_number = ""
|
||||
self.executor.plan.context.get(PLAN_CONTEXT_PROMPT, {}).pop(
|
||||
PLAN_CONTEXT_PHONE, None
|
||||
)
|
||||
self.request.session.pop(SESSION_KEY_SMS_DEVICE, None)
|
||||
self.logger.warning("failed to send SMS message to pre-set number", exc=exc)
|
||||
return self.get(request, *args, **kwargs)
|
||||
return super().get(request, *args, **kwargs)
|
||||
|
||||
def challenge_valid(self, response: ChallengeResponse) -> HttpResponse:
|
||||
|
|
|
@ -7,7 +7,9 @@ from requests_mock import Mocker
|
|||
|
||||
from authentik.core.tests.utils import create_test_admin_user, create_test_flow
|
||||
from authentik.flows.models import FlowStageBinding
|
||||
from authentik.flows.planner import FlowPlan
|
||||
from authentik.flows.tests import FlowTestCase
|
||||
from authentik.flows.views.executor import SESSION_KEY_PLAN
|
||||
from authentik.lib.generators import generate_id
|
||||
from authentik.stages.authenticator_sms.models import (
|
||||
AuthenticatorSMSStage,
|
||||
|
@ -15,7 +17,8 @@ from authentik.stages.authenticator_sms.models import (
|
|||
SMSProviders,
|
||||
hash_phone_number,
|
||||
)
|
||||
from authentik.stages.authenticator_sms.stage import SESSION_KEY_SMS_DEVICE
|
||||
from authentik.stages.authenticator_sms.stage import PLAN_CONTEXT_PHONE, SESSION_KEY_SMS_DEVICE
|
||||
from authentik.stages.prompt.stage import PLAN_CONTEXT_PROMPT
|
||||
|
||||
|
||||
class AuthenticatorSMSStageTests(FlowTestCase):
|
||||
|
@ -172,6 +175,45 @@ class AuthenticatorSMSStageTests(FlowTestCase):
|
|||
phone_number_required=False,
|
||||
)
|
||||
|
||||
def test_stage_context_data_duplicate(self):
|
||||
"""test stage context data (phone number exists already)"""
|
||||
self.client.get(
|
||||
reverse("authentik_flows:configure", kwargs={"stage_uuid": self.stage.stage_uuid}),
|
||||
)
|
||||
plan: FlowPlan = self.client.session[SESSION_KEY_PLAN]
|
||||
plan.context[PLAN_CONTEXT_PROMPT] = {
|
||||
PLAN_CONTEXT_PHONE: "1234",
|
||||
}
|
||||
session = self.client.session
|
||||
session[SESSION_KEY_PLAN] = plan
|
||||
session.save()
|
||||
SMSDevice.objects.create(
|
||||
phone_number="1234",
|
||||
user=self.user,
|
||||
stage=self.stage,
|
||||
)
|
||||
sms_send_mock = MagicMock()
|
||||
with (
|
||||
patch(
|
||||
"authentik.stages.authenticator_sms.models.AuthenticatorSMSStage.send",
|
||||
sms_send_mock,
|
||||
),
|
||||
):
|
||||
print(self.client.session[SESSION_KEY_PLAN])
|
||||
response = self.client.get(
|
||||
reverse("authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug}),
|
||||
)
|
||||
print(response.content.decode())
|
||||
self.assertStageResponse(
|
||||
response,
|
||||
self.flow,
|
||||
self.user,
|
||||
component="ak-stage-authenticator-sms",
|
||||
phone_number_required=True,
|
||||
)
|
||||
plan: FlowPlan = self.client.session[SESSION_KEY_PLAN]
|
||||
self.assertEqual(plan.context[PLAN_CONTEXT_PROMPT], {})
|
||||
|
||||
def test_stage_submit_full(self):
|
||||
"""test stage (submit)"""
|
||||
self.client.get(
|
||||
|
|
Reference in New Issue