Меню
Что нового?

Регистрация пользователей

GatuZa

Новичок
Добрый день! Мой первый пост на форуме! Прошу отнестись снисходительно.
Проблема: функция UserReg не возвращает сообщений об ошибках (например: "Пароли не совпадают!"). Когда пароли не совпадают она не возвращает сообщение. UserReg - метод класса модели Юзеров. Вызывается из контроллера страницы регистрации.

Код контроллера:
PHP: 
<?php
//ini_set('display_errors',1);
//error_reporting(E_ALL);
include_once 'main.php'; 

class reg extends main 
{
    private static $model;
    private $message; // var_dump($this->message) = NULL

    function __construct() 
    {
        $this->title = 'Регистрация';         
        self::$model = users_model::Instance();     
    }
    
    protected function In()
    {                  
        parent::In();
        if ($this->IsPost())
        {            
            if (self::$model->UserReg($_POST['login'], $_POST['pass'], $_POST['pass_c'], $_POST['email']))
            {
                header('Location: index.php'); 
                exit(); 
            }
            else
            {            
                return $this->message = self::$model->UserReg();    
            }
        }
    }
    

    protected function Out() 
    {
        $this->content = $this->SetView('views/reg_tmpl.php', array('message' => $this->message));
        parent::Out();    
    }   
}
Код метода UserReg:
PHP: 
    public function UserReg ($login, $pass, $pass_c, $email) 
    {
          $login = trim($login);
          $pass = trim($pass);
          $pass_c = trim($pass_c);
          $email = trim($email);

        if ($pass !== $pass_c)
        {
            return $res = 'Пароли не совпадают!';
        }
        
        // проверка "существует ли уже такой логин?!"
        $sql = sprintf("SELECT count(*) FROM users WHERE login = '%s'", $login);
        $result = $this->msql->Select($sql);
        if ($result[0]['count(*)'] != 0) 
        {
            return $res = 'Такой пользователь уже существует!';
        }
        
        // проверка "существует ли уже такой email?!" 
        $sql = sprintf("SELECT count(*) FROM users WHERE email = '%s'", $email);
        $result = $this->msql->Select($sql);
        if ($result[0]['count(*)'] != 0)
        {
            return $res = 'Пользователь с таким email уже существует!';
        }
        
        $obj = array();
        $obj['login'] = $login;
        $obj['password'] = md5($pass);
        $obj['email'] = $email;
        
        $this->msql->Insert('users', $obj);
        return TRUE;
    }
Даже если пароли не совпали - выкидывает в index.php.
Помогите пожалуйста.
 
rotoZOOM

rotoZOOM

ACM maniac
Что по-твоему означает вот эта запись?
return $res = 'Пароли не совпадают!';
 

GatuZa

Новичок
rotoZOOM
переменной $res присваиваем строковое значение
___________________________

проблему решил:
PHP: 
            if (($reg = self::$model->UserReg($_POST['login'], $_POST['pass'], $_POST['pass_c'], $_POST['email'])) === TRUE)
            {
                header('Location: index.php'); 
                exit(); 
            }
            else
            {            
                return $this->message = $reg;    
            }
 
Фанат

Фанат

oncle terrible
Команда форума
Классов понаверчено - строго по науке, на каждый чих.
при этом класс работы с sql никак не защищает от инъекций, а в md5() пароли хранить по нынешним временам - это все равно что открытым текстом
и модель почему-то занимается проверками входящих данных.
 

GatuZa

Новичок
rotoZOOM
??? может я чего-то не понимаю, буду рад если Вы объясните.
Фанат
при этом класс работы с sql никак не защищает от инъекций
Нажмите для раскрытия...
в прослойке есть май_риал_эскейпы или этого не достаточно?!))

PHP: 
    public function Insert($table, $object) 
    {     
        $columns = array();
        $values = array();

        foreach ($object as $key => $value) 
        {
            $key = mysql_real_escape_string($key . '');  
            $columns[] = $key;                          
            
            if ($value === null) 
            {
                $values[] = 'null';
            } 
            else 
            {                
                $value = mysql_real_escape_string($value . '');                   
                $values[] = "'$value'";                          
            } 
        }
       
        $columns_s = implode(',', $columns);   
        $values_s = implode(',', $values);   

        $result = mysql_query("INSERT INTO $table ($columns_s) VALUES ($values_s)");
                        
        if (!$result) 
        {
            exit(mysql_error());
        }
        
        return mysql_insert_id();
    }
md5() пароли хранить по нынешним временам - это все равно что открытым текстом
Нажмите для раскрытия...
подскажите пожалуйста как правильно))

модель почему-то занимается проверками входящих данных.
Нажмите для раскрытия...
мне казалось, что модель должна скрывать логику подготовки и передачу данных в БД... на каждую сущность по модели...
Интересно услышать Ваше мнение.
 
rotoZOOM

rotoZOOM

ACM maniac
GatuZa написал(а):
rotoZOOM
??? может я чего-то не понимаю, буду рад если Вы объясните.
Нажмите для раскрытия...
Да я все о своем, о старом. IMHO, не зная элементарных конструкций языка нет смысла погружаться куда то дальше, например в ООП.
Для чего в return идет присвоение значения переменной $res?
 
Фанат

Фанат

oncle terrible
Команда форума
GatuZa написал(а):
в прослойке есть май_риал_эскейпы или этого не достаточно?!))
Нажмите для раскрытия...
Ну, начнём с того, что "май_риал_эскейпы" вообще не предназначены для защиты от инъекций. В названии этой функции нет слов "защита" или "инъекция".
То есть, не то что "недостаточно", а вообще не в тему.

Искейпинг плюс кавычки ближе к истине, но не для "защиты от инъекций", а просто потому что синтаксис так требует - искейпить специальные символы в строках, заключённых в кавычки.
Но я что-то не заметил, чтобы это правило выполнялось в запросе
PHP: 
"SELECT count(*) FROM users WHERE login = '%s'"
Не говоря уже о том, что не всегда мы помещаем данные в запрос в кавычках, и в этом случае "май_риал_эскейпы" вообще никак и ни от чего не защитят.

Пароль надо хранить с солью.
Как минимум,
PHP: 
md5($pass.$email);
мне казалось, что модель должна скрывать логику подготовки и передачу данных в БД
Нажмите для раскрытия...
Совершенно верно.
Но слова "проверка" и "подготовка" имеют разное значение.

Модель должна покрывать работу с данными.
и предоставлять контроллеру методы isLoginExists() и isEmailExists()
проверка же корректности пользовательского ввода - это работа контроллера.
 

GatuZa

Новичок
rotoZOOM
правильно писать вот так?
return('Пароли не совпадают!');

Фанат
ок, спасибо, учту)
покажите пожалуйста правильный пример защиты от инъекций
 
Фанат

Фанат

oncle terrible
Команда форума
GatuZa написал(а):
rotoZOOMправильно писать вот так?
Нажмите для раскрытия...
Правильнее, все-таки, посмотреть в документации, и сделать, как там написано в примере.
Но намек ты ухватил верно, это радует.

Защита от инъекций состоит из двух основных правил.
1. Подставляемые данные передаются через плейсхолдеры (которые, разумеется, должны корректно обрабатываться)
2. Все остальные части запроса, если им случится попасть в запрос из переменных, подставляются из белого списка.

Драйверы mysqli и PDO предосталвяют готовые плейсхолдеры, но с ними возни много.
Для драйвера mysql простой вариант реализации плейсхолдеров будет с использованием механизма printf, который позволит нам передавать данные типа строка или число.
Числа printf сам преобразует к нужному виду, а со строками мы должны будем ему помочь.
Поскольку правило "искейпинг ТОЛЬКО в паре с кавычками" должно быть железным, постановку кавычек мы не можем отдать программисту и должны их ставить сами. Иначе плейсхолдер потеряет всякий смысл
PHP: 
protected function query($args){
  $query = array_shift($args);
  $query = str_replace("%s","'%s'",$query);
  foreach ($args as $key => $val) {
    $args[$key] = mysql_real_escape_string($val);
  }
  $query = vsprintf($query, $args);
  if (!$query) return FALSE;

  $res = mysql_query($query) or trigger_error("db: ".mysql_error()." in ".$query);
  return $res;
}
Недостатки printf-a - нельзя вставить NULL из переменной и нaдо следить, чтобы не было лайков, прописанныз прямо в запросе.

поскольку передавать в функцию массив не очень удобно, сделаем функцию-обертку, которая позволит передавать параметры обычным образом, через запятую.
Ну и заодно сделаем сразу хелпер для получения скаляра из запроса.

PHP: 
public function run() {
    $args = func_get_args();
    return $this->query($args);
}

public function getOne(){
    $args = func_get_args();
    $res  = $this->query($args);
    $row  = $this->fetch_row($res);
    if (!$row) return FALSE;
    return $row[0];
}
Соответственно, в модели пишем
PHP: 
    public function isEmailExists($email) 
    {
        return $this->db->getOne("SELECT 1 FROM users WHERE email = %s", $email);
    }
и благодаря использованию простого хелпера сокращаем количество строк с 6 до одной

Белый список реализуется просто
PHP: 
$orders  = array("name","price","qty"); // собственно, белый список. 
$key     = array_search($_GET['sort'],$orders)); // ищем переданное значение в массиве
$orderby = $orders[$key]; //если не нашли - автоматом выберется первое
$query   = "SELECT * FROM `table` ORDER BY $orderby"; // запрос получился безопасным.
аналогичным образом поступать во всех подобных случаях.
 
Фанат

Фанат

oncle terrible
Команда форума
PHP: 
$key = mysql_real_escape_string($key . '');
Нажмите для раскрытия...
ОМГ, слона-то я и не заметил.

Контрольный вропрос на понимание прочитанного: здесь чудовищного?

PHP: 
( $key. '');
Кстати, а это что такое? Очередное проявление карго-культа?

PHP: 
exit(mysql_error());
А это, кстати, никуда не годится.
 

GatuZa

Новичок
Фанат

Ну, начнём с того, что "май_риал_эскейпы" вообще не предназначены для защиты от инъекций. В названии этой функции нет слов "защита" или "инъекция".
То есть, не то что "недостаточно", а вообще не в тему.
Но я что-то не заметил, чтобы это правило выполнялось в запросе
Нажмите для раскрытия...
Так правильно?)

PHP: 
sql = sprintf ("SELECT count(*) FROM users WHERE login = '%s'", mysql_real_escape_string($login))
Не говоря уже о том, что не всегда мы помещаем данные в запрос в кавычках, и в этом случае "май_риал_эскейпы" вообще никак и ни от чего не защитят.
Нажмите для раскрытия...
Вы имеете ввиду это?!
Если есть время приведите пожалуйста примеры.

PHP: 
        $id_user = intval($id_user);
        $sql = "
            SELECT      us.id_user, us.login, us.password, ro.description, pr.name
            FROM        users AS us
            LEFT JOIN   roles AS ro USING(id_role)
            LEFT JOIN   privs2roles AS p2r USING (id_role)
            LEFT JOIN   privs AS pr USING(id_priv)
            WHERE       us.id_user = $id_user
        ";
PHP: 
( $key. '');
Кстати, а это что такое? Очередное проявление карго-культа?
Нажмите для раскрытия...
это приведение к строке)))

PHP: 
exit(mysql_error());
А это, кстати, никуда не годится.
Нажмите для раскрытия...
почему??)

Модель должна покрывать работу с данными.
и предоставлять контроллеру методы isLoginExists() и isEmailExists()
проверка же корректности пользовательского ввода - это работа контроллера.
Нажмите для раскрытия...
а не проще ли объединить эти два метода в один, чтобы минимизировать к-во обращений к методам модели (ведь я так понимаю чем их меньше - тем быстрее работает приложение?!). Ведь может быть такое, что isLoginExists() прошел, а вот isEmailExists() выдаст FALSE, то есть лишний раз вызвали метод. А так просто всю проверку выполняет один метод модели?! Если я не правильно мыслю прошу поправить.

Насчет корректности пользовательского ввода, в HTML5 есть атрибут required (не допускает отправку пустого поля) + проверка на соответствие типам полей форм (например для type="email" обязательно должна быть собака). Понятно, что все браузеры пока, что не поддерживают эти нововведения, но если предположить, что 100% браузеров будут поддерживать эти фишки, то можно ВООБЩЕ обойтись без проверок в рамках этих возможностей?!
 

GatuZa

Новичок
Мне проще что-то понять на примерах с реальными данными, чем в теории. Поэтому если туплю не обессудьте))
 
Redjik

Redjik

Джедай-мастер
GatuZa
PHP: 
(string)$key;
http://php.net/manual/ru/language.types.type-juggling.php

PHP: 
exit(mysql_error());
А зачем кому-то знать что за ошибка вывалилиась?
На продакшене по-любому забудешь убрать.
Так правильно?)
Нажмите для раскрытия...
С кавычками мудришь, но правильно в принципе.
Хотя могу ошибаться, я препаредами пользуюсь.
 
Sufir

Sufir

Я не волшебник, я только учусь
ведь я так понимаю чем их меньше - тем быстрее работает приложение?!
Нажмите для раскрытия...
Много на спичках сэкономить намерен? Кто тебе это сказал?
HTML5 есть атрибут required (не допускает отправку пустого поля)
Нажмите для раскрытия...
Не доверяй никогда и ничему, что приходит от пользователя.
 
Фанат

Фанат

oncle terrible
Команда форума
GatuZa написал(а):
Фанат
Так правильно?)
Нажмите для раскрытия...
Нет.
У меня написано, как правильно.
Вы имеете ввиду это?!
Нажмите для раскрытия...
И это тоже.
В этом примере переменная приведена к безопасному типу вручную, но нужно сделать так, чтобы этим занималась библиотека, а не программист, который может забыть или перепутать.

Но главное - я не видел ответа на контрольный вопрос.

это приведение к строке)))
Нажмите для раскрытия...
А зачем?
ведь я так понимаю чем обращений к методам модели меньше - тем быстрее работает приложение?!)
Нажмите для раскрытия...
Бред сивой кобылы.
Насчет корректности пользовательского ввода, в HTML5 есть атрибут required
Нажмите для раскрытия...
Какое отношение этот атрибут имеет к проверку существования логина?
 

GatuZa

Новичок
Фанат
Переделал по вашей рекомендации контроллер:

PHP: 
       if ($this->IsPost())
        {   
            // проверка корректности заполнения формы регистрации
            if (!(empty($_POST['login']) OR empty($_POST['pass']) OR empty($_POST['pass_c']) OR empty($_POST['email'])))
            {
                if ($_POST['pass'] !== $_POST['pass_c'])
                {
                    $this->message = 'Пароли не совпадают!';
                }
                else
                {
                    // проверка на идентичные логины и электронные адресса
                    if (self::$model->isLoginExists($_POST['login']))
                    {
                        $this->message = 'Такой логин уже используется!';
                    }
                    else if (self::$model->isEmailExists($_POST['email']))
                    {
                        $this->message = 'Такой электронный адресс уже используется!';
                    }
                    else
                    {
                        // регистрация пользователя в БД
                        self::$model->createUser($_POST['login'], $_POST['pass'], $_POST['email']);
                        header('Location: index.php');
                        exit();
                    }    
                }
            }
            else
            {
                $this->message = 'Все поля формы должны быть заполнены!';
            }
        }
    }
Правильно? Меня смущает обилие управляющих операторов, может можно как-то с меньшим к-ом кода реализовать?!
 
Войдите или зарегистрируйтесь для ответа.
Сверху