PHP OO Clase 'users'. ¿Es correcta?

[Eleazan]

Buenas,

He decidido, poco a poco, ir haciendo cositas en POO... asiq, como resultado de eso, y del uso de Slim, Paris, e Idiorm, me he creado una clase "Users" para manejar los usuarios... o almenos, lo básico (crear, loguear, autentificar).

Y la verdad, yo creo q está bien hecha (en cuanto a POO, sé que queda aún validación!)

Asiq, si no es mucha molestia, la pongo aqui, y si veis algo mal hecho en POO, decídmelo!

Código PHP:

  <?php
 
class Users extends Model {
    
    private $_islog = false;
    
    public function getUser($dni) {
        return $this->factory('Users')->where('dni', $dni)->find_one();
    }
    
    public function logout() {
        //Borramos las cookies
        setcookie('login','');
    setcookie('rol','');
        setcookie('user', '');
        //El mismo borrará los datos de la sesion al recargar la pag!
    
    //session_unset();
    }
    
    public function islog() {
        return $this->_islog;
    }
    
    //Método para loguear en el sistema
    public function login($dni, $pass) {
        $user = $this->getUser($dni);
        if(!$user) return false; //No existe el usuario!
        if(md5($pass) == $user->pass) {
            $cook = $user->dni;
            $cook.= $user->pass;
            $rol = $user->rol;
            $cookie=md5($cook.$_SERVER["REMOTE_ADDR"].$rol.'pAlABRAmagica');
            setcookie('login',$cookie);
            setcookie('user',$user->dni);
            setcookie('rol',$rol);
            
            return $this->getUser($dni);
        }
        else return false;
                
    }
    
    //Método para, con dos valores de las cookies, comprobar si es correcto!
    public function auth($dni, $login) {
        
        $userN = Model::factory('Users')->where_equal('dni', $dni)->find_one(); //->where_equal('pass', $pass)
        if( $userN ) { //Exists
            
            if(isset($_COOKIE['rol'])) $rol = $_COOKIE['rol'];
            else $rol = '';
            $cookie = md5($dni.$userN->pass.$_SERVER["REMOTE_ADDR"].$rol.'pAlABRAmagica');
            if($cookie == $login) {
                $this->cloneU($userN);
                $this->_islog = true;
                return true;
                
                }
            else {
                $this->logout();
                return false;
            }
        }
        else {
            $this->logout();
            return false;
        }
    }
    
    private function cloneU($u) {
        $this->dni = $u->dni;
        $this->id = $u->id;
        $this->pass = $u->pass;
        $this->control = $u->control;
        $this->rol = $u->rol;
        $this->email = $u->email;
        $this->apellidos = $u->apellidos;
        $this->nombre = $u->nombre;
        
    }
   
    public function register($nombre, $apellidos, $pass, $mail, $dni ) {
        
        //Se carga otra instancia, pq sino sobreescribiria la que está activa y logueada... y eso NO interesa :)
        $u = Model::factory('Users')->create();
        
        $u->nombre = $nombre;
        $u->apellidos = $apellidos;
        $u->pass = md5($pass);
        $u->email = $mail;
        $u->dni = $dni;
        $u->rol = 'user';
        
        
        return $u->save();
    }
    
}   

Un saludo, y gracias :)

[Webstudio]

Hola. como estás?
Si bien como concepto tu clase User está bien, me parece que una clase de Usuario que se encarga de la autenticación es como que está mezclando dos cosas en un solo objeto.
Dejame proponerte una versión alternativa, y vos me decís, dale?
Por un lado como dejaría yo la jerarquía de clases:

Código PHP:

<?php
 
/**
 * Tu clase "Usuario" no es más que un simple modelo que representa los valores
 * de un usuario dentro de tu sistema. Para el resto de las cosas, podés utilizar
 * un Mapper que te permita "persistir" Usuarios en donde vos quieras.
 */
class User
{
    protected $_isLoged   = false;
 
    protected $dni       = 0;
    protected $id        = 0;
    protected $pass      = '';
    protected $control   = '';
    protected $rol       = 'user';
    protected $email     = '';
    protected $apellidos = '';
    protected $nombre    = '';
 
    public function loadByArray(array $u)
    {
        $u = (object) $u;
 
        $this->dni = $u->dni;
        $this->id = $u->id;
        $this->pass = $u->pass;
        $this->control = $u->control;
        $this->rol = $u->rol;
        $this->email = $u->email;
        $this->apellidos = $u->apellidos;
        $this->nombre = $u->nombre;
 
        return $this;
    }
}
 
/**
 * El Mapper es el objeto encargado de guardar objectos User
 * donde mejor te venga en gana. Este en particular, lo guarda
 * en la base de datos utilizando un Model en particular,
 * pero podrías guardar usando un web-service o MemCache.
 */
class UserMapper extends Model
{
 
    static public function getByDni($dni)
    {
        /* Asumo que este find_one() devuelve un array */
        $u = $this->factory('Users')->where('dni', $dni)->find_one();
        $user = new User();
        return $user->loadByArray($u);
    }
 
    /**
     * Este método no es necesario, directamente podés utilizar la
     * instrucción "clone" de PHP, de esta manera:
     * $newUser = clone $oldUser;
     */
    public function cloneU($u) {
        $this->dni = $u->dni;
        $this->id = $u->id;
        $this->pass = $u->pass;
        $this->control = $u->control;
        $this->rol = $u->rol;
        $this->email = $u->email;
        $this->apellidos = $u->apellidos;
        $this->nombre = $u->nombre;
 
    }
 
    /**
     * Este método, en vez de recibir los valores sueltos,
     * debería recibir un objecto User ya cargado con sus
     * valores, listo para guardar
     */
    static public function save(User $user)
    {
        /* Si necesitás modificar algo antes de grabar, lo hacés acá */
        $user->cambiosNecesarios();
 
        /* Usas tu Model para guardarlo */
        Model::factory('Users')->save($user);
        return $user;
    }
 
}
 
/**
 * Finalmente, esta clase es la que se encarga de hacer el trabajo
 * sucio a la hora de validar usuarios dentro de tu sistema. De
 * esta manera, mantenés un objeto User lo más limpio posible, y
 * delegás las funcionalidades específicas, a Clases específicas.
 */
class Authentication
{
 
    static public function logout() {
        //Borramos las cookies
        setcookie('login','');
        setcookie('rol','');
        setcookie('user', '');
        //El mismo borrará los datos de la sesion al recargar la pag!
        //session_unset();
    }
 
    public function islog(User $user) {
        return $user->_isLoged;
    }
 
    //Método para loguear en el sistema
    public function auth($dni, $pass) {
        $user = UserMapper::getByDni($dni);
 
        if(!$user) return false; //No existe el usuario!
 
        if(md5($pass) == $user->pass) {
            $cookie = Authentication::_getCookieName($user);
            setcookie('login',$cookie);
            setcookie('user',$user->dni);
            setcookie('rol',$user->rol);
 
            return $user;
        }
 
        return false;
    }
 
    //Método para, con dos valores de las cookies, comprobar si es correcto!
    static public function authByCookie($dni, $login) {
 
        $userN = UserMapper::getByDni($dni);
 
        /* Mejor invertir la condición, para evitar If anidados */
        if( !$userN ) {
            Authentication::logout();
            return false;
        }
 
        $cookie = Authentication::_getCookieName($userN);
 
        if($cookie != $login) {
            $this->logout();
            return false;
        }
 
        return $userN->_islog = true;
    }
 
    /**
     * Mejor tener un método que te devuelva el cookie name encriptado
     * asi evitas tener en dos lugares tu palABRAmagica ;)
     */
    static protected function _getCookieName($user)
    {
        $name = $user->dni . $user->pass . $_SERVER['REMOTE_ADDR'];
        $name.= $user->rol . 'pAlABRAmagica';
        return md5($name);
    }
}
 

Y luego unos ejemplos de uso básicos, ya vos los podrás adaptar a tus casos de uso más personales :

Código PHP:

// Obtenemos un usuario ya existente
$user = UserMapper::getByDni('26965061');
 
// Lo modificamos y grabamos
$user->nombre = 'Pablo';
UserMapper::save($user);
 
/* Si tuvieramos en un array todos los datos de un usuarios
   y lo queremos guardar en la base de datos */
$user = new User;
UserMapper::save($user->loadByArray($datos));
 
/* Si tenés DNI y PASS de un usuario, y querés saber si existe */
if($user = Authentication::auth('26965061', 'pepelepu')) { // Si, va un solo igual
    var_dump($user);
}
 
/* Y si querés saber si el usuario que está logueado, es uno válido */
if($user = Authentication::authByCookie($_COOKIE['dni'], $_COOKIE['cookie']) {
    var_dump($user);
}
 

Te lo dejo para que lo revises y me digas que pensás. Seguramente tiene algún error de sintaxis, o quizás de objetos y todo, pero sucede que lo tipee mientras iba modificando. Cualquier cosa lo vamos mejorando de a poco.

Abrazos !