Merge pull request #31 from edx/clintonb/api-authentication

Requiring authentication for all API calls

course_discovery/apps/api/v1/tests/test_views.py

@@ -5,7 +5,9 @@ from time import time
 
 import ddt
 import jwt
+import responses
 from django.conf import settings
+from django.test import override_settings
 from rest_framework.reverse import reverse
 from rest_framework.test import APITestCase, APIRequestFactory
 
@@ -16,6 +18,30 @@ from course_discovery.apps.core.tests.factories import UserFactory, USER_PASSWOR
 from course_discovery.apps.core.tests.mixins import ElasticsearchTestMixin
 from course_discovery.apps.courses.tests.factories import CourseFactory
 
+OAUTH2_ACCESS_TOKEN_URL = 'http://example.com/oauth2/access_token/'
+
+
+class OAuth2Mixin(object):
+    def get_access_token(self, user):
+        """ Generates an OAuth2 access token for the user. """
+        return user.username
+
+    def generate_oauth2_token_header(self, user):
+        """ Generates a Bearer authorization header to simulate OAuth2 authentication. """
+        return 'Bearer {token}'.format(token=self.get_access_token(user))
+
+    def mock_access_token_response(self, user, status=200):
+        """ Mock the access token endpoint response of the OAuth2 provider. """
+        url = '{root}/{token}'.format(root=OAUTH2_ACCESS_TOKEN_URL.rstrip('/'), token=self.get_access_token(user))
+
+        responses.add(
+            responses.GET,
+            url,
+            body=json.dumps({'username': user.username, 'scope': 'read', 'expires_in': 60}),
+            content_type="application/json",
+            status=status
+        )
+
 
 class SerializationMixin(object):
     def _get_request(self, format=None):
@@ -35,7 +61,7 @@ class SerializationMixin(object):
 
 
 @ddt.ddt
-class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, APITestCase):
+class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, OAuth2Mixin, APITestCase):
     """ Tests for the catalog resource.
 
     Read-only (GET) endpoints should NOT require authentication.
@@ -122,6 +148,13 @@ class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, APITestCas
         self.client.logout()
         self.assert_catalog_created(HTTP_AUTHORIZATION=self.generate_jwt_token_header(self.user))
 
+    @responses.activate
+    @override_settings(OAUTH2_ACCESS_TOKEN_URL=OAUTH2_ACCESS_TOKEN_URL)
+    def test_create_with_oauth2_authentication(self):
+        self.client.logout()
+        self.mock_access_token_response(self.user)
+        self.assert_catalog_created(HTTP_AUTHORIZATION=self.generate_oauth2_token_header(self.user))
+
     def test_courses(self):
         """ Verify the endpoint returns the list of courses contained in the catalog. """
         url = reverse('api:v1:catalog-courses', kwargs={'id': self.catalog.id})
@@ -184,7 +217,7 @@ class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, APITestCas
         self.assertEqual(catalog.query, query)
 
     def test_partial_update(self):
-        """ Verify the endpoint supports partially updating a catlaog's fields. """
+        """ Verify the endpoint supports partially updating a catalog's fields. """
         url = reverse('api:v1:catalog-detail', kwargs={'id': self.catalog.id})
         name = 'Updated Catalog'
         query = self.catalog.query
@@ -201,7 +234,7 @@ class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, APITestCas
 
 
 @ddt.ddt
-class CourseViewSetTests(ElasticsearchTestMixin, SerializationMixin, APITestCase):
+class CourseViewSetTests(ElasticsearchTestMixin, SerializationMixin, OAuth2Mixin, APITestCase):
     def setUp(self):
         super(CourseViewSetTests, self).setUp()
         self.user = UserFactory(is_staff=True, is_superuser=True)
@@ -256,8 +289,19 @@ class CourseViewSetTests(ElasticsearchTestMixin, SerializationMixin, APITestCase
 
     def test_retrieve(self):
         """ Verify the endpoint returns a single course. """
+        self.assert_retrieve_success()
+
+    def assert_retrieve_success(self, **headers):
+        """ Asserts the endpoint returns details for a single course. """
         course = CourseFactory()
         url = reverse('api:v1:course-detail', kwargs={'id': course.id})
-        response = self.client.get(url)
+        response = self.client.get(url, format='json', **headers)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, self.serialize_course(course))
+
+    @responses.activate
+    @override_settings(OAUTH2_ACCESS_TOKEN_URL=OAUTH2_ACCESS_TOKEN_URL)
+    def test_retrieve_with_oauth2_authentication(self):
+        self.client.logout()
+        self.mock_access_token_response(self.user)
+        self.assert_retrieve_success(HTTP_AUTHORIZATION=self.generate_oauth2_token_header(self.user))

course_discovery/apps/api/v1/views.py

@@ -2,11 +2,9 @@ import json
 import logging
 
 from rest_framework import viewsets
-from rest_framework.authentication import SessionAuthentication
 from rest_framework.decorators import detail_route
-from rest_framework.permissions import DjangoModelPermissionsOrAnonReadOnly, IsAuthenticatedOrReadOnly
+from rest_framework.permissions import IsAuthenticated
 from rest_framework.response import Response
-from rest_framework_jwt.authentication import JSONWebTokenAuthentication
 
 from course_discovery.apps.api.pagination import ElasticsearchLimitOffsetPagination
 from course_discovery.apps.api.serializers import CatalogSerializer, CourseSerializer, ContainedCoursesSerializer
@@ -21,8 +19,6 @@ logger = logging.getLogger(__name__)
 class CatalogViewSet(viewsets.ModelViewSet):
     """ Catalog resource. """
 
-    authentication_classes = (SessionAuthentication, JSONWebTokenAuthentication,)
-    permission_classes = (DjangoModelPermissionsOrAnonReadOnly,)
     lookup_field = 'id'
     queryset = Catalog.objects.all()
     serializer_class = CatalogSerializer
@@ -95,10 +91,9 @@ class CatalogViewSet(viewsets.ModelViewSet):
 
 class CourseViewSet(viewsets.ReadOnlyModelViewSet):
     """ Course resource. """
-    authentication_classes = (SessionAuthentication, JSONWebTokenAuthentication,)
     lookup_field = 'id'
     lookup_value_regex = COURSE_ID_REGEX
-    permission_classes = (IsAuthenticatedOrReadOnly,)
+    permission_classes = (IsAuthenticated,)
     serializer_class = CourseSerializer
     pagination_class = ElasticsearchLimitOffsetPagination
 

course_discovery/apps/courses/tests/test_models.py

@@ -28,7 +28,7 @@ class CourseTests(ElasticsearchTestMixin, TestCase):
         for attr, value in attrs.items():
             self.assertEqual(getattr(course, attr), value)
 
-    @responses.activate  # pylint: disable=no-member
+    @responses.activate
     def mock_refresh_all(self):
         """
         Mock the external APIs and refresh all course data.
@@ -108,7 +108,6 @@ class CourseTests(ElasticsearchTestMixin, TestCase):
 
             return request_callback
 
-        # pylint: disable=no-member
         url = '{host}/courses/'.format(host=ECOMMERCE_API_URL)
         responses.add_callback(responses.GET, url, callback=ecommerce_api_callback(url, course_bodies),
                                content_type=JSON)
@@ -238,7 +237,7 @@ class CourseTests(ElasticsearchTestMixin, TestCase):
         }
         self.assertEqual(Course.search(query), expected)
 
-    @responses.activate  # pylint: disable=no-member
+    @responses.activate
     def test_refresh(self):
         """ Verify the method refreshes data for a single course. """
         course_id = 'SesameStreetX/Cookies/1T2016'
@@ -250,7 +249,7 @@ class CourseTests(ElasticsearchTestMixin, TestCase):
 
         # Mock the call to the E-Commerce API
         url = '{host}/courses/{course_id}/'.format(host=ECOMMERCE_API_URL, course_id=course_id)
-        responses.add(responses.GET, url, body=json.dumps(body), content_type=JSON)  # pylint: disable=no-member
+        responses.add(responses.GET, url, body=json.dumps(body), content_type=JSON)
 
         # Refresh the course, and ensure the attributes are correct.
         course = Course.refresh(course_id, ACCESS_TOKEN)

course_discovery/apps/courses/tests/test_refresh_all_courses.py

@@ -4,7 +4,6 @@ from django.core.management import call_command
 from django.test import TestCase
 from django.test.utils import override_settings
 from edx_rest_api_client.client import EdxRestApiClient
-import httpretty
 from mock import patch
 
 from course_discovery.apps.courses.models import Course
@@ -27,7 +26,6 @@ class RefreshAllCoursesCommandTests(TestCase):
             call_command(self.cmd, access_token=access_token)
             mock_refresh.assert_called_once_with(access_token=access_token)
 
-    @httpretty.activate
     def test_call_with_client_credentials(self):
         """ Verify the management command calls Course.refresh_all() with client credentials. """
         access_token = 'secret'
@@ -38,7 +36,6 @@ class RefreshAllCoursesCommandTests(TestCase):
                 call_command(self.cmd)
                 mock_refresh.assert_called_once_with(access_token=access_token)
 
-    @httpretty.activate
     def test_call_with_client_credentials_error(self):
         """ Verify the command requires an access token to complete. """
         with patch.object(EdxRestApiClient, 'get_oauth_access_token') as mock_access_token:

course_discovery/settings/base.py

@@ -171,6 +171,7 @@ SOCIAL_AUTH_EDX_OIDC_KEY = 'replace-me'
 SOCIAL_AUTH_EDX_OIDC_SECRET = 'replace-me'
 SOCIAL_AUTH_EDX_OIDC_URL_ROOT = 'replace-me'
 SOCIAL_AUTH_EDX_OIDC_ID_TOKEN_DECRYPTION_KEY = SOCIAL_AUTH_EDX_OIDC_SECRET
+OAUTH2_ACCESS_TOKEN_URL = 'replace-me'
 
 # Request the user's permissions in the ID token
 EXTRA_SCOPE = ['permissions']
@@ -233,7 +234,15 @@ LOGGING = {
 
 
 REST_FRAMEWORK = {
+    'DEFAULT_AUTHENTICATION_CLASSES': (
+        'rest_framework.authentication.SessionAuthentication',
+        'edx_rest_framework_extensions.authentication.BearerAuthentication',
+        'rest_framework_jwt.authentication.JSONWebTokenAuthentication',
+    ),
     'DEFAULT_PAGINATION_CLASS': 'rest_framework.pagination.LimitOffsetPagination',
+    'DEFAULT_PERMISSION_CLASSES': (
+        'rest_framework.permissions.DjangoModelPermissions',
+    ),
     'PAGE_SIZE': 20,
     'VIEW_DESCRIPTION_FUNCTION': 'rest_framework_swagger.views.get_restructuredtext',
     'TEST_REQUEST_RENDERER_CLASSES': (

course_discovery/settings/local.py

@@ -52,6 +52,7 @@ SOCIAL_AUTH_EDX_OIDC_KEY = 'replace-me'
 SOCIAL_AUTH_EDX_OIDC_SECRET = 'replace-me'
 SOCIAL_AUTH_EDX_OIDC_URL_ROOT = 'replace-me'
 SOCIAL_AUTH_EDX_OIDC_ID_TOKEN_DECRYPTION_KEY = SOCIAL_AUTH_EDX_OIDC_SECRET
+OAUTH2_ACCESS_TOKEN_URL = 'replace-me'
 
 ENABLE_AUTO_AUTH = True
 

docs/testing.rst

@@ -12,3 +12,14 @@ Code quality validation can be run independently with:
 .. code-block:: bash
 
     $ make quality
+
+httpretty
+---------
+
+edX uses `httpretty <http://httpretty.readthedocs.org/en/latest/>`_ a lot to mock HTTP endpoints; however,
+`a bug in httpretty <https://github.com/gabrielfalcao/HTTPretty/issues/65>`_ (that is closed, but still a problem)
+prevents us from using it in this repository. Were you to use `httpretty`, you would find that, although you might
+mock an OAuth2 endpoint, `httpretty` blocks requests to Elasticsearch, leading to test failures.
+
+Given our extensive use of Elasticsearch, and need to mock HTTP endpoints, we use the
+`responses <https://github.com/getsentry/responses>`_ library. It's API is practically the same as that of `httpretty.

pylintrc

@@ -2,17 +2,48 @@
 # ** DO NOT EDIT THIS FILE **
 # ***************************
 #
-# It is generated by:
-#   $ edx_lint write pylintrc
+# This file was generated by edx-lint: http://github.com/edx.edx-lint
 #
+# If you want to change this file, you have two choices, depending on whether
+# you want to make a local change that applies only to this repo, or whether
+# you want to make a central change that applies to all repos using edx-lint.
 #
+# LOCAL CHANGE:
 #
+# 1. Edit the local pylintrc_tweaks file to add changes just to this
+#    repo's file.
 #
+# 2. Run:
 #
+#       $ edx_lint write pylintrc
 #
+# 3. This will modify the local file.  Submit a pull request to get it
+#    checked in so that others will benefit.
 #
 #
-# STAY AWAY!
+# CENTRAL CHANGE:
+#
+# 1. Edit the pylintrc file in the edx-lint repo at
+#    https://github.com/edx/edx-lint/blob/master/edx_lint/files/pylintrc
+#
+# 2. Make a new version of edx_lint, which involves the usual steps of
+#    incrementing the version number, submitting and reviewing a pull
+#    request, and updating the edx-lint version reference in this repo.
+#
+# 3. Install the newer version of edx-lint.
+#
+# 4. Run:
+#
+#       $ edx_lint write pylintrc
+#
+# 5. This will modify the local file.  Submit a pull request to get it
+#    checked in so that others will benefit.
+#
+#
+#
+#
+#
+# STAY AWAY FROM THIS FILE!
 #
 #
 #
@@ -24,7 +55,7 @@
 [MASTER]
 ignore = ,migrations, settings, wsgi.py
 persistent = yes
-load-plugins = edx_lint.pylint,pylint_django
+load-plugins = edx_lint.pylint,pylint_django,pylint_celery
 
 [MESSAGES CONTROL]
 disable = 
@@ -37,6 +68,7 @@ disable =
 	abstract-class-little-used,
 	no-init,
 	fixme,
+	logging-format-interpolation,
 	too-many-lines,
 	no-self-use,
 	too-many-ancestors,
@@ -61,7 +93,7 @@ bad-functions = map,filter,apply,input
 module-rgx = (([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$
 const-rgx = (([A-Z_][A-Z0-9_]*)|(__.*__)|log|urlpatterns|logger|User)$
 class-rgx = [A-Z_][a-zA-Z0-9]+$
-function-rgx = ([a-z_][a-z0-9_]{2,30}|test_[a-z0-9_]+)$
+function-rgx = ([a-z_][a-z0-9_]{2,40}|test_[a-z0-9_]+)$
 method-rgx = ([a-z_][a-z0-9_]{2,40}|setUp|set[Uu]pClass|tearDown|tear[Dd]ownClass|assert[A-Z]\w*|maxDiff|test_[a-z0-9_]+)$
 attr-rgx = [a-z_][a-z0-9_]{2,30}$
 argument-rgx = [a-z_][a-z0-9_]{2,30}$
@@ -92,7 +124,7 @@ ignore-imports = no
 
 [TYPECHECK]
 ignore-mixin-members = yes
-ignored-classes = SQLObject,WSGIRequest,UserFactory,CatalogFactory
+ignored-classes = SQLObject,WSGIRequest,UserFactory,CatalogFactory,responses
 unsafe-load-any-extension = yes
 generated-members = 
 	REQUEST,
@@ -148,4 +180,4 @@ int-import-graph =
 [EXCEPTIONS]
 overgeneral-exceptions = Exception
 
-# d9dbd5cb8a05067710b776137902855c9ca7f6a6
+# aa393e07e100b773853da38867c6a050ff9d6bfb

pylintrc_tweaks

@@ -8,4 +8,4 @@ const-rgx = (([A-Z_][A-Z0-9_]*)|(__.*__)|log|urlpatterns|logger|User)$
 DISABLE+= ,invalid-name,missing-docstring
 
 [TYPECHECK]
-ignored-classes+= ,WSGIRequest,UserFactory,CatalogFactory
+ignored-classes+= ,WSGIRequest,UserFactory,CatalogFactory,responses

requirements/base.txt

-django == 1.8.7
-django-extensions == 1.5.9
-django-waffle == 0.11
-djangorestframework == 3.3.1
+django==1.8.7
+django-extensions==1.5.9
+django-waffle==0.11
+djangorestframework==3.3.1
 djangorestframework-jwt==1.7.2
 django-rest-swagger[reST]==0.3.4
-edx-auth-backends == 0.1.3
+edx-auth-backends==0.1.3
+git+https://github.com/edx/edx-drf-extensions.git@clintonb/authentication#egg=edx-drf-extensions==0.1.0
 edx-rest-api-client==1.5.0
 elasticsearch>=1.0.0,<2.0.0
-pytz == 2015.7
+pytz==2015.7

requirements/docs.txt

-Sphinx == 1.3.1
-sphinx_rtd_theme == 0.1.9
+Sphinx==1.3.1
+sphinx_rtd_theme==0.1.9

requirements/local.txt

@@ -2,10 +2,10 @@
 -r test.txt
 -r docs.txt
 
-django-debug-toolbar == 1.4
+django-debug-toolbar==1.4
 
 # i18n
-transifex-client == 0.11
+transifex-client==0.11
 git+https://github.com/edx/i18n-tools.git@v0.1.4#egg=i18n_tools==0.1.4
 
 # docker devstack

requirements/optional.txt

-newrelic == 2.58.1.44
+newrelic==2.58.1.44

requirements/test.txt

 # Packages required for testing
 -r base.txt
 
-coverage == 4.0.2
+coverage==4.0.2
 ddt==1.0.1
-django-dynamic-fixture == 1.8.5
-django-nose == 1.4.2
-edx-lint == 0.4.0
+django-dynamic-fixture==1.8.5
+django-nose==1.4.2
+edx-lint==0.5.0
 factory-boy==2.6.0
-httpretty==0.8.14
-mock == 1.3.0
-nose-ignore-docstring == 0.2
-pep8 == 1.6.2
+mock==1.3.0
+nose-ignore-docstring==0.2
+pep8==1.6.2
 responses==0.5.0
 testfixtures==4.7.0