From c0fe99714f8ee782cb6838d093e88e263de1bc2c Mon Sep 17 00:00:00 2001 From: Jens L Date: Tue, 24 Oct 2023 02:42:16 +0200 Subject: [PATCH] stages/authenticator_sms: fix error when phone number from context already exists (#7264) --- authentik/stages/authenticator_sms/stage.py | 24 +++++++---- authentik/stages/authenticator_sms/tests.py | 44 ++++++++++++++++++++- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/authentik/stages/authenticator_sms/stage.py b/authentik/stages/authenticator_sms/stage.py index c4e0e16b7..dfbf48c68 100644 --- a/authentik/stages/authenticator_sms/stage.py +++ b/authentik/stages/authenticator_sms/stage.py @@ -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: diff --git a/authentik/stages/authenticator_sms/tests.py b/authentik/stages/authenticator_sms/tests.py index 4b37df0e5..9601cf886 100644 --- a/authentik/stages/authenticator_sms/tests.py +++ b/authentik/stages/authenticator_sms/tests.py @@ -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(