регистрация)

Начал экспериментировать со строительными классами, и я начал с того, что я зарегистрировал свою регистрацию / регистрацию пользователя в одном классе. Хотел остановиться и попросить обратную связь, прежде чем зайти слишком далеко.

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']);

Если метод login вызывает метод _checkCredentials автоматически. Или он должен быть построен как:

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

Помимо этого, мне нравятся некоторые советы о том, как реструктурировать это, и, пожалуйста, укажите на все, что я делаю неправильно!

Спасибо, парни

Я думаю, что ваша основная идея заключалась в том, чтобы отделить обработку пользователя (сеанс) от запроса к базе данных , что, на мой взгляд, очень хорошо.

Однако это не так с вашей реальной реализацией, потому что 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; } } по 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.
Пароли не хранятся в базе данных из-за неправильного использования соли. Вы aren 'ta эксперт по безопасности, поэтому я не думаю, что вы даже должны хранить такую конфиденциальную информацию в своей базе данных!
Он имеет CSRF- отверстие.

Тогда есть досада, что нам еще предстоит создать другую учетную запись на вашем сервере. Вы можете и должны избегать этой проблемы, используя одну из свободно доступных альтернатив, которые были проверены экспертами на уязвимости безопасности:

openid => Lightopenid – очень простая библиотека для использования / интеграции. Даже stackoverflow / jeff atwood использует его, потому что он знает, что сложно правильно войти в систему входа. Даже если вы специалист по безопасности.
google friend connect.
facebook connect.
twitter single sign-in.

Поэтому убедитесь, что вы снова создаете другую систему входа и вместо этого используете, например, действительно простую библиотеку 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 (), рассмотрите объединение этих двух в один метод.

Еще одна рекомендация, которую я могу использовать, – использовать глаголы в методах именования, что означает, что «логин» может быть общим для понимания его эффектов с первого взгляда.