From cc1509cf5705a90c94973d8ceb94bcc43cec6a04 Mon Sep 17 00:00:00 2001
From: Jens Langhammer <jens.langhammer@beryju.org>
Date: Thu, 10 Mar 2022 00:46:42 +0100
Subject: [PATCH] stages/authenticator_validate: fix logic error when multiple
 authenticator devices can be selected

closes #2290

Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org>
---
 .../stages/authenticator_validate/stage.py    | 14 +++--
 .../stages/authenticator_validate/tests.py    | 51 ++++++++++++++++++-
 2 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/authentik/stages/authenticator_validate/stage.py b/authentik/stages/authenticator_validate/stage.py
index 636cb71b2..2308fcbb0 100644
--- a/authentik/stages/authenticator_validate/stage.py
+++ b/authentik/stages/authenticator_validate/stage.py
@@ -87,16 +87,20 @@ class AuthenticatorValidationChallengeResponse(ChallengeResponse):
     def validate_selected_challenge(self, challenge: dict) -> dict:
         """Check which challenge the user has selected. Actual logic only used for SMS stage."""
         # First check if the challenge is valid
+        allowed = False
         for device_challenge in self.stage.request.session.get(SESSION_DEVICE_CHALLENGES):
-            if device_challenge.get("device_class", "") != challenge.get("device_class", ""):
-                raise ValidationError("invalid challenge selected")
-            if device_challenge.get("device_uid", "") != challenge.get("device_uid", ""):
-                raise ValidationError("invalid challenge selected")
+            if device_challenge.get("device_class", "") == challenge.get(
+                "device_class", ""
+            ) and device_challenge.get("device_uid", "") == challenge.get("device_uid", ""):
+                allowed = True
+        if not allowed:
+            raise ValidationError("invalid challenge selected")
+
         if challenge.get("device_class", "") != "sms":
             return challenge
         devices = SMSDevice.objects.filter(pk=int(challenge.get("device_uid", "0")))
         if not devices.exists():
-            raise ValidationError("device does not exist")
+            raise ValidationError("invalid challenge selected")
         select_challenge(self.stage.request, devices.first())
         return challenge
 
diff --git a/authentik/stages/authenticator_validate/tests.py b/authentik/stages/authenticator_validate/tests.py
index 80b4fd9e0..31962327a 100644
--- a/authentik/stages/authenticator_validate/tests.py
+++ b/authentik/stages/authenticator_validate/tests.py
@@ -1,6 +1,7 @@
 """Test validator stage"""
 from unittest.mock import MagicMock, patch
 
+from django.contrib.sessions.middleware import SessionMiddleware
 from django.test.client import RequestFactory
 from django.urls.base import reverse
 from django_otp.plugins.otp_totp.models import TOTPDevice
@@ -9,10 +10,18 @@ from webauthn.helpers import bytes_to_base64url
 
 from authentik.core.tests.utils import create_test_admin_user
 from authentik.flows.models import Flow, FlowStageBinding, NotConfiguredAction
+from authentik.flows.stage import StageView
 from authentik.flows.tests import FlowTestCase
+from authentik.flows.views.executor import FlowExecutorView
 from authentik.lib.generators import generate_id, generate_key
-from authentik.lib.tests.utils import get_request
+from authentik.lib.tests.utils import dummy_get_response, get_request
 from authentik.stages.authenticator_duo.models import AuthenticatorDuoStage, DuoDevice
+from authentik.stages.authenticator_sms.models import (
+    AuthenticatorSMSStage,
+    SMSAuthTypes,
+    SMSDevice,
+    SMSProviders,
+)
 from authentik.stages.authenticator_validate.api import AuthenticatorValidateStageSerializer
 from authentik.stages.authenticator_validate.challenge import (
     get_challenge_for_device,
@@ -21,6 +30,10 @@ from authentik.stages.authenticator_validate.challenge import (
     validate_challenge_webauthn,
 )
 from authentik.stages.authenticator_validate.models import AuthenticatorValidateStage
+from authentik.stages.authenticator_validate.stage import (
+    SESSION_DEVICE_CHALLENGES,
+    AuthenticatorValidationChallengeResponse,
+)
 from authentik.stages.authenticator_webauthn.models import WebAuthnDevice
 from authentik.stages.identification.models import IdentificationStage, UserFields
 
@@ -159,3 +172,39 @@ class AuthenticatorValidateStageTests(FlowTestCase):
         ):
             with self.assertRaises(ValidationError):
                 validate_challenge_duo(duo_device.pk, request, self.user)
+
+    def test_validate_selected_challenge(self):
+        """Test validate_selected_challenge"""
+        # Prepare request with session
+        request = self.request_factory.get("/")
+
+        middleware = SessionMiddleware(dummy_get_response)
+        middleware.process_request(request)
+        request.session[SESSION_DEVICE_CHALLENGES] = [
+            {
+                "device_class": "static",
+                "device_uid": "1",
+            },
+            {
+                "device_class": "totp",
+                "device_uid": "2",
+            },
+        ]
+        request.session.save()
+
+        res = AuthenticatorValidationChallengeResponse()
+        res.stage = StageView(FlowExecutorView())
+        res.stage.request = request
+        with self.assertRaises(ValidationError):
+            res.validate_selected_challenge(
+                {
+                    "device_class": "baz",
+                    "device_uid": "quox",
+                }
+            )
+        res.validate_selected_challenge(
+            {
+                "device_class": "static",
+                "device_uid": "1",
+            }
+        )