diff --git a/common/djangoapps/microsite_configuration/middleware.py b/common/djangoapps/microsite_configuration/middleware.py index d3f1cf8fc6..fa56c8c3bb 100644 --- a/common/djangoapps/microsite_configuration/middleware.py +++ b/common/djangoapps/microsite_configuration/middleware.py @@ -36,50 +36,3 @@ class MicrositeMiddleware(object): """ microsite.clear() return response - - -class MicrositeSessionCookieDomainMiddleware(object): - """ - Special case middleware which should be at the very end of the MIDDLEWARE list (so that it runs first - on the process_response chain). This middleware will define a wrapper function for the set_cookie() function - on the HttpResponse object, if the request is running in a middleware. - - This wrapped set_cookie will change the SESSION_COOKIE_DOMAIN setting so that the cookie can be bound to a - fully customized URL. - """ - - def process_response(self, request, response): - """ - Standard Middleware entry point - """ - - # See if we are running in a Microsite *AND* we have a custom SESSION_COOKIE_DOMAIN defined - # in configuration - if microsite.has_override_value('SESSION_COOKIE_DOMAIN'): - - # define wrapper function for the standard set_cookie() - def _set_cookie_wrapper(key, value='', max_age=None, expires=None, path='/', domain=None, secure=None, httponly=False): - - # only override if we are setting the cookie name to be the one the Django Session Middleware uses - # as defined in settings.SESSION_COOKIE_NAME - if key == settings.SESSION_COOKIE_NAME: - domain = microsite.get_value('SESSION_COOKIE_DOMAIN', domain) - - # then call down into the normal Django set_cookie method - return response.set_cookie_wrapped_func( - key, - value, - max_age=max_age, - expires=expires, - path=path, - domain=domain, - secure=secure, - httponly=httponly - ) - - # then point the HttpResponse.set_cookie to point to the wrapper and keep - # the original around - response.set_cookie_wrapped_func = response.set_cookie - response.set_cookie = _set_cookie_wrapper - - return response diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 3664d84c41..b39ff33534 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -370,8 +370,8 @@ class ViewsQueryCountTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSet return inner @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 4, 32), - (ModuleStoreEnum.Type.split, 3, 13, 32), + (ModuleStoreEnum.Type.mongo, 3, 4, 33), + (ModuleStoreEnum.Type.split, 3, 13, 33), ) @ddt.unpack @count_queries @@ -379,8 +379,8 @@ class ViewsQueryCountTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSet self.create_thread_helper(mock_request) @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 3, 26), - (ModuleStoreEnum.Type.split, 3, 10, 26), + (ModuleStoreEnum.Type.mongo, 3, 3, 27), + (ModuleStoreEnum.Type.split, 3, 10, 27), ) @ddt.unpack @count_queries diff --git a/lms/envs/common.py b/lms/envs/common.py index f11f2a04fb..11653a2f3f 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1174,7 +1174,7 @@ MIDDLEWARE_CLASSES = ( 'openedx.core.djangoapps.theming.middleware.CurrentSiteThemeMiddleware', # This must be last - 'microsite_configuration.middleware.MicrositeSessionCookieDomainMiddleware', + 'openedx.core.djangoapps.site_configuration.middleware.SessionCookieDomainOverrideMiddleware', ) # Clickjacking protection can be enabled by setting this to 'DENY' diff --git a/openedx/core/djangoapps/site_configuration/middleware.py b/openedx/core/djangoapps/site_configuration/middleware.py new file mode 100644 index 0000000000..3df414c7f0 --- /dev/null +++ b/openedx/core/djangoapps/site_configuration/middleware.py @@ -0,0 +1,55 @@ +""" +This file contains Django middleware related to the site_configuration app. +""" + +from django.conf import settings +from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers + + +class SessionCookieDomainOverrideMiddleware(object): + """ + Special case middleware which should be at the very end of the MIDDLEWARE list (so that it runs first + on the process_response chain). This middleware will define a wrapper function for the set_cookie() function + on the HttpResponse object, if the request is running in a middleware. + + This wrapped set_cookie will change the SESSION_COOKIE_DOMAIN setting so that the cookie can be bound to a + fully customized URL. + """ + + def process_response(self, __, response): + """ + Django middleware hook for process responses + """ + + # Check for SESSION_COOKIE_DOMAIN setting override + session_cookie_domain = configuration_helpers.get_value('SESSION_COOKIE_DOMAIN') + if session_cookie_domain: + def _set_cookie_wrapper(key, value='', max_age=None, expires=None, path='/', domain=None, secure=None, + httponly=False): + """ + Wrapper function for set_cookie() which applies SESSION_COOKIE_DOMAIN override + """ + + # only override if we are setting the cookie name to be the one the Django Session Middleware uses + # as defined in settings.SESSION_COOKIE_NAME + if key == configuration_helpers.get_value('SESSION_COOKIE_NAME', settings.SESSION_COOKIE_NAME): + domain = session_cookie_domain + + # then call down into the normal Django set_cookie method + return response.set_cookie_wrapped_func( + key, + value, + max_age=max_age, + expires=expires, + path=path, + domain=domain, + secure=secure, + httponly=httponly + ) + + # then point the HttpResponse.set_cookie to point to the wrapper and keep + # the original around + response.set_cookie_wrapped_func = response.set_cookie + response.set_cookie = _set_cookie_wrapper + + return response diff --git a/openedx/core/djangoapps/site_configuration/tests/mixins.py b/openedx/core/djangoapps/site_configuration/tests/mixins.py index 0758f58787..a1f246e442 100644 --- a/openedx/core/djangoapps/site_configuration/tests/mixins.py +++ b/openedx/core/djangoapps/site_configuration/tests/mixins.py @@ -28,6 +28,7 @@ class SiteMixin(object): site=self.site_other, values={ "SITE_NAME": self.site_other.domain, + "SESSION_COOKIE_DOMAIN": self.site_other.domain, "course_org_filter": "fakeOtherX", "ENABLE_MKTG_SITE": True, "SHOW_ECOMMERCE_REPORTS": True, diff --git a/common/djangoapps/microsite_configuration/tests/test_middleware.py b/openedx/core/djangoapps/site_configuration/tests/test_middleware.py similarity index 72% rename from common/djangoapps/microsite_configuration/tests/test_middleware.py rename to openedx/core/djangoapps/site_configuration/tests/test_middleware.py index 3b8753bff7..96f2b0e962 100644 --- a/common/djangoapps/microsite_configuration/tests/test_middleware.py +++ b/openedx/core/djangoapps/site_configuration/tests/test_middleware.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- """ -Test Microsite middleware. +Test site_configuration middleware. """ import ddt import unittest @@ -20,6 +20,7 @@ from microsite_configuration.tests.tests import ( side_effect_for_get_value, MICROSITE_BACKENDS, ) +from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory # NOTE: We set SESSION_SAVE_EVERY_REQUEST to True in order to make sure @@ -28,13 +29,13 @@ from microsite_configuration.tests.tests import ( @ddt.ddt @override_settings(SESSION_SAVE_EVERY_REQUEST=True) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -class MicrositeSessionCookieTests(DatabaseMicrositeTestCase): +class SessionCookieDomainOverrideTests(DatabaseMicrositeTestCase): """ Tests regarding the session cookie management in the middlware for Microsites """ def setUp(self): - super(MicrositeSessionCookieTests, self).setUp() + super(SessionCookieDomainOverrideTests, self).setUp() # Create a test client, and log it in so that it will save some session # data. self.user = UserFactory.create() @@ -43,10 +44,21 @@ class MicrositeSessionCookieTests(DatabaseMicrositeTestCase): self.client = Client() self.client.login(username=self.user.username, password="password") + self.site = SiteFactory.create( + domain='testserver.fake', + name='testserver.fake' + ) + self.site_configuration = SiteConfigurationFactory.create( + site=self.site, + values={ + "SESSION_COOKIE_DOMAIN": self.site.domain, + } + ) + @ddt.data(*MICROSITE_BACKENDS) - def test_session_cookie_domain_no_microsite(self, site_backend): + def test_session_cookie_domain_no_override(self, site_backend): """ - Tests that non-microsite behaves according to default behavior + Test sessionid cookie when no override is set """ with patch('microsite_configuration.microsite.BACKEND', get_backend(site_backend, BaseMicrositeBackend)): @@ -55,7 +67,7 @@ class MicrositeSessionCookieTests(DatabaseMicrositeTestCase): self.assertNotIn('Domain', str(response.cookies['sessionid'])) @ddt.data(*MICROSITE_BACKENDS) - def test_session_cookie_domain(self, site_backend): + def test_session_cookie_domain_with_microsite_override(self, site_backend): """ Makes sure that the cookie being set in a Microsite is the one specially overridden in configuration @@ -71,7 +83,6 @@ class MicrositeSessionCookieTests(DatabaseMicrositeTestCase): Tests to make sure that a Microsite that specifies None for 'SESSION_COOKIE_DOMAIN' does not set a domain on the session cookie """ - with patch('microsite_configuration.microsite.get_value') as mock_get_value: mock_get_value.side_effect = side_effect_for_get_value('SESSION_COOKIE_DOMAIN', None) with patch('microsite_configuration.microsite.BACKEND', @@ -79,3 +90,10 @@ class MicrositeSessionCookieTests(DatabaseMicrositeTestCase): response = self.client.get('/', HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) self.assertNotIn('test_site.localhost', str(response.cookies['sessionid'])) self.assertNotIn('Domain', str(response.cookies['sessionid'])) + + def test_session_cookie_domain_with_site_configuration_override(self): + """ + Makes sure that the cookie being set is for the overridden domain + """ + response = self.client.get('/', HTTP_HOST=self.site.domain) + self.assertIn(self.site.domain, str(response.cookies['sessionid'])) 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 720b855395..72c03d0f8e 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -248,7 +248,7 @@ class TestAccountAPI(CacheIsolationTestCase, UserAPITestCase): """ self.different_client.login(username=self.different_user.username, password=self.test_password) self.create_mock_profile(self.user) - with self.assertNumQueries(18): + with self.assertNumQueries(19): response = self.send_get(self.different_client) self._verify_full_shareable_account_response(response, account_privacy=ALL_USERS_VISIBILITY) @@ -263,7 +263,7 @@ class TestAccountAPI(CacheIsolationTestCase, UserAPITestCase): """ self.different_client.login(username=self.different_user.username, password=self.test_password) self.create_mock_profile(self.user) - with self.assertNumQueries(18): + with self.assertNumQueries(19): response = self.send_get(self.different_client) self._verify_private_account_response(response, account_privacy=PRIVATE_VISIBILITY)