2011-07-05 109 views
2

我目前正在爲我的應用程序登錄腳本。登錄將使用SSL,所有必需的資源將通過此服務。它不是保護任何東西就像一個銀行,但是我想知道什麼是正確與錯誤特別是對學習的目的我的登錄腳本

我愛我班的一些反饋,我已經開發了。我一直在閱讀網上的各種資料,而且看起來很矛盾。

領域,我覺得需要改進:

  • 使用的東西比SHA1算法用於存儲密碼更強。
  • 維護登錄 - 目前它次,每次20分鐘後出來。

沒有在這裏廢話少說是代碼:

class User extends Model{ 

    private $logLocation; 
    private $loginLog; 


    public function __construct(){ 
     $this->logLocation = 'system/logs/'; 
     $this->loginLog = "logins"; 
    } 

    /** 
    * 
    * Add User 
    * @param array $data An array of data that will get added to User table. 
    */ 
    public function add($data){ 
     $db = Database::getInstance(); 

     $salt = substr(md5(uniqid(rand(), true)),0,3); 

     $query = 'INSERT INTO user( user_id, user_username, user_password, user_salt, user_forename, user_lastname, user_email, user_attempts) 
      VALUES(:user_id, :user_username, sha1(:user_password), :user_salt, :user_forename, :user_lastname, :user_email, 0)'; 
     $args = array(
      ':user_id' => $data['user_id'], 
      ':user_username' => $data['user_username'], 
      ':user_password' => $data['user_password'].$salt, 
      ':user_salt' => $salt, 
      ':user_forename' => $data['user_forename'], 
      ':user_lastname' => $data['user_lastname'], 
      ':user_email' => $data['user_email']); 
     $db->query($query, $args); 

     SessionRegistry::instance()->addFeedback('user Saved Successfully'); 
     return true; 
    } 

    public function getUserId($username){ 
     $db = Database::getInstance(); 
     //Check to see if the username exists 
     $query = "SELECT user_id FROM user WHERE user_username = :username LIMIT 1"; 
     $results = $db->query($query, array(':username' => $username)); 
     return $results[0]['user_id']; 
    } 

    public function getUsername($userId){ 
     $db = Database::getInstance(); 
     //Check to see if the username exists 
     $query = "SELECT user_username FROM user WHERE user_username = :username LIMIT 1"; 
     $results = $db->query($query, array(':username' => $username)); 
     return $results[0]['user_username']; 
    } 

    /** 
    * 
    * Checks login details against that in the database 
    * @param string $username 
    * @param string $password 
    */ 
    public function checkLogin($username, $password){ 
     $db = Database::getInstance(); 
     //Check to see if the username exists 
     $query = "SELECT user_salt, user_password, user_attempts FROM user WHERE user_username = :username LIMIT 1"; 
     $results = $db->query($query, array(':username' => $username)); 

     //No results return false 
     if(count($results) < 1){ 
      $this->logLoginAttempt($username, 'Incorrect Username'); 
      return false; 
     } 

     //Check to see if the user is blocked 
     if((int)$results[0]['user_attempts'] >= 3){ 
      $this->logLoginAttempt($username, 'Blocked User Login'); 
      return false; 
     } 

     //Check to see if the passwords match 
     if(sha1($password.$results[0]['user_salt']) == $results[0]['user_password']){ 
      $this->setLogin($username); 
      return true; 
     } 
     else{ 
      //Incorrect Password 
      $this->logLoginAttempt($username, 'Incorrect Password'); 
      $this->failedLoginIncrement($username); 
      return false; 
     } 
    } 

    /** 
    * 
    * Increments the failed login attempt for a user. 
    * 3 Strikes and they get locked out. 
    * @param string $username 
    */ 
    private function failedLoginIncrement($username){ 
     $db = Database::getInstance();   
     //Update the IP address of the user from where they last logged in 
     $query = 'UPDATE user SET user_attempts = user_attempts + 1 WHERE user_username = :username'; 
     $db->query($query, array(':username' => $username)); 

     //Check to see if the user has reached 3 strikes if so block them. 
     $query = 'SELECT user_attempts FROM user WHERE user_username = :username LIMIT 1'; 
     $results = $db->query($query, array(':username' => $username)); 

     if($results[0]['user_attempts'] >= 3){ 
      //We need to block the user 
      $query = 'UPDATE user SET user_blocked = 1 WHERE user_username = :username'; 
      $db->query($query, array(':username' => $username)); 
     } 
     return true; 
    } 

    /** 
    * 
    * Logs a failed login attempt to a log file so these can be monitored 
    * @param string $username 
    * @param string $reason 
    */ 
    private function logLoginAttempt($username, $reason){ 
     $fh = fopen($this->logLocation.$this->loginLog, 'a+') or die("can't open file"); 
     $logLine = date('d/m/Y h:i') . ' Login Attempt: ' . $username . ' Failure Reason: ' . $reason . " IP: " . $_SERVER['REMOTE_ADDR'] . "\n"; 
     fwrite($fh, $logLine); 
     fclose($fh); 
     return true; 
    } 

    /** 
    * 
    * Sets the login data in the session. Also logs IP and resets the failed attempts. 
    * @param string $username 
    */ 
    private function setLogin($username){   
     $db = Database::getInstance();   
     //Update the IP address of the user from where they last logged in 
     $query = 'UPDATE user SET user_ip = :ip, user_attempts = 0 WHERE user_username = :username'; 
     $db->query($query, array(':username' => $username, ':ip' => $_SERVER['REMOTE_ADDR'])); 

     ini_set("session.use_only_cookies", TRUE); //Forces the session to be stored only in cookies and not passed over a URI. 
     ini_set("session.use_trans_sid", FALSE); //Stop leaking session IDs onto the URI before browser can check to see if cookies are enabled. 
     ini_set("session.cookie_lifetime", 1200); //Time out after 20mins 

     //Now add the session vars to set the user to logged in. 
     session_start(); 
     session_regenerate_id(true); //Regenerate the session Id deleting old session files. 
     $_SESSION['valid'] = 1; 
     $_SESSION['userid'] = sha1($this->getUserId($_POST['username'] . "SALTHERE")); 
    } 

    /** 
    * 
    * Checks to see if a user is currently logged in. 
    */ 
    public function loggedIn(){ 
     if($_SESSION['valid']){ 
      return true; 
     } 
     else{ 
      return false; 
     }  
    } 

    /** 
    * 
    * Logs a current user out by destroying the session 
    */ 
    public function logout(){ 
     // Unset all of the session variables. 
     $_SESSION = array(); 

     // If it's desired to kill the session, also delete the session cookie. 
     // Note: This will destroy the session, and not just the session data! 
     if (ini_get("session.use_cookies")) { 
      $params = session_get_cookie_params(); 
      setcookie(session_name(), '', time() - 42000, 
       $params["path"], $params["domain"], 
       $params["secure"], $params["httponly"] 
      ); 
     }   
     // Finally, destroy the session. 
     session_destroy(); 
    } 
} 

然後我使用這個類,像這樣:

require_once('User.php'); 
$user = new User(); 
$loggedIn = $user->checkLogin($_POST['username'], $_POST['password']); 
if($loggedIn){ 
    //redirect to member area 
} 
else{ 
    //show login screen 
} 

然後在網頁上,我需要檢查,如果用戶登錄在

require_once('User.php'); 
$user = new User(); 
if(!$user->loggedIn()){ 
    //redirect to login page 
} 

我很想聽聽你的想法評論不錯或不好,加上我可以用來改進我的登錄腳本的任何其他想法。

預先感謝您的時間

馬特

+6

你可能會得到更多的幫助,這張貼在這裏,而不是:http://codereview.stackexchange.com –

+0

它有助於有更具體的問題;類似「這不,以防工作」或「我聽說過意見衝突A和B,這是正確的?」 –

回答

0

我建議從數據庫中獲取分離的會話管理。把它們分成兩個不同的類。

+0

感謝戴夫說有一定道理,現在你提到它。我想我試圖讓原則上首先確定的基礎知識。 – Matthew