Allow anonymous GET requests to ratings API (bug 858324)

docs/topics/api/ratings.rst

@@ -7,8 +7,6 @@ Ratings API
 These endpoints allow the retrieval, creation, and modification of ratings on
 apps in Marketplace.
 
-.. note:: All ratings methods require authentication.
-
 
 _`List`
 =======
@@ -17,6 +15,8 @@ _`List`
 
     Get a list of ratings from the Marketplace
 
+    .. note:: Authentication is optional.
+
     **Request**:
 
     :query app: the ID or slug of the app whose ratings are to be returned.
@@ -67,6 +67,8 @@ _`Detail`
     Get a single rating from the Marketplace using its `resource_uri` from the
     `List`_.
 
+    .. note:: Authentication is optional.
+
     **Response**:
 
     .. code-block:: json
@@ -93,6 +95,8 @@ _`Create`
 
     Create a rating.
 
+    .. note:: Authentication required.
+
     **Request**:
 
     :param app: the ID of the app being reviewed
@@ -137,6 +141,8 @@ _`Update`
     Update a rating from the Marketplace using its `resource_uri` from the
     `List`_.
 
+    .. note:: Authentication required.
+
     **Request**:
 
     :param body: text of the rating
@@ -174,6 +180,8 @@ _`Delete`
     Delete a rating from the Marketplace using its `resource_uri` from the
     `List`_.
 
+    .. note:: Authentication required.
+
     **Response**:
 
     :status 204: successfully deleted.

mkt/api/authentication.py

@@ -137,6 +137,22 @@ 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/base.py

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

mkt/api/tests/test_authentication.py

@@ -237,3 +237,20 @@ 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/ratings/resources.py

@@ -1,5 +1,6 @@
-import commonware.log
 from django.conf.urls import url
+
+import commonware.log
 from tastypie import fields, http
 from tastypie.bundle import Bundle
 from tastypie.authorization import Authorization
@@ -14,6 +15,7 @@ from mkt.api.authentication import (AppOwnerAuthorization,
                                     OwnerAuthorization,
                                     OAuthAuthentication,
                                     PermissionAuthorization,
+                                    SelectiveAuthentication,
                                     SharedSecretAuthentication)
 from mkt.api.base import MarketplaceModelResource
 from mkt.api.resources import AppResource, UserResource
@@ -37,7 +39,9 @@ class RatingResource(MarketplaceModelResource):
         list_allowed_methods = ['get', 'post']
         detail_allowed_methods = ['get', 'put', 'delete']
         always_return_data = True
-        authentication = (SharedSecretAuthentication(), OAuthAuthentication())
+        authentication = (SharedSecretAuthentication(),
+                          OAuthAuthentication(),
+                          SelectiveAuthentication('GET'))
         authorization = Authorization()
         fields = ['rating', 'body']
 
@@ -151,12 +155,18 @@ class RatingResource(MarketplaceModelResource):
                 'slug': addon.app_slug
             }
 
-            filters = dict(addon=addon, user=request.user)
+            filters = dict(addon=addon)
             if addon.is_packaged:
                 filters['version'] = addon.current_version
-            existing_review = Review.objects.valid().filter(**filters).exists()
-            data['user'] = {'can_rate': not addon.has_author(request.user),
-                            'has_rated': existing_review}
+
+            if not request.user.is_anonymous():
+                filters['user'] = request.user
+                existing_review = Review.objects.valid().filter(**filters)
+                data['user'] = {'can_rate': not addon.has_author(request.user),
+                                'has_rated': existing_review.exists()}
+            else:
+                data['user'] = None
+
         return data
 
     def override_urls(self):

mkt/ratings/tests/test_resources.py

@@ -34,6 +34,18 @@ class TestRatingResource(BaseOAuth, AMOPaths):
         assert not data['user']['can_rate']
         assert not data['user']['has_rated']
 
+    def test_anonymous_get_list(self):
+        res = self.anon.get(list_url('rating'))
+        data = json.loads(res.content)
+        eq_(res.status_code, 200)
+        assert 'user' not in data
+
+    def test_anonymous_get_detail(self):
+        res = self.anon.get(self.collection_url)
+        data = json.loads(res.content)
+        eq_(res.status_code, 200)
+        eq_(data['user'], None)
+
     def test_non_owner(self):
         res = self.client.get(self.collection_url)
         data = json.loads(res.content)
@@ -67,7 +79,7 @@ class TestRatingResource(BaseOAuth, AMOPaths):
         res, data = self._create()
         eq_(201, res.status_code)
         assert data['resource_uri']
-        eq_(data['report_spam'],  data['resource_uri'] + 'flag/')
+        eq_(data['report_spam'], data['resource_uri'] + 'flag/')
 
     def test_create_bad_data(self):
         """