Login system based off of another site

I have created a PHP login system based off of this site. My main concern is: Is it secure? I chose that because to me it looked secure, but a 2nd or 3rd opinion never hurts. The idea of the project is to have something I can just pull into a new project directory and start off from there depending on my needs so it so it does have a few unused files.

Authentification Class

```
login = $login;
$this->email = $email;
$this->pass = $pass;
//for when it is already known from a global
$this->user_id = $user_id;

//set user session
$this->sess_id = session_id();
if(!$this->sess_id) {
$this->sess_id = $PHPSESSID;
}
}

/**
*
* @return boolean checks if CheckCredentials are valid.
*/
public function login()
{
$this->sess_clear();
if($this->CheckCredentials()){

$this->sess_write();
$this->auth_user_update_date();
$this->auth_user_login_count();
return true;
}
return false;
}
/**
*
* @return boolean Logs the user out
*/
public function logout()
{
return $this->sess_delete();
}

/**
*
* @return array returns the user infos
* Array returns id, login, user_level, default_lang, email
*/
public function checkuser()
{
$this->sess_clear();

$id = $this->sess_read();
$this->user_id = $id ? $id : -1;

$this->auth_user_update_date();

return $this->get_user_infos();
}

/**
*
* @return boolean Gets whether user should login or if it is first admin setup
*/
public function shouldLogAdmin()
{
if($this->IsLoginAdmin())
{
return $this->HasPassword();
}
else
{
//if it is not an admin, return false
throw new Exception("Current user is not an admin", 1

Preamble

As a matter of security, I can safely say that you would fail a professional security audit in seconds. You should be including the hashing library as an external component. You also need to forget everything you know about querying databases and start again.

Also, look at Model-View-Controller. The fact you have a templating engine and separate classes for Database work implies that you are attempting MVC, but the implementation itself is nothing like MVC.
SQLQUERY

As for the code here itself, your SQLQUERY class leaves a lot to be desired.

First: the naming. SQLQUERY makes me instantly think that this is a subclass of PDOStatement. But it isn't. It's a wrapper around a PDO object.

Consider the following:

class DatabaseWrapper {

    private $db;
    const USERS_TABLE = "`users`";

    public function __construct(PDO $db) {
        $this->setInstance($db);
    }

    public function &getInstance() {
        return $this->db;
    }

    protected function setInstance(PDO $db) {
        $this->db = $db;
    }

}

• It type-hints the database object in the constructor for dependency injection.

• It allows you to get a direct reference to the PDO object that is injected.

• Likewise, it also allows to you set it later (and allows classes that extend it to do the same)

• The users table is a class constant.

As for your queries themselves...

You're going to have to never, under any circumstances, use string interpolation (which is not too much of an issue in your implementation, except for the fact that you are using your controller as a model and your model as an array...)

Remove the ->Param() method and instead use $this->db->prepare().

This means that:

protected function AuthUsersByEmail()
{
    return  "SELECT login, active FROM ".USERS_TABLE." WHERE email = ".$this->db->Param('a')." AND user_level < 100";
}

Would instead become:

protected function getUsersByEmail($email, $level = 100) {
    $sql = "SELECT login, active FROM ". static::USERS_TABLE . " WHERE email = ? AND user_level prepare($sql, [$email, $level]);
}

Where prepare is the following:

protected function prepare($sql, array $bindings = []) {
    try {
        $prepare = $this->db->prepare($sql);
        $prepare->execute($bindings);
        return $prepare->fetchAll();
    } catch (PDOException $e) {
        throw new RuntimeException("prepare failed: $sql, " . $e->getMessage());
    }
}

In short, you should make your SQLQUERY class into a model that fetches data for you.

Also something I've noticed is that your entire project will be generating notices like crazy because of poor PHP.

• parent::__construct() should be used to call a parent constructor. using SQLQUERY::__construct() implies that it is a static method, and PHP will complain.

• protected functions are not static! In your entire authentication class, you are simply trying to call protected (and hence internal) functions from the global scope as public static methods.

This system would need a fairly hefty redesign in order to be considered for use in production environments.

You need to split up your files, and huge files are pretty much unreadable. Look at composer for autoloading your classes.

Also, have you tried using the built-in password hashing functions that PHP provides? (password_hash, password_verify, password_needs_rehash). You should really stick with vetted implementations of cryptographic functions that have been extensively peer-reviewed.

I strongly, strongly recommend that you consult PSR0, PSR1 and PSR2.

StackExchange Code Review Q#43317, answer score: 13