From ab7aad05ce28829a0e4a2eb8f57350bca808d78b Mon Sep 17 00:00:00 2001 From: KamilPawel Date: Tue, 27 Jun 2023 16:32:35 +0100 Subject: [PATCH] addressing comments #2 --- portal/helpers/password.py | 12 ++-- portal/static/portal/js/passwordStrength.js | 78 ++++++++++++++------- portal/templates/portal/register.html | 23 +----- portal/tests/test_helper_methods.py | 4 +- portal/tests/test_teacher.py | 1 - 5 files changed, 62 insertions(+), 56 deletions(-) diff --git a/portal/helpers/password.py b/portal/helpers/password.py index 46ceb24e2..3e82836bf 100644 --- a/portal/helpers/password.py +++ b/portal/helpers/password.py @@ -21,7 +21,7 @@ def is_password_pwned(password): response = requests.get(url) if response.status_code != 200: - return True # backend ignore this and frontend tells the user + return False # backend ignore this and frontend tells the user # that we cannot verify this at the moment # Check if the password's hash is found in the response body @@ -31,8 +31,8 @@ def is_password_pwned(password): # api response is using the format of suffix:count hence # we need to get rid of the ending count number if sha1_hash[5:].upper() == suffix[:35].upper(): - return True - return False + return False + return True class PasswordStrength(Enum): @@ -56,7 +56,7 @@ def password_test(self, password): raise forms.ValidationError( f"Password not strong enough, consider using at least {minimum_password_length} characters and making it hard to guess." ) - if is_password_pwned(password): + if not is_password_pwned(password): raise forms.ValidationError("Password is too common, consider using a different password.") elif self is PasswordStrength.INDEPENDENT: @@ -73,7 +73,7 @@ def password_test(self, password): f"Password not strong enough, consider using at least {minimum_password_length} characters, " "upper and lower case letters, and numbers and making it hard to guess." ) - if is_password_pwned(password): + if not is_password_pwned(password): raise forms.ValidationError("Password is too common, consider using a different password.") else: minimum_password_length = 10 @@ -89,7 +89,7 @@ def password_test(self, password): f"Password not strong enough, consider using at least {minimum_password_length} characters, " "upper and lower case letters, numbers, special characters and making it hard to guess." ) - if is_password_pwned(password): + if not is_password_pwned(password): raise forms.ValidationError("Password is too common, consider using a different password.") return password diff --git a/portal/static/portal/js/passwordStrength.js b/portal/static/portal/js/passwordStrength.js index 8123fdc1e..c6d904863 100644 --- a/portal/static/portal/js/passwordStrength.js +++ b/portal/static/portal/js/passwordStrength.js @@ -6,43 +6,49 @@ let password_strengths = [ ]; async function handlePasswordStrength() { - const tPwd = $('#id_teacher_signup-teacher_password').val(); - const sPwd = $('#id_independent_student_signup-password').val(); + const teacherPwd = $('#id_teacher_signup-teacher_password').val(); + const studentPwd = $('#id_independent_student_signup-password').val(); - const isTPwdEmpty = tPwd.length > 0; - const isSPwdEmpty = sPwd.length > 0; + const isTeacherPwdTyped = teacherPwd.length > 0; + const isStudentPwdTyped = studentPwd.length > 0; - const isTPwdStrong = isTPwdEmpty && isPasswordStrong(tPwd, true); - const isSPwdStrong = isSPwdEmpty && isPasswordStrong(sPwd, false); + const isTeacherPwdStrong = + isTeacherPwdTyped && isPasswordStrong(teacherPwd, true); + const isStudentPwdStrong = + isStudentPwdTyped && isPasswordStrong(studentPwd, false); - const isTPwdSafe = isTPwdStrong && (await getPwnedStatus(tPwd)); - const isSPwdSafe = isSPwdStrong && (await getPwnedStatus(sPwd)); + const isTeacherPwdSafe = + isTeacherPwdStrong && (await !isPasswordPwned(teacherPwd)); + const isStudentPwdSafe = + isStudentPwdStrong && (await !isPasswordPwned(studentPwd)); - const tPwdStr = [isTPwdEmpty, isTPwdStrong, isTPwdSafe].filter( - Boolean - ).length; - const sPwdStr = [isSPwdEmpty, isSPwdStrong, isSPwdSafe].filter( - Boolean - ).length; + const teacherPwdStr = [ + isTeacherPwdTyped, + isTeacherPwdStrong, + isTeacherPwdSafe + ].filter(Boolean).length; + const studentPwdStrength = [ + isStudentPwdTyped, + isStudentPwdStrong, + isStudentPwdSafe + ].filter(Boolean).length; $('#teacher-password-sign').css( 'background-color', - password_strengths[tPwdStr].colour + password_strengths[teacherPwdStr].colour ); - $('#teacher-password-text').html(password_strengths[tPwdStr].name); + $('#teacher-password-text').html(password_strengths[teacherPwdStr].name); $('#student-password-sign').css( 'background-color', - password_strengths[sPwdStr].colour + password_strengths[studentPwdStrength].colour ); - $('#student-password-text').html(password_strengths[sPwdStr].name); + $('#student-password-text').html(password_strengths[studentPwdStrength].name); } -const getPwnedStatus = async (password) => { +const isPasswordPwned = async (password) => { const computeSHA1Hash = (password) => CryptoJS.SHA1(password).toString().toUpperCase(); - const doesSuffixExist = (data, suffix) => data.includes(suffix); - try { const hashedPassword = computeSHA1Hash(password); const prefix = hashedPassword.substring(0, 5); @@ -52,19 +58,19 @@ const getPwnedStatus = async (password) => { const response = await fetch(apiUrl); if (!response.ok) { - return true; // ignore the check if the server is down as the popup warns + return false; // ignore the check if the server is down as the popup warns // the user that we cannot check the password } const data = await response.text(); - if (doesSuffixExist(data, suffix)) { - return false; + if (data.includes(suffix)) { + return true; } - return true; + return false; } catch (error) { console.error(`Request failed with error: ${error.message}`); - return false; + return true; } }; @@ -90,3 +96,23 @@ function isPasswordStrong(password, isTeacher) { ); } } + +async function isPwnedPasswordApiAvailable(url) { + try { + const response = await fetch(url, { metheod: 'GET' }); + return response.ok; + } catch (error) { + console.error(error); + return false; + } +} +async function handlePwnedPasswordApiAvailability() { + const url = 'https://api.pwnedpasswords.com/range/00000'; + const isAvailable = await isPwnedPasswordApiAvailable(url); + const errorTitle = 'Password Vulnerability Check Unavailable'; + const errorMessage = + 'We are currently unable to check your password vulnerability. Please ensure that you are using a strong password. If you are happy to continue, please confirm.'; + if (!isAvailable) { + showServiceUnavailable(errorTitle, errorMessage); + } +} diff --git a/portal/templates/portal/register.html b/portal/templates/portal/register.html index e14ffbecf..7726df9eb 100644 --- a/portal/templates/portal/register.html +++ b/portal/templates/portal/register.html @@ -194,28 +194,9 @@

Independent learner