diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index de1ec2e045..9b2d7e9d69 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -309,7 +309,7 @@ class AuthTestCase(ContentStoreTestCase): resp = self.client.get_html(course_url) # re-request, and we should get a redirect to login page - self.assertRedirects(resp, settings.LOGIN_REDIRECT_URL + '?next=/home/') + self.assertRedirects(resp, settings.LOGIN_URL + '?next=/home/') @mock.patch.dict(settings.FEATURES, {"ALLOW_PUBLIC_ACCOUNT_CREATION": False}) def test_signup_button_index_page(self): diff --git a/cms/djangoapps/contentstore/views/public.py b/cms/djangoapps/contentstore/views/public.py index 6306a11427..50ceee25a9 100644 --- a/cms/djangoapps/contentstore/views/public.py +++ b/cms/djangoapps/contentstore/views/public.py @@ -4,6 +4,7 @@ Public views from django.conf import settings from django.template.context_processors import csrf from django.urls import reverse +from django.utils.http import urlquote_plus from django.shortcuts import redirect from django.views.decorators.clickjacking import xframe_options_deny from django.views.decorators.csrf import ensure_csrf_cookie @@ -14,7 +15,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_ from waffle.decorators import waffle_switch from contentstore.config import waffle -__all__ = ['signup', 'login_page', 'howitworks', 'accessibility'] +__all__ = ['signup', 'login_page', 'login_redirect_to_lms', 'howitworks', 'accessibility'] @ensure_csrf_cookie @@ -66,6 +67,20 @@ def login_page(request): ) +def login_redirect_to_lms(request): + """ + This view redirects to the LMS login view. It is used for Django's LOGIN_URL + setting, which is where unauthenticated requests to protected endpoints are redirected. + """ + next_url = request.GET.get('next') + absolute_next_url = request.build_absolute_uri(next_url) + login_url = '{base_url}/login{params}'.format( + base_url=settings.LMS_ROOT_URL, + params='?next=' + urlquote_plus(absolute_next_url) if next_url else '', + ) + return redirect(login_url) + + def howitworks(request): "Proxy view" if request.user.is_authenticated: diff --git a/cms/envs/bok_choy.py b/cms/envs/bok_choy.py index fff1ce11d9..14e4a21636 100644 --- a/cms/envs/bok_choy.py +++ b/cms/envs/bok_choy.py @@ -148,7 +148,7 @@ MOCK_SEARCH_BACKING_FILE = ( # this secret key should be the same as lms/envs/bok_choy.py's SECRET_KEY = "very_secret_bok_choy_key" -LMS_ROOT_URL = "http://localhost:8000" +LMS_ROOT_URL = "http://localhost:8003" if RELEASE_LINE == "master": # On master, acceptance tests use edX books, not the default Open edX books. HELP_TOKENS_BOOKS = { diff --git a/cms/envs/bok_choy_docker.py b/cms/envs/bok_choy_docker.py index 44b1d62e01..cb0c75fc04 100644 --- a/cms/envs/bok_choy_docker.py +++ b/cms/envs/bok_choy_docker.py @@ -6,9 +6,10 @@ Settings for Bok Choy tests that are used when running Studio in Docker-based de # noinspection PyUnresolvedReferences from .bok_choy import * # pylint: disable=wildcard-import -CMS_BASE = '{}:{}'.format(os.environ['BOK_CHOY_HOSTNAME'], os.environ['BOK_CHOY_CMS_PORT']) -LMS_BASE = '{}:{}'.format(os.environ['BOK_CHOY_HOSTNAME'], os.environ['BOK_CHOY_LMS_PORT']) +CMS_BASE = '{}:{}'.format(os.environ['BOK_CHOY_HOSTNAME'], os.environ.get('BOK_CHOY_CMS_PORT', 8031)) +LMS_BASE = '{}:{}'.format(os.environ['BOK_CHOY_HOSTNAME'], os.environ.get('BOK_CHOY_LMS_PORT', 8003)) LMS_ROOT_URL = 'http://{}'.format(LMS_BASE) +LOGIN_REDIRECT_WHITELIST = [CMS_BASE] COMMENTS_SERVICE_URL = 'http://{}:4567'.format(os.environ['BOK_CHOY_HOSTNAME']) EDXNOTES_PUBLIC_API = 'http://{}:8042/api/v1'.format(os.environ['BOK_CHOY_HOSTNAME']) diff --git a/cms/envs/common.py b/cms/envs/common.py index 73c9bde775..d77b809f7e 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -146,6 +146,7 @@ from lms.envs.common import ( _make_locale_paths, ) from path import Path as path +from django.core.urlresolvers import reverse_lazy from lms.djangoapps.lms_xblock.mixin import LmsBlockMixin from cms.lib.xblock.authoring_mixin import AuthoringMixin @@ -371,6 +372,7 @@ CONTEXT_PROCESSORS = ( 'django.contrib.auth.context_processors.auth', # this is required for admin 'django.template.context_processors.csrf', 'help_tokens.context_processor', + 'openedx.core.djangoapps.site_configuration.context_processors.configuration_context', ) # Django templating @@ -426,9 +428,8 @@ DEFAULT_TEMPLATE_ENGINE = TEMPLATES[0] ############################################################################## EDX_ROOT_URL = '' - -LOGIN_REDIRECT_URL = EDX_ROOT_URL + '/signin' -LOGIN_URL = EDX_ROOT_URL + '/signin' +LOGIN_REDIRECT_URL = EDX_ROOT_URL + '/home/' +LOGIN_URL = reverse_lazy('login_redirect_to_lms') # use the ratelimit backend to prevent brute force attacks AUTHENTICATION_BACKENDS = [ diff --git a/cms/envs/devstack_docker.py b/cms/envs/devstack_docker.py index 9ce5038b7f..407d44bd20 100644 --- a/cms/envs/devstack_docker.py +++ b/cms/envs/devstack_docker.py @@ -9,8 +9,8 @@ LOGGING['handlers']['local'] = LOGGING['handlers']['tracking'] = { LOGGING['loggers']['tracking']['handlers'] = ['console'] -LMS_BASE = 'edx.devstack.lms:18000' -CMS_BASE = 'edx.devstack.studio:18010' +LMS_BASE = 'localhost:18000' +CMS_BASE = 'localhost:18010' LMS_ROOT_URL = 'http://{}'.format(LMS_BASE) FEATURES.update({ diff --git a/cms/envs/test.py b/cms/envs/test.py index 0951c0fa75..849130059c 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -134,6 +134,7 @@ if os.environ.get('DISABLE_MIGRATIONS'): LMS_BASE = "localhost:8000" LMS_ROOT_URL = "http://{}".format(LMS_BASE) FEATURES['PREVIEW_LMS_BASE'] = "preview.localhost" +LOGIN_URL = EDX_ROOT_URL + '/signin' CACHES = { diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index 95e6872428..11229321f2 100644 --- a/cms/templates/widgets/header.html +++ b/cms/templates/widgets/header.html @@ -229,6 +229,10 @@ % else: + <% + login_url = settings.LMS_ROOT_URL + '/login' + register_url = settings.LMS_ROOT_URL + '/register' + %> diff --git a/cms/templates/widgets/user_dropdown.html b/cms/templates/widgets/user_dropdown.html index d39d1ac4e2..1b05fb24cf 100644 --- a/cms/templates/widgets/user_dropdown.html +++ b/cms/templates/widgets/user_dropdown.html @@ -39,7 +39,9 @@ - + <% + logout_url = settings.LMS_ROOT_URL + '/logout' + %>
diff --git a/cms/urls.py b/cms/urls.py index 44e223b3a6..812e12d86b 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -83,6 +83,7 @@ urlpatterns = [ url(r'^howitworks$', contentstore.views.howitworks, name='howitworks'), url(r'^signup$', contentstore.views.signup, name='signup'), url(r'^signin$', contentstore.views.login_page, name='login'), + url(r'^signin_redirect_to_lms$', contentstore.views.login_redirect_to_lms, name='login_redirect_to_lms'), url(r'^request_course_creator$', contentstore.views.request_course_creator, name='request_course_creator'), url(r'^course_team/{}(?:/(?P.+))?$'.format(COURSELIKE_KEY_PATTERN), contentstore.views.course_team_handler, name='course_team_handler'), diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 01fe28741e..a8266c6f3a 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -67,12 +67,8 @@ from openedx.features.journals.api import get_journals_context from student.forms import AccountCreationForm, PasswordResetFormNoActive, get_registration_extension_form from student.helpers import ( DISABLE_UNENROLL_CERT_STATES, - auth_pipeline_urls, cert_info, - create_or_set_user_attribute_created_on_site, - do_create_account, generate_activation_email_context, - get_next_url_for_login_page ) from student.message_types import EmailChange, PasswordReset from student.models import ( diff --git a/common/test/acceptance/pages/studio/__init__.py b/common/test/acceptance/pages/studio/__init__.py index 9cda972626..5f219c6052 100644 --- a/common/test/acceptance/pages/studio/__init__.py +++ b/common/test/acceptance/pages/studio/__init__.py @@ -3,4 +3,6 @@ import os # Get the URL of the instance under test HOSTNAME = os.environ.get('BOK_CHOY_HOSTNAME', 'localhost') CMS_PORT = os.environ.get('BOK_CHOY_CMS_PORT', 8031) +LMS_PORT = os.environ.get('BOK_CHOY_LMS_PORT', 8003) BASE_URL = os.environ.get('test_url', 'http://{}:{}'.format(HOSTNAME, CMS_PORT)) +LMS_URL = os.environ.get('test_url', 'http://{}:{}'.format(HOSTNAME, LMS_PORT)) diff --git a/common/test/acceptance/pages/studio/login.py b/common/test/acceptance/pages/studio/login.py index 8d99aee051..0b19b0e612 100644 --- a/common/test/acceptance/pages/studio/login.py +++ b/common/test/acceptance/pages/studio/login.py @@ -4,7 +4,7 @@ Login page for Studio. from bok_choy.page_object import PageObject from bok_choy.promise import EmptyPromise -from common.test.acceptance.pages.studio import BASE_URL +from common.test.acceptance.pages.studio import LMS_URL from common.test.acceptance.pages.studio.course_page import CoursePage from common.test.acceptance.pages.studio.utils import HelpMixin @@ -13,24 +13,25 @@ class LoginMixin(object): """ Mixin class used for logging into the system. """ - def fill_field(self, css, value): + def fill_password(self, password): """ - Fill the login form field with the value. + Fill the password field with the value. """ - self.q(css=css).fill(value) + self.q(css="#login-password").fill(password) def login(self, email, password, expect_success=True): """ Attempt to log in using 'email' and 'password'. """ - self.fill_field('input#email', email) - self.fill_field('input#password', password) - self.q(css='button#submit').first.click() + self.wait_for_element_visibility('#login-email', 'Email field is shown') + self.q(css="#login-email").fill(email) + self.fill_password(password) + self.q(css=".login-button").click() # Ensure that we make it to another page if expect_success: EmptyPromise( - lambda: "signin" not in self.browser.current_url, + lambda: "login" not in self.browser.current_url, "redirected from the login page" ).fulfill() @@ -39,17 +40,20 @@ class LoginPage(PageObject, LoginMixin, HelpMixin): """ Login page for Studio. """ - url = BASE_URL + "/signin" + url = LMS_URL + "/login" def is_browser_on_page(self): - return self.q(css='body.view-signin').visible + return ( + self.q(css="#login-anchor").is_present() and + self.q(css=".login-button").visible + ) class CourseOutlineSignInRedirectPage(CoursePage, LoginMixin): """ - Page shown when the user tries to accesses the course while not signed in. + Page shown when the user tries to access the course while not signed in. """ url_path = "course" def is_browser_on_page(self): - return self.q(css='body.view-signin').visible + return self.q(css=".login-button").visible diff --git a/common/test/acceptance/pages/studio/signup.py b/common/test/acceptance/pages/studio/signup.py index 66a013ac1c..26975a8899 100644 --- a/common/test/acceptance/pages/studio/signup.py +++ b/common/test/acceptance/pages/studio/signup.py @@ -4,7 +4,7 @@ Signup page for studio from bok_choy.page_object import PageObject from common.test.acceptance.pages.common.utils import click_css -from common.test.acceptance.pages.studio import BASE_URL +from common.test.acceptance.pages.studio import LMS_URL from common.test.acceptance.pages.studio.utils import HelpMixin, set_input_value @@ -13,22 +13,29 @@ class SignupPage(PageObject, HelpMixin): Signup page for Studio. """ - url = BASE_URL + "/signup" + url = LMS_URL + "/register" def is_browser_on_page(self): - return self.q(css='body.view-signup').visible + return ( + self.q(css="#register-anchor").is_present() and + self.q(css=".register-button").visible + ) def input_password(self, password): """Inputs a password and then returns the password input""" - return set_input_value(self, '#password', password) + return set_input_value(self, "#register-password", password) - def sign_up_user(self, registration_dictionary): + def sign_up_user(self, email, name, username, password, country="US", favorite_movie="Alf"): """ Register the user. """ - for css, value in registration_dictionary.iteritems(): - set_input_value(self, css, value) + self.q(css="#register-email").fill(email) + self.q(css="#register-name").fill(name) + self.q(css="#register-username").fill(username) + self.q(css="#register-password").fill(password) + self.q(css="#register-country").results[0].send_keys(country) + self.q(css="#register-favorite_movie").fill(favorite_movie) - click_css(page=self, css='#tos', require_notification=False) - click_css(page=self, css='#submit', require_notification=False) - self.wait_for_element_absence('#submit', 'Submit button is gone.') + # Submit it + self.q(css=".register-button").click() + self.wait_for_element_absence('.register-button', 'Register button is gone.') diff --git a/common/test/acceptance/tests/studio/test_studio_general.py b/common/test/acceptance/tests/studio/test_studio_general.py index 469f701f8f..af77c69e96 100644 --- a/common/test/acceptance/tests/studio/test_studio_general.py +++ b/common/test/acceptance/tests/studio/test_studio_general.py @@ -8,6 +8,7 @@ from selenium.webdriver.common.keys import Keys from base_studio_test import StudioCourseTest from common.test.acceptance.fixtures.course import CourseFixture, XBlockFixtureDesc from common.test.acceptance.pages.common.auto_auth import AutoAuthPage +from common.test.acceptance.pages.studio import LMS_URL from common.test.acceptance.pages.studio.asset_index import AssetIndexPageStudioFrontend from common.test.acceptance.pages.studio.course_info import CourseUpdatesPage from common.test.acceptance.pages.studio.edit_tabs import PagesPage @@ -118,15 +119,14 @@ class SignUpAndSignInTest(UniqueCourseTest): index_page.visit() index_page.click_sign_up() - unique_number = uuid.uuid4().hex[:4] - registration_dic = { - '#email': '{}-email@host.com'.format(unique_number), - '#name': '{}-name'.format(unique_number), - '#username': '{}-username'.format(unique_number), - '#password': '{}-password'.format(unique_number), - } # Register the user. - self.sign_up_page.sign_up_user(registration_dic) + unique_number = uuid.uuid4().hex[:4] + self.sign_up_page.sign_up_user( + '{}-email@host.com'.format(unique_number), + '{}-name'.format(unique_number), + '{}-username'.format(unique_number), + '{}-password'.format(unique_number), + ) home = HomePage(self.browser) home.wait_for_page() @@ -145,8 +145,8 @@ class SignUpAndSignInTest(UniqueCourseTest): password_input = self.sign_up_page.input_password('a') # Arbitrary short password that will fail password_input.send_keys(Keys.TAB) # Focus out of the element - index_page.wait_for_element_visibility('#password_error', 'Password Error Message') - self.assertIsNotNone(index_page.q(css='#password_error').text) # Make sure there is an error message + index_page.wait_for_element_visibility('#register-password-validation-error', 'Password Error Message') + self.assertIsNotNone(index_page.q(css='#register-password-validation-error-msg')) # Error message should exist def test_login_with_valid_redirect(self): """ @@ -184,9 +184,8 @@ class SignUpAndSignInTest(UniqueCourseTest): self.browser.get(self.browser.current_url.split('=')[0] + '=http://www.google.com') # Login self.course_outline_sign_in_redirect_page.login(self.user['email'], self.user['password']) - home = HomePage(self.browser) - home.wait_for_page() - self.assertEqual(self.browser.current_url, home.url) + # Verify that we land in LMS instead of the invalid redirect url + self.assertEqual(self.browser.current_url, LMS_URL + "/dashboard") def test_login_with_mistyped_credentials(self): """ @@ -219,16 +218,9 @@ class SignUpAndSignInTest(UniqueCourseTest): ) # Verify that login error is shown self.course_outline_sign_in_redirect_page.wait_for_element_visibility( - '#login_error', + ".js-form-errors.status.submission-error", 'Login error is visible' ) - # Change the password - self.course_outline_sign_in_redirect_page.fill_field('input#password', 'changed_password') - # Login error should not be visible - self.course_outline_sign_in_redirect_page.wait_for_element_invisibility( - '#login_error', - 'Login error is not visible' - ) # Login with correct credentials self.course_outline_sign_in_redirect_page.login(self.user['email'], self.user['password']) self.course_outline_page.wait_for_page() diff --git a/common/test/acceptance/tests/studio/test_studio_help.py b/common/test/acceptance/tests/studio/test_studio_help.py index 1ed9624bff..8f2846ef69 100644 --- a/common/test/acceptance/tests/studio/test_studio_help.py +++ b/common/test/acceptance/tests/studio/test_studio_help.py @@ -84,68 +84,6 @@ class StudioHelpTest(StudioCourseTest): ) -@attr(shard=20) -class SignInHelpTest(AcceptanceTest): - """ - Tests help links on 'Sign In' page - """ - def setUp(self): - super(SignInHelpTest, self).setUp() - self.index_page = IndexPage(self.browser) - self.index_page.visit() - - def test_sign_in_nav_help(self): - """ - Scenario: Help link in navigation bar is working on 'Sign In' page. - Given that I am on the 'Sign In" page. - And I want help about the sign in - And I click the 'Help' in the navigation bar - Then Help link should open. - And help url should be correct - """ - sign_in_page = self.index_page.click_sign_in() - expected_url = _get_expected_documentation_url('/getting_started/index.html') - - # Assert that help link is correct. - assert_nav_help_link( - test=self, - page=sign_in_page, - href=expected_url, - signed_in=False - ) - - -@attr(shard=20) -class SignUpHelpTest(AcceptanceTest): - """ - Tests help links on 'Sign Up' page. - """ - def setUp(self): - super(SignUpHelpTest, self).setUp() - self.index_page = IndexPage(self.browser) - self.index_page.visit() - - def test_sign_up_nav_help(self): - """ - Scenario: Help link in navigation bar is working on 'Sign Up' page. - Given that I am on the 'Sign Up" page. - And I want help about the sign up - And I click the 'Help' in the navigation bar - Then Help link should open. - And help url should be correct - """ - sign_up_page = self.index_page.click_sign_up() - expected_url = _get_expected_documentation_url('/getting_started/index.html') - - # Assert that help link is correct. - assert_nav_help_link( - test=self, - page=sign_up_page, - href=expected_url, - signed_in=False - ) - - @attr(shard=20) class HomeHelpTest(StudioCourseTest): """ diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py index e87c4ee056..74b242a80f 100644 --- a/lms/envs/bok_choy.py +++ b/lms/envs/bok_choy.py @@ -233,7 +233,10 @@ BADGING_BACKEND = 'lms.djangoapps.badges.backends.tests.dummy_backend.DummyBacke # Configure the LMS to use our stub eCommerce implementation ECOMMERCE_API_URL = 'http://localhost:8043/api/v2/' -LMS_ROOT_URL = "http://localhost:8000" +LMS_ROOT_URL = "http://localhost:{}".format(os.environ.get('BOK_CHOY_LMS_PORT', 8003)) +CMS_BASE = "localhost:{}".format(os.environ.get('BOK_CHOY_CMS_PORT', 8031)) +LOGIN_REDIRECT_WHITELIST = [CMS_BASE] + if RELEASE_LINE == "master": # On master, acceptance tests use edX books, not the default Open edX books. HELP_TOKENS_BOOKS = { diff --git a/lms/envs/bok_choy_docker.py b/lms/envs/bok_choy_docker.py index 44b1d62e01..cb0c75fc04 100644 --- a/lms/envs/bok_choy_docker.py +++ b/lms/envs/bok_choy_docker.py @@ -6,9 +6,10 @@ Settings for Bok Choy tests that are used when running Studio in Docker-based de # noinspection PyUnresolvedReferences from .bok_choy import * # pylint: disable=wildcard-import -CMS_BASE = '{}:{}'.format(os.environ['BOK_CHOY_HOSTNAME'], os.environ['BOK_CHOY_CMS_PORT']) -LMS_BASE = '{}:{}'.format(os.environ['BOK_CHOY_HOSTNAME'], os.environ['BOK_CHOY_LMS_PORT']) +CMS_BASE = '{}:{}'.format(os.environ['BOK_CHOY_HOSTNAME'], os.environ.get('BOK_CHOY_CMS_PORT', 8031)) +LMS_BASE = '{}:{}'.format(os.environ['BOK_CHOY_HOSTNAME'], os.environ.get('BOK_CHOY_LMS_PORT', 8003)) LMS_ROOT_URL = 'http://{}'.format(LMS_BASE) +LOGIN_REDIRECT_WHITELIST = [CMS_BASE] COMMENTS_SERVICE_URL = 'http://{}:4567'.format(os.environ['BOK_CHOY_HOSTNAME']) EDXNOTES_PUBLIC_API = 'http://{}:8042/api/v1'.format(os.environ['BOK_CHOY_HOSTNAME']) diff --git a/lms/envs/common.py b/lms/envs/common.py index a68d48b993..f3107233f9 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2519,7 +2519,7 @@ if FEATURES.get('ENABLE_CORS_HEADERS'): # to simulate cross-domain requests. XDOMAIN_PROXY_CACHE_TIMEOUT = 60 * 15 -LOGIN_REDIRECT_WHITELIST = [] +LOGIN_REDIRECT_WHITELIST = [CMS_BASE] ###################### Registration ################################## diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 8b436ac3f7..425e580715 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -21,7 +21,7 @@ SITE_NAME = 'localhost:8000' CELERY_ALWAYS_EAGER = True HTTPS = 'off' -LMS_ROOT_URL = 'http://localhost:8000' +LMS_ROOT_URL = "http://localhost:8000" LMS_INTERNAL_ROOT_URL = LMS_ROOT_URL ENTERPRISE_API_URL = LMS_INTERNAL_ROOT_URL + '/enterprise/api/v1/' @@ -229,7 +229,7 @@ CORS_ALLOW_HEADERS = corsheaders_default_headers + ( 'use-jwt-cookie', ) -LOGIN_REDIRECT_WHITELIST = [] +LOGIN_REDIRECT_WHITELIST = [CMS_BASE] ###################### JWTs ###################### JWT_AUTH.update({ diff --git a/lms/envs/devstack_docker.py b/lms/envs/devstack_docker.py index 4d8ab29d11..98650770f0 100644 --- a/lms/envs/devstack_docker.py +++ b/lms/envs/devstack_docker.py @@ -9,8 +9,8 @@ LOGGING['handlers']['local'] = LOGGING['handlers']['tracking'] = { LOGGING['loggers']['tracking']['handlers'] = ['console'] -LMS_BASE = 'edx.devstack.lms:18000' -CMS_BASE = 'edx.devstack.studio:18010' +LMS_BASE = 'localhost:18000' +CMS_BASE = 'localhost:18010' SITE_NAME = LMS_BASE LMS_ROOT_URL = 'http://{}'.format(LMS_BASE) LMS_INTERNAL_ROOT_URL = LMS_ROOT_URL diff --git a/openedx/core/djangoapps/site_configuration/context_processors.py b/openedx/core/djangoapps/site_configuration/context_processors.py index 801d98b8f7..ad15d3a85c 100644 --- a/openedx/core/djangoapps/site_configuration/context_processors.py +++ b/openedx/core/djangoapps/site_configuration/context_processors.py @@ -3,6 +3,7 @@ Django template context processors. """ from django.conf import settings +from django.utils.http import urlquote_plus from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers @@ -12,5 +13,7 @@ def configuration_context(request): # pylint: disable=unused-argument Configuration context for django templates. """ return { - 'platform_name': configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME) + 'platform_name': configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME), + 'current_url': urlquote_plus(request.build_absolute_uri(request.path)), + 'current_site_url': urlquote_plus(request.build_absolute_uri('/')), } diff --git a/openedx/core/djangoapps/theming/tests/test_views.py b/openedx/core/djangoapps/theming/tests/test_views.py index 5f21a37777..905efd0e84 100644 --- a/openedx/core/djangoapps/theming/tests/test_views.py +++ b/openedx/core/djangoapps/theming/tests/test_views.py @@ -46,7 +46,7 @@ class TestThemingViews(TestCase): self.assertRedirects( response, '{login_url}?next={url}'.format( - login_url=settings.LOGIN_REDIRECT_URL, + login_url=settings.LOGIN_URL, url=THEMING_ADMIN_URL, ) ) diff --git a/openedx/core/djangoapps/user_authn/tests/test_utils.py b/openedx/core/djangoapps/user_authn/tests/test_utils.py index 0b0555b9a4..533bed3bd0 100644 --- a/openedx/core/djangoapps/user_authn/tests/test_utils.py +++ b/openedx/core/djangoapps/user_authn/tests/test_utils.py @@ -1,6 +1,8 @@ """ Test User Authentication utilities """ +from collections import namedtuple import ddt + from django.test import TestCase from django.test.client import RequestFactory from django.test.utils import override_settings @@ -18,19 +20,27 @@ class TestRedirectUtils(TestCase): super(TestRedirectUtils, self).setUp() self.request = RequestFactory() + RedirectCase = namedtuple('RedirectCase', ['url', 'host', 'req_is_secure', 'expected_is_safe']) + @ddt.data( - ('/dashboard', 'testserver', True), - ('https://edx.org/courses', 'edx.org', True), - ('https://test.edx.org/courses', 'edx.org', True), - ('https://www.amazon.org', 'edx.org', False), - ('http://edx.org/courses', 'edx.org', False), - ('http:///edx.org/courses', 'edx.org', False), # Django's is_safe_url protects against "///" + RedirectCase('/dashboard', 'testserver', req_is_secure=True, expected_is_safe=True), + RedirectCase('https://test.edx.org/courses', 'edx.org', req_is_secure=True, expected_is_safe=True), + RedirectCase('https://www.amazon.org', 'edx.org', req_is_secure=True, expected_is_safe=False), + + # https is required only if the request is_secure + RedirectCase('https://edx.org/courses', 'edx.org', req_is_secure=True, expected_is_safe=True), + RedirectCase('http://edx.org/courses', 'edx.org', req_is_secure=False, expected_is_safe=True), + RedirectCase('http://edx.org/courses', 'edx.org', req_is_secure=True, expected_is_safe=False), + + # Django's is_safe_url protects against "///" + RedirectCase('http:///edx.org/courses', 'edx.org', req_is_secure=True, expected_is_safe=False), ) @ddt.unpack @override_settings(LOGIN_REDIRECT_WHITELIST=['test.edx.org']) - def test_safe_redirect(self, url, host, expected_is_safe): + def test_safe_redirect(self, url, host, req_is_secure, expected_is_safe): """ Test safe next parameter """ req = self.request.get('/login', HTTP_HOST=host) + req.is_secure = lambda: req_is_secure actual_is_safe = is_safe_login_or_logout_redirect(req, url) self.assertEqual(actual_is_safe, expected_is_safe) diff --git a/openedx/core/djangoapps/user_authn/utils.py b/openedx/core/djangoapps/user_authn/utils.py index 52571d070f..6a58ac8eed 100644 --- a/openedx/core/djangoapps/user_authn/utils.py +++ b/openedx/core/djangoapps/user_authn/utils.py @@ -23,5 +23,7 @@ def is_safe_login_or_logout_redirect(request, redirect_to): if redirect_to in application.redirect_uris: login_redirect_whitelist.add(urlparse(redirect_to).netloc) - is_safe_url = http.is_safe_url(redirect_to, allowed_hosts=login_redirect_whitelist, require_https=True) + is_safe_url = http.is_safe_url( + redirect_to, allowed_hosts=login_redirect_whitelist, require_https=request.is_secure(), + ) return is_safe_url diff --git a/openedx/core/djangoapps/user_authn/views/logout.py b/openedx/core/djangoapps/user_authn/views/logout.py index 29e6b4f801..c1767e310d 100644 --- a/openedx/core/djangoapps/user_authn/views/logout.py +++ b/openedx/core/djangoapps/user_authn/views/logout.py @@ -29,11 +29,11 @@ class LogoutView(TemplateView): @property def target(self): """ - If a redirect_url is specified in the querystring for this request, and the value is a url - with the same host, the view will redirect to this page after rendering the template. + If a redirect_url is specified in the querystring for this request, and the value is a safe + url for redirect, the view will redirect to this page after rendering the template. If it is not specified, we will use the default target url. """ - target_url = self.request.GET.get('redirect_url') + target_url = self.request.GET.get('redirect_url') or self.request.GET.get('next') if target_url and is_safe_login_or_logout_redirect(self.request, target_url): return target_url diff --git a/openedx/tests/settings.py b/openedx/tests/settings.py index 988ea74c36..22da32f525 100644 --- a/openedx/tests/settings.py +++ b/openedx/tests/settings.py @@ -87,7 +87,7 @@ INSTALLED_APPS = ( 'completion', ) -LMS_ROOT_URL = 'http://localhost:8000' +LMS_ROOT_URL = "http://localhost:8000" MEDIA_ROOT = tempfile.mkdtemp()