Skip to content
This repository was archived by the owner on Mar 15, 2018. It is now read-only.

Commit 37f3a4a

Browse files
committed
adapt note/thread permissions for creation (bug 879545)
* Also, add creation tests.
1 parent 050e7c8 commit 37f3a4a

File tree

2 files changed

+104
-14
lines changed

2 files changed

+104
-14
lines changed

mkt/comm/api.py

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,27 @@
33
from django.db.models import Q
44
from django.http import Http404
55

6+
import commonware.log
67
from rest_framework.exceptions import PermissionDenied
78
from rest_framework.mixins import (CreateModelMixin, DestroyModelMixin,
89
ListModelMixin, RetrieveModelMixin)
910
from rest_framework.permissions import BasePermission
1011
from rest_framework.relations import HyperlinkedRelatedField, RelatedField
1112
from rest_framework.serializers import ModelSerializer
12-
from rest_framework.viewsets import GenericViewSet
1313

1414
from access import acl
1515
from comm.models import (CommunicationNote, CommunicationThread,
16-
CommunicationThreadCC)
16+
CommunicationThreadCC, CommunicationThreadToken)
1717
from mkt.api.authentication import (RestOAuthAuthentication,
1818
RestSharedSecretAuthentication)
19+
from mkt.api.base import CORSViewSet
1920
from mkt.webpay.forms import PrepareForm
2021

22+
log = commonware.log.getLogger('mkt.comm')
23+
2124

2225
class NoteSerializer(ModelSerializer):
23-
thread = HyperlinkedRelatedField(read_only=True,
24-
view_name='comm-thread-detail')
26+
thread = HyperlinkedRelatedField(view_name='comm-thread-detail')
2527
body = RelatedField('body')
2628

2729
class Meta:
@@ -77,7 +79,6 @@ def has_object_permission(self, request, view, obj):
7779
Moreover, other object permissions are also checked agaisnt the ACLs
7880
of the user.
7981
"""
80-
8182
if not request.user.is_authenticated() or obj.read_permission_public:
8283
return obj.read_permission_public
8384

@@ -86,6 +87,7 @@ def has_object_permission(self, request, view, obj):
8687
thread=obj)
8788
user_cc = CommunicationThreadCC.objects.filter(user=profile,
8889
thread=obj)
90+
8991
if user_post.exists() or user_cc.exists():
9092
return True
9193

@@ -114,18 +116,41 @@ def has_object_permission(self, request, view, obj):
114116

115117

116118
class NotePermission(ThreadPermission):
119+
120+
def has_permission(self, request, view):
121+
if view.action == 'create':
122+
if not request.user.is_authenticated():
123+
return False
124+
125+
serializer = view.get_serializer(data=request.DATA)
126+
if not serializer.is_valid():
127+
return False
128+
129+
obj = serializer.object
130+
# Check if author and user who created this request mismatch.
131+
if obj.author.id != request.amo_user.id:
132+
return False
133+
134+
# Determine permission to add the note based on the thread
135+
# permission.
136+
return ThreadPermission.has_object_permission(self,
137+
request, view, obj.thread)
138+
139+
return True
140+
117141
def has_object_permission(self, request, view, obj):
118-
return super(ThreadPermission, self).has_object_permission(request,
119-
view, obj.thread)
142+
return ThreadPermission.has_object_permission(self, request, view,
143+
obj.thread)
120144

121145

122146
class ThreadViewSet(ListModelMixin, RetrieveModelMixin, DestroyModelMixin,
123-
CreateModelMixin, GenericViewSet):
147+
CreateModelMixin, CORSViewSet):
124148
model = CommunicationThread
125149
serializer_class = ThreadSerializer
126150
authentication_classes = (RestOAuthAuthentication,
127151
RestSharedSecretAuthentication)
128152
permission_classes = (ThreadPermission,)
153+
cors_allowed_methods = ['get', 'post']
129154

130155
def list(self, request):
131156
profile = request.amo_user
@@ -145,20 +170,31 @@ def list(self, request):
145170
addon=form.cleaned_data['app'])
146171
else:
147172
# We list all the threads which uses an add-on authored by the
148-
# user.
173+
# user and with read permissions for add-on devs.
149174
notes, cc = list(notes), list(cc)
150175
addons = list(profile.addons.values_list('pk', flat=True))
176+
q_dev = Q(addon__in=addons, read_permission_developer=True)
151177
queryset = CommunicationThread.objects.filter(
152-
Q(pk__in=notes + cc) | Q(addon__in=addons))
178+
Q(pk__in=notes + cc) | q_dev)
153179

154180
self.queryset = queryset
155181
return ListModelMixin.list(self, request)
156182

157183

158184
class NoteViewSet(CreateModelMixin, RetrieveModelMixin, DestroyModelMixin,
159-
GenericViewSet):
185+
CORSViewSet):
160186
model = CommunicationNote
161187
serializer_class = NoteSerializer
162188
authentication_classes = (RestOAuthAuthentication,
163189
RestSharedSecretAuthentication,)
164190
permission_classes = (NotePermission,)
191+
cors_allowed_methods = ['get', 'post', 'delete']
192+
193+
def create(self, request):
194+
res = CreateModelMixin.create(self, request)
195+
if res.status_code == 201:
196+
tok = CommunicationThreadToken.objects.create(
197+
thread=self.object.thread, user=self.object.author)
198+
log.info('Reply token with UUID %s created.' % (tok.uuid))
199+
200+
return res

mkt/comm/tests/test_api.py

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
1+
import json
2+
13
from django.core.urlresolvers import reverse
24

35
from nose.tools import eq_
46
from test_utils import RequestFactory
57

68
from amo.tests import addon_factory
79
from comm.models import (CommunicationNote, CommunicationThread,
8-
CommunicationThreadCC)
10+
CommunicationThreadCC, CommunicationThreadToken)
911
from mkt.api.tests.test_oauth import RestOAuth
1012
from mkt.comm.api import ThreadPermission
13+
from mkt.constants import comm as const
1114
from mkt.site.fixtures import fixture
1215
from mkt.webapps.models import Webapp
1316

@@ -93,6 +96,12 @@ def test_read_staff(self):
9396
read_permission_staff=True)
9497
assert self.check_permissions()
9598

99+
def test_cors_allowed(self):
100+
thread = CommunicationThread.objects.create(addon=self.addon)
101+
res = self.client.get(reverse('comm-thread-detail',
102+
kwargs={'pk': thread.pk}))
103+
self.assertCORS(res, 'get', 'post')
104+
96105

97106
class TestThreadList(RestOAuth):
98107
fixtures = fixture('webapp_337141', 'user_2519')
@@ -127,14 +136,27 @@ def test_addon_filter(self):
127136
res = self.client.get(self.list_url, {'app': '1000'})
128137
eq_(res.status_code, 404)
129138

139+
def test_creation(self):
140+
thread = CommunicationThread.objects.create(addon=self.addon)
141+
version = self.addon.current_version
142+
res = self.client.post(self.list_url, data=json.dumps(
143+
{'addon': self.addon.id, 'version': version.id}))
144+
145+
eq_(res.status_code, 201)
146+
130147

131148
class TestNote(RestOAuth):
132-
fixtures = fixture('webapp_337141', 'user_2519')
149+
fixtures = fixture('webapp_337141', 'user_2519', 'user_999')
133150

134151
def setUp(self):
135152
super(TestNote, self).setUp()
136153
addon = Webapp.objects.get(pk=337141)
137-
self.thread = CommunicationThread.objects.create(addon=addon)
154+
self.list_url = reverse('comm-note-list')
155+
self.thread = CommunicationThread.objects.create(addon=addon,
156+
read_permission_developer=True)
157+
self.thread_url = reverse('comm-thread-detail',
158+
kwargs={'pk': self.thread.id})
159+
self.profile.addonuser_set.create(addon=addon)
138160

139161
def test_response(self):
140162
note = CommunicationNote.objects.create(author=self.profile,
@@ -143,3 +165,35 @@ def test_response(self):
143165
kwargs={'pk': note.id}))
144166
eq_(res.status_code, 200)
145167
eq_(res.json['body'], 'something')
168+
169+
def test_creation(self):
170+
res = self.client.post(self.list_url, data=json.dumps(
171+
{'thread': self.thread_url, 'author': self.profile.id,
172+
'note_type': '0', 'body': 'something'}))
173+
eq_(res.status_code, 201)
174+
count = CommunicationThreadToken.objects.filter(
175+
thread=self.thread, user=self.profile).count()
176+
eq_(count, 1)
177+
178+
def test_creation_denied(self):
179+
self.thread.read_permission_developer = False
180+
self.thread.save()
181+
res = self.client.post(self.list_url, data=json.dumps(
182+
{'thread': self.thread_url, 'author': self.profile.id,
183+
'note_type': '0', 'body': 'something'}))
184+
eq_(res.status_code, 403)
185+
186+
def test_creation_by_diff_user_denied(self):
187+
"""
188+
Test that the creation by specifying a different user as author fails.
189+
"""
190+
self.thread.read_permission_developer = True
191+
self.thread.save()
192+
res = self.client.post(self.list_url, data=json.dumps(
193+
{'thread': self.thread_url, 'author': '999',
194+
'note_type': '0', 'body': 'something'}))
195+
eq_(res.status_code, 403)
196+
197+
def test_cors_allowed(self):
198+
res = self.client.get(self.list_url)
199+
self.assertCORS(res, 'get', 'post', 'delete')

0 commit comments

Comments
 (0)