usuarios - Clase de usuario PHP(inicio de sesión/cierre de sesión/registro)

validar tipo de usuario php

Question

Comencé a experimentar con la creación de clases, y comencé convirtiendo mi registro / inicio de sesión de usuario en una sola clase. Quería parar y pedir comentarios antes de llegar demasiado lejos.

class UserService
{
private $_email;
private $_password;
public function login($email, $password)
{
$this->_email = mysql_real_escape_string($email);
$this->_password = mysql_real_escape_string($password);
$user_id = $this->_checkCredentials();
if($user_id){
$_SESSION[''user_id''] = $user_id;
return $user_id;
}
return false;
}
protected function _checkCredentials()
{
$query = "SELECT *
FROM users
WHERE email = ''$this->_email''";
$result = mysql_query($query);
if(!empty($result)){
$user = mysql_fetch_assoc($result);
$submitted_pass = sha1($user[''salt''] . $this->_password);
if($submitted_pass == $user[''password'']){
return $user[''id''];
}
}
return false;
}
}

Una de las preguntas que tengo relacionadas con mi clase es: ¿debería construirla así:

$User = new UserService();
$User->login($_POST[''email''], $_POST[''password'']);

Donde el método de inicio de sesión llama al método _checkCredentials automáticamente. O debería ser construido como:

$User = new UserService();
$UserId = $User->checkCredentials($_POST[''email''], $_POST[''password'']);
$User->login($UserId);

Aparte de eso, me encantan algunos consejos sobre cómo reestructurar esto y, por favor, señale cualquier cosa que esté haciendo mal.

gracias chicos

Answer 1

Creo que su idea principal fue separar el manejo del usuario (sesión) de la consulta de la base de datos , lo cual, en mi opinión, es algo bueno.

Sin embargo, este no es el caso con su implementación real, porque el login escapa a los datos que se envían a la base de datos, incluso si el resto del método no tiene nada que ver con las bases de datos. No quiere decir que su consulta de base de datos depende de un recurso global para trabajar . Mientras estoy en ello, también sugeriré que use DOP.

Además, sus propiedades $_email y $_password están en el ámbito privado, pero se puede acceder a ellas mediante un método protegido. Esto puede causar problemas. Las propiedades y el método deben tener visibilidad equivalente .

Ahora, puedo ver que su servicio de UserService requiere tres cosas : un controlador de base de datos, un correo electrónico y una contraseña. Tendría sentido ponerlo en un constructor .

Así es como lo haría:

class UserService
{
protected $_email; // using protected so they can be accessed
protected $_password; // and overidden if necessary
protected $_db; // stores the database handler
protected $_user; // stores the user data
public function __construct(PDO $db, $email, $password)
{
$this->_db = $db;
$this->_email = $email;
$this->_password = $password;
}
public function login()
{
$user = $this->_checkCredentials();
if ($user) {
$this->_user = $user; // store it so it can be accessed later
$_SESSION[''user_id''] = $user[''id''];
return $user[''id''];
}
return false;
}
protected function _checkCredentials()
{
$stmt = $this->_db->prepare(''SELECT * FROM users WHERE email=?'');
$stmt->execute(array($this->email));
if ($stmt->rowCount() > 0) {
$user = $stmt->fetch(PDO::FETCH_ASSOC);
$submitted_pass = sha1($user[''salt''] . $this->_password);
if ($submitted_pass == $user[''password'']) {
return $user;
}
}
return false;
}
public function getUser()
{
return $this->_user;
}
}

Entonces utilízalo como tal:

$pdo = new PDO(''mysql:dbname=mydb'', ''myuser'', ''mypass'');
$userService = new UserService($pdo, $_POST[''email''], $_POST[''password'']);
if ($user_id = $userService->login()) {
echo ''Logged it as user id: ''.$user_id;
$userData = $userService->getUser();
// do stuff
} else {
echo ''Invalid login'';
}

Answer 2

Depende de su diseño y de la noción de una solución más simple, y de cómo desea organizar su código y hacerlo más simple, más pequeño y al mismo tiempo mantenerlo.

Pregúntese por qué llamaría a checkCredentials y luego a la llamada de login y tal vez a algún otro método. ¿Tiene una buena razón para hacerlo? ¿Es este un buen diseño? ¿Qué quiero lograr con esto?

Llamar solo al login y realizar cada inicio de sesión es mucho más simple y elegante. Darle otro nombre mientras lo hace es mucho más comprensible y también más fácil de mantener.

Si me preguntas, usaría un constructor.

Answer 3

He dicho esto mucho antes en pero lo que creo que estás haciendo mal es que nuevamente estás intentando crear un sistema de inicio de sesión (incluso Jeff Atwood agrees conmigo en esto) que probablemente no sea seguro. Solo para nombrar algunas cosas que podrían salir mal :

• No realiza la autenticación a través de una conexión segura (https), lo que significa que su nombre de usuario / contraseña podría ser detectado del cable.
• Podría tener un agujero XSS.
• Las contraseñas no se guardan de forma segura en la base de datos debido al uso incorrecto de Salt. No eres un experto en seguridad, así que no creo que debas almacenar información tan confidencial en tu base de datos.
• Tiene un agujero CSRF .

Luego está la molestia que aún tenemos de crear otra cuenta en su servidor. Podría y debería evitar esta molestia utilizando una de las alternativas de libre acceso que han sido probadas para detectar vulnerabilidades de seguridad por expertos:

• openid => Lightopenid es una biblioteca muy fácil de usar / integrar. Incluso / jeff atwood lo está utilizando porque sabe que es difícil obtener el sistema de inicio de sesión correctamente. Incluso si eres un experto en seguridad.
• Google Friend Connect.
• conexion a Facebook.
• inicio de sesión único en twitter.

Así que asegúrate de volver a diseñar otro sistema de inicio de sesión y, en su lugar, utiliza, por ejemplo, la biblioteca lightopenid realmente sencilla y permite a los usuarios iniciar sesión con su cuenta de Google. El siguiente fragmento de código es el único código que necesita para que funcione:

<?php
# Logging in with Google accounts requires setting special identity, so this example shows how to do it.
require ''openid.php'';
try {
$openid = new LightOpenID;
if(!$openid->mode) {
if(isset($_GET[''login''])) {
$openid->identity = ''https://www.google.com/accounts/o8/id'';
header(''Location: '' . $openid->authUrl());
}
?>
<form action="?login" method="post">
<button>Login with Google</button>
</form>
<?php
} elseif($openid->mode == ''cancel'') {
echo ''User has canceled authentication!'';
} else {
echo ''User '' . ($openid->validate() ? $openid->identity . '' has '' : ''has not '') . ''logged in.'';
}
} catch(ErrorException $e) {
echo $e->getMessage();
}

Answer 4

La respuesta realmente depende de otras consideraciones de arquitectura, como si el método checkCredentials () necesita estar disponible fuera del alcance de la clase para otros elementos del sistema. Si su único propósito es ser llamado por el método login (), considere combinar los dos en un solo método.

Una recomendación más que podría tener es usar verbos en los métodos de asignación de nombres, lo que significa que ''inicio de sesión'' puede ser demasiado general para comprender sus efectos a primera vista.