Klasa logowania usera Opcje

[michalboss]

Witam

Napisałem taką klasę logowania użytkownika w systemie.
Nie jest jeszcze skończona, ale chciałem prosić o ewentualne wskazanie błędów w w/w kodzie, ponieważ dopiero zaczynam OOP w php.

[PHP] pobierz, plaintext 
<?php
 
class Login
{
 
    private $_dbh;
    private $_config;
    private $_lang;
    private $_isLoggin = FALSE;
    private $_idSession;
    private $_email;
    private $_password;
 
    public function __construct($dbh, $config, $lang)
    {
        $this -> _dbh = $dbh;
        $this -> _config = $config;
        $this -> _lang = $lang;
    }
 
    /**
     * Metoda sprawdza czy user jest zalogowany w systemie
     * 
     * @access public
     * @return bool
     */
    public function checkLoggin()
    {
        return $this -> _isLoggin;
    }
 
    /**
     * Metoda sprawdza czy istnieją zmienne sesyjne - jeśli tak, to loguje usera w klasie
     * 
     * @access public
     * @return bool
     */
    protected function isLoggin()
    {
        if($_SESSION['isLoggin'] == TRUE)
        {
            $this -> _isLoggin = $_SESSION['isLoggin'];
            $this -> _idSession = session_id();
            $this -> _email = $_SESSION['email'];
            return TRUE;
        }
        else
        {
            return FALSE;
        }
    }
 
    /**
     * Metoda logowania - sprawdzania czy użytkownik jest zalogowany i czy istnieją zmienne formularza logowania 
     * Jeśli tak, to sprawdza w bazie poprawność e-mail oraz hasła i dokonuje logowania w systemie 
     * 
     * @access public
     * @return void
     */
    public function userLogin()
    {
        $errorMsg = '';
        if(!($this -> _isLoggin) && isset($_POST['logEmail']) && isset($_POST['logPassword']))
        {
            $sql = $this -> _dbh -> prepare('SELECT A.IdUser, A.Email, A.Password, A.Salt, A.FirstName, A.LastName, A.ErrorLogin, A.ErrorLoginTime, A.Current  '
                    . 'FROM `users` A '
                    . 'WHERE A.Email=:email LIMIT 1');
            $sql -> bindValue(':email', $_POST['logEmail'], PDO::PARAM_STR);
            $sql -> execute();
            if($sql -> rowCount() > 0)
            {
                if($row = $sql -> fetch())
                {
                    if(self::encode($_POST['logPassword'], $row['Salt']) == $row['Password'])
                    {
                        $_SESSION['isLoggin'] = TRUE;
                        $_SESSION['email'] = $row['Email'];
                        $_SESSION['IdUser'] = $row['IdUser'];
                        $this -> isLoggin();
                    }
                    else
                    {
                        $errorMsg = $this -> _lang['errorLogin'];
                    }
                }
            }
            else
            {
                $errorMsg = $this -> _lang['errorLogin'];
            }
 
            if(!empty($errorMsg))
            {
                throw new Exception($errorMsg);
            }
        }
        else if(!($this -> _isLoggin) && isset($_SESSION['isLoggin']) && $_SESSION['isLoggin'] == TRUE)
        {
            $this -> isLoggin();
        }
    }
 
    /**
     * Metoda wylogowuje uzytkownika i usuwa sesję
     * 
     * @access public
     * @return void
     */
    public function userLogout()
    {
        unset($_SESSION);
        unset($this -> _isLoggin);
        unset($this -> _idSession);
        unset($this -> _email);
        session_destroy();
        header("Location: index.php?msg=logout");
    }
 
    /**
     * Metoda hashuje hasło używając podanej soli z kartoteki usera
     * 
     * @access public
     * @param string $Password
     * @param string $Salt
     * @return string
     */
    public static function encode($Password, $Salt)
    {
        return hash('sha256', $Salt . hash('sha256', $Password . $Salt));
    }
 
    /**
     * Metoda tworzy sól podanego hasła
     * 
     * @access public
     * @param string $Password
     * @return string
     */
    public static function salt($Password)
    {
        return substr(hash('sha256', microtime() . substr($Password, 0, 3)), 0, 5);
    }
 
    public static function timemicro()
    {
        list($z1, $se) = explode(" ", microtime());
        return ((float) $z1 + (float) $se);
    }
 
}
?>
[PHP] pobierz, plaintext 

[CuteOne]

Już trochę lepiej to wygląda ale gdybyś pracował jako programista taki kod nigdy nie wszedłby na produkcję (IMG:style_emoticons/default/wink.gif) Dobra rada - spróbuj swoich sił z jakimś frameworkiem np. Zend 1/2, Symfony 2, CakePHP. Zobaczysz jak powinno się pisać aplikacje, czym tak naprawdę jest OPP (to nie zbiór metod w klasie X) i jak z niego korzystać

Szybkie CR
1. Nie rób jednej klasy, która robi masę różnych rzeczy - widziałeś kiedyś bolida F1, który kosi trawnik, ora pole i rozwozi mleko? Podziel tą klasę na mniejsze odpowiadające różnym zadaniom
- coś odpowiedzialnego za obsługę sesji
- coś co filtruje/waliduje dane
- klasa autoryzacji (nie myl z "logowaniem" - klasa logowania jedynie korzysta z klas autoryzacji)
- klasa obsługująca akcje autoryzacyjne - logowanie, wylogowanie czy rejestracja
- klasa trzymająca dane użytkownika - login, id itp.
- klasa obsługująca hashowanie
itp.
2. Za dużo if else. Poważnie... przemyśl czy nie da się tego uprościć/rozbić na kilka metod
3. " header("Location: index.php?msg=logout");" to powinno wylecieć, metoda logout ma TYLKO jedno zadanie - usuń sesję użytkownika - tyle, żadnych redirectów czy echów. To samo tyczy się innych metod
4. Do konstruktora, możesz przekazać tablicę jako handler bazy danych (brak "wymuszenia" typu danych)
5. Pomijam ocenę tych czasochłonnych operacji przy generowaniu soli (kwestia gustu) jednak istnieją prostsze metody jej generowania (IMG:style_emoticons/default/wink.gif)
6. Cóż to za potworek.. ?

[PHP] pobierz, plaintext 
else if(!($this->isLogged) && isset($_SESSION['isLogged']) && $_SESSION['isLogged'] == TRUE)
[PHP] pobierz, plaintext 

7. I kolejny

[PHP] pobierz, plaintext 
(...)$errorMsg = $this->lang['errorLoginTime'] . ' ' . $timeCheckMin . ' min.';(...)
(...)$errorMsg = $this->lang['errorLogin'];(...)
//itp.
// a później..
if(!empty($errorMsg))
            {
                throw new Exception($errorMsg);
            }
[PHP] pobierz, plaintext 

@edit w metodzie 'userLogin()' robisz

[PHP] pobierz, plaintext 
$this->isLogged = TRUE;
$this->email = $row['Email'];
$this->idUser = $row['IdUser'];
$this->idSession = session_id();
$this->setSessionData();
[PHP] pobierz, plaintext 

po czym w setSessionData() robisz ponownie to samo...

Ten post edytował CuteOne 1.08.2015, 22:58:24