Clean up permission checking in community lists, fix split in
track/untrack between personal/group lists, get rid of remove_document - Legacy-Id: 10680
This commit is contained in:
Ole Laursen
parent
4696dad536
commit
cd5c60ccf1
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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()), [])
|
||||
|
||||
|
|
|
|||
|
|
@ -12,6 +12,8 @@ urlpatterns = patterns('ietf.community.views',
|
|||
url(r'^personal/(?P<secret>[a-f0-9]+)/subscribe/significant/$', 'subscribe_personal_list', {'significant': True}, name='subscribe_significant_personal_list'),
|
||||
url(r'^personal/(?P<secret>[a-f0-9]+)/unsubscribe/$', 'unsubscribe_personal_list', {'significant': False}, name='unsubscribe_personal_list'),
|
||||
url(r'^personal/(?P<secret>[a-f0-9]+)/unsubscribe/significant/$', 'unsubscribe_personal_list', {'significant': True}, name='unsubscribe_significant_personal_list'),
|
||||
url(r'^personal/trackdocument/(?P<name>[^/]+)/$', 'track_document', name='community_personal_track_document'),
|
||||
url(r'^personal/untrackdocument/(?P<name>[^/]+)/$', 'untrack_document', name='community_personal_untrack_document'),
|
||||
|
||||
url(r'^group/(?P<acronym>[\w.@+-]+)/$', 'manage_group_list', name='manage_group_list'),
|
||||
url(r'^group/(?P<acronym>[\w.@+-]+)/view/$', 'view_group_list', name='view_group_list'),
|
||||
|
|
@ -22,11 +24,10 @@ urlpatterns = patterns('ietf.community.views',
|
|||
url(r'^group/(?P<acronym>[\w.@+-]+)/subscribe/significant/$', 'subscribe_group_list', {'significant': True}, name='subscribe_significant_group_list'),
|
||||
url(r'^group/(?P<acronym>[\w.@+-]+)/unsubscribe/$', 'unsubscribe_group_list', {'significant': False}, name='unsubscribe_group_list'),
|
||||
url(r'^group/(?P<acronym>[\w.@+-]+)/unsubscribe/significant/$', 'unsubscribe_group_list', {'significant': True}, name='unsubscribe_significant_group_list'),
|
||||
url(r'^group/(?P<acronym>[\w.@+-]+)/trackdocument/(?P<name>[^/]+)/$', 'track_document', name='community_group_track_document'),
|
||||
url(r'^group/(?P<acronym>[\w.@+-]+)/untrackdocument/(?P<name>[^/]+)/$', 'untrack_document', name='community_group_untrack_document'),
|
||||
|
||||
url(r'^trackdocument/(?P<name>[^/]+)/$', 'track_document', name='community_track_document'),
|
||||
url(r'^untrackdocument/(?P<name>[^/]+)/$', 'untrack_document', name='community_untrack_document'),
|
||||
|
||||
url(r'^(?P<list_id>[\d]+)/remove_document/(?P<name>[^/]+)/$', 'remove_document', name='community_remove_document'),
|
||||
url(r'^(?P<list_id>[\d]+)/remove_rule/(?P<rule_id>[^/]+)/$', 'remove_rule', name='community_remove_rule'),
|
||||
url(r'^(?P<list_id>[\d]+)/subscribe/confirm/(?P<email>[\w.@+-]+)/(?P<date>[\d]+)/(?P<confirm_hash>[a-f0-9]+)/$', 'confirm_subscription', name='confirm_subscription'),
|
||||
url(r'^(?P<list_id>[\d]+)/subscribe/significant/confirm/(?P<email>[\w.@+-]+)/(?P<date>[\d]+)/(?P<confirm_hash>[a-f0-9]+)/$', 'confirm_significant_subscription', name='confirm_significant_subscription'),
|
||||
|
|
|
|||
13
ietf/community/utils.py
Normal file
13
ietf/community/utils.py
Normal file
|
|
@ -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
|
||||
|
||||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -47,8 +47,8 @@
|
|||
<tr>
|
||||
<td>{{ doc.display_name }}</td>
|
||||
<td>{{ doc.get_state }}</td>
|
||||
<td><a href="{{ doc.get_absolute_url }}">{{ doc.title }}</a></td>
|
||||
<td><a class="btn btn-danger btn-xs" href="{% url "community_remove_document" cl.pk doc.pk %}">Remove</a></td>
|
||||
<td><a href="{{ doc.get_absolute_url }}">{{ doc.title }}</a></td>
|
||||
<td><a class="btn btn-danger btn-xs" href="{% if cl.user %}{% url "community_personal_untrack_document" doc.pk %}{% else %}{% url "community_group_untrack_document" %}{% endif %}">Remove</a></td>
|
||||
</tr>
|
||||
{% endfor %}
|
||||
</tbody>
|
||||
|
|
|
|||
|
|
@ -435,9 +435,9 @@
|
|||
</div>
|
||||
{% if user.is_authenticated %}
|
||||
{% if tracking_document %}
|
||||
<a class="btn btn-default btn-xs community-list-add-remove-doc" href="{% url "community_untrack_document" doc.name %}" title="Remove from your personal ID list"><span class="fa fa-bookmark-o"></span> Untrack</a>
|
||||
<a class="btn btn-default btn-xs community-list-add-remove-doc" href="{% url "community_personal_untrack_document" doc.name %}" title="Remove from your personal ID list"><span class="fa fa-bookmark-o"></span> Untrack</a>
|
||||
{% else %}
|
||||
<a class="btn btn-default btn-xs community-list-add-remove-doc" href="{% url "community_track_document" doc.name %}" title="Add to your personal ID list"><span class="fa fa-bookmark"></span> Track</a>
|
||||
<a class="btn btn-default btn-xs community-list-add-remove-doc" href="{% url "community_personal_track_document" doc.name %}" title="Add to your personal ID list"><span class="fa fa-bookmark"></span> Track</a>
|
||||
{% endif %}
|
||||
{% endif %}
|
||||
|
||||
|
|
|
|||
|
|
@ -14,11 +14,11 @@
|
|||
<td>
|
||||
{% if user.is_authenticated %}
|
||||
{% if doc.name in doc_is_tracked %}
|
||||
<a href="{% url "community_untrack_document" doc.name %}" class="community-list-add-remove-doc" title="Remove from your personal ID list">
|
||||
<a href="{% url "community_personal_untrack_document" doc.name %}" class="community-list-add-remove-doc" title="Remove from your personal ID list">
|
||||
<span class="fa fa-bookmark"></span>
|
||||
</a>
|
||||
{% else %}
|
||||
<a href="{% url "community_track_document" doc.name %}" class="community-list-add-remove-doc" title="Add to your personal ID list">
|
||||
<a href="{% url "community_personal_track_document" doc.name %}" class="community-list-add-remove-doc" title="Add to your personal ID list">
|
||||
<span class="fa fa-bookmark-o"></span>
|
||||
</a>
|
||||
{% endif %}
|
||||
|
|
|
|||
Loading…
Reference in a new issue