1
if(isset($_POST['signin'])){
    $password = $_POST['password'];
    $email = $_POST['email'];
    $query = "SELECT * from `accounts`;";
    if(count(fetchAll($query)) > 0){ //this is to catch unknown error.
          foreach(fetchAll($query) as $row){
            if($row['email']==$email&&$row['password']==$password){
                $_SESSION['login'] = true;
                $_SESSION['type'] = $row['type'];
                header('location:admin.php');
            }else{
                echo "<script>alert('Wrong login details.')</script>";
                header("Location:login.php");
            }
        }
    }else{
        echo "<script>alert('Error.')</script>";
    }
}

When I enter wrong email address I get an alert 4 times, and I also have 4 users. Does that mean each row is getting counted and I am getting alert after each user is checked. Please help me fix this.

Nick
  • 138,499
  • 22
  • 57
  • 95
Maya
  • 27
  • 7
  • 1
    add die(); after each header – db1975 Feb 04 '20 at 00:42
  • Thats because your select does_'t filter the email adress. Second your code seems all kind of wrong, because you check password against the database field, which passwords should always be securely hashed. third take a look add pdo for futre development – nbk Feb 04 '20 at 00:42
  • Better way is to select with the given email (escape the input for sql injection) and not to foreach every data in the database. $query = "SELECT * from ``accounts`` WHERE email='".$email."';"; – db1975 Feb 04 '20 at 00:47
  • You're also outputting before header with the echo before header. You can't have both, it's one or the other. – Funk Forty Niner Feb 04 '20 at 00:52
  • Thankyou everyone. – Maya Feb 04 '20 at 00:56

1 Answers1

1

You're getting 4 messages because each of the 4 values in the database doesn't match the value you are checking. You need to move the "Wrong login details" alert out of your foreach loop and instead use $_SESSION['login'] to indicate whether there was a match or not, if not, then output the "Wrong login" alert. Something like this:

$_SESSION['login'] = false;
$users = fetchAll($query);
if (count($users) > 0) { //this is to catch unknown error.
    foreach ($users as $row){
        if($row['email']==$email&&$row['password']==$password){
            $_SESSION['login'] = true;
            $_SESSION['type'] = $row['type'];
            break;
        }
    }
    if ($_SESSION['login']) {
        header('location:admin.php');
    }
    else {
        echo "<script>alert('Wrong login details.')</script>";
    }
}

Notes

  1. You should never store passwords in plain text, instead use password_hash and password_verify to store and check them.

  2. You are fetching all the results of the query twice, you should just fetch once into an array and then iterate the array in the foreach loop. My code does that.

  3. You can't echo an alert and use a header statement. Presuming this is login.php, there is no need for the header statement if you fail anyway.

  4. It would be better to simply SELECT * FROM accounts WHERE email = '$email' and see if any rows were returned, rather than iterating through all values in the table. This will save time when the table gets large. If you do that, you will need to use a prepared statement to avoid the risk of SQL injection. See this Q&A for details on how to do that.

Nick
  • 138,499
  • 22
  • 57
  • 95
  • Thanks that helped me. Also I will look into hashing the password for security. – Maya Feb 04 '20 at 00:56
  • @Maya I'm glad to hear it. I've added some notes based on the comments received to the question as well. – Nick Feb 04 '20 at 01:06