Merge pull request #8212 from edx/mobile/API_decorators

Move generic mobile API view decorators.
This commit is contained in:
Nimisha Asthagiri
2015-05-27 15:36:48 -04:00
12 changed files with 93 additions and 176 deletions

View File

@@ -124,7 +124,6 @@ def _has_access_course_desc(user, action, course):
'load' -- load the courseware, see inside the course
'load_forum' -- can load and contribute to the forums (one access level for now)
'load_mobile' -- can load from a mobile context
'load_mobile_no_enrollment_check' -- can load from a mobile context without checking for enrollment
'enroll' -- enroll. Checks for enrollment window,
ACCESS_REQUIRE_STAFF_FOR_COURSE,
'see_exists' -- can see that the course exists.
@@ -157,30 +156,20 @@ def _has_access_course_desc(user, action, course):
"""
Can this user access this course from a mobile device?
"""
return (
# check mobile requirements
can_load_mobile_no_enroll_check() and
# check enrollment
(
CourseEnrollment.is_enrolled(user, course.id) or
_has_staff_access_to_descriptor(user, course, course.id)
)
)
def can_load_mobile_no_enroll_check():
"""
Can this enrolled user access this course from a mobile device?
Note: does not check for enrollment since it is assumed the caller has done so.
"""
return (
# check start date
can_load() and
# check mobile_available flag
is_mobile_available_for_user(user, course) and
# check staff access, if not then check for unfulfilled milestones
(
# either is a staff user or
_has_staff_access_to_descriptor(user, course, course.id) or
not any_unfulfilled_milestones(course.id, user.id)
(
# check enrollment
CourseEnrollment.is_enrolled(user, course.id) and
# check for unfulfilled milestones
not any_unfulfilled_milestones(course.id, user.id)
)
)
)
@@ -307,7 +296,6 @@ def _has_access_course_desc(user, action, course):
'view_courseware_with_prerequisites': can_view_courseware_with_prerequisites,
'load_forum': can_load_forum,
'load_mobile': can_load_mobile,
'load_mobile_no_enrollment_check': can_load_mobile_no_enroll_check,
'enroll': can_enroll,
'see_exists': see_exists,
'staff': lambda: _has_staff_access_to_descriptor(user, course, course.id),

View File

@@ -11,41 +11,12 @@ from xmodule.modulestore.django import modulestore
from xmodule.modulestore.xml_importer import import_course_from_xml
from ..testutils import (
MobileAPITestCase, MobileCourseAccessTestMixin, MobileEnrolledCourseAccessTestMixin, MobileAuthTestMixin
MobileAPITestCase, MobileCourseAccessTestMixin, MobileAuthTestMixin
)
class TestAbout(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin):
"""
Tests for /api/mobile/v0.5/course_info/{course_id}/about
"""
REVERSE_INFO = {'name': 'course-about-detail', 'params': ['course_id']}
def verify_success(self, response):
super(TestAbout, self).verify_success(response)
self.assertTrue('overview' in response.data)
def init_course_access(self, course_id=None):
# override this method since enrollment is not required for the About endpoint.
self.login()
def test_about_static_rewrite(self):
self.login()
about_usage_key = self.course.id.make_usage_key('about', 'overview')
about_module = modulestore().get_item(about_usage_key)
underlying_about_html = about_module.data
# check that we start with relative static assets
self.assertIn('\"/static/', underlying_about_html)
# but shouldn't finish with any
response = self.api_response()
self.assertNotIn('\"/static/', response.data['overview'])
@ddt.ddt
class TestUpdates(MobileAPITestCase, MobileAuthTestMixin, MobileEnrolledCourseAccessTestMixin):
class TestUpdates(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin):
"""
Tests for /api/mobile/v0.5/course_info/{course_id}/updates
"""
@@ -111,7 +82,7 @@ class TestUpdates(MobileAPITestCase, MobileAuthTestMixin, MobileEnrolledCourseAc
self.assertIn("Update" + str(num), update_data['content'])
class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileEnrolledCourseAccessTestMixin):
class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin):
"""
Tests for /api/mobile/v0.5/course_info/{course_id}/handouts
"""

View File

@@ -4,15 +4,10 @@ URLs for course_info API
from django.conf.urls import patterns, url
from django.conf import settings
from .views import CourseAboutDetail, CourseUpdatesList, CourseHandoutsList
from .views import CourseUpdatesList, CourseHandoutsList
urlpatterns = patterns(
'mobile_api.course_info.views',
url(
r'^{}/about$'.format(settings.COURSE_ID_PATTERN),
CourseAboutDetail.as_view(),
name='course-about-detail'
),
url(
r'^{}/handouts$'.format(settings.COURSE_ID_PATTERN),
CourseHandoutsList.as_view(),

View File

@@ -87,33 +87,3 @@ class CourseHandoutsList(generics.ListAPIView):
else:
# course_handouts_module could be None if there are no handouts
raise Http404(u"No handouts for {}".format(unicode(course.id)))
@mobile_view()
class CourseAboutDetail(generics.RetrieveAPIView):
"""
**Use Case**
Get the HTML for the course about page.
**Example request**:
GET /api/mobile/v0.5/course_info/{organization}/{course_number}/{course_run}/about
**Response Values**
* overview: The HTML for the course About page.
"""
@mobile_course_access(verify_enrolled=False)
def get(self, request, course, *args, **kwargs):
# There are other fields, but they don't seem to be in use.
# see courses.py:get_course_about_section.
#
# This can also return None, so check for that before calling strip()
about_section_html = get_course_about_section(course, "overview")
about_section_html = make_static_urls_absolute(self.request, about_section_html)
return Response(
{"overview": about_section_html.strip() if about_section_html else ""}
)

View File

@@ -7,8 +7,7 @@ Test utilities for mobile API tests:
Test Mixins to be included by concrete test classes and provide implementation of common test methods:
MobileAuthTestMixin - tests for APIs with mobile_view and is_user=False.
MobileAuthUserTestMixin - tests for APIs with mobile_view and is_user=True.
MobileCourseAccessTestMixin - tests for APIs with mobile_course_access and verify_enrolled=False.
MobileEnrolledCourseAccessTestMixin - tests for APIs with mobile_course_access and verify_enrolled=True.
MobileCourseAccessTestMixin - tests for APIs with mobile_course_access.
"""
# pylint: disable=no-member
import ddt
@@ -130,7 +129,6 @@ class MobileAuthUserTestMixin(MobileAuthTestMixin):
class MobileCourseAccessTestMixin(MobileAPIMilestonesMixin):
"""
Test Mixin for testing APIs marked with mobile_course_access.
(Use MobileEnrolledCourseAccessTestMixin when verify_enrolled is set to True.)
Subclasses are expected to inherit from MobileAPITestCase.
Subclasses can override verify_success, verify_failure, and init_course_access methods.
"""
@@ -197,11 +195,6 @@ class MobileCourseAccessTestMixin(MobileAPIMilestonesMixin):
else:
self.verify_failure(response)
class MobileEnrolledCourseAccessTestMixin(MobileCourseAccessTestMixin):
"""
Test Mixin for testing APIs marked with mobile_course_access with verify_enrolled=True.
"""
def test_unenrolled_user(self):
self.login()
self.unenroll()

View File

@@ -31,16 +31,10 @@ class CourseField(serializers.RelatedField):
kwargs={'course_id': course_id},
request=request
)
course_about_url = reverse(
'course-about-detail',
kwargs={'course_id': course_id},
request=request
)
else:
video_outline_url = None
course_updates_url = None
course_handouts_url = None
course_about_url = None
return {
"id": course_id,
@@ -59,7 +53,6 @@ class CourseField(serializers.RelatedField):
"video_outline": video_outline_url,
"course_updates": course_updates_url,
"course_handouts": course_handouts_url,
"course_about": course_about_url,
"subscription_id": course.clean_id(padding_char='_'),
}

View File

@@ -10,7 +10,7 @@ from certificates.models import CertificateStatuses
from certificates.tests.factories import GeneratedCertificateFactory
from .. import errors
from ..testutils import MobileAPITestCase, MobileAuthTestMixin, MobileAuthUserTestMixin, MobileEnrolledCourseAccessTestMixin
from ..testutils import MobileAPITestCase, MobileAuthTestMixin, MobileAuthUserTestMixin, MobileCourseAccessTestMixin
from .serializers import CourseEnrollmentSerializer
@@ -43,7 +43,7 @@ class TestUserInfoApi(MobileAPITestCase, MobileAuthTestMixin):
self.assertTrue(self.username in response['location'])
class TestUserEnrollmentApi(MobileAPITestCase, MobileAuthUserTestMixin, MobileEnrolledCourseAccessTestMixin):
class TestUserEnrollmentApi(MobileAPITestCase, MobileAuthUserTestMixin, MobileCourseAccessTestMixin):
"""
Tests for /api/mobile/v0.5/users/<user_name>/course_enrollments/
"""
@@ -160,7 +160,7 @@ class CourseStatusAPITestCase(MobileAPITestCase):
)
class TestCourseStatusGET(CourseStatusAPITestCase, MobileAuthUserTestMixin, MobileEnrolledCourseAccessTestMixin):
class TestCourseStatusGET(CourseStatusAPITestCase, MobileAuthUserTestMixin, MobileCourseAccessTestMixin):
"""
Tests for GET of /api/mobile/v0.5/users/<user_name>/course_status_info/{course_id}
"""
@@ -178,7 +178,7 @@ class TestCourseStatusGET(CourseStatusAPITestCase, MobileAuthUserTestMixin, Mobi
)
class TestCourseStatusPATCH(CourseStatusAPITestCase, MobileAuthUserTestMixin, MobileEnrolledCourseAccessTestMixin):
class TestCourseStatusPATCH(CourseStatusAPITestCase, MobileAuthUserTestMixin, MobileCourseAccessTestMixin):
"""
Tests for PATCH of /api/mobile/v0.5/users/<user_name>/course_status_info/{course_id}
"""

View File

@@ -212,7 +212,6 @@ class UserCourseEnrollmentsList(generics.ListAPIView):
* url: URL to the downloadable version of the certificate, if exists.
* course: A collection of data about the course:
* course_about: The URI to get the data for the course About page.
* course_updates: The URI to get data for course updates.
* number: The course number.
* org: The organization that created the course.

View File

@@ -1,79 +1,18 @@
"""
Common utility methods and decorators for Mobile APIs.
"""
import functools
from django.http import Http404
from rest_framework import permissions, status, response
from opaque_keys.edx.keys import CourseKey
from courseware.courses import get_course_with_access
from openedx.core.lib.api.permissions import IsUserInUrl
from openedx.core.lib.api.authentication import (
SessionAuthenticationAllowInactiveUser,
OAuth2AuthenticationAllowInactiveUser,
)
from util.milestones_helpers import any_unfulfilled_milestones
from xmodule.modulestore.django import modulestore
from openedx.core.lib.api.view_utils import view_course_access, view_auth_classes
def mobile_course_access(depth=0, verify_enrolled=True):
def mobile_course_access(depth=0):
"""
Method decorator for a mobile API endpoint that verifies the user has access to the course in a mobile context.
"""
def _decorator(func):
"""Outer method decorator."""
@functools.wraps(func)
def _wrapper(self, request, *args, **kwargs):
"""
Expects kwargs to contain 'course_id'.
Passes the course descriptor to the given decorated function.
Raises 404 if access to course is disallowed.
"""
course_id = CourseKey.from_string(kwargs.pop('course_id'))
with modulestore().bulk_operations(course_id):
try:
course = get_course_with_access(
request.user,
'load_mobile' if verify_enrolled else 'load_mobile_no_enrollment_check',
course_id,
depth=depth
)
except Http404:
# any_unfulfilled_milestones called a second time since get_course_with_access returns a bool
if any_unfulfilled_milestones(course_id, request.user.id):
message = {
"developer_message": "Cannot access content with unfulfilled pre-requisites or unpassed entrance exam." # pylint: disable=line-too-long
}
return response.Response(
data=message,
status=status.HTTP_204_NO_CONTENT
)
else:
raise
return func(self, request, course=course, *args, **kwargs)
return _wrapper
return _decorator
return view_course_access(depth=depth, access_action='load_mobile', check_for_milestones=True)
def mobile_view(is_user=False):
"""
Function and class decorator that abstracts the authentication and permission checks for mobile api views.
"""
def _decorator(func_or_class):
"""
Requires either OAuth2 or Session-based authentication.
If is_user is True, also requires username in URL matches the request user.
"""
func_or_class.authentication_classes = (
OAuth2AuthenticationAllowInactiveUser,
SessionAuthenticationAllowInactiveUser
)
func_or_class.permission_classes = (permissions.IsAuthenticated,)
if is_user:
func_or_class.permission_classes += (IsUserInUrl,)
return func_or_class
return _decorator
return view_auth_classes(is_user)

View File

@@ -18,7 +18,7 @@ from xmodule.partitions.partitions import Group, UserPartition
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup
from ..testutils import MobileAPITestCase, MobileAuthTestMixin, MobileEnrolledCourseAccessTestMixin
from ..testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin
class TestVideoAPITestCase(MobileAPITestCase):
@@ -407,7 +407,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin):
@ddt.ddt
class TestVideoSummaryList(
TestVideoAPITestCase, MobileAuthTestMixin, MobileEnrolledCourseAccessTestMixin, TestVideoAPIMixin # pylint: disable=bad-continuation
TestVideoAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin, TestVideoAPIMixin # pylint: disable=bad-continuation
):
"""
Tests for /api/mobile/v0.5/video_outlines/courses/{course_id}..
@@ -863,7 +863,7 @@ class TestVideoSummaryList(
class TestTranscriptsDetail(
TestVideoAPITestCase, MobileAuthTestMixin, MobileEnrolledCourseAccessTestMixin, TestVideoAPIMixin # pylint: disable=bad-continuation
TestVideoAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin, TestVideoAPIMixin # pylint: disable=bad-continuation
):
"""
Tests for /api/mobile/v0.5/video_outlines/transcripts/{course_id}..

View File

@@ -1,6 +1,5 @@
from django.conf import settings
from rest_framework import permissions
from rest_framework.exceptions import PermissionDenied
from django.http import Http404

View File

@@ -1,12 +1,25 @@
"""
Utilities related to API views
"""
import functools
from django.core.exceptions import NON_FIELD_ERRORS, ValidationError
from django.http import Http404
from rest_framework import status, response
from rest_framework.exceptions import APIException
from rest_framework.response import Response
from lms.djangoapps.courseware.courses import get_course_with_access
from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.django import modulestore
from openedx.core.lib.api.authentication import (
SessionAuthenticationAllowInactiveUser,
OAuth2AuthenticationAllowInactiveUser,
)
from openedx.core.lib.api.permissions import IsUserInUrl, IsAuthenticatedOrDebug
from util.milestones_helpers import any_unfulfilled_milestones
class DeveloperErrorViewMixin(object):
"""
@@ -48,3 +61,60 @@ class DeveloperErrorViewMixin(object):
return self.make_validation_error_response(exc)
else:
raise
def view_course_access(depth=0, access_action='load', check_for_milestones=False):
"""
Method decorator for an API endpoint that verifies the user has access to the course.
"""
def _decorator(func):
"""Outer method decorator."""
@functools.wraps(func)
def _wrapper(self, request, *args, **kwargs):
"""
Expects kwargs to contain 'course_id'.
Passes the course descriptor to the given decorated function.
Raises 404 if access to course is disallowed.
"""
course_id = CourseKey.from_string(kwargs.pop('course_id'))
with modulestore().bulk_operations(course_id):
try:
course = get_course_with_access(
request.user,
access_action,
course_id,
depth=depth
)
except Http404:
# any_unfulfilled_milestones called a second time since has_access returns a bool
if check_for_milestones and any_unfulfilled_milestones(course_id, request.user.id):
message = {
"developer_message": "Cannot access content with unfulfilled "
"pre-requisites or unpassed entrance exam."
}
return response.Response(data=message, status=status.HTTP_204_NO_CONTENT)
else:
raise
return func(self, request, course=course, *args, **kwargs)
return _wrapper
return _decorator
def view_auth_classes(is_user=False):
"""
Function and class decorator that abstracts the authentication and permission checks for api views.
"""
def _decorator(func_or_class):
"""
Requires either OAuth2 or Session-based authentication.
If is_user is True, also requires username in URL matches the request user.
"""
func_or_class.authentication_classes = (
OAuth2AuthenticationAllowInactiveUser,
SessionAuthenticationAllowInactiveUser
)
func_or_class.permission_classes = (IsAuthenticatedOrDebug,)
if is_user:
func_or_class.permission_classes += (IsUserInUrl,)
return func_or_class
return _decorator