Refactor to permission groups

doc/development/api/general.rst

@@ -15,7 +15,7 @@ Core
       item_copy_data, register_sales_channel_types, register_global_settings, quota_availability, global_email_filter,
       register_ticket_secret_generators, gift_card_transaction_display,
       register_text_placeholders, register_mail_placeholders, device_info_updated,
-      register_event_permissions, register_organizer_permissions
+      register_event_permission_groups, register_organizer_permission_groups
 
 Order events
 """"""""""""

doc/development/implementation/permissions.rst

@@ -199,26 +199,49 @@ intended to help compliance with data protection rules as imposed e.g. by GDPR.
 Adding permissions
 ------------------
 
-Plugins can add permissions through the ``register_event_permissions`` and ``register_organizer_permission``.
+Plugins can add permissions through the ``register_event_permission_groups`` and ``register_organizer_permission_groups``.
 We recommend to use this only for very significant permissions, as the system will become less usable with too many
 permission levels, also because the team page will show all permission options, even those of disabled plugins.
-We recommend to prefix the permission string with the plugin name and follow the ``<module>.<thing>:<action>`` pattern.
+
+To register your permissions, you need to register a **permission group** (often representing an area of functionality
+or a key model). Below that group, there are **actions**, which represent the actual permissions. Permissions will be
+generated as ``<group_name>:<action>``. Then, you need to define **options** which are the valid combinations of the
+actions that should be possible to select for a team. This two-step mechanism exists to provide a better user experience
+and avoid useless combinations like "write but not read".
 
 Example::
 
-    @receiver(register_event_permissions)
+    @receiver(register_event_permission_groups)
-    def register_default_event_permissions(sender, **kwargs):
+    def register_plugin_event_permissions(sender, **kwargs):
         return [
-            Permission("pretix_myplugin.resource:read", _("Read resources"),
+            PermissionGroup(
-                       "pretix_myplugin", _("Some helptext")),
+                name="pretix_myplugin.resource",
+                label=_("Resources"),
+                actions=["read", "write"],
+                options=[
+                    PermissionOption(actions=tuple(), label=_("No access")),
+                    PermissionOption(actions=("read",), label=_("View")),
+                    PermissionOption(actions=("read", "write"), label=_("View and change")),
+                ],
+                help_text=_("Some help text")
+            ),
         ]
 
 
-    @receiver(register_organizer_permissions)
+    @receiver(register_organizer_permission_groups)
-    def register_default_organizer_permissions(sender, **kwargs):
+    def register_plugin_organizer_permissions(sender, **kwargs):
         return [
-            Permission("pretix_myplugin.resource:read", _("Read resources"),
+            PermissionGroup(
-                       "pretix_myplugin", _("Some helptext")),
+                name="pretix_myplugin.resource",
+                label=_("Resources"),
+                actions=["read", "write"],
+                options=[
+                    PermissionOption(actions=tuple(), label=_("No access")),
+                    PermissionOption(actions=("read",), label=_("View")),
+                    PermissionOption(actions=("read", "write"), label=_("View and change")),
+                ],
+                help_text=_("Some help text")
+            ),
         ]
 
 .. _configuring teams and permissions: https://docs.pretix.eu/guides/teams/

src/pretix/api/serializers/organizer.py

@@ -46,7 +46,7 @@ from pretix.base.models import (
 )
 from pretix.base.models.seating import SeatingPlanLayoutValidator
 from pretix.base.permissions import (
-    get_all_event_permissions, get_all_organizer_permissions,
+    get_all_event_permission_groups, get_all_organizer_permission_groups,
 )
 from pretix.base.plugins import (
     PLUGIN_LEVEL_EVENT, PLUGIN_LEVEL_EVENT_ORGANIZER_HYBRID,
@@ -355,8 +355,18 @@ class TeamSerializer(serializers.ModelSerializer):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        self.fields['limit_event_permissions'].choices = [(p.name, p.name) for p in get_all_event_permissions().values()]
+
-        self.fields['limit_organizer_permissions'].choices = [(p.name, p.name) for p in get_all_organizer_permissions().values()]
+        event_perms_flattened = []
+        organizer_perms_flattened = []
+        for pg in get_all_event_permission_groups().values():
+            for action in pg.actions:
+                event_perms_flattened.append(f"{pg.name}:{action}")
+        for pg in get_all_organizer_permission_groups().values():
+            for action in pg.actions:
+                organizer_perms_flattened.append(f"{pg.name}:{action}")
+
+        self.fields['limit_event_permissions'].choices = [(p, p) for p in event_perms_flattened]
+        self.fields['limit_organizer_permissions'].choices = [(p, p) for p in organizer_perms_flattened]
 
     def to_representation(self, instance):
         r = super().to_representation(instance)
@@ -406,6 +416,25 @@ class TeamSerializer(serializers.ModelSerializer):
 
         if full_data.get('limit_events') and full_data.get('all_events'):
             raise ValidationError('Do not set both limit_events and all_events.')
+
+        full_data.update(data)
+        for pg in get_all_event_permission_groups().values():
+            requested = ",".join(sorted(
+                a for a in pg.actions if self.instance and full_data["limit_event_permissions"].get(f"{pg.name}:{a}")
+            ))
+            if requested not in (",".join(sorted(opt.actions)) for opt in pg.options):
+                raise ValidationError(f"For permission group {pg.name}, the valid combinations of actions are "
+                                      f"'{'\' or \''.join(','.join(opt.actions) for opt in pg.options)}' but you tried to "
+                                      f"set '{requested}'.")
+        for pg in get_all_organizer_permission_groups().values():
+            requested = ",".join(sorted(
+                a for a in pg.actions if self.instance and full_data["limit_organizer_permissions"].get(f"{pg.name}:{a}")
+            ))
+            if requested not in (",".join(sorted(opt.actions)) for opt in pg.options):
+                raise ValidationError(f"For permission group {pg.name}, the valid combinations of actions are "
+                                      f"'{'\' or \''.join(','.join(opt.actions) for opt in pg.options)}' but you tried to "
+                                      f"set '{requested}'.")
+
         return data
 
 

src/pretix/base/models/organizer.py

@@ -398,12 +398,13 @@ class Team(LoggedModel):
         }
 
     def event_permission_set(self, include_legacy=True) -> set:
-        from ..permissions import get_all_event_permissions
+        from ..permissions import get_all_event_permission_groups
 
         result = set()
-        for permission in get_all_event_permissions().keys():
+        for pg in get_all_event_permission_groups().values():
-            if self.all_event_permissions or self.limit_event_permissions.get(permission):
+            for action in pg.actions:
-                result.add(permission)
+                if self.all_event_permissions or self.limit_event_permissions.get(f"{pg.name}:{action}"):
+                    result.add(f"{pg.name}:{action}")
 
         if include_legacy:
             # Add legacy permissions as well for plugin compatibility
@@ -414,12 +415,13 @@ class Team(LoggedModel):
         return result
 
     def organizer_permission_set(self, include_legacy=True) -> set:
-        from ..permissions import get_all_organizer_permissions
+        from ..permissions import get_all_organizer_permission_groups
 
         result = set()
-        for permission in get_all_organizer_permissions().keys():
+        for pg in get_all_organizer_permission_groups().values():
-            if self.all_organizer_permissions or self.limit_organizer_permissions.get(permission):
+            for action in pg.actions:
-                result.add(permission)
+                if self.all_organizer_permissions or self.limit_organizer_permissions.get(f"{pg.name}:{action}"):
+                    result.add(f"{pg.name}:{action}")
 
         if include_legacy:
             # Add legacy permissions as well for plugin compatibility

src/pretix/base/permissions.py

@@ -21,13 +21,15 @@
 #
 
 import logging
-from collections import OrderedDict, namedtuple
+from collections import OrderedDict
+from typing import Dict, List, NamedTuple, Tuple
 
 from django.dispatch import receiver
+from django.utils.functional import Promise
 from django.utils.translation import gettext_lazy as _, pgettext_lazy
 
 from pretix.base.signals import (
-    register_event_permissions, register_organizer_permissions,
+    register_event_permission_groups, register_organizer_permission_groups,
 )
 
 logger = logging.getLogger(__name__)
@@ -35,17 +37,28 @@ _ALL_EVENT_PERMISSIONS = None
 _ALL_ORGANIZER_PERMISSIONS = None
 
 
-Permission = namedtuple('Permission', ('name', 'label', 'plugin_name', 'help_text'))
+class PermissionOption(NamedTuple):
+    actions: Tuple[str, ...]
+    label: str | Promise
+    help_text: str | Promise = None
 
 
-def get_all_event_permissions():
+class PermissionGroup(NamedTuple):
+    name: str
+    label: str | Promise
+    actions: List[str]
+    options: List[PermissionOption]
+    help_text: str | Promise = None
+
+
+def get_all_event_permission_groups() -> Dict[str, PermissionGroup]:
     global _ALL_EVENT_PERMISSIONS
 
     if _ALL_EVENT_PERMISSIONS:
         return _ALL_EVENT_PERMISSIONS
 
     types = OrderedDict()
-    for recv, ret in register_event_permissions.send(None):
+    for recv, ret in register_event_permission_groups.send(None):
         if isinstance(ret, (list, tuple)):
             for r in ret:
                 types[r.name] = r
@@ -55,14 +68,14 @@ def get_all_event_permissions():
     return types
 
 
-def get_all_organizer_permissions():
+def get_all_organizer_permission_groups() -> Dict[str, PermissionGroup]:
     global _ALL_ORGANIZER_PERMISSIONS
 
     if _ALL_ORGANIZER_PERMISSIONS:
         return _ALL_ORGANIZER_PERMISSIONS
 
     types = OrderedDict()
-    for recv, ret in register_organizer_permissions.send(None):
+    for recv, ret in register_organizer_permission_groups.send(None):
         if isinstance(ret, (list, tuple)):
             for r in ret:
                 types[r.name] = r
@@ -72,46 +85,167 @@ def get_all_organizer_permissions():
     return types
 
 
-@receiver(register_event_permissions, dispatch_uid="base_register_default_event_permissions")
+OPTS_ALL_READ = [
+    PermissionOption(actions=tuple(), label=pgettext_lazy("permission_level", "View")),
+    PermissionOption(actions=("write",), label=pgettext_lazy("permission_level", "View and change")),
+]
+OPTS_ALL_READ_SETTINGS_API = [
+    PermissionOption(actions=tuple(), label=pgettext_lazy("permission_level", "View"),
+                     help_text=_("API only")),
+    PermissionOption(actions=("write",), label=pgettext_lazy("permission_level", "View and change")),
+]
+OPTS_ALL_READ_SETTINGS_PARENT = [
+    PermissionOption(actions=tuple(), label=pgettext_lazy("permission_level", "View"),
+                     help_text=_("Menu item will only show up if the user has permission for general settings.")),
+    PermissionOption(actions=("write",), label=pgettext_lazy("permission_level", "View and change")),
+]
+OPTS_READ_WRITE = [
+    PermissionOption(actions=tuple(), label=pgettext_lazy("permission_level", "No access")),
+    PermissionOption(actions=("read",), label=pgettext_lazy("permission_level", "View")),
+    PermissionOption(actions=("read", "write"), label=pgettext_lazy("permission_level", "View and change")),
+]
+
+
+@receiver(register_event_permission_groups, dispatch_uid="base_register_default_event_permissions")
 def register_default_event_permissions(sender, **kwargs):
     return [
-        Permission("event.settings.general:write", _("Change general settings"), None,
+        PermissionGroup(
-                   _("This includes access to all settings not listed explicitly below, including plugin settings.")),
+            name="event.settings.general",
-        Permission("event.settings.payment:write", _("Change payment settings"), None, None),
+            label=_("General settings"),
-        Permission("event.settings.tax:write", _("Change tax rules"), None, None),
+            actions=["write"],
-        Permission("event.settings.invoicing:write", _("Change invoicing settings"), None, None),
+            options=OPTS_ALL_READ_SETTINGS_API,
-        Permission("event.subevents:write", pgettext_lazy("subevent", "Change event series dates"), None,
+            help_text=_(
-                   _("Read access is granted to all teams with access to the event.")),
+                "This includes access to all settings not listed explicitly below, including plugin settings."
-        Permission("event.items:write", _("Change products, quotas, and questions"), None,
+            ),
-                   _("Also includes related objects like categories or discounts. Read access is granted to all teams with access to the event.")),
+        ),
-        Permission("event.orders:read", _("View orders"), None, None),
+        PermissionGroup(
-        Permission("event.orders:write", _("Change orders"), None, _("This includes the ability to cancel and refund individual orders.")),
+            name="event.settings.payment",
-        Permission("event.orders:checkin", _("Check-in orders"), None, None),
+            label=_("Payment settings"),
-        Permission("event.vouchers:read", _("View vouchers"), None, None),
+            actions=["write"],
-        Permission("event.vouchers:write", _("Change vouchers"), None, None),
+            options=OPTS_ALL_READ_SETTINGS_PARENT,
-        Permission("event:cancel", pgettext_lazy("subevent", "Cancel entire event or date"), None, None),
+        ),
+        PermissionGroup(
+            name="event.settings.tax",
+            label=_("Tax settings"),
+            actions=["write"],
+            options=OPTS_ALL_READ_SETTINGS_PARENT,
+        ),
+        PermissionGroup(
+            name="event.settings.invoicing",
+            label=_("Invoicing settings"),
+            actions=["write"],
+            options=OPTS_ALL_READ_SETTINGS_PARENT,
+        ),
+        PermissionGroup(
+            name="event.subevents",
+            label=_("Event series dates"),
+            actions=["write"],
+            options=OPTS_ALL_READ,
+        ),
+        PermissionGroup(
+            name="event.items",
+            label=_("Products, quotas and questions"),
+            actions=["write"],
+            options=OPTS_ALL_READ,
+            help_text=_("Also includes related objects like categories or discounts."),
+        ),
+        PermissionGroup(
+            name="event.orders",
+            label=_("Orders"),
+            actions=["read", "write", "checkin"],
+            options=[
+                PermissionOption(actions=tuple(), label=pgettext_lazy("permission_level", "No access")),
+                PermissionOption(actions=("checkin",), label=pgettext_lazy("permission_level", "Only check-in")),
+                PermissionOption(actions=("read",), label=pgettext_lazy("permission_level", "View all")),
+                PermissionOption(actions=("read", "checkin"), label=pgettext_lazy("permission_level", "View all and check-in")),
+                PermissionOption(actions=("read", "write"), label=pgettext_lazy("permission_level", "View all and change"),
+                                 help_text=_("Includes the ability to cancel and refund individual orders.")),
+            ],
+            help_text=_("Also includes related objects like the waiting list."),
+        ),
+        PermissionGroup(
+            name="event.vouchers",
+            label=_("Vouchers"),
+            actions=["read", "write"],
+            options=OPTS_READ_WRITE,
+        ),
+        PermissionGroup(
+            name="event",
+            label=_("Full event or date cancellation"),
+            actions=["cancel"],
+            options=[
+                # If we ever add more actions, we need a new UI idea here
+                PermissionOption(actions=tuple(), label=pgettext_lazy("permission_level", "Not allowed")),
+                PermissionOption(actions=("cancel",), label=pgettext_lazy("permission_level", "Allowed")),
+            ],
+            help_text="",
+        ),
     ]
 
 
-@receiver(register_organizer_permissions, dispatch_uid="base_register_default_organizer_permissions")
+@receiver(register_organizer_permission_groups, dispatch_uid="base_register_default_organizer_permissions")
 def register_default_organizer_permissions(sender, **kwargs):
     return [
-        Permission("organizer.events:create", _("Create events"), None, None),
+        PermissionGroup(
-        Permission("organizer.settings.general:write", _("Change settings"), None,
+            name="organizer.events",
-                   _("This includes access to all organizer-level functionality not listed explicitly below, including plugin settings.")),
+            label=_("Events"),
-        Permission("organizer.teams:write", _("Change teams"), None,
+            actions=["create"],
-                   _("This includes the ability to give someone (including oneself) additional permissions. Read access "
+            options=[
-                     "is implicitly granted to the same team.")),
+                PermissionOption(actions=tuple(), label=pgettext_lazy("permission_level", "Only existing events")),
-        Permission("organizer.giftcards:read", _("View gift cards"), None, None),
+                PermissionOption(actions=("create",), label=pgettext_lazy("permission_level", "Create new events")),
-        Permission("organizer.giftcards:write", _("Change gift cards"), None, None),
+            ],
-        Permission("organizer.customers:read", _("View customer accounts"), None, None),
+            help_text="",
-        Permission("organizer.customers:write", _("Change customer accounts"), None, None),
+        ),
-        Permission("organizer.reusablemedia:read", _("View reusable media"), None,
+        PermissionGroup(
-                   _("This includes access to data of tickets connected to reusable media.")),
+            name="organizer.settings.general",
-        Permission("organizer.reusablemedia:write", _("Change reusable media"), None, None),
+            label=_("Settings"),
-        Permission("organizer.devices:read", _("View devices and gates"), None, None),
+            actions=["write"],
-        Permission("organizer.devices:write", _("Change devices and gates"), None,
+            options=OPTS_ALL_READ_SETTINGS_API,
-                   _("This includes the ability to give access to events and data oneself does not have access to.")),
+            help_text=_("This includes access to all organizer-level functionality not listed explicitly below, including plugin settings."),
-        Permission("organizer.seatingplans:write", _("Change seating plans"), None,
+        ),
-                   _("Read access is implicitly given to all teams.")),
+        PermissionGroup(
+            name="organizer.teams",
+            label=_("Teams"),
+            actions=["write"],
+            options=[
+                PermissionOption(actions=tuple(), label=pgettext_lazy("permission_level", "No access")),
+                PermissionOption(actions=("write",), label=pgettext_lazy("permission_level", "View and change"),
+                                 help_text=_("Includes the ability to give someone (including oneself) additional permissions.")),
+            ],
+        ),
+        PermissionGroup(
+            name="organizer.giftcards",
+            label=_("Gift cards"),
+            actions=["read", "write"],
+            options=OPTS_READ_WRITE,
+        ),
+        PermissionGroup(
+            name="organizer.customers",
+            label=_("Customers"),
+            actions=["read", "write"],
+            options=OPTS_READ_WRITE,
+        ),
+        PermissionGroup(
+            name="organizer.reusablemedia",
+            label=_("Reusable media"),
+            actions=["read", "write"],
+            options=OPTS_READ_WRITE,
+        ),
+        PermissionGroup(
+            name="organizer.devices",
+            label=_("Devices"),
+            actions=["read", "write"],
+            options=[
+                PermissionOption(actions=tuple(), label=pgettext_lazy("permission_level", "No access")),
+                PermissionOption(actions=("read",), label=pgettext_lazy("permission_level", "View")),
+                PermissionOption(actions=("read", "write"), label=pgettext_lazy("permission_level", "View and change"),
+                                 help_text=_("Includes the ability to give access to events and data oneself does not have access to.")),
+            ],
+        ),
+        PermissionGroup(
+            name="organizer.seatingplans",
+            label=_("Seating plans"),
+            actions=["write"],
+            options=OPTS_ALL_READ,
+        ),
     ]

src/pretix/base/signals.py

@@ -561,16 +561,16 @@ however for this signal, the ``sender`` **may also be None** to allow creating t
 notification settings!
 """
 
-register_event_permissions = GlobalSignal()
+register_event_permission_groups = GlobalSignal()
 """
 This signal is sent out to get all known permissions. Receivers should return an
-instance of pretix.base.permissions.Permission or a list of such instances.
+instance of pretix.base.permissions.PermissionGroup or a list of such instances.
 """
 
-register_organizer_permissions = GlobalSignal()
+register_organizer_permission_groups = GlobalSignal()
 """
 This signal is sent out to get all known permissions. Receivers should return an
-instance of pretix.base.permissions.Permission or a list of such instances.
+instance of pretix.base.permissions.PermissionGroup or a list of such instances.
 """
 
 notification = EventPluginSignal()

src/pretix/control/forms/organizer.py

@@ -77,7 +77,7 @@ from pretix.base.models import (
 from pretix.base.models.customers import CustomerSSOClient, CustomerSSOProvider
 from pretix.base.models.organizer import OrganizerFooterLink, TeamQuerySet
 from pretix.base.permissions import (
-    get_all_event_permissions, get_all_organizer_permissions,
+    get_all_event_permission_groups, get_all_organizer_permission_groups,
 )
 from pretix.base.settings import (
     PERSON_NAME_SCHEMES, PERSON_NAME_TITLE_GROUPS, validate_organizer_settings,
@@ -335,34 +335,62 @@ class TeamForm(forms.ModelForm):
         self.fields['limit_events'].queryset = organizer.events.all().order_by(
             '-has_subevents', '-date_from'
         )
-        self.fields['limit_event_permissions'] = PermissionMultipleChoiceField(
+        self.event_field_names = []
-            label=self.fields['limit_event_permissions'].label,
+        for pg in get_all_event_permission_groups().values():
-            choices=[
+            initial = ",".join(sorted(
-                (p.name, self._make_label(p)) for p in get_all_event_permissions().values()
+                a for a in pg.actions if self.instance and self.instance.limit_event_permissions.get(f"{pg.name}:{a}")
-            ],
+            )) or "EMPTY"
-            widget=forms.CheckboxSelectMultiple(attrs={
+            self.fields[f'event_{pg.name}'] = forms.ChoiceField(
-                'data-inverse-dependency': '#id_all_event_permissions',
+                choices=[
-                'class': 'scrolling-multiple-choice scrolling-multiple-choice-large',
+                    (
-            }),
+                        ",".join(sorted(opt.actions)) or "EMPTY",
-            required=False,
+                        format_html(
-        )
+                            '{label} '
-        self.fields['limit_organizer_permissions'] = PermissionMultipleChoiceField(
+                            '<span class="fa fa-question-circle fa-fw text-muted" data-toggle="tooltip"'
-            label=self.fields['limit_organizer_permissions'].label,
+                            ' data-placement="right" title="{help_text}"></span>',
-            choices=[
+                            label=opt.label,
-                (p.name, self._make_label(p)) for p in get_all_organizer_permissions().values()
+                            help_text=opt.help_text,
-            ],
+                        ) if opt.help_text else opt.label,
-            widget=forms.CheckboxSelectMultiple(attrs={
+                    )
-                'data-inverse-dependency': '#id_all_organizer_permissions',
+                    for opt in pg.options
-                'class': 'scrolling-multiple-choice scrolling-multiple-choice-large',
+                ],
-            }),
+                label=pg.label,
-            required=False,
+                help_text=pg.help_text,
-        )
+                initial=initial,
+                widget=forms.RadioSelect,
+            )
+            self.event_field_names.append(f'event_{pg.name}')
+        self.organizer_field_names = []
+        for pg in get_all_organizer_permission_groups().values():
+            initial = ",".join(sorted(
+                a for a in pg.actions if self.instance and self.instance.limit_organizer_permissions.get(f"{pg.name}:{a}")
+            )) or "EMPTY"
+            self.fields[f'organizer_{pg.name}'] = forms.ChoiceField(
+                choices=[
+                    (
+                        ",".join(sorted(opt.actions)) or "EMPTY",
+                        format_html(
+                            '{label} '
+                            '<span class="fa fa-question-circle fa-fw text-muted" data-toggle="tooltip"'
+                            ' data-placement="right" title="{help_text}"></span>',
+                            label=opt.label,
+                            help_text=opt.help_text,
+                        ) if opt.help_text else opt.label,
+                    )
+                    for opt in pg.options
+                ],
+                label=pg.label,
+                help_text=pg.help_text,
+                initial=initial,
+                widget=forms.RadioSelect,
+            )
+            self.organizer_field_names.append(f'organizer_{pg.name}')
 
     class Meta:
         model = Team
         fields = ['name', 'require_2fa', 'all_events', 'limit_events',
-                  'all_event_permissions', 'limit_event_permissions',
+                  'all_event_permissions',
-                  'all_organizer_permissions', 'limit_organizer_permissions']
+                  'all_organizer_permissions',]
         widgets = {
             'limit_events': forms.CheckboxSelectMultiple(attrs={
                 'data-inverse-dependency': '#id_all_events',
@@ -375,6 +403,33 @@ class TeamForm(forms.ModelForm):
 
     def clean(self):
         data = super().clean()
+
+        data['limit_event_permissions'] = {}
+        if not data['all_event_permissions']:
+            for pg in get_all_event_permission_groups().values():
+                selected = data.get(f'event_{pg.name}', 'EMPTY')
+                if selected == "EMPTY":
+                    selected_actions = []
+                else:
+                    selected_actions = selected.split(',')
+                for action in pg.actions:
+                    if action in selected_actions:
+                        data['limit_event_permissions'][f"{pg.name}:{action}"] = True
+        self.instance.limit_event_permissions = data['limit_event_permissions']
+
+        data['limit_organizer_permissions'] = {}
+        if not data['all_organizer_permissions']:
+            for pg in get_all_organizer_permission_groups().values():
+                selected = data.get(f'organizer_{pg.name}', 'EMPTY')
+                if selected == "EMPTY":
+                    selected_actions = []
+                else:
+                    selected_actions = selected.split(',')
+                for action in pg.actions:
+                    if action in selected_actions:
+                        data['limit_organizer_permissions'][f"{pg.name}:{action}"] = True
+        self.instance.limit_organizer_permissions = data['limit_organizer_permissions']
+
         if self.instance.pk and not data['all_organizer_permissions'] and 'organizer.teams:write' not in data.get('limit_organizer_permissions', []):
             if not self.instance.organizer.teams.exclude(pk=self.instance.pk).filter(
                 TeamQuerySet.organizer_permission_q("organizer.teams:write"),
@@ -385,6 +440,20 @@ class TeamForm(forms.ModelForm):
 
         return data
 
+    @property
+    def changed_data_for_log(self):
+        r = {}
+        for k in self.changed_data:
+            if k == "limit_events":
+                r[k] = [e.id for e in getattr(self.instance, k).all()]
+            elif k.startswith("event_"):
+                r["limit_event_permissions"] = self.instance.limit_event_permissions
+            elif k.startswith("organizer_"):
+                r["limit_organizer_permissions"] = self.instance.limit_organizer_permissions
+            else:
+                r[k] = getattr(self.instance, k)
+        return r
+
 
 class GateForm(forms.ModelForm):
 

src/pretix/control/templates/pretixcontrol/organizers/team_edit.html

@@ -1,6 +1,7 @@
 {% extends "pretixcontrol/organizers/base.html" %}
 {% load i18n %}
 {% load bootstrap3 %}
+{% load getitem %}
 {% block inner %}
     {% if team %}
         <h1>{% trans "Team:" %} {{ team.name }}</h1>
@@ -23,7 +24,11 @@
         <fieldset>
             <legend>{% trans "Organizer permissions" %}</legend>
             {% bootstrap_field form.all_organizer_permissions layout="control" %}
-            {% bootstrap_field form.limit_organizer_permissions layout="control" %}
+            <div class="team-permission-groups col-md-9 col-md-offset-3" data-display-dependency="#id_all_organizer_permissions" data-inverse>
+                {% for f in form.organizer_field_names %}
+                    {% bootstrap_field form|getitem:f layout="control" %}
+                {% endfor %}
+            </div>
         </fieldset>
         <fieldset>
             <legend>{% trans "Event permissions" %}</legend>
@@ -31,7 +36,11 @@
             {% bootstrap_field form.all_events layout="control" %}
             {% bootstrap_field form.limit_events layout="control" %}
             {% bootstrap_field form.all_event_permissions layout="control" %}
-            {% bootstrap_field form.limit_event_permissions layout="control" %}
+            <div class="team-permission-groups col-md-9 col-md-offset-3" data-display-dependency="#id_all_event_permissions" data-inverse>
+                {% for f in form.event_field_names %}
+                    {% bootstrap_field form|getitem:f layout="control" %}
+                {% endfor %}
+            </div>
         </fieldset>
         <div class="form-group submit-group">
             <button type="submit" class="btn btn-primary btn-save">

src/pretix/control/views/organizer.py

@@ -903,10 +903,7 @@ class TeamCreateView(OrganizerDetailViewMixin, OrganizerPermissionRequiredMixin,
         form.instance.organizer = self.request.organizer
         ret = super().form_valid(form)
         form.instance.members.add(self.request.user)
-        form.instance.log_action('pretix.team.created', user=self.request.user, data={
+        form.instance.log_action('pretix.team.created', user=self.request.user, data=form.changed_data_for_log)
-            k: getattr(self.object, k) if k != 'limit_events' else [e.id for e in getattr(self.object, k).all()]
-            for k in form.changed_data
-        })
         return ret
 
     def form_invalid(self, form):
@@ -938,10 +935,7 @@ class TeamUpdateView(OrganizerDetailViewMixin, OrganizerPermissionRequiredMixin,
     @transaction.atomic
     def form_valid(self, form):
         if form.has_changed():
-            self.object.log_action('pretix.team.changed', user=self.request.user, data={
+            self.object.log_action('pretix.team.changed', user=self.request.user, data=form.changed_data_for_log)
-                k: getattr(self.object, k) if k != 'limit_events' else [e.id for e in getattr(self.object, k).all()]
-                for k in form.changed_data
-            })
         messages.success(self.request, _('Your changes have been saved.'))
         return super().form_valid(form)
 

src/pretix/static/pretixcontrol/scss/_forms.scss

@@ -417,6 +417,30 @@ div.scrolling-multiple-choice, div.scrolling-choice {
     }
   }
 }
+.team-permission-groups {
+  border: 1px solid $input-border;
+  border-radius: $input-border-radius;
+  padding: 10px 15px;
+
+  .radio {
+    display: inline-block;
+    margin-right: 10px;
+    &:not(:has(.fa-fw)) {
+      // Visual adjustment between options with and without help text
+      &:after {
+        display: inline-block;
+        content: " ";
+        padding-right: 1.28571em;
+      }
+    }
+  }
+  .control-label {
+    text-align: left;
+  }
+  .form-group:last-child, .help-block {
+    margin-bottom: 0;
+  }
+}
 table td > .checkbox {
   margin: 0;
   position: static;

src/tests/api/test_teams.py

@@ -137,6 +137,18 @@ def test_team_update(token_client, organizer, event, team, second_team):
     assert resp.status_code == 400
     assert "Do not set both" in str(resp.data)
 
+    resp = token_client.patch(
+        '/api/v1/organizers/{}/teams/{}/'.format(organizer.slug, second_team.pk),
+        {
+            'all_organizer_permissions': False,
+            'limit_organizer_permissions': ["organizer.devices:write"],
+        },
+        format='json'
+    )
+    assert resp.status_code == 400
+    assert ("For permission group organizer.devices, the valid combinations of actions are '' or 'read' or "
+            "'read,write' but you tried to set 'write'.") in str(resp.data)
+
     resp = token_client.patch(
         '/api/v1/organizers/{}/teams/{}/'.format(organizer.slug, second_team.pk),
         {

src/tests/control/test_teams.py

@@ -229,12 +229,28 @@ def test_team_remove_last_admin(event, admin_user, admin_team, client):
 @pytest.mark.django_db
 def test_create_team(event, admin_user, admin_team, client):
     client.login(email='dummy@dummy.dummy', password='dummy')
-    client.post('/control/organizer/dummy/team/add', {
+    r = client.post('/control/organizer/dummy/team/add', {
         'name': 'Foo',
-        'limit_organizer_permissions': ['organizer.events:create'],
+        'organizer_organizer.events': "create",
+        'organizer_organizer.settings.general': "EMPTY",
+        'organizer_organizer.teams': "EMPTY",
+        'organizer_organizer.giftcards': "EMPTY",
+        'organizer_organizer.customers': "EMPTY",
+        'organizer_organizer.reusablemedia': "EMPTY",
+        'organizer_organizer.devices': "EMPTY",
+        'organizer_organizer.seatingplans': "EMPTY",
+        'event_event.settings.general': "write",
+        'event_event.settings.payment': "EMPTY",
+        'event_event.settings.tax': "EMPTY",
+        'event_event.settings.invoicing': "EMPTY",
+        'event_event.subevents': "EMPTY",
+        'event_event.items': "EMPTY",
+        'event_event.orders': "EMPTY",
+        'event_event.vouchers': "EMPTY",
+        'event_event': "EMPTY",
         'limit_events': str(event.pk),
-        'limit_event_permissions': ['event.settings.general:write']
     }, follow=True)
+    assert 'alert-success' in r.content.decode()
     with scopes_disabled():
         t = Team.objects.last()
         assert not t.all_event_permissions
@@ -248,13 +264,30 @@ def test_create_team(event, admin_user, admin_team, client):
 @pytest.mark.django_db
 def test_update_team(event, admin_user, admin_team, client):
     client.login(email='dummy@dummy.dummy', password='dummy')
-    client.post('/control/organizer/dummy/team/{}/edit'.format(admin_team.pk), {
+    r = client.post('/control/organizer/dummy/team/{}/edit'.format(admin_team.pk), {
         'name': 'Admin',
-        'limit_organizer_permissions': ['organizer.teams:write'],
         'limit_events': str(event.pk),
         'all_event_permissions': 'on',
         'all_organizer_permissions': '',
+        'organizer_organizer.events': "EMPTY",
+        'organizer_organizer.settings.general': "EMPTY",
+        'organizer_organizer.teams': "write",
+        'organizer_organizer.giftcards': "EMPTY",
+        'organizer_organizer.customers': "EMPTY",
+        'organizer_organizer.reusablemedia': "EMPTY",
+        'organizer_organizer.devices': "EMPTY",
+        'organizer_organizer.seatingplans': "EMPTY",
+        'event_event.settings.general': "write",
+        'event_event.settings.payment': "EMPTY",
+        'event_event.settings.tax': "EMPTY",
+        'event_event.settings.invoicing': "EMPTY",
+        'event_event.subevents': "EMPTY",
+        'event_event.items': "EMPTY",
+        'event_event.orders': "EMPTY",
+        'event_event.vouchers': "EMPTY",
+        'event_event': "EMPTY",
     }, follow=True)
+    assert 'alert-success' in r.content.decode()
     admin_team.refresh_from_db()
     assert admin_team.all_event_permissions
     assert admin_team.limit_event_permissions == {}
@@ -269,7 +302,23 @@ def test_update_last_team_to_be_no_admin(event, admin_user, admin_team, client):
     client.login(email='dummy@dummy.dummy', password='dummy')
     resp = client.post('/control/organizer/dummy/team/{}/edit'.format(admin_team.pk), {
         'name': 'Admin',
-        'event.settings.general:write': 'on'
+        'organizer_organizer.events': "write",
+        'organizer_organizer.settings.general': "EMPTY",
+        'organizer_organizer.teams': "EMPTY",
+        'organizer_organizer.giftcards': "EMPTY",
+        'organizer_organizer.customers': "EMPTY",
+        'organizer_organizer.reusablemedia': "EMPTY",
+        'organizer_organizer.devices': "EMPTY",
+        'organizer_organizer.seatingplans': "EMPTY",
+        'event_event.settings.general': "write",
+        'event_event.settings.payment': "EMPTY",
+        'event_event.settings.tax': "EMPTY",
+        'event_event.settings.invoicing': "EMPTY",
+        'event_event.subevents': "EMPTY",
+        'event_event.items': "EMPTY",
+        'event_event.orders': "EMPTY",
+        'event_event.vouchers': "EMPTY",
+        'event_event': "EMPTY",
     }, follow=True)
     assert 'alert-danger' in resp.content.decode()