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
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

<?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.user_token
FROM  student_table
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.user_token
FROM  teacher_table
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);
}
}


• Hello at Code Review@SE. Please heed How do I ask a Good Question? Feb 16, 2022 at 5:07
• 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. Feb 16, 2022 at 5:27
• 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
Feb 16, 2022 at 8:51
• 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.
– Mast
Feb 16, 2022 at 8:56
• 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. Feb 18, 2022 at 0:54

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.

• 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 Feb 17, 2022 at 10:37
• @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. Feb 17, 2022 at 11:23
• 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 Feb 17, 2022 at 13:22
• 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. Feb 18, 2022 at 0:59