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
This commit is contained in:
Robert Raposa
@@ -2186,10 +2186,12 @@ class EntryPageTestCase(TestCase):
|
||||
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 @@ class EntryPageTestCase(TestCase):
|
||||
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:
|
||||
# <form>
|
||||
# ...
|
||||
# <fieldset>
|
||||
# ...
|
||||
# <input name="csrfmiddlewaretoken" value="...">
|
||||
# ...
|
||||
# </fieldset>
|
||||
# ...
|
||||
# </form>
|
||||
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.
|
||||
|
||||
@@ -7,19 +7,14 @@ import datetime
|
||||
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 @@ class ContentStoreTestCase(ModuleStoreTestCase):
|
||||
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 @@ class AuthTestCase(ContentStoreTestCase):
|
||||
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 <a href="' + reverse('login') + '">sign in</a>.'
|
||||
self.assertContains(resp, expected)
|
||||
|
||||
def test_private_pages_auth(self):
|
||||
"""Make sure pages that do require login work."""
|
||||
auth_pages = (
|
||||
@@ -259,7 +117,8 @@ class AuthTestCase(ContentStoreTestCase):
|
||||
)
|
||||
|
||||
# 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 @@ class AuthTestCase(ContentStoreTestCase):
|
||||
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 @@ class AuthTestCase(ContentStoreTestCase):
|
||||
resp = self.client.get_html(course_url)
|
||||
|
||||
# re-request, and we should get a redirect to login page
|
||||
self.assertRedirects(resp, settings.LOGIN_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
|
||||
is turned off, and not when it is turned on. The Sign In button should always appear.
|
||||
"""
|
||||
response = self.client.get(reverse('homepage'))
|
||||
self.assertNotContains(response, '<a class="action action-signup" href="/signup">Sign Up</a>')
|
||||
|
||||
@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, '<a class="action action-signup" href="/signup">Sign Up</a>')
|
||||
|
||||
@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
|
||||
"""
|
||||
response = self.client.get(reverse('login'))
|
||||
self.assertNotContains(
|
||||
response,
|
||||
'<a href="/signup" class="action action-signin">Don't have a Studio Account? Sign up!</a>'
|
||||
)
|
||||
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'<a class="action action-signup" href="{}/register?next=http%3A%2F%2Ftestserver%2F">Sign Up</a>'.format( # pylint: disable=line-too-long
|
||||
settings.LMS_ROOT_URL
|
||||
)
|
||||
)
|
||||
self.assertContains(
|
||||
response,
|
||||
u'<a class="action action-signin" href="/signin_redirect_to_lms?next=http%3A%2F%2Ftestserver%2F">Sign In</a>' # pylint: disable=line-too-long
|
||||
)
|
||||
|
||||
|
||||
class ForumTestCase(CourseTestCase):
|
||||
|
||||
Reference in New Issue
Block a user