В PHP регистрация и вход


Я хочу, чтобы все было хорошо и безопасно с моим кодом проверки подлинности. Я много сделал регистрацию и системы входа и раньше, но только для целей тестирования, и не учитывать все меры безопасности, таких как SQL-инъекции, хэширование пароля и т. д... Это и есть код. Если вы можете, пожалуйста, укажите на любые функции, которые могут быть расширены.

Зарегистрироваться:

public function setUser(){

$fullName = htmlspecialchars($_POST['fullName']);
$email = htmlspecialchars($_POST['email']);
$password = htmlspecialchars($_POST['password']);
$DOB = htmlspecialchars($_POST['DOB']);
$nationality = htmlspecialchars($_POST['nationality']);
$workTitle = htmlspecialchars($_POST['workTitle']);
$workPlace = htmlspecialchars($_POST['workPlace']);
$phoneNumber = htmlspecialchars($_POST['phoneNumber']);
$userType = htmlspecialchars($_POST['userType']);


//PASSWORD HASHING
$hashedPassword = password_hash($password, PASSWORD_DEFAULT);

if($userType == "Investor"){
  $accNumber = "A" . sprintf("%06d", mt_rand(1, 999999));
}else{
  $accNumber = "B" . sprintf("%06d", mt_rand(1, 999999));
}

try {

  $sql = "INSERT INTO user (fullName, email, password, DOB, nationality, workTitle, workPlace, phoneNumber, userType, accNumber) VALUES (?,?,?,?,?,?,?,?,?,?)";
  $stmt = $this->connect()->prepare($sql);
  $stmt->execute([$fullName, $email, $hashedPassword, $DOB, $nationality, $workTitle, $workPlace, $phoneNumber, $userType, $accNumber]);

} catch (PDOException $e) {
  throw new Exception($e->getMessage());
}
header('location:login.php');

} //setUser()

Логин:

//RETRIEVE HASHED PASSWORD FROM DB
public function getPassword($email){
  $sql = "SELECT password FROM user WHERE email = ?";
  $stmt = $this->connect()->prepare($sql);
  $stmt->execute([$email]);

  if($stmt->rowCount()){
    while($row = $stmt->fetch()){
      return $row['password'];
    }
  }
} //getPassword()

//LOGIN THE USER
public function loginUser(){

$email = htmlspecialchars($_POST['email']);
$password = htmlspecialchars($_POST['password']);
$hashedPassword = $this->getPassword($email);

//Check if hashed password is equal to password
if(password_verify($password,$hashedPassword)){
  try {
    $sql = "SELECT * FROM user WHERE email = ?";
    $stmt = $this->connect()->prepare($sql);
    $stmt->execute([$email]);

    if($stmt->rowCount()){
      while($row = $stmt->fetch()){
        $_SESSION['userID'] = $row['userID'];
        $_SESSION['fullName'] = $row['fullName'];
        $_SESSION['email'] = $row['email'];
        $_SESSION['DOB'] = $row['DOB'];
        $_SESSION['nationality'] = $row['nationality'];
        $_SESSION['workTitle'] = $row['workTitle'];
        $_SESSION['workPlace'] = $row['workPlace'];
        $_SESSION['phoneNumber'] = $row['phoneNumber'];
        $_SESSION['userType'] = $row['userType'];
        $_SESSION['accNumber'] = $row['accNumber'];
        $_SESSION['status'] = $row['status'];
        $_SESSION['profilePic'] = $row['profilePic'];
        header('location:dashboard.php');
      }
    }

  } catch (\Exception $e) {
    throw new Exception($e->getMessage);
  }
}else{
  echo"Invalid email or password";
}

} //loginUser()

Я еще не работал на проверки, но я буду. Мне просто нужно убедиться, что все остальное хорошо идти.



Комментарии
3 ответа

Я вижу только один вопрос, касающийся безопасности:

$accNumber = "A" . sprintf("%06d", mt_rand(1, 999999));

есть шанс для двух клиентов, чтобы получить тот же номер счета. Я хотел бы сделать это просто письмо + ID строки.

Остальной ваш код является довольно безопасным, но есть много бесполезных или лишними или неуместными или неэффективные вещи.

$fullName = htmlspecialchars($_POST['fullName']);
$email = htmlspecialchars($_POST['email']);
$password = htmlspecialchars($_POST['password']);
...

этот материал является неуместным. htmlspecialchars() принадлежит к выходу; это не имеет ничего общего с регистрации пользователя. Не $_POST. А что, если вам придется создать утилиты командной строки, чтобы создать пользователя? А setUser() функция должна принимать данные пользователя в качестве параметра, и где эти данные поступают из не эта функция забота.

throw new Exception($e->getMessage());

внутри блока catch излишни и бесполезны. Если вы оставите за исключением одного, он будет выброшено все то же самое!

$this->connect()

здесь неуместен. С учетом этого класса, связанные с пользователем, на любой базе материалов связи ему не чуждо. Пользователь не является базой данных. Пользователь представляет собой человека. Действительно у них есть имя, номер телефона, место работы... но подключение к базе данных?! У вас есть метод Connect ()? Так что не ваш пользователей, а также. Подключение к базе данных должна быть услуга, предоставляемая через конструктор класса.

header('location:login.php');

опять же, что HTTP вещи не касаются инструкция setuser, оно должно быть перемещены в другом месте.

Ну и в конце инструкция setuser должна быть такой:

public function setUser($fullName, $email, $password, $DOB, $nationality, $workTitle, $workPlace, $phoneNumber, $userType) {

$hashedPassword = password_hash($password, PASSWORD_DEFAULT);

if($userType == "Investor"){
$accNumber = "A";
}else{
$accNumber = "B";
}

$sql = "INSERT INTO user (fullName, email, password, DOB, nationality, workTitle, workPlace, phoneNumber, userType) VALUES (?,?,?,?,?,?,?,?,?,?)";
$stmt = $this->db->prepare($sql);
$stmt->execute([$fullName, $email, $hashedPassword, $DOB, $nationality, $workTitle, $workPlace, $phoneNumber, $userType]);
$id = $this->db->lastInsertId();
$sql = "UPDATE user SET accNumber = concat(?, id) WHERE id = ?";
$stmt = $this->db->prepare($sql)->execute([$accNumber, $id]);
}

логин

public function getPassword($email){

выглядит бесполезным и лишним. Почему вы хотите отдельную функцию, чтобы получить пользователей хэш? Особенно, если вы назовете еще один запрос, чтобы получить все пользовательские данные? На самом деле, весь процесс входа в систему занимает всего несколько строк

if($stmt->rowCount()){

это лишнее

while($row = $stmt->fetch()){

это лишнее. Когда ожидается одна строка, ни петля не требуется

$_SESSION['userID'] = $row['userID'];

до боли однообразным. И загрязняя свои сессии. Почему не сделать просто

$_SESSION['user'] = $row;

это сделает ваши сессии аккуратно. Когда вы wjll нужно добавить в корзину, это обыкновение быть смешаны с данными пользователя.

echo"Invalid email or password";

такой функции никогда не должны сделать никакого вывода, а также.

в конце концов это должно понравиться

public function loginUser($email, $password){
$stmt = $pdo->prepare("SELECT * FROM users WHERE email = ?");
$stmt->execute([$email]);
$user = $stmt->fetch();

if ($user && password_verify($password, $user['password']))
{
$_SESSION['user'] = $user;
return true;
}
}

затем используется такой

if ($user->loginUser($_POST['email'], $_POST['password'])) {
header("Location: somewhere");
exit;
} else {
echo"Invalid email or password";
}

2
ответ дан 26 марта 2018 в 04:03 Источник Поделиться

Я согласен с вашими ответами здравого смысла, и несколько других пунктов:

Выбор нескольких строк для одного аккаунта

Похоже, что loginUser() выбирает все строки из таблицы пользователей, у которых значение в Почте столбца совпадает связанный параметр. Предположительно значений в этом столбце являются уникальными, но даже если база данных имеет ограничение на столбец, PHP-код, вероятно, следует разрешить только одну запись, чтобы быть возвращены из запроса и обновить данные сессии. Таким образом, запрос может выбрать одну строку, и вместо того, чтобы позвонить fetch() в while петли, просто позвоните fetchAll() и гарантировать, что возвращаемое значение является массивом, и имеет один и только один элемент.

Тогда код в настоящее время в while заявление, что может быть консолидация на зацикливание над полями, которые могут быть определены в постоянном:

const USER_FIELDS_TO_COPY_TO_SESSION = ['userID', 'fullName' ....];

Затем используйте список полей для присвоения значений:

if($stmt->rowCount() == 1){
$row = $stmt->fetch();
foreach(self::USER_FIELDS_TO_COPY_TO_SESSION as $field) {
$_SESSION[$field] = $row[$field];
}
}

Или если все значения в этой строке можно поставить в сессии, просто использовать array_merge().

if($stmt->rowCount() == 1){
$row = $stmt->fetch();
$_SESSION = array_merge($_SESSION, $row);
}

Проверка

Для проверки, filter_var() может быть использован для проверки и дезинфекции входов - возможно, вы уже знали, что.

Повторяющегося кода

Для блока логики вроде этого:


if($userType == "Investor"){
$accNumber = "A" . sprintf("%06d", mt_rand(1, 999999));
}else{
$accNumber = "B" . sprintf("%06d", mt_rand(1, 999999));
}

Можно утверждать, что избыточность должна быть уменьшена. Один из способов сделать это, чтобы иметь только те ценности, которые изменяются внутри отделения, а затем общий код. Принять следующие переписан пример кода:

$prefix = 'B'; //default
if($userType == "Investor"){
$prefix = "A";
}
$accNumber = prefix . sprintf("%06d", mt_rand(1, 999999));

Таким образом, вызов sprintf() появляется только один раз. Преимущество здесь в том случае, если значения должны быть обновлены (например, значения, которые передают mt_rand()), что потребовало обновления одной линии вместо двух. Пока это блок кода, концепция может применяться к гораздо больших блоков, а также.

А переписывать линия имеет то же количество строк, как и оригинальная, ее можно упростить, используя тернарный оператор:

$accNumber = ($userType == "Investor" ? "A" : "B") . sprintf("%06d", mt_rand(1, 999999));   

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

1
ответ дан 26 марта 2018 в 04:03 Источник Поделиться

Я вижу много СРП нарушение.

setUser() - Я думаю, что имя функции само по себе является неоднозначным и так открыты для ПСП вопросы добавляются. Она должна "установить пользователя", но это может быть все, что угодно.

И есть несколько вещей в этой функции, которая необходима функция, но не должно быть определено там, только просила из других стран.

Например вы делаете $hashedPassword = password_hash()это должна быть другая функция. Как это определить как вы хэш пароля ничего общего с "инструкция setuser"?

function hashThePassword($password)
{
return password_hash($password, PASSWORD_DEFAULT);
}

Тогда вы можете назвать это, когда это необходимо, и если вы когда-нибудь изменить то, как он работает, это одно место, чтобы быть изменен.

Как было отмечено, htmlspecialchars() для браузера выходных (лиц). При сохранении данных, она должна быть безопасной, но "сырой" в том, что это не в стиле или так или иначе организована, потому что вы, возможно, потребуется использовать данные по-разному.

if($userType == "Investor"){ этот пахнет он должен быть в другом месте. Это не "инструкция setuser" но "getUsersAccountNumber", и снова я вижу еще нужна функция:

function getUsersAccountNumber($userType)
{
if ($userType == "Investor") {
return "A" . sprintf("%06d", mt_rand(1, 999999));
} else {
return "B" . sprintf("%06d", mt_rand(1, 999999));
}
}

Это значит что вы можете звонить в другие места и любые будущие изменения будут проведены в одном месте и пульсации вниз. (Что сказал, Я не утверждаю, что понимаю, почему "тип пользователя" позволит определить свой номер счета, и там были только 2 возможных, а).


loginUser():
Мне не нравится имя. Какова задача "логин пользователя"? Существуют различные возможные задачи, так это открыть для ПСП вопросов. На самом деле вы уже проверяем пароль, что эта функция должна делать, но она не должна определить, как проверка пароля делается. Это не задача для "вход в", Поэтому должна быть отдельная функция.

В этой функции я вижу:
$hashedPassword = $this->getPassword($email);

Где находится этот объект от? Я не вижу класса в коде. Кроме того, get password ($email)? Даже если это все законно, то функция должна быть вызвана getHashedPassword()особенно вокруг других данных, как это широко открыты то, что в заблуждение относительно того, какие данные это.


Просто пару намеков. Имя функции, что они делают и только то, что они делают, когда вы пытаетесь добавить что-то, что не вписывается в имени функции, это, наверное, отдельная централизованной и общей функции, что вам нужно написать и позвонить в этот.

Я также предлагаю получить в привычку использовать ПСР2, даже свободно. Вы сможете пользоваться им в дальнейшем.

0
ответ дан 30 марта 2018 в 12:03 Источник Поделиться