Skip to content

Commit

Permalink
fix: Secure reclassification against rogue nomcom members
Browse files Browse the repository at this point in the history
  • Loading branch information
pselkirk committed Aug 7, 2023
1 parent aec27e6 commit 764a8f7
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 12 deletions.
41 changes: 30 additions & 11 deletions ietf/nomcom/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2873,6 +2873,7 @@ def setUp(self):
nomcom_test_data()
self.nc = NomComFactory.create(**nomcom_kwargs_for_year())
self.chair = self.nc.group.role_set.filter(name='chair').first().person
self.member = self.nc.group.role_set.filter(name='member').first().person
self.nominee = self.nc.nominee_set.order_by('pk').first()
self.position = self.nc.position_set.first()
self.topic = self.nc.topic_set.first()
Expand All @@ -2882,16 +2883,22 @@ def tearDown(self):
super().tearDown()

def test_reclassify_feedback_nominee(self):
url = reverse('ietf.nomcom.views.view_feedback_nominee', kwargs={'year':self.nc.year(), 'nominee_id':self.nominee.id})
login_testing_unauthorized(self,self.chair.user.username,url)
provide_private_key_to_test_client(self)

fb = FeedbackFactory.create(nomcom=self.nc,type_id='comment')
fb.positions.add(self.position)
fb.nominees.add(self.nominee)
fb.save()
self.assertEqual(Feedback.objects.comments().count(), 1)

url = reverse('ietf.nomcom.views.view_feedback_nominee', kwargs={'year':self.nc.year(), 'nominee_id':self.nominee.id})
login_testing_unauthorized(self,self.member.user.username,url)
provide_private_key_to_test_client(self)
response = self.client.post(url, {'feedback_id': fb.id, 'type': 'obe'})
self.assertEqual(response.status_code, 403)

self.client.logout()
self.client.login(username=self.chair.user.username, password=self.chair.user.username + "+password")
provide_private_key_to_test_client(self)

response = self.client.post(url, {'feedback_id': fb.id, 'type': 'obe'})
self.assertEqual(response.status_code, 200)

Expand All @@ -2901,15 +2908,21 @@ def test_reclassify_feedback_nominee(self):
self.assertEqual(Feedback.objects.filter(type='obe').count(), 1)

def test_reclassify_feedback_topic(self):
url = reverse('ietf.nomcom.views.view_feedback_topic', kwargs={'year':self.nc.year(), 'topic_id':self.topic.id})
login_testing_unauthorized(self,self.chair.user.username,url)
provide_private_key_to_test_client(self)

fb = FeedbackFactory.create(nomcom=self.nc,type_id='comment')
fb.topics.add(self.topic)
fb.save()
self.assertEqual(Feedback.objects.comments().count(), 1)

url = reverse('ietf.nomcom.views.view_feedback_topic', kwargs={'year':self.nc.year(), 'topic_id':self.topic.id})
login_testing_unauthorized(self,self.member.user.username,url)
provide_private_key_to_test_client(self)
response = self.client.post(url, {'feedback_id': fb.id, 'type': 'unclassified'})
self.assertEqual(response.status_code, 403)

self.client.logout()
self.client.login(username=self.chair.user.username, password=self.chair.user.username + "+password")
provide_private_key_to_test_client(self)

response = self.client.post(url, {'feedback_id': fb.id, 'type': 'unclassified'})
self.assertEqual(response.status_code, 200)

Expand All @@ -2919,12 +2932,18 @@ def test_reclassify_feedback_topic(self):
self.assertEqual(Feedback.objects.filter(type=None).count(), 1)

def test_reclassify_feedback_unrelated(self):
fb = FeedbackFactory(nomcom=self.nc, type_id='read')
self.assertEqual(Feedback.objects.filter(type='read').count(), 1)

url = reverse('ietf.nomcom.views.view_feedback_unrelated', kwargs={'year':self.nc.year()})
login_testing_unauthorized(self,self.chair.user.username,url)
login_testing_unauthorized(self,self.member.user.username,url)
provide_private_key_to_test_client(self)
response = self.client.post(url, {'feedback_id': fb.id, 'type': 'junk'})
self.assertEqual(response.status_code, 403)

fb = FeedbackFactory(nomcom=self.nc, type_id='read')
self.assertEqual(Feedback.objects.filter(type='read').count(), 1)
self.client.logout()
self.client.login(username=self.chair.user.username, password=self.chair.user.username + "+password")
provide_private_key_to_test_client(self)

response = self.client.post(url, {'feedback_id': fb.id, 'type': 'junk'})
self.assertEqual(response.status_code, 200)
Expand Down
9 changes: 8 additions & 1 deletion ietf/nomcom/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from django.contrib.auth.models import AnonymousUser
from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger
from django.forms.models import modelformset_factory, inlineformset_factory
from django.http import Http404, HttpResponseRedirect, HttpResponse
from django.http import Http404, HttpResponseRedirect, HttpResponse, HttpResponseForbidden
from django.shortcuts import render, get_object_or_404, redirect
from django.template.loader import render_to_string
from django.urls import reverse
Expand Down Expand Up @@ -943,6 +943,8 @@ def view_feedback_unrelated(request, year):
nomcom = get_nomcom_by_year(year)

if request.method == 'POST':
if not nomcom.group.has_role(request.user, ['chair','advisor']):
return HttpResponseForbidden('Restricted to roles: Nomcom Chair, Nomcom Advisor')
feedback_id = request.POST.get('feedback_id', None)
feedback = get_object_or_404(Feedback, id=feedback_id)
type = request.POST.get('type', None)
Expand Down Expand Up @@ -980,6 +982,9 @@ def view_feedback_topic(request, year, topic_id):
# Reclassifying from 'comment' to 'comment' is a no-op,
# so the only meaningful action is to de-classify it.
if request.method == 'POST':
nomcom = get_nomcom_by_year(year)
if not nomcom.group.has_role(request.user, ['chair','advisor']):
return HttpResponseForbidden('Restricted to roles: Nomcom Chair, Nomcom Advisor')
feedback_id = request.POST.get('feedback_id', None)
feedback = get_object_or_404(Feedback, id=feedback_id)
feedback.type = None
Expand Down Expand Up @@ -1014,6 +1019,8 @@ def view_feedback_nominee(request, year, nominee_id):
feedback_types = FeedbackTypeName.objects.filter(used=True, slug__in=settings.NOMINEE_FEEDBACK_TYPES)

if request.method == 'POST':
if not nomcom.group.has_role(request.user, ['chair','advisor']):
return HttpResponseForbidden('Restricted to roles: Nomcom Chair, Nomcom Advisor')
feedback_id = request.POST.get('feedback_id', None)
feedback = get_object_or_404(Feedback, id=feedback_id)
type = request.POST.get('type', None)
Expand Down

0 comments on commit 764a8f7

Please sign in to comment.