Register script security and optimization

Question

function RegisterN() {
if($_SERVER['REQUEST_METHOD'] == 'POST'){
    if (isset($_POST["password"],$_POST["password1"]) && $_POST["password"]!=$_POST["password1"]) { # Check if password and password1 are same
    echo "Your password confirmation is not equal to your password or it missing.";
        } else if(!$_POST["email"]){ # Check if email is present
        echo "Please enter your email."; 
            } else if (!$_POST["password"]){ # Check if password is present again
            echo "Please enter your password";
                } else if (!$_POST["password1"]){ # Check if password1 is present again
                echo "Please enter your second password";
    } else {
     if (strlen ($_POST["password"])>25 || strlen ($_POST["password"])<6) # check password length max 25, min 6
        {
          echo "Password must be between 6 and 25 characters";
        } else {
        $dbh = new Database(); # Open database
        $dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false); # Small SQL injection security 

            $stmt = $dbh->prepare('SELECT email FROM members WHERE email=?'); # Check if email exists
            $stmt->bindParam(1, $_POST['email'], PDO::PARAM_INT);
            $stmt->execute();
            $rows = $stmt->fetchAll(PDO::FETCH_ASSOC); 

            if( ! $rows)
            { # No email found - moving on - If all set - let's register :)
                $email = $_POST['email'];
                $password = $_POST['password'];

                $hash_cost_log2 = 8;
                $hash_portable = FALSE;

                require("includes/PasswordHash.php"); # Hash password lib

                $hasher = new PasswordHash($hash_cost_log2, $hash_portable); # Hashing the password
                $hash = $hasher->HashPassword($password);
                if (strlen($hash) < 20)
                    fail('Failed to hash new password');
                unset($hasher);

                    $STH =$dbh->prepare("INSERT INTO members (email,password) VALUES (:email, :password)"); # Registering new user
                    $STH->bindParam(':email', $email);
                    $STH->bindParam(':password', $hash);
                    $STH->execute();

    echo "Account created successfully.";
    header('Refresh: 2; url=index.php'); # Redirect to index.php in 2 seconds

        } else { # email found - closing database - register fail
            $dbh=null;
            echo('Email already exists.'); 
            }
            $dbh=null; # Closing database after register
            }
            }
} else { 
echo"Invalid Token."; # If user access the page without POST
} }

I want to know if I can make it more secure. Before I made the script, I checked other scripts to see how they work, and what security measures they use.

What the script does:

• Check if request comes from POST

• Check if all fields ware filled ( It's being checked by JS as well, but that can be "cracked" )

• Check if email already exists

• Hash password

• Finally register

In addition, I saw that other scripts are far smaller than mine. Does that happen because they use frameworks?

Update : OOP implemented ( http://pastebin.com/0DKVxrDK ) for check email and register

Answer 1

Here are my recommendation,

• you could make sure users password have a minimum length. 8 alphanumeric characters is ideal If a users password is 1 char, it can easily be broken no matter how good the salt is

• move you database connection and setup into a different class: In the database class, create two separate functions.

  • Does Email Exist and Return boolean
  • Create users returning a new user object

Database Connection Class uses the singleton design pattern. We only need 1 connection object to exist when our application is running. The singleton pattern is a design pattern that restricts the instantiation of a class to one object.

class Database
{
    private static $dbh = null;
    //TODO you should move this inside config file
    private $host = 'mysql:host=localhost;dbname=test';
    private $pwd  = '';
    private $user = '';


    public function __construct()
    {
        try
        {
            Database::$dbh = new PDO($this->host, $this->user, $this->pwd);
            Database::$dbh->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
        }
        catch (PDOException $e)
        {
            echo "Connection Error: " . $e->getMessage();
        }
    }

    public static function getInstance()
    {
        if (Database::$dbh === null) {
            new Database();
        }
        return Database::$dbh;
    }
}

//all of our sql code should be in a centralized location for easy updating
class SQLPreparedQuery {
    public static $EMAIL_EXIST = "SELECT email FROM user WHERE email=? LIMIT 1";
    public static $USER_ADD = "INSET INTO user (username, email, first, last) VALUES(?,?,?,?)";
}

---

require_once './Database.php';
require_once './SQLPreparedQuery.php';
class UserModel {
    public function doesEmailExist($email) {
        $doesEmailExist = false;
        try {
            $dbh = Database::getInstance();

            $stmt = $dbh->prepare(SQLPreparedQuery::$EMAIL_EXIST);
            $stmt->bindParam(1, $email, PDO::PARAM_INT);
            $stmt->execute();
            $result = $stmt->fetchAll(PDO::FETCH_ASSOC);
            $doesEmailExist = (empty($result) ? false : true);
        }
        catch (PDOException $ex) { 
            echo "An Error occured!" . $ex;
        }
        return $doesEmailExist;
    }

//USAGE
//This will return true if it exist and false if it doesn't

$userModel = new UserModel();
$status = $userModel->doesEmailExist('[email protected]');

---

CREATE TABLE `user` (
  `iduser` int(11) NOT NULL AUTO_INCREMENT,
  `first` varchar(45) DEFAULT NULL,
  `last` varchar(45) DEFAULT NULL,
  `email` varchar(45) NOT NULL,
  PRIMARY KEY (`iduser`),
  UNIQUE KEY `email_UNIQUE` (`email`)
) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=latin1$$

INSERT INTO `test`.`user` (`first`, `last`, `email`) VALUES ('james', 'brown', '[email protected]');

You should always add UNIQUE constraints to usernames and email address to ensure uniqueness in your database.

Answer 2

• Hash password

I would recommend reading my "Protip" about secure passwords: https://coderwall.com/p/nnyida - I don't know the class you are using, but mine is just few lines of self-explaining code, so you know what you use.

In addition, I saw that other scripts are far smaller than my. That happens because they use frameworks?

Frameworks simply make your code easier to write, read and use. They will give your code some structure, and share some best-practices, so you are not reinventing wheel.