From 98ba61db03329c126b6d1ff1b64baa7ed2bdd70e Mon Sep 17 00:00:00 2001 From: Chris Morgan Date: Tue, 27 Jun 2006 03:59:16 +0000 Subject: [PATCH] Clean up user class. Implement start of unit testing framework for appdb. Implement start of user class unit test. --- account.php | 17 ++- include/user.php | 79 +++++++---- preferences.php | 16 ++- unit_test/path.php | 3 + unit_test/run_tests | 1 + unit_test/run_tests.php | 11 ++ unit_test/test_user.php | 285 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 378 insertions(+), 34 deletions(-) create mode 100644 unit_test/path.php create mode 100755 unit_test/run_tests create mode 100644 unit_test/run_tests.php create mode 100644 unit_test/test_user.php diff --git a/account.php b/account.php index 51d4276..4f93af9 100644 --- a/account.php +++ b/account.php @@ -114,17 +114,26 @@ function cmd_do_new() $result = $user->create($aClean['ext_email'], $aClean['ext_password'], $aClean['ext_realname'], $aClean['CVSrelease'] ); - if($result == true) + if($result == SUCCESS) { /* if we can log the user in, log them in automatically */ - if($user->login($aClean['ext_email'], $aClean['ext_password'])) + if($user->login($aClean['ext_email'], $aClean['ext_password']) == SUCCESS) $_SESSION['current'] = $user; addmsg("Account created! (".$aClean['ext_email'].")", "green"); redirect(apidb_fullurl()); } - else + else if($result == USER_CREATE_EXISTS) { + addmsg("An account with this e-mail exists already.", "red"); + retry("new", "Failed to create account"); + } else if($result = USER_CREATE_FAILED) + { + addmsg("Error while creating a new user.", "red"); + retry("new", "Failed to create account"); + } else + { + addmsg("Unknown failure while creating new user. Please report this problem to appdb admins.", "red"); retry("new", "Failed to create account"); } } @@ -193,7 +202,7 @@ function cmd_do_login() $user = new User(); $result = $user->login($aClean['ext_email'], $aClean['ext_password']); - if($result == true) + if($result == SUCCESS) { $_SESSION['current'] = $user; addmsg("You are successfully logged in as '$user->sRealname'.", "green"); diff --git a/include/user.php b/include/user.php index 7fb5580..e11221b 100644 --- a/include/user.php +++ b/include/user.php @@ -6,6 +6,14 @@ require_once(BASE."include/version.php"); require_once(BASE."include/util.php"); +define(SUCCESS, 0); +define(USER_CREATE_EXISTS, 1); +define(USER_CREATE_FAILED, 2); +define(USER_LOGIN_FAILED, 3); +define(USER_UPDATE_FAILED, 4); +define(USER_UPDATE_FAILED_EMAIL_EXISTS, 5); /* user updating to an email address that is already in use */ +define(USER_UPDATE_FAILED_NOT_LOGGED_IN, 6); /* user::update() called but user not logged in */ + /** * User class for handling users */ @@ -65,32 +73,32 @@ class User { { // Update timestamp and clear the inactivity flag if it was set query_appdb("UPDATE user_list SET stamp=NOW(), inactivity_warned='false' WHERE userid=".$this->iUserId); - return true; + return SUCCESS; } - return false; + return USER_LOGIN_FAILED; } /* * Creates a new user. - * returns true on success, false on failure + * returns SUCCESS on success, USER_CREATE_EXISTS if the user already exists */ function create($sEmail, $sPassword, $sRealname, $sWineRelease) { if(user_exists($sEmail)) { - addMsg("An account with this e-mail exists already.","red"); - return false; + return USER_CREATE_EXISTS; } else { $hResult = query_parameters("INSERT INTO user_list (realname, email, CVSrelease, password, stamp,". "created) VALUES ('?', '?', '?', password('?'), ?, ?)", $sRealname, $sEmail, $sWineRelease, $sPassword, "NOW()", "NOW()"); - if(!$hResult) addMsg("Error while creating a new user.", "red"); + if(!$hResult) return USER_CREATE_FAILED; $retval = $this->login($sEmail, $sPassword); - $this->setPref("comments:mode", "threaded"); /* set the users default comments:mode to threaded */ + if($retval == SUCCESS) + $this->setPref("comments:mode", "threaded"); /* set the users default comments:mode to threaded */ return $retval; } @@ -100,42 +108,55 @@ class User { /** * Update User Account; */ - function update($sEmail = null, $sPassword = null, $sRealname = null, $sWineRelease = null) + function update() { - if(!$this->isLoggedIn()) return false; + if(!$this->isLoggedIn()) return USER_UPDATE_FAILED_NOT_LOGGED_IN; - if ($sEmail) + /* create an instance of ourselves so we can see what has changed */ + $oUser = new User($this->iUserId); + + if($this->sEmail && ($this->sEmail != $oUser->sEmail)) { - if(user_exists($sEmail) && $sEmail != $this->sEmail) + /* make sure this email isn't already in use */ + if(user_exists($this->sEmail)) { addMsg("An account with this e-mail exists already.","red"); - return false; + return USER_UPDATE_FAILED_EMAIL_EXISTS; } - if (!query_appdb("UPDATE user_list SET email = '".addslashes($sEmail)."' WHERE userid = ".$this->iUserId)) - return false; - $this->sEmail = $sEmail; + if (!query_appdb("UPDATE user_list SET email = '".addslashes($this->sEmail)."' WHERE userid = ".$this->iUserId)) + return USER_UPDATE_FAILED; } - if ($sPassword) + if ($this->sRealname && ($this->sRealname != $oUser->sRealname)) { - if (!query_appdb("UPDATE user_list SET password = password('$sPassword') WHERE userid = ".$this->iUserId)) - return false; + if (!query_appdb("UPDATE user_list SET realname = '".addslashes($this->sRealname)."' WHERE userid = ".$this->iUserId)) + return USER_UPDATE_FAILED; } - if ($sRealname) + if ($this->sWineRelease && ($this->sWineRelease != $oUser->sWineRelease)) { - if (!query_appdb("UPDATE user_list SET realname = '".addslashes($sRealname)."' WHERE userid = ".$this->iUserId)) - return false; - $this->sRealname = $sRealname; + if (!query_appdb("UPDATE user_list SET CVSrelease = '".addslashes($this->sWineRelease)."' WHERE userid = ".$this->iUserId)) + return USER_UPDATE_FAILED; + } + return SUCCESS; + } + + /** + * NOTE: we can't update the users password like we can update other + * fields such as their email or username because the password is hashed + * in the database so we can't keep the users password in a class member variable + * and use update() because we can't check if the password changed without hashing + * the newly supplied one + */ + function update_password($sPassword) + { + if($sPassword) + { + if (query_appdb("UPDATE user_list SET password = password('$sPassword') WHERE userid = ".$this->iUserId)) + return true; } - if ($sWineRelease) - { - if (!query_appdb("UPDATE user_list SET CVSrelease = '".addslashes($sWineRelease)."' WHERE userid = ".$this->iUserId)) - return false; - $this->sWineRelease = $sWineRelease; - } - return true; + return false; } diff --git a/preferences.php b/preferences.php index aed5955..3ac05de 100644 --- a/preferences.php +++ b/preferences.php @@ -111,6 +111,7 @@ if($_POST) $oUser->setPref($arr[1], $value); } + /* make sure the user enters the same password twice */ if ($aClean['ext_password'] == $aClean['ext_password2']) { $str_passwd = $aClean['ext_password']; @@ -119,7 +120,20 @@ if($_POST) { addmsg("The Passwords you entered did not match.", "red"); } - if ($oUser->update($aClean['ext_email'], $str_passwd, $aClean['ext_realname'], $aClean['CVSrelease'])) + + /* update user data fields */ + $oUser->sEmail = $aClean['ext_email']; + $oUser->sRealname = $aClean['ext_realname']; + $oUser->sWineRelease = $aClean['CVSrelease']; + + /* if the password was empty in both cases then skip updating the users password */ + if($str_passwd != "") + { + if(!$oUser->update_password($str_passwd)) + addmsg("Failed to update password", "red"); + } + + if ($oUser->update() == SUCCESS) { addmsg("Preferences Updated", "green"); // we were managing an user, let's go back to the admin after updating tha admin status diff --git a/unit_test/path.php b/unit_test/path.php new file mode 100644 index 0000000..4ae17f9 --- /dev/null +++ b/unit_test/path.php @@ -0,0 +1,3 @@ + diff --git a/unit_test/run_tests b/unit_test/run_tests new file mode 100755 index 0000000..155f394 --- /dev/null +++ b/unit_test/run_tests @@ -0,0 +1 @@ +php -f run_tests.php diff --git a/unit_test/run_tests.php b/unit_test/run_tests.php new file mode 100644 index 0000000..0163edf --- /dev/null +++ b/unit_test/run_tests.php @@ -0,0 +1,11 @@ + \ No newline at end of file diff --git a/unit_test/test_user.php b/unit_test/test_user.php new file mode 100644 index 0000000..29bcd92 --- /dev/null +++ b/unit_test/test_user.php @@ -0,0 +1,285 @@ +login($test_email, $test_password) == SUCCESS) + { + $oUser->delete(); + $oUser = new User(); + } + + /* create the user */ + $retval = $oUser->create("testemail@somesite.com", "password", "Test user", "20051020"); + if($retval != SUCCESS) + { + if($retval == USER_CREATE_EXISTS) + echo "The user already exists!\n"; + else if($retval == USER_LOGIN_FAILED) + echo "User login failed!\n"; + else + echo "ERROR: UNKNOWN ERROR!!\n"; + + return false; + } + + /* try creating the user again, see that we get USER_CREATE_EXISTS */ + $retval = $oUser->create("testemail@somesite.com", "password", "Test user", "20051020"); + if($retval != USER_CREATE_EXISTS) + { + echo "Got '".$retval."' instead of USER_CREATE_EXISTS(".USER_CREATE_EXISTS.")\n"; + return false; + } + + return true; +} + +/* NOTE: relies on test_create_user() being run first and leaving a user */ +/* created in the db */ +function test_user_login() +{ + test_start(__FUNCTION__); + + global $test_email, $test_password; + + /* test that correct information results in a correct login */ + $oUser = new User(); + $retval = $oUser->login($test_email, $test_password); + if($retval != SUCCESS) + { + echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n"; + return false; + } + + /* test that incorrect user results in a login failed */ + $oUser = new User(); + $retval = $oUser->login("some nutty username", $testpassword); + if($retval != USER_LOGIN_FAILED) + { + echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n"; + return false; + } + + /* test that incorrect password results in a login failed */ + $oUser = new User(); + $retval = $oUser->login($test_email, "some password"); + if($retval != USER_LOGIN_FAILED) + { + echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n"; + return false; + } + + return true; +} + + +function test_user_update_set_test($realname, $winerelease) +{ + global $test_email, $test_password; + + /* log the user in */ + $oUser = new User(); + $retval = $oUser->login($test_email, $test_password); + if($retval != SUCCESS) + { + echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n"; + return false; + } + + /* modify the users realname and wine release */ + $oUser->sRealname = $realname; + $oUser->sWineRelease = $winerelease; + $oUser->update(); /* save the changes */ + + /* log the user in again */ + $oUser = new User(); + $retval = $oUser->login($test_email, $test_password); + if($retval != SUCCESS) + { + echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n"; + return false; + } + + /* make sure the realname and wine release match */ + if($oUser->sRealname != $realname) + { + echo "Realname of '".$oUser->sRealname."' doesn't match expected realname of '".$realname."'\n"; + return false; + } + + if($oUser->sWineRelease != $winerelease) + { + echo "Wine release of '".$oUser->sWineRelease."' doesn't match expected wine release of '".$winerelease."'\n"; + return false; + } + + return true; +} + +/* test that we can set values and call user::update() and have the values be saved */ +function test_user_update() +{ + test_start(__FUNCTION__); + + global $test_email, $test_password; + + if(!test_user_update_set_test("some bogus realname", "some crazy wine release")) + { + return false; + } + + if(!test_user_update_set_test("some new bogus realname", "some new crazy wine release")) + { + return false; + } + + return true; +} + +function test_user_delete() +{ + test_start(__FUNCTION__); + + global $test_email, $test_password; + + /* login the user */ + $oUser = new User(); + $retval = $oUser->login($test_email, $test_password); + if($retval != SUCCESS) + { + echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n"; + return false; + } + + /* delete the user */ + $oUser->delete(); + + /* try to log in again */ + $oUser = new User(); + $retval = $oUser->login($test_email, $test_password); + if($retval != USER_LOGIN_FAILED) + { + echo "Got '".$retval."' instead of USER_LOGIN_FAILED(".USER_LOGIN_FAILED.")\n"; + return false; + } + + + /* now create the user again and see that it is created successfully */ + + /* create the user */ + $oUser = new User(); + $retval = $oUser->create($test_email, $test_password, "Test user", "20051020"); + if($retval != SUCCESS) + { + if($retval == USER_CREATE_EXISTS) + echo "The user already exists!\n"; + else if($retval == USER_LOGIN_FAILED) + echo "User login failed!\n"; + else + echo "ERROR: UNKNOWN ERROR!!\n"; + + return false; + } + + return true; +} + +function test_user_getpref_setpref() +{ + test_start(__FUNCTION__); + + global $test_email, $test_password; + + /* login the user */ + $oUser = new User(); + $retval = $oUser->login($test_email, $test_password); + if($retval != SUCCESS) + { + echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n"; + return false; + } + + /* set a preference and retrieve it */ + $pref_key = "testpreference"; + $pref_value = "test value"; + $oUser->setPref($pref_key, $pref_value); + + $got_pref = $oUser->getPref($pref_key); + if($got_pref != $pref_value) + { + echo "Expected preference value of '".$pref_value."' got preference value of '".$got_pref."'\n"; + return false; + } + + return true; +} + + +/*************************/ +/* Main testing routines */ + +if(!test_user_create()) + echo "test_user_create() failed!\n"; +else + echo "test_user_create() passed\n"; + +if(!test_user_login()) + echo "test_user_login() failed!\n"; +else + echo "test_user_login() passed\n"; + +if(!test_user_update()) + echo "test_user_update() failed!\n"; +else + echo "test_user_update() passed\n"; + +if(!test_user_delete()) + echo "test_user_delete() failed!\n"; +else + echo "test_user_delete() passed\n"; + +if(!test_user_getpref_setpref()) + echo "test_user_getpref_setpref() failed!\n"; +else + echo "test_user_getpref_setpref() passed\n"; + +/* TODO: the rest of the user member functions we don't currently test */ + + + +/* clean up the user we created during testing */ +/* so the unit test leaves no trace that it ran */ +$oUser = new User(); + +/* delete the user if they already exist */ +if($oUser->login($test_email, $test_password) == SUCCESS) +{ + $oUser->delete(); + $oUser = new User(); +} + +?> \ No newline at end of file