Security of authentication check for a blog

Question

I am currently making a very big blogging system for my Website where I want really neat code! So, I was wondering: is this method safe/does it have any problems with security? Yes, I'll be encrypting the password anyway. I also want to know if there is a better way to achieve the same idea. Also any tips on implementing high security and high performance?

<?php
class Users {
    protected $db;
    public $id;
    public $username;
    public $password;
    public $lastlogin;
    public $email;
    public $rank;
    public $creationdate;
    public $country;
    public $status;

    public function __construct() {
        $this->db = new mysqli('localhost', 'root', 'php123', 'ht');
    }

    public function login($username, $password) {
        $query = $this->db->prepare("SELECT username, password FROM blog_users WHERE username = ? AND password = ? LIMIT 1");
        $query->bind_param('ss', $username, $password);
        $query->execute();
        $query->bind_result($username, $password);
        $query->store_result();
        if($query->num_rows == 1) {
            while($query->fetch()) {
                $userData = getData($username);

                $this->id             =   $this->userData['id'];
                $this->username       =   $username;
                $this->password       =   $password;
                $this->lastlogin      =   time();
                $this->email          =   $this->userData['email'];
                $this->rank           =   $this->userData['rank'];
                $this->creationdate   =   $this->userData['creationdate'];
                $this->country        =   $this->userData['country'];
                $this->status         =   $this->userData['status'];

                $_SESSION['loggedin'] = 1;
                return true;
            }
        } else {
            return false;
        }
        $query->close();
        $query->free_result();
    }

    public function getData($username) {
        $query = $this->db->prepare("SELECT id, email, rank, creationdate, country, status FROM blog_users WHERE username = ?");
        $query->bind_param('s', $username);
        $query->execute();
        $query->bind_result($id, $email, $rank, $creationdate, $country, $status);
        $query->store_result();
        $userData = array(
            'id'           => $id,
            'email'        => $email,
            'rank'         => $rank,
            'creationdate' => $creationdate,
            'country'      => $country,
            'status'       => $status
        );
        return $userData;
        $query->close();
        $query->free_result;
    }

}
?>

Answer 1

Ok, just a few quick thoughts, will be adding to this answer later today (ie after work):

One class, One task
Chances are, you've already heard of the SOLID principles. Well, here they are again: your code is in violation of the SRP (Single Responsability principle). A class has one task. No more, no less. Your class does 4 things: it stores data (all those public properties), messes with sessions, connects to the DB and performs queries.
The Database connection and queries are all things that should be dealt with in another class, that simply connects to the DB and executes queries.

This other class could then have a method like this:

public function login(User $user)
{
    $query = $this->db->prepare("SELECT username, password FROM blog_users WHERE username = ? AND password = ? LIMIT 1");
    $query->bind_param('ss', $user->username, $user->password);
    $query->execute();
    $query->bind_result($user->username, $user->password);
    $query->store_result();
    if($query->num_rows == 1)
    {
        $query->fetch();
        $query->close();
        return $this->getUserData($user);//changed name of method
    }
    $query->close();
    throw new RuntimeException('User can not login');//error
}

Even this isn't close to efficient code, because it still calls a second method to, basically, repeat the same query. Why would you do so, why not simply write:

public function login(User $user)
{
    $query = $this->db->prepare(
        "SELECT id, email, rank, creationdate,
            country, status, username, password
        FROM blog_users
        WHERE username = ? AND password = ? LIMIT 1"
    );
    $query->bind_param('ss', $user->username, $user->password);
    $query->execute();
    $query->bind_result(
        $user->id,
        $user->email,
        $user->rank,
        $user->creationdate,
        $user->country,
        $user->status,
        $user->username,
        $user->password
    );
    $query->store_result();
    if($query->num_rows == 1)
    {
        $query->fetch();
        $query->free_result();//call free_result...
        $query->close();//and close here, too: your code doesn't do this
        return $user;
    }
    $query->free_result();
    $query->close();
    throw new RuntimeException('User can not login');//error
}

Isn't that a lot shorter, clearer and doesn't this also do exactly what you are doing? K.I.S.S!

Classes with a dependency, should be open to injection
Your current code is hard to test: the constructor connects to the DB, there is no way for me (the user), or you (when testing) to mock the db connection, or use a bogus testing table.
The best way to make your class easier to test it to allow the user to pass a specific connection to the constructor, and then he can use your class/functionality to work on the db of his choice:

public function __construct(mysqli $connection = null)
{//default null means the argument is optional
    if ($connection === null)
        $connection = new mysqli();//connect to default
    $this->db = $connection;//use connection we've created, or the one the user passed through to the constructor
}

And of course: a DB connection that is opened, should be closed: when your class dies (is destructed), the DB connection should die, too:

public function __destruct()
{
    $this->db->close();
    $this->db = null;
}

That should be added to your code, too. If you don't then the connection will be kept open until your script has terminated, which could be longer than you want. If performance and cleanliness matter to you, then this should be a habit.

Standards, coding style
Yes, I complain/nag a lot about coding style and standards but if you are, like you say, creating a sizeable code-base, then you should know that coding style and standards are insanely important.
The unofficial PHP standards can be found here. Though unofficial, all major players (ie Zend, Symfony, Cake, Doctrine, ...) all subscribe to them.

Other niggles:
Your class has tons of properties, and it's those properties that you are assigning in the methods. Why, then aren't you assigning $username as soon as you can, so you don't have to pass it to the getData method as an argument?

Another remark I have is simply this:
If a script contains only PHP code, then that script should not have a closing ?> tag. Get rid of it.