diff --git a/core/management/commands/populate.py b/core/management/commands/populate.py index 26bc6074..53638699 100644 --- a/core/management/commands/populate.py +++ b/core/management/commands/populate.py @@ -125,6 +125,11 @@ class Command(BaseCommand): unix_name=settings.SITH_MAIN_CLUB["unix_name"], address=settings.SITH_MAIN_CLUB["address"], ) + main_club.board_group.permissions.add( + *Permission.objects.filter( + codename__in=["view_subscription", "add_subscription"] + ) + ) bar_club = Club.objects.create( id=2, name=settings.SITH_BAR_MANAGER["name"], diff --git a/core/models.py b/core/models.py index b1caa912..4748f311 100644 --- a/core/models.py +++ b/core/models.py @@ -417,29 +417,6 @@ class User(AbstractUser): def is_board_member(self) -> bool: return self.groups.filter(club_board=settings.SITH_MAIN_CLUB_ID).exists() - @cached_property - def can_read_subscription_history(self) -> bool: - if self.is_root or self.is_board_member: - return True - - from club.models import Club - - for club in Club.objects.filter( - id__in=settings.SITH_CAN_READ_SUBSCRIPTION_HISTORY - ): - if club in self.clubs_with_rights: - return True - return False - - @cached_property - def can_create_subscription(self) -> bool: - return self.is_root or ( - self.memberships.board() - .ongoing() - .filter(club_id__in=settings.SITH_CAN_CREATE_SUBSCRIPTIONS) - .exists() - ) - @cached_property def is_launderette_manager(self): from club.models import Club @@ -679,14 +656,6 @@ class AnonymousUser(AuthAnonymousUser): def __init__(self): super().__init__() - @property - def can_create_subscription(self): - return False - - @property - def can_read_subscription_history(self): - return False - @property def was_subscribed(self): return False diff --git a/core/templates/core/edit.jinja b/core/templates/core/edit.jinja index 25c6bd74..82c7a035 100644 --- a/core/templates/core/edit.jinja +++ b/core/templates/core/edit.jinja @@ -1,19 +1,40 @@ {% extends "core/base.jinja" %} +{# if the template context has the `object_name` variable, + then this one will be used in the page title, + instead of the result of `str(object)` #} +{% if object and not object_name %} + {% set object_name=object %} +{% endif %} + {% block title %} - {% if object %} - {% trans obj=object %}Edit {{ obj }}{% endtrans %} + {% if object_name %} + {% trans name=object_name %}Edit {{ name }}{% endtrans %} {% else %} {% trans %}Save{% endtrans %} {% endif %} {% endblock %} {% block content %} - {% if object %} -

{% trans obj=object %}Edit {{ obj }}{% endtrans %}

+ {% if object_name %} +

{% trans name=object_name %}Edit {{ name }}{% endtrans %}

{% else %}

{% trans %}Save{% endtrans %}

{% endif %} + {% if messages %} +
+ + {% for message in messages %} + {% if message.level_tag == "success" %} + {{ message }} + {% endif %} + {% endfor %} + + + + +
+ {% endif %}
{% csrf_token %} {{ form.as_p() }} diff --git a/core/templates/core/user_detail.jinja b/core/templates/core/user_detail.jinja index cb93b1cd..5fceb126 100644 --- a/core/templates/core/user_detail.jinja +++ b/core/templates/core/user_detail.jinja @@ -166,7 +166,7 @@ {% endif %}
-{% if profile.was_subscribed and (user == profile or user.can_read_subscription_history)%} +{% if profile.was_subscribed and (user == profile or user.has_perm("subscription.view_subscription")) %}
@@ -197,9 +197,9 @@
+
{% endif %} -
{% if user.is_root or user.is_board_member %} diff --git a/core/templates/core/user_tools.jinja b/core/templates/core/user_tools.jinja index 177b4199..4c9b9462 100644 --- a/core/templates/core/user_tools.jinja +++ b/core/templates/core/user_tools.jinja @@ -13,7 +13,7 @@

{% trans %}User Tools{% endtrans %}

- {% if user.can_create_subscription or user.is_root or user.is_board_member %} + {% if user.has_perm("subscription.view_userban") or user.is_root or user.is_board_member %}

{% trans %}Sith management{% endtrans %}

@@ -42,152 +42,202 @@ {% set is_admin_on_a_counter = true %} {% endfor %} - {% if - is_admin_on_a_counter - or user.is_root - or user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID) - %} + {% if is_admin_on_a_counter or user.is_root or user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID) %} +
+

{% trans %}Counters{% endtrans %}

+ + +
+ {% endif %} + + {% if user.is_root or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %} +
+

{% trans %}Accounting{% endtrans %}

+ +
+ {% endif %} + + {% if user.is_root or user.is_com_admin or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) %} +
+

{% trans %}Communication{% endtrans %}

+ +
+ {% endif %} + + {% if user.has_perm("subscription.add_subscription") or user.has_perm("auth.change_perm") or user.is_root or user.is_board_member %} +
+

{% trans %}Subscriptions{% endtrans %}

+ +
+ {% endif %} + + {% if user.memberships.filter(end_date=None).all().count() > 0 %} +
+

{% trans %}Club tools{% endtrans %}

+
    + {% for m in user.memberships.filter(end_date=None).all() %} +
  • {{ m.club }}
    • + {% endfor %} +
+
+ {% endif %} + + {% if user.has_perm("pedagogy.add_uv") or user.has_perm("pedagogy.delete_uvcomment") %} +
+

{% trans %}Pedagogy{% endtrans %}

+ +
+ {% endif %} +
-

{% trans %}Counters{% endtrans %}

+

{% trans %}Elections{% endtrans %}

- +
-
  • - {{ b[1] }} - - - - {% trans %}Edit{% endtrans %} - {% trans %}Stats{% endtrans %} - - -
    • - {% endif %} - {% endfor %} - -
    -{% endif %} - -{% if -user.is_root -or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) -or user.memberships.ongoing().filter(role__gte=7).count() > 10 -%} -
    -

    {% trans %}Accounting{% endtrans %}

    - -
    -{% endif %} - -{% if -user.is_root -or user.is_com_admin -or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) -%} -
    -

    {% trans %}Communication{% endtrans %}

    - -
    -{% endif %} - -{% if user.memberships.filter(end_date=None).all().count() > 0 %} -
    -

    {% trans %}Club tools{% endtrans %}

    -
      - {% for m in user.memberships.filter(end_date=None).all() %} -
    • {{ m.club }}
      • - {% endfor %} -
    -
    -{% endif %} - -{% if -user.is_root -or user.is_in_group(pk=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID) -%} -
    -

    {% trans %}Pedagogy{% endtrans %}

    - -
    -{% endif %} - -
    -

    {% trans %}Elections{% endtrans %}

    - -
    - -
    -

    {% trans %}Other tools{% endtrans %}

    - -
    -
    - +
    +

    {% trans %}Other tools{% endtrans %}

    + +
    +
    + {% endblock %} \ No newline at end of file diff --git a/core/views/forms.py b/core/views/forms.py index 0c0ec0e2..381fc8a3 100644 --- a/core/views/forms.py +++ b/core/views/forms.py @@ -28,6 +28,7 @@ from captcha.fields import CaptchaField from django import forms from django.conf import settings from django.contrib.auth.forms import AuthenticationForm, UserCreationForm +from django.contrib.auth.models import Permission from django.contrib.staticfiles.management.commands.collectstatic import ( staticfiles_storage, ) @@ -440,3 +441,28 @@ class GiftForm(forms.ModelForm): id=user_id ) self.fields["user"].widget = forms.HiddenInput() + + +class PermissionGroupsForm(forms.ModelForm): + """Manage the groups that have a specific permission.""" + + class Meta: + model = Permission + fields = [] + + groups = forms.ModelMultipleChoiceField( + Group.objects.all(), + label=_("Groups"), + widget=AutoCompleteSelectMultipleGroup, + required=False, + ) + + def __init__(self, instance: Permission, **kwargs): + super().__init__(instance=instance, **kwargs) + self.fields["groups"].initial = instance.group_set.all() + + def save(self, commit: bool = True): # noqa FTB001 + instance = super().save(commit=False) + if commit: + instance.group_set.set(self.cleaned_data["groups"]) + return instance diff --git a/core/views/group.py b/core/views/group.py index e17db138..ba6b406d 100644 --- a/core/views/group.py +++ b/core/views/group.py @@ -17,6 +17,10 @@ from django import forms from django.contrib.auth.mixins import PermissionRequiredMixin +from django.contrib.auth.models import Permission +from django.contrib.messages.views import SuccessMessageMixin +from django.core.exceptions import ImproperlyConfigured +from django.shortcuts import get_object_or_404 from django.urls import reverse_lazy from django.utils.translation import gettext_lazy as _ from django.views.generic import ListView @@ -25,6 +29,7 @@ from django.views.generic.edit import CreateView, DeleteView, UpdateView from core.auth.mixins import CanEditMixin from core.models import Group, User from core.views import DetailFormView +from core.views.forms import PermissionGroupsForm from core.views.widgets.select import AutoCompleteSelectMultipleUser # Forms @@ -130,3 +135,62 @@ class GroupDeleteView(CanEditMixin, DeleteView): pk_url_kwarg = "group_id" template_name = "core/delete_confirm.jinja" success_url = reverse_lazy("core:group_list") + + +class PermissionGroupsUpdateView( + PermissionRequiredMixin, SuccessMessageMixin, UpdateView +): + """Manage the groups that have a specific permission. + + Notes: + This is an `UpdateView`, but unlike typical `UpdateView`, + it doesn't accept url arguments to retrieve the object + to update. + As such, a `PermissionGroupsUpdateView` can only deal with + a single hardcoded permission. + + This is not a limitation, but an on-purpose design, + mainly for security matters. + + Example: + ```python + class SubscriptionPermissionView(PermissionGroupsUpdateView): + permission = "subscription.add_subscription" + ``` + """ + + permission_required = "auth.change_permission" + template_name = "core/edit.jinja" + form_class = PermissionGroupsForm + permission = None + success_message = _("Groups have been successfully updated.") + + def get_object(self, *args, **kwargs): + if not self.permission: + raise ImproperlyConfigured( + f"{self.__class__.__name__} is missing the permission attribute. " + "Please fill it with either a permission string " + "or a Permission object." + ) + if isinstance(self.permission, Permission): + return self.permission + if isinstance(self.permission, str): + try: + app_label, codename = self.permission.split(".") + except ValueError as e: + raise ValueError( + "Permission name should be in the form " + "app_label.permission_codename." + ) from e + return get_object_or_404( + Permission, codename=codename, content_type__app_label=app_label + ) + raise TypeError( + f"{self.__class__.__name__}.permission " + f"must be a string or a permission instance." + ) + + def get_success_url(self): + # if children classes define a success url, return it, + # else stay on the same page + return self.success_url or self.request.path diff --git a/docs/tutorial/groups.md b/docs/tutorial/groups.md index bccd713f..2c67b3f0 100644 --- a/docs/tutorial/groups.md +++ b/docs/tutorial/groups.md @@ -228,3 +228,38 @@ Les groupes de ban existants sont les suivants : - `Banned from buying alcohol` : les utilisateurs interdits de vente d'alcool (non mineurs) - `Banned from counters` : les utilisateurs interdits d'utilisation des comptoirs - `Banned to subscribe` : les utilisateurs interdits de cotisation + +## Groupes liés à une permission + +Certaines actions sur le site demandent une permission en particulier, +que l'on veut donner ou retirer n'importe quand. + +Prenons par exemple les cotisations : lors de l'intégration, +on veut permettre aux membres du bureau de l'Integ +de créer des cotisations, et pareil pour les membres du bureau +de la Welcome Week pendant cette dernière. + +Dans ces cas-là, il est pertinent de mettre à disposition +des administrateurs du site une page leur permettant +de gérer quels groupes ont une permission donnée. +Pour ce faire, il existe +[PermissionGroupsUpdateView][core.views.PermissionGroupsUpdateView]. + +Pour l'utiliser, il suffit de créer une vue qui en hérite +et de lui dire quelle est la permission dont on veut gérer +les groupes : + +```python +from core.views.group import PermissionGroupsUpdateView + + +class SubscriptionPermissionView(PermissionGroupsUpdateView): + permission = "subscription.add_subscription" +``` + +Configurez l'url de la vue, et c'est tout ! +La page ainsi générée contiendra un formulaire +avec un unique champ permettant de sélectionner des groupes. +Par défaut, seuls les utilisateurs avec la permission +`auth.change_permission` auront accès à ce formulaire +(donc, normalement, uniquement les utilisateurs Root). diff --git a/locale/fr/LC_MESSAGES/django.po b/locale/fr/LC_MESSAGES/django.po index dc6b5ee6..0cfef902 100644 --- a/locale/fr/LC_MESSAGES/django.po +++ b/locale/fr/LC_MESSAGES/django.po @@ -6,7 +6,7 @@ msgid "" msgstr "" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2025-01-19 18:12+0100\n" +"POT-Creation-Date: 2025-02-12 15:55+0100\n" "PO-Revision-Date: 2016-07-18\n" "Last-Translator: Maréchal \n" @@ -2383,11 +2383,10 @@ msgstr "Confirmation" msgid "Cancel" msgstr "Annuler" -#: core/templates/core/edit.jinja core/templates/core/file_edit.jinja -#: counter/templates/counter/cash_register_summary.jinja +#: core/templates/core/edit.jinja #, python-format -msgid "Edit %(obj)s" -msgstr "Éditer %(obj)s" +msgid "Edit %(name)s" +msgstr "Éditer %(name)s" #: core/templates/core/file.jinja core/templates/core/file_list.jinja msgid "File list" @@ -2457,6 +2456,12 @@ msgstr "octets" msgid "Download" msgstr "Télécharger" +#: core/templates/core/file_edit.jinja +#: counter/templates/counter/cash_register_summary.jinja +#, python-format +msgid "Edit %(obj)s" +msgstr "Éditer %(obj)s" + #: core/templates/core/file_list.jinja msgid "There is no file in this website." msgstr "Il n'y a pas de fichier sur ce site web." @@ -2914,7 +2919,7 @@ msgstr "Blouse" msgid "Not subscribed" msgstr "Non cotisant" -#: core/templates/core/user_detail.jinja +#: core/templates/core/user_detail.jinja core/templates/core/user_tools.jinja #: subscription/templates/subscription/subscription.jinja msgid "New subscription" msgstr "Nouvelle cotisation" @@ -3146,15 +3151,6 @@ msgstr "Supprimer les messages forum d'un utilisateur" msgid "Bans" msgstr "Bans" -#: core/templates/core/user_tools.jinja -msgid "Subscriptions" -msgstr "Cotisations" - -#: core/templates/core/user_tools.jinja -#: subscription/templates/subscription/stats.jinja -msgid "Subscription stats" -msgstr "Statistiques de cotisation" - #: core/templates/core/user_tools.jinja counter/forms.py #: counter/views/mixins.py msgid "Counters" @@ -3227,6 +3223,19 @@ msgstr "Modérer les fichiers" msgid "Moderate pictures" msgstr "Modérer les photos" +#: core/templates/core/user_tools.jinja +msgid "Subscriptions" +msgstr "Cotisations" + +#: core/templates/core/user_tools.jinja +msgid "Manage permissions" +msgstr "Gérer les permissions" + +#: core/templates/core/user_tools.jinja +#: subscription/templates/subscription/stats.jinja +msgid "Subscription stats" +msgstr "Statistiques de cotisation" + #: core/templates/core/user_tools.jinja pedagogy/templates/pedagogy/guide.jinja msgid "Create UV" msgstr "Créer UV" @@ -3355,6 +3364,10 @@ msgstr "Utilisateurs à ajouter au groupe" msgid "Users to remove from group" msgstr "Utilisateurs à retirer du groupe" +#: core/views/group.py +msgid "Groups have been successfully updated." +msgstr "Les groupes ont été mis à jour avec succès." + #: core/views/user.py msgid "We couldn't verify that this email actually exists" msgstr "Nous n'avons pas réussi à vérifier que cette adresse mail existe." @@ -5676,6 +5689,10 @@ msgstr "Cotisations par type" msgid "Existing member" msgstr "Membre existant" +#: subscription/views.py +msgid "the groups that can create subscriptions" +msgstr "les groupes pouvant créer des cotisations" + #: trombi/models.py msgid "subscription deadline" msgstr "fin des inscriptions" diff --git a/sith/settings.py b/sith/settings.py index d3acabec..b7d7e71c 100644 --- a/sith/settings.py +++ b/sith/settings.py @@ -517,14 +517,6 @@ SITH_PRODUCT_SUBSCRIPTION_ONE_SEMESTER = 1 SITH_PRODUCT_SUBSCRIPTION_TWO_SEMESTERS = 2 SITH_PRODUCTTYPE_SUBSCRIPTION = 2 -# Defines which club lets its member the ability to make subscriptions -# Elements of this list are club's id -SITH_CAN_CREATE_SUBSCRIPTIONS = [1] - -# Defines which clubs lets its members the ability to see users subscription history -# Elements of this list are club's id -SITH_CAN_READ_SUBSCRIPTION_HISTORY = [] - # Number of weeks before the end of a subscription when the subscriber can resubscribe SITH_SUBSCRIPTION_END = 10 diff --git a/subscription/tests/test_new_susbcription.py b/subscription/tests/test_new_susbcription.py index ccdff407..8fd5e7c4 100644 --- a/subscription/tests/test_new_susbcription.py +++ b/subscription/tests/test_new_susbcription.py @@ -5,6 +5,7 @@ from typing import Callable import pytest from dateutil.relativedelta import relativedelta +from django.contrib.auth.models import Permission from django.test import Client from django.urls import reverse from django.utils.timezone import localdate @@ -108,7 +109,12 @@ def test_page_access( @pytest.mark.django_db def test_submit_form_existing_user(client: Client, settings: SettingsWrapper): - client.force_login(board_user.make()) + client.force_login( + baker.make( + User, + user_permissions=Permission.objects.filter(codename="add_subscription"), + ) + ) user = old_subscriber_user.make() response = client.post( reverse("subscription:fragment-existing-user"), @@ -133,7 +139,12 @@ def test_submit_form_existing_user(client: Client, settings: SettingsWrapper): @pytest.mark.django_db def test_submit_form_new_user(client: Client, settings: SettingsWrapper): - client.force_login(board_user.make()) + client.force_login( + baker.make( + User, + user_permissions=Permission.objects.filter(codename="add_subscription"), + ) + ) response = client.post( reverse("subscription:fragment-new-user"), { diff --git a/subscription/tests/test_permissions.py b/subscription/tests/test_permissions.py new file mode 100644 index 00000000..fcc290e9 --- /dev/null +++ b/subscription/tests/test_permissions.py @@ -0,0 +1,43 @@ +from django.contrib.auth.models import Permission +from django.test import TestCase +from django.urls import reverse +from model_bakery import baker +from pytest_django.asserts import assertRedirects + +from club.models import Club, Membership +from core.baker_recipes import subscriber_user +from core.models import User + + +class TestSubscriptionPermission(TestCase): + @classmethod + def setUpTestData(cls): + cls.user: User = subscriber_user.make() + cls.admin = baker.make(User, is_superuser=True) + cls.club = baker.make(Club) + baker.make(Membership, user=cls.user, club=cls.club, role=7) + + def test_give_permission(self): + self.client.force_login(self.admin) + response = self.client.post( + reverse("subscription:perms"), {"groups": [self.club.board_group_id]} + ) + assertRedirects(response, reverse("subscription:perms")) + assert self.user.has_perm("subscription.add_subscription") + + def test_remove_permission(self): + self.client.force_login(self.admin) + response = self.client.post(reverse("subscription:perms"), {"groups": []}) + assertRedirects(response, reverse("subscription:perms")) + assert not self.user.has_perm("subscription.add_subscription") + + def test_subscription_page_access(self): + self.client.force_login(self.user) + response = self.client.get(reverse("subscription:subscription")) + assert response.status_code == 403 + + self.club.board_group.permissions.add( + Permission.objects.get(codename="add_subscription") + ) + response = self.client.get(reverse("subscription:subscription")) + assert response.status_code == 200 diff --git a/subscription/urls.py b/subscription/urls.py index 47dbf21e..3d3c9996 100644 --- a/subscription/urls.py +++ b/subscription/urls.py @@ -20,6 +20,7 @@ from subscription.views import ( CreateSubscriptionNewUserFragment, NewSubscription, SubscriptionCreatedFragment, + SubscriptionPermissionView, SubscriptionsStatsView, ) @@ -41,5 +42,10 @@ urlpatterns = [ SubscriptionCreatedFragment.as_view(), name="creation-success", ), + path( + "perms/", + SubscriptionPermissionView.as_view(), + name="perms", + ), path("stats/", SubscriptionsStatsView.as_view(), name="stats"), ] diff --git a/subscription/views.py b/subscription/views.py index b285a137..d5f9d75d 100644 --- a/subscription/views.py +++ b/subscription/views.py @@ -14,13 +14,15 @@ # from django.conf import settings -from django.contrib.auth.mixins import UserPassesTestMixin +from django.contrib.auth.mixins import PermissionRequiredMixin from django.core.exceptions import PermissionDenied from django.urls import reverse, reverse_lazy from django.utils.timezone import localdate +from django.utils.translation import gettext_lazy as _ from django.views.generic import CreateView, DetailView, TemplateView from django.views.generic.edit import FormView +from core.views.group import PermissionGroupsUpdateView from counter.apps import PAYMENT_METHOD from subscription.forms import ( SelectionDateForm, @@ -30,13 +32,9 @@ from subscription.forms import ( from subscription.models import Subscription -class CanCreateSubscriptionMixin(UserPassesTestMixin): - def test_func(self): - return self.request.user.can_create_subscription - - -class NewSubscription(CanCreateSubscriptionMixin, TemplateView): +class NewSubscription(PermissionRequiredMixin, TemplateView): template_name = "subscription/subscription.jinja" + permission_required = "subscription.add_subscription" def get_context_data(self, **kwargs): return super().get_context_data(**kwargs) | { @@ -49,8 +47,9 @@ class NewSubscription(CanCreateSubscriptionMixin, TemplateView): } -class CreateSubscriptionFragment(CanCreateSubscriptionMixin, CreateView): +class CreateSubscriptionFragment(PermissionRequiredMixin, CreateView): template_name = "subscription/fragments/creation_form.jinja" + permission_required = "subscription.add_subscription" def get_success_url(self): return reverse( @@ -72,13 +71,21 @@ class CreateSubscriptionNewUserFragment(CreateSubscriptionFragment): extra_context = {"post_url": reverse_lazy("subscription:fragment-new-user")} -class SubscriptionCreatedFragment(CanCreateSubscriptionMixin, DetailView): +class SubscriptionCreatedFragment(PermissionRequiredMixin, DetailView): template_name = "subscription/fragments/creation_success.jinja" + permission_required = "subscription.add_subscription" model = Subscription pk_url_kwarg = "subscription_id" context_object_name = "subscription" +class SubscriptionPermissionView(PermissionGroupsUpdateView): + """Manage the groups that have access to the subscription creation page.""" + + permission = "subscription.add_subscription" + extra_context = {"object_name": _("the groups that can create subscriptions")} + + class SubscriptionsStatsView(FormView): template_name = "subscription/stats.jinja" form_class = SelectionDateForm