Использование PDO

Question

Только что глянул видос про PDO и сразу переписал старый код.Насколько он безопасный и грамотный?Что добавить, а что убрать?

<?php
    // Auth
    session_start();

    $host = 'localhost';
    $db_user = 'root';
    $db_password = '';
    $db_name = 'daladno_db';

    $dsn = "mysql:host=$host;dbname=$db_name";

    $connection = new PDO($dsn, $db_user, $db_password);

    if(isset($_POST['do_auth'])) {
        $username = $_POST['username'];
        $password = $_POST['password'];

        if ($username == '') {
            $error = 'Invalid username or password';
        }

        else if($password == '') {
            $error = 'Invalid username or password';
        } 
        // No error
        else {
            $sql = 'SELECT * FROM admins WHERE username = ?';
            $stmt = $connection->prepare($sql);
            $stmt->execute([$username]);

            if($stmt->rowCount() == 0) {
                $error = 'Invalid username or password';

            } else {
                $user = $stmt->fetch(PDO::FETCH_ASSOC);

                if( !password_verify($password, $user['password']) ){
                    $error = 'Invalid username or password';
                } else {
                    // Access!
                    $_SESSION['userLogin'] = $user['username'];
                }
            }
        }
    }
?>

Answer 1

Ну если Вас устраивает спагетти-код, то можно было бы использовать специальную функцию для проверки !empty ($username). Еще один момент, что переменной $error, Вы во всех случаях присваиваете одну и ту же строку, дублирование кода, плохой тон.

$user = $stmt->fetch (PDO::FETCH_ASSOC) - вернет либо массив, либо булевое значение false, нет смысла вызывать еще один метод чтобы проверить наличие результата.

Хотя бы что-то вроде того.

<?php /* Authorization */
session_start ();

/**
 * Db connection vars
 */
$host        = 'localhost';
$db_user     = 'root';
$db_password = '';
$db_name     = 'daladno_db';
$dsn         = "mysql:host=$host;dbname=$db_name";

/**
 * Default value of error handler
 */
$error = 'Invalid username or password';

/**
 * Create connection
 */
$connection = new PDO ($dsn, $db_user, $db_password);

/**
 * Check data
 */
if (isset ($_POST['do_auth'])) {
    $username = $_POST['username'];
    $password = $_POST['password'];

    if (!empty ($username) and !empty ($password)) {
        $sql = 'SELECT * FROM admins WHERE username = ?';

        $stmt = $connection->prepare ($sql);
        $stmt->execute ([$username]);

        $user = $stmt->fetch (PDO::FETCH_ASSOC);

        if (!is_bool ($user)) {
            if(password_verify ($password, $user['password']) ){
                $_SESSION['userLogin'] = $user['username'];
                $error = 'Access granted.';
            }
        }
    }
}

Answer 2

Хороший вопрос.

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

Повторений в этом коде слишком много, причем на всех уровнях. представим, что у тебя на сайте не 1 РНР скрипт, а хотя бы 10. И в каждом ты будешь писать код подключения к PDO, серьёзно? А если потом надо будет поменять хост подключения к базе данных? Править во всех файлах?

Нет, конечно. Подключение необходимо вынести в отдельный файл и подключать через require.

Само подключение - это не одна строчка, а много других настроек. Правильный код смотреть здесь

Ещё одна повторяемость на глобальном уровне - это постоянный вызов одних и тех же функций prepare/execute на каждый запрос. Удобнее засунуть их в функцию типа такой

function pdo($pdo, $sql, $args = NULL)
{
    if (!$args)
    {
         return $pdo->query($sql);
    }
    $stmt = $pdo->prepare($sql);
    $stmt->execute($args);
    return $stmt;
}

с её помощью выполнение запросов можно будет писать в 1 строчку:

$user = pdo($pdo, 'SELECT * FROM admins WHERE username = ?', [$username])->fetch();

Ну и наконец, как правильно написал в своём ответе Евгений, нужно убрать многочисленные повторяющиесмя проверки:

<?php
session_start ();
require 'pdo.php';

if (!empty($_POST['username']) && !empty($_POST['password'])) {
    $username = $_POST['username'];
    $password = $_POST['password'];

    $sql = 'SELECT * FROM admins WHERE username = ?';
    $user = pdo($pdo, $sql, [$username])->fetch();
    if($user && password_verify($password, $user['password']) ){
         $_SESSION['userLogin'] = $user['username'];
         header("Location: /admin.php");
         exit;
    }
}
$error = 'Invalid username or password';

Обрати внимание, что после успешного POST запроса всегда должен быть редирект методом GET.

БЛОГ НА HUSL

Использование PDO

Ответы (2 шт):