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);
 }
 }
 

• greybeard: Hello at Code Review@SE. Please heed How do I ask a Good Question?
• greybeard: I want actual, working code from a project, with enough context to give useful advice, in a question titled for what the code is to accomplish. I find it hard to believe your tables are named tableA, tableB, tableC.
• Mast: That's the difference between Stack Overflow and Code Review: Stack Overflow wants a Minimum Complete Verifiable Example (strip everything that seems unrelated to the problem), where Code Review requires code to be as close to the original as possible for the best reviews. It confuses a lot of new people.
• Mast: Old questions are not necessarily up to standards, so please don't use those as an example to use in a "they were allowed to do this there so why is everyone complaining now". We simply can't be everywhere at once and sometimes the rules change over time. We like to think we got wiser over the years. People are not having a bad day, they're trying to help.
• Sᴀᴍ Onᴇᴌᴀ: Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see What should I do when someone answers my question? as well as what you may and may not do after receiving answers.

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.

• Taa Lee: i accepted your challege and updated my script .. no nested if ,used functions, one echo for error and another one for success .. i know its not perfect but i hope its much better than the old one .. tested and works perfect
• KIKO Software: @TaaLee Yes, it is better. I still have a few tips. Try to get your indentation under control. Also, try to keep similar code together. That means all the functions together, and all the control code together. Don't put functions in the middle of other code. Always try to make your code as easy to read, and understand, as possible. You will thank your younger self for it in the future.
• KIKO Software: A correction on the link I gave in my previous comment. I don't agree with the requirement to use tabs instead of spaces. The width of tabs is not defined and depends on your editor. The argument that it would "take longer to process spaces" is nonsense. I agree a tab is easier to remove than spaces, but since you cannot see the difference between tabs and spaces, it's always a surprise what happens in code littered with tabs. I'm not making this up, it's part of the PHP Standards Recommendations, and for good reasons. See: php-fig.org/psr/psr-12/#24-indenting
• Sᴀᴍ Onᴇᴌᴀ: Please refrain from suggestions on updated code. The OP should have created a new post. Please see What should I do when someone answers my question? as well as what you may and may not do after receiving answers.