Skip to main content
added 123 characters in body
Source Link

The only thing I see that I would like to see changed is this

if($user === false){
      die('Incorrect username / password combination!');
} else {
    ...
}

Here you are relying on "very" strict conditions to fail the login, if that's not met for any reason they may get logged in....

It's not terrible because the other things that you should be doing your doing, it's just better if the login condition is strict and the failure is easy. If that makes sense.

Just sort of a best practice thing to keep in mind.

Here is an example of what can happen:

<?php
 //$user is undefined
if($user === false){
      die('Incorrect username / password combination!');
} else {
    echo "pass";
}

Output

 pass

Sandbox

As I said it's not terrible because your doing everything else right. But it's something I would change, just because.

The only other thing is you could get rid of some of these local variables that are one time use like this:

//Compare the passwords.
$validPassword = password_verify($passwordAttempt, $user['user_password']);

//If $validPassword is TRUE, the login has been successful.
if($validPassword){

Could just be

 if(password_verify($passwordAttempt, $user['user_password'])){

But, I get that it's easier to debug and all that when it's more verbose, so that is just my preference.

The only thing I see that I would like to see changed is this

if($user === false){
      die('Incorrect username / password combination!');
} else {
    ...
}

Here you are relying on "very" strict conditions to fail the login, if that's not met for any reason they may get logged in....

It's not terrible because the other things that you should be doing your doing, it's just better if the login condition is strict and the failure is easy. If that makes sense.

Just sort of a best practice thing to keep in mind.

Here is an example of what can happen:

<?php
 //$user is undefined
if($user === false){
      die('Incorrect username / password combination!');
} else {
    echo "pass";
}

Output

 pass

Sandbox

The only thing I see that I would like to see changed is this

if($user === false){
      die('Incorrect username / password combination!');
} else {
    ...
}

Here you are relying on "very" strict conditions to fail the login, if that's not met for any reason they may get logged in....

It's not terrible because the other things that you should be doing your doing, it's just better if the login condition is strict and the failure is easy. If that makes sense.

Just sort of a best practice thing to keep in mind.

Here is an example of what can happen:

<?php
 //$user is undefined
if($user === false){
      die('Incorrect username / password combination!');
} else {
    echo "pass";
}

Output

 pass

Sandbox

As I said it's not terrible because your doing everything else right. But it's something I would change, just because.

The only other thing is you could get rid of some of these local variables that are one time use like this:

//Compare the passwords.
$validPassword = password_verify($passwordAttempt, $user['user_password']);

//If $validPassword is TRUE, the login has been successful.
if($validPassword){

Could just be

 if(password_verify($passwordAttempt, $user['user_password'])){

But, I get that it's easier to debug and all that when it's more verbose, so that is just my preference.

Source Link

The only thing I see that I would like to see changed is this

if($user === false){
      die('Incorrect username / password combination!');
} else {
    ...
}

Here you are relying on "very" strict conditions to fail the login, if that's not met for any reason they may get logged in....

It's not terrible because the other things that you should be doing your doing, it's just better if the login condition is strict and the failure is easy. If that makes sense.

Just sort of a best practice thing to keep in mind.

Here is an example of what can happen:

<?php
 //$user is undefined
if($user === false){
      die('Incorrect username / password combination!');
} else {
    echo "pass";
}

Output

 pass

Sandbox