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 #1915. - Legacy-Id: 11091 Note: SVN reference [11064] has been migrated to Git commit1c509cd64c
Note: SVN reference [11082] has been migrated to Git commit858530c214
This commit is contained in:
commit
40bb23169c
|
@ -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, 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,
|
||||
|
|
|
@ -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
|
||||
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,11 +199,7 @@ 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
|
||||
|
@ -211,8 +207,12 @@ 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 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':
|
||||
|
|
|
@ -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_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")
|
||||
|
|
|
@ -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")
|
||||
|
|
|
@ -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) 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"])
|
||||
|
||||
|
|
|
@ -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")
|
||||
|
@ -1104,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,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"
|
||||
|
|
Loading…
Reference in a new issue