PHP註冊腳本

Question

所以我想爲我的網站寫這個腳本。 它看起來相當混亂和破碎。 也許有人可以幫我整理一下，解釋一下可能不正確。 另外，有沒有辦法讓它更短，對我來說看起來有點不安全。 謝謝。

<?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; 
    } 
} 

?> 

Answer 1

Swoosh，總是有更好的方法來編寫代碼。我希望挑戰你重新思考爲什麼你的代碼很長，爲什麼它可能不安全，而不是爲你重寫你的代碼。希望你會這樣學習更多。

首先，用MD5對密碼進行哈希處理是完全不安全的。請忘記MD5曾經存在，請不要使用SHA1來保證任何安全。相反，你應該使用bcrypt或者PBKDF2（使用SHA2或者Whirlpool和〜2000 + rounds）。我建議你參考我對Secure Hash and Salt for PHP Passwords的回答，以獲得有關文章和圖書館的提示和鏈接，以幫助實現更好的安全性。

其次，從PHP 5.5起，mysql_ *函數是deprecated。使用MySQLi將幫助您，但您應該使用一致的接口（如PDO）來處理連接，並查詢轉義/篩選。雖然你可能不會在PHP 5.5中構建你的軟件，但如果一個主機決定升級你的PHP版本，你將無法控制它。儘可能多地優化未來的兼容性。另外，PDO有一些漂亮的功能，在its documentation中有解釋。

第三，你不應該使用正則表達式來「過濾」你的查詢變量。你可以做的最安全的事情是在任何數字上使用intval/floatval，並通過你使用的數據庫庫，如mysql_escape_string（或mysqli_real_escape_string）或使用prepared statements（它將爲你清理變量）轉義剩下的部分。

第四，你正在把顯示邏輯放到你的對象中。想一想：這個對象有什麼目的/作用？它處理註冊邏輯嗎？它是否存儲註冊數據？這裏使用Single Responsibility Principle是個好主意。看起來這個對象應該像一個混合模型控制器一樣工作，其中包含表示信息。您可以將其擴展到RegistrationModel和RegistrationController類來處理臨時存儲數據，或者甚至決定執行其他操作。但請記住，班級承擔的責任越多，就有越多的方式需要打破。

另外，通過使註冊爲private的所有屬性，您不能有多種註冊方式。如果你想通過OAuth（例如Twitter或Facebook）登錄進程，但需要重用註冊表中的某些邏輯，該怎麼辦？這些屬性至少應該受到保護，以便您可以繼承它們，甚至是公共的，以便另一個對象可以與它們進行交互（例如通知用戶它們的註冊成功的對象）。