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',