Login and registration pages for a PHP and PDO e-commerce project

Question

We are doing an e-commerce project in a PHP-MySQL combination. Here is the code with PDO we wrote for login and register. I've heard that writing less code is better, so how we can reduce the lines of code?

connection.php

<?php

// database Connection variables
define('HOST', 'localhost'); // Database host name ex. localhost
define('USER', 'root'); // Database user. ex. root ( if your on local server)
define('PASSWORD', ''); // user password  (if password is not set for user then keep it empty )
define('DATABASE', 'databasename'); // Database Database name

function DB()
{
    try {
        $db = new PDO('mysql:host='.HOST.';dbname='.DATABASE.'', USER, PASSWORD);
        return $db;
    } catch (PDOException $e) {
        return "Error!: " . $e->getMessage();
        die();
    }
}
?>

login

<div class="row">
        <div class="col-md-5 well">
            <h4>Login</h4>
            <?php
            if ($login_error_message != "") {
                echo '<div class="alert alert-danger"><strong>Error: </strong> ' . $login_error_message . '</div>';
            }
            ?>
            <form action="index.php" method="post">
                <div class="form-group">
                    <label for="">Username/Email</label>
                    <input type="text" name="username" class="form-control"/>
                </div>
                <div class="form-group">
                    <label for="">Password</label>
                    <input type="password" name="password" class="form-control"/>
                </div>
                <div class="form-group">
                    <input type="submit" name="btnLogin" class="btn btn-primary" value="Login"/>
                </div>
            </form>
        </div>
    </div>

register

<div class="row">
        <div class="col-md-5 well">
            <h4>Register</h4>
            <?php
            if ($register_error_message != "") {
                echo '<div class="alert alert-danger"><strong>Error: </strong> ' . $register_error_message . '</div>';
            }
            ?>
            <form action="index.php" method="post">
                <div class="form-group">
                    <label for="">Name</label>
                    <input type="text" name="name" class="form-control"/>
                </div>
                <div class="form-group">
                    <label for="">Email</label>
                    <input type="email" name="email" class="form-control"/>
                </div>
                <div class="form-group">
                    <label for="">Username</label>
                    <input type="text" name="username" class="form-control"/>
                </div>
                <div class="form-group">
                    <label for="">Password</label>
                    <input type="password" name="password" class="form-control"/>
                </div>
                <div class="form-group">
                    <input type="submit" name="btnRegister" class="btn btn-primary" value="Register"/>
                </div>
            </form>
        </div>
    </div>

Library file to handle request of index.php file

<?php

// Start Session
session_start();

// Database connection
require __DIR__ . '/database.php';
$db = DB();

// Application library ( with DemoLib class )
require __DIR__ . '/lib/library.php';
$app = new DemoLib();

$login_error_message = '';
$register_error_message = '';

// check Login request
if (!empty($_POST['btnLogin'])) {

    $username = trim($_POST['username']);
    $password = trim($_POST['password']);

    if ($username == "") {
        $login_error_message = 'Username field is required!';
    } else if ($password == "") {
        $login_error_message = 'Password field is required!';
    } else {
        $user_id = $app->Login($username, $password); // check user login
        if($user_id > 0)
        {
            $_SESSION['user_id'] = $user_id; // Set Session
            header("Location: profile.php"); // Redirect user to the profile.php
        }
        else
        {
            $login_error_message = 'Invalid login details!';
        }
    }
}

// check Register request
if (!empty($_POST['btnRegister'])) {
    if ($_POST['name'] == "") {
        $register_error_message = 'Name field is required!';
    } else if ($_POST['email'] == "") {
        $register_error_message = 'Email field is required!';
    } else if ($_POST['username'] == "") {
        $register_error_message = 'Username field is required!';
    } else if ($_POST['password'] == "") {
        $register_error_message = 'Password field is required!';
    } else if (!filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) {
        $register_error_message = 'Invalid email address!';
    } else if ($app->isEmail($_POST['email'])) {
        $register_error_message = 'Email is already in use!';
    } else if ($app->isUsername($_POST['username'])) {
        $register_error_message = 'Username is already in use!';
    } else {
        $user_id = $app->Register($_POST['name'], $_POST['email'], $_POST['username'], $_POST['password']);
        // set session and redirect user to the profile page
        $_SESSION['user_id'] = $user_id;
        header("Location: profile.php");
    }
}
?>
<!doctype html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Home</title>
    <!-- Latest compiled and minified CSS -->
    <link rel="stylesheet" href="css/bootstrap.min.css">
</head>
<body>

<div class="container">
    <div class="row">
        <div class="col-md-12">
            <h2>
                PHP Login Registration System with PDO Connection using SHA-256 Cryptographic Hash Algorithm to store Password
            </h2>
        </div>
    </div>
    <div class="form-group">
        Note: .
    </div>
    <div class="row">
        <div class="col-md-5 well">
            <h4>Register</h4>
            <?php
            if ($register_error_message != "") {
                echo '<div class="alert alert-danger"><strong>Error: </strong> ' . $register_error_message . '</div>';
            }
            ?>
            <form action="index.php" method="post">
                <div class="form-group">
                    <label for="">Name</label>
                    <input type="text" name="name" class="form-control"/>
                </div>
                <div class="form-group">
                    <label for="">Email</label>
                    <input type="email" name="email" class="form-control"/>
                </div>
                <div class="form-group">
                    <label for="">Username</label>
                    <input type="text" name="username" class="form-control"/>
                </div>
                <div class="form-group">
                    <label for="">Password</label>
                    <input type="password" name="password" class="form-control"/>
                </div>
                <div class="form-group">
                    <input type="submit" name="btnRegister" class="btn btn-primary" value="Register"/>
                </div>
            </form>
        </div>
        <div class="col-md-2"></div>
        <div class="col-md-5 well">
            <h4>Login</h4>
            <?php
            if ($login_error_message != "") {
                echo '<div class="alert alert-danger"><strong>Error: </strong> ' . $login_error_message . '</div>';
            }
            ?>
            <form action="index.php" method="post">
                <div class="form-group">
                    <label for="">Username/Email</label>
                    <input type="text" name="username" class="form-control"/>
                </div>
                <div class="form-group">
                    <label for="">Password</label>
                    <input type="password" name="password" class="form-control"/>
                </div>
                <div class="form-group">
                    <input type="submit" name="btnLogin" class="btn btn-primary" value="Login"/>
                </div>
            </form>
        </div>
    </div>
</div>

</body>
</html>

index.php

<?php


// Start Session
session_start();

// Database connection
require __DIR__ . '/database.php';
$db = DB();

// Application library ( with DemoLib class )
require __DIR__ . '/lib/library.php';
$app = new DemoLib();

$login_error_message = '';
$register_error_message = '';

// check Login request
if (!empty($_POST['btnLogin'])) {

    $username = trim($_POST['username']);
    $password = trim($_POST['password']);

    if ($username == "") {
        $login_error_message = 'Username field is required!';
    } else if ($password == "") {
        $login_error_message = 'Password field is required!';
    } else {
        $user_id = $app->Login($username, $password); // check user login
        if($user_id > 0)
        {
            $_SESSION['user_id'] = $user_id; // Set Session
            header("Location: profile.php"); // Redirect user to the profile.php
        }
        else
        {
            $login_error_message = 'Invalid login details!';
        }
    }
}

// check Register request
if (!empty($_POST['btnRegister'])) {
    if ($_POST['name'] == "") {
        $register_error_message = 'Name field is required!';
    } else if ($_POST['email'] == "") {
        $register_error_message = 'Email field is required!';
    } else if ($_POST['username'] == "") {
        $register_error_message = 'Username field is required!';
    } else if ($_POST['password'] == "") {
        $register_error_message = 'Password field is required!';
    } else if (!filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) {
        $register_error_message = 'Invalid email address!';
    } else if ($app->isEmail($_POST['email'])) {
        $register_error_message = 'Email is already in use!';
    } else if ($app->isUsername($_POST['username'])) {
        $register_error_message = 'Username is already in use!';
    } else {
        $user_id = $app->Register($_POST['name'], $_POST['email'], $_POST['username'], $_POST['password']);
        // set session and redirect user to the profile page
        $_SESSION['user_id'] = $user_id;
        header("Location: profile.php");
    }
}
?>
<!doctype html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Home</title>
    <!-- Latest compiled and minified CSS -->
    <link rel="stylesheet" href="css/bootstrap.min.css">
</head>
<body>

<div class="container">
    <div class="row">
        <div class="col-md-12">
            <h2>
                link
            </h2>
        </div>
    </div>
    <div class="form-group">
        Note: welcome
    </div>
    <div class="row">
        <div class="col-md-5 well">
            <h4>Register</h4>
            <?php
            if ($register_error_message != "") {
                echo '<div class="alert alert-danger"><strong>Error: </strong> ' . $register_error_message . '</div>';
            }
            ?>
            <form action="index.php" method="post">
                <div class="form-group">
                    <label for="">Name</label>
                    <input type="text" name="name" class="form-control"/>
                </div>
                <div class="form-group">
                    <label for="">Email</label>
                    <input type="email" name="email" class="form-control"/>
                </div>
                <div class="form-group">
                    <label for="">Username</label>
                    <input type="text" name="username" class="form-control"/>
                </div>
                <div class="form-group">
                    <label for="">Password</label>
                    <input type="password" name="password" class="form-control"/>
                </div>
                <div class="form-group">
                    <input type="submit" name="btnRegister" class="btn btn-primary" value="Register"/>
                </div>
            </form>
        </div>
        <div class="col-md-2"></div>
        <div class="col-md-5 well">
            <h4>Login</h4>
            <?php
            if ($login_error_message != "") {
                echo '<div class="alert alert-danger"><strong>Error: </strong> ' . $login_error_message . '</div>';
            }
            ?>
            <form action="index.php" method="post">
                <div class="form-group">
                    <label for="">Username/Email</label>
                    <input type="text" name="username" class="form-control"/>
                </div>
                <div class="form-group">
                    <label for="">Password</label>
                    <input type="password" name="password" class="form-control"/>
                </div>
                <div class="form-group">
                    <input type="submit" name="btnLogin" class="btn btn-primary" value="Login"/>
                </div>
            </form>
        </div>
    </div>
</div>

</body>
</html>

myaccount

<?php
// Start Session
session_start();

// check user login
if(empty($_SESSION['user_id']))
{
    header("Location: index.php");
}

// Database connection
require __DIR__ . '/database.php';
$db = DB();

// Application library ( with DemoLib class )
require __DIR__ . '/lib/library.php';
$app = new DemoLib();

$user = $app->UserDetails($_SESSION['user_id']); // get user details

?>
<!doctype html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Profile</title>
    <!-- Latest compiled and minified CSS -->
    <link rel="stylesheet" href="css/bootstrap.min.css">
</head>
<body>
    <div class="container">
        <div class="well">
            <h2>
                Profile
            </h2>
            <h3>Hello <?php echo $user->name ?>,</h3>
            <p>
                Lorem ipsum dolor sit amet, consectetur adipisicing elit. Consectetur deserunt dolore fuga labore magni maxime, quaerat reiciendis tenetur? Accusantium blanditiis doloribus earum error inventore laudantium nesciunt quis reprehenderit ullam vel?
            </p>
            <a href="logout.php" class="btn btn-primary">Logout</a>
        </div>
    </div>
</body>
</html>

logout.php

<?php
/**
 * Tutorial: PHP Login Registration system
 *
 * Page : Logout
 */

// start session
session_start();

// Destroy user session
unset($_SESSION['user_id']);

// Redirect to index.php page
header("Location: index.php");
?>

podliy16 · Accepted Answer · 2016-10-04 08:57:27Z

3

if you really want reduce amount of lines -- delete your comments :)

// start session session_start();

I can understand that this is start session even without comment, and so on. You should comment some unclear part, some dirty hacks, etc, not every line.

Write comments for every line is anti-pattern, you shouldn't do this. Also less code doesn't mean "better code". I think deleting unnecessary comments is only one case, when this works.

And of course you can minimize your html to one line.

answered Oct 4, 2016 at 8:57

podliy16

1013 bronze badges

\$\begingroup\$ Thanks for your review, i will try that, what you say about code quality, security, performance ? \$\endgroup\$
– learning progrmming
Commented Oct 4, 2016 at 9:09
1

\$\begingroup\$ About code quality -- you messed up your representation layer and logic. You should try to implement some View system or take anyone already implemented. For example your account.php should be like: you're getting all needed data, then do something like $view = new View('account.php.html', ['name' => $user->name]); and that's all. You shouldn't have HTML markup and PHP logic in 1 file. Also, with this point you should use some layout, which contain basic HTML for all pages. For example you want to add new css style to your application. You need to change all files now, and this is very bad \$\endgroup\$
– podliy16
Commented Oct 4, 2016 at 10:12
\$\begingroup\$ do you suggest to use any frameworks for this instead of plain php code ? \$\endgroup\$
– learning progrmming
Commented Oct 4, 2016 at 10:18
\$\begingroup\$ Also I think you should wrap your database connection in singleton pattern. It'll prevent you from creating multiple connection in one request. Singleton quite easy to implement and database connection the best candidate for this pattern. About security -- many depends on your DemoLib class, how exactly you're doing SQL queries, which sanitize you're using. At this moment I see possible CSRF attack, because you're not using any token protection on auth. You can check owasp.org/index.php/Top_10_2013-Top_10 every point from Owasp top 10, usually it's enough. \$\endgroup\$
– podliy16
Commented Oct 4, 2016 at 10:18
\$\begingroup\$ @podliy16 you should add this as an answer. \$\endgroup\$
– topher
Commented Oct 4, 2016 at 10:18

| Show 5 more comments

topher · Accepted Answer · 2016-10-04 10:30:33Z

There isn't much we can do on security since that is in the DemoLib.php file that isn't included here. That being said:

1) Do not repeat yourself. The session_start() and inclusion of your db and lib files should be done in a separate file that is included once when required:

init.php

<?php
session_start();

// Database connection
require __DIR__ . '/database.php';
$db = DB();

// Application library ( with DemoLib class )
require __DIR__ . '/lib/library.php';

1) The DemoLib class has functions that begin with capital letters e.g. Register and others that begin with small letters e.g. isEmail. Follow the PHP Standards Recommendations or pick one style and stick to it.

3) The IsEmail and IsUsername functions are poorly named. They should instead be named something more descriptive e.g. isExistingEmail and isExistingUsername or emailExists and usernameExists.

4) Your checks for existing emails and usernames only need to be run if the email and username fields are set.

5) Your code stops the validation if an error is found. This is inefficient since you can run all the validations and check if an error has been found.

if (!empty($_POST['btnRegister'])) {
    $register_error_message = [];
    if ($_POST['name'] == "") {
        $register_error_message[] = 'Name field is required!';
    }
    if ($_POST['email'] == "") {
        $register_error_message[] = 'Email field is required!';
    }
    if ($_POST['username'] == "") {
        $register_error_message[] = 'Username field is required!';
    }
    if ($_POST['password'] == "") {
        $register_error_message[] = 'Password field is required!';
    }
    if (!filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) {
        $register_error_message[] = 'Invalid email address!';
    }
    if (trim($_POST['email']) && $app->emailExists($_POST['email'])) {
        $register_error_message[] = 'Email is already in use!';
    }
    if (trim($_POST['username']) != "" && $app->usernameExists($_POST['username'])) {
        $register_error_message[] = 'Username is already in use!';
    }
    if (!$register_error_message) {
        $user_id = $app->Register($_POST['name'], $_POST['email'], $_POST['username'], $_POST['password']);
        // set session and redirect user to the profile page
        $_SESSION['user_id'] = $user_id;
        header("Location: profile.php");
    }
}

You can then display the list of errors.

6) You are checking for empty strings but not for strings that only contain spaces. You are also not checking if the field actually exists in $_POST. Also there are multiple checks for this empty string. You can use a foreach loop.

7) The required and valid error messages for the email can be combined:

if (!empty($_POST['btnRegister'])) {
    $register_error_message = [];
    $required_fields = ["name", "username", "password"];
    foreach ($required_fields as $f) {
        if (!isset($_POST[$f]) || trim($_POST[$f]) != "") {
            $register_error_message[] = ucwords($f)." field is required!";
        }
    }
    if (!isset($_POST["email"]) || !filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) {
        $register_error_message[] = 'A valid email address is required!';
    }
    ...

Thanks, i will try your suggestions, can you give some feedback related to security & performance ? — learning progrmming, Commented Oct 4, 2016 at 10:39
we are doing live - e-commerce project, if we use say codeignitor to achieve login/signup module, for uploading products , orders can we use plain php code or we must use codeignitor framework only ? as both login/sign up customers are related to products & categories ? — learning progrmming, Commented Oct 4, 2016 at 11:42

Mike Brant · Accepted Answer · 2016-10-04 21:34:37Z

Your DB() function is really pretty meaningless, especially since it seem you are really only trying to set up the dependency once.

Why not just this line in you main execution path:

$pdo = new PDO('mysql:host='.HOST.';dbname='.DATABASE.'', USER, PASSWORD);

What value do you get from having this in a function? Right now the only thinkg your function adds is try-catch handling (something that can easily be done outside the context of of a function) and actually stops code execution if there is a failure (something you really should leave up to the caller to decide how to handle).

If you truly insist on having a function ere, at least name if something meaningful like instantiatePDO(), that clearly indicate what the function does.

Your DB constants are probably named too generically. Consider DB_HOST instead of just HOST and similar so that the "domain" of the constant is also clear.

Ideally these constant are moved out of class/function code and placed in application configuration somewhere where they are not hard-coded in your codebase.

You also should NEVER run an application using root mysql user. Build a user for your application with appropriate privileges. Your root MySQL user should NEVER be configured to used empty password.

Neither of your forms is secure, as they both have vulnerability to CSRF attacks. Typically one would add a token as hidden form field which matches token value in session. On subsequent POST/GET of form, this token should be compared against value in session to determine if this is a legitimate request from the end user who holds the session.

As pointed out in other answers, your data validation is a little odd. Consider using something like PHP's filter_input_array() which can give you more robust functionality in validating and sanitizing your input. There are also a number of PHP libraries out there that do this exact thing. Consider using one.

$user_id = $app->Register($_POST['name'], $_POST['email'], $_POST['username'], $_POST['password']);

Does your $app class know that it is handling user input? Do you need to sanitize before passing this information to this class method?

Consider not giving feedback to user as to whether an existing username or email address is present. You are giving a potential attacker valuable information here. Many consider it best practice to give more generic "unable to create user account"-type messaging for this reason.

I agree with comments on getting towards better application bootstrapping script. You don't want to keep repeating the basics of session start up, setting up dependencies, etc. on every single page. This would be a nightmare to maintain as size of application grows.

Similarly, I am not sure why you have some many pages where these forms are defined. Seems like there might be opportunity for reuse here.

// Destroy user session
unset($_SESSION['user_id']);

This is no sufficient to destroy a session (i.e. log a user out).

You need to do something like

// Unset session variables.
session_unset();

// Also delete the session cookie.
// Note: This will destroy the session, and not just the session data!
if (ini_get("session.use_cookies")) {
    $params = session_get_cookie_params();
    setcookie(session_name(), '', time() - 42000,
        $params["path"], $params["domain"],
        $params["secure"], $params["httponly"]
    );
}

// Finally, destroy the session.
session_destroy();

It is best practice to regenerate session id's when you log a user in. Consider using session_regenerate_id() to swap out session id numbers after successful login.

Stack Exchange Network

Login and registration pages for a PHP and PDO e-commerce project

3 Answers 3

Your Answer

Not the answer you're looking for? Browse other questions tagged
php
security
pdo
or ask your own question.

Hot Network Questions

Login and registration pages for a PHP and PDO e-commerce project

3 Answers 3

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged phpsecuritypdo or ask your own question.

Related

Hot Network Questions

Not the answer you're looking for? Browse other questions tagged
php
security
pdo
or ask your own question.