Mon script d'inscription [conseils]

Par SmyDrowl, il y a 11 ans

PHP

Bonsoir, je viens de finir mon script pour que l'utilisateur s'inscrive et j'aimerais que quelqu'uns d'entre vous critique le code et me dirige vers un truc plus optimisé :)

Mon inscription :

<?php require 'app/includes.php'; $title = 'S\'enregistrer'; if(isset($_POST['identifiant']) && isset($_POST['password']) && isset($_POST['password-repeat']) && isset($_POST['email'])) { $identifiant = $_POST['identifiant']; $password = sha1($_POST['password']); $passwordConfirm = sha1($_POST['password-repeat']); $email = $_POST['email']; if(empty($identifiant) OR empty($password) OR empty($email)) { setFlash('Vous devez remplir tout les champs !', 'danger'); } if(strlen($identifiant) < 5) { setFlash('Votre identifiant doit contenir 5 caractères minimum !', 'danger'); } if(strlen($password) < 8) { setFlash('Votre identifiant doit contenir 8 caractères minimum !', 'danger'); } if($passwordConfirm != $password) { setFlash('Vous n\'avez pas bien confirmé votre mot de passe', 'danger'); } if(!filter_var($email, FILTER_VALIDATE_EMAIL)) { setFlash('Votre adresse e-mail n\'est pas valide !', 'danger'); }else{ $req = $db->query('SELECT identifiant FROM users WHERE identifiant = '.$identifiant.''); $count = $req->rowCount(); if($count < 1) { $req = $db->query('SELECT email FROM users WHERE email = '.$email.''); $count = $req->rowCount(); if($count < 1) { $req = $db->preapre('INSERT INTO users(identifiant, password, email,) VALUES(:identifiant, :password, :email)'); $req->bindValue('identifiant', $identifiant); $req->bindValue('password', $password); $req->bindValue('email', $email); $req->execute(); setFlash('Vous êtes maintenant inscrit sur RoverZProduction !', 'success'); header('Location: '. WEBROOT .'login.php'); }else{ setFlash('Cette adresse e-mail est déjà utilisé par un utilisateur', 'danger'); } }else{ setFlash('Cet identifiant est déjà utilisé par un utilisateur', 'danger'); } } } require 'partials/header.php'; ?> <div class="page-header"> <h3> <i class="fa fa-user"></i> S'enregistrer </h3> </div> <form action="" method="post"> <div class="form-group"> <label for="identifiant"> Votre identifiant </label> <input type="text" name="identifiant" id="identifiant" class="form-control"> </div> <div class="form-group"> <label for="password"> Votre mot de passe </label> <input type="password" name="password" id="password" class="form-control"> </div> <div class="form-group"> <label for="password-repeat"> Confirmation de votre mot de passe </label> <input type="password" name="password-repeat" id="password-repeat" class="form-control"> </div> <div class="form-group"> <label for="email"> Votre adresse e-mail </label> <input type="text" name="email" id="email" class="form-control"> </div> <button type="submit" class="btn btn-success btn-lg btn-block"> <i class="fa fa-check"></i> S'enregistrer </button> </form> <?php require 'partials/footer.php'; ?>

14 réponses

François Cammas, il y a 11 ans

1/

if(strlen($identifiant) < 5) { setFlash('Votre identifiant doit contenir 5 caractères minimum !', 'danger'); } if(strlen($password) < 8) { setFlash('Votre identifiant doit contenir 8 caractères minimum !', 'danger'); }

Lors du bloc password tu utilises identifiant dans ton retour.
Creer des fonctions pour tes methodes recurrentes type : function tailleMinimum($var, $val, $label)

Tu as un problème sur tes conditions, il n'y a que le test sur la validité de l'adresse email qui bloque l'inscription !!! abusé

Alexandre #lbac, il y a 11 ans
$error_message = '': if( condition_non_respecte ) { $error = TRUE; $error_message .= 'Votre identifiant est trop court<br />'; } if( $error === FALSE ) { // On lance la procédure d'inscription } else { // On affiche les erreurs }

Par exemple.

kewai, il y a 11 ans

Tu fais 2 requetes pour rechercher si il n'y a pas déjà un nom ou un email enregistré mais tu pourrais le faire en une requete.

$req = $db->query('SELECT identifiant,email FROM users WHERE identifiant = '.$identifiant.' OR email = '.$email); $results = mysql_fetch($req); foreach ( $results as $key => $value) { if ( strtolower($results[$key]['identifiant']) == strtolower($identifiant)) { setFlash('Cet identifiant est déjà utilisé par un utilisateur', 'danger'); } if ( strtolower($results[$key]['email']) == strtolower($email)) { setFlash('Cette adresse e-mail est déjà utilisé par un utilisateur', 'danger'); } } if(empty($_SESSION['Flash'])) { ... ton insert }
Couss, il y a 11 ans

Pour plus de sécurité de je conseil de préparer tes requêtes de selection aussi car tu controle pas vraiment ce que saisie l'utilisateur, il pourrai essayer d'exploité une faille type SQL injection
ex :

$req = $db->query('SELECT identifiant FROM users WHERE identifiant = '.$identifiant.'');

Il pourrai passer des injections via le champ identifiant.

Pour plus de sécurité également, personnelement j'ai pris l'habitude d'utiliser les "grain de sel" pour mes mots de passe.
C'est un petit string que tu met concatène soit avant, soit après voir les deux du mot de passe souhaité par l'utilisateur avant de faire ta fonction de hachage :

/* dans un fichier de config ou avec les constantes */ $salt = '8ny2Q4U373Ax2nX6Qp3LM4srGeDLbz8'; /* $_POST['password'] est le mot de passe saisie pour l'inscription ou la connection */ $password = sha1($salt.$_POST['password'].$salt);

De plus niveau confort d'utilisation tu pourrai peut être connecté automatiquement l'utilisateur si toutes ces informations sont correct et le redirigé vers sont espace plutôt que de lui redemandé de saisir ses identifiants

Couss

kewai, il y a 11 ans

Aendawan
Tu devrais créer des fonctions à l'extérieur... Par exemple tu aurais pourrait réutiliser la vérification si c'est vide dans les deux... La où pour ma part je trouve que tu tournes autour du pot, c'est que tu déclares toutes tes variables false pour ensuie les déclarer true sur il y a une erreur en te servant toujours de $error_return['error'] = "TRUE"; + d'une spécifique.

Pour ne pas simplement bacler ça, dire que $error_return = []; et d'ajouter des entrées.
Tu verifies si ta variable est bien vide pour continuer ton script.

Meme chose que j'ai dit pour vérifier l'email et le nom... Ca peut être fait en une requete :)

SmyDrowl, il y a 11 ans
<?php $errors = []; if(!array_key_exists('identifiant', $_POST) ||$_POST['identifiant'] == '') { $errors['identifiant'] = 'Vous n\'avez pas renseigné votre identifiant'; } if(!array_key_exists('password', $_POST) ||$_POST['password'] == '') { $errors['password'] = 'Vous n\'avez pas renseigné votre mot de passe'; } if(!array_key_exists('password-repeat', $_POST) ||$_POST['password-repeat'] != $_POST['password']) { $errors['password-repeat'] = 'Vous n\'avez pas correctement confirmer votre mot de passe'; } if(!array_key_exists('email', $_POST) ||$_POST['email'] == '' || !filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) { $errors['email'] = 'Vous n\'avez pas renseigné une adresse e-mail valide'; } global $db; $req = $db->query('SELECT identifiant,email FROM users WHERE identifiant = '.$_POST['identifiant'].' OR email = '.$_POST['identifiant'].''); $resultats = $req->fetchAll(); foreach ($resultats as $key => $value) { if($resultats[$key]['identifiant'] == $_POST['identifiant']) { $errors['identifiant'] = 'Cet identifiant existe déjà !'; } if($resultats[$key]['email'] == $_POST['email']) { $errors['email'] = 'Cette adresse e-mail existe déjà !'; } } if(strlen($_POST['identifiant']) < 5) { $errors['identifiant'] = 'Votre identifiant est trop court !'; } if(strlen($_POST['password']) < 8) { $errors['password'] = 'Votre mot de passe est trop court !'; } if(!empty($errors)) { session_start(); $_SESSION['errors'] = $errors; header('Location: Inscription.php'); } if(!empty($success)) { session_start(); $_SESSION['success'] = $success; header('Location: connexion.php'); } if(!$errors) { $identifiant = $_POST['identifiant']; $password = sha1($salt.$_POST['password'].$salt); $passwordConfirm = sha1($salt.$_POST['password-repeat'].$salt); $email = $_POST['email']; $req = $db->prepare('INSERT INTO users(identifiant, password, email) VALUES($identifiant, $password, $email)'); $req->bindParam('identifiant', $identifiant); $req->bindParam('password', $password); $req->bindParam('email', $email); $req->execute(); $success['inscription'] = 'Vous êtes maintenant inscrit !'; } ?>

voilà où j'en suis et j'ai une erreur au niveau de mes prepare comme quoi ce ne sont pas des objets !

Alexandre #lbac, il y a 11 ans

Pour commencer tu ne dois pas lancer session_start() ici mais dans un fichier fichier que tu inclus en amont (ou en haut de ce fichier) et pour l'objet $db, il est défini quelque part ?

Sinon ton erreur :

Remplaces

$req = $db->prepare('INSERT INTO users(identifiant, password, email) VALUES($identifiant, $password, $email)');

par

$req = $db->prepare('INSERT INTO users(identifiant, password, email) VALUES(:identifiant, :password, :email)');
SmyDrowl, il y a 11 ans

aucun changement je ne vois pas l'erreur :/

Alexandre #lbac, il y a 11 ans

Quelle est l'erreur retournée ?

SmyDrowl, il y a 11 ans

Fatal error: Call to a member function prepare() on a non-object in C:\wamp\www\post-inscription.php on line 25

Alexandre #lbac, il y a 11 ans

Et en remplaçant ça

$req->bindParam('identifiant', $identifiant); $req->bindParam('password', $password); $req->bindParam('email', $email); $req->execute();

par

$req->execute(array( 'identifiant' => $identifiant, 'password' => $password, 'email' => $email, ));
SmyDrowl, il y a 11 ans
$req = $db->prepare('SELECT identifiant FROM users WHERE identifiant = :identifiant'); $req->execute(); $resultats = $req->fetchAll(); foreach ($resultats as $key => $value) { if($resultats[$key]['identifiant'] == $_POST['identifiant']) { $errors['identifiant'] = 'Cet identifiant existe déjà !'; } if($resultats[$key]['email'] == $_POST['email']) { $errors['email'] = 'Cette adresse e-mail existe déjà !'; } }

prepare ou query sa me renvoie la même erreur

Alexandre #lbac, il y a 11 ans

Ca vient de tes données qui ne sont pas bonnes alors. Tu as vérifié que $_POST contient bien quelque chose ?

$db est déclaré quelque part ?

Lramelot, il y a 11 ans

Vos solutions pour la vérification des erreurs me semblent plutôt compliquées. Voici ma façon de faire :

<?php $errors = array(); if(conditionpseudo) { $errors[] = 'Votre pseudo est déjà présent'; if(conditionPseudo2) { $errors[] = 'Votre pseudo est trop long'; ... if(empty($errors)) { // Inscription } // Affichage des erreurs echo implode('<br>', $errors); ?>

C'est ma façon de faire, pas sur que ce soit la meilleure mais bon.