• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

System logowania w obiektowym PHP - czy tak powinien wyglądać?

VPS Starter Arubacloud
0 głosów
1,190 wizyt
pytanie zadane 6 czerwca 2017 w PHP przez To Ja Początkujący (490 p.)

Witam. Niedawno zacząłem naukę obiektowego PHP i w ramach ćwiczeń postanowiłem zrobić system logowania. Niby wszystko działa, ale nie wiem, czy nie ma jakichś błędów i czy nie dałoby się czegoś zrobić lepiej. Moje wątpliwości dotyczą szczególnie tego:
1) czy rzeczywiście jest to standardowe rozwiązanie w OOP,
2) czy powinienem dodać wykrywanie błędów w kodzie (coś w stylu try/catch),
3) zabezpieczenia kodu (mam w zasadzie tylko htmlentities)

Nie będę może załączać całego kodu - problemy powinny być już widoczne w klasie tworzącej bazę, klasie logującej i samym pliku login.php:

DB.php:

<?php
//klasa łącząca z bazą danych
class DB{
    private static $connection = null; //połączenie
    public $pdo;
    private $host = 'localhost', $dbname='home', $username = 'root', $password = ''; //dane bazy

    private function __construct(){
        //nawiązuje nowe połączenie
        try{
            $this->pdo = new PDO("mysql:host={$this->host};dbname={$this->dbname}", $this->username,$this->password);
        } catch(PDOException $e){
            die($e->getMessage());
        }
    }
    //metoda, przez którą wykonuje się połączenie z bazą danych - zapobiega wielokrotnym połączeniom
    //łączenie odbywa się więc przez DB::connection();
    public static function connection(){
            if(!isset(self::$connection)){
                self::$connection = new DB();
            }
            return self::$connection;
        }
        public function __destruct(){
            $this->pdo = null;
            self::$connection = null;
   }
}

Login.php:

<?php

class Login{
    private $login, $password;
    private $con;
    public function __construct($con){
        $this->con = $con->pdo;
        $this->login = $_POST['login'];
        $this->password = $_POST['password'];
        $this->login = $this->clear($this->login);
        $this->password = $this->clear($this->password);
        $this->checkData();
    }
    //zabezpieczenie przed wpisywaniem niedozwolonych znaków
    private function clear($string){
    return htmlentities($string, ENT_QUOTES, 'UTF-8');
    }
    //jeśli puste lub zalogowany
    private function checkData(){
        if($this->login=='' || $this->password== ''){
            header("Location: index.php");
            exit();
        }elseif(isset($_SESSION['username'])){
            header("Location: home.php");
            exit();
        }
    }
    //załadowanie danych z bazy
    public function loadData(){
        $sql = "SELECT id, username, password, time, rank, info FROM users WHERE username='{$this->login}'";
        $request = $this->con->prepare($sql);
        $request->execute();
        $num = $request->rowCount();
        //jeśli jest taki użytkownik
        if($num>0){
        $this->setData($request);
        }else{ //jeśli nie ma takiego użytkownika
        $_SESSION['error'] = 'Błędny login lub hasło';
        header("Location: index.php");
        }
    }
    //logowanie
    private function setData($request){
            $result = $request->fetch(PDO::FETCH_ASSOC);
            //weryfikacja hasła
            if(password_verify($this->password, $result['password'])){
            unset($_SESSION['error']);
             $_SESSION['id'] = $result['id'];
             $_SESSION['username'] = $result['username'];
             $_SESSION['info'] = $result['info'];
             $_SESSION['time'] = $result['time'];
             $_SESSION['rank'] = $result['rank'];
             header("Location: home.php");
             }else{
                 $_SESSION['error'] = 'Błędny login lub hasło';
                header("Location: index.php");
             }
    }
    //wylogowanie
    public static function logout(){
            unset($_SESSION['id']);
            unset($_SESSION['username']);
            unset($_SESSION['rank']);
            unset($_SESSION['info']);
            unset($_SESSION['time']);
            unset($_SESSION['error']);
            unset($_SESSION['error_2']);
            unset($_SESSION['error_change']);
            header("Location: index.php");
            exit();
    }
    
}

I sam plik, do którego wędrują zmienne z POST:

<?php
require_once('init.php');
if(is_logged()){ //przekierowanie, jeśli użytkownik jest już zalogowany
    header("Location: home.php");
    exit();
}
$con = DB::connection();
$login = new Login($con);
$login->loadData();
$con->__destruct();

Mam jeszcze inne klasy, np. rejestrację, ale myślę, że tyle wystarczy - możliwe, że jest tu sporo błędów. Z góry dziękuję za wszelkie sugestie, jak mógłbym poprawić ten kod. smiley

1 odpowiedź

+1 głos
odpowiedź 6 czerwca 2017 przez Boshi VIP (100,240 p.)
Do przepisania od 0 :)

Klasa DB nie robi nic oprócz łączenia się z bazą, bez sensu.

 

klasa login to nie klasa a metoda. Po drugie robi wszystko, waliduje, pobiera dane, zapisuje je.

masz chyba problemy z podstawami php. Pewnie wiesz, że jest takie coś jak pętla. Więc nie rozumiem co to są za unsety pojedyńcze.

Stosuj się do zasady SRP. Czyli pojedyńczej odpowiedzialności. Jeżeli już masz tą klasę Login, ona nie robi nic innego jak loguje. Wszystkie operacje jak to robi wywalasz pod wyższą warstwę abstrackji.
1
komentarz 6 czerwca 2017 przez HaKIM Szeryf (87,590 p.)

Więcej o SRP:

https://drive.google.com/file/d/0ByOwmqah_nuGNHEtcU5OekdDMkk/view

Więcej o całym SOLID:

http://www.butunclebob.com/ArticleS.UncleBob.PrinciplesOfOod

Przeczytaj, od deski do deski, te zasady poprawią Twój kod. :)

komentarz 6 czerwca 2017 przez To Ja Początkujący (490 p.)

Od zera? Nie wiedziałem, że jest aż tak źle, ale przynajmniej znam teraz prawdę. laugh

Poczytam o tym SRP zgodnie z radami, bo obecnie tego nie rozumiem: DB robi za mało, Login robi za dużo. Myślałem, że w obrębie jednej klasy zawiera się daną czynność, dlatego tak to podzieliłem. No nic, będę się uczyć dalej, dzięki ;)

1
komentarz 6 czerwca 2017 przez HaKIM Szeryf (87,590 p.)
edycja 6 czerwca 2017 przez HaKIM

 DB robi za mało, Login robi za dużo.

Tak to nie działa.

Ma robić to, do czego został stworzony. Jeśli praca listonosza polega na dostarczaniu listów to oczywistym jest, że nie będzie ich czytał, a przynajmniej dobry listonosz.

Tak samo jest z kodem, jeżeli metoda odpowiada za walidację, to nie spodziewasz się, że będzie, przy okazji, zwracała zwalidowane dane.

Pseudo kodzik:

class Validation
{
    private data;

    constructor (data) : void
    {
        if (...) {
            throw exception(...);
        }
    
        this->data = data;
    }

    getData() : array
    {
        return this->data;
    }
}

yes

class Validation
{
    private data;

    constructor (data) : void
    {
        if (...) {
            throw exception(...);
        }
    
        return data;
    }
}

no

Tylko teraz trzymaj się tej zasady w całym projekcie (Choć, nie zawsze istnieje taka możliwość. Było nie było, to tylko zasady.).

komentarz 13 czerwca 2017 przez To Ja Początkujący (490 p.)
Poczytałem trochę na temat tworzenia dobrego kodu i wziąłem się za pisanie systemu logowania od nowa. Nie wiem tylko, jak bardzo uogólnić działanie klas, żeby były "odporne" na zmiany. Na przykład, czy powinienem z góry założyć, że liczba danych w formularzu się zwiększy? Jeśli tak, to czy muszę brać pod uwagę również te dość nieracjonalne scenariusze (np. więcej checkboxów albo pól typu date)?
I może jeszcze druga sprawa - czy rozbudowanie klasy DB, żeby poza samym łączeniem z bazą zajmowała się wykonywaniem zapytań sql jest dobrym pomysłem?
1
komentarz 13 czerwca 2017 przez HaKIM Szeryf (87,590 p.)
Nie stworzysz softu odpornego na każdą zmianę bo się nie opłaca. No, może poza warstwą dziedziny która się ciągle zmienia, tam należy dołożyć wszelkich starań aby można było dokonać zmian bez większego płaczu.

Rozwiązaniem będzie pytanie się klienta czy w najbliższym czasie planują zmienić czy dodać to i owo. Jeśli odpowie, że do formularza może pojawić się nowa opcja za rok, a Ty masz kilka tygodnii na dokończenie projektu, to bym się zbytnio nie przejmował, może do tego czasu zbankrutują. :D

Podobne pytania

0 głosów
0 odpowiedzi 220 wizyt
pytanie zadane 30 sierpnia 2020 w PHP przez Bakkit Dyskutant (7,600 p.)
+1 głos
1 odpowiedź 345 wizyt
0 głosów
0 odpowiedzi 303 wizyt
pytanie zadane 3 czerwca 2018 w PHP przez karol928 Początkujący (320 p.)

92,775 zapytań

141,703 odpowiedzi

320,571 komentarzy

62,111 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto polecana książka warta uwagi.
Pełną listę książek znajdziesz tutaj.

Akademia Sekuraka

Niedawno wystartował dodruk tej świetnej, rozchwytywanej książki (około 940 stron). Mamy dla Was kod: pasja (wpiszcie go w koszyku), dzięki któremu otrzymujemy 10% zniżki - dziękujemy zaprzyjaźnionej ekipie Sekuraka za taki bonus dla Pasjonatów! Książka to pierwszy tom z serii o ITsec, który łagodnie wprowadzi w świat bezpieczeństwa IT każdą osobę - warto, polecamy!

...