Hi, I’m a PHP newbie and I’m working on a college project. I have developed a login form, it works when I enter the correct username and password but when I enter anything different, I don’t get the error message. I’ve tried var_dump the errors, error reporting but I don’t get any errors. I have changed the page to use bind parameters, moved session_start at the top, and a few other things I’m losing track of but it behaves the same! I don’t understand what am I doing wrong. I’ve read in a few topics that we shouldn’t create users or passwords using text but this is for learning at the moment. Can you assist me, please?
Below is the PHP code:
> <?php
> session_start();
> error_reporting(E_ALL);
> ini_set('display_errors', 1);
>
> if (isset($_POST["Login"])) {
> checkLogin($_POST["Name"], $_POST["Password"]);
> }
>
> function checkLogin($Username, $Password) {
>
> $servername = "localhost";
> $DB_username = "User1";
> $DB_password = "password";
>
> try {
> $conn = new PDO("mysql:host=" . $servername . ";dbname=e0912343_Fixtures",
>
> $DB_username, $DB_password);
>
> $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
>
> $statement = $conn->query("SELECT * FROM Admin WHERE Username='" . $Username ."' AND PASSWORD='" . $Password . "'");
>
> $result = $statement->fetch();
>
> if ($result == null) { // customer id and password doesn't match
>
> echo '<script type="text/javascript">alert("The username or password entered is not valid.
> Please enter a valid username and password");</script>';
>
> } else {
> // start the session
>
> $_SESSION['Name']= $Username;
>
> header("location: Admin.php"); // redirect user to the Admin.php web page
>
> }
> }
> catch(PDOException $e) {
>
> echo "An error occured: " . $e->getMessage();
> }
>
> $conn = null;
>
> }
>
> ?>
Hi there @Nessyd Welcome to the forums. I have anonymised your login credentials. Even if it is on localhost, it’s never a good idea to post usernames and passwords in a public place.
You’re actually missing a double quote to validate from. The OP’s code is correct.
That’s correct. The only change you would be doing is using password_hash() and password_verify() in place of weaker hashing algorithms. If you’re not using any, you’d have to use password_hash() on account registration and password_verify() on account login. You’d be at a much bigger advantage if you were using that compared to your peers.
The code in the OP is vulnerable to SQL Injection attacks. You should be using prepared statements when plugging data into a query, no matter what the source of the data is or how well you trust the source. Always treat any data to be used in a query as suspect, no matter the source!
Thank you all for the feedback! I have read each suggestion and opted to try a few: made the Java script message one line to eliminate the possibility of a carriage return. Added a password verify and use prepared statements. Sadly, none of it worked. The Login will just not verify the credentials, now it lets me in with any gibberish I enter. I took away the bind param values but left it as a prepared statement, still got the same result. Finally, I cleared my browser, made another page as “Login1” and copied the code on it, just to check if that’d do something else and it’s the same behavior. This time I used the PHP code checker (Thanks Wake689!) and got no issues with all the changes. Any more ideas? Many thanks!
Post your current not-working code and we can have a look.
It sounds as though yoour code was working initially except that the error message didn’t work and that somewhere in your changes you have tried something has got broken.
(btw, I’m not sure where all the chevrons came from in your initial code you posted?).
Since you have made changes to the code, you would need to post the current code to get help with it.
Some points about the first posted code -
Each user written function should be responsible for doing one thing only. A function named checkLogin should only check if the login (authentication) was successful or not and return the result to the calling code. That particularly named function should not be responsible for creating a database connection, displaying errors, setting session variables, or redirecting.
Every redirect needs an exit/die statement after it to stop php code execution.
Don’t unconditionally display raw database errors. Just let php catch and handle the exception from most database statements, where php will ‘automatically’ display/log the raw error information the same as for php errors. The exception to this rule is when inserting/updating duplicate user submitted data. In this case, your code should catch the exception and detect if the error number is for a duplicate index. For all other error numbers, just re-throw the exception and let php handle it.
When the user successfully logs in, you should store the user id (auto-increment primary index) in the session variable, then use that user id to query on each page request to get any other user data.
The redirect upon successful completion of the form processing should be to the exact same url of the current page to cause a get request for that page. If you want to display a one-time success message, store it in a session variable, then test, display, and clear that session variable at the appropriate location in the html document.
There’s no need to close database connections in your code. Php will automatically destroy all resources used on a page when the script ends, which will be a fraction of a second after the point where you tried to close the connection yourself.
Hi wake689, thank you for your help in advance! I think the chevrons may have come from not using this chat window correctly! I used the,</> symbol and the " " and I didn’t notice what happened! Below is my code and the HTML form - without chevrons
<?php
session_start();
if (isset($_POST["Login"])) {
checkLogin($_POST["Name"], $_POST["Password"]);
}
function checkLogin($Username, $Password) {
$servername = "localhost";
$DB_username = "User1";
$DB_password = "password";
try {
$conn = new PDO("mysql:host=" . $servername . ";dbname=e0912343_Fixtures", $DB_username, $DB_password);
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$statement = $conn->prepare("SELECT * FROM Admin WHERE Username='" . $Username ."' AND PASSWORD='" . $Password . "'");
$result = $statement->execute();
if ($result) {
if(password_verify($DB_password, $Password)) { // confirming password
echo "The password is correct";
}
$_SESSION['Name']= $Username;
header("location: Admin.php"); // redirect user to the Admin.php web page
}
else {
echo '<script type="text/javascript">alert("The username or password entered is not valid. Please enter a valid username and password")</script>';
}
}
catch(PDOException $e) {
echo "An error occured: " . $e->getMessage();
}
$conn = null;
}
?>
<html lang="en">
<head>
<title>Login</title>
</head>
<body>
<header>
<table>
<tr>
<td></td>
<td><h1>Fixtures Maintenance</h1></td>
</table>
</header>
<section>
<h1>Login</h1>
<form name="Login" method="post" action="<?php echo htmlspecialchars($_SERVER["PHP_SELF"]);?>">
<table>
<tr>
<td>Username:</td>
<td><input type="text" name="Name"></td>
</tr>
<tr>
<td>Password:</td>
<td><input type="password" name="Password"></td>
</tr>
<tr>
<td colspan=2><input type="submit" name="Login" value="Login"></td>
</tr>
</table>
</form>
</section>
</body>
</html>
The reason why it lets you in even if your password and username are incorrect is the following:
if(password_verify($DB_password, $Password)) { // confirming password
echo "The password is correct";
}
$_SESSION['Name']= $Username;
header("location: Admin.php"); // redirect user to the Admin.php web page
Follow that logic through - if the password does not match, ie if the “if” is false then you don’t echo the “password correct” message but it then goes on to assign the $_SESSION[‘Name’] and to redirect to Admin.php anyway.
The next issue is that I think you are using the database as the user password? You should never, ever, do this.
Do you have a registration page where the user creates a username and password?
Do you understand how password_hash and password_verify work? It means you never store the actual password on your database. Google it to make sure you understand how it works.
Thank you wake689! No, I’ve never used the pw hash nor verify before. I Googled how to code it and that’s and I thought I understood it but obviously I didn’t! Thank you so much!
This is looking ok so far. What I would do next is check the logic. When using the password_verify() function, you wouldn’t need to pass in the password in your WHERE clause. So instead, the query should be
$conn->prepare("SELECT * FROM Admin WHERE Username = :Username");
You would also have to bind the placeholder with the variable. Most people use either bindValue() or bindParam(). I prefer using an array and passing it into the execute() method which does the same thing.
Next, you’d have to check the password the user provided with the fully hashed password from that account using password_verify() when you’ve selected the user’s account.
Just a little reminder, since you’re using password_verify(), your passwords you have in the database should be changed immediately or you’re going to scratch your head at why your passwords are failing at the verify level. password_verify() only tries to verify against algorithms that are within password_hash(). If you still have password hashed in say MD5 or SHA or something, it’ll fail.
If you’re confused on how to get a hashed password, I’d suggest running a test file where you can just have a 1 liner like
That way, you can then copy the password that’s generated using password_hash() and paste it over your old plain text or old hashed passwords. Also, each page refresh when doing tests like these generates a new hash. So you won’t get the same hashed passwords every time which is what you want.
There’s more to your code that I am seeing, but I’ll let you catch up for now.
