@Pyhon3x

Можно-ли улучшить этот код?

Код:
<?php
//Require-скрипты
  require 'connect.php';

//Переменные, и другая ерунда...
  $server_Code = random_int(0, 9999999999);
  $client_Code = random_int(0, 9999999999);
  $vereficationCode = random_int(0, 999999);
  $session = $server_Code + $client_Code;

  $email = $_POST['email'];
  $UN = $_POST['User-Name'];
  $psw = $_POST['psw'];
  if ($psw == null AND $email == null) {
    header('Location: http://test-server.tsecret.net/AUTH-2/Sign_In/index.php?response=NOT-Not_Data');
    exit();
  }
  $psw_repeat = $_POST['psw_repeat'];
//Проверки
  if (strlen($psw) < 8) {
    header('Location: http://test-server.tsecret.net/AUTH-2/Sign_In/index.php?response=NOT-PtL');
  } else if (!preg_match("#[0-9]+#", $psw)) {
    header('Location: http://test-server.tsecret.net/AUTH-2/Sign_In/index.php?response=NOT-number');
  } else {
    //Проверка на корекстность E-Mail
    if (filter_var($email, FILTER_VALIDATE_EMAIL)){
      //Проверка на одинаковость паролей
      if ($psw == $psw_repeat) {
        $psw_hash = hash('sha512', $psw);
        $sql = 'SELECT * FROM `Users` WHERE `Email`=:email';
        $query = $pdo->prepare($sql);
        $query->execute(["email" => $email]);
        $row = $query->fetch(PDO::FETCH_OBJ);

        //Такой E-Mail зарегестрирован?
      if ($row->Email != $email) {
          $sql = 'INSERT INTO `Users`(`Email`, `PSW`, `User_Name`, `session`, `server_Code`, `provider`) VALUES (:email, :psw, :UN, :session, :server_Code, "TS_AUTH")';
          $query = $pdo->prepare($sql);
          $query->execute([
            "email" => $email,
            "psw" => $psw_hash,
            "UN" => $UN,
            "session" => $session,
            "server_Code" => $server_Code
          ]);
          session_start();
          setcookie("session_id", $client_Code, '/AUTH');
          $_SESSION['email'] = $email;
          $_SESSION['UN'] = $UN;
          $_SESSION['vereficationCode'] = $vereficationCode;

          $to  = "<".$email.">" ;

          $subject = "Подтвердите Почту";

          $message = '
            <h1 align="center">Привет , Это TS</h1>
            <h3 align="center">|!| Если вы не проводили регестрацию просто проигнорируйте это сообщение |!|</h3>
            <p align="center">Вы хотели зарегистрироваться на нашем сайте, для это-го перейдите на <a href="accounts.tsecret.net/Log_In/verificatoinEmail.php?Code='.$vereficationCode.'">ЭТУ</a> страницу и введите Verification Code: '.$vereficationCode.'<p>
          ';

          $headers  = "Content-type: text/html; charset=utf-8\r\n";
          $headers .= "From: Центер Аккаунтов TSecret <Account-Center@tsecret.net>\r\n";

          mail($to, $subject, $message, $headers);
          setcookie("PHPSESSID", $_COOKIE['PHPSESSID'], 0, '/', '.tsecret.net');

          header('Location: http://test-server.tsecret.net/AUTH-2/Sign_In/index.php?response=200');

        } else if($row->Email == $email) {
          header('Location: http://test-server.tsecret.net/AUTH-2/Sign_In/index.php?response=NOT-Acc!');
        }
    } else {
      header('Location: http://test-server.tsecret.net/AUTH-2/Sign_In/index.php?response=NOT-P(s)isNot==');
    }
  } else {
     header('Location: http://test-server.tsecret.net/AUTH-2/Sign_In/index.php?response=NOT-EiNC');
     }
}

//https://www.php.net/manual/ru/function.password-verify.php
?>

Спасибо!
  • Вопрос задан
  • 293 просмотра
Решения вопроса 3
FanatPHP
@FanatPHP
Чебуратор тега РНР
1. выкинуть все локейшены с ошибками. это дикость, ни один нормальный сайт так не делает. ошибки надо показывать либо сразу, либо через сессию. локейшены с ошибками встречаются только у дебилов, которые делают видео на ютубе для других дебилов. не надо так палиться сразу.
2. выкинуть домен из ссылок. ты серьёзно собираешься переписывать все ссылки, когда у тебя сайт с временного домена переедет на постоянный? а потом обратно - когда надо будет потестить локально? header('Location: /AUTH-2/Sign_In/'); достаточно для единственного локейшена, который нужен в этом коде
3. убрать всю эту лестницу иф-ов, делать все проверки на одном уровне. ошибки собирать в массив. перед вставкой в БД проверить массив на пустоту.
4. if ($row->Email != $email) - масло масляное. ты УЖЕ проверил емейл в базе, зачем еще раз проверять?
5. else if($row->Email == $email) { - это уже какой-то совсем адок. Ты УЖЕ проверил, что емейл не совпадает. причем два раза. В else мы попадём, если емейлы совпдают. Ещё раз проверять не надо. Два алкоголика садятся на трамвай, один спрашивает водителя - я этом номере до вокзала доеду? Водитель - нет. Второй алкаш - а я?
6. внизу у тебя ссылка на password_verify, но хэшируешь ты все равно кривым алгоритмом. Не осилил?
7. все эти куличики в песочнице с $client_Code $server_Code - это какой-то адок с точки зрения безопасности. Любой школьник, который не тупее дауна, поломает все твои "сессии" за 5 минут.
8. setcookie("PHPSESSID", $_COOKIE['PHPSESSID'], 0, '/', '.tsecret.net'); - опять совершенно бессмысленная строчка. К чему она? Зачем? Что ты хотел тут сказать? И кому?
9. Разбей это простыню хотя бы на функции. Никакой солид у тебя конечно не получится, как и у Георгий Котов который сам не понимает что это значит. Но хотя бы разделить проверки, запись в бд, и отправку емейла можно.

В целом - из плюсов только нормальная работа с БД, в остальном на троечку, и местами - кол (за дыры в безопасности и отсутствие логики)
Ответ написан
Да улучшить можно, я бы по SOLID принципам его переписал, чем не улучшение?
Ответ написан
Комментировать
@Carfik
Автор говнокода
С беглого просмотра: у тебя email чувствителен к регистру, сначала нужно и то что пришло с базы (или записывать в базу ), и то что тебе прилетело заделать в нижний регистр
Ответ написан
Пригласить эксперта
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Похожие вопросы