Обзор на мою нынешнюю структуру и советы по улучшению


Я хотел бы советы на мой код, структура карты (как я должен отделить код, файлы и т. д.). Что я могу сделать о файле func.php как файл с кучей функций, не кажется лучшим подходом для этого. И вообще просто анализ кода, как я могу улучшить его.

createaccount.php

<?php
require_once 'init.php';

if (isset($_POST['submit'])) {
    if (isset($_SESSION['token']) && $_POST['token'] == $_SESSION['token']) {

        $errors = array();

        if (!empty($_POST['username'])) {

            if (strlen($_POST['username']) < 3 || strlen($_POST['username']) > 20) {
                $errors[] = 'The username must be between 3 and 20 characters long';
            }

            if (!preg_match("/^[_a-zA-Z0-9]+$/", $_POST['username'])) {
                $errors[] = 'The username must contain only letters and numbers and _';
            }

            $STH = $DBH->prepare("SELECT username FROM users WHERE username = ?");
            $STH->execute(array($_POST['username']));

            if ($STH->rowCount() > 0) {
                $errors[] = 'The username is already taken';
            }

        } else {
            $errors[] = 'The username field is required';
        }

       if (!empty($_POST['password'])) {

           if (strlen($_POST['password']) < 4) {
               $errors[] = 'The password must be at least 4 characters long';
           }

           if ($_POST['password'] != $_POST['passconf']) {
               $errors[] = 'The passwords must match';
           }

       } else {
           $errors[] = 'The password field is required';
       }

       if (!empty($_POST['email'])) {

            if (!filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) {
                $errors[] = 'Must be a valid email adress';
            } else {

                $STH = $DBH->prepare("SELECT email FROM users WHERE email = ?");
                $STH->execute(array($_POST['email']));

                if ($STH->rowCount() > 0) {
                    $errors[] = 'The email adress is already taken';
                }
            }

       } else {
           $errors[] = 'The email adress is required';
       }

        $resp = recaptcha_check_answer (PRIVATE_KEY,
                        $_SERVER["REMOTE_ADDR"],
                        $_POST["recaptcha_challenge_field"],
                        $_POST["recaptcha_response_field"]);

        if (!$resp->is_valid) {
            $errors[] = 'The reCAPTCHA wasn\'t entered correctly. Go back and try it again.';
        }

        if (empty($errors)) {
            $salt = salt();
            $password = sha1(sha1($_POST['password'] . $salt));
            $activation_key = md5($_POST['username'] . $_POST['email']);

            $STH = $DBH->prepare("INSERT INTO users (username, password, salt, email, activation_key, created) VALUES(:username, :password, :salt, :email, :activation_key, :created)");
            $STH->bindParam(':username', $_POST['username']);
            $STH->bindParam(':password', $password);
            $STH->bindParam(':salt', $salt);
            $STH->bindParam(':email', $_POST['email']);
            $STH->bindParam(':activation_key', $activation_key);
            $STH->bindParam(':created', time());
            $STH->execute();

            if ($DBH->lastInsertId()) {
                header("Location: activate.php");
                exit;
            } else {
                // Log
            }
        }

    }
}

$token = md5(uniqid(rand(), true));
$_SESSION['token'] = $token;

?>

<!DOCTYPE html>
<html>
    <head>
        <title>Heroes of Legacy - Create Account</title>
    </head>
    <body>
        <?php
        if (isset($errors)) {
            foreach ($errors as $error) {
                echo $error . "<br />\n";
            }
        }
        ?>
        <form method="post" action="createaccount.php">
            <input type="hidden" name="token" value="<?php echo $token; ?>" />
            Username <input type="text" name="username" value="" />
            Password <input type="password" name="password" value="" />
            Confirm Password <input type="password" name="passconf" value="" />
            Email Adress <input type="email" name="email" value="" />
            <?php echo recaptcha_get_html(PUBLIC_KEY); ?>
            <input type="submit" name="submit" value="Create Account" />
        </form>
    </body>
</html>

init.php

<?php
session_start();

require_once 'config.php';
require_once 'func.php';

try {
  $DBH = new PDO("mysql:host=$hostname;dbname=$dbname", $username, $password);
}
catch(PDOException $e) {
    echo $e->getMessage();
}

function salt() {
    mt_srand(microtime(true)*100000 + memory_get_usage(true));
    return md5(uniqid(mt_rand(), true));
}

require_once 'lib/recaptchalib.php';

define('PUBLIC_KEY', '');
define('PRIVATE_KEY', '');

config.php

<?php

$hostname = 'localhost';
$username = 'root';
$password = '';
$dbname = 'hol';

login.php

<?php
require_once 'init.php';

if (logged_in()) {
    header("Location: index.php");
    exit;
}

if (isset($_POST['submit'])) {
    if (isset($_SESSION['token']) && $_POST['token'] == $_SESSION['token']) {

        $errors = array();

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

            $STH = $DBH->prepare("SELECT id, salt, password, banned FROM users WHERE username = ?");
            $STH->execute(array($_POST['username']));

            if ($STH->rowCount() > 0) {
                $user = $STH->fetch();

                $passconf = sha1(sha1($_POST['password'] . $user['salt']));

                if ($passconf != $user['password']) {
                    $errors[] = 'The password you entered is incorrect';
                    unset($user);
                } else {

                    session_regenerate_id();
                    $_SESSION['userid'] = $user['id'];
                    $_SESSION['hash'] = sha1($_SESSION['userid'] . $_SERVER['HTTP_USER_AGENT']);
                    unset($user);
                    header("Location: index.php");
                    exit;
                }

            } else {
                $errors[] = 'The username is incorrect';
            }

        } else {
            $errors[] = 'You must enter a username and password!';
        }

    }
}

$token = md5(uniqid(rand(), true));
$_SESSION['token'] = $token;

?>

<!DOCTYPE html>
<html>
    <head>
        <title>Heroes of Legacy - Log in</title>
    </head>
    <body>
        <?php
        if (isset($errors)) {
            foreach ($errors as $error) {
                echo $error . "<br />\n";
            }
        }
        ?>
        <form method="post" action="login.php">
            <input type="hidden" name="token" value="<?php echo $token; ?>" />
            Username <input type="text" name="username" value="" />
            Password <input type="password" name="password" value="" />
            <input type="submit" name="submit" value="Log in" />
        </form>
    </body>
</html>

func.php

<?php
function logged_in() {
    if (isset($_SESSION['userid']) 
        && isset($_SESSION['hash'])) {

        $session_check = sha1($_SESSION['userid'] . $_SERVER['HTTP_USER_AGENT']);

        if ($_SESSION['hash'] == $session_check)
            return true;
    }
    return false;
}

function logout() {
    session_unset();
    session_destroy();
    header("Location: index.php");
    exit;
}

Вот об этом.

-- Обновление:

<?php
//Temporary solution
$request = trim(strtolower($_REQUEST['username']));
usleep(150000);

//Let's say this is data grabbed from a database...
$users = array('asdf', 'Peter', 'Peter2', 'George');
$valid = 'true';

foreach($users as $user) {
    if( strtolower($user) == $request )
        $valid = 'false';
}
echo $valid;
?>


290
1
php
задан 30 ноября 2011 в 09:11 Источник Поделиться
Комментарии
1 ответ

Там, кажется, изрядная доля паранойи в ваш код:

function salt() {
mt_srand(microtime(true)*100000 + memory_get_usage(true));
return md5(uniqid(mt_rand(), true));
}

здесь mt_srand(микровремени(правда)*100000 + помощью функции memory_get_usage(истина)); не используется. Если вы собираетесь использовать его, вы должны упростить его, вы не получите лучше соль таким образом.

$password = sha1(sha1($_POST['password'] . $salt));

Здесь вы хэш пароля + соли в два раза. Что не дает больше безопасности, чем просто хэширования раз. Не говоря уже о том, что соль уже обсуждали, что также является избыточным.

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

if (strlen($_POST['username']) < 3 || strlen($_POST['username']) > 20) 

Было бы лучше сделать что-то вроде:

$lengthUsername = $_POST['username'];
if (lengthUsername < 3 || lengthUsername > 20) {

чтобы не вызвать два раза функцию. Проблема производительности мизер, но вы должны избежать повторения кода в любом случае. В если (использования isset($_POST, где['отправить'])) { ... } это слишком долго. Вы могли переместить некоторые функции (скажем, базы данных, прочее) в функцию и просто вызывать функцию в если заблокировать.


Что я могу сделать о файле func.php как файл с кучей функций, не кажется лучшим подходом для этого.

Это зависит от того. Если вы используете все, что в нем везде, то это нормально. Если не следует тормозить его на более мелкие коллекции файлов и включать по мере необходимости. Но поскольку вы делаете это процедурный путь, это нормально иметь одну или несколько коллекций функции. Вы не рассматривали ОО подхода? Я не говорю, что вы должны, просто любопытно.

Кроме этого, ваш код выглядит нормально. Спасибо за использование PDO и подготовленные операторы, что это мудрое решение. Также хорошо, что вы призываете покинуть после местоположения(), место() завершится с уведомлением, если заголовки уже отправлены.


Что касается обновления:


  • Зачем спать вообще?

  • Этот код достаточно мал, он будет излишним переписывать в ОО

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

class User {

function usernameExists($username) {
$username = trim(strtolower($username);
$usernames = $this->getUsernames();

foreach($users as $user) {
if(strtolower($user) == $request) {
return false
}
}

return true;
}

function getUsernames() {
// return list of usernames from db
}

}

и вы назвали бы в скрипт AJAX, как:

include("User.php");

$user = new User();

if($user->usernameExists($_REQUEST['username'])) {
echo "false";
exit;
}

echo "true"

Код-это намного больше, чем при процедурном стиле, но вы можете использовать пользователя::usernameExists() везде сейчас, если вам это нужно. Также обратите внимание, что я толкнул strtolower в класс, если вы используете его в другом месте такое сравнение нормировок и возможные проверки должны быть в классе метод не в вызывающем коде.

2
ответ дан 30 ноября 2011 в 11:11 Источник Поделиться