0

I have a website I'm trying to add multiple admins to. When logging in, only the first username/password combination in the database works and any others just throw the WRONG INFORMATION error message in the code below. Seems like a problem with the foreach maybe? I'm new to PHP and not sure how else to do it:

<?php
session_start();
include_once('connection.php');


function test_input($data) {

    $data = trim($data);
    $data = stripslashes($data);
    $data = htmlspecialchars($data);
    return $data;
}

if ($_SERVER["REQUEST_METHOD"]== "POST") {

    $adminname = test_input($_POST["adminname"]);
    $password = test_input($_POST["password"]);
    $stmt = $conn->prepare("SELECT * FROM adminlogin");
    $stmt->execute();
    $users = $stmt->fetchAll();

    foreach($users as $user) {

        if(($user['adminname'] == $adminname) &&
            ($user['password'] == $password)) {
              $_SESSION["authenticated"]="test";
                header("Location: adminpage");
        }
        else {
            echo "<script language='javascript'>";
            echo "alert('WRONG INFORMATION')";
            echo "</script>";
            die();
        }
    }
}

?>

Thank you!

murphocles
  • 11
  • 2
  • The else statement is causing the issue. – shahidiqbal Apr 29 '21 at 21:24
  • You need to set a flag with default value false before the if condition. Remove the else condition. On execution of if condition set the flag to true – shahidiqbal Apr 29 '21 at 21:26
  • This is so wrong. You never store plaintext passwords in a database. Use `password_hash()` and `password_verify()`. And why would you not just check the database for a particular username? – miken32 Apr 29 '21 at 21:27
  • You can also match the login credentials directly in the query in the where clause. That is recommended solution rather than to loop through all records. – shahidiqbal Apr 29 '21 at 21:28

1 Answers1

1

You're selecting all the users:

$stmt = $conn->prepare("SELECT * FROM adminlogin");

Then iterating through each of them one at a time:

foreach($users as $user) {

But if the very first iteration isn't the right user, you hit die() and abort the whole program. So only the first user can ever log in.

Don't select all users. The more users you have, the slower and slower this will get. Instead, select a maximum of one row by using a WHERE clause:

$stmt = $conn->prepare("SELECT * FROM adminlogin WHERE username = :username");
$stmt->execute(["username" => $adminname]);

Then verify that you get one row back (i.e., that you had a valid username), then verify that the password matches.

Make sure you build your queries with prepared statements with bound parameters. See this page and this post for some good examples.

Also, never store plain text passwords. Instead use password_hash() and password_verify(). Examples here.

And you generally don't want to filter/encode passwords with functions like stripslashes(), trim(), etc. -- because they'll cut valid characters out.

Alex Howansky
  • 50,515
  • 8
  • 78
  • 98