From b224fedf283251a329ab54b0617db15a9e4e6711 Mon Sep 17 00:00:00 2001 From: Ludovic Stephan Date: Sat, 20 Feb 2021 15:40:52 +0100 Subject: [PATCH 1/6] Fix frozen account display --- kfet/static/kfet/css/index.css | 11 ++++++++++- kfet/static/kfet/js/account.js | 2 ++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/kfet/static/kfet/css/index.css b/kfet/static/kfet/css/index.css index 7d4324b4..f0eaedf0 100644 --- a/kfet/static/kfet/css/index.css +++ b/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; diff --git a/kfet/static/kfet/js/account.js b/kfet/static/kfet/js/account.js index 5ce3c8cd..3e216155 100644 --- a/kfet/static/kfet/js/account.js +++ b/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) { From 209360f535caef667480c05bf3dd69a946005efb Mon Sep 17 00:00:00 2001 From: Ludovic Stephan Date: Sat, 20 Feb 2021 15:42:16 +0100 Subject: [PATCH 2/6] Delete self-update form --- kfet/forms.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/kfet/forms.py b/kfet/forms.py index 4dd8a9bc..79f8bb73 100644 --- a/kfet/forms.py +++ b/kfet/forms.py @@ -85,11 +85,6 @@ 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", From aac94afcd0a59fdf473ca7b7fdb876437458af5b Mon Sep 17 00:00:00 2001 From: Ludovic Stephan Date: Sat, 20 Feb 2021 15:44:44 +0100 Subject: [PATCH 3/6] =?UTF-8?q?Am=C3=A9liore=20le=20formulaire=20de=20mdp?= =?UTF-8?q?=20K-F=C3=AAt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- kfet/forms.py | 23 +++++++++++++++++----- kfet/templates/kfet/account_update.html | 26 ++++++++++++------------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/kfet/forms.py b/kfet/forms.py index 79f8bb73..019a8e41 100644 --- a/kfet/forms.py +++ b/kfet/forms.py @@ -90,21 +90,34 @@ class AccountPwdForm(forms.Form): 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.set_password(password) + if commit: + self.account.save() + + return self.account + class CofForm(forms.ModelForm): def clean_is_cof(self): diff --git a/kfet/templates/kfet/account_update.html b/kfet/templates/kfet/account_update.html index 36b3d75d..dcb55555 100644 --- a/kfet/templates/kfet/account_update.html +++ b/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 %} -
+ {% 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" %}
-{% endblock %} +{% endblock %} \ No newline at end of file From 1450b65dcde895a37524324f55223810cf30a2d1 Mon Sep 17 00:00:00 2001 From: Ludovic Stephan Date: Sat, 20 Feb 2021 15:46:44 +0100 Subject: [PATCH 4/6] Rework complet de `account_update` --- kfet/views.py | 155 +++++++++++++++++++++++--------------------------- 1 file changed, 70 insertions(+), 85 deletions(-) diff --git a/kfet/views.py b/kfet/views.py index 5322082c..992db0ec 100644 --- a/kfet/views.py +++ b/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 ( - not request.user.has_perm("kfet.is_team") and request.user != account.user - ): - raise Http404 + if not account.editable: + # Plus de leak de trigramme ! + return HttpResponseForbidden user_info_form = UserInfoForm(instance=account.user) - if request.user.has_perm("kfet.is_team"): - group_form = UserGroupForm(instance=account.user) - account_form = AccountForm(instance=account) - pwd_form = AccountPwdForm() - if account.balance < 0 and not hasattr(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 + group_form = UserGroupForm(instance=account.user) + account_form = AccountForm(instance=account) + pwd_form = AccountPwdForm() + if hasattr(account, "negative"): + negative_form = AccountNegativeForm(instance=account.negative) else: - account_form = AccountRestrictForm(instance=account) - group_form = None negative_form = None - pwd_form = None if request.method == "POST": - # Update attempt - success = False - missing_perm = True + self_update = request.user == account.user + account_form = AccountForm(request.POST, instance=account) + group_form = UserGroupForm(request.POST, instance=account.user) + pwd_form = AccountPwdForm(request.POST, account=account) - if request.user.has_perm("kfet.is_team"): - account_form = AccountForm(request.POST, instance=account) - 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 - ) + forms = [] + warnings = [] - if request.user.has_perm("kfet.change_account") and account_form.is_valid(): - missing_perm = False + if self_update or request.user.has_perm("kfet.change_account"): + forms.append(account_form) + elif account_form.has_changed(): + warnings.append("compte") - # Updating - account_form.save() + if request.user.has_perm("kfet.manage_perms"): + forms.append(group_form) + elif group_form.has_changed(): + warnings.append("statut d'équipe") - # Checking perm to update password - if ( - 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") + if hasattr(account, "negative"): + negative_form = AccountNegativeForm(request.POST, instance=account.negative) - # Checking perm to manage perms - if request.user.has_perm("kfet.manage_perms") and group_form.is_valid(): - group_form.save() + if request.user.has_perm("kfet.change_accountnegative"): + forms.append(negative_form) + elif negative_form.has_changed(): + warnings.append("négatifs") - if ( - hasattr(account, "negative") - and request.user.has_perm("kfet.change_accountnegative") - and negative_form.is_valid() - ): - negative_form.save() + # Il ne faut pas valider `pwd_form` si elle est inchangée + if pwd_form.has_changed(): + if self_update or request.user.has_perm("kfet.change_account_password"): + forms.append(pwd_form) + else: + warnings.append("mot de passe") - success = True - messages.success( - 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: + # Updating account info + if forms == []: 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): From 47f406e09e4966f1f80be40ecd37364e1cef0eea Mon Sep 17 00:00:00 2001 From: Ludovic Stephan Date: Sat, 20 Feb 2021 17:04:45 +0100 Subject: [PATCH 5/6] Fix tests --- kfet/auth/tests.py | 8 +++-- kfet/forms.py | 2 +- kfet/tests/test_views.py | 74 ++++++++++++++++++++-------------------- 3 files changed, 44 insertions(+), 40 deletions(-) diff --git a/kfet/auth/tests.py b/kfet/auth/tests.py index 32e04812..a7a0b5ad 100644 --- a/kfet/auth/tests.py +++ b/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): """ diff --git a/kfet/forms.py b/kfet/forms.py index 019a8e41..b9adbc81 100644 --- a/kfet/forms.py +++ b/kfet/forms.py @@ -112,7 +112,7 @@ class AccountPwdForm(forms.Form): def save(self, commit=True): password = self.cleaned_data["pwd1"] - self.account.set_password(password) + self.account.change_pwd(password) if commit: self.account.save() diff --git a/kfet/tests/test_views.py b/kfet/tests/test_views.py index 40b9ef77..bc50b023 100644 --- a/kfet/tests/test_views.py +++ b/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_expected = "/k-fet/accounts/001/edit" + url_kwargs = {"trigramme": "100"} + 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": "", - "pwd2": "", + "pwd1": "changed_pwd", + "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() - self.users["user1"].refresh_from_db() + # Comportement attendu : compte modifié, + # utilisateur/mdp inchangé, warning pour le mdp + + self.accounts["team"].refresh_from_db() + self.users["team"].refresh_from_db() self.assertInstanceExpected( - self.accounts["user1"], - {"first_name": "first", "last_name": "last", "trigramme": "051"}, + self.accounts["team"], + {"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() - client.login(username="user1", password="user1") + r = self.client.post(self.url, self.post_data, follow=True) + 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) - self.assertRedirects(r, reverse("kfet.account.read", args=["001"])) - - self.accounts["user1"].refresh_from_db() - self.users["user1"].refresh_from_db() + # Comportement attendu : compte/mdp modifié, utilisateur inchangé 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) - self.assertForbiddenKfet(r) + client = Client() + 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): From 472a44c30fa120a4d1425ed4d582e3efcb4a3ed7 Mon Sep 17 00:00:00 2001 From: Ludovic Stephan Date: Wed, 3 Mar 2021 23:11:39 +0100 Subject: [PATCH 6/6] Remove useless buttons --- kfet/templates/kfet/left_account.html | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/kfet/templates/kfet/left_account.html b/kfet/templates/kfet/left_account.html index a058abf9..e1673d22 100644 --- a/kfet/templates/kfet/left_account.html +++ b/kfet/templates/kfet/left_account.html @@ -11,13 +11,11 @@ + {% if perms.kfet.is_team %}
Éditer - - Créditer - {% if perms.kfet.delete_account %}
+ {% endif %}

{{ account.name|title }}