My php is not working when I enter a username and a password but works when I send the login form empty. What am I doing wrong?

[EnriqueL]

I have developed a code for a login and seems to work well (No syntax error according to https://phpcodechecker.com/ but when I enter a username and a password in the login form, I get an error HTTP 500. I think that everything is ok in the code but obviously there is something that I am not thinking about. The code (excluding db connection):

$id="''"; $username = $_POST['username']; $password = md5($_POST['password']); $func = "SELECT contrasena FROM users WHERE username='$username'";

$realpassask = $conn->query($func); $realpassaskres = $realpassask->fetch_assoc(); $realpass= $realpassaskres[contrasena];

$func2 = "SELECT bloqueado FROM users WHERE username='$username'"; $blockedask = $conn->query($func2); $blockedres = $blockedask->fetch_assoc(); $bloqueado = $blockedres[bloqueado];

//Login if(!empty($username)) { // Check the email with database
$userexists = $pdo->prepare("SELECT COUNT(username) FROM users WHERE username= '$username' LIMIT 1"); $userexists->bindParam(':username', $username); $userexists->execute(); // Get the result $userexistsres = $userexists->fetchColumn(); // Check if result is greater than 0 - user exist if( $userexistsres == 1) { if ($bloqueado == NO) { if ($password != $realpass) { die("contrasena incorrecta"); } Else{ $_SESSION['loguin']="OK"; $_SESSION['username']="$username"; header("Location: ./index.php"); } } Else{ die("Tu usuario ha sido bloqueado o todavía no ha sido aceptado por un administrador. } Else{ die("No hay ninguna cuenta con este nombre de usuario"); } } else { echo 'El campo usuario esta vacio'; } ?>

[Barand]

12 minutes ago, EnriqueL said:

The code (excluding db connection):

Pity, because you need show that bit too.

The first half of your code uses a mysqli connection and the latter half uses PDO. Do you have two connections?

Any chance you could format your code into something legible next time and place it in a code box (<> button in toolbar)? A few line breaks and indents would be appreciated.

[EnriqueL]

Wow I am so sorry about the spacing, I have just checked that out and didn't notice. No, I only have a mysqli connection here it goes the entire code:

 <?php
session_start();
$servername = "localhost";
$dbusername = "";
$dbpassword = "";
$dbname = "";
$conn = new mysqli($servername, $dbusername, $dbpassword, $dbname);
if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}
$id="''"; $username = $_POST['username']; $password = 
 md5($_POST['password']); $func = "SELECT contrasena FROM users WHERE 
 username='$username'";

 $realpassask = $conn->query($func); $realpassaskres = 
 $realpassask->fetch_assoc(); $realpass= $realpassaskres[contrasena];

 $func2 = "SELECT bloqueado FROM users WHERE username='$username'"; 
 $blockedask = $conn->query($func2); $blockedres = 
 $blockedask->fetch_assoc(); $bloqueado = $blockedres[bloqueado];

 //Login if(!empty($username)) {    // Check the email with database     
    $userexists = $pdo->prepare("SELECT COUNT(username) FROM users WHERE 
 username= '$username' LIMIT 1");   $userexists->bindParam(':username', 
 $username);    $userexists->execute();
        // Get the result   $userexistsres = $userexists->fetchColumn();
        // Check if result is greater than 0 - user exist   if( $userexistsres == 1)    {
       if ($bloqueado == NO) {
             if ($password != $realpass) {
             die("contrasena incorrecta");
             } Else{
             $_SESSION['loguin']="OK";
             $_SESSION['username']="$username";
             header("Location: ./index.php");
             }
         } Else{
             die("Tu usuario ha sido bloqueado o todavía no ha sido aceptado por un administrador.
         } Else{
            die("No hay ninguna cuenta con este nombre de usuario");
        } } else {
         echo 'El campo usuario esta vacio'; } ?>

And once again sorry for sending the code all in line I didn't notice

[EnriqueL]

Quote

 

<?php

session_start();
$servername = "localhost";
$dbusername = "";
$dbpassword = "";
$dbname = "";
$conn = new mysqli($servername, $dbusername, $dbpassword, $dbname);
if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}

$id="''";
$username = $_POST['username'];
$password = md5($_POST['password']);
$func = "SELECT contrasena FROM users WHERE username='$username'";

$realpassask = $conn->query($func);
$realpassaskres = $realpassask->fetch_assoc();
$realpass= $realpassaskres[contrasena];

$func2 = "SELECT bloqueado FROM users WHERE username='$username'";
$blockedask = $conn->query($func2);
$blockedres = $blockedask->fetch_assoc();
$bloqueado = $blockedres[bloqueado];

//Login
if(!empty($username))
{
    // Check the email with database
    
    $userexists = $pdo->prepare("SELECT COUNT(username) FROM users WHERE username= '$username' LIMIT 1");
    $userexists->bindParam(':username', $username);
    $userexists->execute();
     
    // Get the result
    $userexistsres = $userexists->fetchColumn();
     
    // Check if result is greater than 0 - user exist
    if( $userexistsres == 1)
    {
       if ($bloqueado == NO) {
            if ($password != $realpass) {
            die("contrasena incorrecta");
            } Else{
            $_SESSION['loguin']="OK";
            $_SESSION['username']="$username";
            header("Location: ./herramientas.php");
            }
        } Else{
            die("Tu usuario ha sido bloqueado o todavía no ha sido aceptado por un administrador. Si el problema persiste contacta con contacto@leonmacias.com");
        }
        } Else{
            die("No hay ninguna cuenta con este nombre de usuario");
        }
} else {
        echo 'El campo usuario esta vacio';
}
?>

 

 

[gizmola]

The first step towards writing decent code is to properly indent and format your  code.  Don't put multiple lines of code on the same line.  You should have a newline at the end of each line.  You should have indentation for any blocks.  PHP is case sensitive for most things other than function names and class names.  Be consistent.  Make all control statements (if-then-else) lower case.

//Login if(!empty($username)) {    // Check the email with database     
    $userexists = $pdo->prepare("SELECT COUNT(username) FROM users WHERE 
 username= '$username' LIMIT 1");   $userexists->bindParam(':username', 
 $username);    $userexists->execute();

The $pdo variable doesn't exist, however this is where it looks like you dropped in some PDO code.  

The consensus of experts at phpfreaks is that PDO is the better database API to use, so we'd recommend you convert everything to pdo anyways.

[EnriqueL]

Thank you very much for the answer. You are right, having everything into a couple of lines was a mistake of copying and pasting from my code editor but defenitely I would never do it like that. I have just checked out the else correction but unfortunately it didn't fix this issue. I guess that it is because of the pdo code that you talked about that to be honest, I have no idea about. In a near future I will consider to switch to pdo but right now my knowledge is not good enough to do it. May I ask you what should I do to convert that pdo into mysql to make this work? And again thank you for this

[gizmola]

I overstated the issue with else.  It's bad form, but not an error.  The uninitialized variable is probably the reason things don't work as you expect.  I fixed a few issues and formatted your code properly:

 

<?php

session_start();
$servername = "localhost";
$dbusername = "";
$dbpassword = "";
$dbname = "";
$conn = new mysqli($servername, $dbusername, $dbpassword, $dbname);
if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}

$id="";
$username = $_POST['username'];
$password = md5($_POST['password']);
$func = "SELECT contrasena FROM users WHERE username='$username'";

$realpassask = $conn->query($func);
$realpassaskres = $realpassask->fetch_assoc();
$realpass = $realpassaskres[contrasena];

$func2 = "SELECT bloqueado FROM users WHERE username='$username'";
$blockedask = $conn->query($func2);
$blockedres = $blockedask->fetch_assoc();
$bloqueado = $blockedres[bloqueado];

//Login
if(!empty($username)) {
    // Check the email with database
    
    $userexists = $pdo->prepare("SELECT COUNT(username) FROM users WHERE username= '$username' LIMIT 1");
    $userexists->bindParam(':username', $username);
    $userexists->execute();
     
    // Get the result
    $userexistsres = $userexists->fetchColumn();
     
    // Check if result is greater than 0 - user exist
    if ($userexistsres == 1) {
        if ($bloqueado == NO) {
            if ($password != $realpass) {
                die("contrasena incorrecta");
            } else {
                $_SESSION['loguin']="OK";
                $_SESSION['username']="$username";
                header("Location: ./herramientas.php");
                exit;
            }
        } else {
            die("Tu usuario ha sido bloqueado o todavía no ha sido aceptado por un administrador. Si el problema persiste contacta con contacto@leonmacias.com");
        }
    } else {
      die("No hay ninguna cuenta con este nombre de usuario");
    }
} else {
    echo 'El campo usuario esta vacio';
}

 

For example, you had $id = "''";  

Not sure what you were trying to do there.  If you are initializing it to a null equivalent empty string then just use "" or ''

I removed the ending '?>' from the file.  You don't need it and it's best not to have end block statements as they can in some circumstances cause issues.    

I'd recommend looking at PSR-2 and adopting those standards.

Something odd about your code is when you do 2 queries in a row where USERNAME = '$username'.   Do one query and either SELECT * or SELECT contrasena,  bloqueado.

Whenever you have a header('Location:...) you need to follow that with exit/die.  (They are the same function, but most people use exit).

Of course currently you are doing those queries and yet you do nothing with them.  

Also because you are not using prepared statements with bound parameters, your code will allow SQL injection.  

Again, our advice is that you use PDO. Here's a tutorial that will teach you everything you need to know.

 

[EnriqueL]

Thank you for all the information given and for spending time on this! I will defenitely look for more information about that danger for sqlinjection with my code as for information about PDO. Regarding the corrections you have made, unfortunately, I have applied them to my code and I am still getting the same issue. If you want you can try to get in and check it out http://leonmacias.com/senasa/

Before you told me that pdo wasn't defined. That's because I got the code for verifyng if the user was in the database before sending the message warning them to be blocked if login was rejected as I didn¡t know how to do it. I guess that that line is wrong. What should I do to make that work?

I have sent in the message before the entire code except for the db username, name and password so if you see something that is missing and that you have been there defenitely is my mistake.

I would like to apologize again for making you loose you time with my noob stuff

[Barand]

The piece of code indicated by gizmola also has other errors

    $userexists = $pdo->prepare("SELECT COUNT(username) FROM users WHERE username= '$username' LIMIT 1");
    $userexists->bindParam(':username', $username);
    $userexists->execute();

You are attempting to bind the $username to the :username placeholder - but you haven't used a placeholder.

Your PDO connection would look like this (and replace your mysqli connection)

$pdo = new PDO("mysql:host=$servername;dbname=$dbname",$dbusername,$dbpassword);
    $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC);
    $pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

... and your two mysqli queries would be replaced with

$stmt = $pdo->prepare("SELECT contrasena
                            , bloqueado
                       FROM users
                       WHERE username = :username 
                      ");
$stmt->execute( ['username' => $username] );
$row = $stmt->fetch();

$realpass = $row['contrasena'];
$bloqueado = $row['bloqueado'];

 

[gizmola]

I apologize if this wasn't clear, but while I fixed some issues and formatting problems, I didn't mean to imply that I made the code work.  Those are things we want you to do for yourself.  

Barand went further towards making your code actually work.

If you have specific questions after making fixes, we welcome you updating the question with the latest code and any new questions you might have.

[EnriqueL]

Ok so if I understood well, we have turned the mysql code into pdo code and is this way:

<?php

session_start();
$servername = "localhost";
$dbusername = "";
$dbpassword = "";
$dbname = "";
$pdo = new PDO("mysql:host=$servername;dbname=$dbname",$dbusername,$dbpassword);
    $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC);
    $pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
if ($pdo->connect_error) {
    die("Connection failed: " . $pdo->connect_error);
}

$id="";
$username = $_POST['username'];
$password = md5($_POST['password']);
$func = "SELECT contrasena FROM users WHERE username='$username'";

$stmt = $pdo->prepare("SELECT contrasena
                            , bloqueado
                       FROM users
                       WHERE username = :username 
                      ");
$stmt->execute( ['username' => $username] );
$row = $stmt->fetch();

$realpass = $row['contrasena'];
$bloqueado = $row['bloqueado'];

//Login
if(!empty($username)) {
    // Check the email with database
    
    $userexists = $pdo->prepare("SELECT COUNT(username) FROM users WHERE username= '$username' LIMIT 1");
    $userexists->bindParam(':username', $username);
    $userexists->execute();
     
    // Get the result
    $userexistsres = $userexists->fetchColumn();
     
    // Check if result is greater than 0 - user exist
    if ($userexistsres == 1) {
        if ($bloqueado == NO) {
            if ($password != $realpass) {
                die("contrasena incorrecta");
            } else {
                $_SESSION['loguin']="OK";
                $_SESSION['username']="$username";
                header("Location: ./herramientas.php");
                exit;
            }
        } else {
            die("Tu usuario ha sido bloqueado o todavía no ha sido aceptado por un administrador. Si el problema persiste contacta con contacto@leonmacias.com");
        }
    } else {
      die("No hay ninguna cuenta con este nombre de usuario");
    }
} else {
    echo 'El campo usuario esta vacio';
}

But still the $pdo variable is not defined and the code shouldn't work, right? That's my main issue, if I can make it work with mysql and in a couple of months when I have more time convert it to pdo is still fine for me  thank you for your help with this issue

[Barand]

$pdo is defined here

$pdo = new PDO("mysql:host=$servername;dbname=$dbname",$dbusername,$dbpassword);

You have completely ignored comments re this code and left it in its incorrect state...

$userexists = $pdo->prepare("SELECT COUNT(username) FROM users WHERE username= '$username' LIMIT 1");
$userexists->bindParam(':username', $username);

BTW, the LIMIT 1 is redundant - that query will only ever return a single row with the record count.

Edited June 29, 2019 by Barand
spwlling error

[gizmola]

First off, there is no reason to query anything until you have insured you have input from the user.  An empty username or password should fail and no querying should occur.  

There is no reason to do multiple queries here.  Do one query by username, and use that result for further analysis.

I can't guarantee this works, but it should be pretty close.  Make sure you understand the changes I made and review documentation if you aren't clear.

 

<?php

session_start();

$servername = "localhost";
$dbusername = "";
$dbpassword = "";
$dbname = "";

$pdo = new PDO("mysql:host=$servername;dbname=$dbname",$dbusername,$dbpassword);
    $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC);
    $pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
if ($pdo->connect_error) {
    die("Connection failed: " . $pdo->connect_error);
}

$id="";


$username = trim($_POST['username']);
$password = trim($_POST['password']);


//Login
if (!empty($username) && !empty($password)) {

    // Check the email with database
    $stmt = $pdo->prepare('SELECT * FROM users WHERE username=:username LIMIT 1');
    $stmt->execute(array('username' => $username));

    // Get the result
    $user = $stmt->fetch(PDO::FETCH_ASSOC); 

    // Check if user exists
    if ($user) {
        if ($user['bloqueado'] == 'NO') {
            if (md5($password) != $user['realpass']) {
                die("contrasena incorrecta");
            } else {
                $_SESSION['loguin'] = "OK";
                $_SESSION['username'] = $username;
                header("Location: ./herramientas.php");
                exit;
            }
        } else {
            die("Tu usuario ha sido bloqueado o todavía no ha sido aceptado por un administrador. Si el problema persiste contacta con contacto@leonmacias.com");
        }
    } else {
      die("No hay ninguna cuenta con este nombre de usuario");
    }
} else {
    echo 'El campo usuario esta vacio';
}

 

 

 

[EnriqueL]

Ok, I have been reading again the code and trying to understand the changes. I am sorry Barand, as you say we had already definied $pdo I don't know what I was thinking about when I wrote that. Regarding to "You are attempting to bind the $username to the :username placeholder - but you haven't used a placeholder." I don't really understand what you mean to say. Honestly, I have no idea about using placeholders on this type of code. The pdo stuff of my code was from the internet as I didn't know how to get the confirmation about if the user existed or not. Could you explain that for me please? and I will take the "LIMIT 1" out of the code right now. Sorry again for that.

[gizmola]

I realized that the name of the password column in the database is 'contrasena', so you need to change this line of code:

if (md5($password) != $user['realpass']) {

to

if (md5($password) != $user['contrasena']) {

 

 

[EnriqueL]

Gizmola, wow that is great thank you. It is working fantastic now. May I ask you a couple of things about changes? I really like to understand things  You  both are great by the way, it is working now!

[gizmola]

Sure, ask away.