Repository: merixstudio/django-trench (language: python)

Issue:
`/mfa/change-primary-method/` accepts only the MFA code from the primary method
**Describe the bug**
When switching the primary MFA method, there is an issue, which is either an error in the code or in the documentation. Which one of them it is, is not really for me to decide

**To Reproduce**
Steps to reproduce the behavior:
Configure both `app` and `email` methods and have app as the primary one
Send a request to `/mfa/change-primary-method/` with the following content:
```
{"method":"email","code":"123456"}
```
The code `123456` is given from the email sent by making a request to `/code/request/` with the following content:
```
{"method": "email"}
```
Upon submitting the request to `/mfa/change-primary-method/` we are returned error code 400 and the following response:
```
{"code":["Code invalid or expired."]}
```
Sending the exact same request, but using a code from the `app` method, will complete the request with status code 204

**Expected behavior**
One of the below would be expected
1. Verification works as the documentation, where you have to provide a code from the new method to verify that it works
2. Verification works as the above reproduction where you provide the new method and confirm it with a code from the current method. Documentation would need to be updated in this case. This method is also what I see in other applications


Discussion / hints:

Fix patch:
diff --git a/trench/serializers.py b/trench/serializers.py
index c5d0a169..8bf0749d 100644
--- a/trench/serializers.py
+++ b/trench/serializers.py
@@ -150,14 +150,16 @@ class Meta:
         fields = ("name", "is_primary")
 
 
-class ChangePrimaryMethodValidator(ProtectedActionValidator):
-    method = ChoiceField(choices=available_method_choices())
-
+class ChangePrimaryMethodCodeValidator(ProtectedActionValidator):
     @staticmethod
     def _validate_mfa_method(mfa: MFAMethod) -> None:
         pass
 
 
+class ChangePrimaryMethodValidator(RequestBodyValidator):
+    method = ChoiceField(choices=available_method_choices())
+
+
 class TokenSerializer(ModelSerializer):
     auth_token = CharField(source="key")
 
diff --git a/trench/views/base.py b/trench/views/base.py
index 699a9739..2d81582a 100644
--- a/trench/views/base.py
+++ b/trench/views/base.py
@@ -29,6 +29,7 @@
 from trench.query.get_mfa_config_by_name import get_mfa_config_by_name_query
 from trench.responses import ErrorResponse
 from trench.serializers import (
+    ChangePrimaryMethodCodeValidator,
     ChangePrimaryMethodValidator,
     CodeLoginSerializer,
     LoginSerializer,
@@ -235,17 +236,18 @@ class MFAPrimaryMethodChangeView(APIView):
 
     @staticmethod
     def post(request: Request) -> Response:
-        mfa_model = get_mfa_model()
-        mfa_method_name = mfa_model.objects.get_primary_active_name(
-            user_id=request.user.id
-        )
-        serializer = ChangePrimaryMethodValidator(
-            user=request.user, mfa_method_name=mfa_method_name, data=request.data
+        method_serializer = ChangePrimaryMethodValidator(data=request.data)
+        method_serializer.is_valid(raise_exception=True)
+
+        code_serializer = ChangePrimaryMethodCodeValidator(
+            user=request.user,
+            mfa_method_name=method_serializer.validated_data["method"],
+            data=request.data,
         )
-        serializer.is_valid(raise_exception=True)
+        code_serializer.is_valid(raise_exception=True)
         try:
             set_primary_mfa_method_command(
-                user_id=request.user.id, name=serializer.validated_data["method"]
+                user_id=request.user.id, name=method_serializer.validated_data["method"]
             )
             return Response(status=HTTP_204_NO_CONTENT)
         except MFAValidationError as cause:


Test patch:
diff --git a/testproject/tests/test_second_step_authentication.py b/testproject/tests/test_second_step_authentication.py
index b5e48dc1..bdb3c204 100644
--- a/testproject/tests/test_second_step_authentication.py
+++ b/testproject/tests/test_second_step_authentication.py
@@ -345,13 +345,14 @@ def test_deactivation_of_disabled_method(
 def test_change_primary_method(active_user_with_many_otp_methods):
     active_user, _ = active_user_with_many_otp_methods
     client = TrenchAPIClient()
-    primary_mfa = active_user.mfa_methods.filter(is_primary=True).first()
-    handler = get_mfa_handler(mfa_method=primary_mfa)
-    client.authenticate_multi_factor(mfa_method=primary_mfa, user=active_user)
+    primary_mfa_method = active_user.mfa_methods.filter(is_primary=True).first()
+    sms_twilio_mfa_method = active_user.mfa_methods.filter(name="sms_twilio").first()
+    handler = get_mfa_handler(mfa_method=sms_twilio_mfa_method)
+    client.authenticate_multi_factor(mfa_method=primary_mfa_method, user=active_user)
     response = client.post(
         path="/auth/mfa/change-primary-method/",
         data={
-            "method": "sms_twilio",
+            "method": sms_twilio_mfa_method.name,
             "code": handler.create_code(),
         },
         format="json",
@@ -360,14 +361,14 @@ def test_change_primary_method(active_user_with_many_otp_methods):
         is_primary=True,
     ).first()
     assert response.status_code == HTTP_204_NO_CONTENT
-    assert primary_mfa != new_primary_method
-    assert new_primary_method.name == "sms_twilio"
+    assert primary_mfa_method != new_primary_method
+    assert new_primary_method.name == sms_twilio_mfa_method.name
 
     # revert changes
     new_primary_method.is_primary = False
     new_primary_method.save()
-    primary_mfa.is_primary = True
-    primary_mfa.save()
+    primary_mfa_method.is_primary = True
+    primary_mfa_method.save()
 
 
 @flaky
@@ -378,11 +379,12 @@ def test_change_primary_method_with_backup_code(
     active_user, backup_code = active_user_with_many_otp_methods
     client = TrenchAPIClient()
     primary_mfa_method = active_user.mfa_methods.filter(is_primary=True).first()
+    sms_twilio_mfa_method_name = "sms_twilio"
     client.authenticate_multi_factor(mfa_method=primary_mfa_method, user=active_user)
     response = client.post(
         path="/auth/mfa/change-primary-method/",
         data={
-            "method": "sms_twilio",
+            "method": sms_twilio_mfa_method_name,
             "code": backup_code,
         },
         format="json",
@@ -392,7 +394,7 @@ def test_change_primary_method_with_backup_code(
     ).first()
     assert response.status_code == HTTP_204_NO_CONTENT
     assert primary_mfa_method != new_primary_method
-    assert new_primary_method.name == "sms_twilio"
+    assert new_primary_method.name == sms_twilio_mfa_method_name
 
     # revert changes
     primary_mfa_method.is_primary = True
@@ -440,7 +442,7 @@ def test_change_primary_method_to_inactive(active_user_with_email_otp):
         format="json",
     )
     assert response.status_code == HTTP_400_BAD_REQUEST
-    assert response.data.get("error") == "Requested MFA method does not exist."
+    assert response.data.get("code")[0].code == "mfa_method_does_not_exist"
 
 
 @pytest.mark.django_db