2011-07-05 110 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