Login to page with account info that is saved in a text file php

Question

I have the following form

<form method="post" action="login.php">
    <table border="0" cellpadding="0">
        <tr>
            <td align="center" colspan="5">
                    <h4>Diary Login <br /><small>Please Login to manage Diary posts.</small></h4>
            </td>
        </tr>
        <tr>
            <td align="right"><b>Username:</b></td>
            <td align="left"><input type="text" name="username" /></td>                     
        </tr>   
        <tr>
            <td align="right"><b>Password:</b></td>
            <td align="left"><input type="password" name="password"/></td>
        </tr>

        <tr>
            <td align="left"><input type="submit" value="Login" name="submitBtn"/>
            <input type="reset" value="Reset" name="reset"/></td>
        </form>

This form gets a username and password from a user and then submits it to they following php code for validation.

<?php
$username = $_POST['username'];
$password = $_POST['password'];

if (isset($_POST['submitBtn'])){
$file = explode( "PHP_EOL", file_get_contents( "accounts.txt" ));
foreach( $file as $line ) {
list($username,$password) == explode( "||", $line );

if($_POST['username'] = $username && $_POST['password']) {
// User authenticated correctly
echo "You've successfully logged in!";
echo "<br>";
} else {
// User did not authenticate
echo "Invalid Username or Password";
}
}
} 
?>

I have a text file called accounts.txt that is storing user data in the following format

user1,password123
user2,passwordabc

I want to be able to check the entered data to see if it matches what is registered in the text file. however when i run this code i get Invalid Username or Password every time even though the data i entered is in the file. so i enter the following and i still get Invalid username or password even though these values are stored in my accounts.txt file

user1
password123

are register in the file. (i know it is not safe to store data in a text file but it is what I am told to do for education purposes if it where up to me i would you a database but i cant). Thanks in advance.

Answer 1

You are using PHP_EOL as string instead of as a constant:

"PHP_EOL"

should be

PHP_EOL

Also, according to your information, the data for each account is separated by a comma, but you try exploding it using a double pipe: ||

Therefore

explode( "||", $line )

should be

explode(",", $line)

OR

If you want to use the double pipe to separate username from password in your text file, you can keep your former explode() function but then you must make sure that the double pipe is actually used.

Also, you are using == for an assignment, but it is only meant for comparison. In order to correctly assign explode( ",", $line ) to list($username,$password) you need to use =:

list($username,$password) = explode(",", $line);

Hints

Please note that when using a comma to separate username from password, and you do not have any control over how the passwords are designed, you might run into a situation such as this:

 // username,password
    some_name,myPass,word

Meaning that if someone picks a comma as part of their password, your call to explode() will return unexpected information.

Please also note that storing user information in a text file like this is quite bad practice because (as it seems, the way you present your code) you have an environment in which the passwords are stored in plain text in that file. That is a security risk. Just in case you care for that.

---

UPDATE #1

I found another problem here:

if($_POST['username'] = $username && $_POST['password'])

Here you use a single = sign for a comparison, which should be ==:

if($_POST['username'] == $username && $_POST['password'] == $password)

Please note that since you didn't actually compare the passwords, i changed $_POST['password'] to $_POST['password'] == $password.

---

UPDATE #2

The reason why you are getting multiple outputs is because you wrapped the entire authentication check in a foreach loop. Your problem is that your code structure allows the loop to continue executing when it is no longer needed (when a successful login was performed).

You could update your loop to this:

$auth = false;

foreach( $file as $line ) {
    list($username, $password) = explode(",", $line);
    if ($_POST['username'] == $username && $_POST['password'] == $password) {
        $auth = true;
        break; // use this to stop iterating over $file and end the foreach-loop
    }
}

if($auth) {
    echo "Login successfull!";
} else {
    echo "Invalid username or password";
}

---

UPDATE #3

If now you feel like further improving your code, you can go on by wrapping the entire login functionality into a function by defining that function on top of your script.

function authenticate($username, $password){
    $file = explode( PHP_EOL, file_get_contents( "accounts.txt" ));
    foreach( $file as $line ) {
        list($username, $password) = explode(",", $line);
        if ($_POST['username'] == $username && $_POST['password'] == $password)
            return true;
    }
    return false;
}

And then call this function to do the authentication like this:

if(authenticate($_POST['username'], $_POST['password'])) {
    echo "Login successfull!";
} else {
    echo "Invalid username or password";
}