From 6e523a87868ad5a3e2ede7bf8472023c23a34637 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Mon, 25 Nov 2013 17:32:34 +0100 Subject: [PATCH 1/3] Convert FailureNotificationResource to DRF (bug 910602) --- docs/api/topics/payment.rst | 2 +- mkt/webpay/resources.py | 46 ++++++++++++++++-------------- mkt/webpay/tests/test_resources.py | 28 +++++++++--------- mkt/webpay/urls.py | 5 ++-- 4 files changed, 42 insertions(+), 39 deletions(-) diff --git a/docs/api/topics/payment.rst b/docs/api/topics/payment.rst index 4a0e6cc5bba..aff1127ca7d 100644 --- a/docs/api/topics/payment.rst +++ b/docs/api/topics/payment.rst @@ -684,7 +684,7 @@ Transaction failure **Response** :status 202: Notification will be sent. - :statuscode 401: The API user is not authorized to report failures. + :statuscode 403: The API user is not authorized to report failures. .. _CORS: https://developer.mozilla.org/en-US/docs/HTTP/Access_control_CORS .. _WebPay: https://github.com/mozilla/webpay diff --git a/mkt/webpay/resources.py b/mkt/webpay/resources.py index 4217dca27b3..7d8f91a8b3f 100644 --- a/mkt/webpay/resources.py +++ b/mkt/webpay/resources.py @@ -7,8 +7,10 @@ import commonware.log import django_filters +from rest_framework import status from rest_framework.decorators import api_view, permission_classes from rest_framework.permissions import AllowAny +from rest_framework.generics import GenericAPIView from rest_framework.mixins import ListModelMixin, RetrieveModelMixin from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet @@ -26,9 +28,12 @@ from mkt.api.authentication import (OAuthAuthentication, OptionalOAuthAuthentication, RestAnonymousAuthentication, + RestOAuthAuthentication, + RestSharedSecretAuthentication, SharedSecretAuthentication) from mkt.api.authorization import (AnonymousReadOnlyAuthorization, - Authorization, OwnerAuthorization, + Authorization, GroupPermission, + OwnerAuthorization, PermissionAuthorization) from mkt.api.base import (CORSResource, CORSMixin, GenericObject, http_error, MarketplaceModelResource, MarketplaceResource, @@ -136,31 +141,30 @@ class PricesViewSet(MarketplaceView, CORSMixin, ListModelMixin, filter_class = PriceFilter -class FailureNotificationResource(MarketplaceModelResource): +class FailureNotificationView(MarketplaceView, GenericAPIView): + authentication_classes = [RestOAuthAuthentication, + RestSharedSecretAuthentication] + permission_classes = [GroupPermission('Transaction', 'NotifyFailure')] + queryset = Contribution.objects.filter(uuid__isnull=False) - class Meta: - authentication = OAuthAuthentication() - authorization = PermissionAuthorization('Transaction', 'NotifyFailure') - detail_allowed_methods = ['patch'] - queryset = Contribution.objects.filter(uuid__isnull=False) - resource_name = 'failure' - - def obj_update(self, bundle, **kw): - form = FailureForm(bundle.data) + def patch(self, request, *args, **kwargs): + form = FailureForm(request.DATA) if not form.is_valid(): - raise self.form_errors(form) - - data = {'transaction_id': bundle.obj, - 'transaction_url': absolutify( - urlparams(reverse('mkt.developers.transactions'), - transaction_id=bundle.obj.uuid)), - 'url': form.cleaned_data['url'], - 'retries': form.cleaned_data['attempts']} - owners = bundle.obj.addon.authors.values_list('email', flat=True) + return Response(form.errors, status=status.HTTP_400_BAD_REQUEST) + + obj = self.get_object() + data = { + 'transaction_id': obj, + 'transaction_url': absolutify( + urlparams(reverse('mkt.developers.transactions'), + transaction_id=obj.uuid)), + 'url': form.cleaned_data['url'], + 'retries': form.cleaned_data['attempts']} + owners = obj.addon.authors.values_list('email', flat=True) send_mail_jinja('Payment notification failure.', 'webpay/failure.txt', data, recipient_list=owners) - return bundle + return Response(status=status.HTTP_202_ACCEPTED) class ProductIconResource(CORSResource, MarketplaceModelResource): diff --git a/mkt/webpay/tests/test_resources.py b/mkt/webpay/tests/test_resources.py index e4215ecbbdf..06e9fb9563d 100644 --- a/mkt/webpay/tests/test_resources.py +++ b/mkt/webpay/tests/test_resources.py @@ -215,49 +215,47 @@ def test_no_locale(self): eq_(res.json['localized'], {}) -class TestNotification(BaseOAuth): +class TestNotification(RestOAuth): fixtures = fixture('webapp_337141', 'user_2519') def setUp(self): - super(TestNotification, self).setUp(api_name='webpay') + super(TestNotification, self).setUp() self.grant_permission(self.profile, 'Transaction:NotifyFailure') self.contribution = Contribution.objects.create(addon_id=337141, uuid='sample:uuid') - self.list_url = ('api_dispatch_list', {'resource_name': 'failure'}) - self.get_url = ['api_dispatch_detail', - {'resource_name': 'failure', - 'pk': self.contribution.pk}] + self.get_url = reverse('failure-detail', + kwargs={'pk': self.contribution.pk}) + self.data = {'url': 'https://someserver.com', 'attempts': 5} def test_list_allowed(self): self._allowed_verbs(self.get_url, ['patch']) def test_notify(self): - url = 'https://someserver.com' - res = self.client.patch(self.get_url, - data=json.dumps({'url': url, 'attempts': 5})) + res = self.client.patch(self.get_url, data=json.dumps(self.data)) eq_(res.status_code, 202) eq_(len(mail.outbox), 1) msg = mail.outbox[0] - assert url in msg.body + assert self.data['url'] in msg.body eq_(msg.recipients(), [u'steamcube@mozilla.com']) def test_no_permission(self): GroupUser.objects.filter(user=self.profile).delete() - res = self.client.patch(self.get_url, data=json.dumps({})) - eq_(res.status_code, 401) + res = self.client.patch(self.get_url, data=json.dumps(self.data)) + eq_(res.status_code, 403) def test_missing(self): res = self.client.patch(self.get_url, data=json.dumps({})) eq_(res.status_code, 400) def test_not_there(self): - self.get_url[1]['pk'] += 1 - res = self.client.patch(self.get_url, data=json.dumps({})) + self.get_url = reverse('failure-detail', + kwargs={'pk': self.contribution.pk + 42}) + res = self.client.patch(self.get_url, data=json.dumps(self.data)) eq_(res.status_code, 404) def test_no_uuid(self): self.contribution.update(uuid=None) - res = self.client.patch(self.get_url, data=json.dumps({})) + res = self.client.patch(self.get_url, data=json.dumps(self.data)) eq_(res.status_code, 404) diff --git a/mkt/webpay/urls.py b/mkt/webpay/urls.py index 26c79484d8e..2780aff574a 100644 --- a/mkt/webpay/urls.py +++ b/mkt/webpay/urls.py @@ -3,13 +3,12 @@ from rest_framework import routers from tastypie.api import Api -from mkt.webpay.resources import (FailureNotificationResource, +from mkt.webpay.resources import (FailureNotificationView, PreparePayResource, PricesViewSet, ProductIconResource, sig_check, StatusPayResource) api = Api(api_name='webpay') -api.register(FailureNotificationResource()) api.register(ProductIconResource()) api.register(PreparePayResource()) api.register(StatusPayResource()) @@ -20,5 +19,7 @@ urlpatterns = patterns('', url(r'^', include(api.urls)), url(r'^webpay/', include(api_router.urls)), + url(r'^webpay/failure/(?P\d+)/', FailureNotificationView.as_view(), + name='failure-detail'), url(r'^webpay/sig_check/$', sig_check, name='webpay.sig_check') ) From 7da8d2fb205fe8e0e84ebd11e814a034428ec49b Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Mon, 25 Nov 2013 18:35:37 +0100 Subject: [PATCH 2/3] Convert PreparePayResource to DRF (bug 910609) --- mkt/webpay/resources.py | 44 +++++++++++++++--------------- mkt/webpay/tests/test_resources.py | 17 ++++++------ mkt/webpay/urls.py | 8 +++--- 3 files changed, 35 insertions(+), 34 deletions(-) diff --git a/mkt/webpay/resources.py b/mkt/webpay/resources.py index 7d8f91a8b3f..dfa678af2aa 100644 --- a/mkt/webpay/resources.py +++ b/mkt/webpay/resources.py @@ -32,12 +32,13 @@ RestSharedSecretAuthentication, SharedSecretAuthentication) from mkt.api.authorization import (AnonymousReadOnlyAuthorization, - Authorization, GroupPermission, + GroupPermission, OwnerAuthorization, PermissionAuthorization) -from mkt.api.base import (CORSResource, CORSMixin, GenericObject, http_error, +from mkt.api.base import (CORSResource, CORSMixin, http_error, MarketplaceModelResource, MarketplaceResource, MarketplaceView) +from mkt.api.exceptions import AlreadyPurchased from mkt.purchase.webpay import _prepare_pay, sign_webpay_jwt from mkt.purchase.utils import payments_enabled from mkt.webpay.forms import FailureForm, PrepareForm, ProductIconForm @@ -50,35 +51,34 @@ log = commonware.log.getLogger('z.webpay') -class PreparePayResource(CORSResource, MarketplaceResource): - webpayJWT = fields.CharField(attribute='webpayJWT', readonly=True) - contribStatusURL = fields.CharField(attribute='contribStatusURL', - readonly=True) +class PreparePayView(CORSMixin, MarketplaceView, GenericAPIView): + authentication_classes = [RestOAuthAuthentication, + RestSharedSecretAuthentication] + permission_classes = [AllowAny] + cors_allowed_methods = ['post'] - class Meta(MarketplaceResource.Meta): - always_return_data = True - authentication = (SharedSecretAuthentication(), OAuthAuthentication()) - authorization = Authorization() - detail_allowed_methods = [] - list_allowed_methods = ['post'] - object_class = GenericObject - resource_name = 'prepare' - validation = CleanedDataFormValidation(form_class=PrepareForm) + def post(self, request, *args, **kwargs): + form = PrepareForm(request.DATA) + if not form.is_valid(): + return Response(form.errors, status=status.HTTP_400_BAD_REQUEST) + app = form.cleaned_data['app'] - def obj_create(self, bundle, request, **kwargs): region = getattr(request, 'REGION', None) - app = bundle.data['app'] - if region and region.id not in app.get_price_region_ids(): log.info('Region {0} is not in {1}' .format(region.id, app.get_price_region_ids())) if payments_enabled(request): log.info('Flag not active') - raise http_error(http.HttpForbidden, - 'Payments are limited and flag not enabled') + return Response('Payments are limited and flag not enabled', + status=status.HTTP_403_FORBIDDEN) - bundle.obj = GenericObject(_prepare_pay(request, bundle.data['app'])) - return bundle + try: + data = _prepare_pay(request._request, app) + except AlreadyPurchased: + return Response({'reason': u'Already purchased app.'}, + status=status.HTTP_409_CONFLICT) + + return Response(data, status=status.HTTP_201_CREATED) class StatusPayResource(CORSResource, MarketplaceModelResource): diff --git a/mkt/webpay/tests/test_resources.py b/mkt/webpay/tests/test_resources.py index 06e9fb9563d..23514f9676f 100644 --- a/mkt/webpay/tests/test_resources.py +++ b/mkt/webpay/tests/test_resources.py @@ -25,20 +25,21 @@ from stats.models import Contribution -class TestPrepare(PurchaseTest, BaseOAuth): +class TestPrepare(PurchaseTest, RestOAuth): fixtures = fixture('webapp_337141', 'user_2519', 'prices') def setUp(self): - BaseOAuth.setUp(self, api_name='webpay') + RestOAuth.setUp(self) # Avoid calling PurchaseTest.setUp(). self.create_switch('marketplace') - self.list_url = list_url('prepare') + self.list_url = reverse('webpay-prepare') self.user = UserProfile.objects.get(pk=2519) def test_allowed(self): self._allowed_verbs(self.list_url, ['post']) def test_anon(self): - eq_(self.anon.post(self.list_url, data={}).status_code, 401) + res = self.anon.post(self.list_url, data=json.dumps({'app': 337141})) + eq_(res.status_code, 403) def test_good(self): self.setup_base() @@ -58,7 +59,7 @@ def test_already_purchased(self, has_purchased): self.setup_package() res = self.client.post(self.list_url, data=json.dumps({'app': 337141})) eq_(res.status_code, 409) - eq_(res.content, '{"reason": "Already purchased app."}') + eq_(res.json, {"reason": "Already purchased app."}) def _post(self): return self.client.post(self.list_url, @@ -223,7 +224,7 @@ def setUp(self): self.grant_permission(self.profile, 'Transaction:NotifyFailure') self.contribution = Contribution.objects.create(addon_id=337141, uuid='sample:uuid') - self.get_url = reverse('failure-detail', + self.get_url = reverse('webpay-failurenotification', kwargs={'pk': self.contribution.pk}) self.data = {'url': 'https://someserver.com', 'attempts': 5} @@ -248,7 +249,7 @@ def test_missing(self): eq_(res.status_code, 400) def test_not_there(self): - self.get_url = reverse('failure-detail', + self.get_url = reverse('webpay-failurenotification', kwargs={'pk': self.contribution.pk + 42}) res = self.client.patch(self.get_url, data=json.dumps(self.data)) eq_(res.status_code, 404) @@ -325,7 +326,7 @@ def test(self): with self.settings(APP_PURCHASE_SECRET=secret, APP_PURCHASE_KEY=key, APP_PURCHASE_AUD=aud): - res = self.client.post(reverse('webpay.sig_check')) + res = self.client.post(reverse('webpay-sig_check')) eq_(res.status_code, 201, res) data = json.loads(res.content) req = jwt.decode(data['sig_check_jwt'].encode('ascii'), secret) diff --git a/mkt/webpay/urls.py b/mkt/webpay/urls.py index 2780aff574a..2c213edffc3 100644 --- a/mkt/webpay/urls.py +++ b/mkt/webpay/urls.py @@ -4,13 +4,12 @@ from tastypie.api import Api from mkt.webpay.resources import (FailureNotificationView, - PreparePayResource, PricesViewSet, + PreparePayView, PricesViewSet, ProductIconResource, sig_check, StatusPayResource) api = Api(api_name='webpay') api.register(ProductIconResource()) -api.register(PreparePayResource()) api.register(StatusPayResource()) api_router = routers.SimpleRouter() @@ -19,7 +18,8 @@ urlpatterns = patterns('', url(r'^', include(api.urls)), url(r'^webpay/', include(api_router.urls)), + url(r'^webpay/prepare/', PreparePayView.as_view(), name='webpay-prepare'), url(r'^webpay/failure/(?P\d+)/', FailureNotificationView.as_view(), - name='failure-detail'), - url(r'^webpay/sig_check/$', sig_check, name='webpay.sig_check') + name='webpay-failurenotification'), + url(r'^webpay/sig_check/$', sig_check, name='webpay-sig_check') ) From a4a74199f48792faaa5995a803f3751e3aba7525 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Mon, 25 Nov 2013 19:19:55 +0100 Subject: [PATCH 3/3] Convert StatusPayResource to DRF (bug 910612) --- mkt/purchase/webpay.py | 4 +- mkt/webpay/resources.py | 65 +++++++++++------------------- mkt/webpay/tests/test_resources.py | 20 +++++---- mkt/webpay/urls.py | 8 ++-- 4 files changed, 41 insertions(+), 56 deletions(-) diff --git a/mkt/purchase/webpay.py b/mkt/purchase/webpay.py index 6ee8814d80f..533ed0f3a01 100644 --- a/mkt/purchase/webpay.py +++ b/mkt/purchase/webpay.py @@ -127,9 +127,7 @@ def _prepare_pay(request, addon): 'Preparing JWT for: %s' % (addon.pk), severity=3) if request.API: - url = reverse('api_dispatch_detail', kwargs={ - 'resource_name': 'status', 'api_name': 'webpay', - 'uuid': uuid_}) + url = reverse('webpay-status', kwargs={'uuid': uuid_}) else: url = reverse('webpay.pay_status', args=[addon.app_slug, uuid_]) return {'webpayJWT': jwt_, 'contribStatusURL': url} diff --git a/mkt/webpay/resources.py b/mkt/webpay/resources.py index dfa678af2aa..38ea85e1478 100644 --- a/mkt/webpay/resources.py +++ b/mkt/webpay/resources.py @@ -2,8 +2,7 @@ import time from django.conf import settings -from django.conf.urls.defaults import url -from django.core.exceptions import ObjectDoesNotExist +from django.http import Http404 import commonware.log import django_filters @@ -25,19 +24,14 @@ from market.models import Price from stats.models import Contribution -from mkt.api.authentication import (OAuthAuthentication, - OptionalOAuthAuthentication, +from mkt.api.authentication import (OptionalOAuthAuthentication, RestAnonymousAuthentication, RestOAuthAuthentication, - RestSharedSecretAuthentication, - SharedSecretAuthentication) -from mkt.api.authorization import (AnonymousReadOnlyAuthorization, - GroupPermission, - OwnerAuthorization, - PermissionAuthorization) -from mkt.api.base import (CORSResource, CORSMixin, http_error, - MarketplaceModelResource, MarketplaceResource, - MarketplaceView) + RestSharedSecretAuthentication) +from mkt.api.authorization import (AllowOwner, AnonymousReadOnlyAuthorization, + GroupPermission, PermissionAuthorization) +from mkt.api.base import (CORSResource, CORSMixin, MarketplaceModelResource, + MarketplaceResource, MarketplaceView) from mkt.api.exceptions import AlreadyPurchased from mkt.purchase.webpay import _prepare_pay, sign_webpay_jwt from mkt.purchase.utils import payments_enabled @@ -45,9 +39,9 @@ from mkt.webpay.models import ProductIcon from mkt.webpay.serializers import PriceSerializer - from . import tasks + log = commonware.log.getLogger('z.webpay') @@ -81,46 +75,33 @@ def post(self, request, *args, **kwargs): return Response(data, status=status.HTTP_201_CREATED) -class StatusPayResource(CORSResource, MarketplaceModelResource): - - class Meta(MarketplaceModelResource.Meta): - always_return_data = True - authentication = (SharedSecretAuthentication(), OAuthAuthentication()) - authorization = OwnerAuthorization() - detail_allowed_methods = ['get'] - queryset = Contribution.objects.filter(type=amo.CONTRIB_PURCHASE) - resource_name = 'status' +class StatusPayView(CORSMixin, MarketplaceView, GenericAPIView): + authentication_classes = [RestOAuthAuthentication, + RestSharedSecretAuthentication] + permission_classes = [AllowOwner] + cors_allowed_methods = ['get'] + queryset = Contribution.objects.filter(type=amo.CONTRIB_PURCHASE) + lookup_field = 'uuid' - def obj_get(self, request=None, **kw): + def get_object(self): try: - obj = super(StatusPayResource, self).obj_get(request=request, **kw) - except ObjectDoesNotExist: + obj = super(StatusPayView, self).get_object() + except Http404: # Anything that's not correct will be raised as a 404 so that it's # harder to iterate over contribution values. log.info('Contribution not found') return None - if not OwnerAuthorization().is_authorized(request, object=obj): - raise http_error(http.HttpForbidden, - 'You are not an author of that app.') - - if not obj.addon.has_purchased(request.amo_user): + if not obj.addon.has_purchased(self.request.amo_user): log.info('Not in AddonPurchase table') return None return obj - def base_urls(self): - return [ - url(r"^(?P%s)/(?P[^/]+)/$" % - self._meta.resource_name, - self.wrap_view('dispatch_detail'), - name='api_dispatch_detail') - ] - - def full_dehydrate(self, bundle): - bundle.data = {'status': 'complete' if bundle.obj.id else 'incomplete'} - return bundle + def get(self, request, *args, **kwargs): + self.object = self.get_object() + data = {'status': 'complete' if self.object else 'incomplete'} + return Response(data) class PriceFilter(django_filters.FilterSet): diff --git a/mkt/webpay/tests/test_resources.py b/mkt/webpay/tests/test_resources.py index 23514f9676f..2232feca046 100644 --- a/mkt/webpay/tests/test_resources.py +++ b/mkt/webpay/tests/test_resources.py @@ -47,9 +47,8 @@ def test_good(self): res = self.client.post(self.list_url, data=json.dumps({'app': 337141})) contribution = Contribution.objects.get() eq_(res.status_code, 201) - eq_(res.json['contribStatusURL'], reverse('api_dispatch_detail', - kwargs={'api_name': 'webpay', 'resource_name': 'status', - 'uuid': contribution.uuid})) + eq_(res.json['contribStatusURL'], + reverse('webpay-status', kwargs={'uuid': contribution.uuid})) ok_(res.json['webpayJWT']) @patch('mkt.webapps.models.Webapp.has_purchased') @@ -74,17 +73,16 @@ def test_waffle_fallback(self): eq_(self._post().status_code, 201) -class TestStatus(BaseOAuth): +class TestStatus(RestOAuth): fixtures = fixture('webapp_337141', 'user_2519') def setUp(self): - super(TestStatus, self).setUp(api_name='webpay') + super(TestStatus, self).setUp() self.contribution = Contribution.objects.create( addon_id=337141, user_id=2519, type=CONTRIB_PURCHASE, uuid='some:uid') - self.get_url = ('api_dispatch_detail', { - 'api_name': 'webpay', 'resource_name': 'status', - 'uuid': self.contribution.uuid}) + self.get_url = reverse('webpay-status', + kwargs={'uuid': self.contribution.uuid}) def test_allowed(self): self._allowed_verbs(self.get_url, ['get']) @@ -112,6 +110,12 @@ def test_no_purchase(self): eq_(res.status_code, 200, res.content) eq_(res.json['status'], 'incomplete', res.content) + def test_not_owner(self): + userprofile2 = UserProfile.objects.get(pk=31337) + self.contribution.update(user=userprofile2) + res = self.client.get(self.get_url) + eq_(res.status_code, 403, res.content) + class TestPrices(RestOAuth): diff --git a/mkt/webpay/urls.py b/mkt/webpay/urls.py index 2c213edffc3..79bacba1aee 100644 --- a/mkt/webpay/urls.py +++ b/mkt/webpay/urls.py @@ -6,11 +6,10 @@ from mkt.webpay.resources import (FailureNotificationView, PreparePayView, PricesViewSet, ProductIconResource, sig_check, - StatusPayResource) + StatusPayView) api = Api(api_name='webpay') api.register(ProductIconResource()) -api.register(StatusPayResource()) api_router = routers.SimpleRouter() api_router.register(r'prices', PricesViewSet) @@ -18,7 +17,10 @@ urlpatterns = patterns('', url(r'^', include(api.urls)), url(r'^webpay/', include(api_router.urls)), - url(r'^webpay/prepare/', PreparePayView.as_view(), name='webpay-prepare'), + url(r'^webpay/status/(?P[^/]+)/', StatusPayView.as_view(), + name='webpay-status'), + url(r'^webpay/prepare/', PreparePayView.as_view(), + name='webpay-prepare'), url(r'^webpay/failure/(?P\d+)/', FailureNotificationView.as_view(), name='webpay-failurenotification'), url(r'^webpay/sig_check/$', sig_check, name='webpay-sig_check')