Merged in [11064] [11082] from housley@vigilsec.com:

    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.
    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. Fixes ietf-tools#1915.
 - Legacy-Id: 11091
Note: SVN reference [11064] has been migrated to Git commit 1c509cd

Note: SVN reference [11082] has been migrated to Git commit 858530c

Author: levkowetz

ietf/doc/views_charter.py

@@ -23,7 +23,7 @@
     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))

ietf/doc/views_doc.py

@@ -52,7 +52,7 @@
     get_initial_notify, make_notify_changed_event, crawl_history, default_consensus)
 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,

ietf/group/edit.py

@@ -16,7 +16,7 @@
 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
 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,20 +199,20 @@ 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):
-        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
         new_group = True
     else:
         raise Http404
 
-    if not group_type and group:
-        group_type = group.type_id
+    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)
@@ -364,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':

ietf/group/info.py

@@ -55,7 +55,7 @@
 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_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,7 +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)
+    can_manage = can_manage_group(request.user, group)
 
     if group.features.has_milestones:
         if group.state_id != "proposed" and (is_chair or can_manage):
@@ -374,7 +374,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):
@@ -489,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")

ietf/group/milestones.py

@@ -12,7 +12,7 @@
 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")

ietf/group/models.py

@@ -80,6 +80,14 @@ def has_role(self, user, role_names):
             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) and (p != self)):
+            if p.acronym == sought_parent:
+                return True
+            p = p.parent
+        return False
+
     def is_bof(self):
         return (self.state.slug in ["bof", "bof-conc"])
 

ietf/group/tests_info.py

@@ -25,7 +25,7 @@
 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,14 +240,27 @@ def test_group_charter(self):
             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",
             acronym="testteam",
             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 @@ def test_group_about(self):
             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")
@@ -1104,5 +1125,44 @@ def test_edit_status_update(self):
         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

ietf/group/utils.py

@@ -91,6 +91,18 @@ def can_manage_group_type(user, group_type):
 
     return has_role(user, 'Secretariat')
 
+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):
     if group_type == "rg":
         return "IRTF Chair"