Skip to content

ARCH-1253: remove studio signin and signup pages #22416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
robrap merged 1 commit into from
Dec 4, 2019
Merged

ARCH-1253: remove studio signin and signup pages #22416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 4 additions & 32 deletions cms/djangoapps/contentstore/tests/test_contentstore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
# <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.
Expand Down
202 changes: 23 additions & 179 deletions cms/djangoapps/contentstore/tests/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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"""

Expand All @@ -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 <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 = (
Expand All @@ -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()
Expand All @@ -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):
"""
Expand All @@ -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, '<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
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,
'<a href="/signup" class="action action-signin">Don&#39;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):
Expand Down
59 changes: 23 additions & 36 deletions cms/djangoapps/contentstore/views/public.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,64 +5,51 @@

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):
"""
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:
Expand Down
Loading