Класс пользователя PHP (вход/выход/регистрация)
Начал экспериментировать с созданием классов, и я начал с преобразования моей регистрации пользователя/входа в один класс. Хотел остановиться и попросить обратной связи, прежде чем зайти слишком далеко.
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;
}
}
Один из вопросов, которые у меня есть, связанных с моим классом: должен ли я строить его так:
$User = new UserService();
$User->login($_POST['email'], $_POST['password']);
Где метод входа автоматически вызывает метод _checkCredentials. Или он должен быть построен следующим образом:
$User = new UserService();
$UserId = $User->checkCredentials($_POST['email'], $_POST['password']);
$User->login($UserId);
Кроме этого, мне очень нравятся некоторые советы о том, как реструктурировать это и, пожалуйста, укажите, что я делаю неправильно!
Спасибо, ребята
4 answers
Я думаю, что ваша основная идея состояла в том, чтобы отделить обработку пользователя (сеанс) от запроса базы данных , что, на мой взгляд, хорошо.
Однако это не относится к вашей фактической реализации, потому что login ускользает от данных, которые должны быть отправлены в базу данных, даже если остальная часть метода не имеет ничего общего с базами данных. Не говоря уже о том, что ваш запрос к базе данных зависит от глобального ресурса для работы. Пока я этим занимаюсь, я также предложу вам использовать PDO.
Кроме того, ваши свойства $_email и $_password находятся в частной области, но доступ к ним должен осуществляться защищенным методом. Это может вызвать проблемы. Свойства и метод должны иметь эквивалентную видимость.
Теперь я вижу, что ваш UserService требуется три вещи: обработчик базы данных, электронная почта и пароль. Было бы разумно поместить его в конструктор.
Вот как я бы это сделал:
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;
}
}
Затем используйте его как таковой:
$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';
}
Я уже много раз говорил об этом в stackoverflow, но я думаю, что вы делаете неправильно, что вы снова пытаетесь создать систему входа в систему (даже Джефф Этвуд согласен со мной в этом), которая, вероятно, будет небезопасной. Просто назову несколько вещей, которые могут пойти не так:
- Вы не выполняете аутентификацию по безопасному соединению (https), что означает, что ваше имя пользователя/пароль могут быть выведены из сети.
- Это могло бы иметь XSS-отверстие.
- Пароли не хранятся безопасно в базе данных из-за неправильного использования соли. Вы не эксперт по безопасности, поэтому я не думаю, что вам вообще следует хранить такую конфиденциальную информацию в своей базе данных!
- В нем есть CSRF-отверстие.
Тогда есть досада, что нам еще предстоит создать другую учетную запись на вашем сервере. Вы могли бы и должны избежать этой проблемы, используя одну из свободно доступных альтернатив, которые были протестированы для уязвимостей в системе безопасности экспертами:
- openid => Lightopenid - это действительно простая в использовании/интеграции библиотека. Даже stackoverflow/Джефф Этвуд использует его, потому что он знает, что трудно правильно получить систему входа в систему. Даже если вы являетесь экспертом по безопасности.
- подключение к другу Google.
- подключение к facebook.
- единый вход в Twitter.
Так что берегите себя, чтобы снова придумать другую систему входа в систему и вместо этого использовать, например, действительно простая библиотека lightopenid и позволяет пользователям входить в систему с помощью учетной записи Google. Приведенный ниже фрагмент кода - это единственный код, который вам нужен, чтобы заставить его работать:
<?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();
}
Это зависит от вашего дизайна и представления о более простом решении, а также от того, как вы хотите организовать свой код, сделать его проще, меньше и в то же время сделать его доступным для обслуживания.
Спросите себя, почему вы бы вызвали checkCredentials, а затем вызвали login, а затем, возможно, какой-то другой метод. У вас есть веская причина для этого, хороший ли это дизайн, чего я хочу добиться с помощью этого.
Вызов просто login и выполнение каждой операции входа в систему намного проще и элегантнее. Присвоение ему другого названия при этом гораздо более понятно, а также более удобно в обслуживании.
Если вы спросите меня, я бы использовал конструктор.
Ответ действительно зависит от других соображений архитектуры, например, должен ли метод checkCredentials() быть доступен за пределами области действия класса для других элементов системы. Если его единственная цель состоит в том, чтобы вызываться методом login(), подумайте о том, чтобы объединить их в один метод.
Еще одна рекомендация, которую я мог бы дать, - использовать глаголы в методах именования, что означает, что "вход" может быть слишком общим для понимания его последствий на первый взгляд.