Merge PR #24264 open-craft/kshitij/bb-2595/tnl-7209/anonymous-apis

* Commits:
  Allow anonymous access for course blocks API

lms/djangoapps/course_api/blocks/forms.py

@@ -3,8 +3,7 @@ Course API Forms
 """
 
 
-import six
-from django.contrib.auth.models import User
+from django.contrib.auth.models import AnonymousUser, User
 from django.core.exceptions import ValidationError
 from django.forms import CharField, ChoiceField, Form, IntegerField
 from django.http import Http404
@@ -49,7 +48,7 @@ class BlockListGetForm(Form):
         try:
             return int(value)
         except ValueError:
-            raise ValidationError(u"'{}' is not a valid depth value.".format(value))
+            raise ValidationError("'{}' is not a valid depth value.".format(value))
 
     def clean_requested_fields(self):
         """
@@ -76,7 +75,7 @@ class BlockListGetForm(Form):
         try:
             usage_key = UsageKey.from_string(usage_key)
         except InvalidKeyError:
-            raise ValidationError(u"'{}' is not a valid usage key.".format(six.text_type(usage_key)))
+            raise ValidationError("'{}' is not a valid usage key.".format(str(usage_key)))
 
         return usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
 
@@ -106,38 +105,86 @@ class BlockListGetForm(Form):
         cleaned_data['user'] = self._clean_requested_user(cleaned_data, usage_key.course_key)
         return cleaned_data
 
+    def clean_username(self):
+        """
+        Return cleaned username.
+
+        Overrides the default behaviour that maps an empty string to None. This
+        allows us to differentiate between no username being provided (None) vs
+        an empty username being provided ('').
+        """
+        # In case all_blocks is specified, ignore the username.
+        if self.cleaned_data.get('all_blocks', False):
+            return None
+
+        # See if 'username' was provided as a parameter in the raw data.
+        # If so, we return the already-cleaned version of that, otherwise we
+        # return None
+        if 'username' in self.data:
+            return self.cleaned_data['username']
+        return None
+
     def _clean_requested_user(self, cleaned_data, course_key):
         """
         Validates and returns the requested_user, while checking permissions.
         """
         requesting_user = self.initial['requesting_user']
         requested_username = cleaned_data.get('username', None)
+        all_blocks = cleaned_data.get('all_blocks', False)
+
+        if requested_username is None and not all_blocks:
+            raise ValidationError({'username': ["This field is required unless all_blocks is requested."]})
+
+        if requesting_user.is_anonymous:
+            return self._verify_anonymous_user(requested_username, course_key, all_blocks)
 
-        if not requested_username:
-            return self._verify_no_user(requesting_user, cleaned_data, course_key)
+        if all_blocks:
+            return self._verify_all_blocks(requesting_user, course_key)
         elif requesting_user.username.lower() == requested_username.lower():
             return self._verify_requesting_user(requesting_user, course_key)
         else:
             return self._verify_other_user(requesting_user, requested_username, course_key)
 
     @staticmethod
-    def _verify_no_user(requesting_user, cleaned_data, course_key):
+    def _verify_anonymous_user(username, course_key, all_blocks):
         """
-        Verifies form for when no username is specified, including permissions.
+        Verifies form for when the requesting user is anonymous.
         """
-        # Verify that access to all blocks is requested
-        # (and not unintentionally requested).
-        if not cleaned_data.get('all_blocks', None):
-            raise ValidationError({'username': ['This field is required unless all_blocks is requested.']})
+        if all_blocks:
+            raise PermissionDenied(
+                "Anonymous users do not have permission to access all blocks in '{course_key}'.".format(
+                    course_key=str(course_key),
+                )
+            )
+
+        # Check for '' and explicitly '' since the only valid option for anonymous users is
+        # an empty string that corresponds to an anonymous user.
+        if username != '':
+            raise PermissionDenied("Anonymous users cannot access another user's blocks.")
+
+        if not permissions.is_course_public(course_key):
+            raise PermissionDenied(
+                "Course blocks for '{course_key}' cannot be accessed anonymously.".format(
+                    course_key=course_key,
+                )
+            )
+
+        return AnonymousUser()
 
+    @staticmethod
+    def _verify_all_blocks(requesting_user, course_key):  # pylint: disable=useless-return
+        """
+        Verifies form for when no username is specified, including permissions.
+        """
         # Verify all blocks can be accessed for the course.
         if not permissions.can_access_all_blocks(requesting_user, course_key):
             raise PermissionDenied(
-                u"'{requesting_username}' does not have permission to access all blocks in '{course_key}'."
-                .format(requesting_username=requesting_user.username, course_key=six.text_type(course_key))
+                "'{requesting_username}' does not have permission to access all blocks in '{course_key}'.".format(
+                    requesting_username=requesting_user.username,
+                    course_key=str(course_key),
+                )
             )
 
-        # return None for user
         return None
 
     @staticmethod
@@ -147,8 +194,9 @@ class BlockListGetForm(Form):
         """
         if not permissions.can_access_self_blocks(requesting_user, course_key):
             raise PermissionDenied(
-                u"Course blocks for '{requesting_username}' cannot be accessed."
-                .format(requesting_username=requesting_user.username)
+                "Course blocks for '{requesting_username}' cannot be accessed.".format(
+                    requesting_username=requesting_user.username,
+                )
             )
         return requesting_user
 
@@ -158,11 +206,18 @@ class BlockListGetForm(Form):
         Verifies whether the requesting user can access another user's view of
         the blocks in the course.
         """
+        # If accessing a public course, and requesting only content available publicly,
+        # we can allow the request.
+        if requested_username == '' and permissions.is_course_public(course_key):
+            return AnonymousUser()
+
         # Verify requesting user can access the user's blocks.
         if not permissions.can_access_others_blocks(requesting_user, course_key):
             raise PermissionDenied(
-                u"'{requesting_username}' does not have permission to access view for '{requested_username}'."
-                .format(requesting_username=requesting_user.username, requested_username=requested_username)
+                "'{requesting_username}' does not have permission to access view for '{requested_username}'.".format(
+                    requesting_username=requesting_user.username,
+                    requested_username=requested_username,
+                )
             )
 
         # Verify user exists.
@@ -170,5 +225,7 @@ class BlockListGetForm(Form):
             return User.objects.get(username=requested_username)
         except User.DoesNotExist:
             raise Http404(
-                u"Requested user '{requested_username}' does not exist.".format(requested_username=requested_username)
+                "Requested user '{requested_username}' does not exist.".format(
+                    requested_username=requested_username,
+                )
             )

lms/djangoapps/course_api/blocks/permissions.py

 """
 Encapsulates permissions checks for Course Blocks API
 """
-
+from django.contrib.auth.models import User
+from opaque_keys.edx.keys import CourseKey
 
 from lms.djangoapps.courseware.access import has_access
+from lms.djangoapps.courseware.access_response import AccessResponse
+from lms.djangoapps.courseware.access_utils import ACCESS_DENIED, ACCESS_GRANTED, check_public_access
+from lms.djangoapps.courseware.courses import get_course
 from student.models import CourseEnrollment
 from student.roles import CourseStaffRole
+from xmodule.course_module import COURSE_VISIBILITY_PUBLIC
 
 
 def can_access_all_blocks(requesting_user, course_key):
@@ -24,11 +29,25 @@ def can_access_others_blocks(requesting_user, course_key):
     return has_access(requesting_user, CourseStaffRole.ROLE, course_key)
 
 
-def can_access_self_blocks(requesting_user, course_key):
+def can_access_self_blocks(requesting_user: User, course_key: CourseKey) -> AccessResponse:
     """
     Returns whether the requesting_user can access own blocks.
     """
-    return (
+    user_is_enrolled_or_staff = (  # pylint: disable=consider-using-ternary
         (requesting_user.id and CourseEnrollment.is_enrolled(requesting_user, course_key)) or
         has_access(requesting_user, CourseStaffRole.ROLE, course_key)
     )
+    if user_is_enrolled_or_staff:
+        return ACCESS_GRANTED
+    try:
+        return is_course_public(course_key)
+    except ValueError:
+        return ACCESS_DENIED
+
+
+def is_course_public(course_key: CourseKey) -> AccessResponse:
+    """
+    This checks if a course is publicly accessible or not.
+    """
+    course = get_course(course_key, depth=0)
+    return check_public_access(course, [COURSE_VISIBILITY_PUBLIC])

lms/djangoapps/course_api/blocks/tests/test_views.py

@@ -4,9 +4,11 @@ Tests for Blocks Views
 
 
 from datetime import datetime
+from unittest import mock
+from unittest.mock import Mock
+from urllib.parse import urlencode, urlunparse
 
-import six
-from six.moves.urllib.parse import urlencode, urlunparse
+from django.conf import settings
 from django.urls import reverse
 from opaque_keys.edx.locator import CourseLocator
 
@@ -38,7 +40,7 @@ class TestBlocksView(SharedModuleStoreTestCase):
         cls.course_usage_key = cls.store.make_course_usage_key(cls.course_key)
 
         cls.non_orphaned_block_usage_keys = set(
-            six.text_type(item.location)
+            str(item.location)
             for item in cls.store.get_items(cls.course_key)
             # remove all orphaned items in the course, except for the root 'course' block
             if cls.store.get_parent_location(item.location) or item.category == 'course'
@@ -49,17 +51,18 @@ class TestBlocksView(SharedModuleStoreTestCase):
 
         # create and enroll user in the toy course
         self.user = UserFactory.create()
+        self.admin_user = AdminFactory.create()
         self.client.login(username=self.user.username, password='test')
         CourseEnrollmentFactory.create(user=self.user, course_id=self.course_key)
 
         # default values for url and query_params
         self.url = reverse(
             'blocks_in_block_tree',
-            kwargs={'usage_key_string': six.text_type(self.course_usage_key)}
+            kwargs={'usage_key_string': str(self.course_usage_key)}
         )
         self.query_params = {'depth': 'all', 'username': self.user.username}
 
-    def verify_response(self, expected_status_code=200, params=None, url=None):
+    def verify_response(self, expected_status_code=200, params=None, url=None, cacheable=False):
         """
         Ensure that sending a GET request to the specified URL returns the
         expected status code.
@@ -77,7 +80,13 @@ class TestBlocksView(SharedModuleStoreTestCase):
         if params:
             self.query_params.update(params)
         response = self.client.get(url or self.url, self.query_params)
-        self.assertEqual(response.status_code, expected_status_code)
+        self.assertEqual(response.status_code, expected_status_code, str(response.content))
+        if cacheable:
+            assert response.get('Cache-Control', None) == 'max-age={}'.format(
+                settings.CACHE_MIDDLEWARE_SECONDS
+            )
+        else:
+            assert response.get('Cache-Control', None) is None
         return response
 
     def verify_response_block_list(self, response):
@@ -94,7 +103,7 @@ class TestBlocksView(SharedModuleStoreTestCase):
         Verify that the response contains the expected blocks
         """
         self.assertSetEqual(
-            set(six.iterkeys(response.data['blocks'])),
+            set(response.data['blocks'].keys()),
             self.non_orphaned_block_usage_keys,
         )
 
@@ -103,7 +112,7 @@ class TestBlocksView(SharedModuleStoreTestCase):
         Verify the response has the expected structure
         """
         self.verify_response_block_dict(response)
-        for block_key_string, block_data in six.iteritems(response.data['blocks']):
+        for block_key_string, block_data in response.data['blocks'].items():
             block_key = deserialize_usage_key(block_key_string, self.course_key)
             xblock = self.store.get_item(block_key)
 
@@ -116,7 +125,7 @@ class TestBlocksView(SharedModuleStoreTestCase):
 
             if xblock.has_children:
                 self.assertSetEqual(
-                    set(six.text_type(child.location) for child in xblock.get_children()),
+                    set(str(child.location) for child in xblock.get_children()),
                     set(block_data['children']),
                 )
 
@@ -148,19 +157,97 @@ class TestBlocksView(SharedModuleStoreTestCase):
         else:
             self.assertFalse(expression)
 
-    def test_not_authenticated(self):
+    def test_not_authenticated_non_public_course_with_other_username(self):
+        """
+        Verify behaviour when accessing course blocks of a non-public course for another user anonymously.
+        """
+        self.client.logout()
+        self.verify_response(403)
+
+    def test_not_authenticated_non_public_course_with_all_blocks(self):
+        """
+        Verify behaviour when accessing all course blocks of a non-public course anonymously.
+        """
         self.client.logout()
-        self.verify_response(401)
+        self.query_params.pop('username')
+        self.query_params['all_blocks'] = True
+        self.verify_response(403)
 
-    def test_not_enrolled(self):
+    def test_not_authenticated_non_public_course_with_blank_username(self):
+        """
+        Verify behaviour when accessing course blocks of a non-public course for anonymous user anonymously.
+        """
+        self.client.logout()
+        self.query_params['username'] = ''
+        self.verify_response(403)
+
+    @mock.patch("course_api.blocks.forms.permissions.is_course_public", Mock(return_value=True))
+    def test_not_authenticated_public_course_with_other_username(self):
+        """
+        Verify behaviour when accessing course blocks of a public course for another user anonymously.
+        """
+        self.client.logout()
+        self.verify_response(403)
+
+    @mock.patch("course_api.blocks.forms.permissions.is_course_public", Mock(return_value=True))
+    def test_not_authenticated_public_course_with_all_blocks(self):
+        """
+        Verify behaviour when accessing all course blocks of a public course anonymously.
+        """
+        self.client.logout()
+        self.query_params.pop('username')
+        self.query_params['all_blocks'] = True
+        self.verify_response(403)
+
+    @mock.patch("course_api.blocks.forms.permissions.is_course_public", Mock(return_value=True))
+    def test_not_authenticated_public_course_with_blank_username(self):
+        """
+        Verify behaviour when accessing course blocks of a public course for anonymous user anonymously.
+        """
+        self.client.logout()
+        self.query_params['username'] = ''
+        self.verify_response(cacheable=True)
+
+    def test_not_enrolled_non_public_course(self):
+        """
+        Verify behaviour when accessing course blocks for a non-public course as a user not enrolled in course.
+        """
         CourseEnrollment.unenroll(self.user, self.course_key)
         self.verify_response(403)
 
+    @mock.patch("course_api.blocks.forms.permissions.is_course_public", Mock(return_value=True))
+    def test_not_enrolled_public_course(self):
+        """
+        Verify behaviour when accessing course blocks for a public course as a user not enrolled in course.
+        """
+        self.query_params['username'] = ''
+        CourseEnrollment.unenroll(self.user, self.course_key)
+        self.verify_response(cacheable=True)
+
+    @mock.patch("course_api.blocks.forms.permissions.is_course_public", Mock(return_value=True))
+    def test_public_course_all_blocks_and_empty_username(self):
+        """
+        Verify behaviour when specifying both all_blocks and username='', and ensure the response is not cached.
+        """
+        self.query_params['username'] = ''
+        self.query_params['all_blocks'] = True
+        # Verify response for a regular user.
+        self.verify_response(403, cacheable=False)
+        # Verify response for an unenrolled user.
+        CourseEnrollment.unenroll(self.user, self.course_key)
+        self.verify_response(403, cacheable=False)
+        # Verify response for an anonymous user.
+        self.client.logout()
+        self.verify_response(403, cacheable=False)
+        # Verify response for a staff user.
+        self.client.login(username=self.admin_user.username, password='test')
+        self.verify_response(cacheable=False)
+
     def test_non_existent_course(self):
         usage_key = self.store.make_course_usage_key(CourseLocator('non', 'existent', 'course'))
         url = reverse(
             'blocks_in_block_tree',
-            kwargs={'usage_key_string': six.text_type(usage_key)}
+            kwargs={'usage_key_string': str(usage_key)}
         )
         self.verify_response(403, url=url)
 
@@ -174,16 +261,16 @@ class TestBlocksView(SharedModuleStoreTestCase):
         self.verify_response(400)
 
     def test_no_user_staff_all_blocks(self):
-        self.client.login(username=AdminFactory.create().username, password='test')
+        self.client.login(username=self.admin_user.username, password='test')
         self.query_params.pop('username')
         self.query_params['all_blocks'] = True
         self.verify_response()
 
     def test_basic(self):
         response = self.verify_response()
-        self.assertEqual(response.data['root'], six.text_type(self.course_usage_key))
+        self.assertEqual(response.data['root'], str(self.course_usage_key))
         self.verify_response_block_dict(response)
-        for block_key_string, block_data in six.iteritems(response.data['blocks']):
+        for block_key_string, block_data in response.data['blocks'].items():
             block_key = deserialize_usage_key(block_key_string, self.course_key)
             self.assertEqual(block_data['id'], block_key_string)
             self.assertEqual(block_data['type'], block_key.block_type)
@@ -196,7 +283,7 @@ class TestBlocksView(SharedModuleStoreTestCase):
     def test_block_counts_param(self):
         response = self.verify_response(params={'block_counts': ['course', 'chapter']})
         self.verify_response_block_dict(response)
-        for block_data in six.itervalues(response.data['blocks']):
+        for block_data in response.data['blocks'].values():
             self.assertEqual(
                 block_data['block_counts']['course'],
                 1 if block_data['type'] == 'course' else 0,
@@ -215,7 +302,7 @@ class TestBlocksView(SharedModuleStoreTestCase):
             'student_view_data': self.BLOCK_TYPES_WITH_STUDENT_VIEW_DATA + ['chapter']
         })
         self.verify_response_block_dict(response)
-        for block_data in six.itervalues(response.data['blocks']):
+        for block_data in response.data['blocks'].values():
             self.assert_in_iff(
                 'student_view_data',
                 block_data,
@@ -233,12 +320,13 @@ class TestBlocksView(SharedModuleStoreTestCase):
             - other_course_settings
             - course_visibility
         """
+        self.client.login(username=self.admin_user.username, password='test')
         response = self.verify_response(params={
             'all_blocks': True,
             'requested_fields': ['other_course_settings', 'course_visibility'],
         })
         self.verify_response_block_dict(response)
-        for block_data in six.itervalues(response.data['blocks']):
+        for block_data in response.data['blocks'].values():
             self.assert_in_iff(
                 'other_course_settings',
                 block_data,
@@ -264,12 +352,13 @@ class TestBlocksView(SharedModuleStoreTestCase):
             - other_course_settings
             - course_visibility
         """
+        self.client.login(username=self.admin_user.username, password='test')
         response = self.verify_response(params={
             'all_blocks': True,
             'requested_fields': ['course_visibility'],
         })
         self.verify_response_block_dict(response)
-        for block_data in six.itervalues(response.data['blocks']):
+        for block_data in response.data['blocks'].values():
             self.assertNotIn(
                 'other_course_settings',
                 block_data
@@ -284,7 +373,7 @@ class TestBlocksView(SharedModuleStoreTestCase):
     def test_navigation_param(self):
         response = self.verify_response(params={'nav_depth': 10})
         self.verify_response_block_dict(response)
-        for block_data in six.itervalues(response.data['blocks']):
+        for block_data in response.data['blocks'].values():
             self.assertIn('descendants', block_data)
 
     def test_requested_fields_param(self):
@@ -314,7 +403,7 @@ class TestBlocksInCourseView(TestBlocksView):
     def setUp(self):
         super(TestBlocksInCourseView, self).setUp()
         self.url = reverse('blocks_in_course')
-        self.query_params['course_id'] = six.text_type(self.course_key)
+        self.query_params['course_id'] = str(self.course_key)
 
     def test_no_course_id(self):
         self.query_params.pop('course_id')
@@ -324,4 +413,4 @@ class TestBlocksInCourseView(TestBlocksView):
         self.verify_response(400, params={'course_id': 'invalid_course_id'})
 
     def test_non_existent_course(self):
-        self.verify_response(403, params={'course_id': six.text_type(CourseLocator('non', 'existent', 'course'))})
+        self.verify_response(403, params={'course_id': str(CourseLocator('non', 'existent', 'course'))})

lms/djangoapps/course_api/blocks/views.py

@@ -7,6 +7,7 @@ import six
 from django.core.exceptions import ValidationError
 from django.db import transaction
 from django.http import Http404
+from django.utils.cache import patch_response_headers
 from django.utils.decorators import method_decorator
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
@@ -22,8 +23,8 @@ from .api import get_blocks
 from .forms import BlockListGetForm
 
 
-@view_auth_classes()
 @method_decorator(transaction.non_atomic_requests, name='dispatch')
+@view_auth_classes(is_authenticated=False)
 class BlocksView(DeveloperErrorViewMixin, ListAPIView):
     """
     **Use Case**
@@ -53,12 +54,16 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView):
 
         * username: (string) Required, unless ``all_blocks`` is specified.
           Specify the username for the user whose course blocks are requested.
-          Only users with course staff permissions can specify other users'
-          usernames. If a username is specified, results include blocks that
-          are visible to that user, including those based on group or cohort
-          membership or randomized content assigned to that user.
+          A blank/empty username can be used to request the blocks accessible
+          to anonymous users (for public courses). Only users with course staff
+          permissions can specify other users' usernames. If a username is
+          specified, results include blocks that are visible to that user,
+          including those based on group or cohort membership or randomized
+          content assigned to that user.
 
           Example: username=anjali
+                   username=''
+                   username
 
         * student_view_data: (list) Indicates for which block types to return
           student_view_data.
@@ -211,7 +216,7 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView):
             raise ValidationError(params.errors)
 
         try:
-            return Response(
+            response = Response(
                 get_blocks(
                     request,
                     params.cleaned_data['usage_key'],
@@ -226,11 +231,17 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView):
                     hide_access_denials=hide_access_denials,
                 )
             )
+            # If the username is an empty string, and not None, then we are requesting
+            # data about the anonymous view of a course, which can be cached. In this
+            # case we add the usual caching headers to the response.
+            if params.cleaned_data.get('username', None) == '':
+                patch_response_headers(response)
+            return response
         except ItemNotFoundError as exception:
             raise Http404(u"Block not found: {}".format(text_type(exception)))
 
 
-@view_auth_classes()
+@view_auth_classes(is_authenticated=False)
 class BlocksInCourseView(BlocksView):
     """
     **Use Case**