Leaving aside absolutely everything about the code itself(but if you find stuff that's not ok feel free to comment on them).
Before you point somewhere else, this is about code review, comments although not code in themselves, are required for a good understanding of the code.
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_hashandpassword_verifywhich are now built in? – Benjamin Gruenbaum 4 hours ago