Flashback PHP - A Habbo CMS written in vanilla PHP

[Koala]

Hello Devbest community,
I am here today to publish a development thread for my (hopefully last) Habbo related project. Flashback PHP is a Habbo Retro content-management system written in vanilla PHP. This means;

• No bulky template engines
• No frameworks like Laravel, Symphony etc..

Current progress

• Core - DONE
• User class - Currently working on, register / login done so far

Upcoming features

• Simple template handler (Most likely search -> replace -> render)
• Awesome visual look..

Security measures

• PDO, prepared statements
• Output escaping
• Hashed passwords
• more?

Github
(not up yet, started this project 15 minutes ago. currently sitting in train)

Snippets

User.php, userLogin method

    //existing user login validation
    public function userLogin($username, $password) {

        $this->username = $username;
        $this->password = $password;
        $this->query = 'SELECT * FROM users WHERE username=:username LIMIT 1';
        
        $stmt = $this->connection->prepare($this->query);
        $stmt->bindParam(':username', $this->username, PDO::PARAM_STR);
        $stmt->execute();
        
        $this->result = $stmt->fetch(PDO::FETCH_ASSOC);

        if($stmt->rowCount() > 0) {
            if(password_verify($this->password, $this->result['password'])) {
                return true;
            } else {
                return false;
            }
        }
    }

Post automatically merged: Nov 12, 2020

Oh yeah, and just a friendly reminder - I do not have too much experience in PHP, Python is my main skill set. Any tips / critique is more than welcome. End of the day, we are all learning new staff every day. Thank you.

 

[Markshall]

Good luck with this.
Is this more of an learning-curve project to build experience in PHP as you go along? If so, since you’re asking for tips, here’s a quick one for you:

password_verify returns a Boolean value, and so does the method you’ve posted. So instead of doing this:

if($stmt->rowCount() > 0) {
  if(password_verify($this->password, $this->result['password'])) {
    return true;
  } else {
    return false;
  }
}

You can just do this:

if ($stmt->rowCount() > 0) {
  return password_verify($this->password, $this->result['password']);
}

Or even simpler:

return $stmt->rowCount() && password_verify($this->password, $this->result['password']);

 

[Koala]

Good luck with this.
Is this more of an learning-curve project to build experience in PHP as you go along? If so, since you’re asking for tips, here’s a quick one for you:

password_verify returns a Boolean value, and so does the method you’ve posted. So instead of doing this:

if($stmt->rowCount() > 0) {
  if(password_verify($this->password, $this->result['password'])) {
    return true;
  } else {
    return false;
  }
}

You can just do this:

if ($stmt->rowCount() > 0) {
  return password_verify($this->password, $this->result['password']);
}

Or even simpler:

return $stmt->rowCount() && password_verify($this->password, $this->result['password']);

Yes, exactly. I plan to learn on the fly, lol.
I appreciate your feedback and guidance. Currently I am using userLogin method as follows:
login.php

<?php
    $user = new User($pdo);
    if($user->userLogin($username, $password)) {
        //user login was successful
    } else {
        //it was not
    }

So my question is, that since userLogin() returns a boolean true / false, it is completely fine to handle logic like this with if/else?

also, I am passing the $pdo as constructor parameter to my classes that need it, since I really dont need any custom functionalities with PDO, so creating a custom wrapper for dbh would be kinda useless, right?

 

[Markshall]

Yes, exactly. I plan to learn on the fly, lol.
I appreciate your feedback and guidance. Currently I am using userLogin method as follows:
login.php

<?php
    $user = new User($pdo);
    if($user->userLogin($username, $password)) {
        //user login was successful
    } else {
        //it was not
    }

So my question is, that since userLogin() returns a boolean true / false, it is completely fine to handle logic like this with if/else?

also, I am passing the $pdo as constructor parameter to my classes that need it, since I really dont need any custom functionalities with PDO, so creating a custom wrapper for dbh would be kinda useless, right?

Yep, there’s nothing wrong with doing an if/else statement in the manner you’re proposing to do it. That’s what they’re there for.

What I was referring to in my original post is how to structure the return statement of your userLogin method.

As for passing your PDO instance as a parameter to another object, that’s fine too. It’s a technique called dependency injection and is a common thing to do. What may be a good idea is to create a parameter which accepts an array of dependencies in case your app grows and you don’t end up having a method accepting 5 parameters all for different instances of other objects, instead it receives 1 parameter which is just an array of this different instances.

For example, instead of:

class User {
  public function __construct($db, $anotherClass, $anotherClassAgain) {
    $this->db = $db;
    $this->anotherClass = $anotherClass;
    $this->anotherClassAgain = $anotherClassAgain;
  }
}
$User = new User($db, $anotherClass, $anotherClassAgain);

You could just do:

class User {
  public function __construct($deps) {
    foreach ($deps as $name => $instance) {
      $this->$name = $instance;
    }
  }
}
$User = new User([
  'db' => $db,
  'anotherClass' => $anotherClass,
  'anotherClassAgain' => $anotherClassAgain
]);

I hope what I’m saying is making sense lol. I’m typing this on my iPhone so it’s hard writing these code examples.

 

[Weasel]

Yep, there’s nothing wrong with doing an if/else statement in the manner you’re proposing to do it. That’s what they’re there for.

What I was referring to in my original post is how to structure the return statement of your userLogin method.

As for passing your PDO instance as a parameter to another object, that’s fine too. It’s a technique called dependency injection and is a common thing to do. What may be a good idea is to create a parameter which accepts an array of dependencies in case your app grows and you don’t end up having a method accepting 5 parameters all for different instances of other objects, instead it receives 1 parameter which is just an array of this different instances.

For example, instead of:

class User {
  public function __construct($db, $anotherClass, $anotherClassAgain) {
    $this->db = $db;
    $this->anotherClass = $anotherClass;
    $this->anotherClassAgain = $anotherClassAgain;
  }
}
$User = new User($db, $anotherClass, $anotherClassAgain);

You could just do:

class User {
  public function __construct($deps) {
    foreach ($deps as $name => $instance) {
      $this->$name = $instance;
    }
  }
}
$User = new User([
  'db' => $db,
  'anotherClass' => $anotherClass,
  'anotherClassAgain' => $anotherClassAgain
]);

I hope what I’m saying is making sense lol. I’m typing this on my iPhone so it’s hard writing these code examples.

As for the second part, I highly advice against that. You want to know which parameters you pass to a class, so you can also set the expected type.

 

[Koala]

Yep, there’s nothing wrong with doing an if/else statement in the manner you’re proposing to do it. That’s what they’re there for.

What I was referring to in my original post is how to structure the return statement of your userLogin method.

As for passing your PDO instance as a parameter to another object, that’s fine too. It’s a technique called dependency injection and is a common thing to do. What may be a good idea is to create a parameter which accepts an array of dependencies in case your app grows and you don’t end up having a method accepting 5 parameters all for different instances of other objects, instead it receives 1 parameter which is just an array of this different instances.

For example, instead of:

class User {
  public function __construct($db, $anotherClass, $anotherClassAgain) {
    $this->db = $db;
    $this->anotherClass = $anotherClass;
    $this->anotherClassAgain = $anotherClassAgain;
  }
}
$User = new User($db, $anotherClass, $anotherClassAgain);

You could just do:

class User {
  public function __construct($deps) {
    foreach ($deps as $name => $instance) {
      $this->$name = $instance;
    }
  }
}
$User = new User([
  'db' => $db,
  'anotherClass' => $anotherClass,
  'anotherClassAgain' => $anotherClassAgain
]);

I hope what I’m saying is making sense lol. I’m typing this on my iPhone so it’s hard writing these code examples.

That might be a subject for another time. As for now, I only refactored some code inside my user class per your instructions. Thank you for the tips.

 

[Weasel]

That might be a subject for another time. As for now, I only refactored some code inside my user class per your instructions. Thank you for the tips.

If you put the code on GitHub, people can help you improve the code, which might help learning.

Also, is there a reason you're not using any framework or packages?

 

[Koala]

If you put the code on GitHub, people can help you improve the code, which might help learning.

Also, is there a reason you're not using any framework or packages?

I will put the code on GitHub as soon as I can manage some sort of frontend for this.
What comes to your question, yes and no. Mostly because of the learning curve, after all, I am at the moment only experimenting with PHP, coming from Python.
Other minor thing is that I want this to be plug and play solution, leeches have no clue how to use Composer.

 

[Raizer]

Lol Who is saying this? With a proper manual let people follow the guide properly. Cosmic has over 800 forks with people without any knowledge. They are able to run Cosmic without any problems

 

[JayC]

Lol Who is saying this? With a proper manual let people follow the guide properly. Cosmic has over 800 forks with people without any knowledge. They are able to run Cosmic without any problems

Yeah, exactly this. I had someone who wanted help setting up cosmic, and I had never used composer before either. Just followed the tutorial and a little googling for when the tutorial didn't explain why some of the steps were needed. Now that I have used composer, it is really nice being able to manage your packages for you.

 

[Weasel]

Also, using Laravel and/or packages doesn't mean they need to use composer. You can just create a release with the vendor included and provide a solution as-is with only changes to the config. Not optimal, but it's possible.

 

[Raizer]

^ indeed thats also possible. But ye, it costs some space if you include the vendors ofc. My prefer has use some mvc structure in your project. Goodluck!

 

[Koala]

Lol Who is saying this? With a proper manual let people follow the guide properly. Cosmic has over 800 forks with people without any knowledge. They are able to run Cosmic without any problems

In my opinion, there is a difference being able to read and actually being able to understand what you read. I think, it is not good from learning point of view that you don't know what you are doing, and why you are doing it. I am not arguing about people being able to manage dependencies if you hold their hand.

Yeah, exactly this. I had someone who wanted help setting up cosmic, and I had never used composer before either. Just followed the tutorial and a little googling for when the tutorial didn't explain why some of the steps were needed. Now that I have used composer, it is really nice being able to manage your packages for you.

Composer is awesome, no doubt about that.

Also, using Laravel and/or packages doesn't mean they need to use composer. You can just create a release with the vendor included and provide a solution as-is with only changes to the config. Not optimal, but it's possible.

As I stated before, I am doing this mainly from the learning part of view. I want to do as much as I can with only native PHP. I know, I could save countless hours using likes of Symphony + Twig, but I also want to understand what is going on, under the hood. For example, I love how Twig handles syntax and output escaping, but I want to try that myself. Call me stupid, call me what ever. I am here to learn.

It's all about preferences, and will to learn.

 

[Markshall]

As for the second part, I highly advice against that. You want to know which parameters you pass to a class, so you can also set the expected type.

I understand you. I just feel as the application grows and other parameters are required, it gets hard to keep track of which should be in which order. With an array of dependencies for example, you can pass them in any order. I’m not suggesting creating the constructor method and only have 1 parameter be the dependency array, I’m suggesting that if he needs to pass in 7 parameters, 4 of which being dependencies, then perhaps passing 3 parameters and the array of dependencies would be a better and tidier option.
I try to follow a convention of where there are more than 4 parameters required to a method, then passing via object/array is better and tidier.

$User->userLogin($username, $password, $hash, $redirectUrl, $DBclass, $anotherClass, $anotherClassAgain);

...vs...

$User->userLogin($username, $password, $hash, $redirectUrl, [
  'db' => $DBclass,
  'anotherClass' => $otherClass,
  'anotherClassAgain' => $anotherClassAgain
]);

That might be a subject for another time. As for now, I only refactored some code inside my user class per your instructions. Thank you for the tips.

No problem. I kind of went beyond the scope of what you were asking. Guess I just got carried away with what I was typing as I was going along lol.
Good luck with your project nonetheless. Would be cool to see it on GitHub either way.

 

[Weasel]

I understand you. I just feel as the application grows and other parameters are required, it gets hard to keep track of which should be in which order. With an array of dependencies for example, you can pass them in any order. I’m not suggesting creating the constructor method and only have 1 parameter be the dependency array, I’m suggesting that if he needs to pass in 7 parameters, 4 of which being dependencies, then perhaps passing 3 parameters and the array of dependencies would be a better and tidier option.
I try to follow a convention of where there are more than 4 parameters required to a method, then passing via object/array is better and tidier.

$User->userLogin($username, $password, $hash, $redirectUrl, $DBclass, $anotherClass, $anotherClassAgain);

...vs...

$User->userLogin($username, $password, $hash, $redirectUrl, [
  'db' => $DBclass,
  'anotherClass' => $otherClass,
  'anotherClassAgain' => $anotherClassAgain
]);

No problem. I kind of went beyond the scope of what you were asking. Guess I just got carried away with what I was typing as I was going along lol.
Good luck with your project nonetheless. Would be cool to see it on GitHub either way.

It can happen that you need to pass a lot of variables. Passing over classes through an array is still very bad practice. Especially because when you do so, you can't define the type. You're taking away a lot of strength of PHP by doing do. I highly, and I can't stress this enough, no matter what reason, do not advice to pass along data like that. Unless it's for example an array of strings that are all going to be used in the same way (for example, to sort a list or something)

 

[Markshall]

It can happen that you need to pass a lot of variables. Passing over classes through an array is still very bad practice. Especially because when you do so, you can't define the type. You're taking away a lot of strength of PHP by doing do. I highly, and I can't stress this enough, no matter what reason, do not advice to pass along data like that. Unless it's for example an array of strings that are all going to be used in the same way (for example, to sort a list or something)

I see. What is an example of your preferred method in this case? I know this is going off-topic of the main thread but still interesting!

 

[Weasel]

I see. What is an example of your preferred method in this case? I know this is going off-topic of the main thread but still interesting!

Well, sometimes it might just happen that a class requires a lot of parameters. It's not that bad. With Coding Standards like PSR, you can create code that's still very easy to read.

 

[Koala]

Done some work on this. School is taking way too much time atm. However, here are some updates. Feedback is more than welcome.

Spoiler: register.tpl

Decided to use bootstrap because I am terrible with CSS, well not terrible but I find it hard to be happy with what I do with it.

Created a simple 'template engine' from scratch.
I am more than happy to take feedback on this. Since I am unsure if using something like this is heavily exploitable.

<?php
//simple template engine for php 7.4
class Template {
    public $template_data = array();
    public $template;

    public function __construct($path = '') {
        if(!empty($path)) {
            if(file_exists($path)) {
                $this->template = file_get_contents($path);
            } else {
                echo '<b>Template Error: </b> File not found. If you see this message, please report it to the administrator.';
            }
        }
    }

    public function pass($search, $replace) {
        if(!empty($search)) {
            $this->template_data[strtoupper($search)] = htmlspecialchars($replace);
        }
    }

    public function render() {
        if(count($this->template_data) > 0) {
            foreach($this->template_data as $key => $tpl_data) {
                $this->template = str_replace('{'.$key.'}', $tpl_data, $this->template);
            }
        }
        echo $this->template;
    }
}

And here is how I am currently using it. I would be very, very happy if you guys with more experience with PHP could guide me towards right direction. I am sure that there are lot of bad practices in this file

<?php
//includes
require_once 'include/session.php';
require_once 'include/autoload.php';
require_once 'include/database.php';

//some logc
$message = "";
$user = new User($pdo);

if(isset($_POST["submit"])) {
    if(!empty($_POST["email"]) && !empty($_POST["username"]) && !empty($_POST["password"]) && !empty($_POST["verify"])) {
        //assign user input to variables
        $email = $_POST["email"];
        $username = $_POST["username"];
        $password = $_POST["password"];
        $verify = $_POST["verify"];
        //creates new user with username, password and email
        if($user->newUser($username, $password, $email)) {
            header("location: index.php");
        }
    } else {
        $message = "Whoops, something went wrong! Please check all of the fields, and try again.";
    }
}

//render template
$template = new Template(TEMPLATE_PATH.'/register.tpl.html');
$template->pass('register_message', $message);
$template->render();

Github still not up, I am super lazy.

 

[BIOS]

Done some work on this. School is taking way too much time atm. However, here are some updates. Feedback is more than welcome.

Spoiler: register.tpl

Decided to use bootstrap because I am terrible with CSS, well not terrible but I find it hard to be happy with what I do with it.

Created a simple 'template engine' from scratch.
I am more than happy to take feedback on this. Since I am unsure if using something like this is heavily exploitable.

<?php
//simple template engine for php 7.4
class Template {
    public $template_data = array();
    public $template;

    public function __construct($path = '') {
        if(!empty($path)) {
            if(file_exists($path)) {
                $this->template = file_get_contents($path);
            } else {
                echo '<b>Template Error: </b> File not found. If you see this message, please report it to the administrator.';
            }
        }
    }

    public function pass($search, $replace) {
        if(!empty($search)) {
            $this->template_data[strtoupper($search)] = htmlspecialchars($replace);
        }
    }

    public function render() {
        if(count($this->template_data) > 0) {
            foreach($this->template_data as $key => $tpl_data) {
                $this->template = str_replace('{'.$key.'}', $tpl_data, $this->template);
            }
        }
        echo $this->template;
    }
}

And here is how I am currently using it. I would be very, very happy if you guys with more experience with PHP could guide me towards right direction. I am sure that there are lot of bad practices in this file

<?php
//includes
require_once 'include/session.php';
require_once 'include/autoload.php';
require_once 'include/database.php';

//some logc
$message = "";
$user = new User($pdo);

if(isset($_POST["submit"])) {
    if(!empty($_POST["email"]) && !empty($_POST["username"]) && !empty($_POST["password"]) && !empty($_POST["verify"])) {
        //assign user input to variables
        $email = $_POST["email"];
        $username = $_POST["username"];
        $password = $_POST["password"];
        $verify = $_POST["verify"];
        //creates new user with username, password and email
        if($user->newUser($username, $password, $email)) {
            header("location: index.php");
        }
    } else {
        $message = "Whoops, something went wrong! Please check all of the fields, and try again.";
    }
}

//render template
$template = new Template(TEMPLATE_PATH.'/register.tpl.html');
$template->pass('register_message', $message);
$template->render();

Github still not up, I am super lazy.

I know this may be more of a learning experience project type of thing, but I would highly recommend using a pre-existing templating library. I quite like

You must be registered for see links

.

Some reasoning and other tips:

• Twig is pretty fast, flexible in the sense it can be extended, and most importantly it is secure (provides you with methods to
  You must be registered for see links
  output for different contexts, in-fact it actually default escapes output for the HTML context, so in 90% of cases you do not have to worry about XSS).
• Highly consider using something like composer. It will make your life a whole lot easier in the long run, and your code much cleaner. For example, rather than manually including each of your required files at the top, you could use an
  You must be registered for see links
  .
• Also look into the concept of
  You must be registered for see links
  . This allows you to essentially inject all of your services (User) into a container which you can use anywhere, rather than having to instantiate a new object of it everywhere you need to use it. That also makes unit testing much easier, should you choose to learn more about that (recommended, essentially confirms your code is behaving the way you expect it to).
• I noticed you called $user->newUser() without validating any of the user submitted info, perhaps you should add another method under the User class to validate each of the inputs are valid (type, length, and semantically correct - e.g. validate e-mail's using filter_var($email, FILTER_VALIDATE_EMAIL).
• After adding a new user ($user->newUser()), you sender a Location header to redirect. You should call exit/die() after redirections to make sure the rest of the script does not execute.
• Not required, but you might also want to research type hinting, here's a link:
  You must be registered for see links
  .
• Again, not required, but I'd recommend following what's sometimes referred to as short circuiting for cleaner code. e.g. your code

if(isset($_POST["submit"])) {
    if(!empty($_POST["email"]) && !empty($_POST["username"]) && !empty($_POST["password"]) && !empty($_POST["verify"])) {
//assign user input to variables
$email = $_POST["email"];
$username = $_POST["username"];
$password = $_POST["password"];
$verify = $_POST["verify"];
//creates new user with username, password and email
if($user->newUser($username, $password, $email)) {
header("location: index.php");
}
} else {
$message = "Whoops, something went wrong! Please check all of the fields, and try again.";
}
}

could be rewritten as:

if (isset($_POST["submit"])) {
    if (empty($_POST["email"]) || empty($_POST["username"]) || empty($_POST["password"]) || empty($_POST["verify"])) {
        $message = "Whoops, something went wrong! Please check all of the fields, and try again.";
        // return a response early here.
    }
    //assign user input to variables
    $email = $_POST["email"];
    $username = $_POST["username"];
    $password = $_POST["password"];
    $verify = $_POST["verify"];
    //creates new user with username, password and email
    if ($user->newUser($username, $password, $email)) {
        header("location: index.php");
    }
}

Hopefully some of this helped, and good luck

 

[Koala]

I know this may be more of a learning experience project type of thing, but I would highly recommend using a pre-existing templating library. I quite like

You must be registered for see links

.

Some reasoning and other tips:

• Twig is pretty fast, flexible in the sense it can be extended, and most importantly it is secure (provides you with methods to
  You must be registered for see links
  output for different contexts, in-fact it actually default escapes output for the HTML context, so in 90% of cases you do not have to worry about XSS).
• Highly consider using something like composer. It will make your life a whole lot easier in the long run, and your code much cleaner. For example, rather than manually including each of your required files at the top, you could use an
  You must be registered for see links
  .
• Also look into the concept of
  You must be registered for see links
  . This allows you to essentially inject all of your services (User) into a container which you can use anywhere, rather than having to instantiate a new object of it everywhere you need to use it. That also makes unit testing much easier, should you choose to learn more about that (recommended, essentially confirms your code is behaving the way you expect it to).
• I noticed you called $user->newUser() without validating any of the user submitted info, perhaps you should add another method under the User class to validate each of the inputs are valid (type, length, and semantically correct - e.g. validate e-mail's using filter_var($email, FILTER_VALIDATE_EMAIL).
• After adding a new user ($user->newUser()), you sender a Location header to redirect. You should call exit/die() after redirections to make sure the rest of the script does not execute.
• Not required, but you might also want to research type hinting, here's a link:
  You must be registered for see links
  .
• Again, not required, but I'd recommend following what's sometimes referred to as short circuiting for cleaner code. e.g. your code

if(isset($_POST["submit"])) {
    if(!empty($_POST["email"]) && !empty($_POST["username"]) && !empty($_POST["password"]) && !empty($_POST["verify"])) {
//assign user input to variables
$email = $_POST["email"];
$username = $_POST["username"];
$password = $_POST["password"];
$verify = $_POST["verify"];
//creates new user with username, password and email
if($user->newUser($username, $password, $email)) {
header("location: index.php");
}
} else {
$message = "Whoops, something went wrong! Please check all of the fields, and try again.";
}
}

could be rewritten as:

if (isset($_POST["submit"])) {
    if (empty($_POST["email"]) || empty($_POST["username"]) || empty($_POST["password"]) || empty($_POST["verify"])) {
        $message = "Whoops, something went wrong! Please check all of the fields, and try again.";
        // return a response early here.
    }
    //assign user input to variables
    $email = $_POST["email"];
    $username = $_POST["username"];
    $password = $_POST["password"];
    $verify = $_POST["verify"];
    //creates new user with username, password and email
    if ($user->newUser($username, $password, $email)) {
        header("location: index.php");
    }
}

Hopefully some of this helped, and good luck

Thank you alot! Probably the most detailed piece of information I've gotten here so far. Taking a look at some of the stuff. I am already somewhat familiar with Twig, but I just want to play around with my own "template engine".. How ever, my syntax is very much the same to the Twig syntax - so I can just switch to Twig without even modifying my .tpl files. Might be releasing this with Twig, or maybe not - you never know.