私のログインスクリプト

Question

私は現在自分のアプリケーションのログインスクリプトを開発中です。ログインは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 
} 

in

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

私はあなたの考えをよく聞きたいと思います悪いと私のログインスクリプトを改善するために使用できる他のアイデア。お時間を事前に

おかげ

マット

Answer 1

私は、データベース・アクセスからのセッション管理を分離することをお勧めします。何とか2つの異なるクラスに入れてください。