diff --git a/.gitignore b/.gitignore index f50ad217f3..fa417911e5 100644 --- a/.gitignore +++ b/.gitignore @@ -131,3 +131,6 @@ lms/lib/comment_client/python autodeploy.properties .ws_migrations_complete dist + +# Visual Studio Code +.vscode diff --git a/common/djangoapps/third_party_auth/middleware.py b/common/djangoapps/third_party_auth/middleware.py index 1e5069e8fb..81e4d3de78 100644 --- a/common/djangoapps/third_party_auth/middleware.py +++ b/common/djangoapps/third_party_auth/middleware.py @@ -23,32 +23,3 @@ class ExceptionMiddleware(SocialAuthExceptionMiddleware): redirect_uri = pipeline.AUTH_DISPATCH_URLS[auth_entry] return redirect_uri - - -class PipelineQuarantineMiddleware(object): - """ - Middleware flushes the session if a user agent with a quarantined session - attempts to leave the quarantined set of views. - """ - - def process_view(self, request, view_func, view_args, view_kwargs): # pylint: disable=unused-argument - """ - Check the session to see if we've quarantined the user to a particular - step of the authentication pipeline; if so, look up which modules the - user is allowed to browse to without breaking the pipeline. If the view - that's been requested is outside those modules, then flush the session. - - In general, this middleware should be used in cases where allowing the - user to exit the running pipeline would be undesirable, and where it'd - be better to flush the session state rather than allow it. Pipeline - quarantining is utilized by the Enterprise application to enforce - collection of user consent for sharing data with a linked third-party - authentication provider. - """ - if not pipeline.running(request): - return - - view_module = view_func.__module__ - quarantined_modules = request.session.get('third_party_auth_quarantined_modules') - if quarantined_modules is not None and not any(view_module.startswith(mod) for mod in quarantined_modules): - request.session.flush() diff --git a/common/djangoapps/third_party_auth/settings.py b/common/djangoapps/third_party_auth/settings.py index c768332270..917f398e7d 100644 --- a/common/djangoapps/third_party_auth/settings.py +++ b/common/djangoapps/third_party_auth/settings.py @@ -10,12 +10,9 @@ If true, it: b) calls apply_settings(), passing in the Django settings """ -from openedx.features.enterprise_support.api import insert_enterprise_pipeline_elements - _FIELDS_STORED_IN_SESSION = ['auth_entry', 'next'] _MIDDLEWARE_CLASSES = ( 'third_party_auth.middleware.ExceptionMiddleware', - 'third_party_auth.middleware.PipelineQuarantineMiddleware', ) _SOCIAL_AUTH_LOGIN_REDIRECT_URL = '/dashboard' @@ -58,9 +55,6 @@ def apply_settings(django_settings): 'third_party_auth.pipeline.login_analytics', ] - # Add enterprise pipeline elements if the enterprise app is installed - insert_enterprise_pipeline_elements(django_settings.SOCIAL_AUTH_PIPELINE) - # Required so that we can use unmodified PSA OAuth2 backends: django_settings.SOCIAL_AUTH_STRATEGY = 'third_party_auth.strategy.ConfigurationModelStrategy' diff --git a/common/djangoapps/third_party_auth/tests/test_middleware.py b/common/djangoapps/third_party_auth/tests/test_middleware.py deleted file mode 100644 index d2608fd415..0000000000 --- a/common/djangoapps/third_party_auth/tests/test_middleware.py +++ /dev/null @@ -1,60 +0,0 @@ -""" -Test the session-flushing middleware -""" -import unittest - -from django.conf import settings -from django.test import Client -from social_django.models import Partial - - -@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -class TestSessionFlushMiddleware(unittest.TestCase): - """ - Ensure that if the pipeline is exited when it's been quarantined, - the entire session is flushed. - """ - def setUp(self): - self.client = Client() - self.fancy_variable = 13025 - self.token = 'pipeline_running' - self.tpa_quarantined_modules = ('fake_quarantined_module',) - - def tearDown(self): - Partial.objects.all().delete() - - def test_session_flush(self): - """ - Test that a quarantined session is flushed when navigating elsewhere - """ - session = self.client.session - session['fancy_variable'] = self.fancy_variable - session['partial_pipeline_token'] = self.token - session['third_party_auth_quarantined_modules'] = self.tpa_quarantined_modules - session.save() - Partial.objects.create(token=session.get('partial_pipeline_token')) - self.client.get('/') - self.assertEqual(self.client.session.get('fancy_variable', None), None) - - def test_session_no_running_pipeline(self): - """ - Test that a quarantined session without a running pipeline is not flushed - """ - session = self.client.session - session['fancy_variable'] = self.fancy_variable - session['third_party_auth_quarantined_modules'] = self.tpa_quarantined_modules - session.save() - self.client.get('/') - self.assertEqual(self.client.session.get('fancy_variable', None), self.fancy_variable) - - def test_session_no_quarantine(self): - """ - Test that a session with a running pipeline but no quarantine is not flushed - """ - session = self.client.session - session['fancy_variable'] = self.fancy_variable - session['partial_pipeline_token'] = self.token - session.save() - Partial.objects.create(token=session.get('partial_pipeline_token')) - self.client.get('/') - self.assertEqual(self.client.session.get('fancy_variable', None), self.fancy_variable) diff --git a/common/djangoapps/third_party_auth/tests/test_settings.py b/common/djangoapps/third_party_auth/tests/test_settings.py index 9bddb7ecb7..bfb4655404 100644 --- a/common/djangoapps/third_party_auth/tests/test_settings.py +++ b/common/djangoapps/third_party_auth/tests/test_settings.py @@ -2,7 +2,6 @@ import unittest -from openedx.features.enterprise_support.api import enterprise_enabled from third_party_auth import provider, settings from third_party_auth.tests import testutil @@ -56,8 +55,3 @@ class SettingsUnitTest(testutil.TestCase): # bad in prod. settings.apply_settings(self.settings) self.assertFalse(self.settings.SOCIAL_AUTH_RAISE_EXCEPTIONS) - - @unittest.skipUnless(enterprise_enabled(), 'enterprise not enabled') - def test_enterprise_elements_inserted(self): - settings.apply_settings(self.settings) - self.assertIn('enterprise.tpa_pipeline.handle_enterprise_logistration', self.settings.SOCIAL_AUTH_PIPELINE) diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py index 00b4881808..31f74d61d9 100644 --- a/lms/djangoapps/courseware/tests/test_course_info.py +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -388,7 +388,7 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest self.assertEqual(resp.status_code, 200) def test_num_queries_instructor_paced(self): - self.fetch_course_info_with_queries(self.instructor_paced_course, 25, 3) + self.fetch_course_info_with_queries(self.instructor_paced_course, 24, 3) def test_num_queries_self_paced(self): - self.fetch_course_info_with_queries(self.self_paced_course, 25, 3) + self.fetch_course_info_with_queries(self.self_paced_course, 24, 3) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 4508797807..418a154886 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -211,8 +211,8 @@ class IndexQueryTestCase(ModuleStoreTestCase): NUM_PROBLEMS = 20 @ddt.data( - (ModuleStoreEnum.Type.mongo, 10, 143), - (ModuleStoreEnum.Type.split, 4, 143), + (ModuleStoreEnum.Type.mongo, 10, 142), + (ModuleStoreEnum.Type.split, 4, 142), ) @ddt.unpack def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count): @@ -1464,12 +1464,12 @@ class ProgressPageTests(ProgressPageBaseTests): """Test that query counts remain the same for self-paced and instructor-paced courses.""" SelfPacedConfiguration(enabled=self_paced_enabled).save() self.setup_course(self_paced=self_paced) - with self.assertNumQueries(40, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1): + with self.assertNumQueries(39, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1): self._get_progress_page() @ddt.data( - (False, 40, 26), - (True, 33, 22) + (False, 39, 25), + (True, 32, 21) ) @ddt.unpack def test_progress_queries(self, enable_waffle, initial, subsequent): diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 3c13e5c650..35964b9524 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -404,8 +404,8 @@ class ViewsQueryCountTestCase( return inner @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 4, 32), - (ModuleStoreEnum.Type.split, 3, 12, 32), + (ModuleStoreEnum.Type.mongo, 3, 4, 31), + (ModuleStoreEnum.Type.split, 3, 12, 31), ) @ddt.unpack @count_queries @@ -413,8 +413,8 @@ class ViewsQueryCountTestCase( self.create_thread_helper(mock_request) @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 3, 28), - (ModuleStoreEnum.Type.split, 3, 9, 28), + (ModuleStoreEnum.Type.mongo, 3, 3, 27), + (ModuleStoreEnum.Type.split, 3, 9, 27), ) @ddt.unpack @count_queries diff --git a/openedx/core/djangoapps/bookmarks/tests/test_views.py b/openedx/core/djangoapps/bookmarks/tests/test_views.py index e8344f70ec..30ce17cc2d 100644 --- a/openedx/core/djangoapps/bookmarks/tests/test_views.py +++ b/openedx/core/djangoapps/bookmarks/tests/test_views.py @@ -268,7 +268,7 @@ class BookmarksListViewTests(BookmarksViewsTestsBase): self.assertEqual(response.data['developer_message'], u'Parameter usage_id not provided.') # Send empty data dictionary. - with self.assertNumQueries(8): # No queries for bookmark table. + with self.assertNumQueries(7): # No queries for bookmark table. response = self.send_post( client=self.client, url=reverse('bookmarks'), diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 443dbe046b..dd6f2dd017 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -174,7 +174,7 @@ class TestOwnUsernameAPI(CacheIsolationTestCase, UserAPITestCase): Test that a client (logged in) can get her own username. """ self.client.login(username=self.user.username, password=TEST_PASSWORD) - self._verify_get_own_username(15) + self._verify_get_own_username(14) def test_get_username_inactive(self): """ @@ -184,7 +184,7 @@ class TestOwnUsernameAPI(CacheIsolationTestCase, UserAPITestCase): self.client.login(username=self.user.username, password=TEST_PASSWORD) self.user.is_active = False self.user.save() - self._verify_get_own_username(15) + self._verify_get_own_username(14) def test_get_username_not_logged_in(self): """ @@ -193,7 +193,7 @@ class TestOwnUsernameAPI(CacheIsolationTestCase, UserAPITestCase): """ # verify that the endpoint is inaccessible when not logged in - self._verify_get_own_username(13, expected_status=401) + self._verify_get_own_username(12, expected_status=401) @ddt.ddt @@ -305,7 +305,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): """ self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD) self.create_mock_profile(self.user) - with self.assertNumQueries(19): + with self.assertNumQueries(18): response = self.send_get(self.different_client) self._verify_full_shareable_account_response(response, account_privacy=ALL_USERS_VISIBILITY) @@ -320,7 +320,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): """ self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD) self.create_mock_profile(self.user) - with self.assertNumQueries(19): + with self.assertNumQueries(18): response = self.send_get(self.different_client) self._verify_private_account_response(response, account_privacy=PRIVATE_VISIBILITY) @@ -395,12 +395,12 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): self.assertEqual(False, data["accomplishments_shared"]) self.client.login(username=self.user.username, password=TEST_PASSWORD) - verify_get_own_information(17) + verify_get_own_information(16) # Now make sure that the user can get the same information, even if not active self.user.is_active = False self.user.save() - verify_get_own_information(11) + verify_get_own_information(10) def test_get_account_empty_string(self): """ @@ -414,7 +414,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): legacy_profile.save() self.client.login(username=self.user.username, password=TEST_PASSWORD) - with self.assertNumQueries(17): + with self.assertNumQueries(16): response = self.send_get(self.client) for empty_field in ("level_of_education", "gender", "country", "bio"): self.assertIsNone(response.data[empty_field]) diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index 02726c473c..ac982aff74 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -142,7 +142,7 @@ class TestCourseHomePage(CourseHomePageTestCase): course_home_url(self.course) # Fetch the view and verify the query counts - with self.assertNumQueries(38, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(37, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_home_url(self.course) self.client.get(url) diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index ad2d481381..49c979632a 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -127,7 +127,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase): course_updates_url(self.course) # Fetch the view and verify that the query counts haven't changed - with self.assertNumQueries(31, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(30, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_updates_url(self.course) self.client.get(url) diff --git a/openedx/features/enterprise_support/api.py b/openedx/features/enterprise_support/api.py index a3bffe22ec..293ab6294d 100644 --- a/openedx/features/enterprise_support/api.py +++ b/openedx/features/enterprise_support/api.py @@ -26,7 +26,6 @@ from openedx.core.lib.token_utils import JwtBuilder try: from enterprise import utils as enterprise_utils from enterprise.models import EnterpriseCourseEnrollment, EnterpriseCustomer - from enterprise.tpa_pipeline import get_enterprise_customer_for_request from enterprise.utils import consent_necessary_for_course except ImportError: pass @@ -240,9 +239,9 @@ def enterprise_customer_for_request(request, tpa_hint=None): if not enterprise_enabled(): return None - ec = get_enterprise_customer_for_request(request) + ec = None - if not ec and tpa_hint: + if tpa_hint: try: ec = EnterpriseCustomer.objects.get(enterprise_customer_identity_provider__provider_id=tpa_hint) except EnterpriseCustomer.DoesNotExist: @@ -308,24 +307,6 @@ def get_enterprise_consent_url(request, course_id, user=None, return_to=None): return full_url -def insert_enterprise_pipeline_elements(pipeline): - """ - If the enterprise app is enabled, insert additional elements into the - pipeline so that data sharing consent views are used. - """ - if not enterprise_enabled(): - return - - additional_elements = ( - 'enterprise.tpa_pipeline.handle_enterprise_logistration', - ) - # Find the item we need to insert the data sharing consent elements before - insert_point = pipeline.index('social_core.pipeline.social_auth.load_extra_data') - - for index, element in enumerate(additional_elements): - pipeline.insert(insert_point + index, element) - - def get_cache_key(**kwargs): """ Get MD5 encoded cache key for given arguments. diff --git a/openedx/features/enterprise_support/tests/test_api.py b/openedx/features/enterprise_support/tests/test_api.py index 052f7f434c..fbe49f2231 100644 --- a/openedx/features/enterprise_support/tests/test_api.py +++ b/openedx/features/enterprise_support/tests/test_api.py @@ -14,7 +14,6 @@ from openedx.features.enterprise_support.api import ( enterprise_enabled, get_dashboard_consent_notification, get_enterprise_consent_url, - insert_enterprise_pipeline_elements ) @@ -24,33 +23,9 @@ class TestEnterpriseApi(unittest.TestCase): Test enterprise support APIs. """ - @override_settings(ENABLE_ENTERPRISE_INTEGRATION=False) - def test_utils_with_enterprise_disabled(self): - """ - Test that disabling the enterprise integration flag causes - the utilities to return the expected default values. - """ - self.assertFalse(enterprise_enabled()) - self.assertEqual(insert_enterprise_pipeline_elements(None), None) - @override_settings(ENABLE_ENTERPRISE_INTEGRATION=True) - def test_utils_with_enterprise_enabled(self): - """ - Test that enabling enterprise integration (which is currently on by default) causes the - the utilities to return the expected values. - """ - self.assertTrue(enterprise_enabled()) - pipeline = ['abc', 'social_core.pipeline.social_auth.load_extra_data', 'def'] - insert_enterprise_pipeline_elements(pipeline) - self.assertEqual(pipeline, ['abc', - 'enterprise.tpa_pipeline.handle_enterprise_logistration', - 'social_core.pipeline.social_auth.load_extra_data', - 'def']) - - @override_settings(ENABLE_ENTERPRISE_INTEGRATION=True) - @mock.patch('openedx.features.enterprise_support.api.get_enterprise_customer_for_request') @mock.patch('openedx.features.enterprise_support.api.EnterpriseCustomer') - def test_enterprise_customer_for_request(self, ec_class_mock, get_ec_pipeline_mock): + def test_enterprise_customer_for_request(self, ec_class_mock): """ Test that the correct EnterpriseCustomer, if any, is returned. """ @@ -68,8 +43,6 @@ class TestEnterpriseApi(unittest.TestCase): ec_class_mock.DoesNotExist = Exception ec_class_mock.objects.get.side_effect = get_ec_mock - get_ec_pipeline_mock.return_value = None - request = mock.MagicMock() request.GET.get.return_value = 'real-uuid' self.assertEqual(enterprise_customer_for_request(request), 'this-is-actually-an-enterprise-customer') @@ -85,10 +58,6 @@ class TestEnterpriseApi(unittest.TestCase): self.assertEqual(enterprise_customer_for_request(request, tpa_hint='fake-provider-id'), None) self.assertEqual(enterprise_customer_for_request(request, tpa_hint=None), None) - get_ec_pipeline_mock.return_value = 'also-a-real-enterprise' - - self.assertEqual(enterprise_customer_for_request(request), 'also-a-real-enterprise') - def check_data_sharing_consent(self, consent_required=False, consent_url=None): """ Used to test the data_sharing_consent_required view decorator. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 3bb704c336..6eef908b54 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -35,11 +35,9 @@ django-statici18n==1.4.0 django-storages==1.4.1 django-method-override==0.1.0 django-user-tasks==0.1.4 -git+https://github.com/edx/django-rest-framework.git@1ceda7c086fddffd1c440cc86856441bbf0bd9cb#egg=djangorestframework==3.6.3 django==1.8.18 django-waffle==0.12.0 djangorestframework-jwt==1.11.0 -djangorestframework-oauth==1.1.0 enum34==1.1.6 edx-ccx-keys==0.2.1 edx-celeryutils==0.2.4 @@ -51,7 +49,7 @@ edx-lint==0.4.3 astroid==1.3.8 edx-django-oauth2-provider==1.1.4 edx-django-sites-extensions==2.3.0 -edx-enterprise==0.38.5 +edx-enterprise==0.38.6 edx-oauth2-provider==1.2.0 edx-opaque-keys==0.4.0 edx-organizations==0.4.5 diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index bf9cb4d5d1..a193bc787f 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -69,6 +69,13 @@ git+https://github.com/edx/rfc6266.git@v0.0.5-edx#egg=rfc6266==0.0.5-edx # Used for testing git+https://github.com/edx/lettuce.git@0.2.20.002#egg=lettuce==0.2.20.002 +# Why a DRF fork? See: https://openedx.atlassian.net/browse/PLAT-1581 +git+https://github.com/edx/django-rest-framework.git@1ceda7c086fddffd1c440cc86856441bbf0bd9cb#egg=djangorestframework==3.6.3 + +# Why a drf-oauth fork? To add Django 1.11 compatibility to the abandoned repo. +# This dependency will be removed by this work: https://openedx.atlassian.net/browse/PLAT-1660 +git+https://github.com/edx/django-rest-framework-oauth.git@0a43e8525f1e3048efe4bc70c03de308a277197c#egg=djangorestframework-oauth==1.1.1 + # Our libraries: -e git+https://github.com/edx/codejail.git@a320d43ce6b9c93b17636b2491f724d9e433be47#egg=codejail==0.0 -e git+https://github.com/edx/event-tracking.git@0.2.1#egg=event-tracking==0.2.1