[Cookies] Classe de connexion à l'admin [Résolu]

[gtraxx]

Bonsoir tous le monde,
j'essaie de me construire une class pour la connections a l'administration des admins et j'aimerai savoir si je suis sur la bonne voie.
En fait le formulaire sera des plus simpliste:

1. 2 champs (users et pass)
2. L'envoi
3. message d'erreur éventuelle

Je ne souhaite pas d'inscription donc simplement se connecter a l'administration.
Cependant j'utilise PDO pour me connecter a la base de donnée et je doit implémenter les sessions + tous ce qui va avec (en gros je devrais me renseigner pour sécuriser tous sa)
Voici le code pour l'instant sans savoir si cela fonctionnera:

Code :

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
34
class UserAuthLogin{
 
	public $users_login;
	public $mdp_login;
 
	function __construct(){
		$this->users_login = $_REQUEST['users'];
		$this->mdp_login = sha1($_REQUEST['cryptpass']);
	}
	function LoadAuth(){
		try{
			$dbh = new PDO('mysql:host=localhost;dbname=madb','root','******');
			$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
			$stmt = $dbh->prepare("SELECT DISTINCT id_user,users from member where :users and :cryptpass");
		    $stmt->bindParam(':users',$this->users_login,PDO::PARAM_STR);
		    $stmt->bindParam(':cryptpass', $this->mdp_login,PDO::PARAM_STR);
			$stmt->setFetchMode(PDO::FETCH_ASSOC);
			$stmt->execute();
			$result = $stmt->fetchAll();
		}catch(Exception $e) {
				echo ("Erreur ! : " . $e->getMessage() . "<br/>");
				echo 'N° : '.$e->getCode();
		}
		return $result;
	}
	function AuthSession(){
		if (isset($this->users_login)) {
			self::LoadAuth();
			$_SESSION['userid'] = $this->users_login;
			$_SESSION['username'] = $this->mdp_login;
			header($page);
		}
	}
}

[Yoshio]

Quelques remarques vite fait :
- $_REQUEST c'est mal, préfère $_GET ou $_POST
- je mettrais les variable membre login/motdepasse en private
- faudrait tester avec isset si le login/mdp existe avant de l'affecter à une variable
- pas de catch dans une classe, c'est pas a ce niveau que tu gère les erreur. (j'ai pas dit pas de try hein )
- self:: s'utilise pour les classe statique, utilise $this-> plutot dans ce cas

[gtraxx]

Bon j'ai corriger , arrêter moi si je me suis trompé

Code :

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
34
class UserAuthLogin{
 
	private $users_login;
	private $mdp_login;
 
	function __construct(){
		if(isset($_POST['users']) || isset($_POST['cryptpass'])){
 
            $this->users_login = $_POST['users'];
		    $this->mdp_login = sha1($_POST['cryptpass']);
          }
	}
	function LoadAuth(){
		try{
			$dbh = new PDO('mysql:host=localhost;dbname=madb','root','******');
			$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
			$stmt = $dbh->prepare("SELECT DISTINCT id_user,users from member where :users and :cryptpass");
		    $stmt->bindParam(':users',$this->users_login,PDO::PARAM_STR);
		    $stmt->bindParam(':cryptpass', $this->mdp_login,PDO::PARAM_STR);
			$stmt->setFetchMode(PDO::FETCH_ASSOC);
			$stmt->execute();
			$result = $stmt->fetchAll();
		}
		return $result;
	}
	function AuthSession(){
		if (isset($this->users_login)) {
			$this->LoadAuth();
			$_SESSION['userid'] = $this->users_login;
			$_SESSION['username'] = $this->mdp_login;
			header($page);
		}
	}
}

faudrait tester avec isset si le login/mdp existe avant de l'affecter à une variable

Heuu j'ai pas saisi lol, en fait ce sera AuthSession qui sera retourné ou plus précisement assigner a smarty.
Pas de catch je teste ma connexion a la db a quel niveau
Au fait pourquoi $_REQUEST est déprécié ? je l'utilise au boulot (pas le choix) comme on est une dizaine de développeur le code est partagé (on me donne que des $REQUEST .

[Yoshio]

Je sais pas si $_REQUEST est déprécié mais en tout cas quand tu utilise $_REQUEST tu ne sait pas d'où viennent les données. Est-ce qu'elles viennent d'un POST, d'un GET, d'un Cookie ? Donc il faut prendre l'habitude d'utiliser les bonnes variable globale.

Pour le catch, il faut le faire tout simplement en dehors de ta classe. Tu lance les exceptions avec throw ou met des bloc try dans ta classe et tu les récupère en dehors de la classe. C'est pas à toi de décider si oui ou non tu veux capturer les erreurs. C'est à celui qui utilise ta classe (bon ici c'est toi dans les 2 cas mais ça change rien).

Pour le isset t'a bien fait ça. Il faut s'assurer que le $_POST['users'] et l'autre variable $_POST sont bien initialisée sinon tu aura un notice il me semble.

Je te conseille de mettre un error_reporting(E_ALL | E_STRICT) en début de ton script, juste pendant la phase de création/test/degugging.

[gtraxx]

Tu lance les exceptions avec throw ou met des bloc try dans ta classe et tu les récupère en dehors de la classe.

Vous n'auriez pas un exemple , j'ai fais ceci après le try :

Code :

1
2
3
catch(Exception $e) {
				throw $e;
		}

Ensuite pensez vous qu'un simple assign avec smarty fera l'affaire pour

Code :

$_POST['users'] et $_POST['cryptpass']

et lancez le script.
Il faut que je fasse tourner ce script afin de voir si les données sont bien traité ensuite je rajouterai de quoi sécuriser un peux mieux le script, quelques conseil sur ce point serai très apprécié.
très gentil de votre part de me donner un coup de main

[Yoshio]

Je n'ai jamais utiliser smarty donc je ne sait pas s'il fera l'affaire mais niveau sécurité ce qu'il faut faire en gros :
- se protéger des injection sql (PDO avec requête préparée ou mysql_real_escape_string)
- se protégé des attaque cross script scripting en parsant l'affichage des donnés (au cas ou tu afficherais le login sur la page, il faut faire attention que le login ne contienne pas de code javascript ect) avec htmlentities().
- ne jamais faire confiance à des donnée post, get, provenant d'un formulaire, cookies, ... car elles peuvent êtres toute modifiée comme bon nous semble

Niveau try/catch/throw :
La classe :

Code :

1
2
3
4
5
6
7
class MaClasse
{
    public function __construct()
    {
        if (false) throw new Exception('Pas bon du tout !');
    }
}

L'utilisation :

Code :

1
2
3
4
5
6
7
8
9
10
...
try
{
    $obj = new MaClasse();
}
catch(Exception $e)
{
	echo $e->getMessage();
}
...

Bon, l'exemple est complètement débile mais ca illustre assez bien
Tu peux mettre des try dans la classe il n'y a pas de souci de ce coté.

Le throw sert à lancer une exception.

Tu devrais lire quelque article sur les exception pour mieux comprendre, commence par ici peut être : http://be.php.net/manual/fr/language.exceptions.php

[gtraxx]

un truc dans ce genre la pour la classe :

Code :

1
2
3
4
5
6
$stmt->execute();
			$result = $stmt->fetchAll();
			if ($result == false)
			throw new Exception('Erreur de chargement');
			else
			return $result;

j'ai repris le code a partir de la fonction execute
Mais voici la class au complet pour l'instant:

Code :

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
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
<?php
class UserAuthLogin{
	/**
	 * Request login user
	 *
	 * @var string
	 */
	public $users_login;
	/**
	 * Request login pass
	 *
	 * @var string
	 */
	public $mdp_login;
	/**
	 * Construc class initial for request
	 *
	 */
	function __construct(){
		if(isset($_POST['users']) || isset($_POST['cryptpass'])){
 
            $this->users_login = htmlentities($_POST['users']);
		    $this->mdp_login = htmlentities(sha1($_POST['cryptpass']));
          }
	}
	/**
	 * function connect database
	 *
	 * @return Array
	 */
	function loadAuth(){
 
			$dbh = new PDO('mysql:host=localhost;dbname=*******','root','*******');
			$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
			$stmt = $dbh->prepare("SELECT DISTINCT id_user,users from member where :users and :cryptpass");
		    $stmt->bindParam(':users',$this->users_login,PDO::PARAM_STR);
		    $stmt->bindParam(':cryptpass', $this->mdp_login,PDO::PARAM_STR);
			$stmt->setFetchMode(PDO::FETCH_ASSOC);
			$stmt->execute();
			$result = $stmt->fetchAll();
			if ($result == false)
			throw new Exception('Erreur de chargement');
			else
			return $result;
	}
	/**
	 * function Send session and redirect page
	 *
	 */
	function authSession(){
		if (isset($this->users_login)) {
			if(count($this->LoadAuth()) == 1){
				$_SESSION['userid'] = $this->users_login;
				$_SESSION['username'] = $this->mdp_login;
				header($page);
			}
		}
	}
}

Dite moi si je fais erreur

[Yoshio]

Es-tu sur que fetchAll retourne false en cas d'echec ?

Sinon oui on peux faire quelque chose comme ca.

[gtraxx]

j'editais mon message le temps que vous postiez le vôtre lol:
voici la class au complet :

Code :

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
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
<?php
class UserAuthLogin{
	/**
	 * Request login user
	 *
	 * @var string
	 */
	public $users_login;
	/**
	 * Request login pass
	 *
	 * @var string
	 */
	public $mdp_login;
	/**
	 * Construc class initial for request
	 *
	 */
	function __construct(){
		if(isset($_POST['users']) || isset($_POST['cryptpass'])){
 
            $this->users_login = htmlentities($_POST['users']);
		    $this->mdp_login = htmlentities(sha1($_POST['cryptpass']));
          }
	}
	/**
	 * function connect database
	 *
	 * @return Array
	 */
	function loadAuth(){
 
			$dbh = new PDO('mysql:host=localhost;dbname=*******','root','*******');
			$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
			$stmt = $dbh->prepare("SELECT DISTINCT id_user,users from member where :users and :cryptpass");
		    $stmt->bindParam(':users',$this->users_login,PDO::PARAM_STR);
		    $stmt->bindParam(':cryptpass', $this->mdp_login,PDO::PARAM_STR);
			$stmt->setFetchMode(PDO::FETCH_ASSOC);
			$stmt->execute();
			$result = $stmt->fetchAll();
			if ($result == false)
			throw new Exception('Erreur de chargement');
			else
			return $result;
	}
	/**
	 * function Send session and redirect page
	 *
	 */
	function authSession(){
		if (isset($this->users_login)) {
			if(count($this->LoadAuth()) == 1){
				$_SESSION['userid'] = $this->users_login;
				$_SESSION['username'] = $this->mdp_login;
				header($page);
			}
		}
	}
}

Mais pour le false je ne sais pas c'est en testant la class que je le verrai

[Yoshio]

Tu m'a mal compris pour le htmlentities. Tu ne doit l'utiliser que quand tu affiche des données à l'écran.

Donc genre à la ligne

Code :

echo htmlentities($this->users_login);

Bien que dans ton cas il n'y a même pas besoin car si le login n'est pas bien former ca s'arretera au check login/password.

Enlève le DISTINCT de ta requête SQL. Si tu as plus que 1 résultat il y a un problème. D'ailleurs c'est WHERE "nom_champ = :nom" normalement.

[gtraxx]

je pense que ceci est plus approprié :

Code :

SELECT id_user,users from member where users=:users and pass=:cryptpass

[Yoshio]

Pas la peine de récupérer le login de l'utilisateur, tu l'as déjà.

[gtraxx]

Donc j'enleve dans le where :

Code :

users=:users

je ne récupère que le mots de pass

Code :

pass=:cryptpass

Ben si j'ai bien compris

[Yoshio]

Euh le mot de passe je voit pas a quoi il pourrait te server par la suite.

[gtraxx]

Donc ceci :

Code :

1
2
 $stmt->bindParam(':users',$this->users_login,PDO::PARAM_STR);
	     $stmt->bindParam(':cryptpass', $this->mdp_login,PDO::PARAM_STR);

ne me sert a rien alors , y compris le where de la requête :

Code :

where users=:users and pass=:cryptpass

C'est suite a votre commentaire disant :

Pas la peine de récupérer le login de l'utilisateur, tu l'as déjà.

Je sais je suis en mode ralenti c'est temps-ci

[Yoshio]

Ce qui sert à rien c'est le "users" ici : "SELECT DISTINCT id_user,user".

Il n'est pas nécessaire de récupérer le nom de l'utilisateur car tu l'a déja. Bon j'avoue c'est aller chercher la ptite bête aussi

[gtraxx]

ah ok lol j'avais pas compris, j'ai de toute façon modifier la requête afin de faire un * c'est plus simple .
Comme sa je peux écrire directement une autre fonction pour l'envoi de mots de pass en cas d'oubli qui se baserai sur la fonction qui charge les données dans la base.
Par contre vous aviez raison sur le "false" dans la fonction de connexion a la sgbd.

Code :

1
2
3
4
if ($result == false)
				throw new Exception('Erreur de chargement');
			else
		return $result;

En faite, dans le formulaire si je tape une donnée correct pas de problème par contre si je l'envoi vide ou avec une donnée incorrect l'exception se mets en route:

Fatal error: Uncaught exception 'Exception' with message 'Erreur de chargement' in /opt/lampp/htdocs/clashdesign/_scriptroot/backend/admin/login.php:45 Stack trace: #0 /opt/lampp/htdocs/clashdesign/_scriptroot/backend/admin/login.php(55): UserAuthLogin->loadAuth() #1 /opt/lampp/htdocs/clashdesign/_dynamicroot/backend/login.phtml(9): UserAuthLogin->authSession() #2 {main} thrown in /opt/lampp/htdocs/clashdesign/_scriptroot/backend/admin/login.php on line 45

En gros je voulais simplement une exception si le chargement est impossible et non pour qu'il vérifie les données envoyé a ce niveau.
Quoi qu'il en soit je doit impérativement faire une série de test

[gtraxx]

j'ai rien dit tout fonctionne correctement et j'ai grossi ma ptit class
Je vous remercie de votre aide