From 2202545aec6e3ca3bcbd787ac92e09d5eddd9b1b Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Wed, 27 Nov 2019 11:31:24 -0500 Subject: [PATCH] remove studio signin and signup pages This completes the work started in https://github.com/edx/edx-platform/pull/19453 to use the LMS login and registration for Studio, rather than Studio providing its own implementation. LMS login/registration are being used for the following reasons: 1. LMS logistration properly handles all SSO integrations. 2. A single logistration is simpler to maintain and understand. 3. Allows Studio to work more like all other IDAs that use LMS logistration. The original switch to use LMS logistration for Studio also added the toggle `DISABLE_STUDIO_SSO_OVER_LMS` to provide the community some additional time for switching. This commit removes this toggle, which at this point means all deployments will use the LMS logistration. This change requires sharing cookies across LMS and Studio. Should that prove to be a problem for certain Open edX instances, there are discussions of possible alternative solutions. See https://github.com/edx/edx-platform/pull/19845#issuecomment-559154256 Detailed changes: * Fix some Studio links that still went to old Studio signin and signup. * Remove DISABLE_STUDIO_SSO_OVER_LMS feature toggle. * Remove old studio signin and signup pages and templates. * Fix url name "login", which had different meanings for Studio and LMS. * Use the following settings: LOGIN_URL, FRONTEND_LOGIN_URL, FRONTEND_LOGOUT_URL, and FRONTEND_REGISTER_URL. * Redirect /signin and /signup to the LMS logistration. * Add custom metric `uses_pattern_library`. * Add custom metric `student_activate_account`. * Add Django Settings to allow /signin, /signup, and /login_post to be disabled once ready. This work also relates to ARCH-218 and DEPR-6. ARCH-1253 --- .../contentstore/tests/test_contentstore.py | 36 +--- cms/djangoapps/contentstore/tests/tests.py | 202 ++---------------- cms/djangoapps/contentstore/views/public.py | 59 ++--- cms/djangoapps/maintenance/tests.py | 5 +- cms/envs/common.py | 57 ++++- cms/envs/production.py | 7 - cms/envs/test.py | 3 - cms/static/cms/js/build.js | 1 - cms/static/cms/js/spec/main_webpack.js | 1 - cms/static/js/factories/login.js | 63 ------ cms/static/js/factories/register.js | 59 ----- cms/static/js/spec/views/login_studio_spec.js | 35 --- cms/templates/activation_active.html | 37 ---- cms/templates/activation_complete.html | 48 ----- cms/templates/activation_invalid.html | 42 ---- cms/templates/howitworks.html | 6 +- cms/templates/js/mock/login.underscore | 12 -- cms/templates/login.html | 61 ------ cms/templates/register.html | 116 ---------- .../registration/activation_complete.html | 13 +- cms/templates/registration/reg_complete.html | 3 - cms/templates/widgets/header.html | 7 +- cms/templates/widgets/user_dropdown.html | 11 +- cms/urls.py | 14 +- common/djangoapps/edxmako/tests.py | 73 ++++--- .../djangoapps/student/tests/test_helpers.py | 7 +- common/djangoapps/student/views/management.py | 17 +- .../tests/specs/test_google.py | 2 +- .../tests/studio/test_studio_general.py | 63 +----- .../test/test-theme/cms/templates/login.html | 59 ----- lms/envs/test.py | 5 +- .../cache_toolbox/tests/test_middleware.py | 8 +- .../djangoapps/theming/tests/test_helpers.py | 14 +- .../tests/test_theme_style_overrides.py | 50 ----- .../djangoapps/theming/tests/test_views.py | 5 +- .../core/djangoapps/user_api/accounts/api.py | 8 +- .../core/djangoapps/user_authn/urls_common.py | 11 +- .../djangoapps/user_authn/views/logout.py | 6 +- .../user_authn/views/tests/test_login.py | 5 +- .../user_authn/views/tests/test_views.py | 6 +- themes/red-theme/cms/templates/login.html | 58 ----- webpack.common.config.js | 1 - 42 files changed, 227 insertions(+), 1069 deletions(-) delete mode 100644 cms/static/js/factories/login.js delete mode 100644 cms/static/js/factories/register.js delete mode 100644 cms/static/js/spec/views/login_studio_spec.js delete mode 100644 cms/templates/activation_active.html delete mode 100644 cms/templates/activation_complete.html delete mode 100644 cms/templates/activation_invalid.html delete mode 100644 cms/templates/js/mock/login.underscore delete mode 100644 cms/templates/login.html delete mode 100644 cms/templates/register.html delete mode 100644 cms/templates/registration/reg_complete.html delete mode 100644 common/test/test-theme/cms/templates/login.html delete mode 100644 themes/red-theme/cms/templates/login.html diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 8090ad5987ba..6c156db19179 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -2186,10 +2186,12 @@ def test_how_it_works(self): self._test_page("/howitworks") def test_signup(self): - self._test_page("/signup") + # deprecated signup url redirects to LMS register. + self._test_page("/signup", 301) def test_login(self): - self._test_page("/signin") + # deprecated signin url redirects to LMS login. + self._test_page("/signin", 302) def test_logout(self): # Logout redirects. @@ -2202,36 +2204,6 @@ def test_accessibility(self): self._test_page('/accessibility') -class SigninPageTestCase(TestCase): - """ - Tests that the CSRF token is directly included in the signin form. This is - important to make sure that the script is functional independently of any - other script. - """ - - def test_csrf_token_is_present_in_form(self): - # Expected html: - #
- # ... - #
- # ... - # - # ... - #
- # ... - #
- response = self.client.get("/signin") - csrf_token = response.cookies.get("csrftoken") - form = lxml.html.fromstring(response.content).get_element_by_id("login_form") - csrf_input_field = form.find(".//input[@name='csrfmiddlewaretoken']") - - self.assertIsNotNone(csrf_token) - self.assertIsNotNone(csrf_token.value) - self.assertIsNotNone(csrf_input_field) - - self.assertTrue(_compare_salted_tokens(csrf_token.value, csrf_input_field.attrib["value"])) - - def _create_course(test, course_key, course_data): """ Creates a course via an AJAX request and verifies the URL returned in the response. diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 6c5793fa665c..4d25aac60c6f 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -7,19 +7,14 @@ import time import mock -import pytest from contentstore.tests.test_course_settings import CourseTestCase from contentstore.tests.utils import AjaxEnabledTestClient, parse_json, registration, user from ddt import data, ddt, unpack from django.conf import settings -from django.contrib.auth.models import User from django.core.cache import cache -from django.test import TestCase from django.test.utils import override_settings from django.urls import reverse -from freezegun import freeze_time from pytz import UTC -from six.moves import range from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -87,36 +82,7 @@ def activate_user(self, email): self.assertTrue(user(email).is_active) -@pytest.mark.django_db -def test_create_account_email_already_exists(django_db_use_migrations): - """ - This is tricky. Django's user model doesn't have a constraint on - unique email addresses, but we *add* that constraint during the - migration process: - see common/djangoapps/student/migrations/0004_add_email_index.py - - The behavior we *want* is for this account creation request - to fail, due to this uniqueness constraint, but the request will - succeed if the migrations have not run. - - django_db_use_migration is a pytest fixture that tells us if - migrations have been run. Since pytest fixtures don't play nice - with TestCase objects this is a function and doesn't get to use - assertRaises. - """ - if django_db_use_migrations: - email = 'a@b.com' - pw = 'xyz' - username = 'testuser' - User.objects.create_user(username, email, pw) - - # Hack to use the _create_account shortcut - case = ContentStoreTestCase() - resp = case._create_account("abcdef", email, "password") # pylint: disable=protected-access - - assert resp.status_code == 400, 'Migrations are run, but creating an account with duplicate email succeeded!' - - +@ddt class AuthTestCase(ContentStoreTestCase): """Check that various permissions-related things work""" @@ -138,114 +104,6 @@ def check_page_get(self, url, expected): self.assertEqual(resp.status_code, expected) return resp - def test_public_pages_load(self): - """Make sure pages that don't require login load without error.""" - pages = ( - reverse('login'), - reverse('signup'), - ) - for page in pages: - print(u"Checking '{0}'".format(page)) - self.check_page_get(page, 200) - - def test_create_account_errors(self): - # No post data -- should fail - registration_url = reverse('user_api_registration') - resp = self.client.post(registration_url, {}) - self.assertEqual(resp.status_code, 400) - - def test_create_account(self): - self.create_account(self.username, self.email, self.pw) - self.activate_user(self.email) - - def test_create_account_username_already_exists(self): - User.objects.create_user(self.username, self.email, self.pw) - resp = self._create_account(self.username, "abc@def.com", "password") - # we have a constraint on unique usernames, so this should fail - self.assertEqual(resp.status_code, 409) - - def test_create_account_pw_already_exists(self): - User.objects.create_user(self.username, self.email, self.pw) - resp = self._create_account("abcdef", "abc@def.com", self.pw) - # we can have two users with the same password, so this should succeed - self.assertEqual(resp.status_code, 200) - - def test_login(self): - self.create_account(self.username, self.email, self.pw) - - # Not activated yet. Login should fail. - self._login(self.email, self.pw) - - self.activate_user(self.email) - - # Now login should work - self.login(self.email, self.pw) - - def test_login_ratelimited(self): - # try logging in 30 times, the default limit in the number of failed - # login attempts in one 5 minute period before the rate gets limited - for i in range(30): - resp = self._login(self.email, 'wrong_password{0}'.format(i)) - self.assertEqual(resp.status_code, 403) - resp = self._login(self.email, 'wrong_password') - self.assertContains(resp, 'Too many failed login attempts.', status_code=403) - - @override_settings(MAX_FAILED_LOGIN_ATTEMPTS_ALLOWED=3) - @override_settings(MAX_FAILED_LOGIN_ATTEMPTS_LOCKOUT_PERIOD_SECS=2) - def test_excessive_login_failures(self): - # try logging in 3 times, the account should get locked for 3 seconds - # note we want to keep the lockout time short, so we don't slow down the tests - - with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MAX_FAILED_LOGIN_ATTEMPTS': True}): - self.create_account(self.username, self.email, self.pw) - self.activate_user(self.email) - - for i in range(3): - resp = self._login(self.email, 'wrong_password{0}'.format(i)) - self.assertContains( - resp, - 'Email or password is incorrect.', - status_code=403, - ) - - # now the account should be locked - - resp = self._login(self.email, 'wrong_password') - self.assertContains( - resp, - 'This account has been temporarily locked due to excessive login failures.', - status_code=403, - ) - - with freeze_time('2100-01-01'): - self.login(self.email, self.pw) - - # make sure the failed attempt counter gets reset on successful login - resp = self._login(self.email, 'wrong_password') - self.assertContains( - resp, - 'Email or password is incorrect.', - status_code=403, - ) - - # account should not be locked out after just one attempt - self.login(self.email, self.pw) - - # do one more login when there is no bad login counter row at all in the database to - # test the "ObjectNotFound" case - self.login(self.email, self.pw) - - def test_login_link_on_activation_age(self): - self.create_account(self.username, self.email, self.pw) - # we want to test the rendering of the activation page when the user isn't logged in - self.client.logout() - resp = self._activate_user(self.email) - - # check the the HTML has links to the right login page. Note that this is merely a content - # check and thus could be fragile should the wording change on this page - expected = 'You can now sign in.' - self.assertContains(resp, expected) - def test_private_pages_auth(self): """Make sure pages that do require login work.""" auth_pages = ( @@ -259,7 +117,8 @@ def test_private_pages_auth(self): ) # need an activated user - self.test_create_account() + self.create_account(self.username, self.email, self.pw) + self.activate_user(self.email) # Create a new session self.client = AjaxEnabledTestClient() @@ -278,14 +137,6 @@ def test_private_pages_auth(self): print(u"Checking '{0}'".format(page)) self.check_page_get(page, expected=200) - def test_index_auth(self): - - # not logged in. Should return a redirect. - resp = self.client.get_html('/home/') - self.assertEqual(resp.status_code, 302) - - # Logged in should work. - @override_settings(SESSION_INACTIVITY_TIMEOUT_IN_SECONDS=1) def test_inactive_session_timeout(self): """ @@ -308,37 +159,30 @@ def test_inactive_session_timeout(self): resp = self.client.get_html(course_url) # re-request, and we should get a redirect to login page - self.assertRedirects(resp, settings.LOGIN_URL + '?next=/home/') + self.assertRedirects(resp, settings.LOGIN_URL + '?next=/home/', target_status_code=302) - @mock.patch.dict(settings.FEATURES, {"ALLOW_PUBLIC_ACCOUNT_CREATION": False}) - def test_signup_button_index_page(self): + @data( + (True, 'assertContains'), + (False, 'assertNotContains')) + @unpack + def test_signin_and_signup_buttons_index_page(self, allow_account_creation, assertion_method_name): """ Navigate to the home page and check the Sign Up button is hidden when ALLOW_PUBLIC_ACCOUNT_CREATION flag - is turned off - """ - response = self.client.get(reverse('homepage')) - self.assertNotContains(response, 'Sign Up') - - @mock.patch.dict(settings.FEATURES, {"ALLOW_PUBLIC_ACCOUNT_CREATION": False}) - def test_signup_button_login_page(self): - """ - Navigate to the login page and check the Sign Up button is hidden when ALLOW_PUBLIC_ACCOUNT_CREATION flag - is turned off - """ - response = self.client.get(reverse('login')) - self.assertNotContains(response, 'Sign Up') - - @mock.patch.dict(settings.FEATURES, {"ALLOW_PUBLIC_ACCOUNT_CREATION": False}) - def test_signup_link_login_page(self): - """ - Navigate to the login page and check the Sign Up link is hidden when ALLOW_PUBLIC_ACCOUNT_CREATION flag - is turned off + is turned off, and not when it is turned on. The Sign In button should always appear. """ - response = self.client.get(reverse('login')) - self.assertNotContains( - response, - 'Don't have a Studio Account? Sign up!' - ) + with mock.patch.dict(settings.FEATURES, {"ALLOW_PUBLIC_ACCOUNT_CREATION": allow_account_creation}): + response = self.client.get(reverse('homepage')) + assertion_method = getattr(self, assertion_method_name) + assertion_method( + response, + u'Sign Up'.format( # pylint: disable=line-too-long + settings.LMS_ROOT_URL + ) + ) + self.assertContains( + response, + u'Sign In' # pylint: disable=line-too-long + ) class ForumTestCase(CourseTestCase): diff --git a/cms/djangoapps/contentstore/views/public.py b/cms/djangoapps/contentstore/views/public.py index e9d5603cbb74..f0c81023de6d 100644 --- a/cms/djangoapps/contentstore/views/public.py +++ b/cms/djangoapps/contentstore/views/public.py @@ -5,48 +5,25 @@ from django.conf import settings from django.shortcuts import redirect -from django.template.context_processors import csrf from django.utils.http import urlquote_plus -from django.views.decorators.clickjacking import xframe_options_deny -from django.views.decorators.csrf import ensure_csrf_cookie from waffle.decorators import waffle_switch from contentstore.config import waffle from edxmako.shortcuts import render_to_response -from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers -__all__ = ['signup', 'login_page', 'login_redirect_to_lms', 'howitworks', 'accessibility'] +__all__ = ['register_redirect_to_lms', 'login_redirect_to_lms', 'howitworks', 'accessibility'] -@ensure_csrf_cookie -@xframe_options_deny -def signup(request): +def register_redirect_to_lms(request): """ - Display the signup form. + This view redirects to the LMS register view. It is used to temporarily keep the old + Studio signup url alive. """ - csrf_token = csrf(request)['csrf_token'] - if request.user.is_authenticated: - return redirect('/course/') - - return render_to_response('register.html', {'csrf': csrf_token}) - - -@ensure_csrf_cookie -@xframe_options_deny -def login_page(request): - """ - Display the login form. - """ - csrf_token = csrf(request)['csrf_token'] - - return render_to_response( - 'login.html', - { - 'csrf': csrf_token, - 'forgot_password_link': "//{base}/login#forgot-password-modal".format(base=settings.LMS_BASE), - 'platform_name': configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME), - } + register_url = '{register_url}{params}'.format( + register_url=settings.FRONTEND_REGISTER_URL, + params=_build_next_param(request), ) + return redirect(register_url, permanent=True) def login_redirect_to_lms(request): @@ -54,15 +31,25 @@ 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 '', + login_url = '{login_url}{params}'.format( + login_url=settings.FRONTEND_LOGIN_URL, + params=_build_next_param(request), ) return redirect(login_url) +def _build_next_param(request): + """ Returns the next param to be used with login or register. """ + next_url = request.GET.get('next') + next_url = next_url if next_url else settings.LOGIN_REDIRECT_URL + if next_url: + # Warning: do not use `build_absolute_uri` when `next_url` is empty because `build_absolute_uri` would + # build use the login url for the next url, which would cause a login redirect loop. + absolute_next_url = request.build_absolute_uri(next_url) + return '?next=' + urlquote_plus(absolute_next_url) + return '' + + def howitworks(request): "Proxy view" if request.user.is_authenticated: diff --git a/cms/djangoapps/maintenance/tests.py b/cms/djangoapps/maintenance/tests.py index 4135de466e91..d6719adbd679 100644 --- a/cms/djangoapps/maintenance/tests.py +++ b/cms/djangoapps/maintenance/tests.py @@ -92,11 +92,12 @@ def test_require_login(self, url): # Expect a redirect to the login page redirect_url = '{login_url}?next={original_url}'.format( - login_url=reverse('login'), + login_url=settings.LOGIN_URL, original_url=url, ) - self.assertRedirects(response, redirect_url) + # Studio login redirects to LMS login + self.assertRedirects(response, redirect_url, target_status_code=302) @ddt.data(*MAINTENANCE_URLS) def test_global_staff_access(self, url): diff --git a/cms/envs/common.py b/cms/envs/common.py index 1ef5653aeaaf..cd622cceb09d 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -475,8 +475,6 @@ ############################################################################## EDX_ROOT_URL = '' -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 = [ @@ -496,13 +494,21 @@ LMS_BASE = 'localhost:18000' LMS_ROOT_URL = "https://localhost:18000" LMS_INTERNAL_ROOT_URL = LMS_ROOT_URL + +LOGIN_REDIRECT_URL = EDX_ROOT_URL + '/home/' +# TODO: Determine if LOGIN_URL could be set to the FRONTEND_LOGIN_URL value instead. +LOGIN_URL = reverse_lazy('login_redirect_to_lms') +FRONTEND_LOGIN_URL = lambda settings: settings.LMS_ROOT_URL + '/login' +derived('FRONTEND_LOGIN_URL') +FRONTEND_LOGOUT_URL = lambda settings: settings.LMS_ROOT_URL + '/logout' +derived('FRONTEND_LOGOUT_URL') +FRONTEND_REGISTER_URL = lambda settings: settings.LMS_ROOT_URL + '/register' +derived('FRONTEND_REGISTER_URL') + LMS_ENROLLMENT_API_PATH = "/api/enrollment/v1/" ENTERPRISE_API_URL = LMS_INTERNAL_ROOT_URL + '/enterprise/api/v1/' ENTERPRISE_CONSENT_API_URL = LMS_INTERNAL_ROOT_URL + '/consent/api/v1/' ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS = {} -FRONTEND_LOGIN_URL = LOGIN_URL -FRONTEND_LOGOUT_URL = lambda settings: settings.LMS_ROOT_URL + '/logout' -derived('FRONTEND_LOGOUT_URL') # Public domain name of Studio (should be resolvable from the end-user's browser) CMS_BASE = 'localhost:18010' @@ -2122,3 +2128,44 @@ 'country': 'hidden', } EDXAPP_PARSE_KEYS = {} + +###################### DEPRECATED URLS ########################## + +# .. toggle_name: DISABLE_DEPRECATED_SIGNIN_URL +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: Toggle for removing the deprecated /signin url. +# .. toggle_category: n/a +# .. toggle_use_cases: incremental_release +# .. toggle_creation_date: 2019-12-02 +# .. toggle_expiration_date: 2020-06-01 +# .. toggle_warnings: This url can be removed once it no longer has any real traffic. +# .. toggle_tickets: ARCH-1253 +# .. toggle_status: supported +DISABLE_DEPRECATED_SIGNIN_URL = False + +# .. toggle_name: DISABLE_DEPRECATED_SIGNUP_URL +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: Toggle for removing the deprecated /signup url. +# .. toggle_category: n/a +# .. toggle_use_cases: incremental_release +# .. toggle_creation_date: 2019-12-02 +# .. toggle_expiration_date: 2020-06-01 +# .. toggle_warnings: This url can be removed once it no longer has any real traffic. +# .. toggle_tickets: ARCH-1253 +# .. toggle_status: supported +DISABLE_DEPRECATED_SIGNUP_URL = False + +# .. toggle_name: DISABLE_DEPRECATED_LOGIN_POST +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: Toggle for removing the deprecated /login_post url. +# .. toggle_category: n/a +# .. toggle_use_cases: incremental_release +# .. toggle_creation_date: 2019-12-02 +# .. toggle_expiration_date: 2020-06-01 +# .. toggle_warnings: This url can be removed once it no longer has any real traffic. Note: We have permission to remove for traffic from user_agent including `mitx-quantum`. +# .. toggle_tickets: ARCH-1253 +# .. toggle_status: supported +DISABLE_DEPRECATED_LOGIN_POST = False diff --git a/cms/envs/production.py b/cms/envs/production.py index 66e4658e7e66..afa19de42384 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -308,13 +308,6 @@ def get_env_setting(setting): HEARTBEAT_EXTENDED_CHECKS = ENV_TOKENS.get('HEARTBEAT_EXTENDED_CHECKS', HEARTBEAT_EXTENDED_CHECKS) HEARTBEAT_CELERY_TIMEOUT = ENV_TOKENS.get('HEARTBEAT_CELERY_TIMEOUT', HEARTBEAT_CELERY_TIMEOUT) -# Login using the LMS as the identity provider. -# Turning the flag to True means that the LMS will NOT be used as the Identity Provider (idp) -if FEATURES.get('DISABLE_STUDIO_SSO_OVER_LMS', False): - LOGIN_URL = reverse_lazy('login') - FRONTEND_LOGIN_URL = LOGIN_URL - FRONTEND_LOGOUT_URL = reverse_lazy('logout') - LOGIN_REDIRECT_WHITELIST = [reverse_lazy('home')] # Specific setting for the File Upload Service to store media in a bucket. diff --git a/cms/envs/test.py b/cms/envs/test.py index 35bc42324c98..012b5a06cab9 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -19,7 +19,6 @@ import os from uuid import uuid4 - from django.utils.translation import ugettext_lazy from path import Path as path @@ -142,8 +141,6 @@ LMS_BASE = "localhost:8000" LMS_ROOT_URL = "http://{}".format(LMS_BASE) FEATURES['PREVIEW_LMS_BASE'] = "preview.localhost" -LOGIN_URL = EDX_ROOT_URL + '/signin' - CACHES = { # This is the cache used for most things. Askbot will not work without a diff --git a/cms/static/cms/js/build.js b/cms/static/cms/js/build.js index 10dec34c5a09..1abe9f7b14c3 100644 --- a/cms/static/cms/js/build.js +++ b/cms/static/cms/js/build.js @@ -27,7 +27,6 @@ 'js/factories/index', 'js/factories/manage_users', 'js/factories/outline', - 'js/factories/register', 'js/factories/settings', 'js/factories/settings_advanced', 'js/factories/settings_graders', diff --git a/cms/static/cms/js/spec/main_webpack.js b/cms/static/cms/js/spec/main_webpack.js index 5cea9bfd0a87..3ca488a8db4a 100644 --- a/cms/static/cms/js/spec/main_webpack.js +++ b/cms/static/cms/js/spec/main_webpack.js @@ -22,7 +22,6 @@ window.edx.StringUtils = StringUtils; import './xblock/cms.runtime.v1_spec.js'; import '../../../js/spec/factories/xblock_validation_spec.js'; import '../../../js/spec/views/container_spec.js'; -import '../../../js/spec/views/login_studio_spec.js'; import '../../../js/spec/views/modals/edit_xblock_spec.js'; import '../../../js/spec/views/module_edit_spec.js'; import '../../../js/spec/views/move_xblock_spec.js'; diff --git a/cms/static/js/factories/login.js b/cms/static/js/factories/login.js deleted file mode 100644 index b528e075a2ce..000000000000 --- a/cms/static/js/factories/login.js +++ /dev/null @@ -1,63 +0,0 @@ - -'use strict'; - -import cookie from 'jquery.cookie'; -import utility from 'utility'; -import ViewUtils from 'common/js/components/utils/view_utils'; - -export default function LoginFactory(homepageURL) { - function postJSON(url, data, callback) { - $.ajax({ - type: 'POST', - url: url, - dataType: 'json', - data: data, - success: callback - }); - } - - // Clear the login error message when credentials are edited - $('input#email').on('input', function () { - $('#login_error').removeClass('is-shown'); - }); - - $('input#password').on('input', function () { - $('#login_error').removeClass('is-shown'); - }); - - $('form#login_form').submit(function (event) { - event.preventDefault(); - var $submitButton = $('#submit'), - deferred = new $.Deferred(), - promise = deferred.promise(); - ViewUtils.disableElementWhileRunning($submitButton, function () { return promise; }); - var submit_data = $('#login_form').serialize(); - - postJSON('/login_post', submit_data, function (json) { - if (json.success) { - var next = /next=([^&]*)/g.exec(decodeURIComponent(window.location.search)); - if (next && next.length > 1 && !isExternal(next[1])) { - ViewUtils.redirect(next[1]); - } else { - ViewUtils.redirect(homepageURL); - } - } else if ($('#login_error').length === 0) { - $('#login_form').prepend( - '
' + - json.value + - '
' - ); - $('#login_error').addClass('is-shown'); - deferred.resolve(); - } else { - $('#login_error') - .stop() - .addClass('is-shown') - .html(json.value); - deferred.resolve(); - } - }); - }); -}; - -export { LoginFactory } diff --git a/cms/static/js/factories/register.js b/cms/static/js/factories/register.js deleted file mode 100644 index cb016ac17041..000000000000 --- a/cms/static/js/factories/register.js +++ /dev/null @@ -1,59 +0,0 @@ -define(['jquery', 'jquery.cookie'], function($) { - 'use strict'; - return function() { - $('form :input') - .focus(function() { - $('label[for="' + this.id + '"]').addClass('is-focused'); - }) - .blur(function() { - $('label').removeClass('is-focused'); - }); - - $('form#register_form').submit(function(event) { - event.preventDefault(); - var submit_data = $('#register_form').serialize(); - - $.ajax({ - url: '/create_account', - type: 'POST', - dataType: 'json', - headers: {'X-CSRFToken': $.cookie('csrftoken')}, - notifyOnError: false, - data: submit_data, - success: function(json) { - location.href = '/course/'; - }, - error: function(jqXHR, textStatus, errorThrown) { - var json = $.parseJSON(jqXHR.responseText); - $('#register_error').html(json.value).stop().addClass('is-shown'); - } - }); - }); - - $('input#password').blur(function() { - var $formErrors = $('#password_error'), - data = { - password: $('#password').val() - }; - - // Uninitialize the errors on blur - $formErrors.empty(); - $formErrors.addClass('hidden'); - - $.ajax({ - url: '/api/user/v1/validation/registration', - type: 'POST', - dataType: 'json', - data: data, - success: function(json) { - _.each(json.validation_decisions, function(value, key) { - if (key === 'password' && value) { - $formErrors.html(value); - $formErrors.removeClass('hidden'); - } - }); - } - }); - }); - }; -}); diff --git a/cms/static/js/spec/views/login_studio_spec.js b/cms/static/js/spec/views/login_studio_spec.js deleted file mode 100644 index 38a01dc311aa..000000000000 --- a/cms/static/js/spec/views/login_studio_spec.js +++ /dev/null @@ -1,35 +0,0 @@ - -'use strict'; - -import $ from 'jquery'; -import LoginFactory from 'js/factories/login'; -import AjaxHelpers from 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers'; -import ViewUtils from 'common/js/components/utils/view_utils'; - -describe('Studio Login Page', () => { - var $submitButton; - - beforeEach(function() { - loadFixtures('mock/login.underscore'); - var login_factory = LoginFactory('/home/'); - $submitButton = $('#submit'); - }); - - it('disable the submit button once it is clicked', function() { - spyOn(ViewUtils, 'redirect').and.callFake(function() {}); - var requests = AjaxHelpers.requests(this); - expect($submitButton).not.toHaveClass('is-disabled'); - $submitButton.click(); - AjaxHelpers.respondWithJson(requests, {success: true}); - expect($submitButton).toHaveClass('is-disabled'); - }); - - it('It will not disable the submit button if there are errors in ajax request', function() { - var requests = AjaxHelpers.requests(this); - expect($submitButton).not.toHaveClass('is-disabled'); - $submitButton.click(); - expect($submitButton).toHaveClass('is-disabled'); - AjaxHelpers.respondWithError(requests, {}); - expect($submitButton).not.toHaveClass('is-disabled'); - }); -}); diff --git a/cms/templates/activation_active.html b/cms/templates/activation_active.html deleted file mode 100644 index a6e75ecdf816..000000000000 --- a/cms/templates/activation_active.html +++ /dev/null @@ -1,37 +0,0 @@ -<%page expression_filter="h"/> -<%! from django.utils.translation import ugettext as _ %> -<%inherit file="base.html" /> - -<%block name="content"> -
-
-

- ${_("{studio_name} Account Activation").format(studio_name=settings.STUDIO_SHORT_NAME)} -

-
-
- -
-
-
-
- -
-
-

${_("Your account is already active")}

-
-

${_("This account, set up using {email}, has already been activated. Please sign in to start working within {studio_name}.".format(email=user.email, studio_name=settings.STUDIO_NAME))}

-
-
- - -
-
-
- diff --git a/cms/templates/activation_complete.html b/cms/templates/activation_complete.html deleted file mode 100644 index fa39f13ac8e4..000000000000 --- a/cms/templates/activation_complete.html +++ /dev/null @@ -1,48 +0,0 @@ -<%! -from openedx.core.djangolib.markup import Text -from django.utils.translation import ugettext as _ -%> -<%page expression_filter="h"/> -<%inherit file="base.html" /> - -<%block name="content"> -
-
-

- ${Text(_("{studio_name} Account Activation")).format( - studio_name=Text(settings.STUDIO_SHORT_NAME), - )} -

-
-
- -
-
-
-
- -
-
-

${_("Your account activation is complete!")}

-
-

- ${Text(_("Thank you for activating your account. You may now sign in and start using {studio_name} to author courses.")).format( - studio_name=Text(settings.STUDIO_NAME) - )} -

-
-
- - -
-
-
- diff --git a/cms/templates/activation_invalid.html b/cms/templates/activation_invalid.html deleted file mode 100644 index 66c5e06c9d5e..000000000000 --- a/cms/templates/activation_invalid.html +++ /dev/null @@ -1,42 +0,0 @@ -<%! -from openedx.core.djangolib.markup import HTML, Text -from django.utils.translation import ugettext as _ -%> -<%page expression_filter="h"/> -<%inherit file="base.html" /> - -<%block name="content"> -
-
-

- ${Text(_("{studio_name} Account Activation")).format( - studio_name=Text(settings.STUDIO_SHORT_NAME) - )} -

-
-
- -
-
-
-
- -
-
-

${_("Your account activation is invalid")}

-
-

${_("We're sorry. Something went wrong with your activation. Check to make sure the URL you went to was correct, as e-mail programs will sometimes split it into two lines.")}

-

- ${Text(_("If you still have issues, contact {platform_name} Support. In the meantime, you can also return to {link_start}the {studio_name} homepage.{link_end}")).format( - platform_name=Text(settings.PLATFORM_NAME), - studio_name=Text(settings.STUDIO_NAME), - link_start=HTML(''), - link_end=HTML('') - )} -

-
-
-
-
-
- diff --git a/cms/templates/howitworks.html b/cms/templates/howitworks.html index 0e0349a755a9..7e46eda0604f 100644 --- a/cms/templates/howitworks.html +++ b/cms/templates/howitworks.html @@ -3,7 +3,7 @@ <%def name="online_help_token()"><% return "welcome" %> <%namespace name='static' file='static_content.html'/> <%! - from django.urls import reverse + from django.conf import settings from django.utils.translation import ugettext as _ from openedx.core.djangolib.markup import HTML, Text %> @@ -161,10 +161,10 @@

${_("Sign Up for {studio_name} Today!").format(studio_name=settin diff --git a/cms/templates/js/mock/login.underscore b/cms/templates/js/mock/login.underscore deleted file mode 100644 index 8d07f6db6fd6..000000000000 --- a/cms/templates/js/mock/login.underscore +++ /dev/null @@ -1,12 +0,0 @@ -
-
- - - - -
- -
- -
-
diff --git a/cms/templates/login.html b/cms/templates/login.html deleted file mode 100644 index 3b2cdc94322a..000000000000 --- a/cms/templates/login.html +++ /dev/null @@ -1,61 +0,0 @@ -<%namespace name='static' file='/static_content.html'/> -<%page expression_filter="h"/> -<%inherit file="base.html" /> -<%def name="online_help_token()"><% return "login" %> -<%! -from django.urls import reverse -from django.utils.translation import ugettext as _ -from openedx.core.djangolib.js_utils import js_escaped_string -%> -<%block name="title">${_("Sign In")} -<%block name="bodyclass">not-signedin view-signin - -<%block name="content"> - -
-
-
-

${_("Sign In to {studio_name}").format(studio_name=settings.STUDIO_NAME)}

- % if static.get_value('ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION')): - - % endif -
- -
-
- -
- ${_("Required Information to Sign In to {studio_name}").format(studio_name=settings.STUDIO_NAME)} - - -
    -
  1. - - -
    2. - -
  2. - - - ${_("Forgot password?")} -
    3. -
-
- -
- -
- - - -
-
-
-
- - -<%block name="page_bundle"> - <%static:webpack entry="js/factories/login"> - LoginFactory("${reverse('homepage') | n, js_escaped_string}"); - - diff --git a/cms/templates/register.html b/cms/templates/register.html deleted file mode 100644 index 3af9949bb61b..000000000000 --- a/cms/templates/register.html +++ /dev/null @@ -1,116 +0,0 @@ -<%inherit file="base.html" /> -<%def name="online_help_token()"><% return "register" %> -<%! -from django.utils.translation import ugettext as _ -from django.urls import reverse -%> - -<%block name="title">${_("Sign Up")} -<%block name="bodyclass">not-signedin view-signup - -<%block name="content"> - -
-
-
-

${_("Sign Up for {studio_name}").format(studio_name=settings.STUDIO_NAME)}

- -
- -

${_("Ready to start creating online courses? Sign up below and start creating your first {platform_name} course today.").format(platform_name=settings.PLATFORM_NAME)}

- -
-
-
-
- -
- ${_("Required Information to Sign Up for {studio_name}").format(studio_name=settings.STUDIO_NAME)} - -
    -
  1. - - ## Translators: This is the placeholder text for a field that requests an email address. - -
    2. - -
  2. - - ## Translators: This is the placeholder text for a field that requests the user's full name. - -
    3. - -
  3. - - ## Translators: This is the placeholder text for a field that asks the user to pick a username - - ${_("This will be used in public discussions with your courses and in our edX101 support forums")} -
    4. - -
  4. - - - -
    5. - -
  5. -
    - - -
    - -
    - - -
    -
    6. - -
  6. - - -
    7. -
-
- -
- -
- - - -
-
- - -
-
- - -<%block name="requirejs"> - require(["js/factories/register"], function (RegisterFactory) { - RegisterFactory(); - }); - diff --git a/cms/templates/registration/activation_complete.html b/cms/templates/registration/activation_complete.html index 86592dded3b6..0c5364c96947 100644 --- a/cms/templates/registration/activation_complete.html +++ b/cms/templates/registration/activation_complete.html @@ -1,7 +1,9 @@ +<%page expression_filter="h"/> <%inherit file="../base.html" /> <%! +from django.conf import settings from django.utils.translation import ugettext as _ -from django.urls import reverse +from openedx.core.djangolib.markup import HTML, Text %> <%namespace name='static' file='../static_content.html'/> @@ -23,9 +25,14 @@ %endif %if user_logged_in: - ${_("Visit your {link_start}dashboard{link_end} to see your courses.").format(link_start='', link_end='')} + ${Text(_("Visit your {link_start}dashboard{link_end} to see your courses.")).format( + link_start=HTML(''), + link_end=HTML('') + )} %else: - ${_("You can now {link_start}sign in{link_end}.").format(link_start=''.format(url=reverse('login')), link_end='')} + ${Text(_("You can now {link_start}sign in{link_end}.")).format( + link_start=HTML('').format(url=settings.LOGIN_URL, link_end=HTML('')) + )} %endif

diff --git a/cms/templates/registration/reg_complete.html b/cms/templates/registration/reg_complete.html deleted file mode 100644 index 6183868965d4..000000000000 --- a/cms/templates/registration/reg_complete.html +++ /dev/null @@ -1,3 +0,0 @@ -<%! from django.utils.translation import ugettext as _ %> -

Check your email

-

${_("We've sent an email message to {email} with instructions for activating your account.").format(email=email)}

diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index 7e81bb5ebe9d..f33497c0534f 100644 --- a/cms/templates/widgets/header.html +++ b/cms/templates/widgets/header.html @@ -237,9 +237,6 @@

${_("Account Navigation")}

    @@ -248,11 +245,11 @@

    ${_("Account Navigation")}

    % if static.get_value('ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION')):     % endif
diff --git a/cms/templates/widgets/user_dropdown.html b/cms/templates/widgets/user_dropdown.html index a59fc3b75bd5..82c146eb7587 100644 --- a/cms/templates/widgets/user_dropdown.html +++ b/cms/templates/widgets/user_dropdown.html @@ -4,10 +4,16 @@ from django.conf import settings from django.urls import reverse from django.utils.translation import ugettext as _ + from edx_django_utils.monitoring import set_custom_metric from student.roles import GlobalStaff %> % if uses_pattern_library: + <%! + ## TODO: Use metric to see if CMS ever uses pattern library or if this case can be deleted. + ## NOTE: When removing, remove all references to `set_custom_metric`. + set_custom_metric('uses_pattern_library', True) + %>

${_("Currently signed in as:")} @@ -26,12 +32,15 @@

  • - ${_("Sign Out")} + ${_("Sign Out")}

    • % else: + <%! + set_custom_metric('uses_pattern_library', False) + %>

    ${_("Currently signed in as:")} diff --git a/cms/urls.py b/cms/urls.py index bb0108d82103..089a49f0a629 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -86,8 +86,6 @@ # restful api url(r'^$', contentstore.views.howitworks, name='homepage'), 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), @@ -180,6 +178,18 @@ url(r'^accessibility$', contentstore.views.accessibility, name='accessibility'), ] +if not settings.DISABLE_DEPRECATED_SIGNIN_URL: + # TODO: Remove deprecated signin url when traffic proves it is no longer in use + urlpatterns += [ + url(r'^signin$', contentstore.views.login_redirect_to_lms), + ] + +if not settings.DISABLE_DEPRECATED_SIGNUP_URL: + # TODO: Remove deprecated signup url when traffic proves it is no longer in use + urlpatterns += [ + url(r'^signup$', contentstore.views.register_redirect_to_lms, name='register_redirect_to_lms'), + ] + JS_INFO_DICT = { 'domain': 'djangojs', # We need to explicitly include external Django apps that are not in LOCALE_PATHS. diff --git a/common/djangoapps/edxmako/tests.py b/common/djangoapps/edxmako/tests.py index 7a270ae4b8f7..2a02af33d010 100644 --- a/common/djangoapps/edxmako/tests.py +++ b/common/djangoapps/edxmako/tests.py @@ -4,11 +4,11 @@ import ddt from django.conf import settings -from django.urls import reverse from django.http import HttpResponse from django.test import TestCase from django.test.client import RequestFactory from django.test.utils import override_settings +from django.urls import reverse from edx_django_utils.cache import RequestCache from mock import Mock, patch @@ -25,45 +25,52 @@ class ShortcutsTests(UrlResetMixin, TestCase): Test the edxmako shortcuts file """ @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'}) - @override_settings(MKTG_URL_LINK_MAP={'ABOUT': 'login'}) def test_marketing_link(self): - # test marketing site on - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - expected_link = 'https://dummy-root/about-us' - link = marketing_link('ABOUT') - self.assertEquals(link, expected_link) - # test marketing site off - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): - # we are using login because it is common across both cms and lms - expected_link = reverse('login') - link = marketing_link('ABOUT') - self.assertEquals(link, expected_link) + with override_settings(MKTG_URL_LINK_MAP={'ABOUT': self._get_test_url_name()}): + # test marketing site on + with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): + expected_link = 'https://dummy-root/about-us' + link = marketing_link('ABOUT') + self.assertEquals(link, expected_link) + # test marketing site off + with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): + expected_link = reverse(self._get_test_url_name()) + link = marketing_link('ABOUT') + self.assertEquals(link, expected_link) @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'}) - @override_settings(MKTG_URL_LINK_MAP={'ABOUT': 'login'}) def test_is_marketing_link_set(self): - # test marketing site on - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - self.assertTrue(is_marketing_link_set('ABOUT')) - self.assertFalse(is_marketing_link_set('NOT_CONFIGURED')) - # test marketing site off - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): - self.assertTrue(is_marketing_link_set('ABOUT')) - self.assertFalse(is_marketing_link_set('NOT_CONFIGURED')) + with override_settings(MKTG_URL_LINK_MAP={'ABOUT': self._get_test_url_name()}): + # test marketing site on + with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): + self.assertTrue(is_marketing_link_set('ABOUT')) + self.assertFalse(is_marketing_link_set('NOT_CONFIGURED')) + # test marketing site off + with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): + self.assertTrue(is_marketing_link_set('ABOUT')) + self.assertFalse(is_marketing_link_set('NOT_CONFIGURED')) @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'}) - @override_settings(MKTG_URL_LINK_MAP={'ABOUT': 'login'}) def test_is_any_marketing_link_set(self): - # test marketing site on - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - self.assertTrue(is_any_marketing_link_set(['ABOUT'])) - self.assertTrue(is_any_marketing_link_set(['ABOUT', 'NOT_CONFIGURED'])) - self.assertFalse(is_any_marketing_link_set(['NOT_CONFIGURED'])) - # test marketing site off - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): - self.assertTrue(is_any_marketing_link_set(['ABOUT'])) - self.assertTrue(is_any_marketing_link_set(['ABOUT', 'NOT_CONFIGURED'])) - self.assertFalse(is_any_marketing_link_set(['NOT_CONFIGURED'])) + with override_settings(MKTG_URL_LINK_MAP={'ABOUT': self._get_test_url_name()}): + # test marketing site on + with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): + self.assertTrue(is_any_marketing_link_set(['ABOUT'])) + self.assertTrue(is_any_marketing_link_set(['ABOUT', 'NOT_CONFIGURED'])) + self.assertFalse(is_any_marketing_link_set(['NOT_CONFIGURED'])) + # test marketing site off + with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): + self.assertTrue(is_any_marketing_link_set(['ABOUT'])) + self.assertTrue(is_any_marketing_link_set(['ABOUT', 'NOT_CONFIGURED'])) + self.assertFalse(is_any_marketing_link_set(['NOT_CONFIGURED'])) + + def _get_test_url_name(self): + if settings.ROOT_URLCONF == 'lms.urls': + # return any lms url name + return 'dashboard' + else: + # return any cms url name + return 'organizations' class AddLookupTests(TestCase): diff --git a/common/djangoapps/student/tests/test_helpers.py b/common/djangoapps/student/tests/test_helpers.py index 07ca1d319b15..83d6911f03e2 100644 --- a/common/djangoapps/student/tests/test_helpers.py +++ b/common/djangoapps/student/tests/test_helpers.py @@ -10,7 +10,6 @@ from django.test import TestCase from django.test.client import RequestFactory from django.test.utils import override_settings -from django.urls import reverse from mock import patch from testfixtures import LogCapture @@ -57,7 +56,7 @@ def _add_session(request): def test_next_failures(self, log_level, log_name, unsafe_url, http_accept, user_agent, expected_log): """ Test unsafe next parameter """ with LogCapture(LOGGER_NAME, level=log_level) as logger: - req = self.request.get(reverse("login") + "?next={url}".format(url=unsafe_url)) + req = self.request.get(settings.LOGIN_URL + "?next={url}".format(url=unsafe_url)) req.META["HTTP_ACCEPT"] = http_accept # pylint: disable=no-member req.META["HTTP_USER_AGENT"] = user_agent # pylint: disable=no-member get_next_url_for_login_page(req) @@ -75,7 +74,7 @@ def test_next_failures(self, log_level, log_name, unsafe_url, http_accept, user_ @override_settings(LOGIN_REDIRECT_WHITELIST=['test.edx.org', 'test2.edx.org']) def test_safe_next(self, next_url, host): """ Test safe next parameter """ - req = self.request.get(reverse("login") + "?next={url}".format(url=next_url), HTTP_HOST=host) + req = self.request.get(settings.LOGIN_URL + "?next={url}".format(url=next_url), HTTP_HOST=host) req.META["HTTP_ACCEPT"] = "text/html" # pylint: disable=no-member next_page = get_next_url_for_login_page(req) self.assertEqual(next_page, next_url) @@ -103,7 +102,7 @@ def test_third_party_auth_hint(self, tpa_hint, next_url, expected_url, running_p mock_running_pipeline.return_value = running_pipeline def validate_login(): - req = self.request.get(reverse("login") + "?next={url}".format(url=next_url)) + req = self.request.get(settings.LOGIN_URL + "?next={url}".format(url=next_url)) req.META["HTTP_ACCEPT"] = "text/html" # pylint: disable=no-member self._add_session(req) next_page = get_next_url_for_login_page(req) diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 26cd92422251..131a614b1bea 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -14,10 +14,7 @@ from django.contrib import messages from django.contrib.auth.decorators import login_required from django.contrib.auth.models import AnonymousUser, User -from django.contrib.auth.views import password_reset_confirm from django.contrib.sites.models import Site -from django.core import mail -from django.core.exceptions import ObjectDoesNotExist from django.core.validators import ValidationError, validate_email from django.db import transaction from django.db.models.signals import post_save @@ -25,10 +22,7 @@ from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden from django.shortcuts import redirect from django.template.context_processors import csrf -from django.template.response import TemplateResponse from django.urls import reverse -from django.utils.encoding import force_bytes, force_text -from django.utils.http import base36_to_int, urlsafe_base64_encode from django.utils.translation import ugettext as _ from django.views.decorators.csrf import csrf_exempt, ensure_csrf_cookie from django.views.decorators.http import require_GET, require_http_methods, require_POST @@ -53,16 +47,11 @@ from openedx.core.djangoapps.catalog.utils import get_programs_with_type from openedx.core.djangoapps.embargo import api as embargo_api from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY -from openedx.core.djangoapps.oauth_dispatch.api import destroy_oauth_tokens from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming import helpers as theming_helpers -from openedx.core.djangoapps.theming.helpers import get_current_site -from openedx.core.djangoapps.user_api.accounts.utils import is_secondary_email_feature_enabled from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle -from openedx.core.djangoapps.user_api.models import UserRetirementRequest from openedx.core.djangoapps.user_api.preferences import api as preferences_api -from openedx.core.djangoapps.user_authn.message_types import PasswordReset from openedx.core.djangolib.markup import HTML, Text from student.helpers import DISABLE_UNENROLL_CERT_STATES, cert_info, generate_activation_email_context from student.message_types import AccountActivation, EmailChange, EmailChangeConfirmation, RecoveryEmailCreate @@ -83,10 +72,8 @@ from student.signals import REFUND_ORDER from student.tasks import send_activation_email from student.text_me_the_app import TextMeTheAppFragmentView -from util.request_rate_limiter import BadRequestRateLimiter, PasswordResetEmailRateLimiter from util.db import outer_atomic from util.json_request import JsonResponse -from util.password_policy_validators import normalize_password, validate_password from xmodule.modulestore.django import modulestore log = logging.getLogger("edx.student") @@ -519,8 +506,12 @@ def activate_account(request, key): """ # If request is in Studio call the appropriate view if theming_helpers.get_project_root_name().lower() == u'cms': + monitoring_utils.set_custom_metric('student_activate_account', 'cms') return activate_account_studio(request, key) + # TODO: Use metric to determine if there are any `activate_account` calls for cms in Production. + # If not, the templates wouldn't be needed for cms, but we still need a way to activate for cms tests. + monitoring_utils.set_custom_metric('student_activate_account', 'lms') try: registration = Registration.objects.get(activation_key=key) except (Registration.DoesNotExist, Registration.MultipleObjectsReturned): diff --git a/common/djangoapps/third_party_auth/tests/specs/test_google.py b/common/djangoapps/third_party_auth/tests/specs/test_google.py index fa6ab9beb126..89bab97df769 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_google.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_google.py @@ -106,7 +106,7 @@ def fake_auth_complete(inst, *args, **kwargs): # Now our custom registration form creates or logs in the user: email, password = data_parsed['user_details']['email'], 'random_password' created_user = UserFactory(email=email, password=password) - login_response = self.client.post(reverse('login'), {'email': email, 'password': password}) + login_response = self.client.post(reverse('login_api'), {'email': email, 'password': password}) self.assertEqual(login_response.status_code, 200) # Now our custom login/registration page must resume the pipeline: diff --git a/common/test/acceptance/tests/studio/test_studio_general.py b/common/test/acceptance/tests/studio/test_studio_general.py index eaeda52cafad..ac36517c75dc 100644 --- a/common/test/acceptance/tests/studio/test_studio_general.py +++ b/common/test/acceptance/tests/studio/test_studio_general.py @@ -157,8 +157,8 @@ def test_login_with_valid_redirect(self): Given I have opened a new course in Studio And I am not logged in And I visit the url "/course/slashes:MITx+999+Robot_Super_Course" - And I should see that the path is "/signin?next=/course/slashes%3AMITx%2B999%2BRobot_Super_Course" - When I fill in and submit the signin form + And I should see the path is "/signin_redirect_to_lms?next=/course/slashes%3AMITx%2B999%2BRobot_Super_Course" + When I fill in and submit the LMS login form Then I should see that the path is "/course/slashes:MITx+999+Robot_Super_Course" """ self.install_course_fixture() @@ -171,65 +171,6 @@ def test_login_with_valid_redirect(self): # Verify that correct course is displayed after sign in. self.assertEqual(self.browser.current_url, course_url) - def test_login_with_invalid_redirect(self): - """ - Scenario: Login with an invalid redirect - Given I have opened a new course in Studio - And I am not logged in - And I visit the url "/signin?next=http://www.google.com/" - When I fill in and submit the signin form - Then I should see that the path is "/home/" - """ - self.install_course_fixture() - # Visit course - self.course_outline_sign_in_redirect_page.visit() - # Change redirect url - 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']) - # 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): - """ - Given I have opened a new course in Studio - And I am not logged in - And I visit the Studio homepage - When I click the link with the text "Sign In" - Then I should see that the path is "/signin" - And I should not see a login error message - And I fill in and submit the signin form incorrectly - Then I should see a login error message - And I edit the password field - Then I should not see a login error message - And I submit the signin form - And I wait for "2" seconds - Then I should see that the path is "/course/slashes:MITx+999+Robot_Super_Course" - """ - self.install_course_fixture() - self.course_outline_sign_in_redirect_page.visit() - # Verify login_error is not present - self.course_outline_sign_in_redirect_page.wait_for_element_absence( - '#login_error', - 'Login error not be present' - ) - # Login with wrong credentials - self.course_outline_sign_in_redirect_page.login( - self.user['email'], - 'wrong_password', - expect_success=False - ) - # Verify that login error is shown - self.course_outline_sign_in_redirect_page.wait_for_element_visibility( - ".js-form-errors.status.submission-error", - 'Login error is 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() - # Verify that correct course is displayed after sign in. - self.assertEqual(self.browser.current_url, self.course_outline_page.url) - class CoursePagesTest(StudioCourseTest): """ diff --git a/common/test/test-theme/cms/templates/login.html b/common/test/test-theme/cms/templates/login.html deleted file mode 100644 index e1bc7cc81648..000000000000 --- a/common/test/test-theme/cms/templates/login.html +++ /dev/null @@ -1,59 +0,0 @@ -<%namespace name='static' file='/static_content.html'/> -<%page expression_filter="h"/> - -<%inherit file="base.html" /> -<%def name="online_help_token()"><% return "login" %> -<%! -from django.urls import reverse -from django.utils.translation import ugettext as _ -from openedx.core.djangolib.js_utils import js_escaped_string -%> -<%block name="title">${_("Sign In")} -<%block name="bodyclass">not-signedin view-signin - -<%block name="content"> -
    -
    -
    -

    ${_("Sign In to {studio_name}").format(studio_name=settings.STUDIO_NAME)}

    - -
    - -
    -
    - -
    - ${_("Required Information to Sign In to {studio_name}").format(studio_name=settings.STUDIO_NAME)} - - -
      -
    1. - - -
      2. - -
    2. - - - ${_("Forgot password?")} -
      3. -
    -
    - -
    - -
    - - - -
    -
    -
    -
    - - -<%block name="page_bundle"> - <%static:webpack entry="js/factories/login"> - LoginFactory("${reverse('homepage') | n, js_escaped_string}"); - - diff --git a/lms/envs/test.py b/lms/envs/test.py index a2fed948e611..812e270c9e55 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -506,7 +506,10 @@ LMS_ROOT_URL = "http://localhost:8000" -FRONTEND_LOGOUT_URL = LMS_ROOT_URL + '/logout' +# Needed for derived settings used by cms only. +FRONTEND_LOGIN_URL = '/login' +FRONTEND_LOGOUT_URL = '/logout' +FRONTEND_REGISTER_URL = '/register' ECOMMERCE_API_URL = 'https://ecommerce.example.com/api/v2/' ECOMMERCE_PUBLIC_URL_ROOT = None diff --git a/openedx/core/djangoapps/cache_toolbox/tests/test_middleware.py b/openedx/core/djangoapps/cache_toolbox/tests/test_middleware.py index 1c169107c8a9..8f7869b27e07 100644 --- a/openedx/core/djangoapps/cache_toolbox/tests/test_middleware.py +++ b/openedx/core/djangoapps/cache_toolbox/tests/test_middleware.py @@ -1,6 +1,7 @@ """Tests for cached authentication middleware.""" from __future__ import absolute_import +from django.conf import settings from django.contrib.auth.models import User from django.urls import reverse from django.test import TestCase @@ -19,7 +20,7 @@ def setUp(self): self.user = UserFactory(password=password) self.client.login(username=self.user.username, password=password) - def _test_change_session_hash(self, test_url, redirect_url): + def _test_change_session_hash(self, test_url, redirect_url, target_status_code=200): """ Verify that if a user's session auth hash and the request's hash differ, the user is logged out. The URL to test and the @@ -31,7 +32,7 @@ def _test_change_session_hash(self, test_url, redirect_url): self.assertEqual(response.status_code, 200) with patch.object(User, 'get_session_auth_hash', return_value='abc123'): response = self.client.get(test_url) - self.assertRedirects(response, redirect_url) + self.assertRedirects(response, redirect_url, target_status_code=target_status_code) @skip_unless_lms def test_session_change_lms(self): @@ -43,4 +44,5 @@ def test_session_change_lms(self): def test_session_change_cms(self): """Test session verification with CMS-specific URLs.""" home_url = reverse('home') - self._test_change_session_hash(home_url, reverse('login') + '?next=' + home_url) + # Studio login redirects to LMS login + self._test_change_session_hash(home_url, settings.LOGIN_URL + '?next=' + home_url, target_status_code=302) diff --git a/openedx/core/djangoapps/theming/tests/test_helpers.py b/openedx/core/djangoapps/theming/tests/test_helpers.py index 2055d9fb1577..16e30f94e8c9 100644 --- a/openedx/core/djangoapps/theming/tests/test_helpers.py +++ b/openedx/core/djangoapps/theming/tests/test_helpers.py @@ -153,16 +153,12 @@ def test_strip_site_theme_templates_path_theme_disabled(self): @skip_unless_cms class TestHelpersCMS(TestCase): - """Test comprehensive theming helper functions.""" - - @with_comprehensive_theme('red-theme') - def test_get_template_path_with_theme_enabled(self): - """ - Tests template paths are returned from enabled theme. - """ - template_path = get_template_path_with_theme('login.html') - self.assertEqual(template_path, 'red-theme/cms/templates/login.html') + """ + Test comprehensive theming helper functions. + Note: There is no `test_get_template_path_with_theme_enabled` because there currently + is no template to be themed. + """ @with_comprehensive_theme('red-theme') def test_get_template_path_with_theme_for_missing_template(self): """ diff --git a/openedx/core/djangoapps/theming/tests/test_theme_style_overrides.py b/openedx/core/djangoapps/theming/tests/test_theme_style_overrides.py index 62fe295ca357..5160b0a93e34 100644 --- a/openedx/core/djangoapps/theming/tests/test_theme_style_overrides.py +++ b/openedx/core/djangoapps/theming/tests/test_theme_style_overrides.py @@ -142,32 +142,6 @@ def test_include_custom_template(self): self.assertContains(resp, "This is a custom template.") -@skip_unless_cms -class TestComprehensiveThemeCMS(TestCase): - """ - Test html, sass and static file overrides for comprehensive themes. - """ - - def setUp(self): - """ - Clear static file finders cache and register cleanup methods. - """ - super(TestComprehensiveThemeCMS, self).setUp() - - # Clear the internal staticfiles caches, to get test isolation. - staticfiles.finders.get_finder.cache_clear() - - @with_comprehensive_theme("test-theme") - def test_template_override(self): - """ - Test that theme templates are used instead of default templates. - """ - resp = self.client.get('/signin') - self.assertEqual(resp.status_code, 200) - # This string comes from login.html of test-theme - self.assertContains(resp, "Login Page override for test-theme.") - - @skip_unless_lms class TestComprehensiveThemeDisabledLMS(TestCase): """ @@ -191,30 +165,6 @@ def test_logo(self): self.assertEqual(result, settings.REPO_ROOT / 'lms/static/images/logo.png') -@skip_unless_cms -class TestComprehensiveThemeDisabledCMS(TestCase): - """ - Test default html, sass and static file when no theme is applied. - """ - - def setUp(self): - """ - Clear static file finders cache and register cleanup methods. - """ - super(TestComprehensiveThemeDisabledCMS, self).setUp() - - # Clear the internal staticfiles caches, to get test isolation. - staticfiles.finders.get_finder.cache_clear() - - def test_template_override(self): - """ - Test that defaults templates are used when no theme is applied. - """ - resp = self.client.get('/signin') - self.assertEqual(resp.status_code, 200) - self.assertNotContains(resp, "Login Page override for test-theme.") - - @skip_unless_lms class TestStanfordTheme(TestCase): """ diff --git a/openedx/core/djangoapps/theming/tests/test_views.py b/openedx/core/djangoapps/theming/tests/test_views.py index 0da252a511cb..f25740634eb0 100644 --- a/openedx/core/djangoapps/theming/tests/test_views.py +++ b/openedx/core/djangoapps/theming/tests/test_views.py @@ -46,12 +46,15 @@ def test_preview_theme_access(self): """ # Anonymous users get redirected to the login page response = self.client.get(THEMING_ADMIN_URL) + # Studio login redirects to LMS login + expected_target_status_code = 200 if settings.ROOT_URLCONF == 'lms.urls' else 302 self.assertRedirects( response, '{login_url}?next={url}'.format( login_url=settings.LOGIN_URL, url=THEMING_ADMIN_URL, - ) + ), + target_status_code=expected_target_status_code ) # Logged in non-global staff get a 404 diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 2c24507eca50..4518233ed994 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -11,15 +11,12 @@ from django.conf import settings from django.core.exceptions import ObjectDoesNotExist from django.core.validators import ValidationError, validate_email -from django.db import IntegrityError, transaction -from django.http import HttpResponseForbidden from django.utils.translation import override as override_language from django.utils.translation import ugettext as _ +from edx_django_utils.monitoring import set_custom_metric from pytz import UTC from six import text_type # pylint: disable=ungrouped-imports -from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers -from openedx.core.djangoapps.theming.helpers import get_current_request from openedx.core.djangoapps.user_api import accounts, errors, helpers from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle from openedx.core.djangoapps.user_api.errors import ( @@ -349,6 +346,9 @@ def activate_account(activation_key): errors.UserAPIInternalError: the operation failed due to an unexpected error. """ + # TODO: Confirm this `activate_account` is only used for tests. If so, this should not be used for tests, and we + # should instead use the `activate_account` used for /activate. + set_custom_metric('user_api_activate_account', 'True') if waffle().is_enabled(PREVENT_AUTH_USER_WRITES): raise errors.UserAPIInternalError(SYSTEM_MAINTENANCE_MSG) try: diff --git a/openedx/core/djangoapps/user_authn/urls_common.py b/openedx/core/djangoapps/user_authn/urls_common.py index 96322a4b9170..352fb799cec3 100644 --- a/openedx/core/djangoapps/user_authn/urls_common.py +++ b/openedx/core/djangoapps/user_authn/urls_common.py @@ -35,9 +35,7 @@ name='registration_validation' ), - # Login - url(r'^login_post$', login.login_user, name='login_post'), - url(r'^login_ajax$', login.login_user, name="login"), + url(r'^login_ajax$', login.login_user, name="login_api"), # Moved from user_api/legacy_urls.py # `user_api` prefix is preserved for backwards compatibility. @@ -64,6 +62,13 @@ ] +if not getattr(settings, 'DISABLE_DEPRECATED_LOGIN_POST', False): + # TODO: Remove login_post once it no longer has real traffic. + # It was only used by old Studio sign-in and some miscellaneous callers, which should no longer be in use. + urlpatterns += [ + url(r'^login_post$', login.login_user, name='login_post'), + ] + # password reset django views (see above for password reset views) urlpatterns += [ url( diff --git a/openedx/core/djangoapps/user_authn/views/logout.py b/openedx/core/djangoapps/user_authn/views/logout.py index 60087ffe95df..1dc51d57446f 100644 --- a/openedx/core/djangoapps/user_authn/views/logout.py +++ b/openedx/core/djangoapps/user_authn/views/logout.py @@ -73,11 +73,7 @@ def dispatch(self, request, *args, **kwargs): logout(request) - # If we are using studio logout directly and there is not OIDC logouts we can just redirect the user - if settings.FEATURES.get('DISABLE_STUDIO_SSO_OVER_LMS', False) and not self.oauth_client_ids: - response = redirect(self.target) - else: - response = super(LogoutView, self).dispatch(request, *args, **kwargs) + response = super(LogoutView, self).dispatch(request, *args, **kwargs) # Clear the cookie used by the edx.org marketing site delete_logged_in_cookies(response) diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index c372192488f7..9245c0a3c9b5 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -67,10 +67,7 @@ def setUp(self): self.client = Client() cache.clear() - try: - self.url = reverse('login_post') - except NoReverseMatch: - self.url = reverse('login') + self.url = reverse('login_api') def _create_user(self, username, user_email): user = UserFactory.build(username=username, email=user_email) diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_views.py b/openedx/core/djangoapps/user_authn/views/tests/test_views.py index 11303c96371a..d116eef6b716 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_views.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_views.py @@ -142,8 +142,8 @@ def test_password_change(self): self.client.logout() # Verify that the new password can be used to log in - login_url = reverse('login_post') - response = self.client.post(login_url, {'email': self.OLD_EMAIL, 'password': self.NEW_PASSWORD}) + login_api_url = reverse('login_api') + response = self.client.post(login_api_url, {'email': self.OLD_EMAIL, 'password': self.NEW_PASSWORD}) assert response.status_code == 200 response_dict = json.loads(response.content.decode('utf-8')) assert response_dict['success'] @@ -161,7 +161,7 @@ def test_password_change(self): self.assertFalse(result) # Verify that the new password continues to be valid - response = self.client.post(login_url, {'email': self.OLD_EMAIL, 'password': self.NEW_PASSWORD}) + response = self.client.post(login_api_url, {'email': self.OLD_EMAIL, 'password': self.NEW_PASSWORD}) assert response.status_code == 200 response_dict = json.loads(response.content.decode('utf-8')) assert response_dict['success'] diff --git a/themes/red-theme/cms/templates/login.html b/themes/red-theme/cms/templates/login.html deleted file mode 100644 index cfe88f40c8d3..000000000000 --- a/themes/red-theme/cms/templates/login.html +++ /dev/null @@ -1,58 +0,0 @@ -<%namespace name='static' file='/static_content.html'/> -<%page expression_filter="h"/> - -<%inherit file="base.html" /> -<%def name="online_help_token()"><% return "login" %> -<%! -from django.urls import reverse -from django.utils.translation import ugettext as _ -%> -<%block name="title">${_("Sign In")} -<%block name="bodyclass">not-signedin view-signin - -<%block name="content"> -
    -
    -
    -

    ${_("Sign In to {studio_name}").format(studio_name=settings.STUDIO_NAME)}

    - -
    - -
    -
    - -
    - ${_("Required Information to Sign In to {studio_name}").format(studio_name=settings.STUDIO_NAME)} - - -
      -
    1. - - -
      2. - -
    2. - - - ${_("Forgot password?")} -
      3. -
    -
    - -
    - -
    - - - -
    -
    -
    -
    - - -<%block name="page_bundle"> - <%static:webpack entry="js/factories/login"> - LoginFactory("${reverse('homepage') | n, js_escaped_string}"); - - diff --git a/webpack.common.config.js b/webpack.common.config.js index 5e8602b06a1b..d37d0dfac69e 100644 --- a/webpack.common.config.js +++ b/webpack.common.config.js @@ -72,7 +72,6 @@ module.exports = Merge.smart({ // Studio Import: './cms/static/js/features/import/factories/import.js', CourseOrLibraryListing: './cms/static/js/features_jsx/studio/CourseOrLibraryListing.jsx', - 'js/factories/login': './cms/static/js/factories/login.js', 'js/factories/textbooks': './cms/static/js/factories/textbooks.js', 'js/factories/container': './cms/static/js/factories/container.js', 'js/factories/context_course': './cms/static/js/factories/context_course.js',