From 1c509cd64c203913c0fed6b42d4e5f0451a8c84c Mon Sep 17 00:00:00 2001 From: Russ Housley Date: Sat, 2 Apr 2016 15:10:10 +0000 Subject: [PATCH 1/3] The secretariat and the Team Chair can now edit team groups. In addition, if the team in within the IETF, Area Directors can edit it. And, if the team is within the IRTF, the IRTF Chair can edit it. Fixes #1915. - Legacy-Id: 11064 --- ietf/group/edit.py | 17 +++++++++++------ ietf/group/info.py | 6 ++++-- ietf/group/models.py | 8 ++++++++ ietf/group/tests_info.py | 23 ++++++++++++++++++++++- ietf/group/utils.py | 9 +++++++++ 5 files changed, 54 insertions(+), 9 deletions(-) diff --git a/ietf/group/edit.py b/ietf/group/edit.py index b2773d54c..084b9540b 100644 --- a/ietf/group/edit.py +++ b/ietf/group/edit.py @@ -16,7 +16,7 @@ from ietf.doc.utils import get_tags_for_stream_id from ietf.doc.utils_charter import charter_name_for_group from ietf.group.models import ( Group, Role, GroupEvent, GroupHistory, GroupStateName, GroupStateTransitions, GroupTypeName, GroupURL, ChangeStateGroupEvent ) -from ietf.group.utils import save_group_in_history, can_manage_group_type +from ietf.group.utils import save_group_in_history, can_manage_group_type, can_manage_team_group from ietf.group.utils import get_group_or_404 from ietf.ietfauth.utils import has_role from ietf.person.fields import SearchableEmailsField @@ -199,11 +199,19 @@ def submit_initial_charter(request, group_type=None, acronym=None): def edit(request, group_type=None, acronym=None, action="edit"): """Edit or create a group, notifying parties as necessary and logging changes as group events.""" - if not can_manage_group_type(request.user, group_type): + group = get_group_or_404(acronym, group_type) + if not group_type and group: + group_type = group.type_id + + if group_type == "team": + can_edit = can_manage_team_group(request.user, group) or group.has_role(request.user, "chair") + else: + can_edit = can_manage_group_type(request.user, group_type) + + if not can_edit: return HttpResponseForbidden("You don't have permission to access this view") if action == "edit": - group = get_object_or_404(Group, acronym=acronym) new_group = False elif action in ("create","charter"): group = None @@ -211,9 +219,6 @@ def edit(request, group_type=None, acronym=None, action="edit"): else: raise Http404 - if not group_type and group: - group_type = group.type_id - if request.method == 'POST': form = GroupForm(request.POST, group=group, group_type=group_type) if form.is_valid(): diff --git a/ietf/group/info.py b/ietf/group/info.py index c0a82a19d..94ed3b2ec 100644 --- a/ietf/group/info.py +++ b/ietf/group/info.py @@ -55,7 +55,7 @@ from ietf.doc.utils import get_chartering_type from ietf.doc.templatetags.ietf_filters import clean_whitespace from ietf.group.models import Group, Role, ChangeStateGroupEvent from ietf.name.models import GroupTypeName -from ietf.group.utils import get_charter_text, can_manage_group_type, milestone_reviewer_for_group_type, can_provide_status_update +from ietf.group.utils import get_charter_text, can_manage_group_type, can_manage_team_group, milestone_reviewer_for_group_type, can_provide_status_update from ietf.group.utils import can_manage_materials, get_group_or_404 from ietf.utils.pipe import pipe from ietf.utils.textupload import get_cleaned_text_file_content @@ -366,6 +366,8 @@ def construct_group_menu_context(request, group, selected, group_type, others): is_chair = group.has_role(request.user, "chair") can_manage = can_manage_group_type(request.user, group.type_id) + if group.type_id == "team": + can_manage = can_manage_team_group(request.user, group) if group.features.has_milestones: if group.state_id != "proposed" and (is_chair or can_manage): @@ -374,7 +376,7 @@ def construct_group_menu_context(request, group, selected, group_type, others): if group.features.has_materials and can_manage_materials(request.user, group): actions.append((u"Upload material", urlreverse("ietf.doc.views_material.choose_material_type", kwargs=kwargs))) - if group.type_id in ("rg", "wg") and group.state_id != "conclude" and can_manage: + if group.state_id != "conclude" and (is_chair or can_manage): actions.append((u"Edit group", urlreverse("group_edit", kwargs=kwargs))) if group.features.customize_workflow and (is_chair or can_manage): diff --git a/ietf/group/models.py b/ietf/group/models.py index 49fd9569e..68f4e2baf 100644 --- a/ietf/group/models.py +++ b/ietf/group/models.py @@ -80,6 +80,14 @@ class Group(GroupInfo): role_names = [role_names] return user.is_authenticated() and self.role_set.filter(name__in=role_names, person__user=user).exists() + def is_decendant_of(self, sought_parent): + p = self.parent + while (p != None): + if p.acronym == sought_parent: + return True + p = p.parent + return False + def is_bof(self): return (self.state.slug in ["bof", "bof-conc"]) diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index d07053f18..8f87e18a9 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -25,7 +25,7 @@ from ietf.name.models import DocTagName, GroupStateName, GroupTypeName from ietf.person.models import Person, Email from ietf.utils.test_utils import TestCase, unicontent from ietf.utils.mail import outbox, empty_outbox -from ietf.utils.test_data import make_test_data +from ietf.utils.test_data import make_test_data, create_person from ietf.utils.test_utils import login_testing_unauthorized from ietf.group.factories import GroupFactory, RoleFactory, GroupEventFactory from ietf.meeting.factories import SessionFactory @@ -240,6 +240,17 @@ class GroupPagesTests(TestCase): self.assertTrue(milestone.docs.all()[0].name in unicontent(r)) def test_group_about(self): + + def verify_cannot_edit_group(username): + self.client.login(username=username, password=username+"+password") + r = self.client.get(url) + self.assertEqual(r.status_code, 403) + + def verify_can_edit_group(username): + self.client.login(username=username, password=username+"+password") + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + make_test_data() group = Group.objects.create( type_id="team", @@ -247,7 +258,9 @@ class GroupPagesTests(TestCase): name="Test Team", description="The test team is testing.", state_id="active", + parent = Group.objects.get(acronym="farfut"), ) + create_person(group, "chair", name="Testteam Chairman", username="teamchairman") for url in [group.about_url(), urlreverse('ietf.group.info.group_about',kwargs=dict(acronym=group.acronym)), @@ -260,6 +273,14 @@ class GroupPagesTests(TestCase): self.assertTrue(group.acronym in unicontent(r)) self.assertTrue(group.description in unicontent(r)) + url = urlreverse('ietf.group.edit.edit', kwargs=dict(acronym=group.acronym)) + + for username in ['plain','iana','iab chair','irtf chair','marschairman']: + verify_cannot_edit_group(username) + + for username in ['secretary','teamchairman','ad']: + verify_can_edit_group(username) + def test_materials(self): make_test_data() group = Group.objects.create(type_id="team", acronym="testteam", name="Test Team", state_id="active") diff --git a/ietf/group/utils.py b/ietf/group/utils.py index 9d56c70ec..6d41fb95b 100644 --- a/ietf/group/utils.py +++ b/ietf/group/utils.py @@ -91,6 +91,15 @@ def can_manage_group_type(user, group_type): return has_role(user, 'Secretariat') +def can_manage_team_group(user, group): + if group.type_id != "team": + return False + elif group.is_decendant_of("ietf") and has_role(user, ('Area Director', 'Secretariat')): + return True + elif group.is_decendant_of("irtf") and has_role(user, ('IRTF Chair', 'Secretariat')): + return True + return has_role(user, ('Secretariat')) + def milestone_reviewer_for_group_type(group_type): if group_type == "rg": return "IRTF Chair" From 570107dbf16e38286a168cf6abc1d3a5c02b46b5 Mon Sep 17 00:00:00 2001 From: Russ Housley Date: Sat, 2 Apr 2016 17:21:08 +0000 Subject: [PATCH 2/3] Only the Secretariat can see the history for parked IPR statements. Fixes #1922. - Legacy-Id: 11070 --- ietf/ipr/views.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ietf/ipr/views.py b/ietf/ipr/views.py index 6de21b37f..a58191fef 100644 --- a/ietf/ipr/views.py +++ b/ietf/ipr/views.py @@ -370,6 +370,11 @@ def email(request, id): def history(request, id): """Show the history for a specific IPR disclosure""" ipr = get_object_or_404(IprDisclosureBase, id=id).get_child() + + if not has_role(request.user, 'Secretariat'): + if ipr.state.slug != 'posted': + raise Http404 + events = ipr.iprevent_set.all().order_by("-time", "-id").select_related("by") if not has_role(request.user, "Secretariat"): events = events.exclude(type='private_comment') From 858530c21430859c6397de7a72483086139956f4 Mon Sep 17 00:00:00 2001 From: Russ Housley Date: Sat, 2 Apr 2016 21:06:33 +0000 Subject: [PATCH 3/3] Cleaned up the checking permission for a user to manage a group. Also, cleanly handle a set of group parent links did for a loop. - Legacy-Id: 11082 --- ietf/doc/views_charter.py | 8 ++++---- ietf/doc/views_doc.py | 4 ++-- ietf/group/edit.py | 25 ++++++++++-------------- ietf/group/info.py | 8 +++----- ietf/group/milestones.py | 11 ++++++----- ietf/group/models.py | 2 +- ietf/group/tests_info.py | 41 ++++++++++++++++++++++++++++++++++++++- ietf/group/utils.py | 17 +++++++++------- 8 files changed, 76 insertions(+), 40 deletions(-) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index 39b091cff..251324a77 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -23,7 +23,7 @@ from ietf.doc.utils_charter import ( historic_milestones_for_charter, derive_new_work_text ) from ietf.doc.mails import email_state_changed, email_charter_internal_review from ietf.group.models import ChangeStateGroupEvent, MilestoneGroupEvent -from ietf.group.utils import save_group_in_history, save_milestone_in_history, can_manage_group_type +from ietf.group.utils import save_group_in_history, save_milestone_in_history, can_manage_group from ietf.ietfauth.utils import has_role, role_required from ietf.name.models import GroupStateName from ietf.person.models import Person @@ -58,7 +58,7 @@ def change_state(request, name, option=None): charter = get_object_or_404(Document, type="charter", name=name) group = charter.group - if not can_manage_group_type(request.user, group.type_id): + if not can_manage_group(request.user, group): return HttpResponseForbidden("You don't have permission to access this view") chartering_type = get_chartering_type(charter) @@ -246,7 +246,7 @@ def change_title(request, name, option=None): logging the title as a comment.""" charter = get_object_or_404(Document, type="charter", name=name) group = charter.group - if not can_manage_group_type(request.user, group.type_id): + if not can_manage_group(request.user, group): return HttpResponseForbidden("You don't have permission to access this view") login = request.user.person if request.method == 'POST': @@ -359,7 +359,7 @@ def submit(request, name=None, option=None): charter = get_object_or_404(Document, type="charter", name=name) group = charter.group - if not can_manage_group_type(request.user, group.type_id): + if not can_manage_group(request.user, group): return HttpResponseForbidden("You don't have permission to access this view") path = os.path.join(settings.CHARTER_PATH, '%s-%s.txt' % (charter.canonical_name(), charter.rev)) diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 89218495f..edbad1e87 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -52,7 +52,7 @@ from ietf.doc.utils import ( add_links_in_new_revision_events, augment_events_wi get_initial_notify, make_notify_changed_event, crawl_history) from ietf.community.models import CommunityList from ietf.group.models import Role -from ietf.group.utils import can_manage_group_type, can_manage_materials +from ietf.group.utils import can_manage_group, can_manage_materials from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream, user_is_person, role_required from ietf.name.models import StreamName, BallotPositionName from ietf.person.models import Email @@ -445,7 +445,7 @@ def document_main(request, name, rev=None): if chartering and not snapshot: milestones = doc.group.groupmilestone_set.filter(state="charter") - can_manage = can_manage_group_type(request.user, doc.group.type_id) + can_manage = can_manage_group(request.user, doc.group) return render_to_response("doc/document_charter.html", dict(doc=doc, diff --git a/ietf/group/edit.py b/ietf/group/edit.py index 084b9540b..bd9ec2d7c 100644 --- a/ietf/group/edit.py +++ b/ietf/group/edit.py @@ -16,7 +16,7 @@ from ietf.doc.utils import get_tags_for_stream_id from ietf.doc.utils_charter import charter_name_for_group from ietf.group.models import ( Group, Role, GroupEvent, GroupHistory, GroupStateName, GroupStateTransitions, GroupTypeName, GroupURL, ChangeStateGroupEvent ) -from ietf.group.utils import save_group_in_history, can_manage_group_type, can_manage_team_group +from ietf.group.utils import save_group_in_history, can_manage_group from ietf.group.utils import get_group_or_404 from ietf.ietfauth.utils import has_role from ietf.person.fields import SearchableEmailsField @@ -186,7 +186,7 @@ def submit_initial_charter(request, group_type=None, acronym=None): # This is where we start ignoring the passed in group_type group_type = group.type_id - if not can_manage_group_type(request.user, group_type): + if not can_manage_group(request.user, group): return HttpResponseForbidden("You don't have permission to access this view") if not group.charter: @@ -199,18 +199,6 @@ def submit_initial_charter(request, group_type=None, acronym=None): def edit(request, group_type=None, acronym=None, action="edit"): """Edit or create a group, notifying parties as necessary and logging changes as group events.""" - group = get_group_or_404(acronym, group_type) - if not group_type and group: - group_type = group.type_id - - if group_type == "team": - can_edit = can_manage_team_group(request.user, group) or group.has_role(request.user, "chair") - else: - can_edit = can_manage_group_type(request.user, group_type) - - if not can_edit: - return HttpResponseForbidden("You don't have permission to access this view") - if action == "edit": new_group = False elif action in ("create","charter"): @@ -219,6 +207,13 @@ def edit(request, group_type=None, acronym=None, action="edit"): else: raise Http404 + if not new_group: + group = get_group_or_404(acronym, group_type) + 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, "chair")): + return HttpResponseForbidden("You don't have permission to access this view") + if request.method == 'POST': form = GroupForm(request.POST, group=group, group_type=group_type) if form.is_valid(): @@ -369,7 +364,7 @@ def conclude(request, acronym, group_type=None): """Request the closing of group, prompting for instructions.""" group = get_group_or_404(acronym, group_type) - if not can_manage_group_type(request.user, group.type_id): + if not can_manage_group(request.user, group): return HttpResponseForbidden("You don't have permission to access this view") if request.method == 'POST': diff --git a/ietf/group/info.py b/ietf/group/info.py index 94ed3b2ec..d66719436 100644 --- a/ietf/group/info.py +++ b/ietf/group/info.py @@ -55,7 +55,7 @@ from ietf.doc.utils import get_chartering_type from ietf.doc.templatetags.ietf_filters import clean_whitespace from ietf.group.models import Group, Role, ChangeStateGroupEvent from ietf.name.models import GroupTypeName -from ietf.group.utils import get_charter_text, can_manage_group_type, can_manage_team_group, milestone_reviewer_for_group_type, can_provide_status_update +from ietf.group.utils import get_charter_text, can_manage_group_type, can_manage_group, milestone_reviewer_for_group_type, can_provide_status_update from ietf.group.utils import can_manage_materials, get_group_or_404 from ietf.utils.pipe import pipe from ietf.utils.textupload import get_cleaned_text_file_content @@ -365,9 +365,7 @@ def construct_group_menu_context(request, group, selected, group_type, others): actions = [] is_chair = group.has_role(request.user, "chair") - can_manage = can_manage_group_type(request.user, group.type_id) - if group.type_id == "team": - can_manage = can_manage_team_group(request.user, group) + can_manage = can_manage_group(request.user, group) if group.features.has_milestones: if group.state_id != "proposed" and (is_chair or can_manage): @@ -491,7 +489,7 @@ def group_about(request, acronym, group_type=None): e = group.latest_event(type__in=("changed_state", "requested_close",)) requested_close = group.state_id != "conclude" and e and e.type == "requested_close" - can_manage = can_manage_group_type(request.user, group.type_id) + can_manage = can_manage_group(request.user, group) can_provide_update = can_provide_status_update(request.user, group) status_update = group.latest_event(type="status_update") diff --git a/ietf/group/milestones.py b/ietf/group/milestones.py index 434294fff..0b316fbe8 100644 --- a/ietf/group/milestones.py +++ b/ietf/group/milestones.py @@ -12,7 +12,7 @@ from ietf.doc.models import DocEvent from ietf.doc.utils import get_chartering_type from ietf.doc.fields import SearchableDocumentsField from ietf.group.models import GroupMilestone, MilestoneGroupEvent -from ietf.group.utils import (save_milestone_in_history, can_manage_group_type, milestone_reviewer_for_group_type, +from ietf.group.utils import (save_milestone_in_history, can_manage_group, milestone_reviewer_for_group_type, get_group_or_404) from ietf.name.models import GroupMilestoneStateName from ietf.group.mails import email_milestones_changed @@ -93,8 +93,8 @@ def edit_milestones(request, acronym, group_type=None, milestone_set="current"): raise Http404 needs_review = False - if not can_manage_group_type(request.user, group.type_id): - if group.role_set.filter(name="chair", person__user=request.user): + if not can_manage_group(request.user, group): + if group.has_role(request.user, "chair"): if milestone_set == "current": needs_review = True else: @@ -329,8 +329,9 @@ def reset_charter_milestones(request, group_type, acronym): if not group.features.has_milestones: raise Http404 - if (not can_manage_group_type(request.user, group_type) and - not group.role_set.filter(name="chair", person__user=request.user)): + can_manage = can_manage_group(request.user, group) + is_chair = group.has_role(request.user, "chair") + if (not can_manage) and (not is_chair): return HttpResponseForbidden("You are not chair of this group.") current_milestones = group.groupmilestone_set.filter(state="active") diff --git a/ietf/group/models.py b/ietf/group/models.py index 68f4e2baf..29344fc2e 100644 --- a/ietf/group/models.py +++ b/ietf/group/models.py @@ -82,7 +82,7 @@ class Group(GroupInfo): def is_decendant_of(self, sought_parent): p = self.parent - while (p != None): + while ((p != None) and (p != self)): if p.acronym == sought_parent: return True p = p.parent diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 8f87e18a9..a6dec522d 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -1125,5 +1125,44 @@ class StatusUpdateTests(TestCase): self.assertEqual(response.status_code, 302) self.assertEqual(chair.group.latest_event(type='status_update').desc,'This came from a file.') +class GroupParentLoopTests(TestCase): - + def test_group_parent_loop(self): + make_test_data() + mars = Group.objects.get(acronym="mars") + test1 = Group.objects.create( + type_id="team", + acronym="testteam1", + name="Test One", + description="The test team 1 is testing.", + state_id="active", + parent = mars, + ) + test2 = Group.objects.create( + type_id="team", + acronym="testteam2", + name="Test Two", + description="The test team 2 is testing.", + state_id="active", + parent = test1, + ) + # Change the parent of Mars to make a loop + mars.parent = test2 + + # In face of the loop in the parent links, the code should not loop forever + import signal + + def timeout_handler(signum, frame): + raise Exception("Infinite loop in parent links is not handeled properly.") + + signal.signal(signal.SIGALRM, timeout_handler) + signal.alarm(1) # One second + try: + test2.is_decendant_of("ietf") + except Exception: + raise + finally: + signal.alarm(0) + + # If we get here, then there is not an infinite loop + return \ No newline at end of file diff --git a/ietf/group/utils.py b/ietf/group/utils.py index 6d41fb95b..55b4faa0f 100644 --- a/ietf/group/utils.py +++ b/ietf/group/utils.py @@ -91,13 +91,16 @@ def can_manage_group_type(user, group_type): return has_role(user, 'Secretariat') -def can_manage_team_group(user, group): - if group.type_id != "team": - return False - elif group.is_decendant_of("ietf") and has_role(user, ('Area Director', 'Secretariat')): - return True - elif group.is_decendant_of("irtf") and has_role(user, ('IRTF Chair', 'Secretariat')): - return True +def can_manage_group(user, group): + if group.type_id == "rg": + return has_role(user, ('IRTF Chair', 'Secretariat')) + elif group.type_id == "wg": + return has_role(user, ('Area Director', 'Secretariat')) + elif group.type_id == "team": + if group.is_decendant_of("ietf"): + return has_role(user, ('Area Director', 'Secretariat')) + elif group.is_decendant_of("irtf"): + return has_role(user, ('IRTF Chair', 'Secretariat')) return has_role(user, ('Secretariat')) def milestone_reviewer_for_group_type(group_type):