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
This commit is contained in:
parent
570107dbf1
commit
858530c214
|
@ -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))
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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':
|
||||
|
|
|
@ -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")
|
||||
|
|
|
@ -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")
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
|
@ -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):
|
||||
|
|
Loading…
Reference in a new issue