altf4arnold/datatracker
1
0
Fork 0
Code Issues Pull requests Actions 4 Packages Projects Releases Wiki Activity

fix: Don't allow group chair to change group parent (#6496)

Browse source 
* fix: Don't allow group chair to change group parent (#6037)

* test: Fix test_edit_parent_field, add test_edit_parent (whole form)

* test: Verify that the chair can't circumvent the system to change the group parent

* fix: 403 if user tries to edit an unknown or hidden field

* fix: Give edwg GroupFeatures a parent type

This tracks a change that was made directly in the production database
to fix the immediate cause of #6037.

* Empty commit to trigger github unit test
This commit is contained in:
Paul Selkirk 2023-11-05 03:32:03 -08:00 committed by GitHub
parent a3b4162841
commit b5ee9ec9ab
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 109 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
ietf/group/forms.py
View file

@ -1,4 +1,4 @@
# Copyright The IETF Trust 2017-2020, All Rights Reserved
# Copyright The IETF Trust 2017-2023, All Rights Reserved
# -*- coding: utf-8 -*-
@ -103,6 +103,8 @@ class GroupForm(forms.Form):
else:
field = None
self.hide_parent = kwargs.pop('hide_parent', False)
super(self.__class__, self).__init__(*args, **kwargs)
if not group_features or group_features.has_chartering_process:
@ -138,18 +140,21 @@ class GroupForm(forms.Form):
self.fields['acronym'].widget.attrs['readonly'] = ""
# Sort out parent options
self.fields['parent'].queryset = self.fields['parent'].queryset.filter(type__in=parent_types)
if need_parent:
self.fields['parent'].required = True
self.fields['parent'].empty_label = None
# if this is a new group, fill in the default parent, if any
if self.group is None or (not hasattr(self.group, 'pk')):
self.fields['parent'].initial = self.fields['parent'].queryset.filter(
acronym=default_parent
).first()
# label the parent field as 'IETF Area' if appropriate, for consistency with past behavior
if parent_types.count() == 1 and parent_types.first().pk == 'area':
self.fields['parent'].label = "IETF Area"
if self.hide_parent:
self.fields.pop('parent')
else:
self.fields['parent'].queryset = self.fields['parent'].queryset.filter(type__in=parent_types)
if need_parent:
self.fields['parent'].required = True
self.fields['parent'].empty_label = None
# if this is a new group, fill in the default parent, if any
if self.group is None or (not hasattr(self.group, 'pk')):
self.fields['parent'].initial = self.fields['parent'].queryset.filter(
acronym=default_parent
).first()
# label the parent field as 'IETF Area' if appropriate, for consistency with past behavior
if parent_types.count() == 1 and parent_types.first().pk == 'area':
self.fields['parent'].label = "IETF Area"
if field:
keys = list(self.fields.keys())

82
ietf/group/tests_info.py
View file

@ -1,4 +1,4 @@
# Copyright The IETF Trust 2009-2022, All Rights Reserved
# Copyright The IETF Trust 2009-2023, All Rights Reserved
# -*- coding: utf-8 -*-
@ -946,10 +946,88 @@ class GroupEditTests(TestCase):
r = self.client.post(url, {
'description': 'Ignored description',
})
self.assertEqual(r.status_code, 302)
self.assertEqual(r.status_code, 403)
group = Group.objects.get(pk=group.pk) # refresh
self.assertEqual(group.description, 'Updated description')
def test_edit_parent(self):
group = GroupFactory.create(type_id='wg', parent=GroupFactory.create(type_id='area'))
chair = RoleFactory(group=group, name_id='chair').person
url = urlreverse('ietf.group.views.edit', kwargs=dict(group_type=group.type_id, acronym=group.acronym, action='edit'))
# parent is not shown to group chair
login_testing_unauthorized(self, chair.user.username, url)
r = self.client.get(url)
self.assertEqual(r.status_code, 200)
q = PyQuery(r.content)
self.assertEqual(len(q('form select[name=parent]')), 0)
# view ignores attempt to change parent
old_parent = group.parent
new_parent = GroupFactory(type_id='area')
self.assertNotEqual(new_parent.acronym, group.parent.acronym)
r = self.client.post(url, dict(
name=group.name,
acronym=group.acronym,
state=group.state_id,
parent=new_parent.pk))
self.assertEqual(r.status_code, 302)
group = Group.objects.get(pk=group.pk)
self.assertNotEqual(group.parent, new_parent)
self.assertEqual(group.parent, old_parent)
# parent is shown to AD and Secretariat
for priv_user in ('ad', 'secretary'):
self.client.logout()
login_testing_unauthorized(self, priv_user, url)
r = self.client.get(url)
self.assertEqual(r.status_code, 200)
q = PyQuery(r.content)
self.assertEqual(len(q('form select[name=parent]')), 1)
new_parent = GroupFactory(type_id='area')
self.assertNotEqual(new_parent.acronym, group.parent.acronym)
r = self.client.post(url, dict(
name=group.name,
acronym=group.acronym,
state=group.state_id,
parent=new_parent.pk))
self.assertEqual(r.status_code, 302)
group = Group.objects.get(pk=group.pk)
self.assertEqual(group.parent, new_parent)
def test_edit_parent_field(self):
group = GroupFactory.create(type_id='wg', parent=GroupFactory.create(type_id='area'))
chair = RoleFactory(group=group, name_id='chair').person
url = urlreverse('ietf.group.views.edit', kwargs=dict(group_type=group.type_id, acronym=group.acronym, action='edit', field='parent'))
# parent is not shown to group chair
login_testing_unauthorized(self, chair.user.username, url)
r = self.client.get(url)
self.assertEqual(r.status_code, 403)
# chair is not allowed to change parent
new_parent = GroupFactory(type_id='area')
self.assertNotEqual(new_parent.acronym, group.parent.acronym)
r = self.client.post(url, dict(parent=new_parent.pk))
self.assertEqual(r.status_code, 403)
# parent is shown to AD and Secretariat
for priv_user in ('ad', 'secretary'):
self.client.logout()
login_testing_unauthorized(self, priv_user, url)
r = self.client.get(url)
self.assertEqual(r.status_code, 200)
q = PyQuery(r.content)
self.assertEqual(len(q('form select[name=parent]')), 1)
new_parent = GroupFactory(type_id='area')
self.assertNotEqual(new_parent.acronym, group.parent.acronym)
r = self.client.post(url, dict(parent=new_parent.pk))
self.assertEqual(r.status_code, 302)
group = Group.objects.get(pk=group.pk)
self.assertEqual(group.parent, new_parent)
def test_conclude(self):
group = GroupFactory(acronym="mars")

11
ietf/group/views.py
View file

@ -945,14 +945,17 @@ def edit(request, group_type=None, acronym=None, action="edit", field=None):
if not (can_manage_group(request.user, group)
or group.has_role(request.user, group.features.groupman_roles)):
permission_denied(request, "You don't have permission to access this view")
hide_parent = not has_role(request.user, ("Secretariat", "Area Director", "IRTF Chair"))
else:
# This allows ADs to create RG and the IRTF Chair to create WG, but we trust them not to
if not has_role(request.user, ("Secretariat", "Area Director", "IRTF Chair")):
permission_denied(request, "You don't have permission to access this view")
hide_parent = False
if request.method == 'POST':
form = GroupForm(request.POST, group=group, group_type=group_type, field=field)
form = GroupForm(request.POST, group=group, group_type=group_type, field=field, hide_parent=hide_parent)
if field and not form.fields:
permission_denied(request, "You don't have permission to edit this field")
if form.is_valid():
clean = form.cleaned_data
if new_group:
@ -1114,7 +1117,9 @@ def edit(request, group_type=None, acronym=None, action="edit", field=None):
else:
init = dict()
form = GroupForm(initial=init, group=group, group_type=group_type, field=field)
form = GroupForm(initial=init, group=group, group_type=group_type, field=field, hide_parent=hide_parent)
if field and not form.fields:
permission_denied(request, "You don't have permission to edit this field")
return render(request, 'group/edit.html',
dict(group=group,

4
ietf/name/fixtures/names.json
View file

@ -3034,7 +3034,9 @@
"material_types": "[\n \"slides\"\n]",
"matman_roles": "[\n \"chair\"\n]",
"need_parent": false,
"parent_types": [],
"parent_types": [
"rfcedtyp"
],
"req_subm_approval": true,
"role_order": "[\n \"chair\"\n]",
"session_purposes": "[\n \"regular\"\n]",