2013-02-13 2 views
1

그래서이 스크립트를 내 사이트에 쓰려고합니다. 꽤 엉망이고 부서져 보입니다. 어쩌면 누군가가 나를 조금 깔끔하게 정리하고 잘못된 정보를 설명 할 수 있습니다. 또한 더 짧게 만들 수있는 방법이 있으며, 조금 안전하지 않은 것 같습니다. 감사합니다.PHP 등록 스크립트

<?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('/[^[email protected]]/', '', $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._-][email protected][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; 
    } 
} 

?> 
+2

"상당히 엉망이고 부서졌다"는 것에 대한 설명이 도움이 될 것입니다. 오류가 있습니까? 예상대로 행동하지 않는 것이 있습니까? –

답변

3

스 워시, 항상 코드를 작성하는 더 좋은 방법이 있습니다. 코드를 왜 다시 작성해야하는지, 코드를 다시 작성하기보다는 왜 안전하지 않은지 다시 생각해보고 싶습니다. 잘하면이 방법을 더 배우게됩니다.

먼저 MD5로 비밀번호를 해싱하는 것은 완전히 안전하지 않습니다. MD5가 존재한다는 사실을 기억하십시오. SHA1조차도 안전한 것을 위해 사용하지 마십시오. 대신 bcrypt 또는 PBKDF2 (SHA2 또는 Whirlpool 및 ~ 2000 + rounds)를 사용해야합니다. 보다 나은 보안을 구현하는 데 도움이되는 기사 및 라이브러리에 대한 팁과 링크는 내 대답 Secure Hash and Salt for PHP Passwords을 참조하십시오.

둘째, mysql_ * 함수는 PHP 5.5부터 deprecated입니다. MySQLi를 사용하면 얻을 수 있지만 연결을 처리하고 이스케이프/필터링을 쿼리하려면 PDO과 같은 일관된 인터페이스를 사용해야합니다. PHP 5.5에서 소프트웨어를 구현하지는 않지만 호스트가 PHP 버전을 업그레이드하기로 결정한 경우 항상 제어 할 수있는 것은 아닙니다. 미래의 호환성을 위해 최대한 많이 최적화하십시오. 또한 PDO에는 its documentation에 설명 된 몇 가지 멋진 기능이 있습니다.

셋째, 정규식을 사용하여 쿼리 변수를 "필터링"해서는 안됩니다. 가장 안전한 방법은 숫자에 intval/floatval을 사용하고 나머지는 mysql_escape_string (또는 mysqli_real_escape_string)과 같은 DB 라이브러리를 통해 이스케이프 처리하거나 prepared statements을 사용하면됩니다 (변수가 삭제됩니다).

넷째, 표시 논리를 개체에 넣고 있습니다. 그것에 대해 생각해보십시오.이 목적은 어떤 목적/역할을 수행합니까? 등록 논리를 처리합니까? 등록 데이터를 저장합니까? 여기에 Single Responsibility Principle을 사용하는 것이 좋습니다. 객체가 하이브리드 모델 제어기처럼 동작하고 가상의 정보가있는 것처럼 보입니다. 이것을 임시로 저장하거나 심지어 다른 것을하기로 결정하기 위해 RegistrationModel 클래스와 RegistrationController 클래스로 이것을 확장 할 수 있습니다. 그러나 수업 시간이 길어질수록 더 많은 책임을지게 될 것입니다.

또한 등록의 모든 속성을 비공개로 설정하면 두 가지 이상의 등록 방법을 사용할 수 없습니다. 트위터 나 페이스 북과 같은 OAuth를 통해 로그인하는 등 프로세스에 대한 지름길을 원했지만 등록에서 일부 로직을 재사용해야하는 경우에는 어떻게해야할까요? 이러한 속성은 최소한 사용자가 상속하거나 공개 할 수 있도록 보호되어야하므로 다른 객체 (예 : 등록이 성공적임을 사용자에게 알리는 객체)와 상호 작용할 수 있습니다.