diff --git a/ietf/community/models.py b/ietf/community/models.py index 38de908cb..fe1ce2fd6 100644 --- a/ietf/community/models.py +++ b/ietf/community/models.py @@ -8,7 +8,7 @@ from django.db.models import signals, Q from ietf.utils.mail import send_mail from ietf.doc.models import Document, DocEvent -from ietf.group.models import Group, Role +from ietf.group.models import Group from ietf.community.rules import TYPES_OF_RULES, RuleManager from ietf.community.display import (TYPES_OF_SORT, DisplayField, @@ -24,21 +24,6 @@ class CommunityList(models.Model): secret = models.CharField(max_length=255, null=True, blank=True) cached = models.TextField(null=True, blank=True) - def check_manager(self, user): - if user == self.user: - return True - if not self.group or self.group.type.slug not in ('area', 'wg'): - return False - try: - person = user.person - except: - return False - if self.group.type.slug == 'area': - return bool(Role.objects.filter(name__slug='ad', email__in=person.email_set.all(), group=self.group).count()) - elif self.group.type.slug == 'wg': - return bool(Role.objects.filter(name__slug='chair', email__in=person.email_set.all(), group=self.group).count()) - return False - def long_name(self): if self.user: return 'Personal ID list of %s' % self.user.username diff --git a/ietf/community/tests.py b/ietf/community/tests.py index e5fae2c21..71e27d952 100644 --- a/ietf/community/tests.py +++ b/ietf/community/tests.py @@ -7,10 +7,10 @@ from ietf.utils.test_data import make_test_data from ietf.utils.test_utils import login_testing_unauthorized, TestCase class CommunityListTests(TestCase): - def test_track_untrack_document(self): + def test_track_untrack_document_for_personal_list_through_ajax(self): draft = make_test_data() - url = urlreverse("community_track_document", kwargs={ "name": draft.name }) + url = urlreverse("community_personal_track_document", kwargs={ "name": draft.name }) login_testing_unauthorized(self, "plain", url) # track @@ -21,10 +21,35 @@ class CommunityListTests(TestCase): self.assertEqual(list(clist.added_ids.all()), [draft]) # untrack - url = urlreverse("community_untrack_document", kwargs={ "name": draft.name }) + url = urlreverse("community_personal_untrack_document", kwargs={ "name": draft.name }) r = self.client.post(url, HTTP_X_REQUESTED_WITH='XMLHttpRequest') self.assertEqual(r.status_code, 200) self.assertEqual(json.loads(r.content)["success"], True) clist = CommunityList.objects.get(user__username="plain") self.assertEqual(list(clist.added_ids.all()), []) + + def test_track_untrack_document_for_group_list(self): + draft = make_test_data() + + url = urlreverse("community_group_track_document", kwargs={ "name": draft.name, "acronym": draft.group.acronym }) + login_testing_unauthorized(self, "marschairman", url) + + # track + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + + r = self.client.post(url) + self.assertEqual(r.status_code, 302) + clist = CommunityList.objects.get(group__acronym=draft.group.acronym) + self.assertEqual(list(clist.added_ids.all()), [draft]) + + # untrack + url = urlreverse("community_group_untrack_document", kwargs={ "name": draft.name, "acronym": draft.group.acronym }) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + + r = self.client.post(url) + self.assertEqual(r.status_code, 302) + clist = CommunityList.objects.get(group__acronym=draft.group.acronym) + self.assertEqual(list(clist.added_ids.all()), []) diff --git a/ietf/community/urls.py b/ietf/community/urls.py index 7c499d4f7..99458eacf 100644 --- a/ietf/community/urls.py +++ b/ietf/community/urls.py @@ -12,6 +12,8 @@ urlpatterns = patterns('ietf.community.views', url(r'^personal/(?P[a-f0-9]+)/subscribe/significant/$', 'subscribe_personal_list', {'significant': True}, name='subscribe_significant_personal_list'), url(r'^personal/(?P[a-f0-9]+)/unsubscribe/$', 'unsubscribe_personal_list', {'significant': False}, name='unsubscribe_personal_list'), url(r'^personal/(?P[a-f0-9]+)/unsubscribe/significant/$', 'unsubscribe_personal_list', {'significant': True}, name='unsubscribe_significant_personal_list'), + url(r'^personal/trackdocument/(?P[^/]+)/$', 'track_document', name='community_personal_track_document'), + url(r'^personal/untrackdocument/(?P[^/]+)/$', 'untrack_document', name='community_personal_untrack_document'), url(r'^group/(?P[\w.@+-]+)/$', 'manage_group_list', name='manage_group_list'), url(r'^group/(?P[\w.@+-]+)/view/$', 'view_group_list', name='view_group_list'), @@ -22,11 +24,10 @@ urlpatterns = patterns('ietf.community.views', url(r'^group/(?P[\w.@+-]+)/subscribe/significant/$', 'subscribe_group_list', {'significant': True}, name='subscribe_significant_group_list'), url(r'^group/(?P[\w.@+-]+)/unsubscribe/$', 'unsubscribe_group_list', {'significant': False}, name='unsubscribe_group_list'), url(r'^group/(?P[\w.@+-]+)/unsubscribe/significant/$', 'unsubscribe_group_list', {'significant': True}, name='unsubscribe_significant_group_list'), + url(r'^group/(?P[\w.@+-]+)/trackdocument/(?P[^/]+)/$', 'track_document', name='community_group_track_document'), + url(r'^group/(?P[\w.@+-]+)/untrackdocument/(?P[^/]+)/$', 'untrack_document', name='community_group_untrack_document'), - url(r'^trackdocument/(?P[^/]+)/$', 'track_document', name='community_track_document'), - url(r'^untrackdocument/(?P[^/]+)/$', 'untrack_document', name='community_untrack_document'), - url(r'^(?P[\d]+)/remove_document/(?P[^/]+)/$', 'remove_document', name='community_remove_document'), url(r'^(?P[\d]+)/remove_rule/(?P[^/]+)/$', 'remove_rule', name='community_remove_rule'), url(r'^(?P[\d]+)/subscribe/confirm/(?P[\w.@+-]+)/(?P[\d]+)/(?P[a-f0-9]+)/$', 'confirm_subscription', name='confirm_subscription'), url(r'^(?P[\d]+)/subscribe/significant/confirm/(?P[\w.@+-]+)/(?P[\d]+)/(?P[a-f0-9]+)/$', 'confirm_significant_subscription', name='confirm_significant_subscription'), diff --git a/ietf/community/utils.py b/ietf/community/utils.py new file mode 100644 index 000000000..e120a2f6c --- /dev/null +++ b/ietf/community/utils.py @@ -0,0 +1,13 @@ +from ietf.group.models import Role + +def can_manage_community_list_for_group(user, group): + if not user or not user.is_authenticated() or not group: + return False + + if group.type_id == 'area': + return Role.objects.filter(name__slug='ad', person__user=user, group=group).exists() + elif group.type_id in ('wg', 'rg'): + return Role.objects.filter(name__slug='chair', person__user=user, group=group).exists() + else: + return False + diff --git a/ietf/community/views.py b/ietf/community/views.py index 88bf4a7e9..fcc0a6d10 100644 --- a/ietf/community/views.py +++ b/ietf/community/views.py @@ -6,14 +6,13 @@ import json from django.db import IntegrityError from django.conf import settings -from django.contrib.auth import REDIRECT_FIELD_NAME from django.http import HttpResponse, HttpResponseForbidden, Http404, HttpResponseRedirect from django.shortcuts import get_object_or_404, render, redirect -from django.utils.http import urlquote from django.contrib.auth.decorators import login_required from ietf.community.models import CommunityList, Rule, EmailSubscription from ietf.community.forms import RuleForm, DisplayForm, SubscribeForm, UnSubscribeForm +from ietf.community.utils import can_manage_community_list_for_group from ietf.group.models import Group from ietf.doc.models import DocEvent, Document @@ -48,38 +47,37 @@ def _manage_list(request, clist): 'rule_form': rule_form}) +@login_required def manage_personal_list(request): - user = request.user - if not user.is_authenticated(): - path = urlquote(request.get_full_path()) - tup = settings.LOGIN_URL, REDIRECT_FIELD_NAME, path - return HttpResponseRedirect('%s?%s=%s' % tup) clist = CommunityList.objects.get_or_create(user=request.user)[0] - if not clist.check_manager(request.user): - path = urlquote(request.get_full_path()) - tup = settings.LOGIN_URL, REDIRECT_FIELD_NAME, path - return HttpResponseRedirect('%s?%s=%s' % tup) return _manage_list(request, clist) +@login_required def manage_group_list(request, acronym): group = get_object_or_404(Group, acronym=acronym) - if group.type.slug not in ('area', 'wg'): - raise Http404 + if not can_manage_community_list_for_group(request.user, group): + return HttpResponseForbidden("You do not have permission to access this view") + clist = CommunityList.objects.get_or_create(group=group)[0] - if not clist.check_manager(request.user): - path = urlquote(request.get_full_path()) - tup = settings.LOGIN_URL, REDIRECT_FIELD_NAME, path - return HttpResponseRedirect('%s?%s=%s' % tup) return _manage_list(request, clist) @login_required -def track_document(request, name): +def track_document(request, name, acronym=None): doc = get_object_or_404(Document, docalias__name=name) if request.method == "POST": - clist = CommunityList.objects.get_or_create(user=request.user)[0] + if acronym: + group = get_object_or_404(Group, acronym=acronym) + if not can_manage_community_list_for_group(request.user, group): + return HttpResponseForbidden("You do not have permission to access this view") + + clist = CommunityList.objects.get_or_create(group=group)[0] + else: + clist = CommunityList.objects.get_or_create(user=request.user)[0] + clist.added_ids.add(doc) + if request.is_ajax(): return HttpResponse(json.dumps({ 'success': True }), content_type='text/plain') else: @@ -90,9 +88,15 @@ def track_document(request, name): }) @login_required -def untrack_document(request, name): +def untrack_document(request, name, acronym=None): doc = get_object_or_404(Document, docalias__name=name) - clist = get_object_or_404(CommunityList, user=request.user) + if acronym: + group = get_object_or_404(Group, acronym=acronym) + if not can_manage_community_list_for_group(request.user, group): + return HttpResponseForbidden("You do not have permission to access this view") + clist = get_object_or_404(CommunityList, group=group) + else: + clist = get_object_or_404(CommunityList, user=request.user) if request.method == "POST": clist.added_ids.remove(doc) @@ -106,23 +110,13 @@ def untrack_document(request, name): }) @login_required -def remove_document(request, list_id, name): - clist = get_object_or_404(CommunityList, pk=list_id) - if not clist.check_manager(request.user): - return HttpResponseForbidden("You do not have permission to access this view") - - doc = get_object_or_404(Document, docalias__name=name) - clist.added_ids.remove(doc) - - return HttpResponseRedirect(clist.get_manage_url()) - - def remove_rule(request, list_id, rule_id): clist = get_object_or_404(CommunityList, pk=list_id) - if not clist.check_manager(request.user): - path = urlquote(request.get_full_path()) - tup = settings.LOGIN_URL, REDIRECT_FIELD_NAME, path - return HttpResponseRedirect('%s?%s=%s' % tup) + + if ((clist.user and clist.user != request.user) + or (clist.group and not can_manage_community_list_for_group(request.user, clist.group))): + return HttpResponseForbidden("You do not have permission to access this view") + rule = get_object_or_404(Rule, pk=rule_id) rule.delete() return HttpResponseRedirect(clist.get_manage_url()) @@ -218,30 +212,19 @@ def _csv_list(request, clist): writer.writerow(row) return response - +@login_required def csv_personal_list(request): - user = request.user - if not user.is_authenticated(): - path = urlquote(request.get_full_path()) - tup = settings.LOGIN_URL, REDIRECT_FIELD_NAME, path - return HttpResponseRedirect('%s?%s=%s' % tup) - clist = CommunityList.objects.get_or_create(user=user)[0] - if not clist.check_manager(user): - path = urlquote(request.get_full_path()) - tup = settings.LOGIN_URL, REDIRECT_FIELD_NAME, path - return HttpResponseRedirect('%s?%s=%s' % tup) + clist = CommunityList.objects.get_or_create(user=request.user)[0] return _csv_list(request, clist) +@login_required def csv_group_list(request, acronym): group = get_object_or_404(Group, acronym=acronym) - if group.type.slug not in ('area', 'wg'): - raise Http404 + if not can_manage_community_list_for_group(request.user, group): + return HttpResponseForbidden("You do not have permission to access this view") + clist = CommunityList.objects.get_or_create(group=group)[0] - if not clist.check_manager(request.user): - path = urlquote(request.get_full_path()) - tup = settings.LOGIN_URL, REDIRECT_FIELD_NAME, path - return HttpResponseRedirect('%s?%s=%s' % tup) return _csv_list(request, clist) def view_csv_personal_list(request, secret): diff --git a/ietf/templates/community/manage_clist.html b/ietf/templates/community/manage_clist.html index bd6630359..b39c8f971 100644 --- a/ietf/templates/community/manage_clist.html +++ b/ietf/templates/community/manage_clist.html @@ -47,8 +47,8 @@ {{ doc.display_name }} {{ doc.get_state }} - {{ doc.title }} - Remove + {{ doc.title }} + Remove {% endfor %} diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index 85d26bb4e..70b55f621 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -435,9 +435,9 @@ {% if user.is_authenticated %} {% if tracking_document %} - Untrack + Untrack {% else %} - Track + Track {% endif %} {% endif %} diff --git a/ietf/templates/doc/search/search_result_row.html b/ietf/templates/doc/search/search_result_row.html index 343c3da14..1944819ad 100644 --- a/ietf/templates/doc/search/search_result_row.html +++ b/ietf/templates/doc/search/search_result_row.html @@ -14,11 +14,11 @@ {% if user.is_authenticated %} {% if doc.name in doc_is_tracked %} - + {% else %} - + {% endif %}