Может ли этот скрипт быть улучшен?

Question

Хорошо, мне интересно, можно ли улучшить этот код, хранящийся в user_login.php, или я делаю это неправильно.Я сбит с толку, потому что все мои скрипты в приложении имеют длину около 30-40 строк, и мне было интересно, если я что-то упустил.

Этот скрипт вызывается с помощью вызова ajax, как и все остальные в моем приложении, кромедля файлов шаблонов.

<?php

# Ignore
if (!defined('APP_ON')) { die('Silence...'); }

# Gets the variables sent
$user_name = post('user_name');
$user_password = extra_crypt(post('user_password'));

# Check if the user exists
if (!user::check($user_name, $user_password)) { template::bad($lang['incorrect_login']); }

# Logging in
$id = user::get_id($user_name, $user_password);
$u = new user($id);
$u->login();
template::good($lang['correct_login']);

?>

Я собираюсь объяснить это:

# Ignore
if (!defined('APP_ON')) { die('Silence...'); }

Это в основном проверка того, что файл не вызывается напрямую.Каждый URL-адрес перенаправляется в файл index.php, который управляет URL-адресом (es: www.mysite.com/user/view/id/1) и содержит нужный файл.В первых строках этого файла index.php есть define('APP_ON', true);.Индексный файл также инициализирует приложение, вызывающее некоторый набор функций и некоторые классы.

# Gets the variables sent
$user_name = post('user_name');
$user_password = extra_crypt(post('user_password'));

Функция post() управляет восстановлением $ _POST ['user_name'], выполняя некоторые проверки.Функция extra_crypt() просто зашифровывает пароль, используя sha1 и собственный алгоритм.Я использую подготовленный оператор для запросов sql, поэтому не беспокойтесь о экранировании переменных post.

# Check if the user exists
if (!user::check($user_name, $user_password)) { template::bad($lang['incorrect_login']);

Класс user имеет как статические, так и обычные методы.Статические не требуют идентификатора, чтобы начать с, нормальные методы делают.Например, user::check(); проверяет, существуют ли имя пользователя и пароль в базе данных.Класс template имеет только два статических метода (template::bad() и template::good()), которые управляют быстрым диалоговым окном для отправки пользователю без заголовка или нижнего колонтитула.Вместо этого, если вы создаете экземпляр класса $t = new template('user_view'), вызывается шаблон user_view_body.php, и вы можете управлять этой страницей с помощью $t->assign(), которая присваивает шаблону статические переменные, или с помощью $t->loop(), которая запускает цикл и т. Д. Наконец, $langявляется массивом, имеющим несколько общих строк на языке, заданном пользователем.

# Logging in
$id = user::get_id($user_name, $user_password);
$u = new user($id);
$u->login();
template::good($lang['correct_login']);

В конце мы имеем фактический логин.Класс user создается, а идентификатор восстанавливается.Пользователь с таким идентификатором вошел в систему, и мы возвращаем окно сообщения template::good() пользователю.Для любого уточнения напишите комментарий выше.

Answer 1

Прежде всего, функция дайджеста сообщения, такая как sha1, не является функцией шифрования. Поэтому, пожалуйста, удалите crypt из названия функции, чтобы избежать путаницы. Более того, вся эта идея «специального алгоритма» для хранения паролей пугает меня до чертиков. sha256 - лучший выбор, чем sha1, но sha1 не так уж и плох, в конце концов, он все еще одобрен NIST. В отличие от md5, никто не смог сгенерировать коллизию для sha1 (хотя это случится в ближайшие несколько лет). Если вы используете sha1, убедитесь, что соль является префиксом, чтобы предотвратить атаку префикса.

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

Параметризованные библиотеки запросов, такие как PDO и ADODB, очень хороши, и я рекомендовал использовать их. Это отличный пример дезинфекции во время использования.

Ваш шаблон * метод 1013 * может использоваться для очистки для XSS. Smarty поставляется с модулем фильтра вывода htmlspecialchars, который делает это.

Answer 2

С таким большим количеством функций, которые вы не предоставили, трудно сказать, нужно ли что-то исправлять.Вы уверены, что ввод чистый в post()?Безопасны ли звонки из вашей базы данных в user::check()?Используете ли вы сеансовые куки для упрощения пользовательских настроек / хранения информации?Ваш класс template хорошо написан?Когда вы создаете нового пользователя, вы регистрируете всю необходимую информацию (время входа, IP-адрес, если необходимо, и т. Д.)?Вы гарантируете, что пользователь дважды не вошел в систему, если это важно здесь?

Единственное, что я могу вам сказать о том, что вы написали, это то, что алгоритм sha1 полностью нарушен и вместо этого вы должны использовать что-то другое;см. комментарии ниже для предложений.