0

Is there a better way to do these queries?

I call these functions from an other php to get data back to my Android Application in JSON. But I feel that this code is "dirty".

This code works. But can there be issues if there are many user requests? I want to keep all the stuff fast an slim for following stuff. Now there are about 100 people running this app. Everything is ok now. But how it will be if there are more?

<?php require_once("db_connection.php");?>

<?php
define('TIMEZONE', 'Europe/Paris');
date_default_timezone_set(TIMEZONE);

    function storeUser($email, $password, $uuid, $name){
        global $connection;
        $date = date("Y-m-d H:i:s");
        
        $query = "SELECT * FROM treuepass_users_all WHERE email ='{$email}'";
            $res = mysqli_query($connection, $query);
            $num = mysqli_num_rows($res);
            if ($num == 0)
            {
        $query = "SELECT * FROM treuepass_users_all WHERE uuid ='{$uuid}'";
            $res = mysqli_query($connection, $query);
            $num = mysqli_num_rows($res);
            if ($num > 0)
            {
                $query2 = "UPDATE treuepass_users_all SET email = '{$email}', password = '{$password}', name = '{$name}'  WHERE uuid ='{$uuid}'";
                $res2 = mysqli_query($connection, $query2);
                return $res2;
                mysqli_close($connection);
            }  
                else   //////Wenn sich HANDY das erste mal anmeldet
                $query = "INSERT INTO treuepass_users_all (uuid, dateofregister, email, password, name) VALUES ('{$uuid}', '{$date}', '{$email}', '{$password}', '{$name}')"; 
                $res = mysqli_query($connection, $query);
                $query2 = "UPDATE treuepass_users_all SET lastlogin = '{$date}', logincounter = logincounter +1 WHERE uuid ='{$uuid}'";
                $res2 = mysqli_query($connection, $query2);
                return $res2;
                mysqli_close($connection);
            }else{
                return false;
            }
    }
    

    function getUserByUsernameAndPassword($email, $password, $uuid){
        $date = date("Y-m-d H:i:s");
        global $connection;
        
        $query1 = "UPDATE treuepass_users_all SET uuid = '{$uuid}', lastlogin = '{$date}', logincounter = logincounter +1  WHERE email = '{$email}' AND password = '{$password}'";
        $user1 = mysqli_query($connection, $query1);
        
        $query2 = "SELECT * FROM treuepass_users_all WHERE email = '{$email}' AND password = '{$password}'";
        $user2 = mysqli_query($connection, $query2);
        
        if($user2){
            while ($res = mysqli_fetch_assoc($user2)){
                return $res;
            }
        }
        else{
            return false;
        }
        mysqli_close($connection);
    }
    
    
    function getUserByUUID($uuid){
        global $connection;
        //////Wenn UUID bereits Vorhanden
        $date = date("Y-m-d H:i:s");
        
        $query2 = "UPDATE treuepass_users_all SET lastlogin = '{$date}', logincounter = logincounter +1 WHERE uuid ='{$uuid}'";
        $res2 = mysqli_query($connection, $query2);
        
        $query = "SELECT * FROM treuepass_users_all WHERE uuid ='{$uuid}'";
            $res = mysqli_query($connection, $query);
            $num = mysqli_num_rows($res);
            if ($num > 0)
            {
                while ($dsatz = mysqli_fetch_assoc($res))
                return $dsatz;
                mysqli_close($connection);
            }  
                else   //////Wenn sich HANDY das erste mal anmeldet
                $query = "INSERT INTO treuepass_users_all (uuid, dateofregister, lastlogin, logincounter) VALUES ('{$uuid}', '{$date}', '{$date}', '1')"; 
                $res = mysqli_query($connection, $query);
                $query3 = "SELECT * FROM treuepass_users_all WHERE uuid ='{$uuid}'";
                $res3 = mysqli_query($connection, $query3);
                if($res3){
                    while ($res = mysqli_fetch_assoc($res3)){
                        return $res;
                    }
                }
                else{
                    return false;
            }
                mysqli_close($connection);
        }
            
            
    function getUpdateUserDataLocation($locationid, $id, $stampcard1counter, $stampcard1stampsnow, $stampcard1redeemed, $stampcard2counter, $stampcard2stampsnow, $stampcard2redeemed, $stampcard3counter, $stampcard3stampsnow, $stampcard3redeemed, $vouchercounter, $vouchernow, $voucherredeemed){
        global $connection;
        $date = date("Y-m-d H:i:s");
        

        $locationtable5 = "treuepass_history_$locationid";
        $query5 = "INSERT INTO $locationtable5 (uuid, date, time, stampcard1counter, stampcard1redeemed, stampcard2counter, stampcard2redeemed, stampcard3counter, stampcard3redeemed, voucherredeemed)
                   VALUES ('$id', '$date', '$date', '$stampcard1counter','$stampcard1redeemed', '$stampcard2counter','$stampcard2redeemed', '$stampcard3counter','$stampcard3redeemed', '$voucherredeemed')"; 
        mysqli_query($connection, $query5);
    
        
        $locationtable = "treuepass_users_$locationid";
        $query3 = "UPDATE $locationtable 
                        SET
                        stampcard1counter = stampcard1counter+'{$stampcard1counter}', stampcard1stampsnow = '{$stampcard1stampsnow}', stampcard1redeemed = stampcard1redeemed+'{$stampcard1redeemed}',
                        stampcard2counter = stampcard2counter+'{$stampcard2counter}', stampcard2stampsnow = '{$stampcard2stampsnow}', stampcard2redeemed = stampcard2redeemed+'{$stampcard2redeemed}',
                        stampcard3counter = stampcard3counter+'{$stampcard3counter}', stampcard3stampsnow = '{$stampcard3stampsnow}', stampcard3redeemed = stampcard3redeemed+'{$stampcard3redeemed}',
                        vouchercounter = vouchercounter+'{$vouchercounter}', vouchernow = '{$vouchernow}', voucherredeemed = voucherredeemed+'{$voucherredeemed}'
                        WHERE uuid ='{$id}'";
        $res3 = mysqli_query($connection, $query3);
        
        
        $query = "SELECT * FROM $locationtable WHERE uuid ='{$id}'";
            $res = mysqli_query($connection, $query);
            $num = mysqli_num_rows($res);
            if ($num > 0)
            {
                while ($dsatz = mysqli_fetch_assoc($res))
                return $dsatz;
                mysqli_close($connection);
                
            }          ////////////////////////////////////////////
                else   // Wenn sich HANDY das erste mal anmeldet // 
                $query = "INSERT INTO $locationtable (uuid, stampcard1counter, stampcard1stampsnow, stampcard1redeemed, stampcard2counter, stampcard2stampsnow, stampcard2redeemed, stampcard3counter, stampcard3stampsnow, stampcard3redeemed, vouchercounter,                                          vouchernow, voucherredeemed)
                          VALUES ('$id', '$stampcard1counter','$stampcard1stampsnow','$stampcard1redeemed', '$stampcard2counter','$stampcard2stampsnow','$stampcard2redeemed', '$stampcard3counter','$stampcard3stampsnow','$stampcard3redeemed',
                          '$vouchercounter','$vouchernow','$voucherredeemed')"; 
                
                mysqli_query($connection, $query);
                mysqli_close($connection);
    }
        
    function getUsersLocationStampcard($userid, $locationid){
        global $connection;
            $locationtable = "treuepass_users_$locationid";
            
            $query = "SELECT * FROM $locationtable WHERE uuid ='{$userid}'";
                $res = mysqli_query($connection, $query);
                if($res){
                    while ($response = mysqli_fetch_assoc($res)){
                        return $response;
                    }
                }
                else{
                    return false;
            }
                mysqli_close($connection);
    }   
?>
mkrieger1
  • 19,194
  • 5
  • 54
  • 65
  • 4
    Oh yes definitely – RiggsFolly Aug 02 '18 at 12:21
  • Almost ( I say almost as I have not checked all the code) ALL your double queries on the same table could be done as one query – RiggsFolly Aug 02 '18 at 12:25
  • 2
    Your script is wide open to [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) in either the `MYSQLI_` or `PDO` API's – RiggsFolly Aug 02 '18 at 12:26
  • 1
    It looks to me like you are using **Plain Text Passwords** PHP provides [`password_hash()`](http://php.net/manual/en/function.password-hash.php) and [`password_verify()`](http://php.net/manual/en/function.password-verify.php) please use them. And here are some [good ideas about passwords](https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet) If you are using a PHP version prior to 5.5 [there is a compatibility pack available here](https://github.com/ircmaxell/password_compat) – RiggsFolly Aug 02 '18 at 12:26
  • 3
    This type of question may be better asked on [Code Review](https://codereview.stackexchange.com/) – RiggsFolly Aug 02 '18 at 12:28
  • Some sensible code indentation would be a good idea. It helps us read the code and more importantly it will help **you debug your code** [Take a quick look at a coding standard](http://www.php-fig.org/psr/psr-2/) for your own benefit. You may be asked to amend this code in a few weeks/months and you will thank me in the end. – RiggsFolly Aug 02 '18 at 12:28
  • It is always better to inject the connection into a function using a parameter than using `global` – RiggsFolly Aug 02 '18 at 12:29
  • you may also want to pay attention to the `mysql` slow-log (enable it if it is not). Some of those queries could scan the entire table : for example if you did not index `uuid` of table `$locationtable`. As the userbase grows, slow queries will mean slow death for your app. – YvesLeBorg Aug 02 '18 at 12:46
  • Thanks for your fast answer RiggsFolly! For the Password i use md5 encryption. $password=md5($passwordtmp); So the connection isnt good as global? I have to say that i "learn" only from youtube ^^" – AndroidJava Newbie Aug 02 '18 at 12:52
  • @RiggsFolly yep, OWASP should be mandatory reading :) – YvesLeBorg Aug 02 '18 at 12:52
  • Please dont __roll your own__ password hashing specially not using MD5(). PHP provides [`password_hash()`](http://php.net/manual/en/function.password-hash.php) and [`password_verify()`](http://php.net/manual/en/function.password-verify.php) please use them. And here are some [good ideas about passwords](https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet) If you are using a PHP version prior to 5.5 [there is a compatibility pack available here](https://github.com/ircmaxell/password_compat) – RiggsFolly Aug 02 '18 at 12:54
  • @AndroidJavaNewbie youtube is one gig, i prefer learning from answers i find here. Finding questions and answers here is a non perishable skill. Also, you can copy-paste good code snippets in your test code base to kick the tires. – YvesLeBorg Aug 02 '18 at 12:55
  • Hmm it seems i have to start new hmm? ^^ – AndroidJava Newbie Aug 02 '18 at 13:16

1 Answers1

0

Thanks for all the Comments! I spend the whole day for rewrite my code xD

But now i get all the stuff you told me. - I did the thing with the connection inside the php - I only have 1 php for all the stuff now - Password Hashing with 'password_hash()' - Prepared Statemants for MySQLi

Here some Snippet:

    //////////////////////////////////////////////////STORE USER
if (isset($_POST['uuid']) && isset($_POST['email']) && isset($_POST['password']) && isset($_POST['name'])) {

        $sql = "SELECT * FROM treuepass_users_all WHERE email = ?";
        $stmt = $mysqli->prepare($sql);
        $stmt->bind_param("s", $_POST['email']);
        $stmt->execute();
        $result = $stmt->get_result();
        if($result->num_rows == 1)
        {
            $response["error"] = TRUE;
            $response["error_msg"] = "E-Mail Adresse bereits registriert!";
            echo json_encode($response);
            exit;
        }else{

        $sql = "INSERT INTO treuepass_users_all (uuid, dateofregister, email, password, name, lastlogin, logincounter) VALUES (?, ?, ?, ?, ?, ?, ?)
                ON DUPLICATE KEY UPDATE email=?, password=?, name=?, lastlogin=?, logincounter=logincounter +1";
        $stmt = $mysqli->prepare($sql);
        $one = "1";
        $hash = password_hash($_POST['password'], PASSWORD_DEFAULT);
        $stmt->bind_param("sssssssssss", $_POST['uuid'], $date, $_POST['email'], $hash, $_POST['name'], $date, $one,     $_POST['email'], $hash, $_POST['name'], $date);
        $stmt->execute();

        $sql = "SELECT * FROM treuepass_users_all WHERE uuid = ?";
        $stmt = $mysqli->prepare($sql);
        $stmt->bind_param("s", $_POST['uuid']);
        $stmt->execute();
        $result = $stmt->get_result();

        while ($row = $result->fetch_assoc())
        {
        $response["error"] = FALSE;
        $response["user"]["id"] = $row['id'];
        $response["user"]["uuid"] = $row['uuid'];
        $response["user"]["locked"] = $row['locked'];
        $response["user"]["dateofregister"] = $row['dateofregister'];
        $response["user"]["email"] = $row['email'];
        $response["user"]["username"] = $row['username'];
        $response["user"]["name"] = $row['name'];
        $response["user"]["surname"] = $row['surname'];
        $response["user"]["dayofbirth"] = $row['dayofbirth'];
        $response["user"]["monthofbirth"] = $row['monthofbirth'];
        $response["user"]["yearofbirth"] = $row['yearofbirth'];
        $response["user"]["gender"] = $row['gender'];
        $response["user"]["lastlogin"] = $row['lastlogin'];
        $response["user"]["logincounter"] = $row['logincounter'];
        echo json_encode($response);
        }
    }
}

I hope i did it well? :)