Registration form using procedural MySQLi

Question

I'm relatively new to coding with PHP and mySQLi. I want to learn more but I'm really limited in resources. Most of my research went into W3Schools, PHP.net, and of course whatever I could find on StackOverflow. Now I'm trying to learn with practical projects like this account registration form.

Really I just want feedback on what I could improve, things I may have done wrong, and just overall a bit of feedback.

Main Questions:

• Am I on the right track or do I need to go back and put a bit more research into what I'm doing. (if so, where did I go wrong?)

• Is there anything that can be optimized, as in any unnecessarily long or drawn out?

There are lots of simple examples out there, but I tried to combine what I could find with what I had learnt together and create a fairly secure and robust form. It all works the way I want it to, but I keep looking at the code and wondering... is this even right?

I realize those are pretty broad questions, but I would accept even a fairly generic answer. Even if it's not what I want to hear (like my code sucks).

Note: After pasting all the code, I found parts that got absolutely butchered by Code Reviews own indenting. I'm really sorry if it wasn't fixed, I tried to fix it all.

---

php/session.php

Some of these vars are not really relevant to the registration page, they can probably be removed or moved elsewhere. $date and $hideSelf are used in the registration form. Otherwise, not relevant here. I left it all in just in case.

To explain the others, my main menu is based on the current directory in $curDir , depending on its value different menu items echo the $activePage var ... Could probably accomplish the same with JS but I was learning PHP... so why not? At one time I was also echoing the $curPage var on every page just as the page title. I eventually took that out and just typed out the name for each page so its pretty much unused.

$dateTime is my long way of displaying the current date and time for the user (immediately visible in the main menu) while $dateModified... obviously is the date modified (at the bottom of the page). Only relevant in the fact that they are displayed on the same page but not that they are used in the registration code.

<?php
session_start();

    // Misc
date_default_timezone_set( "America/Edmonton" );
$activePage = " active";
$hideSelf    = " d-none";

    // Directories
$selfDir = $_SERVER[ 'PHP_SELF' ];
$curPage = ucfirst(basename($selfDir, ".php" ));
$curDir = dirname($selfDir);

    // Dates/Times
$date          = date('Y-m-j H:i:s');   
$dateTime      = date('M jS, Y - g:i A (T)');
$dateModified = date('M jS, Y', getlastmod());

php/config.php

<?php
// These are obviously changed for displaying purposes
define('DB_SERVER', 'localhost');
define('DB_USER', 'username');
define('DB_PASS', 'password');
define('DB_NAME', 'name');

$con = new mysqli(DB_SERVER, DB_USER, DB_PASS, DB_NAME);

// CHECK CONNECTION
if($con->connect_error){
    $dbStatus = "Error";
}else{
    $dbStatus = "Connected";
}

dashboard/register.php

<?php 
include '../php/session.php';

// REMOVE BAD CHARACTERS
function test_input($data){
    if(!empty($data) || $data !== null){
        $data = trim($data);
        $data = stripslashes($data);
        $data = htmlspecialchars($data);
        return $data;
    }else{
        $_SESSION['ERROR'] = "Empty Field";
        header("Location: /dashboard/register.php");
        exit();
    }
}
// USER REGISTER
if(isset($_POST['register'])){
    // -- Inputs -- //
    $useremail  = test_input($_POST['reg_email']);
    $username       = test_input($_POST['reg_username']);
    $userfirst  = test_input($_POST['reg_fname']);
    $userlast       = test_input($_POST['reg_lname']);
    $userphone  = test_input($_POST['reg_phone']);
    $usergender     = test_input($_POST['reg_gender']);
    $userpass       = test_input($_POST['reg_password']);
    $userpasscheck  = test_input($_POST['reg_password_confirm']);

    // Filter through input regex conditions
    // Input email //
    if(!filter_var($useremail, FILTER_VALIDATE_EMAIL)){
        $_SESSION['ERROR'] = "Invalid Email";
        header("Location: /dashboard/register.php");
        exit();
    // Input username   //
    }elseif(!preg_match("/^[\w\d]{3,16}$/", $username)){
        $_SESSION['ERROR'] = "Invalid Username";
        header("Location: /dashboard/register.php");
        exit();
    // Input first name //
    }elseif(!preg_match("/^[\w\d]{2,32}$/", $userfirst)){
        $_SESSION['ERROR'] = "Invalid First Name";
        header("Location: /dashboard/register.php");
        exit();
    // Input last name //
    }elseif(!preg_match("/^[\w\d]{2,32}$/", $userlast)){
        $_SESSION['ERROR'] = "Invalid Last Name";
        header("Location: /dashboard/register.php");
        exit();
    // Input gender //
    }elseif(!preg_match("/^[MF]{1}$/", $usergender)){
        $_SESSION['ERROR'] = "Invalid Gender?";
        header("Location: /dashboard/register.php");
        exit();
    // Input phone //
    }elseif(!preg_match("/^[+]{0,1}[0-9]{1}[-0-9]{13}$/", $userphone)){
        $_SESSION['ERROR'] = "Invalid Phone Number";;
        header("Location: /dashboard/register.php");
        exit();
    // Input password //
    }elseif($userpass !== $userpasscheck){
        $_SESSION['ERROR'] = "Passwords do not match";
        header("Location: /dashboard/register.php");
        exit();
    }elseif(!preg_match("/^[\w\d]{6,32}$/", $userpass)){
        $_SESSION['ERROR'] = "Invalid Password";
        header("Location: /dashboard/register.php");
        exit();
    }else{
        // DB DEFINITIONS
        include_once '../php/config.php';
        // Check user name and email //
        $sql = "SELECT user_name,user_email FROM _CPAN_users WHERE user_name = ? OR user_email = ?;";
        if($stmt = mysqli_prepare($con, $sql)){
            mysqli_stmt_bind_param($stmt, "ss", $username,$useremail);
          mysqli_stmt_execute($stmt);
            mysqli_stmt_store_result($stmt);
            if(mysqli_stmt_num_rows($stmt) === 0){
                mysqli_stmt_free_result($stmt);
                mysqli_stmt_close($stmt);
                // Encrypt Password
                $hash = password_hash($userpass, PASSWORD_DEFAULT);
                // Generate token
                $token = bin2hex(openssl_random_pseudo_bytes(8));
                // Add new user
                $sql = "INSERT INTO _CPAN_users(user_name,user_email,user_pass,user_registered,user_token) VALUES (?, ?, ?, ?, ?);";
                if($stmt = mysqli_prepare($con, $sql)){
                    mysqli_stmt_bind_param($stmt, "sssss",$username,$useremail,$hash,$date,$token);
                    mysqli_stmt_execute($stmt);
                    // Get key id of user for meta table
                    $usernum = mysqli_stmt_insert_id($stmt);
                    mysqli_stmt_close($stmt);
                    // Add user meta
                    $sql = "INSERT INTO _CPAN_usersmeta(meta_user,meta_fname,meta_lname,meta_phone,meta_gender) VALUES (?, ?, ?, ?, ?);";
                    if($stmt = mysqli_prepare($con, $sql)){
                        mysqli_stmt_bind_param($stmt, "issss",$usernum,$userfirst,$userlast,$userphone,$usergender);
                        mysqli_stmt_execute($stmt);
                        mysqli_stmt_close($stmt);
                        mysqli_close($con);
                        // Generate Confirmation Email
                        $headers    = "MIME-Version: 1.0"."\r\n";
                        $headers    .= "Content-type:text/html;charset=UTF-8"."\r\n";
                        $headers    .= "From: no-reply"."\r\n";
                        $subject  = "Account Verification";
                        $message    = "<strong>-- This is an automated message from: www.website.com.  If this message was not meant for you please disregard and delete this email. --</strong>
                        <p>Please use this link below to verify your account!</p>
                        <a style='display:block;width:50%;padding:1.25em;margin-left:auto;margin-right:auto;background:#104F69;border:0px;border-radius:0.25em;color:#fff;text-align:center;text-decoration:none;font-weight:bold;' href='https://www.website.com/dashboard/confirmation.php?auth_username=".$username."&auth_token=".$token."'>VERIFY</a>";
                        // Send Email
                        $sendmail = mail($useremail, $subject, $message, $headers);
                        if($sendmail === TRUE){
                            $_SESSION['ERROR'] = "Verification email has been sent!";
                            header("Location: /dashboard/register.php");
                            exit();
                        }else{
                            // Send Email - Error
                            $_SESSION['ERROR'] = "There was a problem with your email. Please contact an admin!";
                            header("Location: /dashboard/register.php");
                            exit();
                        }
                    }else{
                        // SQL - Error
                        $_SESSION['ERROR'] = "Error : prepare,insert,meta";
                        mysqli_close($con);
                        header("Location: /dashboard/register.php");
                        exit();
                }
                }else{
                    // SQL - Error
                    $_SESSION['ERROR'] = "Error : prepare,insert,user";
                    mysqli_close($con);
                    header("Location: /dashboard/register.php");
                    exit();
                }
            }else{
                // Username or Email taken - Error
                $_SESSION['ERROR'] = "Username or email already taken";
                mysqli_close($con);
                header("Location: /dashboard/register.php");
                exit();
            }
        }else{
            // SQL - Error
            $_SESSION['ERROR'] = "Error : prepare,select";
            header("Location: /dashboard/register.php");
            exit();
        }
    }
}
// END REGISTER
?>

dashboard/register.php (HTML - on same page)

I want to move the form action to a different page - Not sure why I haven't yet, it would make things so much cleaner. Really is it that big of a deal if it gets left at the top? Please don't scold me if it is... I honestly don't know any better... yet.

<!doctype html>
<html lang="en">
<head>
    <meta charset="utf-8">
    <meta name="Viewport" content="width=device-width, initial-scale=1">
    <meta name="Description" content="redacted">
    <meta name="Keywords" content="redacted">
    <title>redacted| Register</title>
    <link rel="icon" href="../imgs/logo_64.png">

    <link rel="stylesheet" href="../css/bootstrap.min.css">
    <link rel="stylesheet" href="../css/all.min.css">
    <link rel="stylesheet" href="../css/framework.css">
</head>

<body data-spy="scroll" data-target="#context-nav" data-offset="75">
    <?php include '../incs/template/header.php';?>
    <div class="container-fluid mt-4">
        <div class="row">
            <div class="col mt-4">
                <h1 class="display-4">Register</h1>
                <div class="alert alert-primary<?php if(!isset($_SESSION['ERROR'])){echo $hideSelf;}?>"><strong>
                    <?php 
                    if(isset($_SESSION['ERROR'])){
                        echo $_SESSION['ERROR'];
                        unset($_SESSION['ERROR']);
                    }
                    ?>
                    </strong></div>
                <?php include '../incs/template/register.php';?>
            </div>
        </div>
        <?php include '../incs/template/footer.php';?>
    </div>
    <?php include '../incs/modals.php';?>

    <script src="../js/jquery.min.js"></script>
    <script src="../js/bootstrap.min.js"></script>
    <script src="../js/framework.js"></script>
</body>
</html>

incs/template/register.php

<div class="border bg-light border-secondary rounded-lg d-block w-75 p-4 mx-auto">
    <form id="register_form" class="d-block w-50 mx-auto" method="post">
        <div class="form-group mt-4">
            <label for="reg_email"><strong>Email:</strong></label>
            <input type="text" class="form-control" id="reg_email" name="reg_email" maxlength="64">
        </div>
        <div class="form-group mt-4">
            <label for="reg_username"><strong>Desired Username:</strong></label>
            <input type="text" class="form-control" id="reg_username" name="reg_username" maxlength="16">
        </div>
        <div class="row">
            <div class="col-md-6">
                <div class="form-group mt-4">
                    <label for="reg_fname"><strong>First Name:</strong></label>
                    <input type="text" class="form-control" id="reg_fname" name="reg_fname" maxlength="32">
                </div>
            </div>
            <div class="col-md-6">
                <div class="form-group mt-4">
                    <label for="reg_lname"><strong>Last Name:</strong></label>
                    <input type="text" class="form-control" id="reg_lname" name="reg_lname"  maxlength="32">
                </div>
            </div>
        </div>
        <hr>
        <div class="form-group mt-4">
            <label for="reg_phone"><strong>Phone:</strong></label>
            <input type="text" class="form-control" id="reg_phone" name="reg_phone" maxlength="14">
        </div>
        <hr>
        <div class="form-check">
            <input type="radio" class="form-check-input" id="reg_gender_m" name="reg_gender" value="M">
            <label class="form-check-label" for="reg_gender-m"><strong>Male</strong></label>
        </div>
        <div class="form-check">
            <input type="radio" class="form-check-input" id="reg_gender_f" name="reg_gender" value="F">
            <label class="form-check-label" for="reg_gender-f"><strong>Female</strong></label>
        </div>
        <hr>
        <div class="row">
            <div class="col-md-6">
                <div class="form-group mt-4">
                    <label for="reg_password"><strong>Password:</strong></label>
                    <input type="password" class="form-control" id="reg_password" name="reg_password" maxlength="32">
                </div>
            </div>
            <div class="col-md-6">
                <div class="form-group mt-4">
                    <label for="login_password_confirm"><strong>Reenter Password:</strong></label>
                    <input type="password" class="form-control" id="reg_password_confirm" name="reg_password_confirm"  maxlength="32">
                </div>
            </div>
        </div>
        <button type="submit" name="register" class="btn btn-secondary btn-block w-75 mx-auto my-4">Register</button>
    </form>
</div>

Any feedback is appreciated as long it isn't condescending. Please be nice if my code sucks, I just want some constructive feedback. Just let me know if there's anything I missed including that seems relevant.

Answer 1

First off, this is a quite good a code for someone "relatively new to coding". This is definitely above average. There are no critical faults, just some misconceptions. Quite common misconceptions I would say, so let's sort them out.

The "BAD CHARACTERS" misconception.

This is a nasty one.
In reality, there is no such thing as "bad characters", let alone your routine has very little to do with them, rather spoiling the input data. The whole test_input function is a cargo cult code snippet that bad tutorials copy from each other.

See my other answer regarding a similar function.

On a side note, the if(!empty($data) || $data !== null) is a collection of misplaced and repeated operators alone.

• using empty() for a variable that is deliberately set is useless. you only use this function if there is a possibility for a tested variable to be not set. Which is not the case, you are defining it right in the function definition. so you can use just if(!$data) instead.
• the second condition is useless as well, because empty() (as well as !$data) would test for the null already
• checking for the emptiness before trim is a bit ahead of time. What if after trim() it will become an empty string?

so in the end you can safely just use trim() instead of test_input()

WET AKA "Write Everything Twice" code.

Well, actually you repeat the code to report an error not twice but a dozen times.

Every time you see a repetition, think of creating a function.

function error($message, $location) {
    $_SESSION['ERROR'] = $message;
    header("Location: $location");
    exit();
}

then you can make it in one line instead of three

error ("Invalid Email", "/dashboard/register.php");

That's just for sake of demonstration though as you won't likely be using this function at all, as we will see a bit later

The same goes for mysqli interaction. As you may noticed, it is quite laborious yet extremely repetitive. Why not to encapsulate all the repeated code into a function again? I have such a function of my own, a mysqli helper function. Just compare the amount of code

    $sql = "SELECT user_name,user_email FROM _CPAN_users WHERE user_name = ? OR user_email = ?;";
    $stmt = mysqli_prepare($con, $sql);
    mysqli_stmt_bind_param($stmt, "ss", $username,$useremail);
    mysqli_stmt_execute($stmt);
    mysqli_stmt_store_result($stmt);
    if(mysqli_stmt_num_rows($stmt) === 0){
        mysqli_stmt_free_result($stmt);
        mysqli_stmt_close($stmt);

vs.

    $sql = "SELECT 1 FROM _CPAN_users WHERE user_name = ? OR user_email = ?";
    $stmt = prepared_query($con, $sql, [$username,$useremail]);
    if(!$stmt->get_result()->fetch_row()){

The idea is to write only the meaningful code, encapsulating all the reprtitions into a function.

Torturous error reporting.

To be honest, the way this form is reporting errors is more like a torture, letting a user to fix only one error at a time. It's like cutting a dog's tail in chunks. Why not to verify all inputs at once and then give a user all error messages at once as well?

Instead of your current approach, better collect all errors into array, and then verify whether this array is empty or not. If not - then process the user data. If yes, then store it in a session and do your redirect.

$errors = [];
if(!filter_var($useremail, FILTER_VALIDATE_EMAIL)){
    $errors[] = "Invalid Email";
}
if(!preg_match("/^[\w\d]{3,16}$/", $username)){
    $errors[] =  = "Invalid Username";
}

and so on. and then

if ($errors) {
    $_SESSION['ERROR'] = $errors;
    header("Location: /dashboard/register.php");
    exit();
}

Program errors are not user errors.

Honestly, how do you think, what a site user is supposed to do when given an error message like "Error : prepare,select"? How do you suppose to get informed of this error (in order to be able to fix it)?

That's a completely different kind of errors that has nothing to do with a site user but belongs to a programmer only. And thus have to be dealt with using a completely different approach. You can check my article on the PHP error reporting basics.

In a nutshell, you don't check every operation's result manually but make PHP to raise errors by itself. Then handle them all in a single place.

The code

As a result you will have a pretty neat piece of code (as a bonus you will get rid of this disgusting "right shift" when your code eventually moves off screen due to all these conditions) like this:

if(isset($_POST['register']))
{
    $useremail  = trim($_POST['reg_email']);
    ...


    $errors = [];
    if(!filter_var($useremail, FILTER_VALIDATE_EMAIL)){
        $errors[] = "Invalid Email";
    }
    if(!preg_match("/^[\w\d]{3,16}$/", $username)){
        $errors[] =  = "Invalid Username";
    }

    ...


    $sql = "SELECT 1 FROM _CPAN_users WHERE user_name = ? OR user_email = ?";
    $stmt = prepared_query($con, $sql, [$username,$useremail]);
    if(!$stmt->get_result()->fetch_row()){
        $errors[] =  = "Username or email already taken";
    }

    if ($errors) {
        $_SESSION['ERROR'] = $errors;
        header("Location: /dashboard/register.php");
        exit();
    }

    $hash = password_hash($userpass, PASSWORD_DEFAULT);
    $token = bin2hex(openssl_random_pseudo_bytes(8));

    $sql = "INSERT INTO _CPAN_users(user_name,user_email,user_pass,user_registered,user_token) VALUES (?, ?, ?, ?, ?);";
    $stmt = prepared_query($con, $sql, [$username,$useremail,$hash,$date,$token]);
    $usernum = mysqli_stmt_insert_id($stmt);

    $sql = "INSERT INTO _CPAN_usersmeta(meta_user,meta_fname,meta_lname,meta_phone,meta_gender) VALUES (?, ?, ?, ?, ?);";
    $stmt = prepared_query($con, $sql, [$usernum,$userfirst,$userlast,$userphone,$usergender]);

    // email stuff goes on

}
// END REGISTER

Answer 2

YCS didn't leave too many stones unturned, so I'll just mention some regex refinements as garnish to his great review:

• [\w\d] is best written as \w. The digits are included in the "word" metacharacter range.

• {1} can safely be omitted, it is the default quantifier for whatever preceded it.

• [+] is more simply written as \+

• {0,1} is more simply written as ?

• [0-9] is more simply written as \d

---

Beyond the regex, I recommend:

• not declaring single-use variables like $sendmail; just check return value === true and move on.

• if you are going to use mysqli(), try the object-oriented syntax-- it's tidier / less verbous.