# Classes for user registration and authentication

<?php
namespace Users;

use mysqli;

class Users {
protected $db; public function __construct() {$this->db = new mysqli('localhost', 'root', 'php123', 'sms');
}
public function login($username,$password) {
$query =$this->db->prepare("SELECT username, password FROM users WHERE username=? AND password=? LIMIT 1");
$password = UserData::hash($password);
$query->bind_param('ss',$username, $password);$query->execute();
$query->bind_result($username, $password);$query->store_result();
if($query->num_rows ==1) { while($query->fetch()) {
$_SESSION['loggedin'] = 1; UserData::getAndSet($username);

}
} else {
return false;
}
$query->free_result();$query->close();
}
public function register($username,$password, $rank,$name) {
$errors = array();$query = $this->db->prepare("INSERT INTO users VALUES('',$username, $password,$rank, $name);");$password = UserData::hash($password);$query->bind_param('ssss', $username,$password, $rank,$name);

if(strlen($username) < 3 || strlen($username) > 15) {
$errors[] = "The length of the username can only range from 3 to 15 characters."; } if(strlen($password) < 3 || strlen($password) > 15) {$errors[] = "The length of the password can only range from 3 to 15 characters.";
}
if(strlen($name) < 3 || strlen($name) > 15) {
$errors[] = "The length of the name can only range from 3 to 15 characters."; }$query->execute();
$query->close(); } } class UserData { protected$db;

public $username; public$password;
public $rank; public$name;

public function __construct() {
$this->db = new mysqli('localhost', 'root', 'php123', 'sms'); } public function getAndSet($username) {
$query =$this->db->prepare("SELECT username, password, rank, name FROM users WHERE username=?");
$query->bind_param('s',$username);
$query->execute();$query->bind_result($username,$password, $rank,$name);
$query->store_result(); while($query->fetch()) {
$this->username =$username;
$this->password =$password;
$this->rank =$rank;
$this->name =$name;
}
$query->free_result();$query->close();
}
public function hash($string) {$fSalt = "{z6Q9&2eHn69,9/cXa=<,Nm[?^^Tw-";
$sSalt = ":L+;fwS<q%)3f?N_d)(Ta_*fN&ax}+";$sting = md5(md5($fSalt) . md5($string) . md5($sSalt)); for($i=0; $i < 3;$i++) {
$string = md5($string);
$string = sha1($string);
$string = bin2hex($string);
$string = md5($string);
}

return $string; } } class UserUtil { protected$db;
public function __construct() {
$this->db = new mysqli('localhost', 'root', 'php123', 'sms'); } }$Users    = new Users();
$UserData = new UserData();$UserUtil = new UserUtil();

?>


This is the beginning of my project, "Student Management System" and before I proceed I want this code to be rated, and how can I improve this code with better ways.

• Why are you starting a new database connection for each class? Pass in a database connection object to the constructor:

public function __construct(mysqli $dbConnection) {  This allows your class to work without worrying about actually doing the connection. The class's job is to handle users, not handle database connections! • You not only said LIMIT 1 in your query, but you also check that $query->num_rows is 1, and then you even loop over the results (even though there's only one). This sort of triple check is redundant and unreadable. You know there's either 1 or 0 results because you LIMITed the query!

• Your hashing algorithm is not secure. Use PHP 5.5's password_hash(), or if you don't use 5.5 (And you should!!!) use Password Compat, a library written by the same guy who wrote the password_hash() function for PHP's core.

• Do not limit your users' password length - Setting a minimum length is good. Setting a maximum length is a kitten killing crime. Don't.

• Your login() method returns false on error, but nothing on success. Either return true on success, or simply throw an exception on errors.

• Your UserData class is not a class, it's a namespace. You only use it for static functions (even though you don't declare them as static in your class!).

• What does your getAndSet method does? It's not clear. Use a proper name or add documentation.

• Your UserUtil class does absolutely nothing.

Generally, I'd say you can improve the structure of your code by having the following 3 classes:

• UserService - is responsible for registering, authenticating and verifying users using the following two objects. Basically higher level management.
• User - Represents a single user in the system. Has absolutely no database interaction whatsoever, and has no idea where it came from. It accepts parameters through the constructor and getters/setters:

class User {
public function __construct($id,$username, $hashedPassword,$name) {

• UserMapper - Which is responsible for mapping and/or creating user objects to/from the database. For example

class UserMapper {
public function fetch(User $user) { //$user object will already have the ID filled in. Fetch according to that and fill the same $user object. } }  Example of execution: class UserService { private$mapper;

public function __construct(UserMapper $mapper) { ... } public function register($username, $password,$name) {
$user = new User($username, $password,$name);
$user->validateRegistrationDetails(); //Throws ValidationException on validation error.$user->generateHash();
$this->mapper->save($user); //Save user to database;
}
}


Note that in the above, the User is unaware of the database, which means it's perfectly reusable across applications with different storage systems.

• Thank you brother. I will read everything and I will comment on what you have said. So your first recommendation is Dependency Injection? Whenever I try it, it doesn't work. Can you please help me with that? – Abandoned Account Jun 22 '14 at 10:16
• I understand about the fact that my hashing algorithm is not secure. I usually use PBKDF2 but this is not going to be a public project, but rather a private project as practise to learn new stuff. – Abandoned Account Jun 22 '14 at 10:18
• I've set a maximum length because, don't you think it will be a problem to the performance? (That was just a logic I applied there, and who will have a long password?) – Abandoned Account Jun 22 '14 at 10:19
• I don't understand about the static thing of the 'UserData Class'. Like C# do I have to make it static void? – Abandoned Account Jun 22 '14 at 10:21
• The method getAndSet does this: It gets the username and fetches all data of that user and sets all the variables with that data for easy access. – Abandoned Account Jun 22 '14 at 10:22
1. Database connections all over the place.
3. Instead of using a lot of classes in one file, make one class per file, then include them where needed.
4. Try to write code that conforms to loose-coupling.

The following code has not been tested but should give some insight into how to structure your code, more or less (just some structure to ponder about).

<?php

ini_set("display_error", "1");
error_reporting(E_ALL);

interface IConnectInfo {

const HOST = "localhost";
const UNAME = "root";
const DBNAME = "sms";
const PW = "php123";

function testConnection();
}

class ConSQL implements IConnectInfo {

// Passing values using scope resolution operator
private $server = IConnectInfo::HOST; private$currentDB = IConnectInfo::DBNAME;
private $user = IConnectInfo::UNAME; private$pass = IConnectInfo::PW;

public function testConnection() {

$hookup = new mysqli($this->server, $this->user,$this->pass, $this->currentDB); if (mysqli_connect_error()) { die("bad mojo"); } echo "You're hooked up Ace! <br>".$hookup->host_info;

$hookup->close(); } } class Users extends ConSQL { public function login($username, $password) {$query = $this->db->prepare("SELECT username, password FROM users WHERE username=? AND password=? LIMIT 1");$password = UserData::hash($password);$query->bind_param('ss', $username,$password);
$query->execute();$query->bind_result($username,$password);
$query->store_result(); if($query->num_rows ==1) {
while($query->fetch()) {$_SESSION['loggedin'] = 1;

UserData::getAndSet($username); header("Location: dashboard.php"); } } else { return false; }$query->free_result();
$query->close(); } public function register($username, $password,$rank, $name) {$errors = array();
$query =$this->db->prepare("INSERT INTO users VALUES('', $username,$password, $rank,$name);");

$password = UserData::hash($password);

$query->bind_param('ssss',$username, $password,$rank, $name); if(strlen($username) < 3 || strlen($username) > 15) {$errors[] = "The length of the username can only range from 3 to 15 characters.";
}
if(strlen($password) < 3 || strlen($password) > 15) {
$errors[] = "The length of the password can only range from 3 to 15 characters."; } if(strlen($name) < 3 || strlen($name) > 15) {$errors[] = "The length of the name can only range from 3 to 15 characters.";
}

$query->execute();$query->close();
}
}

class UserData extends ConSQL {

public $username; public$password;
public $rank; public$name;

public function getAndSet($username) {$query = $this->db->prepare("SELECT username, password, rank, name FROM users WHERE username=?");$query->bind_param('s', $username);$query->execute();
$query->bind_result($username, $password,$rank, $name);$query->store_result();
while($query->fetch()) {$this->username = $username;$this->password = $password;$this->rank     = $rank;$this->name     = $name; }$query->free_result();
$query->close(); } public function hash($string) {
$fSalt = "{z6Q9&2eHn69,9/cXa=<,Nm[?^^Tw-";$sSalt = ":L+;fwS<q%)3f?N_d)(Ta_*fN&ax}+";

$sting = md5(md5($fSalt) . md5($string) . md5($sSalt));

for($i=0;$i < 3; $i++) {$string = md5($string);$string = sha1($string);$string = bin2hex($string);$string = md5($string); } return$string;
}
}

$Users = new Users();$Users->testConnection();
$Users->login();$Users->register();

$UserData = new UserData();$UserData->testConnection();

$UserUtil = new UserUtil();$UserUtil->testConnection();

?>


Get this book, which gave me some valuable insights into thinking in an OOP way.

• This code is outdated Ive updated the code. – Abandoned Account Jul 10 '14 at 12:03