From 3ec9d7b67887908b838fa19cf18bb6f9323bbbfe Mon Sep 17 00:00:00 2001 From: Henrik Levkowetz Date: Thu, 25 Apr 2019 13:19:30 +0000 Subject: [PATCH] Updated the role handling to use GroupFeatures.groupman_roles consistently for group management access. Fixes a IRTF RG delegate permissions issue. - Legacy-Id: 16160 --- ietf/community/utils.py | 2 +- ietf/group/models.py | 2 +- ietf/group/tests_info.py | 5 +++-- ietf/group/utils.py | 13 +++++++------ ietf/group/views.py | 4 ++-- ietf/review/utils.py | 2 +- 6 files changed, 15 insertions(+), 13 deletions(-) diff --git a/ietf/community/utils.py b/ietf/community/utils.py index e2d284b59..de1064645 100644 --- a/ietf/community/utils.py +++ b/ietf/community/utils.py @@ -48,7 +48,7 @@ def can_manage_community_list(user, clist): return True if clist.group.type_id in ['area', 'wg', 'rg', 'ag', 'program', ]: - return Role.objects.filter(name__slug__in=clist.group.features.admin_roles, person__user=user, group=clist.group).exists() + return Role.objects.filter(name__slug__in=clist.group.features.groupman_roles, person__user=user, group=clist.group).exists() return False diff --git a/ietf/group/models.py b/ietf/group/models.py index 2ec0aca9a..3bab6e071 100644 --- a/ietf/group/models.py +++ b/ietf/group/models.py @@ -229,7 +229,7 @@ class GroupFeatures(models.Model): about_page = models.CharField(max_length=64, blank=False, default="ietf.group.views.group_about" ) default_tab = models.CharField(max_length=64, blank=False, default="ietf.group.views.group_about" ) material_types = jsonfield.JSONField(max_length=64, blank=False, default=["slides"]) - admin_roles = jsonfield.JSONField(max_length=64, blank=False, default=["chair"]) + admin_roles = jsonfield.JSONField(max_length=64, blank=False, default=["chair"]) # Trac Admin docman_roles = jsonfield.JSONField(max_length=128, blank=False, default=["ad","chair","delegate","secr"]) groupman_roles = jsonfield.JSONField(max_length=128, blank=False, default=["ad","chair",]) matman_roles = jsonfield.JSONField(max_length=128, blank=False, default=["ad","chair","delegate","secr"]) diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 01b3e7160..c6e7bf186 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -866,7 +866,7 @@ class MilestoneTests(TestCase): r = self.client.get(url) self.assertEqual(r.status_code, 200) - milestones_before = GroupMilestone.objects.count() + milestones_before = GroupMilestone.objects.filter(group=group).count() events_before = group.groupevent_set.count() due = self.last_day_of_month(datetime.date.today() + datetime.timedelta(days=365)) @@ -881,7 +881,8 @@ class MilestoneTests(TestCase): 'action': "save", }) self.assertEqual(r.status_code, 302) - self.assertEqual(GroupMilestone.objects.count(), milestones_before + 1) + m = GroupMilestone.objects.filter(group=group) + self.assertEqual(m.count(), milestones_before + 1) m = GroupMilestone.objects.get(desc="Test 3") self.assertEqual(m.state_id, "review") diff --git a/ietf/group/utils.py b/ietf/group/utils.py index 8ae517dd2..01f366987 100644 --- a/ietf/group/utils.py +++ b/ietf/group/utils.py @@ -98,6 +98,8 @@ def save_milestone_in_history(milestone): return h def can_manage_group_type(user, group, type_id=None): + if not user.is_authenticated: + return False if type_id is None: type_id = group.type_id log.assertion("isinstance(type_id, (type(''), type(u'')))") @@ -117,7 +119,7 @@ def can_manage_group_type(user, group, type_id=None): def can_manage_group(user, group): if can_manage_group_type(user, group): return True - return group.has_role(user, group.features.admin_roles) + return group.has_role(user, group.features.groupman_roles) def milestone_reviewer_for_group_type(group_type): if group_type == "rg": @@ -203,12 +205,11 @@ def construct_group_menu_context(request, group, selected, group_type, others): # actions actions = [] - is_admin = group.has_role(request.user, group.features.admin_roles) - can_manage = can_manage_group_type(request.user, group) + can_manage = can_manage_group(request.user, group) can_edit_group = False # we'll set this further down if group.features.has_milestones: - if group.state_id != "proposed" and (is_admin or can_manage): + if group.state_id != "proposed" and can_manage: actions.append((u"Edit milestones", urlreverse('ietf.group.milestones.edit_milestones;current', kwargs=kwargs))) if group.features.has_documents: @@ -229,11 +230,11 @@ def construct_group_menu_context(request, group, selected, group_type, others): actions.append((u"Secretary settings", urlreverse(ietf.group.views.change_review_secretary_settings, kwargs=kwargs))) actions.append((u"Email open assignments summary", urlreverse(ietf.group.views.email_open_review_assignments, kwargs=dict(acronym=group.acronym, group_type=group.type_id)))) - if group.state_id != "conclude" and (is_admin or can_manage): + if group.state_id != "conclude" and can_manage: can_edit_group = True actions.append((u"Edit group", urlreverse("ietf.group.views.edit", kwargs=dict(kwargs, action="edit")))) - if group.features.customize_workflow and (is_admin or can_manage): + if group.features.customize_workflow and can_manage: actions.append((u"Customize workflow", urlreverse("ietf.group.views.customize_workflow", kwargs=kwargs))) if group.state_id in ("active", "dormant") and not group.type_id in ["sdo", "rfcedtyp", "isoc", ] and can_manage: diff --git a/ietf/group/views.py b/ietf/group/views.py index c6a405439..f02585138 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -899,7 +899,7 @@ def edit(request, group_type=None, acronym=None, action="edit", field=None): if not group_type and group: group_type = group.type_id if not (can_manage_group(request.user, group) - or group.has_role(request.user, group.features.admin_roles)): + or group.has_role(request.user, group.features.groupman_roles)): return HttpResponseForbidden("You don't have permission to access this view") if request.method == 'POST': @@ -1088,7 +1088,7 @@ def customize_workflow(request, group_type=None, acronym=None): raise Http404 if not (can_manage_group(request.user, group) - or group.has_role(request.user, group.features.admin_roles)): + or group.has_role(request.user, group.features.groupman_roles)): return HttpResponseForbidden("You don't have permission to access this view") if group_type == "rg": diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 32994b329..eeb809acd 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -718,7 +718,7 @@ def setup_reviewer_field(field, review_req): def get_default_filter_re(person): if type(person) != Person: person = Person.objects.get(id=person) - groups_to_avoid = [ r.group for r in person.role_set.all() if r.name in r.group.features.admin_roles and r.group.features.acts_like_wg ] + groups_to_avoid = [ r.group for r in person.role_set.all() if r.name in r.group.features.groupman_roles and r.group.features.acts_like_wg ] if not groups_to_avoid: return '^draft-%s-.*$' % ( person.last_name().lower(), ) else: