From b4c89f009a7eea1dd94b883eae057a81646f8aa8 Mon Sep 17 00:00:00 2001
From: Johan Castiblanco <johan.castiblanco@edunext.co>
Date: Mon, 11 Dec 2023 12:40:34 -0500
Subject: [PATCH] refactor: change uid  pipeline auth exception to 403

This change the exception raised to a 403 msg.

refactor: update test to the response 403

This update the test to match the saml association with
a message in 403 forbidden view.

chore: pr assert recommendations

chore: pr change msg support

chore: pr feedback semantic
---
 eox_nelp/locale/ar/LC_MESSAGES/django.mo      | Bin 536 -> 1211 bytes
 eox_nelp/locale/ar/LC_MESSAGES/django.po      |  14 +++++
 eox_nelp/third_party_auth/pipeline.py         |  22 +++++--
 .../third_party_auth/tests/test_pipeline.py   |  55 ++++++++++--------
 4 files changed, 63 insertions(+), 28 deletions(-)

diff --git a/eox_nelp/locale/ar/LC_MESSAGES/django.mo b/eox_nelp/locale/ar/LC_MESSAGES/django.mo
index c8f3020b453217a78341c1743a6b57d43df2d479..92dd762f8af5318e85995f4a5b89c72572efc226 100644
GIT binary patch
literal 1211
zcmY*W%We}%6m8zfNV8x)i@SjhYGd;l1&0O#F(MM4M8P1zuIMh?h1ykCRW*s1pd^^E
zU;|$eK@tlv0Y#Achhbg4nQxg>Z9_syr|Uk?J-7P5(?{P1ILClLfmz@LkOKY;fD-T$
zm;&Aar+|NfA3zSgJ`@C7z;W19M}i>6eI55n+#9(6K<)!>T)|=BDBur5#qqO)y0lzV
zUu8O&vm^{jWr?M!TH*;=MJ)3lV^l1d<zy^twMc1VNmoe94A&;O$5S5LG08)f5!0Nk
z?hDy27~+^4LrcPTh!q`VJIoK!oF#qY%fi@6zNf`gk@7ZX_qgr~V}z2XObaPzrXZbY
z>TqdA>=h{qItUZ6mpUAyu?%V61}nUTT2SjNT0$F3R&>#$DOS=lh&EZTr!?Y<;trE-
z{@dVSL8~X&Eu2fj2UrzqYQ)uYD0$i|Epp!)HcVS2;mxdV!ltTGX<<HG<WGe!4rd{;
zMi1*Vi(z9voSmUN^R>p@xkvMV*GqSqv0+m)X;NmDuF;H4H1E?bCb862R(R#udpCHJ
zNxp2$vATvn+0Iy-hfU79=+f-5h<`<Ec>Ypju2J6?TrF2hx3DkXENu3B5M;r1zLzo~
zr~TU6a2wrk-VUes>%6WO*I`}8D#5R;(bQvMOABeHSsLC}x@&4AdxdMFzouzyMly;j
zbe$@MY!p>tR>$TAm>0Zxrdp|7i!RdHvt#>eRP`Mf_jA?if#j7dNM1tn@^mS1AKl1}
zf_%koQoim!<OBDG+>YCF-^gv|EBPuS-}7hrdcH<(BOgqn?yDQRPi};KkwK|@2dYiC
zgFq1)qWd`l@8Q03n}ORv{!_jJBVThv0>MX&800Hs&Vk!4oP5>o5k9{rz<~%aZA1{}
zRXuZizHJMfpV732F+*PlHSUYsF5}uk$4k#M8RE8oO5a8G2+|M$3yOX&ir8M^fOMVm
z=V*q^Vj{$T1>riT7LW0QurQv(`>U9>J$W0{?T(fC7a3u|s;?ZnfBdt!oiY6Z)#c!S
Diq!HU

delta 147
zcmdnZIfJGCo)F7a1|VPtVi_Pd0b*7l_5orLNC09kAWj5gZXnJ9;(kU3h6*6f2E-gp
zKn$e$fizGU3_xaq08og5!8NZWwMd~dzqE)UI5j6Vxdg%vD9X<QayGALoWRI=WATmk
KH`d*3VE_QEB^i4F

diff --git a/eox_nelp/locale/ar/LC_MESSAGES/django.po b/eox_nelp/locale/ar/LC_MESSAGES/django.po
index ea70e092..6172b60d 100644
--- a/eox_nelp/locale/ar/LC_MESSAGES/django.po
+++ b/eox_nelp/locale/ar/LC_MESSAGES/django.po
@@ -26,3 +26,17 @@ msgstr "أدخل"
 #: eox_nelp/middleware.py:111
 msgid "Enter your"
 msgstr "حدد"
+
+#: eox_nelp/third_party_auth/pipeline.py:99
+msgid "It is not allowed to auto associate staff or admin users"
+msgstr "لا يجوز ربط الموظفين أو المستخدمين الإداريين تلقائيًا"
+
+#: eox_nelp/third_party_auth/pipeline.py:99
+msgid ""
+"You are trying to access with a user that already exists with privileged permissions."
+" Please try to authenticate on the site where you registered for the first time "
+"or contact support to change permissions."
+msgstr ""
+"أنت تحاول الوصول مع مستخدم موجود بالفعل بأذونات مميزة."
+"يرجى محاولة المصادقة على الموقع الذي قمت بالتسجيل فيه لأول مرة"
+"أو اتصل بالدعم لتغيير الأذونات."
diff --git a/eox_nelp/third_party_auth/pipeline.py b/eox_nelp/third_party_auth/pipeline.py
index fa43c2fe..29e9ebcb 100644
--- a/eox_nelp/third_party_auth/pipeline.py
+++ b/eox_nelp/third_party_auth/pipeline.py
@@ -6,9 +6,11 @@
 """
 from django.conf import settings
 from django.contrib.auth import logout
+from django.http import HttpResponseForbidden
+from django.utils.translation import gettext_lazy as _
 from social_core.pipeline.social_auth import social_details as social_core_details
 
-from eox_nelp.third_party_auth.exceptions import EoxNelpAuthException
+from eox_nelp.edxapp_wrapper.edxmako import edxmako
 
 
 def social_details(backend, details, response, *args, **kwargs):
@@ -70,7 +72,7 @@ def close_mismatch_session(request, *args, user=None, **kwargs):  # pylint: disa
 
 
 def safer_associate_username_by_uid(  # pylint: disable=unused-argument
-    backend, details, response, *args, user=None, **kwargs,
+    request, backend, details, response, *args, user=None, **kwargs,
 ):
     """Pipeline to retrieve user if possible matching uid with the username of a user.
     The uid is based in the configuration of the saml with the field `attr_user_permanent_id`:
@@ -94,8 +96,20 @@ def safer_associate_username_by_uid(  # pylint: disable=unused-argument
     if not user_match:
         return None
     if user_match.is_staff or user_match.is_superuser:
-        raise EoxNelpAuthException(backend, "It is not allowed to auto associate staff or admin users")
-
+        return HttpResponseForbidden(
+            edxmako.shortcuts.render_to_string(
+                "static_templates/server-error.html",
+                {
+                    "page_header": _("It is not allowed to auto associate staff or admin users"),
+                    "page_content": _(
+                        "You are trying to access with a user that already exists with privileged permissions."
+                        " Please try to authenticate on the site where you registered for the first time "
+                        "or contact support to change permissions."
+                    ),
+                },
+                request=request,
+            )
+        )
     return {
         "user": user_match,
         "is_new": False,
diff --git a/eox_nelp/third_party_auth/tests/test_pipeline.py b/eox_nelp/third_party_auth/tests/test_pipeline.py
index 14933589..1e8bfdc1 100644
--- a/eox_nelp/third_party_auth/tests/test_pipeline.py
+++ b/eox_nelp/third_party_auth/tests/test_pipeline.py
@@ -1,10 +1,11 @@
 """ Test file for third_party_auth pipeline functions."""
 from ddt import data, ddt
 from django.contrib.auth import get_user_model
+from django.http import HttpResponseForbidden
 from django.test import TestCase
 from mock import Mock
+from rest_framework import status
 
-from eox_nelp.third_party_auth.exceptions import EoxNelpAuthException
 from eox_nelp.third_party_auth.pipeline import safer_associate_username_by_uid
 
 User = get_user_model()
@@ -24,70 +25,76 @@ def setUp(self):  # pylint: disable=invalid-name
             "attributes": {},
         }
         self.user, _ = User.objects.get_or_create(username="vader")
+        self.backend = Mock()
+        self.request = Mock()
 
     def test_user_already_matched(self):
         """Test the pipeline method is called with already matched user.
         Expected behavior:
             - Return None.
         """
-        backend = Mock()
-
-        pipe_output = safer_associate_username_by_uid(backend, self.details, self.response, user=self.user)
+        pipe_output = safer_associate_username_by_uid(
+            self.request, self.backend, self.details, self.response, user=self.user,
+        )
 
-        self.assertEqual(None, pipe_output)
+        self.assertIsNone(pipe_output)
 
     def test_user_not_associated(self):
         """Test the pipeline method try to match with uid but there is not user with that username.
         Expected behavior:
-            - Pipe Return None.
+            - Pipe method returns None.
             - Strategy storage get_user method is called with desired params.
         """
         test_uid = "1888222999"
-        backend = Mock()
-        backend.get_idp.return_value.get_user_permanent_id.return_value = test_uid
-        backend.strategy.storage.user.get_user.return_value = None
 
-        pipe_output = safer_associate_username_by_uid(backend, self.details, self.response)
+        self.backend.get_idp.return_value.get_user_permanent_id.return_value = test_uid
+        self.backend.strategy.storage.user.get_user.return_value = None
+
+        pipe_output = safer_associate_username_by_uid(self.request, self.backend, self.details, self.response)
 
         self.assertEqual(None, pipe_output)
-        backend.strategy.storage.user.get_user.assert_called_with(username=test_uid)
+        self.backend.strategy.storage.user.get_user.assert_called_with(username=test_uid)
 
     @data(
         {"is_staff": True, "username": "1222333444"},
         {"is_superuser": True, "username": "1222333555"},
     )
-    def test_staff_user_raise_exc(self, user_kwargs):
+    def test_staff_user_return_forbidden(self, user_kwargs):
         """Test the pipeline method try to match with uid, the user with matched username exists
         but is staff or superuser.
         Expected behavior:
-            - Raise EoxNelpAuthException exception
+            - The pipe method returns an HttpResponseForbidden object.
+            - The pipe method response has a 403 forbidden status.
             - Strategy storage get_user method is called with desired params.
         """
         test_uid = user_kwargs["username"]
         past_user, _ = User.objects.get_or_create(**user_kwargs)
-        backend = Mock()
-        backend.get_idp.return_value.get_user_permanent_id.return_value = test_uid
-        backend.strategy.storage.user.get_user.return_value = past_user
 
-        self.assertRaises(EoxNelpAuthException, safer_associate_username_by_uid, backend, self.details, self.response)
-        backend.strategy.storage.user.get_user.assert_called_with(username=test_uid)
+        self.backend.get_idp.return_value.get_user_permanent_id.return_value = test_uid
+        self.backend.strategy.storage.user.get_user.return_value = past_user
+
+        pipe_output = safer_associate_username_by_uid(self.request, self.backend, self.details, self.response)
+
+        self.assertIsInstance(pipe_output, HttpResponseForbidden)
+        self.assertEqual(status.HTTP_403_FORBIDDEN, pipe_output.status_code)
+        self.backend.strategy.storage.user.get_user.assert_called_with(username=test_uid)
 
     def test_user_associate_username_with_uid(self):
         """Test the pipeline method try to match with uid, the user with matched username exists
         and is returned.
         Expected behavior:
             - Strategy storage get_user method is called with desired params.
-            - The method return the desirect with `user` and `is_new` keys.
+            - The method returns the desirect with `user` and `is_new` keys.
         """
         test_uid = "1777888999"
         past_user, _ = User.objects.get_or_create(
             username=test_uid,
         )
-        backend = Mock()
-        backend.get_idp.return_value.get_user_permanent_id.return_value = test_uid
-        backend.strategy.storage.user.get_user.return_value = past_user
 
-        pipe_output = safer_associate_username_by_uid(backend, self.details, self.response)
+        self.backend.get_idp.return_value.get_user_permanent_id.return_value = test_uid
+        self.backend.strategy.storage.user.get_user.return_value = past_user
+
+        pipe_output = safer_associate_username_by_uid(self.request, self.backend, self.details, self.response)
 
         self.assertEqual({"user": past_user, "is_new": False}, pipe_output)
-        backend.strategy.storage.user.get_user.assert_called_with(username=test_uid)
+        self.backend.strategy.storage.user.get_user.assert_called_with(username=test_uid)