Подтвердить что ты не робот

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

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

спасибо ребятам

4b9b3361

Ответ 1

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

Однако это не относится к вашей реальной реализации, потому что 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';
}

Ответ 2

Я уже много говорил об этом в stackoverflow, но я думаю, что вы делаете неправильно, так это то, что вы снова пытаетесь создать систему входа (даже Jeff Atwood соглашается со мной на этом), который, вероятно, будет небезопасным. Просто чтобы назвать несколько вещей, которые могли бы пойти не так:

  • Вы не выполняете аутентификацию по безопасному соединению (https), что означает, что ваше имя пользователя/пароль можно было обмануть из провода.
  • Он может иметь отверстие XSS.
  • пароли не хранятся безопасно в базе данных из-за неправильного использования соли. Вы aren эксперт по безопасности, поэтому я не думаю, что вы все равно должны хранить такую ​​конфиденциальную информацию в своей базе данных!
  • Он имеет CSRF -hole.

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

  • openid = > Lightopenid - это очень простая библиотека для использования/интеграции. Даже stackoverflow/jeff atwood использует его, потому что он знает, что сложно правильно войти в систему входа. Даже если вы специалист по безопасности.
  • подключиться к Google.
  • facebook connect.
  • односторонний вход в твиттер.

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

Ответ 3

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

Спросите себя, почему вы назовете checkCredentials, а затем вызовите login, а затем, возможно, какой-нибудь другой метод. У вас есть веская причина для этого, это хороший дизайн, чего я хочу достичь с этим.

Вызов только login, и выполнение каждой операции входа в систему намного проще и элегантно. Давать ему другое имя, делая это гораздо понятнее, а также более ремонтопригодным.

Если вы спросите меня, я бы использовал конструктор.

Ответ 4

Ответ действительно зависит от других соображений архитектуры, таких как метод checkCredentials(), который должен быть доступен за пределами класса для других элементов системы. Если его единственная цель должна быть вызвана методом login(), рассмотрите объединение этих двух в один метод.

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