5
\$\begingroup\$

This is a simple User class that I'm using throughout the application for anything related to the users. To tell if a user is logged in or not, I check the presence of the userID in the $_SESSION. When I'm logging in the user, I simply set the $_SESSION["userID"] equal to the ID of the user.

class User{

protected $pdo;

public function __construct(PDO $pdo)
{
    $this->pdo = $pdo;
}

public function getID()
{
    if(!$this->isConnected()) return 0;
    return $_SESSION["userID"];
}

public function isConnected()
{
    return !empty($_SESSION["userID"]);
}

public function login($username, $password)
{
    $q1 = $this->pdo->prepare("SELECT * FROM users WHERE username = ?");
    $q1->execute([$username]);
    $user = $q1->fetch();

    if(empty($user)) return false;
    if(!password_verify($password, $user->password)) return false;

    $_SESSION["userID"] = $user->id;
    $_SESSION["username"] = $user->username;
    return true;
}   
}

I'm putting everything in a class just so that it's organized and easily accessible, but I'm pretty sure I'm not doing this properly. I presume that the correct way to handle this is to use properties for the class (example: $isConnected, $id) and change them when the user logs in/logs out.

However, I'm not sure how I would do that and avoid overwriting the user object every time the script loads (page refresh), so at the moment I'm doing it like that.

It works fine, but I really feel like I'm missing the point of OOP here. I'm a beginner when it comes to OOP with PHP, so please don't be too harsh! I'd really like some input from experienced developers.

\$\endgroup\$
1
\$\begingroup\$

If your class takes care of the authentication of your application don't call it User but Auth or Authentication.

You shouldn't "putting everything in a class". Wrapping your code in a class is not OOP. See SOLID and start with the Single responsibility. Every class should have one single responsibility. Your class takes care to recover users data and handles authentication on your application. Too much responsibility for one class. Separate the user handling from the database query.

$_SESSION is an hard dependency. $_SESSION is global, and global is evil. If at some point you choose to store session data in a different storage you will need to change the implementation of the class. If you write orthogonal code you will enables to changes isolated areas of the code without influence other areas. You can use abstractions such as those provided by packages like Symfony2, HttpFoundation.

Although there are few who worry about it.

\$\endgroup\$
  • \$\begingroup\$ So then I should split my code into different classes, have one ` Auth ` where I handle the connection to the application - with the code to handle the login, rememberMe functions and things like that - and then a ` User ` class that regroups all the attributes of the user, and that provides access to the database to get the informations I need. Does that sound about right, or am I still missing something? In any cases thanks for the reply, and for the userful links, I'll have a look at that. \$\endgroup\$ – Drown May 28 '15 at 13:57
1
\$\begingroup\$

To expand on Leggendario's answer:

The User class appears to have the following responsibilities:

  • Authenticating a user
  • Extracting user data from persistent storage
  • Testing the session to see if a User has logged in
  • Populating the session with user data after login

This is where some of the other common patterns come into play:

  • Service objects: Authenticating a user and dealing with the session
  • Data Access Layer (DAL) or Repository Pattern: CRUD operations on persistent storage
  • Domain Model: A model object representing something from persistent storage

You could split your solution up into these classes:

  1. User: This is the Domain Model representing a row from the users table in the database

  2. UserRepository: This is the class responsible for CRUD operations on the users table

  3. UserAuthenticationService: This is the class responsible for authenticating a User in the database and retrieving the user from the session.

First, the Domain Model: User

class User
{
    private $username;
    private $id;
    // More private fields for columns in the users table

    public User($username, $id) {
        $this->username = $username;
        $this->id = $id;
    }

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

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

    // More getters and setters as needed
}

Next is the UserRepository and DatabaseMapper classes:

class UserRepository
{
    private $pdo;
    private $map;

    public UserRepository($pdo, $map = null) {
        $this->pdo = $pdo;
        $this->map = $map || new DatabaseMapper();
    }

    public function findByUsername($username) {
        $query = $this->pdo->prepare("SELECT * FROM users WHERE username = ?");
        $query->execute([$username]);
        $data = $query->fetch();

        if (empty($data)) {
            return null;
        }

        return $this->map->toUser($data);
    }
}

class DatabaseMapper
{
    public function toUser($data) {
        $user = new User($data->username, $data->id]);

        // Map other columns to fields using setters

        return $user;
    }
}

Lastly, the UserAuthenticationService:

class AuthenticationException extends Exception
{
}

class UserAuthenticationService
{
    private $session;
    private $repository;

    public UserAuthenticationService(UserRepository $repository, $session) {
        $this->session = $session;
        $this->repository = $repository;
    }

    public function isConnected() {
        return isset($this->session['user']);
    }

    public function authenticate($username, $password) {
        $user = $this->repository->findByUsername($username);

        if (!isset($user)) {
            throw new AuthenticationException("User not found");
        }

        if (!password_verify($password, $user->getPassword())) {
            throw new AuthenticationException("Passwords do not match");
        }

        $this->session['user'] = $user;

        return $user;
    }
}

If authentication fails, a custom exception class AuthenticationException gets thrown. Now the code to use these classes to log a user in:

$repository = new UserRepository(new PDO(...));
$authenticationService = new UserAuthenticationService($repository, $_SESSION);
$user = null;

try {
    $user = $authenticationService->authenticate($_POST['username'], $_POST['password']);
} catch (AuthenticationException $ex) {
    echo 'Username or password is incorrect';
    $user = null;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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