Use AnonymousReadOnlyAuthorization on RatingResource rather than SelectiveAuthentication; remove SelectiveAuthentication

mkt/api/authentication.py

@@ -137,22 +137,6 @@ class OAuthAuthentication(Authentication):
         return True
 
 
-class SelectiveAuthentication(Authentication):
-    """
-    Authenticate all requests using verbs passed as positional arguments to the
-    constructor. Example usage:
-
-    class Meta:
-        authentication = SelectiveAuthentication('GET', 'POST')
-    """
-    def __init__(self, *args):
-        self.skip_authentication = args
-        super(SelectiveAuthentication, self).__init__()
-
-    def is_authenticated(self, request, **kwargs):
-        return request.method in self.skip_authentication
-
-
 class OptionalOAuthAuthentication(OAuthAuthentication):
     """
     Like OAuthAuthentication, but doesn't require there to be

mkt/api/authorization.py

@@ -0,0 +1,12 @@
+from tastypie.authorization import ReadOnlyAuthorization
+
+
+class AnonymousReadOnlyAuthorization(ReadOnlyAuthorization):
+    def is_authorized(self, request, object=None):
+        """
+        Only allow ``GET`` requests for anonymous users.
+        """
+        if request.user.is_anonymous():
+            sup = super(AnonymousReadOnlyAuthorization, self)
+            return sup.is_authorized(request, object)
+        return True

mkt/api/base.py

@@ -101,19 +101,18 @@ class Marketplace(object):
         of Authentication methods. If so it will go through in order, when one
         passes, it will use that.
 
-        If authentication backends return a response (e.g. DigestAuth), it will
-        be squashed. To get around this, raise an ImmediateHttpResponse with the
-        desired response.
+        Any authentication method can still return a HttpResponse to break out
+        of the loop if they desire.
         """
         for auth in self._auths():
             auth_result = auth.is_authenticated(request)
 
             if isinstance(auth_result, http.HttpResponse):
-                return False
+                raise ImmediateHttpResponse(response=auth_result)
 
             if auth_result:
                 log.info('Logged in using %s' % auth.__class__.__name__)
-                return True
+                return
 
         raise ImmediateHttpResponse(response=http.HttpUnauthorized())
 

mkt/api/tests/test_authentication.py

@@ -237,20 +237,3 @@ class TestMultipleAuthentication(TestCase):
             eq_(self.resource.is_authenticated(req), None)
         # This never even got called.
         ok_(not next_auth.is_authenticated.called)
-
-
-class TestSelectiveAuthentication(TestCase):
-
-    def setUp(self):
-        self.factory = RequestFactory()
-
-    def test_single_verb(self):
-        auth = authentication.SelectiveAuthentication('GET')
-        eq_(auth.is_authenticated(self.factory.get('/')), True)
-        eq_(auth.is_authenticated(self.factory.post('/')), False)
-
-    def test_multiple_verbs(self):
-        auth = authentication.SelectiveAuthentication('GET', 'PUT')
-        eq_(auth.is_authenticated(self.factory.get('/')), True)
-        eq_(auth.is_authenticated(self.factory.put('/')), True)
-        eq_(auth.is_authenticated(self.factory.post('/')), False)

mkt/api/tests/test_authorization.py

@@ -0,0 +1,36 @@
+from django.contrib.auth.models import AnonymousUser, User
+
+from nose.tools import eq_
+
+from amo.tests import TestCase
+from test_utils import RequestFactory
+
+from mkt.api.authorization import AnonymousReadOnlyAuthorization
+from mkt.site.fixtures import fixture
+
+
+class TestAnonymousReadOnlyAuthorization(TestCase):
+    fixtures = fixture('user_2519')
+
+    def setUp(self):
+        self.get = RequestFactory().get('/')
+        self.post = RequestFactory().post('/')
+        self.auth = AnonymousReadOnlyAuthorization()
+        self.anon = AnonymousUser()
+        self.user = User.objects.get(pk=2519)
+
+    def test_get_anonymous(self):
+        self.get.user = self.anon
+        eq_(self.auth.is_authorized(self.get), True)
+
+    def test_get_authenticated(self):
+        self.get.user = self.user
+        eq_(self.auth.is_authorized(self.get), True)
+
+    def test_post_anonymous(self):
+        self.post.user = self.anon
+        eq_(self.auth.is_authorized(self.post), False)
+
+    def test_post_authenticated(self):
+        self.post.user = self.user
+        eq_(self.auth.is_authorized(self.post), True)

mkt/ratings/resources.py

@@ -12,11 +12,12 @@ from addons.models import Addon
 from lib.metrics import record_action
 
 from mkt.api.authentication import (AppOwnerAuthorization,
-                                    OwnerAuthorization,
                                     OAuthAuthentication,
+                                    OptionalOAuthAuthentication,
+                                    OwnerAuthorization,
                                     PermissionAuthorization,
-                                    SelectiveAuthentication,
                                     SharedSecretAuthentication)
+from mkt.api.authorization import AnonymousReadOnlyAuthorization
 from mkt.api.base import MarketplaceModelResource
 from mkt.api.resources import AppResource, UserResource
 from mkt.ratings.forms import ReviewForm
@@ -40,9 +41,8 @@ class RatingResource(MarketplaceModelResource):
         detail_allowed_methods = ['get', 'put', 'delete']
         always_return_data = True
         authentication = (SharedSecretAuthentication(),
-                          OAuthAuthentication(),
-                          SelectiveAuthentication('GET'))
-        authorization = Authorization()
+                          OptionalOAuthAuthentication())
+        authorization = AnonymousReadOnlyAuthorization()
         fields = ['rating', 'body']
 
         filtering = {

mkt/ratings/tests/test_resources.py

@@ -40,6 +40,10 @@ class TestRatingResource(BaseOAuth, AMOPaths):
         eq_(res.status_code, 200)
         assert 'user' not in data
 
+    def test_anonymous_post_list_fails(self):
+        res, data = self._create(anonymous=True)
+        eq_(res.status_code, 401)
+
     def test_anonymous_get_detail(self):
         res = self.anon.get(self.collection_url)
         data = json.loads(res.content)
@@ -59,7 +63,7 @@ class TestRatingResource(BaseOAuth, AMOPaths):
         assert data['user']['can_rate']
         assert data['user']['has_rated']
 
-    def _create(self, data=None):
+    def _create(self, data=None, anonymous=False):
         default_data = {
             'app': self.app.id,
             'body': 'Rocking the free web.',
@@ -68,7 +72,8 @@ class TestRatingResource(BaseOAuth, AMOPaths):
         if data:
             default_data.update(data)
         json_data = json.dumps(default_data)
-        res = self.client.post(list_url('rating'), data=json_data)
+        client = self.anon if anonymous else self.client
+        res = client.post(list_url('rating'), data=json_data)
         try:
             res_data = json.loads(res.content)
         except ValueError: