Login/logout script Error messages not working as intended

Beginners - Learning PHP
#1

I successfully created my first login/logout script using php. But I still have a few issues with it.

I now want to display an error when a user enters a wrong username or a wrong password.
I only want this error to show up whenever a user enter a wrong username OR a wrong password.

I’ve tried using an else statement, but when doing this the alert/notification pops up everytime I refresh or hard refresh the page.

How would I change my code to get this to work?

The way the files are in my github now, the error shows up everytime I refresh the page as well or even when I log out.

Since its a lot of code I added everything to my github:

thanks in advance for any help. I started with php since as today, so I dont know a lot yet.

#2

you should wrap the entire form processing code in an if condition request method is POST. Atm it runs every time you access login.php

#3

By that do you mean the html form code or the php code above that?

I’m really new to php and coding in general so it is still kind off chinese.

#4

the php code you process the form with. atm it runs regardless of having a submitted form to process or not

#5

You mean this portion of the code

   // Prepare our SQL, preparing the SQL statement will prevent SQL injection.
		if ($stmt = $con->prepare('SELECT id, password FROM accounts WHERE username = ?')) {
			// Bind parameters (s = string, i = int, b = blob, etc), in our case the username is a string so we use "s"
			$stmt->bind_param('s', $_POST['username']);
			$stmt->execute();
			// Store the result so we can check if the account exists in the database.
			$stmt->store_result();
				if ($stmt->num_rows > 0) {
				$stmt->bind_result($id, $password);
				$stmt->fetch();
				// Account exists, now we verify the password.
				// Note: remember to use password_hash in your registration file to store the hashed passwords.
				if ($_POST['password'] === $password) {
					// Verification success! User has loggedin!
					// Create sessions so we know the user is logged in, they basically act like cookies but remember the data on the server.
					session_regenerate_id();
					$_SESSION['loggedin'] = TRUE;
					$_SESSION['name'] = $_POST['username'];
					$_SESSION['id'] = $id;
					header('Location: index.php');
				} else {
					echo "<script type='text/javascript'>";
					echo "alert('Press Enter or OK to go to the login screen.');";
					echo "</script>";
				}
			} else {
				echo "<script type='text/javascript'>";
				echo "alert('Press Enter or OK to go to the login screen.');";
				echo "</script>";
			}
			$stmt->close();
		}

When I modify this piece of code and refresh, it gives me another alert to confirm or cancel and then still shows me the same error alert after confirming. So thats not working.

#6

Op, do not output internal system errors to the user and stop storing plain text passwords.

1 Like
#7

The storing of plain passwords is just for the test environment. It will change to hashed passwords when going to the live version.

About the internal error messages, could you ellaborate?
It doesnt solve the issue of showing the alerts when refreshing the login page after wrong username of password is entered.

I only want the error to show once and that is when the user enters after entering a wrong username or password. But when the user does a refresh of the page or a hard refresh, the error shouldnt show up anymore (Wrong Username or Wrong Password.

#8

This…

Failed to connect to MySQL: ’ . mysqli_connect_error());

#9

Thank you very much. Didnt notice that as I am new to php and coding.

How would I go about only showing up the Wrong Username and Wrong Password alert when a user actually enters a wrong username and password. Since right now it always shows this error when refreshing the page after your entered it once.
It also shows the error now when logging out.

#10

Rather than explain the whole login logic I will refer you to this login code. See what you can get from it.

#11

What I did now was modify my code like this:

<?php
	session_start();
		// Change this to your connection info.
		$DATABASE_HOST = 'localhost';
		$DATABASE_USER = 'root';
		$DATABASE_PASS = 'root';
		$DATABASE_NAME = 'cat_docqueue';
		// Try and connect using the info above.
		$con = mysqli_connect($DATABASE_HOST, $DATABASE_USER, $DATABASE_PASS, $DATABASE_NAME);

if ($_SERVER['REQUEST_METHOD'] == 'POST')
{
		// Prepare our SQL, preparing the SQL statement will prevent SQL injection.
		if ($stmt = $con->prepare('SELECT id, password FROM accounts WHERE username = ?')) {
			// Bind parameters (s = string, i = int, b = blob, etc), in our case the username is a string so we use "s"
			$stmt->bind_param('s', $_POST['username']);
			$stmt->execute();
			// Store the result so we can check if the account exists in the database.
			$stmt->store_result();
				if ($stmt->num_rows > 0) {
				$stmt->bind_result($id, $password);
				$stmt->fetch();
				// Account exists, now we verify the password.
				// Note: remember to use password_hash in your registration file to store the hashed passwords.
				if ($_POST['password'] === $password) {
					// Verification success! User has loggedin!
					// Create sessions so we know the user is logged in, they basically act like cookies but remember the data on the server.
					session_regenerate_id();
					$_SESSION['loggedin'] = TRUE;
					$_SESSION['name'] = $_POST['username'];
					$_SESSION['id'] = $id;
					header('Location: index.php');
				}
			}
			$stmt->close();
		}
}
?>

Now the issue is it doesn’t show any errors. It just redirects to the login.php file. I’m really trying to fix the issue myself, but being new to php it is very difficult to find the correct solution.

#12

After trials and errors I have found it is safer not to pull the password out of MySQL.

Here’s is the login script that I used

    public function read($email, $password) {
    $db = DB::getInstance();
    $pdo = $db->getConnection();
    /* Setup the Query for reading in login data from database table */
    $this->query = 'SELECT id FROM members WHERE email=:email AND password=:password';


    $this->stmt = $pdo->prepare($this->query); // Prepare the query:
    $this->stmt->execute([
        ':email' => $email,
        ':password' => hash('whirlpool', $password)
        ]); // Execute the query with the supplied user's parameter(s):

    $this->stmt->setFetchMode(PDO::FETCH_OBJ);
    unset($password);
    if ($this->user_id = $this->stmt->fetchColumn()) {
        return $this->user_id;
    } else {
        return FALSE;
    }
}

Just substitute $ for $this-> and get rid of of the public declaration. Then you will basically have a function.

This is the script that grabs the user’s id

$submit = filter_input(INPUT_POST, 'submit', FILTER_SANITIZE_FULL_SPECIAL_CHARS);
if (isset($submit) && $submit === 'enter') {
    //echo "<pre>" . print_r($_POST, 1) . "</pre>";
    $email = filter_input(INPUT_POST, 'email', FILTER_SANITIZE_EMAIL);
    $password = filter_input(INPUT_POST, 'password', FILTER_SANITIZE_FULL_SPECIAL_CHARS);
    $id = $login->read($email, $password);
    if ($id) {
        after_successful_login($id);
        header("Location: member_page.php");
        exit();
    } else {
        //echo "You're a Failure!<br>";
       /* The above for local testing only */
      /* I would either log in the failed attempt or just give a generic message 
         to the user */
    }
}

I didn’t really write the following after successful login function but modified it to fit my needs:

// Actions to preform after every successful login
function after_successful_login($id = NULL) {
    // Regenerate session ID to invalidate the old one.
    // Super important to prevent session hijacking/fixation.
    session_regenerate_id();
    $lifetime = 60 * 60 * 24 * 7;
    setcookie(session_name(), session_id(), time() + $lifetime);
    $_SESSION['user_id'] = $id;

    // Save these values in the session, even when checks aren't enabled 
    $_SESSION['ip'] = $_SERVER['REMOTE_ADDR'];
    $_SESSION['user_agent'] = $_SERVER['HTTP_USER_AGENT'];
    $_SESSION['last_login'] = time();
}

The main thing is to store a password hash in the database table and not to pull it out of it. I personally would NOT use password_verify and password_hash functions, but use the hash() function. (php.net will explain it better than I can) I personally don’t store anything in session other the the user’s id and when I need that information I just read it in from the database table using the user’s id. The less you have of the user’s information in sessions the better your are security wise, but nothing is 100 percent secure on the internet. Though you should make it as secure as possible.

#13

The rest of the PHP world disagrees with you. I really don’t want to be an ass but help on the beginners forum should definitely follow community best practices and guidelines so arguing against one of the definite great things that have happened to PHP in the later years is really troublesome

1 Like
#14

I agree with Jim and password_verify. I don’t really know if pulling the passwor from the database is negative or not. Unless a database is immune from memory leaks, espionage and the like, then i don’t see a problem with pulling it from a db. I pull it from the db but maybe it is wrong. Either way, i stick with password_verify. However, i would use a hash_equals for a username or email and not just equivalent checks (== or ===).

I don’t recommend telling a user which entry is incorrect. I simply respond with username and password cannot be verified. I’m not going to inform a hacker that one or the other is correct. Just sayin’

we are different and our code is different. The logic should be the same: accept a username/email and a password. check this with a database query. if one is incorrect, then set an error to true or 1. return to the login where an if block is placed near the form: if error is true or 1 then show message. I do this for incorrect login credentials, token expired messages and connectivity failures (i use a try catch block on the pdo connection which sets a connection error variable to 1).

Thus, you need to add an error variable (such as $loginCredentialsError) after you check if username matches db username and password verify db password, password and they do not match. You should return to the login page where a php if block will display an error.

#15

The password is hashed, so there’s no security risk for pulling it out. On the other hand, a good hash has a salt, so it’s different even on the same input. If you can verify a hash with string comparison, then it’s a bad hash.

2 Likes
#16

Thank you for clarifying this matter, Christian. I am thankful for PHPHelp forum because i am surrounded by very knowledgeable or expert level programmers. I need this environment to be a better programmer. I truly appreciate everyone here that helps beginners like me. A special thanks to astonecipher, phdr, jiml, benanamen and chorn. I’d probably give up if i didn’t have this forum to help me with my struggles.

really, Thank You.

Sponsor our Newsletter | Privacy Policy | Terms of Service