FuelPHP Forums

 /**
  * Runs before every request to this controller.  It sets some template
  * variables which are used throughout the application.
  *
  * @return  void
  */
 public function before()
 {
  parent::before();
  
  /**
   * 1. check if user logged in
   * 2. if not check if remembered cookie is set and login if possible
   * 3. user not logged in
   */
  if(Auth::check())
  {
   $this->current_user = Model_User::find_by_username(Auth::get_screen_name());
   $this->logged_in = true;
  }
  elseif($user = static::is_remembered())
  {
   if (Auth::check())
   {
    $this->current_user = $user;
    $this->logged_in = true;
   }
  }
  else
  {
   $this->current_user = null;
   $this->logged_in = Auth::check() ? true : false;
  }
  
  View::set_global('current_user', $this->current_user);
  View::set_global('logged_in', $this->current_user);
  
  $this->template->header = View::forge('home/header');
  $this->template->footer = View::forge('home/footer');
 }
 
 /**
  * Check if remember me is set and valid
  */
 protected static function is_remembered()
 {
  \Config::load('simpleauth', true);
  
  $encoded_val = Cookie::get(Config::get('simpleauth.remember_me.cookie_name'));
  
  if ($encoded_val)
  {
   $val = base64_decode($encoded_val);
   list($saltedpasswordhash, $cookie_pass, $login_hash) = explode(':', $val);
   $user = Model_User::find_by_remember_me($cookie_pass);
   $dbpasshash = sha1(sha1($user->password).sha1(Config::get('simpleauth.salt')));
         
   if ($user)
   {
    // set auth session variables
    \Session::set('username', $user->username);
    \Session::set('login_hash', $login_hash);
    \Session::instance()->rotate();   

    $user->last_login = time();
    $user->save();
    return $user;
   }
   else
   {
    return false;
   }
  }

  return false;
 }

There's a few things very wrong here:
- Don't put a user's password in a cookie, I don't care how many times you (re-)hash it. Just don't. A password is something very personal you should consider to be a great secret never to be shared or hinted at. You only use a user's password (or whatever relates to it, or part of it) to check when logging in, don't ever transmit it and don't use it for anything else.
- I can steal your session by just stealing your cookie, I would be able to do everything once I have the cookie. This will even tell me the user's hashed password and the user's current login hash.
- This won't rotate the user's login-hash, thus keeping it valid as long as the cookie keeps using it. Enlarging the problem from my previous point. You should force a re-login when creating something like this, not re-using an old login.
- This is logic that should be in the auth driver, not in a controller.
- $dbpasshash seems useless, even causing errors when the cookie's data is wrong. Remember me functionality is very dangerous, you need to do a lot more research to understand how to implement it safely.