Merge branch 'Aufinal/fix_privilege_escalation' into 'master'

Fix le problème d'auth par mdp K-Fêt

Closes #240

See merge request klub-dev-ens/gestioCOF!380

cof/settings/common.py

@@ -111,7 +111,6 @@ MIDDLEWARE = [
     "django.middleware.common.CommonMiddleware",
     "django.middleware.csrf.CsrfViewMiddleware",
     "django.contrib.auth.middleware.AuthenticationMiddleware",
-    "kfet.auth.middleware.TemporaryAuthMiddleware",
     "django.contrib.messages.middleware.MessageMiddleware",
     "django.middleware.clickjacking.XFrameOptionsMiddleware",
     "django.middleware.security.SecurityMiddleware",

kfet/auth/decorators.py

@@ -0,0 +1,23 @@
+from functools import wraps
+
+from kfet.auth.backends import AccountBackend
+
+
+def kfet_password_auth(view_func):
+    def get_kfet_password(request):
+        return request.META.get("HTTP_KFETPASSWORD") or request.POST.get("KFETPASSWORD")
+
+    @wraps(view_func)
+    def _wrapped_view(request, *args, **kwargs):
+        if request.method == "POST":
+            temp_request_user = AccountBackend().authenticate(
+                request, kfet_password=get_kfet_password(request)
+            )
+
+            if temp_request_user:
+                request.real_user = request.user
+                request.user = temp_request_user
+
+        return view_func(request, *args, **kwargs)
+
+    return _wrapped_view

kfet/auth/middleware.py

@@ -1,37 +0,0 @@
-from django.contrib.auth import get_user_model
-
-from .backends import AccountBackend
-
-User = get_user_model()
-
-
-class TemporaryAuthMiddleware:
-    """Authenticate another user for this request if AccountBackend succeeds.
-
-    By the way, if a user is authenticated, we refresh its from db to add
-    values from CofProfile and Account of this user.
-
-    """
-
-    def __init__(self, get_response):
-        self.get_response = get_response
-
-    def __call__(self, request):
-        if request.user.is_authenticated:
-            # avoid multiple db accesses in views and templates
-            request.user = User.objects.select_related("profile__account_kfet").get(
-                pk=request.user.pk
-            )
-
-        temp_request_user = AccountBackend().authenticate(
-            request, kfet_password=self.get_kfet_password(request)
-        )
-
-        if temp_request_user:
-            request.real_user = request.user
-            request.user = temp_request_user
-
-        return self.get_response(request)
-
-    def get_kfet_password(self, request):
-        return request.META.get("HTTP_KFETPASSWORD") or request.POST.get("KFETPASSWORD")

kfet/auth/tests.py

@@ -10,7 +10,6 @@ from kfet.models import Account
 
 from . import KFET_GENERIC_TRIGRAMME, KFET_GENERIC_USERNAME
 from .backends import AccountBackend, GenericBackend
-from .middleware import TemporaryAuthMiddleware
 from .models import GenericTeamToken
 from .utils import get_kfet_generic_user
 from .views import GenericLoginView
@@ -268,8 +267,6 @@ class TemporaryAuthTests(TestCase):
 
         self.factory = RequestFactory()
 
-        self.middleware = TemporaryAuthMiddleware(mock.Mock())
-
         user1_acc = Account(trigramme="000")
         user1_acc.change_pwd("kfet_user1")
         user1_acc.save({"username": "user1"})
@@ -289,51 +286,13 @@ class TemporaryAuthTests(TestCase):
         )
         self.user2.user_permissions.add(self.perm)
 
-    def test_middleware_header(self):
-        """
-        A user can be authenticated if ``HTTP_KFETPASSWORD`` header of a
-        request contains a valid kfet password.
-        """
-        request = self.factory.get("/", HTTP_KFETPASSWORD="kfet_user2")
-        request.user = self.user1
-
-        self.middleware(request)
-
-        self.assertEqual(request.user, self.user2)
-        self.assertEqual(request.real_user, self.user1)
-
-    def test_middleware_post(self):
-        """
-        A user can be authenticated if ``KFETPASSWORD`` of POST data contains
-        a valid kfet password.
-        """
-        request = self.factory.post("/", {"KFETPASSWORD": "kfet_user2"})
-        request.user = self.user1
-
-        self.middleware(request)
-
-        self.assertEqual(request.user, self.user2)
-        self.assertEqual(request.real_user, self.user1)
-
-    def test_middleware_invalid(self):
-        """
-        The given password must be a password of an Account.
-        """
-        request = self.factory.post("/", {"KFETPASSWORD": "invalid"})
-        request.user = self.user1
-
-        self.middleware(request)
-
-        self.assertEqual(request.user, self.user1)
-        self.assertFalse(hasattr(request, "real_user"))
-
     def test_context_processor(self):
         """
         Context variables give the real authenticated user and his permissions.
         """
         self.client.login(username="user1", password="user1")
 
-        r = self.client.get("/k-fet/accounts/", HTTP_KFETPASSWORD="kfet_user2")
+        r = self.client.post("/k-fet/accounts/000/edit", HTTP_KFETPASSWORD="kfet_user2")
 
         self.assertEqual(r.context["user"], self.user1)
         self.assertNotIn("kfet.is_team", r.context["perms"])
@@ -344,8 +303,10 @@ class TemporaryAuthTests(TestCase):
         """
         self.client.login(username="user1", password="user1")
 
-        r1 = self.client.get("/k-fet/accounts/", HTTP_KFETPASSWORD="kfet_user2")
+        r1 = self.client.post(
+            "/k-fet/accounts/000/edit", HTTP_KFETPASSWORD="kfet_user2"
+        )
         self.assertEqual(r1.wsgi_request.user, self.user2)
 
-        r2 = self.client.get("/k-fet/accounts/")
+        r2 = self.client.post("/k-fet/accounts/000/edit")
         self.assertEqual(r2.wsgi_request.user, self.user1)

kfet/templates/kfet/account_group_form.html

@@ -30,9 +30,6 @@
     </div>
   </div>
   {% include "kfet/form_field_snippet.html" with field=form.permissions %}
-  {% if not perms.kfet.manage_perms %}
-    {% include "kfet/form_authentication_snippet.html" %}
-  {% endif %}
   {% include "kfet/form_submit_snippet.html" with value="Enregistrer" %}
 </form>
 

kfet/views.py

@@ -24,6 +24,7 @@ from django.views.generic.edit import CreateView, DeleteView, UpdateView
 
 from gestioncof.models import CofProfile
 from kfet import KFET_DELETED_TRIGRAMME, consumers
+from kfet.auth.decorators import kfet_password_auth
 from kfet.config import kfet_config
 from kfet.decorators import teamkfet_required
 from kfet.forms import (
@@ -119,6 +120,7 @@ def account_is_validandfree_ajax(request):
 
 @login_required
 @teamkfet_required
+@kfet_password_auth
 def account_create(request):
 
     # Enregistrement
@@ -320,6 +322,7 @@ def account_read(request, trigramme):
 
 
 @login_required
+@kfet_password_auth
 def account_update(request, trigramme):
     account = get_object_or_404(Account, trigramme=trigramme)
 
@@ -518,6 +521,7 @@ class CheckoutList(ListView):
 # Checkout - Create
 
 
+@method_decorator(kfet_password_auth, name="dispatch")
 class CheckoutCreate(SuccessMessageMixin, CreateView):
     model = Checkout
     template_name = "kfet/checkout_create.html"
@@ -629,6 +633,7 @@ def getAmountBalance(data):
     )
 
 
+@method_decorator(kfet_password_auth, name="dispatch")
 class CheckoutStatementCreate(SuccessMessageMixin, CreateView):
     model = CheckoutStatement
     template_name = "kfet/checkoutstatement_create.html"
@@ -665,6 +670,7 @@ class CheckoutStatementCreate(SuccessMessageMixin, CreateView):
         return super().form_valid(form)
 
 
+@method_decorator(kfet_password_auth, name="dispatch")
 class CheckoutStatementUpdate(SuccessMessageMixin, UpdateView):
     model = CheckoutStatement
     template_name = "kfet/checkoutstatement_update.html"
@@ -705,6 +711,7 @@ class CategoryList(ListView):
 
 
 # Category - Update
+@method_decorator(kfet_password_auth, name="dispatch")
 class CategoryUpdate(SuccessMessageMixin, UpdateView):
     model = ArticleCategory
     template_name = "kfet/category_update.html"
@@ -959,6 +966,7 @@ def kpsul_checkout_data(request):
 
 
 @teamkfet_required
+@kfet_password_auth
 def kpsul_update_addcost(request):
     addcost_form = AddcostForm(request.POST)
 
@@ -996,6 +1004,7 @@ def get_missing_perms(required_perms, user):
 
 
 @teamkfet_required
+@kfet_password_auth
 def kpsul_perform_operations(request):
     # Initializing response data
     data = {"operationgroup": 0, "operations": [], "warnings": {}, "errors": {}}
@@ -1187,6 +1196,7 @@ def kpsul_perform_operations(request):
 
 
 @teamkfet_required
+@kfet_password_auth
 def kpsul_cancel_operations(request):
     # Pour la réponse
     data = {"canceled": [], "warnings": {}, "errors": {}}
@@ -1545,6 +1555,7 @@ def transfers_create(request):
 
 
 @teamkfet_required
+@kfet_password_auth
 def perform_transfers(request):
     data = {"errors": {}, "transfers": [], "transfergroup": 0}
 
@@ -1626,6 +1637,7 @@ def perform_transfers(request):
 
 
 @teamkfet_required
+@kfet_password_auth
 def cancel_transfers(request):
     # Pour la réponse
     data = {"canceled": [], "warnings": {}, "errors": {}}
@@ -1739,6 +1751,7 @@ class InventoryList(ListView):
 
 
 @teamkfet_required
+@kfet_password_auth
 def inventory_create(request):
 
     articles = Article.objects.select_related("category").order_by(
@@ -1833,6 +1846,7 @@ class OrderList(ListView):
 
 
 @teamkfet_required
+@kfet_password_auth
 def order_create(request, pk):
     supplier = get_object_or_404(Supplier, pk=pk)
 
@@ -1985,6 +1999,7 @@ class OrderRead(DetailView):
 
 
 @teamkfet_required
+@kfet_password_auth
 def order_to_inventory(request, pk):
     order = get_object_or_404(Order, pk=pk)
 
@@ -2092,6 +2107,7 @@ def order_to_inventory(request, pk):
     )
 
 
+@method_decorator(kfet_password_auth, name="dispatch")
 class SupplierUpdate(SuccessMessageMixin, UpdateView):
     model = Supplier
     template_name = "kfet/supplier_form.html"

requirements.txt

@@ -19,6 +19,6 @@ ldap3
 channels==1.1.5
 python-dateutil
 wagtail==2.4.*
-wagtailmenus==2.12.*
+wagtailmenus<3
 wagtail-modeltranslation==0.10.*
 django-cors-headers==2.2.0