Skip to content

Commit 4f34c04

Browse files
fix: Authorize interim session requests using data-driven group roles (ietf-tools#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
1 parent 1b95fdf commit 4f34c04

7 files changed

Lines changed: 84 additions & 29 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Generated by Django 2.2.28 on 2022-06-14 13:14
2+
3+
from django.db import migrations
4+
5+
6+
def forward(apps, schema_editor):
7+
GroupFeatures = apps.get_model('group', 'GroupFeatures')
8+
features = GroupFeatures.objects.get(type_id='dir')
9+
if 'chair' not in features.groupman_roles:
10+
features.groupman_roles.append('chair')
11+
features.save()
12+
13+
14+
def reverse(apps, schema_editor):
15+
GroupFeatures = apps.get_model('group', 'GroupFeatures')
16+
features = GroupFeatures.objects.get(type_id='dir')
17+
if 'chair' in features.groupman_roles:
18+
features.groupman_roles.remove('chair')
19+
features.save()
20+
21+
22+
class Migration(migrations.Migration):
23+
24+
dependencies = [
25+
('group', '0055_editorial_stream'),
26+
]
27+
28+
operations = [
29+
migrations.RunPython(forward, reverse),
30+
]

ietf/group/models.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,10 @@ def active_wgs(self):
109109
def closed_wgs(self):
110110
return self.wgs().exclude(state__in=Group.ACTIVE_STATE_IDS)
111111

112+
def with_meetings(self):
113+
return self.get_queryset().filter(type__features__has_meetings=True)
114+
115+
112116
class Group(GroupInfo):
113117
objects = GroupManager()
114118

ietf/group/utils.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,28 @@ def can_manage_group(user, group):
120120
return True
121121
return group.has_role(user, group.features.groupman_roles)
122122

123+
def groups_managed_by(user, group_queryset=None):
124+
"""Find groups user can manage"""
125+
if group_queryset is None:
126+
group_queryset = Group.objects.all()
127+
query_terms = Q(pk__in=[]) # ensure empty set is returned if no other terms are added
128+
if user.is_authenticated or user.person:
129+
# find the GroupTypes entirely managed by this user based on groupman_authroles
130+
types_can_manage = []
131+
for type_id, groupman_authroles in GroupFeatures.objects.values_list('type_id', 'groupman_authroles'):
132+
if has_role(user, groupman_authroles):
133+
types_can_manage.append(type_id)
134+
query_terms |= Q(type_id__in=types_can_manage)
135+
# find the Groups managed by this user based on groupman_roles
136+
groups_can_manage = []
137+
for group_id, role_name, groupman_roles in user.person.role_set.values_list(
138+
'group_id', 'name_id', 'group__type__features__groupman_roles'
139+
):
140+
if role_name in groupman_roles:
141+
groups_can_manage.append(group_id)
142+
query_terms |= Q(pk__in=groups_can_manage)
143+
return group_queryset.filter(query_terms)
144+
123145
def milestone_reviewer_for_group_type(group_type):
124146
if group_type == "rg":
125147
return "IRTF Chair"

ietf/meeting/forms.py

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,14 @@
1414
from django.conf import settings
1515
from django.core import validators
1616
from django.core.exceptions import ValidationError
17-
from django.db.models import Q
1817
from django.forms import BaseInlineFormSet
1918
from django.utils.functional import cached_property
2019

2120
import debug # pyflakes:ignore
2221

2322
from ietf.doc.models import Document, DocAlias, State, NewRevisionDocEvent
24-
from ietf.group.models import Group, GroupFeatures
25-
from ietf.ietfauth.utils import has_role
23+
from ietf.group.models import Group
24+
from ietf.group.utils import groups_managed_by
2625
from ietf.meeting.models import Session, Meeting, Schedule, countries, timezones, TimeSlot, Room
2726
from ietf.meeting.helpers import get_next_interim_number, make_materials_directories
2827
from ietf.meeting.helpers import is_interim_meeting_approved, get_next_agenda_name
@@ -106,10 +105,7 @@ def clean(self):
106105

107106
class InterimMeetingModelForm(forms.ModelForm):
108107
group = GroupModelChoiceField(
109-
queryset=Group.objects.filter(
110-
type_id__in=GroupFeatures.objects.filter(
111-
has_meetings=True
112-
).values_list('type_id',flat=True),
108+
queryset=Group.objects.with_meetings().filter(
113109
state__in=('active', 'proposed', 'bof')
114110
).order_by('acronym'),
115111
required=False,
@@ -187,24 +183,14 @@ def is_virtual(self):
187183
return True
188184

189185
def set_group_options(self):
190-
'''Set group options based on user accessing the form'''
191-
if has_role(self.user, "Secretariat"):
192-
return # don't reduce group options
193-
q_objects = Q()
194-
if has_role(self.user, "Area Director"):
195-
q_objects.add(Q(type__in=["wg", "ag", "team"], state__in=("active", "proposed", "bof")), Q.OR)
196-
if has_role(self.user, "IRTF Chair"):
197-
q_objects.add(Q(type__in=["rg", "rag"], state__in=("active", "proposed")), Q.OR)
198-
if has_role(self.user, "WG Chair"):
199-
q_objects.add(Q(type="wg", state__in=("active", "proposed", "bof"), role__person=self.person, role__name="chair"), Q.OR)
200-
if has_role(self.user, "RG Chair"):
201-
q_objects.add(Q(type="rg", state__in=("active", "proposed"), role__person=self.person, role__name="chair"), Q.OR)
202-
if has_role(self.user, "Program Lead") or has_role(self.user, "Program Chair"):
203-
q_objects.add(Q(type="program", state__in=("active", "proposed"), role__person=self.person, role__name__in=["chair", "lead"]), Q.OR)
204-
205-
queryset = Group.objects.filter(q_objects).distinct().order_by('acronym')
186+
"""Set group options based on user accessing the form"""
187+
queryset = groups_managed_by(
188+
self.user,
189+
Group.objects.with_meetings(),
190+
).filter(
191+
state_id__in=['active', 'proposed', 'bof']
192+
).order_by('acronym')
206193
self.fields['group'].queryset = queryset
207-
208194
# if there's only one possibility make it the default
209195
if len(queryset) == 1:
210196
self.fields['group'].initial = queryset[0]

ietf/meeting/tests_forms.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@
33
"""Tests of forms in the Meeting application"""
44
from django.conf import settings
55
from django.core.files.uploadedfile import SimpleUploadedFile
6-
from django.test import override_settings
6+
from django.test import override_settings, RequestFactory
77

8-
from ietf.meeting.forms import FileUploadForm, ApplyToAllFileUploadForm, InterimSessionModelForm
8+
from ietf.group.factories import GroupFactory
9+
from ietf.meeting.forms import (FileUploadForm, ApplyToAllFileUploadForm, InterimSessionModelForm,
10+
InterimMeetingModelForm)
11+
from ietf.person.factories import PersonFactory
912
from ietf.utils.test_utils import TestCase
1013

1114

@@ -119,3 +122,14 @@ def test_remote_participation_options(self):
119122
choice_vals = [choice[0] for choice in form.fields['remote_participation'].choices]
120123
self.assertNotIn('meetecho', choice_vals)
121124
self.assertIn('manual', choice_vals)
125+
126+
127+
class InterimMeetingModelFormTests(TestCase):
128+
def test_enforces_authroles(self):
129+
"""User can only request sessions for groups they can manage"""
130+
GroupFactory(type_id='wg', state_id='active')
131+
request = RequestFactory().get('/some/url')
132+
request.user = PersonFactory().user
133+
form = InterimMeetingModelForm(request)
134+
self.assertEqual(form.fields['group'].queryset.count(), 0,
135+
'person with no roles cannot request interims for any group')

ietf/meeting/tests_views.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4605,8 +4605,7 @@ def test_interim_request_options(self):
46054605
r = self.client.get("/meeting/interim/request/")
46064606
self.assertEqual(r.status_code, 200)
46074607
q = PyQuery(r.content)
4608-
Group.objects.filter(type_id__in=GroupFeatures.objects.filter(has_meetings=True).values_list('type_id',flat=True), state__in=('active', 'proposed', 'bof'))
4609-
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(),
4608+
self.assertEqual(Group.objects.with_meetings().filter(state__in=('active', 'proposed', 'bof')).count(),
46104609
len(q("#id_group option")) - 1) # -1 for options placeholder
46114610
self.client.logout()
46124611

ietf/name/fixtures/names.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2757,7 +2757,7 @@
27572757
"default_used_roles": "[\n \"ad\",\n \"chair\",\n \"reviewer\",\n \"secr\",\n \"delegate\"\n]",
27582758
"docman_roles": "[\n \"chair\"\n]",
27592759
"groupman_authroles": "[\n \"Secretariat\"\n]",
2760-
"groupman_roles": "[\n \"ad\",\n \"secr\",\n \"delegate\"\n]",
2760+
"groupman_roles": "[\n \"ad\",\n \"secr\",\n \"delegate\",\n \"chair\"\n]",
27612761
"has_chartering_process": false,
27622762
"has_default_jabber": false,
27632763
"has_documents": false,

0 commit comments

Comments
 (0)