Leaving aside absolutely everything about the code itself (but if you find stuff that's not ok feel free to comment on them).
What I'm asking here is the following:
- Is my code properly commented? Should I add more comments?
- Is it understandable as it stands?
- Are there some general rules that I should adhere to when commenting?
- Could my comments be improved in any way?
I'll leave a few classes I've been working on.
class ChangePassword
:
class ChangePassword{
private $_errors = array();
public function __construct(){
require_once 'PasswordHash.php';
require_once 'ValidateData.php';
require_once 'SqlQueryController.php';
if(isset($_POST['resetPassword'])){
/**
* @param associative array
* stripAllWhiteSpaces will remove ALL white spaces.
* example: $stringBefore = ' this is an example';
* $stringAfter = 'thisisanexample';
*/
$credentials = ValidateData::stripAllWhiteSpaces(array('passwordCurrent' => $_POST['passwordCurrent'],
'passwordNew' => $_POST['passwordNew'],
'passwordNewAgain'=> $_POST['passwordNewAgain']
)
);
$this->doResetPassword($credentials);
}
}
public function doResetPassword($credentials){
/**
* @bool returns true if value is empty
*/
if(ValidateData::isEmpty($credentials)){
$_errors[] = '<p>Some fields are empty</p>';
} else {
if($credentials['passwordNew'] != $credentials['passwordNewAgain']){
$_errors[] = '<p>Passwords do not match.</p>';
}
if($credentials['passwordNew'] == $credentials['passwordCurrent']){
$_errors[] = '<p>Your new password cannot be the same as your old password.</p>';
}
/**
* @bool
* Example of valid password: Thequickbrown200!
*/
if( ! ValidateData::pregMatch('/^(?=.*\d)(?=.*[A-Za-z])[0-9A-Za-z!@#$%]{5,200}$/', $credentials['passwordNew'])){
$_errors[] = '<p>The password must be between 5 and 200 characters long, must contain at least one number, at least one letter and at least one non Alphanumeric character.</p>';
}
}
if( ! empty($_errors)){
foreach($_errors as $error){
echo $error;
}
return;
}
$this->insertPassword($credentials);
$_SESSION = array();
session_destroy();
}
public function insertPassword($credentials){
$passwordHash = new PasswordHash();
$hashedPassword = $passwordHash->hashPassword($credentials['passwordNew']);
$usernameId = $_SESSION['id'];
$sqlQueryController = new SqlQueryController();
$query = "UPDATE users_table
SET login_password=:passwordNew
WHERE login_id=:usernameId";
$array = array(':passwordNew' => $hashedPassword,
':usernameId' => $usernameId);
if($sqlQueryController->runQueryExecute($query, $array)){
echo '<p>Successfully changed the password</p>';
} else {
echo '<p>An error occurred while changing the password</p>';
}
}
}
class Login
:
<?php
class Login{
public function __construct(){
require_once 'PasswordHash.php';
require_once 'SqlQueryController.php';
require_once 'ValidateData.php';
if(isset($_POST['login'])){
/**
* @param associative array
* stripAllWhiteSpaces will remove ALL white spaces.
* example: $stringBefore = ' this is an example';
* $stringAfter = 'thisisanexample';
*/
$credentials = ValidateData::stripAllWhiteSpaces(array('username' => $_POST['username'],
'password' => $_POST['password']
)
);
$this->doLogin($credentials);
}
}
/**
* Log in with post data
*/
public function doLogin($credentials){
if( ! ValidateData::isEmpty($credentials)){
$passwordHash = new PasswordHash();
$sqlQueryController = new SqlQueryController();
$query = "SELECT login_password
FROM users_table
WHERE login_username=:username
LIMIT 1";
$array = array(':username' => $credentials['username']);
/**
* @param associative array
* return an associative array using PDO's fetch();
*/
$hash = $sqlQueryController->runQueryFetch($query, $array);
/**
* @bool
* verifies password based on the $hash
* and the password provided by the user
*/
$passwordVerify = $passwordHash->verifyPassword($credentials['password'], $hash['login_password']);
$query = "SELECT login_username, login_id
FROM users_table
WHERE login_username=:username LIMIT 1";
$array = array(':username' => $credentials['username']);
$userVerify = $sqlQueryController->runQueryFetch($query, $array);
if(($passwordVerify == 1) && ($userVerify['login_username'] == $credentials['username'])){
/**
* Great, the user's logged in
* Time to set the session and redirect him
*/
$_SESSION['id'] = $userVerify['login_id'];
$_SESSION['username'] = $userVerify['login_username'];
#session_write_close();
header('Location: logged_in.php');
die();
} else {
echo '<p> The username or password do not match any registered users.</p>';
}
} else {
echo '<p> You must fill in all fields.</p>';
}
}
}
class RecoverPassword
:
<?php
class RecoverPassword{
public function __construct(){
require_once 'PasswordHash.php';
require_once 'SendMailRecoverPassword.php';
require_once 'ValidateData.php';
require_once 'SqlQueryController.php';
if(isset($_POST['recoverPassword'])){
/**
* @param associative array
* stripAllWhiteSpaces will remove ALL white spaces.
* example: $stringBefore = ' this is an example';
* $stringAfter = 'thisisanexample';
*/
$credentials = ValidateData::stripAllWhiteSpaces(array('email' => $_POST['email']
)
);
$this->doRecoverPassword($credentials);
}
}
public function doRecoverPassword($credentials){
if(ValidateData::validateEmail($credentials['email'])){
$sqlQueryController = new SqlQueryController();
$query = "SELECT login_email
FROM users_table
WHERE login_email=:email LIMIT 1";
$array = array(':email' => $credentials['email']);
$emailExist = $sqlQueryController->runQueryFetchAssoc($query, $array);
if($emailExist){
/**
* If a proper SMTP is not configured, the password
* will not be changed and the page will die
* with a user friendly error.
* A more useful error can be found in the log fiels
* The __construct() of the class is built
* in such a way that it will throw the exception and die
* after. Point is, don't move this further down the page
* or the password WILL be changed but the email will NOT
* be sent if the SMTP is not configured!
*/
$swift = new SendMailRecoverPassword();
$passwordHash = new PasswordHash();
/*
* Create a random string of letteres and numbers
* This will be the users new password
*/
$randomPassword = str_shuffle('abcdefghijklmnopqrstqwxz0123456789ABCDEFGHIJKLMNOPQRSTWXZ');
/**
* Hash the random string
*/
$newPassword = $passwordHash->hashPassword($randomPassword);
/**
* Update the new hashed password
* replacing the old password
*/
$query = "UPDATE users_table
SET login_password=:password
WHERE login_email=:email LIMIT 1";
$array = array(':password' => $newPassword,
':email' => $credentials['email']);
$sqlQueryController->runQueryExecute($query, $array);
/**
* Create the message
*/
$swift->createMessage($randomPassword, $credentials['email']);
/**
* Return the newly created message
*/
$message = $swift->getMessage();
/**
* Send the message
*/
if($swift->sendMessage($message)){
echo '<p>Check your inbox for the new password. Your old password will no longer work</p>';
}
} else {
echo '<p>Email doesn\'t exist!</p>';
}
} else {
echo '<p>Email is invalid</p>';
}
}
}
password_hash
andpassword_verify
which are now built in? \$\endgroup\$ – Benjamin Gruenbaum Sep 23 '14 at 22:21