A PHP User System

This PHP User System was built with MySQLi and I also used Composer. I'm planning to improve this, and add more stuff and release it as a sort of a module for Composer.

User.php (Controller File in the src/Controller) folder.

request = $request;
        $this->response = $response;
        $this->userMapper = $mapper;
    }

    public function login()
    {
        $username = $this->request->getParameter('username');
        $password = $this->request->getParameter('password');

        if($this->userMapper->authenticateLoginDetails($username, $password)) {
            $this->userMapper->createSession($username);
            $this->userMapper->fetchUserDataAndPopulateUserObject($username);
            $this->response->redirect('/dashboard');
            return true;
        }
        $this->response->redirect('/');
        return false;
    }

    public function logout()
    {
        $this->userMapper->destroySession();
        $this->userMapper->cleanUserObject();
        $this->response->redirect('/');
    }
}

Module Files:

User.php (src/Users/User.php)

```
database = $database;
}

public function setID($value)
{
$this->id= $value;
}

public function getID()
{
return $this->id;
}

public function setUsername($value)
{
$this->username = $value;
}

public function getUsername()
{
return $this->username;
}

public function updateUsername($newUsername)
{
$this->setUsername($newUsername);

$query = $this->database->prepare("UPDATE users SET username=? WHERE id=?");
$query->bind_param("ss", $newUsername, $this->getID());
$query->execute();
$query->close();
}

public function setPassword($value)
{
$this->password = $value;
}

public function getPassword()
{
return $this->password;
}

public function updatePassword($newPassword)
{
$this->setPassword($newPassword);

• How may I make it more flexible?

Depend on abstractions not on concretions (SOLI D )

__construct(Request $request, Response $response, Mapper $mapper)

Each concrete (Request, Response, Mapper) could be replaced with a Interface making the code more flexible for changes in the dependency and not avoiding tight coupling.

• Is my code efficient? How may I make it more efficient?

When I read efficient, I understand that as fast, if that is the case, do not worry about it, that could be considered as premature optimization.
There might be some places where one might reconsider if the logic is really justified, readable and easy to refactor, again, it really has nothing to do with speed.

session_start();
    unset($_SESSION['username']);
    unset($_SESSION['logoutTime']);
    session_destroy();
    session_start();
    session_destroy();

Reconsider the logic in this code, take a extra look at session_destroy and the example on removing a session.

Also single $_SESSION = []; will remove any data.

• Do you have any recommendations for my code?

Access to the $_SESSION should be abstracted away into a class.

A couple of places in your code return a boolean, consider using no return value when none is needed and istead throw a Exception if something went wrong.

• Have I written SOLID code?

Single responsibility principle - There seems to be abit of trouble adhering to this principle especially in the Mapper, it acts both as a proxy to the user, wrapper for the session. It should take a map a user not depend on one via. the constructor.

Dependency inversion principle - Remove your hard dependencies from the constructors and add interfaces instead.

__construct(FooInterface $foo = null)
  {
    $this->foo = $foo ?: new MyDefaultFoo;
  }

Take care, as it is very hard to forfill these principles at all times, I would recommend to strive for them but accept it's not always what you end up with. Also there are other 3 and 5 letter acronyms also worth remembering and striving for. =]

• Do you see any flaws in my code? If so please point out.

At multiple places in the codebase do you run the session_start() this function should only be run once, and only if the session is not running, calling isUserOnline() and destroySession() would result in a PHP Notice: A session had already been started - ignoring session_start() in php shell code on line 1, other combinations of session related method would also end up in this. Consider reading this question about checking if the session is running

Consider the difference between private and protected. Also keep in mind why you close of access to modification, if you make both a setValue($x) { $this->x = $x } and getValue($x) { return $this->x; } having a private $x; has no use and might as well be a public $x.

- Is the system secure? If not, how may I make it more secure?

I am not a security expert and will leave any real security advice to a expert any day of the week. If you need security review I would recommend a question with a really specific context. The only things I can take away from the code and give some comments one is

-
In any security/permission related context always use === instead of == regarding password_verify($rawPassword, $databasePassword) == true, but in this case it does not really matter, just do not make it a habit.

-
Watch out for while($query->fetch() only one user should be fetched and the username should be a unique value.

StackExchange Code Review Q#85854, answer score: 7