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

Ocena kodu logowania.

+2 głosów
111 wizyt
pytanie zadane 16 czerwca w PHP przez Dziedzic Obywatel (1,200 p.)

Witam, proszę Was o ocenę kodu logowania napisanego przeze mnie w PHP. Jeśli jest coś źle zrobione proszę o napisanie co mógłbym poprawić.

<?php



    session_start();

    $message = "Wszystko OK!";

    $conn = mysqli_connect("host", "nick", "hasło", "nazwa bazy danych");



    if ($conn->connect_error) {

        die($conn->connect_error);

    } else {  

        if (isset($_SESSION['user_id'])) {

            header('Location: user_panel.php');

        } else {  

            if (!empty($_POST["Name"]) && mysqli_real_escape_string($conn, $_POST["Name"]) == $_POST["Name"] || !empty($_POST["Password"]) && mysqli_real_escape_string($conn, $_POST["Name"]) == $_POST["Name"]) {

                $name = $_POST["Name"];

                $sql = "SELECT * FROM `players` WHERE BINARY name = '$name'";

                $results = mysqli_fetch_assoc(mysqli_query($conn, $sql));

                

                if (!($results == null)) {

                    if (count($results) > 0 && password_verify($_POST["Name"], $results["password"])) {

                        $_SESSION["user_id"] = $results["id"];

                        header('Location: user_panel.php');

                    } else {

                        $message = "Nieprawidłowe dane.";

                    }

                } else {

                    $message = "Nieprawidłowe dane.";

                }

            } else {

                $message = "Wszystko OK!";

            }

        }

    }



?>

 

2 odpowiedzi

+6 głosów
odpowiedź 16 czerwca przez Comandeer Guru (554,620 p.)
wybrane 16 czerwca przez Dziedzic
 
Najlepsza
  • Skoro stosujesz die, to możesz się pozbyć tego else. Zawsze to jeden poziom zagnieżdżenia mniej, a kod będzie działał tak samo (bo jak wejdzie do ifa z die, to i tak dalej nie pójdzie).
  • Jak już jesteśmy przy tym die – po co userowi techniczne info o błędzie? Takie coś jest przeznaczone dla programisty i powinno trafić do logów, a user powinien po prostu dostać komunikat, ze coś nie działa.
  • Zresztą podobny zabieg można zrobić ze sprawdzaniem S_SESSION. Na dobrą sprawę to sprawdzanie powinieneś zrobić na samym początku, bo do tego Ci nawet połączenie z bazą niepotrzebne. Jak zmienna sesyjna istnieje, to header + die i tyle. Nie potrzeba else.
  • Czemu porównujesz wprowadzone przez usera dane do wyniku przepuszczenia ich przez mysqli_real_escape_string? To mocno ogranicza tak naprawdę bezpieczeństwo hasła, w którym mogą być przeróżne znaki. Pierwsze losowo wygenerowane hasło z generatora haseł online-vx"`%Z2#m2r/&%%. Powinieneś używać prepared statements. Inna rzecz: po co to porównanie robisz dla hasła, skoro go nawet nie umieszczasz w zapytaniu?
  • password_verify chyba powinieneś używać $_POST['Password'] zamiast $_POST['Name'].
  • Przy zastosowaniu tzw. guard clauses całą tę drabinkę ifów można by znacząco uprościć.
+2 głosów
odpowiedź 16 czerwca przez Wiciorny Mędrzec (154,820 p.)

https://pl.wikipedia.org/wiki/SQL_injection

WHERE BINARY name = '$name'";

 

Podobne pytania

+1 głos
1 odpowiedź 425 wizyt
+2 głosów
0 odpowiedzi 181 wizyt
pytanie zadane 10 lutego 2018 w Bezpieczeństwo, hacking przez Hysek Obywatel (1,040 p.)
0 głosów
1 odpowiedź 142 wizyt
pytanie zadane 18 września 2018 w C i C++ przez Zimny. Nowicjusz (140 p.)
Porady nie od parady
Zadając pytanie postaraj się o poprawną pisownię i czytelne formatowanie tekstu.Kompozycja

84,091 zapytań

132,861 odpowiedzi

293,830 komentarzy

55,529 pasjonatów

Motyw:

Akcja Pajacyk

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

Oto dwie polecane książki warte uwagi. Pełną listę znajdziesz tutaj.

...