Avis sur mon code

coucougael94

coucougael94 Le 17 décembre 2016 à 23:18 (Édité le 25 janvier 2019 à 17:52)

Bonjour,
Je viens de finir une class "User" qui permet la gestion des utilisateurs de manière simple et J'aimerai vos avis !
Voici le code :
class User
{
   const ETAT_X  = 'x' ;//On ne sait pas encore..
   const ETAT_NC = 'nc';//Pas connécter
   const ETAT_C  = 'c' ;//Connecter, user normale
   const ETAT_M  = 'm' ;//Modéro
   const ETAT_A  = 'a' ;//Adm

   public function __construct($pseudo="0", $id=0, $pass="0")
   {
      $this->id=0;
      if($pseudo=="0" AND $id==0 AND $pass=="0")
         $this->connectUser();
      elseif($pass!="0")
      {
         if($pseudo!="0")
            connectThisMember(getId($pseudo), $pass);
         if($id!=0)
            connectThisMember(getPseudo($id), $pass);
      }
      else
         $this->etat=self::ETAT_X;
   }
   public function isAdmin()
   {
      return ($this->etat()==self::ETAT_A);
   }
   public function isConnect()
   {
      if($this->etat()==self::ETAT_NC)
         return true;
      else
         return true;
   }
   public function isModero()
   {
      return ($this->etat()==self::ETAT_M);
   }
   public function haveRight()//true si have right
   {
      return ($this->etat()==self::ETAT_M OR $this->etat()==self::ETAT_A);
   }
   private function etat()
   {
      if($this->etat==self::ETAT_X)
      {
         $this->connectUser();
      }
      else
         return $this->etat;
   }
   public function connectUser()//connecte l'user avec les données de seission et de cookie
   {
      if(isset($_SESSION['pseudo']) AND isset($_SESSION['pass']))
      {
         return $this->connectThisMember($_SESSION['pseudo'], $_SESSION['pass']);
      }
      else if(isset($_COOKIE['pseudo']) AND isset($_COOKIE['pass']))
      {
         return $this->connectThisMember($_COOKIE['pseudo'], $_COOKIE['pass']);
      }
      $this->etat=self::ETAT_NC;
   }
   public function connectThisMember($pseudo, $passH, $withInput=false)
   {
      global $bdd;
      global $header_message;
      $id_user=User::getId($pseudo);
      if(!$this->hACWTA($id_user) AND $withInput==false)
      {//Compte jamais connécté avec cet ip, utilise les variables, maybe hacker
         $this->etat=self::ETAT_NC;
         $header_message.='<div class="header-msg">Ce compte ne s\'est jamais connécté avec cet adresse IP. Pour plus de sécurité, veuillez <a href="'.$this->folderRoot.'connexion">vous reconnecter</a>.</div>';
         return false;
      }
      elseif($this->haveBrutForce($id_user))
      {// Compte brutforcer
         $this->etat=self::ETAT_NC;
         if($withInput)
            $header_message.='<div class="header-msg">Ce compte à recement eut plusieurs tentative de connexion échoué. Pour plus de sécurité, veuillez tenter de vous reconnecter dans moins de 5 minutes. <a href="">En savoir plus</a></div>';
         return false;
      }
      else// if((hACWTA($id_user) AND haveBrutForce($id_user))OR $withInput==true)
      {
         $req_pseudo= $bdd->prepare("SELECT sel FROM user WHERE pseudo=?");
         $req_pseudo->execute(array($pseudo));
         //$req_topic->execute(array(htmlspecialchars($pseudo),genPass($pass,)));
         $dat_pseudo=$req_pseudo->fetch();
         if(!empty($dat_pseudo['sel']))
         {
            $pass=$passH;
            if($withInput)
               $pass=genPass($passH,$dat_pseudo['sel']);
            $req_pass= $bdd->prepare("SELECT sel,id,type FROM user WHERE pseudo=? AND pass=?");
            $req_pass->execute(array($pseudo,genPass($pass,$dat_pseudo['sel'])));
            $dat_pass=$req_pass->fetch();
            if(!empty($dat_pseudo['sel']))
            {
               $this->setUserData($pseudo,genPass($pass,$dat_pseudo['sel']));
               $this->addTryToCo($this->id);
               if($withInput)
                  $header_message.='<div class="header-msg">Vous êtes connecté.</div>';
               return true;//ajout in bdd de reussite de co avec l'ip...
            }
            else
            {
               $header_message.='<div class="header-msg">Mauvais pseudonyme ou mauvais mot de passe.</div>';
               $this->addTryToCo($this->id, false);
               return false;//incrémentation in bdd de tentative de co
            }
         }
         else
         {
            $header_message.='<div class="header-msg">Mauvais pseudonyme ou mauvais mot de passe.</div>';
            $this->addTryToCo($this->id, false);
            return false;//incrémentation in bdd de tentative de co
         }
      }
      //$header_message.='<div class="header-msg">Pas connécter.</div>';
      return false;//compte jamais connécter avec cet adresse, maybe hacker
   }
   static function addTryToCo($id_user, $succes=true)
   {
      $ip=User::get_ip();
      global $bdd;
      $req_tentative= $bdd->prepare("INSERT INTO tentativeconnexion (id_user, ip, succes) VALUES(?,?,?)");
      $req_tentative->execute(array($id_user, $ip, $succes));
      $dat_tentative=$req_tentative->fetch();
   }

   //true si a déja connécter avec succès
   private function hACWTA($id_user)
   {//haveAlreadyConnectWithThisAccount
      global $bdd;// /!\CET FONCT EST DIF A NBCONNEXION CAR CET FONCT USE L'IP
      $ip=$this->get_ip();
      $req_hACWTA= Bdd::db()->prepare("SELECT COUNT(*) AS nbr FROM tentativeconnexion WHERE succes='1' AND ip=? AND id_user=?");
      $req_hACWTA->execute(array($ip,$id_user));
      $dat_hACWTA=$req_hACWTA->fetch();
      return ($dat_hACWTA['nbr']>=1);
   }
   static function nbConnexion($id_user, $failed=true)
   {
      global $bdd;
      $ip=User::get_ip();
      $failed_="";
      if($failed)
         $failed_="1";
      else
         $failed_="0";
      $req_nbConnexion= $bdd->prepare("SELECT COUNT(*) AS nbr FROM tentativeconnexion WHERE succes=? AND id_user=?");
      $req_nbConnexion->execute(array($failed_, $id_user));
      $dat_nbConnexion=$req_nbConnexion->fetch();
      return $dat_nbConnexion['nbr'];
   }
   static function haveBrutForce($id_user)
   {//haveBrutForce
      global $bdd;
      $ip=User::get_ip();
      $req_nbConnexion= $bdd->prepare("SELECT COUNT(*) AS nbr FROM tentativeconnexion WHERE succes=0 AND id_user=? AND `date`>= ADDDATE(NOW(),INTERVAL -1 DAY) LIMIT 0,25 ");
      $req_nbConnexion->execute(array($id_user));
      $dat_nbConnexion=$req_nbConnexion->fetch();
      return ($dat_nbConnexion['nbr']>=6);
   }

   //retourne 0 en cas d'echec
   static function getId($pseudo)
   {
      global $bdd;
      $req_pseudo= Bdd::db()->prepare("SELECT id FROM user WHERE pseudo=?");
      $req_pseudo->execute(array($pseudo));
      $dat_pseudo=$req_pseudo->fetch();
      if(!empty($dat_pseudo['id']))
         return $dat_pseudo['id'];
      else
         return 0;
   }

   //retourne 0 en cas d'echec
   static function getPseudo($id)
   {
      global $bdd;
      $req_id= $bdd->prepare("SELECT pseudo FROM user WHERE id=?");
      $req_id->execute(array($id));
      $dat_id=$req_id->fetch();
      if(!empty($dat_id['pseudo']))
         return $dat_id['pseudo'];
      else
         return 0;
   }
   public function id()
   {
      return $this->id;
   }
   public function pseudo()
   {
      return $this->pseudo;
   }
   static function get_ip() {
      // IP si internet partagé
      if (isset($_SERVER['HTTP_CLIENT_IP'])) {
         return $_SERVER['HTTP_CLIENT_IP'];
      }
      // IP derrière un proxy
      elseif (isset($_SERVER['HTTP_X_FORWARDED_FOR'])) {
         return $_SERVER['HTTP_X_FORWARDED_FOR'];
      }
      // Sinon : IP normale
      else {
         return (isset($_SERVER['REMOTE_ADDR']) ? $_SERVER['REMOTE_ADDR'] : '');
      }
   }
   //primfx : wysibb : 15px -> roboto 
   //f_message : 14px->roboto
   //https://fonts.googleapis.com/css?family=Roboto:400,500,300,700,400italic
   //input submit : 14px -> MS Shell Dlg \32 
   //
   public function setUserData($pseudo, $passH)
   {
      global $bdd;
      $req_pseudo= $bdd->prepare("SELECT sel, id, type FROM user WHERE pseudo=?");
      $req_pseudo->execute(array($pseudo));
      $dat_pseudo=$req_pseudo->fetch();
      if(!empty($dat_pseudo['id']))
      {
         setcookie('pseudo', $pseudo, time() + 365*24*3600, null, null, false, true);
         setcookie('pass', $passH, time() + 365*24*3600, null, null, false, true);
         $_SESSION['pseudo']=$pseudo;
         $_SESSION['id']=$dat_pseudo['id'];
         $_SESSION['pass']=$passH;
         $dat_enc=decrypt($dat_pseudo['type'], $dat_pseudo['sel']);
         $dat=str_replace($dat_pseudo['sel'], '', $dat_enc);
         if($dat=="norm")
            $etat=self::ETAT_C;
         elseif($dat=="moder")
            $etat=self::ETAT_M;
         elseif($dat=="admin")
            $etat=self::ETAT_A;
         $this->id=User::getId($pseudo);
         $this->pseudo=$pseudo;
         return true;
      }else echo 'have break, have kitkat !<br>have break, have kitkat !<br>have break, have kitkat !<br>have break, have kitkat !<br>have break, have kitkat !';//return false;
      $etat=self::ETAT_NC;
      return true;
   }
   public function setFolderRoot($folder)
   {
      $this->folderRoot=$folder;
   }
   private $folderRoot;

   private $etat;
   /* int stockant la valeur (connécter ou non) de l'user.
   0 : on n'a pas encore vérifier
   1 : l'user n'est pas connécté
   2 : l'user est connécté
   3 : l'user est connécté et est modelo
   4 : l'user est connécté et est adm
   */
   //type en bdd :
    //norm
    //admin
    //moder
   private $pseudo;
   /*
   getteur et setteur n'existe pas
   */
   private $id;
   /*
   finder : getId(pseudoEnQ)
   getteur et setteur n'existe pas
   */
   private $pass;
   /*
   getteur et setteur n'existe pas
   */
}

Info :
Il y a 5 type/etat d'utilisateur :
Pas connecté;
Connecté;
Admin;
Modérteur (Perso, je dis souvent Modero/modelo);
Il y a un système anti-brutforce (essai de toutes les combinaisons)
genPass() est une fonction a l'exterieure de la classe (Est-ce mieu de mettre genPass en static dans la classe ?) ;
Les mots de passe sont hashé avec un sel;
Si l'utilisateur se connecte avec les cookie, on surveille si l'adresse ip a déja connécté le membre pointé par les cookies, sinon, ont lui demande de ce connécté manuellement;
$bdd = objet PDO se trouvant a l'exterieure de ce fichier. (Dois-je faire autrement et comment ?).
Mon constructeur est brouillon.


Le but de cet classe est d'être simple.
Merci de vos conseils et remarques.
Profil introuvable

Profil introuvable Le 29 décembre 2016 à 01:43

Hello 😉

Je ne maîtrise pas le PHP mais je peux déjà te dire que ton code est bien structuré, pour les fautes, c'est une personne qui maîtrise le PHP qui pourra te dire 😉
coucougael94

coucougael94 Le 29 décembre 2016 à 11:51

Merci !
TheFlameflo

TheFlameflo Le 30 décembre 2016 à 00:19

Salut !

Je trouve le code plutôt pas mal.

Cependant, quelque chose qui m'a un peu dérangé, c'est le fait que tu dois mettre "global $bdd" partout.

Ce que j'utilise personnellement c'est le pattern "Singleton".
Celui-ci permet d'instancier un objet une seule fois et la prochaine fois qu'on l'instancie, on obtient le même à chaque coup.
Cependant, si tu ne veux pas te casser la tête étant donné qu'il faudra que tu crées une class Database, utiliser "global $bdd" est correct.

De plus, il y a aussi les requêtes dans ton code.
Ce que je vais dire peut avoir l'air bizarre, mais trois ligne c'est un peu long...
C'est vrai qu'une fois, trois ligne ce n'est pas si grave, mais quand on a plusieurs requêtes, ça commence à s’alourdir.
Ce que j'ai, c'est une classe Table, qui fait appel à Database pour me fournir les résultats.
Exemple :

Ma classe Database est un singleton que j'appelle plusieurs fois.
Pour éviter les requêtes à trois lignes, je créée une fonction nommée "query" et une autre nommée "prepare" :
/**
     * Send a request to get, update or delete dateas without attributes
     * @param  string  $statement The request
     * @param  boolean $one       Select if we only will show one result
     * @return mixed   $result    The results of the request
     */
    public function query($statement, $one = true)
    {
    $req = $this->getPDO()->query($statement);
    if(!is_null($one))
    {
      if($one == true)
      {
        $result = $req->fetch();
      }
      else
      {
        $result = $req->fetchAll();
      }
      return $result;
    }
    }

   /**
     * Send a request to get, update or delete dateas with attributes
     * @param  string  $statement The request
     * @param  array   $attributes The array for the execute
     * @param  boolean $one       Select if we only will show one result
     * @return mixed   $result    The results of the request
     */
    public function prepare($statement, $attributes = [], $one = true)
    {
        $req = $this->getPDO()->prepare($statement);
    try {
          $req->execute($attributes);
    } catch (PDOException $e) {
      var_dump($e);
    }
    if(!is_null($one))
    {
      if($one == true)
      {
        $result = $req->fetch();
      }
      else
      {
        $result = $req->fetchAll();
      }
      return $result;
    }
    }
Comme tu peux le voir, tout est adapté pour que ce soit simple :
On fournit la requête, si on veut un seul résultat ou plusieurs et on nous retourne le résultat. (noter aussi qu'il y a ce qu'on envoie dans la requête pour prepare).

Pour expliquer, getPDO() est une instanciation unique d'un objet PDO.
Ça donne ça :
/**
     * Create a new intance of PDO or use the actual instance
     * @return Object A instance of PDO
     */
    private function getPDO()
    {
        if(is_null($this->pdo))
        {
            try
            {
            $this->pdo = new PDO('mysql:host=' . $this->db_host . ';dbname=' . $this->db_name, $this->db_user, $this->db_pass, array(PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8'));
            $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
            }
            catch(PDOException $e)
            {
                echo 'Error : ' . $e->getMessage();
            }
        }
        return $this->pdo;
    }

Bref, tout est là pour que ce soit simple.

Mais, je veux que ce soit encore plus simple, je vais créer un objet Table avec une fonction qui simplifie encore la requête :
public function query($statement, $attributes = [], $one = true)
    {
        if(empty($attributes))
        {
            $query = $this->db->query($statement, $one);
        }
        else
        {
            $query = $this->db->prepare($statement, $attributes, $one);
        }
        return $query;
    }
Pour préciser, le $this->db vient d'une injection de dépendances lors du constructeur.

Par la suite, je fais un tas de fonctions qui m'aideront plus tard, comme une fonction qui ramasse tout d'une table, une autre avec une LIMIT, une autre avec un ordre, etc.
Pour que ça marche, j'ai un champ table qui contient le nom de ma table dans mon objet.
Lors que je fais hériter une classe de Table, je lui attribue un nom de table (par exemple user) pour que cette requête fonctionne, ça donne quelque chose comme ça :
public function all()
    {
        $query = $this->db->query("SELECT * FROM {$this->table}", [], false);
        return $query;
    }
J'aimerais bien pouvoir t'expliquer le reste, mais ça irait dans tous les sens étant donné que j'utilise le pattern MVC.

Je me suis basé sur les tutoriels de Grafikart (TP sur la création d'un blog en POO) pour créer un projet qui ressemble, avec des fonctions en plus comme un espace membre, un panel d'administration complet, etc.

En gros, le plus important c'est surtout que tu comprennes ton code et qu'il fonctionne.
C'est vrai qu'avoir un système pour les tables et les bases de de données c'est bien, mais on peut facilement tout faire bugger.

Bref, j'espère t'avoir aidé, si tu as d'autre questions, n'hésites pas !

coucougael94

coucougael94 Le 31 décembre 2016 à 17:31

Merci !
Je me suis basé sur les tutoriels de Grafikart (TP sur la création d'un blog en POO) pour créer un projet qui ressemble, avec des fonctions en
plus comme un espace membre, un panel d'administration complet, etc.
Je m'en suis rendu compte, ton query() est presque le même que dans les vidéos de grafikart.
Bonne année !
Vous devez être connecté pour poster une réponse. Se connecter ou Créer un compte