Discussion :

[Gwennin]

Bonjour,

Je viens de terminer un script de connections à une interface privé avec nom d'utilisateur et mots de passe. N'étant pas sur qu'il soit parfait et sans faille je demande de l'aide pour "valider" mon script et comment l'améliorer.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
<?php
   $admin = explode('/',strstr($_SERVER['REQUEST_URI'], 'admin'));
 
   if ($admin[0]== 'admin')
      {
         if ($_COOKIE['login'])
            {
            }
         elseif ($_POST['login'] != NULL and $_POST['passwd'] != NULL)
            {
               $link = mysql_connect('host', 'user', 'psswd');
               mysql_select_db('db', $link); 
 
               $login = $_POST['login'].':'.md5($_POST['passwd']);
 
               $sql = 'SELECT COUNT(*) AS nb FROM login WHERE userpass = \''.$login.'\'';
               $rep = mysql_query($sql);
 
               if ($rep)
                  {
                     $time = time()*60*5;
                     setcookie('login', $login, $time);
                  }
            }
         else
            {
               echo '<meta http-equiv="refresh" content="O; URL=/_/admin/login.php">';
            }
      }
   else
      {
      }
?>

Vos suggestions?

Merci pour vos réponses...

[Grepsd]

Bonsoir,

donc, pourquoi utiliser les cookies alors que les sessions gères ça simplement et de façon plus sécurisée ?

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

 $admin = explode('/',strstr($_SERVER['REQUEST_URI'], 'admin'));

Dans le cas où 'admin' n'est pas dans l'URI, que se passe-t-il ? :p

Ta façon de stocker les mots de pass des utilisateurs est également assez singulière, pourquoi ce choix ?

De plus, la fonction mysql_query() ne retourne pas autre chose qu'une ressource, même si tu n'as aucun résultat, il me semble que le retour sera tout de même une ressource mysql, ton script sera donc toujours exécuté comme si l'utilisateur avait saisies les bonnes informations.

De plus je ne saurais que trop te conseiller d'utiliser pdo ou MySQLi pour tout ce qui est de gestion MySQL, notamment pour la protection des requêtes contre les injections mais également pour le futur où les fonctions mysql_* simple seront dépréciées.


Bref, au final, ce n'est pas mal fait, mais des choses restent cependant à revoir, le seul véritable problème à mes yeux est le test que tu effectue sur le mysql_query.

Utilises donc:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
 
$resQuery = mysql_query($query);
$countResults = mysql_result($resQuery,0 , 'nb');
 
if ($countResults > 0) {
/* some code */
} else {
/* some code */
}

Que l'on me corrige si je me suis trompé.

Bonne soirée.

[Gwennin]

Bonjour,

donc, pourquoi utiliser les cookies alors que les sessions gères ça simplement et de façon plus sécurisée ?

Les session sont plus sécurisé que les cookies?

Dans le cas où 'admin' n'est pas dans l'URI, que se passe-t-il ? :p

Finalement mon script sera placé dans ma page head.php qui est inclut dans toutes mes pages... Si on ne se trouve pas dans admin ça ne fait rien...

Ma façon de stocker mes mots de passes viens de l'authentification des htaccess et htpasswd; il y une raison pour cela; Les administrateurs et correcteur actuel de mon site se connectent avec le htaccess, le mot de passe est crypté et ne vais pas leurs demander quelle est leur mot de passe....

Pour la ressource MySQL je vais regler ça; et sinon j'ai déjà prévu de migrer vers pdo, merci...

[Gwennin]

Voici ce que ça donne:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
<?php
   $admin = explode('/',strstr($_SERVER['REQUEST_URI'], 'admin'));
 
   if ($admin[0]== 'admin')
      {
         if ($_COOKIE['login'])
            {
            }
         elseif ($_POST['login'] != NULL and $_POST['passwd'] != NULL)
            {
               $link = mysql_connect('localhost', 'scripts', 'glbbreiz'); //conections a mysql et choix de la db
               mysql_select_db('scripts', $link); 
 
               $login = $_POST['login'].':'.md5($_POST['passwd']);
 
               $sql = 'SELECT COUNT(*) AS nb FROM login WHERE userpass = \''.$login.'\'';
               $resQuery = mysql_query($sql);
               $resCount = mysql_result($resQuery, 0, 'nb');
 
               if ($resCount > 0)
                  {
                     $time = time()*60*5;
                     setcookie('login', $login, $time);
                  }
            }
         else
            {
               echo '<meta http-equiv="refresh" content="O; URL=/_/admin/login.php">';
            }
      }
?>

Il me semble que la variable $resCount est toujours égale a 1, non?
Je pourait mettre un

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

 if ($resCount == 1)

a la place???

[langevert]

Stocker le md5 du mot de passe dans le cookie c'est pas terrible car en récuperant le cookie tu peux retrouver le mot de passe original (si celui-ci est dans un dictionnaire)

Ensuite, comment compte tu faire pour vérifier sur tes pages privées que la personne est bien connectée? En vérifiant la valeur du cookie?? Il ne faut pas oublier qu'il est très facile de créer / modifier un cookie
Je te conseille d'utiliser les sessions qui sont + sécurisées (car contrairement aux cookies, les infos sont stockées sur le serveur).

[Gwennin]

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
<?php
   session_start();
   $admin = explode('/',strstr($_SERVER['REQUEST_URI'], 'admin'));
 
   if ($admin[0]== 'admin')
      {
         if (isset($_SESSION['login']))
            {
            }
         elseif ($_POST['login'] != NULL and $_POST['passwd'] != NULL)
            {
               $link = mysql_connect('hst', 'usr', 'mdp');
               mysql_select_db('db', $link); 
 
               $login = $_POST['login'].':'.md5($_POST['passwd']);
 
               $sql = 'SELECT COUNT(*) AS nb FROM login WHERE userpass = \''.$login.'\'';
               $resQuery = mysql_query($sql);
               $resCount = mysql_result($resQuery, 0, 'nb');
 
               if ($resCount == 1)
                  {
                     $_SESSION['login'] = $login;
                  }
            }
         else
            {
               echo '<meta http-equiv="refresh" content="0; URL=/_/admin/login.php">';
            }
      }
?>

C'est vrais je n'y avais pas pensé... je pensait que c'était sur avec le md5...

[Grepsd]

Il me semble que la variable $resCount est toujours égale a 1, non?

Dans ton cas je suppose que oui, mais j'ai été habitué à utiliser cette méthode au cas où... il y aurai plusieurs enregistrement(ça peut arriver pour de multiples raisons).

De même le:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

if ($resCount)

Conviendrait mais je n'aime pas faire des test logique de cette façon

Sinon ça me semble correct.

PS: pour PDO c'est le bon choix :p

[Gwennin]

Je vais adopter la version avec $resCount = 1

ça m'évitera des erreurs d'ajout dix fois du même utilisateur...

Merci

[Gwennin]

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
<?php
   session_start();
 
   $admin = explode('/',strstr($_SERVER['REQUEST_URI'], 'admin')); //on cherche dans quel répertoire l'on se trouve
 
   if ($admin[0] == 'admin' and empty($_SESSION['login'])) //si l'on est dans admin et que la session login n'est pas déclaré
      {
         if ($_POST['login'] != NULL and $_POST['passwd']) //si $_POST['login'] et $_POST['passwd']
ne sont pas NULL            {
               $link = mysql_connect('hst', 'usr', 'mdp'); //Connection a MySQL
               mysql_select_db('db', $link);
 
               $sql = 'SELECT * FROM login WHERE user = \''.$_POST['login'].'\''; //Trouver la ligne ou user est égale à $_POST['login']
               $query = mysql_query($sql);
               $res = mysql_fetch_array($query);
 
               if ($res['pass'] == md5($_POST['passwd'])) //si le mot de passe rentré est égal à celui de la db avec un md5
                  {
                     $_SESSION['login'] = $res['id']; // enregistrer la session login avec l'id de l'utilisateur (pas de mdp retransmit) 
                  } //fin if
               else
                  {
                     echo '<meta http-equiv="refresh" content="0; URL=/_/admin/login.php?e=0">'; //si le mdp rentré est erroné redirection avec msg d'erreur 
                  } //fin else
            } //fin if
         else
            {
               echo '<meta http-equiv="refresh" content="0; URL=/_/admin/login.php?e=1">'; // si un des champs est null
            } //fin else
      } //fin if
?>

[Grepsd]

Bonsoir,

essais d'utiliser des test strict quand tu le peux (!== et ===), je pense notamment aux comparaisons avec 'null'.

Pour ce qui de ton système de comparaison des mots de pass, essais également de ne sélectionner dans ta requête que les champs qui te seront utils,

'SELECT `pass` FROM table WHERE `login` = ?'
Remplacer login par une version protégée contre les injections du login voulu.

Sinon je pense que tout fonctionne à première vue, toutes mes remarques concernent donc l'optimisation.

Bonne soirée.

[Gwennin]

je ne connaisait pas les comparaisons strictes... merci;
c'est pas le login que j'enregistre dans la session mais l'id pour être plus securise, je ne communique pas de mot de passe, n'y d'user.

les injections de sql je règlerait ça quand je serait a travailer avec pdo...

merci

[Grepsd]

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

f ($res['pass'] == md5($_POST['passwd']))

Là tu compare les mots de pass :p

Mais pourquoi ne pas sélectionner count(id) avec la WHERE CLAUSE (login=?) AND (pass=?)

NB: remplacer les ? par les valeurs qui vont bien, ici, les login et mot de pass.

Sauvegarde uniquement l'ID de l'utilisateur en session est une très bonne chose.

[Gwennin]

ça revient au même,
enfin je pense...
Non?

[Grepsd]

Oui oui, ça revient au même ou presque, mais la question était de savoir pourquoi tu as changée ta façon de faire :p

[Gwennin]

Je trouvait ça tout simplement plus lisible au premier coup d'œuil ...

J'ai donc préféré cette technique.

[Grepsd]

okok :p

Tiens nous au courant de l'évolution de ton programme ^^

Bonne soirée/nuit

[Gwennin]

N'ayant plus eu de message sur les forums où j'ai posté j'ai jugé que le script devait être plus ou moins correcte;

Je l'ai donc mis en place...