From 4f34c0478e1f75960306ddefbf8071afb35b7633 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 22 Jun 2022 16:00:44 -0300 Subject: [PATCH] fix: Authorize interim session requests using data-driven group roles (#4120) * fix: only use non-empty Q object as interim group filter * refactor: add with_meetings queryset to GroupManager * test: users can only request interims for managed groups * fix: find managed groups from groupman_roles/authroles * feat: let chair manage directorate groups * test: remove debug statements and unused imports * test: remove do-nothing code from test --- .../0056_dir_chair_groupman_role.py | 30 ++++++++++++++++ ietf/group/models.py | 4 +++ ietf/group/utils.py | 22 ++++++++++++ ietf/meeting/forms.py | 34 ++++++------------- ietf/meeting/tests_forms.py | 18 ++++++++-- ietf/meeting/tests_views.py | 3 +- ietf/name/fixtures/names.json | 2 +- 7 files changed, 84 insertions(+), 29 deletions(-) create mode 100644 ietf/group/migrations/0056_dir_chair_groupman_role.py diff --git a/ietf/group/migrations/0056_dir_chair_groupman_role.py b/ietf/group/migrations/0056_dir_chair_groupman_role.py new file mode 100644 index 000000000..b69eb03ae --- /dev/null +++ b/ietf/group/migrations/0056_dir_chair_groupman_role.py @@ -0,0 +1,30 @@ +# Generated by Django 2.2.28 on 2022-06-14 13:14 + +from django.db import migrations + + +def forward(apps, schema_editor): + GroupFeatures = apps.get_model('group', 'GroupFeatures') + features = GroupFeatures.objects.get(type_id='dir') + if 'chair' not in features.groupman_roles: + features.groupman_roles.append('chair') + features.save() + + +def reverse(apps, schema_editor): + GroupFeatures = apps.get_model('group', 'GroupFeatures') + features = GroupFeatures.objects.get(type_id='dir') + if 'chair' in features.groupman_roles: + features.groupman_roles.remove('chair') + features.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ('group', '0055_editorial_stream'), + ] + + operations = [ + migrations.RunPython(forward, reverse), + ] diff --git a/ietf/group/models.py b/ietf/group/models.py index 3eaa9cb4e..d598d862d 100644 --- a/ietf/group/models.py +++ b/ietf/group/models.py @@ -109,6 +109,10 @@ class GroupManager(models.Manager): def closed_wgs(self): return self.wgs().exclude(state__in=Group.ACTIVE_STATE_IDS) + def with_meetings(self): + return self.get_queryset().filter(type__features__has_meetings=True) + + class Group(GroupInfo): objects = GroupManager() diff --git a/ietf/group/utils.py b/ietf/group/utils.py index e6628ca41..8a5fd3728 100644 --- a/ietf/group/utils.py +++ b/ietf/group/utils.py @@ -120,6 +120,28 @@ def can_manage_group(user, group): return True return group.has_role(user, group.features.groupman_roles) +def groups_managed_by(user, group_queryset=None): + """Find groups user can manage""" + if group_queryset is None: + group_queryset = Group.objects.all() + query_terms = Q(pk__in=[]) # ensure empty set is returned if no other terms are added + if user.is_authenticated or user.person: + # find the GroupTypes entirely managed by this user based on groupman_authroles + types_can_manage = [] + for type_id, groupman_authroles in GroupFeatures.objects.values_list('type_id', 'groupman_authroles'): + if has_role(user, groupman_authroles): + types_can_manage.append(type_id) + query_terms |= Q(type_id__in=types_can_manage) + # find the Groups managed by this user based on groupman_roles + groups_can_manage = [] + for group_id, role_name, groupman_roles in user.person.role_set.values_list( + 'group_id', 'name_id', 'group__type__features__groupman_roles' + ): + if role_name in groupman_roles: + groups_can_manage.append(group_id) + query_terms |= Q(pk__in=groups_can_manage) + return group_queryset.filter(query_terms) + def milestone_reviewer_for_group_type(group_type): if group_type == "rg": return "IRTF Chair" diff --git a/ietf/meeting/forms.py b/ietf/meeting/forms.py index 3785cee4a..04885cfc8 100644 --- a/ietf/meeting/forms.py +++ b/ietf/meeting/forms.py @@ -14,15 +14,14 @@ from django import forms from django.conf import settings from django.core import validators from django.core.exceptions import ValidationError -from django.db.models import Q from django.forms import BaseInlineFormSet from django.utils.functional import cached_property import debug # pyflakes:ignore from ietf.doc.models import Document, DocAlias, State, NewRevisionDocEvent -from ietf.group.models import Group, GroupFeatures -from ietf.ietfauth.utils import has_role +from ietf.group.models import Group +from ietf.group.utils import groups_managed_by from ietf.meeting.models import Session, Meeting, Schedule, countries, timezones, TimeSlot, Room from ietf.meeting.helpers import get_next_interim_number, make_materials_directories from ietf.meeting.helpers import is_interim_meeting_approved, get_next_agenda_name @@ -106,10 +105,7 @@ class InterimSessionInlineFormSet(BaseInlineFormSet): class InterimMeetingModelForm(forms.ModelForm): group = GroupModelChoiceField( - queryset=Group.objects.filter( - type_id__in=GroupFeatures.objects.filter( - has_meetings=True - ).values_list('type_id',flat=True), + queryset=Group.objects.with_meetings().filter( state__in=('active', 'proposed', 'bof') ).order_by('acronym'), required=False, @@ -187,24 +183,14 @@ class InterimMeetingModelForm(forms.ModelForm): return True def set_group_options(self): - '''Set group options based on user accessing the form''' - if has_role(self.user, "Secretariat"): - return # don't reduce group options - q_objects = Q() - if has_role(self.user, "Area Director"): - q_objects.add(Q(type__in=["wg", "ag", "team"], state__in=("active", "proposed", "bof")), Q.OR) - if has_role(self.user, "IRTF Chair"): - q_objects.add(Q(type__in=["rg", "rag"], state__in=("active", "proposed")), Q.OR) - if has_role(self.user, "WG Chair"): - q_objects.add(Q(type="wg", state__in=("active", "proposed", "bof"), role__person=self.person, role__name="chair"), Q.OR) - if has_role(self.user, "RG Chair"): - q_objects.add(Q(type="rg", state__in=("active", "proposed"), role__person=self.person, role__name="chair"), Q.OR) - if has_role(self.user, "Program Lead") or has_role(self.user, "Program Chair"): - q_objects.add(Q(type="program", state__in=("active", "proposed"), role__person=self.person, role__name__in=["chair", "lead"]), Q.OR) - - queryset = Group.objects.filter(q_objects).distinct().order_by('acronym') + """Set group options based on user accessing the form""" + queryset = groups_managed_by( + self.user, + Group.objects.with_meetings(), + ).filter( + state_id__in=['active', 'proposed', 'bof'] + ).order_by('acronym') self.fields['group'].queryset = queryset - # if there's only one possibility make it the default if len(queryset) == 1: self.fields['group'].initial = queryset[0] diff --git a/ietf/meeting/tests_forms.py b/ietf/meeting/tests_forms.py index eb9b5049a..4a06d7786 100644 --- a/ietf/meeting/tests_forms.py +++ b/ietf/meeting/tests_forms.py @@ -3,9 +3,12 @@ """Tests of forms in the Meeting application""" from django.conf import settings from django.core.files.uploadedfile import SimpleUploadedFile -from django.test import override_settings +from django.test import override_settings, RequestFactory -from ietf.meeting.forms import FileUploadForm, ApplyToAllFileUploadForm, InterimSessionModelForm +from ietf.group.factories import GroupFactory +from ietf.meeting.forms import (FileUploadForm, ApplyToAllFileUploadForm, InterimSessionModelForm, + InterimMeetingModelForm) +from ietf.person.factories import PersonFactory from ietf.utils.test_utils import TestCase @@ -119,3 +122,14 @@ class InterimSessionModelFormTests(TestCase): choice_vals = [choice[0] for choice in form.fields['remote_participation'].choices] self.assertNotIn('meetecho', choice_vals) self.assertIn('manual', choice_vals) + + +class InterimMeetingModelFormTests(TestCase): + def test_enforces_authroles(self): + """User can only request sessions for groups they can manage""" + GroupFactory(type_id='wg', state_id='active') + request = RequestFactory().get('/some/url') + request.user = PersonFactory().user + form = InterimMeetingModelForm(request) + self.assertEqual(form.fields['group'].queryset.count(), 0, + 'person with no roles cannot request interims for any group') diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index a3351ffdc..f15f504c2 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -4605,8 +4605,7 @@ class InterimTests(TestCase): r = self.client.get("/meeting/interim/request/") self.assertEqual(r.status_code, 200) q = PyQuery(r.content) - Group.objects.filter(type_id__in=GroupFeatures.objects.filter(has_meetings=True).values_list('type_id',flat=True), state__in=('active', 'proposed', 'bof')) - self.assertEqual(Group.objects.filter(type_id__in=GroupFeatures.objects.filter(has_meetings=True).values_list('type_id',flat=True), state__in=('active', 'proposed', 'bof')).count(), + self.assertEqual(Group.objects.with_meetings().filter(state__in=('active', 'proposed', 'bof')).count(), len(q("#id_group option")) - 1) # -1 for options placeholder self.client.logout() diff --git a/ietf/name/fixtures/names.json b/ietf/name/fixtures/names.json index 260e40da5..59d23a419 100644 --- a/ietf/name/fixtures/names.json +++ b/ietf/name/fixtures/names.json @@ -2757,7 +2757,7 @@ "default_used_roles": "[\n \"ad\",\n \"chair\",\n \"reviewer\",\n \"secr\",\n \"delegate\"\n]", "docman_roles": "[\n \"chair\"\n]", "groupman_authroles": "[\n \"Secretariat\"\n]", - "groupman_roles": "[\n \"ad\",\n \"secr\",\n \"delegate\"\n]", + "groupman_roles": "[\n \"ad\",\n \"secr\",\n \"delegate\",\n \"chair\"\n]", "has_chartering_process": false, "has_default_jabber": false, "has_documents": false,