User registration and authentication in PHP and PDO

Please criticize as thoroughly as possible, even the smallest thing will be very useful for me.

I'm trying to create a safe and easy system to change for future projects. I am aware that it should have a lot more functionality (check email, access attempts, notification by email, etc.), but this is what I have at the moment. I am wondering if it is safe even now.

I want to continue working on it but I am not 100% sure if I'm on the right track or if I need to add or remove some code (I like it to be as short as possible).

login.sql

SET SQL_MODE = "NO_AUTO_VALUE_ON_ZERO";
SET time_zone = "+00:00";

-- Database: `login`

-- Table structure for table `users`
CREATE TABLE IF NOT EXISTS `users` (
  `user_id` int(11) NOT NULL AUTO_INCREMENT,
  `user_name` varchar(15) NOT NULL,
  `user_email` varchar(40) NOT NULL,
  `user_pass` varchar(255) NOT NULL,
  `joining_date` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
  PRIMARY KEY (`user_id`)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;

config.php

```
conn = null;
try {
$this->conn = new PDO("mysql:host=" . $this->host . ";dbname=" . $this->db_name, $this->username, $this->password);
$this->conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
catch(PDOException $exception) {
echo "Connection error: " . $exception->getMessage();
}
return $this->conn;
}
}

// Functions for managing users
class USER {
private $conn;
public function __construct() {
$database = new Database();
$db = $database->dbConnection();
$this->conn = $db;
}

public function runQuery($sql) {
$stmt = $this->conn->prepare($sql);
return $stmt;
}

public function register($uname,$umail,$upass) {
try {
$new_password = password_hash($upass, PASSWORD_DEFAULT);
$stmt = $this->conn->prepare("INSERT INTO users(user_name,user_email,user_pass) VALUES(:uname, :umail, :upass)");
$stmt->bindpar

Preamble

Writing a secure user management and authentication system is not an easy task. It has many pitfalls and you should follow a lot of guidelines and best practices along the way.

Security and your goal


I like it to be as short as possible

really don't fit together as you could throw things over board, that may have reduced your vulnerability.

As your example fulfills lots of functions, I'd like to focus on some parts in this answer:

Transforming the user input

The way you transform the user input has some flaws. During registration and login you manipulate the input like this:

$upass = strip_tags($_POST['txt_password']);

The question is: Why are you using strip_tags here? Of course you don't want to re-print user input with tags to avoid certain attacks.

But in terms of user experience consider the situation, which might be very confusing and frustrating:

• A user tries a username called smith.

• You strip tags and test it against your user table, where a user called smith is already present.

• You show the registration form again with the value smith and the error "username already taken".

Another example where you're actively changing the password:

• The users' password is <>h<>e<>l<>l<>o.

• The user will not know that you have altered the password.

• As long as you don't change your login/registration process this might work, but the user will not be able to login once your remove strip_tags for any reason.

• Also, and even more dangerous: The complex password <>h<>e<>l<>l<>o becomes only a hashed (and salted) version of hello, which might be very easy to crack.

So instead of transforming the user's input, show messages that help the user to insert valid values, like "The username can't contain the symbols <>".

To avoid problems with tags in those values, don't output raw data and print the username or email using htmlentities.

Throwing exceptions

During your registration you somewhere catch an exception:

catch(PDOException $e) {
    echo $e->getMessage();
}

In a live setting, you should never present exceptions to the user. If an attacker somehow manages to raise that exception, he will be presented with insides of your system and database layout, that only you should have. Instead of printing the exception's message, present the user a default error and log the message into a log file on your server.

Configuration

Currently you have your database settings stored in the Database class itself:

class Database {
    private $host = "localhost";
    private $db_name = "login";
    private $username = "root";
    private $password = "";
}

As your project and your class hierarchy will grow and when you want to use your system in different projects, this will become really hard to maintain.

It would be better to store the settings in some sort of configuration file and inject them into the necessary classes.

Imagine, later you want to send registration emails to the user and you need to configure that as well. You now can keep all configuration in one place, without altering your classes.

Finally I would recommend to take look at the configuration and user management/authentication components of other PHP frameworks, like Symfony.

StackExchange Code Review Q#140824, answer score: 5