From 276ea2fe84da522d37a986cbd206b7764ff28871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Andr=C3=A9s=20Rocha?= Date: Tue, 16 Sep 2014 14:25:25 -0400 Subject: [PATCH 1/3] Fix tests not using ModuleStoreTestCase Some tests test were using `CourseFactory` buy not deriving from `ModuleStoreTestCase`, introducing a course leaks that broke other unrelated tests. --- lms/djangoapps/certificates/tests/tests.py | 3 ++- lms/djangoapps/instructor_analytics/tests/test_basic.py | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/certificates/tests/tests.py b/lms/djangoapps/certificates/tests/tests.py index 619d458912..52d73f42e9 100644 --- a/lms/djangoapps/certificates/tests/tests.py +++ b/lms/djangoapps/certificates/tests/tests.py @@ -5,12 +5,13 @@ Tests for the certificates models. from django.test import TestCase from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from student.tests.factories import UserFactory from certificates.models import CertificateStatuses, GeneratedCertificate, certificate_status_for_student -class CertificatesModelTest(TestCase): +class CertificatesModelTest(ModuleStoreTestCase): """ Tests for the GeneratedCertificate model """ diff --git a/lms/djangoapps/instructor_analytics/tests/test_basic.py b/lms/djangoapps/instructor_analytics/tests/test_basic.py index 591abc1d77..1274f1ef24 100644 --- a/lms/djangoapps/instructor_analytics/tests/test_basic.py +++ b/lms/djangoapps/instructor_analytics/tests/test_basic.py @@ -15,6 +15,7 @@ from instructor_analytics.basic import ( ) from courseware.tests.factories import InstructorFactory from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase class TestAnalyticsBasic(TestCase): @@ -52,7 +53,7 @@ class TestAnalyticsBasic(TestCase): self.assertEqual(set(AVAILABLE_FEATURES), set(STUDENT_FEATURES + PROFILE_FEATURES)) -class TestCourseSaleRecordsAnalyticsBasic(TestCase): +class TestCourseSaleRecordsAnalyticsBasic(ModuleStoreTestCase): """ Test basic course sale records analytics functions. """ def setUp(self): """ @@ -100,7 +101,7 @@ class TestCourseSaleRecordsAnalyticsBasic(TestCase): self.assertEqual(sale_record['total_codes'], 5) -class TestCourseRegistrationCodeAnalyticsBasic(TestCase): +class TestCourseRegistrationCodeAnalyticsBasic(ModuleStoreTestCase): """ Test basic course registration codes analytics functions. """ def setUp(self): """ From 72651e64ad5d8e7bacc06e37aa46762c1aa9f79a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Andr=C3=A9s=20Rocha?= Date: Tue, 16 Sep 2014 16:53:57 -0400 Subject: [PATCH 2/3] Modify notification_pref tests to avoid changes to module store Calling the client root was causing the module store to be modified, affecting test in other Django applications that rely on the default state of the module store. --- lms/djangoapps/notification_prefs/tests.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lms/djangoapps/notification_prefs/tests.py b/lms/djangoapps/notification_prefs/tests.py index d2d245c35a..2a12f70047 100644 --- a/lms/djangoapps/notification_prefs/tests.py +++ b/lms/djangoapps/notification_prefs/tests.py @@ -20,11 +20,6 @@ from util.testing import UrlResetMixin class NotificationPrefViewTest(UrlResetMixin, TestCase): INITIALIZATION_VECTOR = "\x00" * 16 - @classmethod - def setUpClass(cls): - # Make sure global state is set up appropriately - Client().get("/") - @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) def setUp(self): super(NotificationPrefViewTest, self).setUp() From 8857509b2dc2c87936c5993bf6543f48f2aa262e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Andr=C3=A9s=20Rocha?= Date: Mon, 8 Sep 2014 18:30:22 -0400 Subject: [PATCH 3/3] Add edX OAuth2 provider with OpenID Connect support --- lms/djangoapps/oauth2_handler/__init__.py | 1 + lms/djangoapps/oauth2_handler/handlers.py | 174 ++++++++++++++++++++++ lms/djangoapps/oauth2_handler/tests.py | 148 ++++++++++++++++++ lms/envs/aws.py | 5 +- lms/envs/common.py | 30 ++++ lms/envs/dev.py | 3 + lms/envs/test.py | 3 + lms/urls.py | 6 + requirements/edx/github.txt | 2 + 9 files changed, 371 insertions(+), 1 deletion(-) create mode 100644 lms/djangoapps/oauth2_handler/__init__.py create mode 100644 lms/djangoapps/oauth2_handler/handlers.py create mode 100644 lms/djangoapps/oauth2_handler/tests.py diff --git a/lms/djangoapps/oauth2_handler/__init__.py b/lms/djangoapps/oauth2_handler/__init__.py new file mode 100644 index 0000000000..fe4953f6ac --- /dev/null +++ b/lms/djangoapps/oauth2_handler/__init__.py @@ -0,0 +1 @@ +from oauth2_handler.handlers import IDTokenHandler, UserInfoHandler diff --git a/lms/djangoapps/oauth2_handler/handlers.py b/lms/djangoapps/oauth2_handler/handlers.py new file mode 100644 index 0000000000..32a38d29bc --- /dev/null +++ b/lms/djangoapps/oauth2_handler/handlers.py @@ -0,0 +1,174 @@ +""" Handlers for OpenID Connect provider. """ + +import branding +from courseware.access import has_access +from student.models import anonymous_id_for_user +from user_api.models import UserPreference +from lang_pref import LANGUAGE_KEY + + +class OpenIDHandler(object): + """ Basic OpenID Connect scope handler. """ + + def scope_openid(self, _data): + """ Only override the sub (subject) claim. """ + return ['sub'] + + def claim_sub(self, data): + """ + Return the value of the sub (subject) claim. The value should be + unique for each user. + + """ + + # Use the anonymous ID without any course as unique identifier. + # Note that this ID is derived using the value of the `SECRET_KEY` + # setting, this means that users will have different sub + # values for different deployments. + value = anonymous_id_for_user(data['user'], None) + return value + + +class ProfileHandler(object): + """ Basic OpenID Connect `profile` scope handler with `locale` claim. """ + + def scope_profile(self, _data): + """ Add the locale claim. """ + return ['locale'] + + def claim_locale(self, data): + """ + Return the locale for the users based on their preferences. + Does not return a value if the users have not set their locale preferences. + + """ + + language = UserPreference.get_preference(data['user'], LANGUAGE_KEY) + return language + + +class CourseAccessHandler(object): + """ + Defines two new scopes: `course_instructor` and `course_staff`. Each one is + valid only if the user is instructor or staff of at least one course. + + Each new scope has a corresponding claim: `instructor_courses` and + `staff_courses` that lists the course_ids for which the user as instructor + or staff privileges. + + The claims support claim request values. In other words, if no claim is + requested it returns all the courses for the corresponding privileges. If a + claim request is used, then it only returns the from the list of requested + values that have the corresponding privileges. + + For example, if the user is staff of course_a and course_b but not + course_c, the request: + + scope = openid course_staff + + will return: + + {staff_courses: [course_a, course_b] } + + If the request is: + + claims = {userinfo: {staff_courses=[course_b, course_d]}} + + the result will be: + + {staff_courses: [course_b] }. + + This is useful to quickly determine if a user has the right + privileges for a given course. + + For a description of the function naming and arguments, see: + + `oauth2_provider/oidc/handlers.py` + + """ + + def scope_course_instructor(self, data): + """ + Scope `course_instructor` valid only if the user is an instructor + of at least one course. + + """ + + course_ids = self._courses_with_access_type(data, 'instructor') + return ['instructor_courses'] if course_ids else None + + def scope_course_staff(self, data): + """ + Scope `course_staff` valid only if the user is an instructor of at + least one course. + + """ + + course_ids = self._courses_with_access_type(data, 'staff') + return ['staff_courses'] if course_ids else None + + def claim_instructor_courses(self, data): + """ + Claim `instructor_courses` with list of course_ids for which the + user has instructor privileges. + + """ + return self._courses_with_access_type(data, 'instructor') + + def claim_staff_courses(self, data): + """ + Claim `staff_courses` with list of course_ids for which the user + has staff privileges. + + """ + return self._courses_with_access_type(data, 'staff') + + def _courses_with_access_type(self, data, access_type): + """ + Utility function to list all courses for a user according to the + access type. + + The field `data` follows the handler specification in: + + `oauth2_provider/oidc/handlers.py` + + """ + + user = data['user'] + values = set(data.get('values', [])) + + courses = branding.get_visible_courses() + courses = (c for c in courses if has_access(user, access_type, c)) + course_ids = (unicode(c.id) for c in courses) + + # If values was provided, return only the requested authorized courses + if values: + return [c for c in course_ids if c in values] + else: + return [c for c in course_ids] + + +class IDTokenHandler(OpenIDHandler, ProfileHandler, CourseAccessHandler): + """ + Configure the ID Token handler for the LMS. + + Note that the values of the claims `instructor_courses` and + `staff_courses` are not included in the ID Token. The rationale is + that for global staff, the list of courses returned could be very + large. Instead they could check for specific courses using the + UserInfo endpoint. + + """ + + def claim_instructor_courses(self, data): + # Don't return list of courses in ID Tokens + return None + + def claim_staff_courses(self, data): + # Don't return list of courses in ID Tokens + return None + + +class UserInfoHandler(OpenIDHandler, ProfileHandler, CourseAccessHandler): + """ Configure the UserInfo handler for the LMS. """ + pass diff --git a/lms/djangoapps/oauth2_handler/tests.py b/lms/djangoapps/oauth2_handler/tests.py new file mode 100644 index 0000000000..9b19357dcb --- /dev/null +++ b/lms/djangoapps/oauth2_handler/tests.py @@ -0,0 +1,148 @@ +# pylint: disable=missing-docstring +from django.test.utils import override_settings +from django.test import TestCase + +from courseware.tests.tests import TEST_DATA_MIXED_MODULESTORE +from lang_pref import LANGUAGE_KEY +from opaque_keys.edx.locations import SlashSeparatedCourseKey +from student.models import anonymous_id_for_user +from student.roles import CourseStaffRole, CourseInstructorRole +from student.tests.factories import UserFactory, UserProfileFactory +from user_api.models import UserPreference + +# Will also run default tests for IDTokens and UserInfo +from oauth2_provider.tests import IDTokenTestCase, UserInfoTestCase + + +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class BaseTestMixin(TestCase): + profile = None + + def setUp(self): + super(BaseTestMixin, self).setUp() + + self.course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') + self.course_id = unicode(self.course_key) + + self.user_factory = UserFactory + self.set_user(self.make_user()) + + def set_user(self, user): + super(BaseTestMixin, self).set_user(user) + self.profile = UserProfileFactory(user=self.user) + + +class IDTokenTest(BaseTestMixin, IDTokenTestCase): + def test_sub_claim(self): + scopes, claims = self.get_new_id_token_values('openid') + self.assertIn('openid', scopes) + + sub = claims['sub'] + + expected_sub = anonymous_id_for_user(self.user, None) + self.assertEqual(sub, expected_sub) + + def test_user_without_locale_claim(self): + scopes, claims = self.get_new_id_token_values('openid profile') + self.assertIn('profile', scopes) + self.assertNotIn('locale', claims) + + def test_user_wit_locale_claim(self): + language = 'en' + UserPreference.set_preference(self.user, LANGUAGE_KEY, language) + scopes, claims = self.get_new_id_token_values('openid profile') + + self.assertIn('profile', scopes) + + locale = claims['locale'] + self.assertEqual(language, locale) + + def test_no_special_course_access(self): + scopes, claims = self.get_new_id_token_values('openid course_instructor course_staff') + + self.assertNotIn('course_staff', scopes) + self.assertNotIn('staff_courses', claims) + + self.assertNotIn('course_instructor', scopes) + self.assertNotIn('instructor_courses', claims) + + def test_course_staff_courses(self): + CourseStaffRole(self.course_key).add_users(self.user) + + scopes, claims = self.get_new_id_token_values('openid course_staff') + + self.assertIn('course_staff', scopes) + self.assertNotIn('staff_courses', claims) # should not return courses in id_token + + def test_course_instructor_courses(self): + CourseInstructorRole(self.course_key).add_users(self.user) + + scopes, claims = self.get_new_id_token_values('openid course_instructor') + + self.assertIn('course_instructor', scopes) + self.assertNotIn('instructor_courses', claims) # should not return courses in id_token + + +class UserInfoTest(BaseTestMixin, UserInfoTestCase): + def token_for_scope(self, scope): + full_scope = 'openid %s' % scope + self.set_access_token_scope(full_scope) + + token = self.access_token.token # pylint: disable=no-member + return full_scope, token + + def get_with_scope(self, scope): + scope, token = self.token_for_scope(scope) + result, claims = self.get_userinfo(token, scope) + self.assertEqual(result.status_code, 200) + + return claims + + def get_with_claim_value(self, scope, claim, values): + _full_scope, token = self.token_for_scope(scope) + + result, claims = self.get_userinfo( + token, + claims={claim: {'values': values}} + ) + + self.assertEqual(result.status_code, 200) + return claims + + def test_request_staff_courses_using_scope(self): + CourseStaffRole(self.course_key).add_users(self.user) + claims = self.get_with_scope('course_staff') + + courses = claims['staff_courses'] + self.assertIn(self.course_id, courses) + self.assertEqual(len(courses), 1) + + def test_request_instructor_courses_using_scope(self): + CourseInstructorRole(self.course_key).add_users(self.user) + claims = self.get_with_scope('course_instructor') + + courses = claims['instructor_courses'] + self.assertIn(self.course_id, courses) + self.assertEqual(len(courses), 1) + + def test_request_staff_courses_with_claims(self): + CourseStaffRole(self.course_key).add_users(self.user) + + values = [self.course_id, 'some_invalid_course'] + claims = self.get_with_claim_value('course_staff', 'staff_courses', values) + self.assertEqual(len(claims), 2) + + courses = claims['staff_courses'] + self.assertIn(self.course_id, courses) + self.assertEqual(len(courses), 1) + + def test_request_instructor_courses_with_claims(self): + CourseInstructorRole(self.course_key).add_users(self.user) + + values = ['edX/toy/TT_2012_Fall', self.course_id, 'invalid_course_id'] + claims = self.get_with_claim_value('course_instructor', 'instructor_courses', values) + self.assertEqual(len(claims), 2) + + courses = claims['instructor_courses'] + self.assertIn(self.course_id, courses) + self.assertEqual(len(courses), 1) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index a252ad019e..c430974d33 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -443,10 +443,13 @@ TIME_ZONE_DISPLAYED_FOR_DEADLINES = ENV_TOKENS.get("TIME_ZONE_DISPLAYED_FOR_DEAD ##### X-Frame-Options response header settings ##### X_FRAME_OPTIONS = ENV_TOKENS.get('X_FRAME_OPTIONS', X_FRAME_OPTIONS) - ##### Third-party auth options ################################################ THIRD_PARTY_AUTH = AUTH_TOKENS.get('THIRD_PARTY_AUTH', THIRD_PARTY_AUTH) +##### OAUTH2 Provider ############## +if FEATURES.get('ENABLE_OAUTH2_PROVIDER'): + OAUTH_OIDC_ISSUER = ENV_TOKENS['OAUTH_OIDC_ISSUER'] + ##### ADVANCED_SECURITY_CONFIG ##### ADVANCED_SECURITY_CONFIG = ENV_TOKENS.get('ADVANCED_SECURITY_CONFIG', {}) diff --git a/lms/envs/common.py b/lms/envs/common.py index 46feaa19f6..0b210652b3 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -123,6 +123,9 @@ FEATURES = { # with Shib. Feature was requested by Stanford's office of general counsel 'SHIB_DISABLE_TOS': False, + # Toggles OAuth2 authentication provider + 'ENABLE_OAUTH2_PROVIDER': False, + # Can be turned off if course lists need to be hidden. Effects views and templates. 'COURSES_ARE_BROWSABLE': True, @@ -335,6 +338,28 @@ STATUS_MESSAGE_PATH = ENV_ROOT / "status_message.json" ############################ OpenID Provider ################################## OPENID_PROVIDER_TRUSTED_ROOTS = ['cs50.net', '*.cs50.net'] +############################ OAUTH2 Provider ################################### + +# OpenID Connect issuer ID. Normally the URL of the authentication endpoint. + +OAUTH_OIDC_ISSUER = 'https:/example.com/oauth2' + +# OpenID Connect claim handlers + +OAUTH_OIDC_ID_TOKEN_HANDLERS = ( + 'oauth2_provider.oidc.handlers.BasicIDTokenHandler', + 'oauth2_provider.oidc.handlers.ProfileHandler', + 'oauth2_provider.oidc.handlers.EmailHandler', + 'oauth2_handler.IDTokenHandler' +) + +OAUTH_OIDC_USERINFO_HANDLERS = ( + 'oauth2_provider.oidc.handlers.BasicUserInfoHandler', + 'oauth2_provider.oidc.handlers.ProfileHandler', + 'oauth2_provider.oidc.handlers.EmailHandler', + 'oauth2_handler.UserInfoHandler' +) + ################################## EDX WEB ##################################### # This is where we stick our compiled template files. Most of the app uses Mako # templates @@ -1311,6 +1336,11 @@ INSTALLED_APPS = ( 'external_auth', 'django_openid_auth', + # OAuth2 Provider + 'provider', + 'provider.oauth2', + 'oauth2_provider', + # For the wiki 'wiki', # The new django-wiki from benjaoming 'django_notify', diff --git a/lms/envs/dev.py b/lms/envs/dev.py index d57ab3bd5b..7294d05098 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -204,6 +204,9 @@ OPENID_USE_AS_ADMIN_LOGIN = False OPENID_PROVIDER_TRUSTED_ROOTS = ['*'] +############################## OAUTH2 Provider ################################ +FEATURES['ENABLE_OAUTH2_PROVIDER'] = True + ######################## MIT Certificates SSL Auth ############################ FEATURES['AUTH_USE_CERTIFICATES'] = False diff --git a/lms/envs/test.py b/lms/envs/test.py index cb493ecb66..a0e52a1d0b 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -217,6 +217,9 @@ OPENID_UPDATE_DETAILS_FROM_SREG = True OPENID_USE_AS_ADMIN_LOGIN = False OPENID_PROVIDER_TRUSTED_ROOTS = ['*'] +############################## OAUTH2 Provider ################################ +FEATURES['ENABLE_OAUTH2_PROVIDER'] = True + ###################### Payment ##############################3 # Enable fake payment processing page FEATURES['ENABLE_PAYMENT_FAKE'] = True diff --git a/lms/urls.py b/lms/urls.py index abb9896b96..f8009960d0 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -459,6 +459,12 @@ if settings.FEATURES.get('AUTH_USE_OPENID_PROVIDER'): url(r'^openid/provider/xrds/$', 'external_auth.views.provider_xrds', name='openid-provider-xrds') ) +if settings.FEATURES.get('ENABLE_OAUTH2_PROVIDER'): + urlpatterns += ( + url(r'^oauth2/', include('oauth2_provider.urls', namespace='oauth2')), + ) + + if settings.FEATURES.get('ENABLE_LMS_MIGRATION'): urlpatterns += ( url(r'^migrate/modules$', 'lms_migration.migrate.manage_modulestores'), diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 28f4da6232..8f7f2e472d 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -10,6 +10,7 @@ -e git+https://github.com/edx/django-staticfiles.git@d89aae2a82f2b#egg=django-staticfiles -e git+https://github.com/edx/django-pipeline.git@88ec8a011e481918fdc9d2682d4017c835acd8be#egg=django-pipeline -e git+https://github.com/edx/django-wiki.git@cd0b2b31997afccde519fe5b3365e61a9edb143f#egg=django-wiki +-e git+https://github.com/edx/django-oauth2-provider.git@0.2.7-fork-edx-1#egg=django-oauth2-provider -e git+https://github.com/gabrielfalcao/lettuce.git@cccc3978ad2df82a78b6f9648fe2e9baddd22f88#egg=lettuce -e git+https://github.com/dementrock/pystache_custom.git@776973740bdaad83a3b029f96e415a7d1e8bec2f#egg=pystache_custom-dev -e git+https://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk @@ -31,3 +32,4 @@ -e git+https://github.com/edx/opaque-keys.git@d45d0bd8d64c69531be69178b9505b5d38806ce0#egg=opaque-keys -e git+https://github.com/edx/ease.git@97de68448e5495385ba043d3091f570a699d5b5f#egg=ease -e git+https://github.com/edx/i18n-tools.git@56f048af9b6868613c14aeae760548834c495011#egg=i18n-tools +-e git+https://github.com/edx/edx-oauth2-provider.git@0.2.1#egg=oauth2-provider