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

Author: russhousley

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)
 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, 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':

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, 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")

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

@@ -82,7 +82,7 @@ def has_role(self, user, role_names):
 
     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

ietf/group/tests_info.py

@@ -1125,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,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):