From 770e7aac49df80aad07e1d9f362c960c3c5f4cee Mon Sep 17 00:00:00 2001 From: njdup Date: Mon, 30 Jun 2014 10:32:26 -0700 Subject: [PATCH] User registration prevents using password as username. Some users erroneously set their password as their username with the original layout, because the username field directly followed the password field. Users may be accustomed to the common occurrence of a password confirmation field directly following the password field. To fix the issue, I did the following: - Moved the existing username and real name form fields above the password field. - Added a validation in the create_account handler in common/djangoapps/student/views.py, which confirms that the password field does not match the username field. New tests created to check the added functionality. --- AUTHORS | 1 + cms/templates/register.html | 12 +++---- .../student/tests/test_password_policy.py | 36 +++++++++++++++++++ common/djangoapps/student/views.py | 8 +++++ lms/templates/register.html | 22 ++++++------ 5 files changed, 62 insertions(+), 17 deletions(-) diff --git a/AUTHORS b/AUTHORS index c646f11baa..a87736c7a8 100644 --- a/AUTHORS +++ b/AUTHORS @@ -161,3 +161,4 @@ Daniel Friedman Asad Iqbal Peter Pinch Muhammad Shoaib +Nicholas Dupoux diff --git a/cms/templates/register.html b/cms/templates/register.html index 58cdff858d..3ef794aeee 100644 --- a/cms/templates/register.html +++ b/cms/templates/register.html @@ -31,9 +31,9 @@ -
  • - - +
  • + +
  • @@ -42,9 +42,9 @@ ${_("This will be used in public discussions with your courses and in our edX101 support forums")}
    • -
  • - - +
  • + +
  • diff --git a/common/djangoapps/student/tests/test_password_policy.py b/common/djangoapps/student/tests/test_password_policy.py index a649c53544..cb5c0f883a 100644 --- a/common/djangoapps/student/tests/test_password_policy.py +++ b/common/djangoapps/student/tests/test_password_policy.py @@ -236,3 +236,39 @@ class TestPasswordPolicy(TestCase): self.assertEqual(response.status_code, 200) obj = json.loads(response.content) self.assertTrue(obj['success']) + + +class TestUsernamePasswordNonmatch(TestCase): + """ + Test that registration username and password fields differ + """ + def setUp(self): + super(TestUsernamePasswordNonmatch, self).setUp() + self.url = reverse('create_account') + + self.url_params = { + 'username': 'username', + 'email': 'foo_bar@bar.com', + 'name': 'username', + 'terms_of_service': 'true', + 'honor_code': 'true', + } + + def test_with_username_password_match(self): + self.url_params['username'] = "foobar" + self.url_params['password'] = "foobar" + response = self.client.post(self.url, self.url_params) + self.assertEquals(response.status_code, 400) + obj = json.loads(response.content) + self.assertEqual( + obj['value'], + "Username and password fields cannot match", + ) + + def test_with_username_password_nonmatch(self): + self.url_params['username'] = "foobar" + self.url_params['password'] = "nonmatch" + response = self.client.post(self.url, self.url_params) + self.assertEquals(response.status_code, 200) + obj = json.loads(response.content) + self.assertTrue(obj['success']) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 28cc714687..333156550c 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1275,6 +1275,14 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many extended_profile = {} extended_profile[field] = post_vars[field] + # Make sure that password and username fields do not match + username = post_vars['username'] + password = post_vars['password'] + if username == password: + js['value'] = _("Username and password fields cannot match") + js['field'] = 'username' + return JsonResponse(js, status=400) + # Ok, looks like everything is legit. Create the account. try: with transaction.commit_on_success(): diff --git a/lms/templates/register.html b/lms/templates/register.html index 7e35ca4452..9c5c3480a5 100644 --- a/lms/templates/register.html +++ b/lms/templates/register.html @@ -167,6 +167,17 @@
    • +
  • + + + ${_("Needed for any certificates you may earn")} +
    • +
  • + + + ${_('Will be shown in any discussions or forums you participate in')} (${_('cannot be changed later')}) +
    • + % if settings.FEATURES.get('ENABLE_THIRD_PARTY_AUTH') and running_pipeline: % endif - -
  • - - - ${_('Will be shown in any discussions or forums you participate in')} (${_('cannot be changed later')}) -
    • -
  • - - - ${_("Needed for any certificates you may earn")} -
    • % else: