Basic object-oriented PHP login, logout, and registration scripts

Question

I am new to OOP in general and have recently started trying to develop object-oriented PHP code instead of relying on a primarily procedural style, as this is a requirement for my studies. One part of this involves websites that may handle data exchange and connect to MySQL databases with users (which should also be compatible with the MVC pattern).

After looking at a few tutorials online I have a few scripts for a basic system in which users may log in, log out, and register. It seems to function mostly as expected but I am concerned with standards and conventions and would just like to know if there is anything that stands out as not being efficient or secure.

---

Folder Structure

For this I have used the following folder structure:

• includes/
  • db.inc.php
  • User.class.php
• home.php
• index.php

---

Users Table

The users table was set up like so:

• user_id - INT(11), PRIMARY KEY, AUTO_INCREMENT
• user_name - VARCHAR(255), latin1_swedish_ci*
• user_email - VARCHAR(60), latin1_swedish_ci*
• user_password - VARCHAR(255), latin1_swedish_ci*

I did want to change the collation but was unsure on what the best option would be for these?

---

I feel like I could have split the code into more files but was unsure on whether this was completely necessary.

Could you please have a look over this and each script to see if there is anything I could do to improve? It is very important that I am able to protect against security exploits and implement databases correctly.

---

./includes/db.inc.php

<?php
// Begin/resume session
session_start();

// Include necessary file
include_once 'User.class.php';

// Define variable for custom error messages
$errors = [];

// Define key variables for connection
$db_host = 'localhost';
$db_user = 'root';
$db_pass = '';
$db_name = 'oop_login';

// Establish a new connection using PDO
try {
    $db_conn = new PDO("mysql:host={$db_host};dbname={$db_name}", $db_user, $db_pass);
    $db_conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
} catch (PDOException $e) {
    array_push($errors, $e->getMessage());
}

// Make use of database with users
$user = new User($db_conn);

---

./includes/User.class.php

<?php
class User
{
    // Refer to database connection
    private $db;

    // Instantiate object with database connection
    public function __construct($db_conn)
    {
        $this->db = $db_conn;
    }

    // Register new users
    public function register($user_name, $user_email, $user_password)
    {
        try {
            // Hash password
            $user_hashed_password = password_hash($user_password, PASSWORD_DEFAULT);

            // Define query to insert values into the users table
            $sql = "INSERT INTO users(user_name, user_email, user_password) VALUES(:user_name, :user_email, :user_password)";

            // Prepare the statement
            $query = $this->db->prepare($sql);

            // Bind parameters
            $query->bindParam(":user_name", $user_name);
            $query->bindParam(":user_email", $user_email);
            $query->bindParam(":user_password", $user_hashed_password);

            // Execute the query
            $query->execute();
        } catch (PDOException $e) {
            array_push($errors, $e->getMessage());
        }
    }

    // Log in registered users with either their username or email and their password
    public function login($user_name, $user_email, $user_password)
    {
        try {
            // Define query to insert values into the users table
            $sql = "SELECT * FROM users WHERE user_name=:user_name OR user_email=:user_email LIMIT 1";

            // Prepare the statement
            $query = $this->db->prepare($sql);

            // Bind parameters
            $query->bindParam(":user_name", $user_name);
            $query->bindParam(":user_email", $user_email);

            // Execute the query
            $query->execute();

            // Return row as an array indexed by both column name
            $returned_row = $query->fetch(PDO::FETCH_ASSOC);

            // Check if row is actually returned
            if ($query->rowCount() > 0) {
                // Verify hashed password against entered password
                if (password_verify($user_password, $returned_row['user_password'])) {
                    // Define session on successful login
                    $_SESSION['user_session'] = $returned_row['user_id'];
                    return true;
                } else {
                    // Define failure
                    return false;
                }
            }
        } catch (PDOException $e) {
            array_push($errors, $e->getMessage());
        }
    }

    // Check if the user is already logged in
    public function is_logged_in() {
        // Check if user session has been set
        if (isset($_SESSION['user_session'])) {
            return true;
        }
    }

    // Redirect user
    public function redirect($url) {
        header("Location: $url");
    }

    // Log out user
    public function log_out() {
        // Destroy and unset active session
        session_destroy();
        unset($_SESSION['user_session']);
        return true;
    }
}

---

./home.php

<?php
// Include necessary file
include_once './includes/db.inc.php';

// Check if user is not logged in
if (!$user->is_logged_in()) {
    $user->redirect('index.php');
}

try {
    // Define query to select values from the users table
    $sql = "SELECT * FROM users WHERE user_id=:user_id";

    // Prepare the statement
    $query = $db_conn->prepare($sql);

    // Bind the parameters
    $query->bindParam(':user_id', $_SESSION['user_session']);

    // Execute the query
    $query->execute();

    // Return row as an array indexed by both column name
    $returned_row = $query->fetch(PDO::FETCH_ASSOC);
} catch (PDOException $e) {
    array_push($errors, $e->getMessage());
}

if (isset($_GET['logout']) && ($_GET['logout'] == 'true')) {
    $user->log_out();
    $user->redirect('index.php');
}

?>

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <meta http-equiv="X-UA-Compatible" content="ie=edge">
    <title>OOP PHP - Home</title>
</head>
<body>
    <h1>Home</h1>

    <?php if (count($errors > 0)): ?>
    <p>Error(s):</p>
    <ul>
        <?php foreach ($errors as $error): ?>
            <li><?= $error ?></li>
        <?php endforeach ?>
    </ul>
    <?php endif ?>

    <p>Welcome, <?= $returned_row['user_name']; ?>. <a href="?logout=true">Log out</a></p>
</body>
</html>

---

./index.php

<?php
// Include necessary file
require_once('./includes/db.inc.php');

// Check if user is already logged in
if ($user->is_logged_in()) {
    // Redirect logged in user to their home page
    $user->redirect('home.php');
}

// Check if log-in form is submitted
if (isset($_POST['log_in'])) {
    // Retrieve form input
    $user_name = trim($_POST['user_name_email']);
    $user_email = trim($_POST['user_name_email']);
    $user_password = trim($_POST['user_password']);

    // Check for empty and invalid inputs
    if (empty($user_name) || empty($user_email)) {
        array_push($errors, "Please enter a valid username or e-mail address");
    } elseif (empty($user_password)) {
        array_push($errors, "Please enter a valid password.");
    } else {
        // Check if the user may be logged in
        if ($user->login($user_name, $user_email, $user_password)) {
            // Redirect if logged in successfully
            $user->redirect('home.php');
        } else {
            array_push($errors, "Incorrect log-in credentials.");
        }
    }
}

// Check if register form is submitted
if (isset($_POST['register'])) {
    // Retrieve form input
    $user_name = trim($_POST['user_name']);
    $user_email = trim($_POST['user_email']);
    $user_password = trim($_POST['user_password']);

    // Check for empty and invalid inputs
    if (empty($user_name)) {
        array_push($errors, "Please enter a valid username.");
    } elseif (empty($user_email)) {
        array_push($errors, "Please enter a valid e-mail address.");
    } elseif (empty($user_password)) {
        array_push($errors, "Please enter a valid password.");
    } elseif (!filter_var($user_email, FILTER_VALIDATE_EMAIL)) {
        array_push($errors, "Please enter a valid e-mail address.");
    } else {
        try {
            // Define query to select matching values
            $sql = "SELECT user_name, user_email FROM users WHERE user_name=:user_name OR user_email=:user_email";

            // Prepare the statement
            $query = $db_conn->prepare($sql);

            // Bind parameters
            $query->bindParam(':user_name', $user_name);
            $query->bindParam(':user_email', $user_email);

            // Execute the query
            $query->execute();

            // Return clashes row as an array indexed by both column name
            $returned_clashes_row = $query->fetch(PDO::FETCH_ASSOC);

            // Check for usernames or e-mail addresses that have already been used
            if ($returned_clashes_row['user_name'] == $user_name) {
                array_push($errors, "That username is taken. Please choose something different.");
            } elseif ($returned_clashes_row['user_email'] == $user_email) {
                array_push($errors, "That e-mail address is taken. Please choose something different.");
            } else {
                // Check if the user may be registered
                if ($user->register($user_name, $user_email, $user_password)) {
                    echo "Registered";
                }
            }
        } catch (PDOException $e) {
            array_push($errors, $e->getMessage());
        }
    }
}
?>

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <meta http-equiv="X-UA-Compatible" content="ie=edge">
    <title>OOP PHP - Login and Register</title>
</head>
<body>
    <h1>Welcome</h1>

    <?php if (count($errors > 0)): ?>
    <p>Error(s):</p>
    <ul>
        <?php foreach ($errors as $error): ?>
            <li><?= $error ?></li>
        <?php endforeach ?>
    </ul>
    <?php endif ?>

    <!-- Log in -->
    <h2>Log in</h2>
    <form action="index.php" method="POST">
        <label for="user_name_email">Username or E-mail Address:</label>
        <input type="text" name="user_name_email" id="user_name_email" required>

        <label for="user_password_log_in">Password:</label>
        <input type="password" name="user_password" id="user_password_log_in" required>

        <input type="submit" name="log_in" value="Log in">
    </form>

    <!-- Register -->
    <h2>Register</h2>
    <form action="index.php" method="POST">
        <label for="user_name">Username:</label>
        <input type="text" name="user_name" id="user_name" required>

        <label for="user_email">E-mail Address:</label>
        <input type="email" name="user_email" id="user_email" required>

        <label for="user_password">Password:</label>
        <input type="password" name="user_password" id="user_password" required>

        <input type="submit" name="register" value="Register">
    </form>
</body>
</html>

Answer 1

This response is far from exhausing, but I hope it contains a few good hints.

Files

When dealing with OOP you'll quickly learn that it is good practice to put one class in one file and then autoload these files. It is still acceptable to include files manually in small projects, as long as you do this in a sensible manner. Your file structure looks like this:

/home.php
/index.php
   |
   \---> /includes/db.inc.php
             |
             \---> /includes/user.class.php

Which looks weird. db.inc.php includes much more than just database related stuff, it starts the session and contains the user class. That is confusing. A better structure would be:

/home.php
/index.php
   |
   \---> /includes/start.inc
             |
             \---> /includes/db.inc
             \---> /classes/user.inc

Where start.inc start the session, database, and initializes the user. This could look something like this:

<?php

// begin or resume session
session_start();

// database access parameters
$db_access = ['host'     => 'localhost',
              'username' => 'root',
              'password' => '',
              'database' => 'oop_login'];

// connect to database
require('db.inc');

// add user class
require('../classes/user.inc');

// make use of database with users
$user = new User($database);

It's an include file where you 'start' everything you need. It's not perfect, but better than what you had.

note 1: Make sure that files in the includes and classes directory cannot be accessed by normal visitors of your website, even if they give just a blank page. You don't want their hands on it.

note 2: Files that you include don't need the .php extension, so 'db.inc' will work fine.

Variable Names

You are consistently using under_score notation for your variables. You also don't use too many abbreviations. Both greatly help readability. I do notice some difficulty in getting the semantics of the names correct and consistent. Weird examples are:

$this->db = $db_conn; // is it a database or a database connection?

$query = $this->db->prepare($sql); // preparing a statement results in a query?

$returned_row = $query->fetch(PDO::FETCH_ASSOC); // will there always be a row?!

Only one class? Or not?

In reality you've only made one class, called 'user'. I expect it to deal with everything to do with the user. To my horror however, you access the users table in 'home.php' and in 'index.php'. I must stress that in OOP: The responsibility of dealing with the users table should be completely confined to the user class. That's reason for making classes in the first place: Isolate functionality in manageable chunks. Another reason for classes is to abstract functionalily for you so you don't have to deal with the nastly details outside the class itself.

What clearly doesn't belong in the user class is redirect($url). This has nothing to do with an user.

Security

You've paid attention here and you use prepared statements and password hashing. Good.

Error handling

I don't like your error handing. Yes, you use exceptions, that's good, but a global array with all errors? That's not very OOP.

Conclusion

There are some other minor things in your code I could comment on, but the main issue with your code is that it was clearly designed from a procedural mindset. Not strange, given where you come from. However, instead of tackling a rather difficult user-login-logout system, why not choose something relatively easy, like the tower of Hanoi. Try to split the problem into discernible parts, like pegs, disks and moves, make classes for those. Use classes for everything, just to feel what that's like. Learn to use inheritance and chaining, important properties of classes.