Authenticate and log in users

Question

How can I improve my login script and how to check for any possible injection?

PS. the script must run on multiple platforms, so I need empty arrays for cases such as the Android ones.

user_table:

CREATE TABLE `user_table` (
`user_id` int(11) NOT NULL AUTO_INCREMENT,
`user_type` varchar(255) NOT NULL,
 PRIMARY KEY (`user_id`)
) ENGINE=InnoDB AUTO_INCREMENT=63 DEFAULT CHARSET=latin1

Field	Type	Null	Key	Default	Extra
user_id	int(11)	NO	PRI	NULL	auto_increment
user_type	varchar(255)	NO		NULL

student_table:

CREATE TABLE `student_table` (
`user_id` int(11) NOT NULL,
`user_type` varchar(255) NOT NULL,
`user_email` varchar(255) NOT NULL,
`user_pass ` varchar(255) NOT NULL,
PRIMARY KEY (`user_id`)
) ENGINE=InnoDB DEFAULT CHARSET=latin1

Field	Type	Null	Key	Default	Extra
user_id	int(11)	NO	PRI	NULL
user_type	varchar(255)	NO		NULL
user_email	varchar(255)	NO		NULL
user_pass	varchar(255)	NO		NULL

teacher_table:

 CREATE TABLE `teacher_table` (
`user_id` int(11) NOT NULL,
`Class` varchar(255) NOT NULL,
`DEPARTMENT` varchar(255) NOT NULL,
`user_email` varchar(255) NOT NULL,
`user_pass ` varchar(255) NOT NULL,
PRIMARY KEY (`user_id`)
) ENGINE=InnoDB DEFAULT CHARSET=latin1

Field	Type	Null	Key	Default	Extra
user_id	int(11)	NO	PRI	NULL
Class	varchar(255)	NO		NULL
DEPARTMENT	varchar(255)	NO		NULL
user_email	varchar(255)	NO		NULL
user_pass	varchar(255)	NO		NULL

management_table:

CREATE TABLE `management_table` (
`user_id` int(11) NOT NULL,
`Class` varchar(255) NOT NULL,
`DEPARTMENT` varchar(255) NOT NULL,
`HEAD` varchar(255) NOT NULL,
`user_email` varchar(255) NOT NULL,
`user_pass ` varchar(255) NOT NULL,
PRIMARY KEY (`user_id`)
) ENGINE=InnoDB DEFAULT CHARSET=latin1

Field	Type	Null	Key	Default	Extra
user_id	int(11)	NO	PRI	NULL
Class	varchar(255)	NO		NULL
DEPARTMENT	varchar(255)	NO		NULL
HEAD	varchar(255)	NO		NULL
user_email	varchar(255)	NO		NULL
user_pass	varchar(255)	NO		NULL

user_status table:

CREATE TABLE `user_status` (
`user_id` int(11) NOT NULL,
`Login_id` int(11) NOT NULL,
`user_token` varchar(255) NOT NULL,
`user_status` varchar(255) NOT NULL,
PRIMARY KEY (`user_id`)
) ENGINE=InnoDB DEFAULT CHARSET=latin1

Field	Type	Null	Key	Default	Extra
user_id	int(11)	NO		NULL
Login_id	int(11)	NO	PRI	NULL	auto_increment
user_token	varchar(255)	NO		NULL
user_status	varchar(255)	NO		NULL

login script:

<?php

if (!filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) {
    // echo $emailErr = "Invalid email format";
    exit;

 }else{

    $db_name ="mysql:host=localhost;dbname=DATABASE;charset=utf8";
    $db_username="root";
    $db_password="";

  try{ 

    $PDO = new PDO($db_name, $db_username, $db_password);
    //echo "connection success";

 }catch (PDOException $error){

    //echo "connection error";
    exit;

 }

    $email = $_POST['email'];
    $pass = $_POST['pass'];

    $stmt = $PDO->prepare("
      SELECT student_table.user_id ,student_table.user_pass
      FROM student_table
      WHERE student_table.user_email = :EMAIL
      UNION 
      SELECT
      teacher_table.user_id ,teacher_table.user_pass
      FROM teacher_table
      WHERE teacher_table.user_email = :EMAIL
      UNION 
      SELECT
      management_table.user_id ,management_table.user_pass
      FROM management_table
      WHERE management_table.user_email = :EMAIL
      " );

 $stmt->bindParam(':EMAIL', $email);
 $stmt->execute();
 $row = $stmt->fetch(PDO::FETCH_ASSOC);

 if (!empty($row)) {

 if (password_verify($pass, $row['user_pass'])) {

 function guidv4($data = null) {
 $data = $data ?? random_bytes(16);
 assert(strlen($data) == 16);
 $data[6] = chr(ord($data[6]) & 0x0f | 0x40);
 $data[8] = chr(ord($data[8]) & 0x3f | 0x80);
 return vsprintf('%s%s-%s-%s-%s-%s%s%s', str_split(bin2hex($data), 
 4));
 }

 $user_id = $row['user_id'];
 $user_online = 'ONLINE';
 $user_token = guidv4();

 $sql_insert = "
 INSERT INTO user_status 
 (user_id, user_token,user_status) 
 VALUES 
 (:ID,:TOKEN,:ONLINE ); ";

 $stmt = $PDO->prepare($sql_insert);
 $stmt->bindParam(':ID', $user_id);
 $stmt->bindParam(':ONLINE', $user_online);
 $stmt->bindParam(':TOKEN', $user_token);
 $stmt->execute();

 $sql_select ="
 SELECT user_table.user_type 
 FROM user_table 
 WHERE user_table.user_id = :USER_ID";

 $stmt = $PDO->prepare($sql_select);
 $stmt->bindParam(':USER_ID', $user_id);
 $stmt->execute();
 $row2 = $stmt->fetch(PDO::FETCH_ASSOC); 

 foreach ($row2 as $key ) {

 $user_id = $row['user_id'];

 if ($key == 'STUDENT') {

 $stmt2 = $PDO->prepare("
 SELECT
 student_table.user_type,
 user_status.login_id,
 user_status.user_token 
 FROM  student_table
 LEFT JOIN user_status ON user_status.user_id = 
 student_table.user_id
 WHERE  student_table.user_id = :USERID 
 "); 

 $stmt2->bindParam(':USERID', $user_id);
 if ($stmt2->execute()) {
 $row3 = $stmt2->fetch(PDO::FETCH_ASSOC); 
 $returnApp = array( 'LOGIN' => 'Log_In_Success', 
              'user' =>$row2['user_type'],
              'type' => $row3['user_type'],
              'id' => $row3['login_id'],  
              'token' => $row3['user_token']);
               echo json_encode($returnApp);
              
          }
          
            }if ($key == 'TEACHER') {
                
                  $stmt3 = $PDO->prepare("
                                SELECT
                                       teacher_table.Class,
                                       teacher_table.DEPARTMENT,
                                       user_status.login_id,
                                       user_status.user_token 
                                FROM  teacher_table
                                LEFT JOIN user_status ON user_status.user_id = teacher_table.user_id
                                WHERE  teacher_table.user_id = :USERID 
                                
                              "); 

              $stmt3->bindParam(':USERID', $user_id);

              if ($stmt3->execute()) {
              $row4 = $stmt3->fetch(PDO::FETCH_ASSOC); 
              $returnApp = array( 'LOGIN' => 'Log_In_Success', 
              'user' =>$row2['user_type'],
              'CL' =>$row4['Class'],
              'DEP' => $row4['DEPARTMENT'],
              'id' => $row4['login_id'],  
              'token' => $row4['user_token']);
               echo json_encode($returnApp);
              }
          }if ($key == 'MANAGEMENT'){
                
                  $stmt4 = $PDO->prepare("
                                SELECT
                                       management_table.CLASS,
                                       management_table.DEPARTMENT,
                                       management_table.HEAD,
                                       user_status.login_id,
                                       user_status.user_token 
                                FROM  management_table
                                LEFT JOIN user_status ON user_status .user_id = management_table.user_id
                                WHERE  management_table.user_id = :USERID 
                                
                              "); 

              $stmt4->bindParam(':USERID', $user_id);

              if ($stmt4->execute()) {
              $row5 = $stmt4->fetch(PDO::FETCH_ASSOC); 
              $returnApp = array( 'LOGIN' => 'Log_In_Success', 
              'user' =>$row2['user_type'],
              'CL' =>$row5['CLASS'],
              'DEP' => $row5['DEPARTMENT'],
              'HD' => $row5['HEAD'],
              'id' => $row5['login_id'],  
              'token' => $row5['user_token']);
               echo json_encode($returnApp);   
       }    
      }
     }
    }else {       
     $returnApp = array( 'LOGIN' => 'Log_In_Failed');
     echo json_encode($returnApp);
    }
   } else {
     $returnApp = array( 'LOGIN' => 'Email_Doesnt_Exist');
     echo json_encode($returnApp);
  }
 }
    

Answer 1

The good thing about your code is that that you use prepared statements with bound variables. Too often we see input variables in query strings.

Also nice is the use of the filter_var() function instead of a complex regular expression.

But that's where the good things end. You haven't properly indented the code, which makes it hard to read. And hard to read it is. This is a very complex piece of code with lots of nested if ...else ... and exit. To illustrate this point; This is what the structure of your code looks like:

if (....) {
    exit;
} else {
    try { 
    } catch (....) {
        exit;
    }
    if (....) {
        if (....) {
            function name(....) 
            {
                return ....;
            }
            foreach (....) {
                if (....) {
                    if (....) {
                    }
              
                }
                if (....) {
                    if (....) {
                    }
                }
                if (....) {
                    if (....) {
                    }    
                }
            }
        } else {       
        }
    } else {
    }
}

Even when properly indented, and without most of the code, it is difficult to understand. The technical term for this type of code is: Spaghetti code. It is difficult to understand and harder to maintain.

That is, I have to say, the biggest security risk here.

Another problem, also caused by the lack of structure, is that there's quite a bit of repetition of code. See: Don't repeat yourself. I count five echo json_encode($returnApp);. You have four select queries, going through the same routine every time. You could create a function for that. Something like:

function retrieveData($db, $query, $parameters = [])
{
    $statement = $db->prepare($query); 
    foreach ($parameters as $placeholder => $value) {
        $statement->bindValue($placeholder, $value);
    }
    $data = null;
    if ($statement->execute()) {
        $data = $statement->fetchAll(PDO::FETCH_ASSOC); 
    }
    return $data;
}

Or any variant thereof. The basic idea is to encapsulate things you do repeatedly in an easy to read, and therefore easy to understand, function. On top of that you can much better test this function in isolation, than you can with code that is embedded in other code. Note that I even made the function more powerful than you actually need right now: It can cope with multiple parameters and output multiple rows.

I challenge you to restructure your code so you only need one echo json_encode($returnApp); and you don't have any if blocks, nested deeper than 1 level, anymore. Use functions for this. You'll find that your code will be much easier to read and maintain.