Script PHP Register

J'essaie donc d'écrire ce script pour mon site. Il semble assez dérangé et brisé. Peut-être que quelqu'un peut m'aider à le ranger un peu et à expliquer ce qui pourrait être incorrect. De plus, est-ce qu'il y a un moyen de le rendre plus court, me paraît un peu dangereux. Je vous remercie.

<?php class Register { private $username; private $password; private $password2; private $passmd5; private $email; private $email2; private $errors; private $rtoken; public function __construct() { $this->errors = array(); $this->username = $this->filter($_POST['ruser']); $this->password = $this->filter($_POST['rpass']); $this->password2 = $this->filter($_POST['rpass2']); $this->email = $this->filter($_POST['remail']); $this->email2 = $this->filter($_POST['remail2']); $this->rtoken = $_POST['rtoken']; $this->passmd5 = md5($this->password); } public function process() { if ($this->valid_rtoken() && $this->valid_data()) $this->register(); return count($this->errors) ? 0 : 1; } public function filter($var) { return preg_replace('/[^a-zA-Z0-9@.]/', '', $var); } public function register() { mysql_query("INSERT INTO users(username,password,email) VALUES ('{$this->username}','{$this->passmd5}','{$this->email}')"); if (mysql_affected_rows() < 1) $this->errors[] = '<font color="red">Database error</font>'; } public function user_exists() { $data = mysql_query("SELECT ID FROM users WHERE username = '{$this->username}'"); return mysql_num_rows($data) ? 1 : 0; } public function email_exists() { $data = mysql_query("SELECT ID FROM users WHERE email = '{$this->email}'"); return mysql_num_rows($data) ? 1 : 0; } public function show_errors() { echo ""; foreach ($this->errors as $key => $value) echo $value . "<br>"; } public function valid_data() { if ($this->user_exists()) $this->errors[] = '<font color="red">Username Exists</font>'; if ($this->email_exists()) $this->errors[] = '<font color="red">email exists</font>'; if (empty($this->username)) $this->errors[] = '<font color="red">check your username</font>'; if (empty($this->password)) $this->errors[] = '<font color="red">check your password</font>'; if ($this->password != $this->password2) $this->errors[] = '<font color="red">Passwords do not match</font>'; if (empty($this->email) || !eregi('^[a-zA-Z0-9._-]+@[a-zA-Z0-9._-]+\.[a-zA-Z]{2,4}$', $this->email)) $this->errors[] = '<font color="red">Check your email</font>'; if ($this->email != $this->email2) $this->errors[] = '<font color="red">Emails do not match</font>'; return count($this->errors) ? 0 : 1; } public function valid_rtoken() { if (!isset($_SESSION['rtoken']) || $this->rtoken != $_SESSION['rtoken']) $this->errors[] = '<font color="red">Check</font>'; return count($this->errors) ? 0 : 1; } } ?> 

Swoosh, il existe toujours de meilleurs moyens d'écrire du code. J'espère vous défier de repenser pourquoi votre code est long et pourquoi il pourrait être dangereux, plutôt que de réécrire votre code pour vous. J'espère que vous en apprendrez davantage de cette façon.

Pour commencer, le hashage du mot de passe avec MD5 est complètement insécurisé. N'oubliez pas que MD5 a déjà existé, et n'utilisez pas même SHA1 pour quelque chose de sécurisé. Au lieu de cela, vous devez utiliser bcrypt ou PBKDF2 (avec SHA2 ou Whirlpool et ~ 2000 + rounds). Je vous suggère de vous référer à ma réponse à Secure Hash and Salt pour les passwords PHP pour get des conseils et des liens vers des articles et des bibliothèques afin d'aider à améliorer la security.

Deuxièmement, les fonctions mysql_ * sont obsolètes à partir de PHP 5.5. L'utilisation de MySQLi vous permettra, mais vous devriez utiliser une interface cohérente, telle que PDO, pour gérer les connections et interroger l'échappement / filtrage. Bien que vous ne puissiez pas build votre logiciel en PHP 5.5, vous n'aurez pas toujours le contrôle si un hôte décide de mettre à niveau votre version de PHP. Optimisez pour la compatibilité future autant que vous le pouvez maintenant. De plus, PDO possède des fonctionnalités rustiques expliquées dans sa documentation .

Troisièmement, vous ne devez pas utiliser une expression régulière pour "filterr" vos variables de requête. La chose la plus sûre que vous pouvez faire est d'utiliser l'intval / floatval sur n'importe quel nombre, et d'échapper au rest à travers la bibliothèque DB que vous utilisez comme mysql_escape_ssortingng (ou mysqli_real_escape_ssortingng ) OU utiliser des instructions préparées (ce qui va désintégrer les variables pour vous).

Quasortingèmement, vous mettez la logique d'affichage dans votre object. Pensez-y: quel objective / rôle cet object satisfait-il? Est-ce que cela gère la logique d'logging? Est-ce qu'il stocke datatables d'logging? C'est une bonne idée d'utiliser le principe de responsabilité unique ici. Il semble que l'object est censé agir comme un model-controller hybride, avec des informations de présentation. Vous pouvez l'étendre aux classs RegistrationModel et RegistrationController pour gérer le stockage temporaire des données, ou même décider de faire autre chose. Mais callbackez-vous, plus les responsabilités que votre class a les plus de façons d'enrayer.

De plus, en créant tous les attributes de Registre privé, vous ne pouvez pas avoir plus d'une façon de vous inscrire. Qu'en est-il si vous vouliez un raccourci sur le process, par exemple pour vous connecter via OAuth (comme Twitter ou Facebook), mais avez-vous besoin de réutiliser une partie de la logique dans Register? Ces attributes devraient au less être protégés afin que vous puissiez les hériter ou même public afin qu'un autre object puisse les interagir (par exemple, un object qui notifie à l'user que son logging réussit).