Klasa użytkownika PHP (Logowanie/wylogowanie/Rejestracja)

Zacząłem eksperymentować z budowaniem klas, a zacząłem od przekształcenia rejestracji/logowania użytkowników w jedną klasę. Chciałem się zatrzymać i poprosić o opinie, zanim zajdzie za daleko.

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

Jedno z pytań, które mam związane z moją klasą, to: czy powinienem budować to tak:

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

Gdzie metoda login wywołuje metodę _checkcredentials automatycznie. A może powinien być zbudowany jak:

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

Poza tym uwielbiam kilka wskazówek, jak zrestrukturyzować to i proszę Zwróć uwagę na wszystko, co robię źle!

Thanks guys

Author: Gumbo, 2011-01-16

4 answers

Myślę, że twoim głównym pomysłem było oddzielenie obsługi użytkownika (sesji) od zapytania bazy danych , co moim zdaniem jest dobrą rzeczą.

Nie ma to jednak miejsca w przypadku Twojej rzeczywistej implementacji, ponieważ login przetwarza dane, które mają być wysłane do bazy danych, nawet jeśli reszta metody nie ma nic wspólnego z bazami danych. Nie mówiąc, że Twoje zapytanie do bazy danych zależy od globalnego zasobu do pracy . Podczas gdy jestem przy tym, będę również sugerować, że używasz PDO.

Również twoje właściwości $_email i $_password znajdują się w zakresie prywatnym, ale mają być dostępne za pomocą metody chronionej. Może to powodować problemy. właściwości i metoda powinny mieć równoważną widoczność .

Teraz widzę, że twój UserService wymaga trzech rzeczy: obsługi bazy danych, adresu e-mail i hasła. Sensowne byłoby umieścić go w konstruktorze .

Oto jak bym to zrobił:

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

Następnie użyj go jako takiego:

$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';
}
 36
Author: netcoder,
Warning: date(): Invalid date.timezone value 'Europe/Kyiv', we selected the timezone 'UTC' for now. in /var/www/agent_stack/data/www/doraprojects.net/template/agent.layouts/content.php on line 54
2012-08-20 22:07:50

Mówiłem już o tym na stackoverflow wcześniej, ale myślę, że robisz źle, ponieważ znowu próbujesz stworzyć system logowania (nawet Jeff Atwood zgadza się ze mną w tej sprawie), co prawdopodobnie będzie niebezpieczne. Aby wymienić tylko kilka rzeczy, które mogą pójść źle :

  • nie wykonujesz uwierzytelniania przez bezpieczne połączenie (https), co oznacza, że Twoja nazwa użytkownika/hasło może być wąchane z przewodu.
  • może mieć XSS-dziura.
  • hasła nie są przechowywane bezpiecznie w bazie danych z powodu nieprawidłowego użycia soli. Jesteś nie jest ekspertem od bezpieczeństwa, więc i tak nie powinieneś przechowywać takich poufnych informacji w swojej bazie danych!
  • ma CSRF - dziura.

W takim razie jest irytacja, że nie mamy jeszcze innego konta na twoim serwerze. Możesz i powinieneś uniknąć tego kłopotów, korzystając z jednej z swobodnie dostępnych alternatyw, które zostały przetestowane w przypadku luk w zabezpieczeniach przez ekspertów:

  • openid = > Lightopenid jest naprawdę łatwą biblioteką w użyciu/integracji. Nawet stackoverflow / jeff atwood używa go, ponieważ wie, że trudno jest poprawnie uzyskać system logowania. Nawet jeśli jesteś ekspertem od bezpieczeństwa.
  • Google friend connect.
  • facebook connect.
  • pojedyncze logowanie na Twitterze.

Więc zabezpiecz się przed ponownym wymyśleniem innego systemu logowania i zamiast tego użyj na przykład naprawdę prostego biblioteka lightopenid i pozwolić użytkownikom zalogować się tam konto google. Poniższy fragment to jedyny kod, którego potrzebujesz, aby go uruchomić:

<?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();
}
 14
Author: Alfred,
Warning: date(): Invalid date.timezone value 'Europe/Kyiv', we selected the timezone 'UTC' for now. in /var/www/agent_stack/data/www/doraprojects.net/template/agent.layouts/content.php on line 54
2017-05-23 11:47:23

To zależy od Twojego projektu i koncepcji prostszego rozwiązania, a także od tego, jak chcesz uporządkować swój kod i zachować go prostszym, mniejszym i jednocześnie uczynić możliwym do utrzymania.

Zadaj sobie pytanie, dlaczego zadzwonisz checkCredentials, potem zadzwonisz login, a potem może jakąś inną metodę. Czy masz ku temu dobry powód, czy jest to dobry projekt, co chcę z tym osiągnąć.

Wywołanie po prostu login i wykonywanie każdej operacji logowania jest znacznie prostsze i eleganckie. Dawanie jest to inna nazwa, która jest znacznie bardziej zrozumiała i łatwiejsza do utrzymania.

Jeśli O mnie chodzi, użyłbym konstruktora.
 1
Author: Secko,
Warning: date(): Invalid date.timezone value 'Europe/Kyiv', we selected the timezone 'UTC' for now. in /var/www/agent_stack/data/www/doraprojects.net/template/agent.layouts/content.php on line 54
2011-01-16 18:24:25

Odpowiedź naprawdę zależy od innych aspektów architektury, takich jak to, czy metoda checkCredentials() musi być dostępna poza zakresem klasy dla innych elementów systemu. Jeśli jej jedynym celem jest wywołanie metodą login (), rozważ połączenie tych dwóch metod w jedną metodę.

Jeszcze jedną rekomendacją, którą mogę mieć, jest użycie czasowników w metodach nazywania, co oznacza, że 'login' może być zbyt ogólny, aby zrozumieć jego skutki na pierwszy rzut oka.

 0
Author: Dennis Kreminsky,
Warning: date(): Invalid date.timezone value 'Europe/Kyiv', we selected the timezone 'UTC' for now. in /var/www/agent_stack/data/www/doraprojects.net/template/agent.layouts/content.php on line 54
2018-08-30 14:41:46