From 678f1510372b8924e5d23271f31c2961238d2137 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Wed, 2 May 2018 20:09:37 -0700 Subject: [PATCH] Factor passwd hash logic into a function ... rather than having it duplicated in 3 places. Please review https://boinc.berkeley.edu/trac/wiki/CodingStyle --- html/inc/user_util.inc | 24 ++++++++++++++++++++++++ html/ops/login_action.php | 12 +----------- html/user/login_action.php | 15 ++++----------- html/user/lookup_account.php | 19 +++++++------------ 4 files changed, 36 insertions(+), 34 deletions(-) diff --git a/html/inc/user_util.inc b/html/inc/user_util.inc index b28198de16..9c48187e4a 100644 --- a/html/inc/user_util.inc +++ b/html/inc/user_util.inc @@ -25,11 +25,35 @@ include_once("../inc/email.inc"); include_once("../inc/recaptchalib.php"); require_once("../inc/password_compat/password.inc"); +// Password hash has old format. +// Update user record with new format +// function do_passwd_rehash($user, $passwd_hash) { $database_passwd_hash = password_hash($passwd_hash, PASSWORD_DEFAULT); $result = $user->update(" passwd_hash='$database_passwd_hash' "); } +// return true if the passwd hash (old format) +// matches the user's passwd hash (possibly new format) +// +function check_passwd_hash($user, $passwd_hash) { + if (password_verify($passwd_hash, $user->passwd_hash)) { + // on valid login, rehash password to upgrade hash overtime + // as the defaults change. + // + if (password_needs_rehash($user->passwd_hash, PASSWORD_DEFAULT)) { + do_passwd_rehash($user, $passwd_hash); + } + } else if ($passwd_hash == $user->passwd_hash) { + // user record has old format. Change to new. + // + do_passwd_rehash($user, $passwd_hash); + } else { + return false; + } + return true; +} + function is_banned_email_addr($email_addr) { global $banned_email_domains; if (isset($banned_email_domains)) { diff --git a/html/ops/login_action.php b/html/ops/login_action.php index fa7b67f112..7020eb1cd8 100644 --- a/html/ops/login_action.php +++ b/html/ops/login_action.php @@ -36,17 +36,7 @@ if ($email_addr && $passwd) { admin_error_page("No account found with email address $email_addr"); } $passwd_hash = md5($passwd.$email_addr); - if (password_verify($passwd_hash, $user->passwd_hash)) { - // on valid login, rehash password if necessary to upgrade hash overtime - // as the defaults change. - if (password_needs_rehash($user->passwd_hash, PASSWORD_DEFAULT)) { - do_passwd_rehash($user, $passwd_hash); - } - } else if ($passwd_hash == $user->passwd_hash) { - // if password is the legacy md5 hash, then rehash to update to - // a more secure hash - do_passwd_rehash($user, $passwd_hash); - } else { + if (!check_passwd_hash($user, $passwd_hash)) { admin_error_page("Login failed"); } $authenticator = $user->authenticator; diff --git a/html/user/login_action.php b/html/user/login_action.php index 5d973f2553..a137bbfed1 100644 --- a/html/user/login_action.php +++ b/html/user/login_action.php @@ -50,20 +50,13 @@ function login_with_email($email_addr, $passwd, $next_url, $perm) { sleep(LOGIN_FAIL_SLEEP_SEC); error_page("This account has been administratively disabled."); } + // allow authenticator as password + // if ($passwd != $user->authenticator) { $passwd_hash = md5($passwd.$email_addr); - if (password_verify($passwd_hash, $user->passwd_hash)) { - // on valid login, rehash password if necessary to upgrade hash overtime - // as the defaults change. - if (password_needs_rehash($user->passwd_hash, PASSWORD_DEFAULT)) { - do_passwd_rehash($user, $passwd_hash); - } - } else if ($passwd_hash == $user->passwd_hash) { - // if password is the legacy md5 hash, then rehash to update to - // a more secure hash - do_passwd_rehash($user, $passwd_hash); - } else { + + if (!check_passwd_hash($user, $passwd_hash)) { sleep(LOGIN_FAIL_SLEEP_SEC); page_head("Password incorrect"); echo "The password you entered is incorrect. Please go back and try again.\n"; diff --git a/html/user/lookup_account.php b/html/user/lookup_account.php index 6b72978886..341893cbcd 100644 --- a/html/user/lookup_account.php +++ b/html/user/lookup_account.php @@ -62,6 +62,8 @@ if (LDAP_HOST && $ldap_auth) { xml_error(ERR_DB_NOT_FOUND); } + // here the caller was testing for existence of acct w/ given email + // if (!$passwd_hash) { echo "\n"; echo " \n"; @@ -72,22 +74,16 @@ if (LDAP_HOST && $ldap_auth) { $auth_hash = md5($user->authenticator.$user->email_addr); // if no password set, set password to account key + // WHEN WOULD THIS EVER HAPPEN? + // WHY SET IT TO AUTHENTICATOR? + // SHOULD RETURN PASSWD FAILURE? // if (!strlen($user->passwd_hash)) { $user->passwd_hash = password_hash($auth_hash, PASSWORD_DEFAULT); $user->update(" passwd_hash='$user->passwd_hash' "); } - - if (password_verify($passwd_hash, $user->passwd_hash)) { - // on valid login, rehash password if necessary to upgrade hash overtime - // as the defaults change. - if (password_needs_rehash($user->passwd_hash, PASSWORD_DEFAULT)) { - do_passwd_rehash($user, $passwd_hash); - } - } else if ($passwd_hash == $user->passwd_hash) { - // if password is the legacy md5 hash, then rehash to update to - // a more secure hash - do_passwd_rehash($user, $passwd_hash); + + if (check_passwd_hash($user, $passwd_hash)) { } else if ($auth_hash == $passwd_hash) { // if the passed hash matches the auth hash, then allow it } else { @@ -95,7 +91,6 @@ if (LDAP_HOST && $ldap_auth) { sleep(LOGIN_FAIL_SLEEP_SEC); xml_error(ERR_BAD_PASSWD); } - } echo "\n"; echo "$user->authenticator\n";