Added password strength check + Tests (#918)

* Added password strength check + Tests

Also changed automatically generated passwords to Password2 instead of Password1

* Fixed A Couple Of Things

Small errors caught on code climate

* Changed Password Reset Checks

Student passwords (not automatically generated) now need to be at least 8 characters long. 
Also fixed 8 character condition (equal or greater than)

* Changed Password Length Message On Test

* Changed Password Error/Warning Message

Instead of only pointing out length, it points out the password requirements whenever at least one isn't met

* Restructured The Dynamic Update Function

portal/forms/auth_forms.py

@@ -57,8 +57,8 @@ def clean_new_password1(self):
                     "Password not strong enough, consider using at least 8 characters, upper and "
                     + "lower case letters, and numbers")
         elif hasattr(self.user, 'student'):
-            if not password_strength_test(new_password1, length=6, upper=False, lower=False,
+            if not password_strength_test(new_password1, length=8, upper=False, lower=False,
                                           numbers=False):
                 raise forms.ValidationError(
-                    "Password not strong enough, consider using at least 6 characters")
+                    "Password not strong enough, consider using at least 8 characters, upper and lower case letters, and numbers")
         return new_password1

portal/forms/play.py

@@ -131,9 +131,9 @@ def clean_name(self):
     def clean_password(self):
         password = self.cleaned_data.get('password', None)
 
-        if password and not password_strength_test(password, length=6, upper=False, lower=False, numbers=False):
+        if password and not password_strength_test(password, length=8, upper=False, lower=False, numbers=False):
             raise forms.ValidationError(
-                "Password not strong enough, consider using at least 6 characters")
+                "Password not strong enough, consider using at least 8 characters, upper and lower case letters, and numbers")
 
         return password
 
@@ -204,8 +204,8 @@ def clean_username(self):
     def clean_password(self):
         password = self.cleaned_data.get('password', None)
 
-        if password and not password_strength_test(password, length=6, upper=False, lower=False, numbers=False):
-            raise forms.ValidationError("Password not strong enough, consider using at least 6 characters")
+        if password and not password_strength_test(password, length=8, upper=False, lower=False, numbers=False):
+            raise forms.ValidationError("Password not strong enough, consider using at least 8 characters, upper and lower case letters, and numbers")
 
         return password
 

portal/forms/registration.py

@@ -67,8 +67,8 @@ def clean_new_password1(self):
                     "Password not strong enough, consider using at least 8 characters, upper and " +
                     "lower case letters, and numbers")
         elif hasattr(self.user, 'student'):
-            if not password_strength_test(new_password1, length=6, upper=False, lower=False, numbers=False):
-                raise forms.ValidationError("Password not strong enough, consider using at least 6 characters")
+            if not password_strength_test(new_password1, length=8, upper=False, lower=False, numbers=False):
+                raise forms.ValidationError("Password not strong enough, consider using at least 8 characters, upper and lower case letters, and numbers")
         return new_password1
 
 

portal/forms/teach.py

@@ -325,10 +325,10 @@ class TeacherSetStudentPass(forms.Form):
     def clean_password(self):
         password = self.cleaned_data.get('password', None)
 
-        if password and not password_strength_test(password, length=6, upper=False, lower=False,
+        if password and not password_strength_test(password, length=8, upper=False, lower=False,
                                                    numbers=False):
             raise forms.ValidationError(
-                "Password not strong enough, consider using at least 6 characters")
+                "Password not strong enough, consider using at least 8 characters, upper and lower case letters, and numbers")
 
         return password
 

portal/helpers/password.py

@@ -38,7 +38,9 @@
 
 
 def password_strength_test(password, length=8, upper=True, lower=True, numbers=True):
+    most_used_passwords_2018 = ['Abcd1234', 'Password1', 'Qwerty123']
     return (len(password) >= length and
             (not upper or re.search(r'[A-Z]', password)) and
             (not lower or re.search(r'[a-z]', password)) and
-            (not numbers or re.search(r'[0-9]', password)))
+            (not numbers or re.search(r'[0-9]', password)) and
+            (password not in most_used_passwords_2018))

portal/static/portal/js/passwordStrength.js

@@ -39,63 +39,71 @@ var TEACHER_PASSWORD_FIELD_ID = '';
 var INDEP_STUDENT_PASSWORD_FIELD_ID = '';
 var teacher_password_field = '';
 var indep_student_password_field = '';
+var most_used_passwords_2018 = ['Abcd1234', 'Password1', 'Qwerty123'];
 
 var password_strengths = [
     { name: 'No password!', colour: '#FF0000' },
     { name: 'Password too weak', colour: '#DBA901' },
-    { name: 'Strong password', colour: '#088A08' }
+    { name: 'Strong password', colour: '#088A08' },
+    { name: 'Password too common', colour: '#DBA901' }
 ];
 
 $(function() {
 
     teacher_password_field = $('#' + TEACHER_PASSWORD_FIELD_ID);
     indep_student_password_field = $('#' + INDEP_STUDENT_PASSWORD_FIELD_ID);
 
-    setUpDynamicUpdates(teacher_password_field, updateTeacherPasswordStrength);
-    setUpDynamicUpdates(indep_student_password_field, updateIndependentStudentPasswordStrength);
+    setUpDynamicUpdates(teacher_password_field, true);
+    setUpDynamicUpdates(indep_student_password_field, false);
 
-    updateTeacherPasswordStrength();
-    updateIndependentStudentPasswordStrength();
+    updatePasswordStrength(true);
+    updatePasswordStrength(false);
 });
 
-function setUpDynamicUpdates(password_field, update_function) {
-    password_field.on('keyup', update_function);
-    password_field.on('paste', update_function);
-    password_field.on('cut', update_function);
+function setUpDynamicUpdates(password_field, isTeacher) {
+    password_field.on('keyup', function(){
+        updatePasswordStrength(isTeacher)
+    });
+    password_field.on('paste', function(){
+        updatePasswordStrength(isTeacher)
+    });
+    password_field.on('cut', function(){
+        updatePasswordStrength(isTeacher)
+    });
 }
 
-function updateTeacherPasswordStrength() {
+function updatePasswordStrength(isTeacher) {
     // The reason for the timeout is that if we just got $('#...').val() we'd get the
     // old value before the keypress / change. Apparently even jQuery itself implements
     // things this way, so maybe there is no better workaround.
 
     setTimeout(function() {
+        var password;
 
-        var password = $('#' + TEACHER_PASSWORD_FIELD_ID).val();
+        if (isTeacher) {
+            password = $('#' + TEACHER_PASSWORD_FIELD_ID).val();
+        }
+        else {
+            password = $('#' + INDEP_STUDENT_PASSWORD_FIELD_ID).val();
+        }
 
         var strength = 0;
         if (password.length > 0) { strength++; }
-        if (password.length > 8 && !(password.search(/[A-Z]/) === -1 || password.search(/[a-z]/) === -1 || password.search(/[0-9]/) === -1)) { strength++; }
-
-        updatePasswordCSS('#teacher-password-sign', '#teacher-password-text', strength);
-    });
-}
-
-function updateIndependentStudentPasswordStrength() {
+        if (password.length >= 8 && !(password.search(/[A-Z]/) === -1 || password.search(/[a-z]/) === -1 || password.search(/[0-9]/) === -1)) { strength++; }
+        if ($.inArray(password, most_used_passwords_2018) >= 0 && strength == 2) { strength = 3; }
 
-    setTimeout(function() {
-
-        var password = $('#' + INDEP_STUDENT_PASSWORD_FIELD_ID).val();
+        if (isTeacher) {
+            updatePasswordCSS('#teacher-password-sign', '#teacher-password-text', strength);
+        }
+        else {
+            updatePasswordCSS('#student-password-sign', '#student-password-text', strength);
+        }
 
-        var strength = 0;
-        if (password.length > 0) { strength++; }
-        if (password.length > 5) { strength++; }
-
-        updatePasswordCSS('#student-password-sign', '#student-password-text', strength);
     });
 }
 
 function updatePasswordCSS(passwordStrengthSign, passwordStrengthText, strength) {
     $(passwordStrengthSign).css('background-color', password_strengths[strength].colour);
     $(passwordStrengthText).html(password_strengths[strength].name);
-}
+}
+

portal/tests/pageObjects/portal/signup_page.py

@@ -47,7 +47,7 @@ def __init__(self, browser):
 
         assert self.on_correct_page('signup_page')
 
-    def signup(self, title, first_name, last_name, email, password, confirm_password, newsletter=False):
+    def signup(self, title, first_name, last_name, email, password, confirm_password, success=True, newsletter=False):
         Select(self.browser.find_element_by_id('id_teacher_signup-teacher_title')).select_by_value(title)
         self.browser.find_element_by_id('id_teacher_signup-teacher_first_name').send_keys(first_name)
         self.browser.find_element_by_id('id_teacher_signup-teacher_last_name').send_keys(last_name)
@@ -59,7 +59,11 @@ def signup(self, title, first_name, last_name, email, password, confirm_password
             self.browser.find_element_by_id('id_teacher_signup-newsletter_ticked').click()
 
         self.browser.find_element_by_name('teacher_signup').click()
-        return email_verification_needed_page.EmailVerificationNeededPage(self.browser)
+
+        if success:
+            return email_verification_needed_page.EmailVerificationNeededPage(self.browser)
+        else:
+            return self
 
     def independent_student_signup(self, name, username, email_address, password, confirm_password, success=True,
                                    newsletter=False):
@@ -82,3 +86,7 @@ def independent_student_signup(self, name, username, email_address, password, co
     def has_independent_student_signup_failed(self, error):
         errors = self.browser.find_element_by_id('form-signup-independent-student').find_element_by_class_name('errorlist').text
         return error in errors
+
+    def has_teacher_signup_failed(self, error):
+        errors = self.browser.find_element_by_id('form-reg-teacher').find_element_by_class_name('errorlist').text
+        return error in errors

portal/tests/test_independent_student.py

@@ -64,15 +64,21 @@ def test_signup_with_newsletter(self):
         page, _, _, _, _ = create_independent_student(page, newsletter=True)
         assert is_email_verified_message_showing(self.selenium)
 
-    def test_signup_failed(self):
+    def test_signup_failure_short_password(self):
         page = self.go_to_homepage()
         page = submit_independent_student_signup_form(page, password='test')
         assert page.has_independent_student_signup_failed(
-            'Password not strong enough, consider using at least 6 characters')
+            'Password not strong enough, consider using at least 8 characters, upper and lower case letters, and numbers')
+
+    def test_signup_failure_common_password(self):
+        page = self.go_to_homepage()
+        page = submit_independent_student_signup_form(page, password='Password1')
+        assert page.has_independent_student_signup_failed(
+            'Password not strong enough, consider using at least 8 characters, upper and lower case letters, and numbers')
 
     def test_signup_invalid_name(self):
         page = self.go_to_homepage().go_to_signup_page()
-        page = page.independent_student_signup('Florian!', 'Florian', '[email protected]', 'Password1', 'Password1',
+        page = page.independent_student_signup('Florian!', 'Florian', '[email protected]', 'Password2', 'Password2',
                                                success=False)
 
         assert self.is_signup_page(page)
@@ -81,7 +87,7 @@ def test_signup_invalid_name(self):
 
     def test_signup_invalid_username(self):
         page = self.go_to_homepage().go_to_signup_page()
-        page = page.independent_student_signup('Florian', '///', '[email protected]', 'Password1', 'Password1',
+        page = page.independent_student_signup('Florian', '///', '[email protected]', 'Password2', 'Password2',
                                                success=False)
 
         assert self.is_signup_page(page)
@@ -92,15 +98,15 @@ def test_signup_username_already_in_use(self):
         page = self.go_to_homepage()
         page, name, username, email, password = create_independent_student(page)
         page = self.go_to_homepage().go_to_signup_page()
-        page = page.independent_student_signup('Florian', username, '[email protected]', 'Password1', 'Password1',
+        page = page.independent_student_signup('Florian', username, '[email protected]', 'Password2', 'Password2',
                                                success=False)
 
         assert self.is_signup_page(page)
         assert page.has_independent_student_signup_failed('That username is already in use')
 
     def test_signup_password_do_not_match(self):
         page = self.go_to_homepage().go_to_signup_page()
-        page = page.independent_student_signup('Florian', 'Florian', '[email protected]', 'Password1', 'Password2',
+        page = page.independent_student_signup('Florian', 'Florian', '[email protected]', 'Password2', 'Password3',
                                                success=False)
 
         assert self.is_signup_page(page)

portal/tests/test_school_student.py

@@ -159,7 +159,7 @@ def test_update_password_too_weak(self):
 
         page = page.go_to_account_page().update_password_failure('tiny', 'tiny', student_password)
         assert self.is_account_page(page)
-        assert page.was_form_invalid('student_account_form', 'Password not strong enough, consider using at least 6 characters')
+        assert page.was_form_invalid('student_account_form', 'Password not strong enough, consider using at least 8 characters, upper and lower case letters, and numbers')
 
     def test_update_password_success(self):
         email, password = signup_teacher_directly()

portal/tests/test_teacher.py

@@ -42,7 +42,7 @@
 
 from base_test import BaseTest
 from pageObjects.portal.home_page import HomePage
-from utils.teacher import signup_teacher, signup_teacher_directly, signup_duplicate_teacher_fail
+from utils.teacher import signup_teacher, signup_teacher_directly, signup_duplicate_teacher_fail, submit_teacher_signup_form
 from utils.organisation import create_organisation_directly, join_teacher_to_organisation
 from utils.classes import create_class_directly
 from utils.student import create_school_student_directly
@@ -76,6 +76,20 @@ def test_signup_duplicate_failure(self):
         page, _, _ = signup_duplicate_teacher_fail(page, email)
         assert page.__class__.__name__ == 'LoginPage'
 
+    def test_signup_failure_short_password(self):
+        self.selenium.get(self.live_server_url)
+        page = HomePage(self.selenium)
+        page = submit_teacher_signup_form(page, password='test')
+        assert page.has_teacher_signup_failed(
+            'Password not strong enough, consider using at least 8 characters, upper and lower case letters, and numbers')
+
+    def test_signup_failure_common_password(self):
+        self.selenium.get(self.live_server_url)
+        page = HomePage(self.selenium)
+        page = submit_teacher_signup_form(page, password='Password1')
+        assert page.has_teacher_signup_failed(
+            'Password not strong enough, consider using at least 8 characters, upper and lower case letters, and numbers')
+
     def test_login_failure(self):
         self.selenium.get(self.live_server_url)
         page = HomePage(self.selenium)
@@ -143,7 +157,7 @@ def test_edit_details(self):
             'title': 'Mrs',
             'first_name': 'Paulina',
             'last_name': 'Koch',
-            'current_password': 'Password1',
+            'current_password': 'Password2',
         })
         assert self.is_dashboard_page(page)
         assert is_teacher_details_updated_message_showing(self.selenium)
@@ -171,7 +185,7 @@ def test_edit_details_non_admin(self):
             'title': 'Mr',
             'first_name': 'Florian',
             'last_name': 'Aucomte',
-            'current_password': 'Password1',
+            'current_password': 'Password2',
         })
         assert self.is_dashboard_page(page)
         assert is_teacher_details_updated_message_showing(self.selenium)

portal/tests/utils/student.py

@@ -43,7 +43,7 @@
 
 def generate_school_details():
     name = 'Student %d' % generate_school_details.next_id
-    password = 'Password1'
+    password = 'Password2'
 
     generate_school_details.next_id += 1
 
@@ -96,7 +96,7 @@ def generate_independent_student_details():
     name = 'Student %d' % generate_independent_student_details.next_id
     username = 'Student user %d' % generate_independent_student_details.next_id
     email_address = 'Student%[email protected]' % generate_independent_student_details.next_id
-    password = 'Password1'
+    password = 'Password2'
 
     generate_independent_student_details.next_id += 1
 

portal/tests/utils/teacher.py

@@ -48,7 +48,7 @@ def generate_details(**kwargs):
     first_name = kwargs.get('first_name', 'Test')
     last_name = kwargs.get('last_name', 'Teacher')
     email_address = kwargs.get('email_address', 'testteacher%[email protected]' % random.randint(1, sys.maxint))
-    password = kwargs.get('password', 'Password1')
+    password = kwargs.get('password', 'Password2')
 
     return title, first_name, last_name, email_address, password
 
@@ -99,3 +99,14 @@ def signup_teacher(page, newsletter=False):
     mail.outbox = []
 
     return page, email_address, password
+
+
+def submit_teacher_signup_form(page, password='test'):
+    page = page.go_to_signup_page()
+
+    title, first_name, last_name, email_address, _ = generate_details()
+    return page.signup(title, first_name, last_name, email_address,
+                       password=password,
+                       confirm_password=password,
+                       success=False,
+                       newsletter=True)