Merge branch 'Aufinal/account_update_forms' into 'master'

Refactor la vue `account_update`

Closes #232 and #119

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

kfet/auth/tests.py

@@ -284,7 +284,11 @@ class TemporaryAuthTests(TestCase):
         self.perm = Permission.objects.get(
             content_type__app_label="kfet", codename="is_team"
         )
-        self.user2.user_permissions.add(self.perm)
+        self.perm2 = Permission.objects.get(
+            content_type__app_label="kfet", codename="can_force_close"
+        )
+        self.user1.user_permissions.add(self.perm)
+        self.user2.user_permissions.add(self.perm, self.perm2)
 
     def test_context_processor(self):
         """
@@ -295,7 +299,7 @@ class TemporaryAuthTests(TestCase):
         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"])
+        self.assertNotIn("kfet.can_force_close", r.context["perms"])
 
     def test_auth_not_persistent(self):
         """

kfet/forms.py

@@ -85,31 +85,39 @@ class AccountNoTriForm(AccountForm):
         exclude = ["trigramme"]
 
 
-class AccountRestrictForm(AccountForm):
-    class Meta(AccountForm.Meta):
-        fields = ["is_frozen"]
-
-
 class AccountPwdForm(forms.Form):
     pwd1 = forms.CharField(
         label="Mot de passe K-Fêt",
         required=False,
         help_text="Le mot de passe doit contenir au moins huit caractères",
-        widget=forms.PasswordInput,
+        widget=forms.PasswordInput(attrs={"autocomplete": "new-password"}),
+        min_length=8,
     )
     pwd2 = forms.CharField(
-        label="Confirmer le mot de passe", required=False, widget=forms.PasswordInput
+        label="Confirmer le mot de passe",
+        required=False,
+        widget=forms.PasswordInput(attrs={"autocomplete": "new-password"}),
     )
 
+    def __init__(self, *args, account=None, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.account = account
+
     def clean(self):
         pwd1 = self.cleaned_data.get("pwd1", "")
         pwd2 = self.cleaned_data.get("pwd2", "")
-        if len(pwd1) < 8:
-            raise ValidationError("Mot de passe trop court")
         if pwd1 != pwd2:
-            raise ValidationError("Les mots de passes sont différents")
+            self.add_error("pwd2", "Les mots de passe doivent être identiques !")
         super().clean()
 
+    def save(self, commit=True):
+        password = self.cleaned_data["pwd1"]
+        self.account.change_pwd(password)
+        if commit:
+            self.account.save()
+
+        return self.account
+
 
 class CofForm(forms.ModelForm):
     def clean_is_cof(self):

kfet/static/kfet/css/index.css

@@ -41,10 +41,19 @@
 }
 
 .frozen-account {
-    background:#5072e0;
+    background:#000FBA;
     color:#fff;
 }
 
+.frozen-account .btn-default {
+    color: #aaa;
+}
+
+.frozen-account .btn-default:hover, .frozen-account .btn-default.focus, 
+.frozen-account .btn-default:focus {
+    color: #ed2545;
+}
+
 
 .main .table a:not(.btn) {
     color: inherit;

kfet/static/kfet/js/account.js

@@ -69,6 +69,8 @@ var AccountView = Backbone.View.extend({
     attr_data_balance: function () {
         if (this.model.id == 0) {
             return '';
+        } else if (this.model.get("is_frozen")) {
+            return "frozen";
         } else if (this.model.get("balance") < 0) {
             return 'neg';
         } else if (this.model.get("balance") <= 5) {

kfet/templates/kfet/account_update.html

@@ -6,29 +6,29 @@
 
 {% block title %}
 {% if account.user == request.user %}
-    Modification de mes informations
+Modification de mes informations
 {% else %}
-    {{ account.trigramme }} - Édition
+{{ account.trigramme }} - Édition
 {% endif %}
 {% endblock %}
 
 {% block header-title %}
 {% if account.user == request.user %}
-    Modification de mes informations
+Modification de mes informations
 {% else %}
-    Édition du compte {{ account.trigramme }}
+Édition du compte {{ account.trigramme }}
 {% endif %}
 {% endblock %}
 
 {% block footer %}
 {% if not account.is_team %}
-  {% include "kfet/base_footer.html" %}
+{% include "kfet/base_footer.html" %}
 {% endif %}
 {% endblock %}
 
 {% block main %}
 
-<form action="" method="post" class="form-horizontal">
+<form action="" method="post" class="form-horizontal" autocomplete="off">
   {% csrf_token %}
   {% include 'kfet/form_snippet.html' with form=user_info_form %}
   {% include 'kfet/form_snippet.html' with form=account_form %}
@@ -36,21 +36,21 @@
   {% include 'kfet/form_snippet.html' with form=pwd_form %}
   {% include 'kfet/form_snippet.html' with form=negative_form %}
   {% if perms.kfet.is_team %}
-    {% include 'kfet/form_authentication_snippet.html' %}
+  {% include 'kfet/form_authentication_snippet.html' %}
   {% endif %}
   {% include 'kfet/form_submit_snippet.html' with value="Mettre à jour" %}
 </form>
 
 <script type="text/javascript">
 
-$(document).ready(function() {
+  $(document).ready(function () {
     $('#id_authz_overdraft_until').datetimepicker({
-        format: 'YYYY-MM-DD HH:mm',
+      format: 'YYYY-MM-DD HH:mm',
-        stepping: 5,
+      stepping: 5,
-        locale: 'fr',
+      locale: 'fr',
     });
-});
+  });
 
 </script>
 
-{% endblock %}
+{% endblock %}

kfet/templates/kfet/left_account.html

@@ -11,13 +11,11 @@
     </div>
   </div>
 
+  {% if perms.kfet.is_team %}
   <div class="buttons">
     <a class="btn btn-default" href="{% url 'kfet.account.update' account.trigramme %}">
       <span class="glyphicon glyphicon-cog"></span><span>Éditer</span>
     </a>
-    <a class="btn btn-default" disabled>
-      <span class="glyphicon glyphicon-credit-card"></span><span>Créditer</span>
-    </a>
     {% if perms.kfet.delete_account %}
     <hr>
     <button class="btn btn-default" id="button-delete">
@@ -28,6 +26,7 @@
     </form>
     {% endif %}
   </div>
+  {% endif %}
 
   <div class="text">
     <h4>{{ account.name|title }}</h4>

kfet/tests/test_views.py

@@ -11,6 +11,7 @@ from django.utils import timezone
 from .. import KFET_DELETED_TRIGRAMME
 from ..auth import KFET_GENERIC_TRIGRAMME
 from ..auth.models import KFetGroup
+from ..auth.utils import hash_password
 from ..config import kfet_config
 from ..models import (
     Account,
@@ -296,8 +297,8 @@ class AccountReadViewTests(ViewTestCaseMixin, TestCase):
 
 class AccountUpdateViewTests(ViewTestCaseMixin, TestCase):
     url_name = "kfet.account.update"
-    url_kwargs = {"trigramme": "001"}
+    url_kwargs = {"trigramme": "100"}
-    url_expected = "/k-fet/accounts/001/edit"
+    url_expected = "/k-fet/accounts/100/edit"
 
     http_methods = ["GET", "POST"]
 
@@ -317,26 +318,16 @@ class AccountUpdateViewTests(ViewTestCaseMixin, TestCase):
         "promo": "",
         # 'is_frozen': not checked
         # Account password
-        "pwd1": "",
+        "pwd1": "changed_pwd",
-        "pwd2": "",
+        "pwd2": "changed_pwd",
     }
 
     def get_users_extra(self):
         return {
-            "user1": create_user("user1", "001"),
             "team1": create_team("team1", "101", perms=["kfet.change_account"]),
+            "team2": create_team("team2", "102"),
         }
 
-    # Users with forbidden access users should get a 404 here, to avoid leaking trigrams
-    # See issue #224
-    def test_forbidden(self):
-        for method in ["get", "post"]:
-            for user in self.auth_forbidden:
-                self.assertRedirectsToLoginOr404(user, method, self.url_expected)
-                self.assertRedirectsToLoginOr404(
-                    user, method, "/k-fet/accounts/NEX/edit"
-                )
-
     def assertRedirectsToLoginOr404(self, user, method, url):
         client = Client()
         meth = getattr(client, method)
@@ -356,46 +347,55 @@ class AccountUpdateViewTests(ViewTestCaseMixin, TestCase):
         r = self.client.get(self.url)
         self.assertEqual(r.status_code, 200)
 
-    def test_get_ok_self(self):
-        client = Client()
-        client.login(username="user1", password="user1")
-        r = client.get(self.url)
-        self.assertEqual(r.status_code, 200)
-
     def test_post_ok(self):
         client = Client()
         client.login(username="team1", password="team1")
 
-        r = client.post(self.url, self.post_data)
+        r = client.post(self.url, self.post_data, follow=True)
         self.assertRedirects(r, reverse("kfet.account.read", args=["051"]))
 
-        self.accounts["user1"].refresh_from_db()
+        # Comportement attendu : compte modifié,
-        self.users["user1"].refresh_from_db()
+        # utilisateur/mdp inchangé, warning pour le mdp
+
+        self.accounts["team"].refresh_from_db()
+        self.users["team"].refresh_from_db()
 
         self.assertInstanceExpected(
-            self.accounts["user1"],
+            self.accounts["team"],
-            {"first_name": "first", "last_name": "last", "trigramme": "051"},
+            {"first_name": "team", "last_name": "member", "trigramme": "051"},
+        )
+        self.assertEqual(self.accounts["team"].password, hash_password("kfetpwd_team"))
+
+        self.assertTrue(
+            any("mot de passe" in str(msg).casefold() for msg in r.context["messages"])
         )
 
     def test_post_ok_self(self):
-        client = Client()
+        r = self.client.post(self.url, self.post_data, follow=True)
-        client.login(username="user1", password="user1")
+        self.assertRedirects(r, reverse("kfet.account.read", args=["051"]))
 
-        post_data = {"first_name": "The first", "last_name": "The last"}
+        self.accounts["team"].refresh_from_db()
+        self.users["team"].refresh_from_db()
 
-        r = client.post(self.url, post_data)
+        # Comportement attendu : compte/mdp modifié, utilisateur inchangé
-        self.assertRedirects(r, reverse("kfet.account.read", args=["001"]))
-
-        self.accounts["user1"].refresh_from_db()
-        self.users["user1"].refresh_from_db()
 
         self.assertInstanceExpected(
-            self.accounts["user1"], {"first_name": "first", "last_name": "last"}
+            self.accounts["team"],
+            {"first_name": "team", "last_name": "member", "trigramme": "051"},
         )
+        self.assertEqual(self.accounts["team"].password, hash_password("changed_pwd"))
 
     def test_post_forbidden(self):
-        r = self.client.post(self.url, self.post_data)
+        client = Client()
-        self.assertForbiddenKfet(r)
+        client.login(username="team2", password="team2")
+        r = client.post(self.url, self.post_data)
+
+        self.assertTrue(
+            any(
+                "permission refusée" in str(msg).casefold()
+                for msg in r.context["messages"]
+            )
+        )
 
 
 class AccountDeleteViewTests(ViewTestCaseMixin, TestCase):

kfet/views.py

@@ -16,7 +16,12 @@ from django.core.exceptions import SuspiciousOperation
 from django.db import transaction
 from django.db.models import Count, F, Max, OuterRef, Prefetch, Q, Subquery, Sum
 from django.forms import formset_factory
-from django.http import Http404, HttpResponseBadRequest, JsonResponse
+from django.http import (
+    Http404,
+    HttpResponseBadRequest,
+    HttpResponseForbidden,
+    JsonResponse,
+)
 from django.shortcuts import get_object_or_404, redirect, render
 from django.urls import reverse, reverse_lazy
 from django.utils import timezone
@@ -36,7 +41,6 @@ from kfet.forms import (
     AccountNegativeForm,
     AccountNoTriForm,
     AccountPwdForm,
-    AccountRestrictForm,
     AccountStatForm,
     AccountTriForm,
     AddcostForm,
@@ -332,109 +336,89 @@ def account_read(request, trigramme):
 # Account - Update
 
 
-@login_required
+@teamkfet_required
 @kfet_password_auth
 def account_update(request, trigramme):
     account = get_object_or_404(Account, trigramme=trigramme)
 
     # Checking permissions
-    if not account.editable or (
+    if not account.editable:
-        not request.user.has_perm("kfet.is_team") and request.user != account.user
+        # Plus de leak de trigramme !
-    ):
+        return HttpResponseForbidden
-        raise Http404
 
     user_info_form = UserInfoForm(instance=account.user)
 
-    if request.user.has_perm("kfet.is_team"):
+    group_form = UserGroupForm(instance=account.user)
-        group_form = UserGroupForm(instance=account.user)
+    account_form = AccountForm(instance=account)
-        account_form = AccountForm(instance=account)
+    pwd_form = AccountPwdForm()
-        pwd_form = AccountPwdForm()
+    if hasattr(account, "negative"):
-        if account.balance < 0 and not hasattr(account, "negative"):
+        negative_form = AccountNegativeForm(instance=account.negative)
-            AccountNegative.objects.create(account=account, start=timezone.now())
-            account.refresh_from_db()
-        if hasattr(account, "negative"):
-            negative_form = AccountNegativeForm(instance=account.negative)
-        else:
-            negative_form = None
     else:
-        account_form = AccountRestrictForm(instance=account)
-        group_form = None
         negative_form = None
-        pwd_form = None
 
     if request.method == "POST":
-        # Update attempt
+        self_update = request.user == account.user
-        success = False
+        account_form = AccountForm(request.POST, instance=account)
-        missing_perm = True
+        group_form = UserGroupForm(request.POST, instance=account.user)
+        pwd_form = AccountPwdForm(request.POST, account=account)
 
-        if request.user.has_perm("kfet.is_team"):
+        forms = []
-            account_form = AccountForm(request.POST, instance=account)
+        warnings = []
-            group_form = UserGroupForm(request.POST, instance=account.user)
-            pwd_form = AccountPwdForm(request.POST)
-            if hasattr(account, "negative"):
-                negative_form = AccountNegativeForm(
-                    request.POST, instance=account.negative
-                )
 
-            if request.user.has_perm("kfet.change_account") and account_form.is_valid():
+        if self_update or request.user.has_perm("kfet.change_account"):
-                missing_perm = False
+            forms.append(account_form)
+        elif account_form.has_changed():
+            warnings.append("compte")
 
-                # Updating
+        if request.user.has_perm("kfet.manage_perms"):
-                account_form.save()
+            forms.append(group_form)
+        elif group_form.has_changed():
+            warnings.append("statut d'équipe")
 
-                # Checking perm to update password
+        if hasattr(account, "negative"):
-                if (
+            negative_form = AccountNegativeForm(request.POST, instance=account.negative)
-                    request.user.has_perm("kfet.change_account_password")
-                    and pwd_form.is_valid()
-                ):
-                    pwd = pwd_form.cleaned_data["pwd1"]
-                    account.change_pwd(pwd)
-                    account.save()
-                    messages.success(request, "Mot de passe mis à jour")
 
-                # Checking perm to manage perms
+            if request.user.has_perm("kfet.change_accountnegative"):
-                if request.user.has_perm("kfet.manage_perms") and group_form.is_valid():
+                forms.append(negative_form)
-                    group_form.save()
+            elif negative_form.has_changed():
+                warnings.append("négatifs")
 
-                if (
+        # Il ne faut pas valider `pwd_form` si elle est inchangée
-                    hasattr(account, "negative")
+        if pwd_form.has_changed():
-                    and request.user.has_perm("kfet.change_accountnegative")
+            if self_update or request.user.has_perm("kfet.change_account_password"):
-                    and negative_form.is_valid()
+                forms.append(pwd_form)
-                ):
+            else:
-                    negative_form.save()
+                warnings.append("mot de passe")
 
-                success = True
+        # Updating account info
-                messages.success(
+        if forms == []:
-                    request,
-                    "Informations du compte %s mises à jour" % account.trigramme,
-                )
-
-        # Modification de ses propres informations
-        if request.user == account.user:
-            missing_perm = False
-            account.refresh_from_db()
-            account_form = AccountRestrictForm(request.POST, instance=account)
-            pwd_form = AccountPwdForm(request.POST)
-
-            if account_form.is_valid():
-                account_form.save()
-                success = True
-                messages.success(request, "Vos informations ont été mises à jour")
-
-                if request.user.has_perm("kfet.is_team") and pwd_form.is_valid():
-                    pwd = pwd_form.cleaned_data["pwd1"]
-                    account.change_pwd(pwd)
-                    account.save()
-                    messages.success(request, "Votre mot de passe a été mis à jour")
-
-        if missing_perm:
-            messages.error(request, "Permission refusée")
-        if success:
-            return redirect("kfet.account.read", account.trigramme)
-        else:
             messages.error(
-                request, "Informations non mises à jour. Corrigez les erreurs"
+                request, "Informations non mises à jour : permission refusée"
             )
+        else:
+            if all(form.is_valid() for form in forms):
+                for form in forms:
+                    form.save()
+
+                if len(warnings):
+                    messages.warning(
+                        request,
+                        "Permissions insuffisantes pour modifier"
+                        " les informations suivantes : {}.".format(", ".join(warnings)),
+                    )
+                if self_update:
+                    messages.success(request, "Vos informations ont été mises à jour !")
+                else:
+                    messages.success(
+                        request,
+                        "Informations du compte %s mises à jour" % account.trigramme,
+                    )
+
+                return redirect("kfet.account.read", account.trigramme)
+            else:
+                messages.error(
+                    request, "Informations non mises à jour : corrigez les erreurs"
+                )
 
     return render(
         request,
@@ -449,7 +433,8 @@ def account_update(request, trigramme):
         },
     )
 
-    # Account - Delete
+
+# Account - Delete
 
 
 class AccountDelete(PermissionRequiredMixin, DeleteView):