Password hashing and database
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
5
down vote
favorite
I would like to ask someone if the code i will put here is Secured for SQL injection and my password hashing and checking while user is logging in is secure and done well.
My Database connection :
$hostname='localhost';
$username='root';
$password='';
try
$dbh = new PDO("mysql:host=$hostname;dbname=myshop",$username,$password);
$dbh->query ('SET NAMES utf8');
$dbh->query ('SET CHARACTER_SET utf8_unicode_ci');
$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); // <== add this line
catch (PDOException $e)
echo $e->getMessage();
My registration script :
include_once 'config.php';
$data = file_get_contents("php://input");
$data = json_decode($data, true);
try
$stmt = $dbh->prepare("INSERT INTO `accounts` (`account_id`, `firstname`, `lastname`, `username`, `password`, `email`, `city`, `postalcode`, `adress`, `country`, `role`)
VALUES (NULL, :firstname, :lastname, :username, :password, :email, :city, :postalcode, :adress, :country, 0) ");
$stmt->bindParam(':firstname', $data['firstname']);
$stmt->bindParam(':lastname', $data['lastname']);
$stmt->bindParam(':username', $data['username']);
$stmt->bindParam(':password', password_hash($data['password'], PASSWORD_DEFAULT));
$stmt->bindParam(':email', $data['email']);
$stmt->bindParam(':city', $data['city']);
$stmt->bindParam(':postalcode', $data['postalcode']);
$stmt->bindParam(':adress', $data['adress']);
$stmt->bindParam(':country', $data['country']);
$stmt->execute();
catch (PDOException $e)
if (strpos($e->getMessage(), "for key 'login'") !== false)
echo 'Duplicate login entry';
exit;
if (strpos($e->getMessage(), "for key 'email'") !== false)
echo 'Duplicate e-mail entry';
exit;
else
throw $e;
echo 'Success';
My login script :
include_once 'config.php';
$data = file_get_contents("php://input");
$data = json_decode($data, true);
try
$stmt = $dbh->prepare("SELECT * from accounts WHERE
username=:username");
$stmt->bindParam(':username', $data['username']);
$stmt->execute();
catch (PDOException $e)
throw $e;
$rows = $stmt->rowCount();
$result = $stmt->fetchAll(PDO::FETCH_ASSOC);
$json = json_encode($result);
if ($rows === 1)
if (password_verify($data['password'], $result[0]['password']))
echo $json;
exit;
else
echo 'Wrong password';
exit;
if ($rows === 0)
echo 'No account';
exit;
else Â
up vote
5
down vote
favorite
I would like to ask someone if the code i will put here is Secured for SQL injection and my password hashing and checking while user is logging in is secure and done well.
My Database connection :
$hostname='localhost';
$username='root';
$password='';
try
$dbh = new PDO("mysql:host=$hostname;dbname=myshop",$username,$password);
$dbh->query ('SET NAMES utf8');
$dbh->query ('SET CHARACTER_SET utf8_unicode_ci');
$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); // <== add this line
catch (PDOException $e)
echo $e->getMessage();
My registration script :
include_once 'config.php';
$data = file_get_contents("php://input");
$data = json_decode($data, true);
try
$stmt = $dbh->prepare("INSERT INTO `accounts` (`account_id`, `firstname`, `lastname`, `username`, `password`, `email`, `city`, `postalcode`, `adress`, `country`, `role`)
VALUES (NULL, :firstname, :lastname, :username, :password, :email, :city, :postalcode, :adress, :country, 0) ");
$stmt->bindParam(':firstname', $data['firstname']);
$stmt->bindParam(':lastname', $data['lastname']);
$stmt->bindParam(':username', $data['username']);
$stmt->bindParam(':password', password_hash($data['password'], PASSWORD_DEFAULT));
$stmt->bindParam(':email', $data['email']);
$stmt->bindParam(':city', $data['city']);
$stmt->bindParam(':postalcode', $data['postalcode']);
$stmt->bindParam(':adress', $data['adress']);
$stmt->bindParam(':country', $data['country']);
$stmt->execute();
catch (PDOException $e)
if (strpos($e->getMessage(), "for key 'login'") !== false)
echo 'Duplicate login entry';
exit;
if (strpos($e->getMessage(), "for key 'email'") !== false)
echo 'Duplicate e-mail entry';
exit;
else
throw $e;
echo 'Success';
My login script :
include_once 'config.php';
$data = file_get_contents("php://input");
$data = json_decode($data, true);
try
$stmt = $dbh->prepare("SELECT * from accounts WHERE
username=:username");
$stmt->bindParam(':username', $data['username']);
$stmt->execute();
catch (PDOException $e)
throw $e;
$rows = $stmt->rowCount();
$result = $stmt->fetchAll(PDO::FETCH_ASSOC);
$json = json_encode($result);
if ($rows === 1)
if (password_verify($data['password'], $result[0]['password']))
echo $json;
exit;
else
echo 'Wrong password';
exit;
if ($rows === 0)
echo 'No account';
exit;
else
echo 'Something went wrong';
exit;
php
Your code is secure against SQL injection but it's largely inefficient. And this army of bindParams
's takes the largest part in it. All you need is just to send an array you already have directly into execute()
:
$stmt->execute($data);
Registration
On the other hand, your use of try
and catch
operator for insert is proper, which is a rare sight. The only objection is error messages that shouldn't be bluntly echoed out. Better to collect them and show nicely along with a form. So I would make the whole registration code as
$error = '';
$data['password'] = password_hash($data['password'], PASSWORD_DEFAULT);
try
$stmt = $dbh->prepare("INSERT INTO `accounts` (`account_id`, `firstname`, `lastname`, `username`, `password`, `email`, `city`, `postalcode`, `adress`, `country`, `role`)
VALUES (NULL, :firstname, :lastname, :username, :password, :email, :city, :postalcode, :adress, :country, 0) ");
$stmt->execute($data);
catch (PDOException $e)
if (strpos($e->getMessage(), "for key 'login'"))
$error = 'Duplicate login entry';
elseif (strpos($e->getMessage(), "for key 'email'"))
$error = 'Duplicate e-mail entry';
else
throw $e;
if ($error)
echo $error;
else
header("Location: ...");
exit;
Login
The login code could be improved as well. I've got a ready made example, Authenticating a user using PDO and password_verify(), you may check it out for the actual code. Here I will just comment on some parts
} catch (PDOException $e)
throw $e;
I fancy you just copy-pasted this part from the insert example. But if you think on it, it will become clear that this code will behave exactly the same without try, catch and throw operators! So they should be simple taken out.
$rows = $stmt->rowCount();
is essentially useless, as you have the fetch result that can be used instead.
$result = $stmt->fetchAll(PDO::FETCH_ASSOC);
fetchAll()
doesn't seem to be a wise choice, as it adds unnecessary dimension to the resulting array. fetch()
should be used instead.
edited Apr 2 at 10:14
t3chb0t
32.1k54195
32.1k54195
answered Apr 2 at 9:51
Your Common Sense
2,415524
2,415524
add a comment |Â
Thank you for your review of my code and a very good response. I will try to workout my code with all your tips and give u feedback about how it went ! :)
â SupremeDEV
Apr 3 at 12:59
Thank you for your review of my code and a very good response. I will try to workout my code with all your tips and give u feedback about how it went ! :)
â SupremeDEV
Apr 3 at 12:59
Thank you for your review of my code and a very good response. I will try to workout my code with all your tips and give u feedback about how it went ! :)
â SupremeDEV
Apr 3 at 12:59
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f190303%2fpassword-hashing-and-database%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Thank you for your review of my code and a very good response. I will try to workout my code with all your tips and give u feedback about how it went ! :)
â SupremeDEV
Apr 3 at 12:59