Skip to main content
added 917 characters in body
Source Link

Don't come out of PHP mode unless you're trying to send content to the browser. Doing...

return $data;
}
?>

<?php
$username = $password = $password_hash = '';

Will cause the new lines and blank spaces between PHP blocks to be echoed to the browser. Since this is not your intent, it can result in unintended and difficult to fix problems. Change to:

return $data;
}
$username = $password = $password_hash = '';

For the same reason, do not close the PHP tag at the end of the file unless you're trying to send more content to the browser. In signin.php for example, it makes sense to have the closing tag ?> because you need to send the HTML template below it to the browser. But the end of connection.php and logout.php should not have ?> since you're not trying to send more blank space to the browser.

I understood, what about the code?

I didn't give feedback on the logic because the other answers covered it well. The things that struck me the most were:

  1. You take the existence of a cookie as proof that the user is logged in. Nooooo! Anyone can send any cookie named anything to any site. There are a few good alternatives, but at a minimum your solution must ensure whatever you get from the user (eg: cookie, token) is vetted before you do anything else.
  2. You insert what the user provided directly in your MySQL queries. This is death if anybody wants to hurt you. Never trust content supplied by the browser as safe even if you wrote the website/client application. Look up and use parameterized queries.
  3. You use really weak hashing of your passwords. This is too easy to break. Use PHP's built in functions password_hash and password_verify
  4. You were smart enough to get started and build a working program, and insightful enough to hash your passwords and to realize that you probably missed some things. You'll be alright :-)

Don't come out of PHP mode unless you're trying to send content to the browser. Doing...

return $data;
}
?>

<?php
$username = $password = $password_hash = '';

Will cause the new lines and blank spaces between PHP blocks to be echoed to the browser. Since this is not your intent, it can result in unintended and difficult to fix problems. Change to:

return $data;
}
$username = $password = $password_hash = '';

For the same reason, do not close the PHP tag at the end of the file unless you're trying to send more content to the browser. In signin.php for example, it makes sense to have the closing tag ?> because you need to send the HTML template below it to the browser. But the end of connection.php and logout.php should not have ?> since you're not trying to send more blank space to the browser.

Don't come out of PHP mode unless you're trying to send content to the browser. Doing...

return $data;
}
?>

<?php
$username = $password = $password_hash = '';

Will cause the new lines and blank spaces between PHP blocks to be echoed to the browser. Since this is not your intent, it can result in unintended and difficult to fix problems. Change to:

return $data;
}
$username = $password = $password_hash = '';

For the same reason, do not close the PHP tag at the end of the file unless you're trying to send more content to the browser. In signin.php for example, it makes sense to have the closing tag ?> because you need to send the HTML template below it to the browser. But the end of connection.php and logout.php should not have ?> since you're not trying to send more blank space to the browser.

I understood, what about the code?

I didn't give feedback on the logic because the other answers covered it well. The things that struck me the most were:

  1. You take the existence of a cookie as proof that the user is logged in. Nooooo! Anyone can send any cookie named anything to any site. There are a few good alternatives, but at a minimum your solution must ensure whatever you get from the user (eg: cookie, token) is vetted before you do anything else.
  2. You insert what the user provided directly in your MySQL queries. This is death if anybody wants to hurt you. Never trust content supplied by the browser as safe even if you wrote the website/client application. Look up and use parameterized queries.
  3. You use really weak hashing of your passwords. This is too easy to break. Use PHP's built in functions password_hash and password_verify
  4. You were smart enough to get started and build a working program, and insightful enough to hash your passwords and to realize that you probably missed some things. You'll be alright :-)
Source Link

Don't come out of PHP mode unless you're trying to send content to the browser. Doing...

return $data;
}
?>

<?php
$username = $password = $password_hash = '';

Will cause the new lines and blank spaces between PHP blocks to be echoed to the browser. Since this is not your intent, it can result in unintended and difficult to fix problems. Change to:

return $data;
}
$username = $password = $password_hash = '';

For the same reason, do not close the PHP tag at the end of the file unless you're trying to send more content to the browser. In signin.php for example, it makes sense to have the closing tag ?> because you need to send the HTML template below it to the browser. But the end of connection.php and logout.php should not have ?> since you're not trying to send more blank space to the browser.