首页
学习
活动
专区
圈层
工具
发布
首页
学习
活动
专区
圈层
工具
MCP广场
社区首页 >问答首页 >PHP中的网站用户注册脚本

PHP中的网站用户注册脚本
EN

Code Review用户
提问于 2020-11-08 21:13:57
回答 1查看 83关注 0票数 6

我有一个注册系统,但我真的不知道它是否安全,因为我不太了解安全,我害怕损害我的客户的数据,我相信我创建的方法对安全是有益的,但我仍然怀疑它是否安全

我使用封装创建了几个变量,然后使用它们,对吗?还是应该在方法中创建它们?

正则表达式是否足以防止JavaScript攻击?

我的代码中是否有漏洞或错误?

我们是否可以认为,我已经为我的客户的安全创造了有效的方法?

代码语言:javascript
运行
复制
conn = new PDO("mysql:host=$host;dbname=$dbname", $user, $pass);
            $this->conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
            return $this->conn;
        }catch(PDOException $e) {
            throw new Exception($e->getMessage());
        }
    }


    public function setEmail($e) {
        $this->email = $e;
    }

    public function getEmail() {
        return $this->email;
    }

    public function setName($n) {
        $this->name = $n;
    }

    public function getName() {
        return $this->name;
    }

    public function setPassword($password, $confirm) {
        if ($password === $confirm) {
            $this->password = password_hash($password, PASSWORD_BCRYPT);
        } else {
            // Handle input error here
            echo "Password does not match!";
        }
    }

    public function getPasswordHash() {
        return $this->password;
    }


    // public function CheckHashes() {
    //     echo $this->password;
    // }

    public function CheckName(){
        $this->check_name = $this->conn->prepare("SELECT name FROM users WHERE name = :name");
        $this->check_name->execute(array(
            ":name" => $this->getName()
        ));

        $this->line = $this->check_name->rowCount();

        if($this->line != 0){
            $_SESSION['msg'] = 'Name already exists!';
            header("Location: signup.php");
            exit();
        }
    }

    public function CheckEmail(){
        $checkmail = $this->conn->prepare("SELECT email FROM users WHERE email = :email");
        $checkmail->execute(array(
            ":email" => $this->getEmail()
        ));

        $theline = $checkmail->rowCount();

        if($theline != 0){
            $_SESSION['msg'] = 'Email already exists!';
            header("Location: signup.php");
            exit();
        }
    }

    public function CheckFields(){
        if(empty($this->password) || empty($this->name) || empty($this->email)){
            $_SESSION['msg'] = 'Please, fill out all the fields!';
            header("Location: signup.php");
            exit();
        }
    }

    public function ValidateNameLength(){
        if(strlen($this->getName()) < 10){
            $_SESSION['msg'] = 'Can only use 10 or more characters as name!';
            header("Location: signup.php");
            exit();
        }

        if(strlen($this->getName()) > 25){
            $_SESSION['msg'] = 'Can only use a maximum of 25 characters as name!';
            header("Location: signup.php");
            exit();
        }
    }

    public function ValidateEmail(){
        if(!filter_var($this->getEmail(), FILTER_VALIDATE_EMAIL)){
            $_SESSION['msg'] = 'Incorrect email!';
            header("Location: signup.php");
            exit();
        }
    }
    
    public function Insert() {
        if (preg_match("/^[A-Za-z0-9\s]*$/", $this->getName()) ) {
                $this->sql = "INSERT INTO users (name, email, pass, ip_address, http_client_ip, http_x_forwarded_for, http_user_agent) VALUES(:name, :email, :pass, :ip_address, :http_client_ip, :http_x_forwarded_for, :http_user_agent)";
                $this->result = $this->conn->prepare($this->sql);
                $this->remote_addr = $_SERVER['REMOTE_ADDR'];
                $this->http_client_ip = $_SERVER['HTTP_CLIENT_IP'];
                $this->http_x_forwarded_for = $_SERVER['HTTP_X_FORWARDED_FOR'];
                $this->http_user_agent = $_SERVER['HTTP_USER_AGENT'];
                $this->result->execute(array(
                    ":name" => $this->getName(),
                    ":email" => $this->getEmail(),
                    ":pass" => $this->getPasswordHash(),
                    ":ip_address" => $this->remote_addr,
                    ":http_client_ip" => $this->http_client_ip,
                    ":http_x_forwarded_for" => $this->http_x_forwarded_for,
                    ":http_user_agent" => $this->http_user_agent
                ));

                if($this->result == true){
                    $_SESSION['msg'] = 'Registered successfully!';
                    header("Location: signin.php");
                    exit();
                }
        }

        else {
            $_SESSION['msg'] = 'You can only use letters and numbers as name!';
            header("Location: signup.php");
            exit();
        }

    }
}

$obj = new SignUp('localhost', 'dbname', 'user', 'password');
$obj->setEmail('email@gmail.com');
$obj->setName('Anne');
$obj->setPassword('password', 'password');
$obj->CheckName();
$obj->CheckEmail();
$obj->CheckFields();
$obj->ValidateNameLength();
$obj->ValidateEmail();
$obj->Insert();
// $obj->getEmail();
// echo $obj->CheckHashes();
// echo "\n";
EN

回答 1

Code Review用户

回答已采纳

发布于 2020-11-09 11:30:54

安全

你主要担心的是保安。在您的代码中,我没有看到任何明显的安全问题。要判断您的代码是否真的安全,我需要查看整个系统,而您只提供了其中的一部分。这片看起来很安全,这就是我所能说的。

编码风格

您的代码有一些实际的样式问题。在我看来你是新来的。我会处理从大到小的问题。

Code重复

有相当多的代码重复。两个明显的例子是:

1:设置错误消息,跳转到signup.phpexit()。可能有这样一种方法:

代码语言:javascript
运行
复制
private function returnWithError($errorMessage)
{
    $_SESSION['error'] = $errorMessage;
    header('Location: ' . $this->signUpPage);
    exit();
} 

我将在错误消息中省略HTML标记。HTML应该由将错误输出给用户的代码添加。这更有意义,因为格式化错误的是输出代码的责任。我会再谈这件事的。

2:方法CheckName()CheckEmail()非常相似。他们都可以使用这样的方法:

代码语言:javascript
运行
复制
private function existingColumnValue($columnName, $value)
{                                                               
    $statement = $this->database->prepare("SELECT userId FROM users WHERE $columnName = :value");  
    $statement->execute([':value' => $value]);
    return $statement->rowCount() > 0;
}

此方法只检查列中是否存在值。它可以用于users表中的所有列。它不是重定向和退出,而是返回一个布尔值。现在您可以将CheckName()CheckEmail()重写为:

代码语言:javascript
运行
复制
public function existingName($name)
{            
  return existingColumnValue('name', $name);                                                   
}

public function existingEmail($email)
{            
  return existingColumnValue('email', $email);                                                   
}

责任

现在,让我们深入研究这个类要做的事情。主要关注的是:是否接受新用户进入数据库。这是它的责任,同样也是如此。因此,设置数据库连接、重定向或格式化输出不应该是该类的一部分。理想情况下,页面底部的代码应该如下所示:

代码语言:javascript
运行
复制
$database = new PDO(.....);
$users    = new Users($database);
if ($users->isValidNewUser($name, $email, $password)) {
    $userId = $users->addUser($name, $email, $password);
}
else {
    $_SESSION['error'] = $users->getLastError();
    header('Location: signup.php');
}

我根据它与之交互的数据库表命名了这个类。数据库句柄现在在这个类之外创建。这完全取决于你。我使用该类的两种方法:isValidNewUser()addUser()。这还指向该类的未来扩展,包括getUser()removeUser()等方法。所以Users类现在所做的就是与数据库交互。当然,您想知道isValidNewUser()addUser()是什么样子的:

代码语言:javascript
运行
复制
private function setError($errorMessage)
{
   $this->errorMessage = $errorMessage;
   return FALSE;
}

public function getLastError()
{
   return $this->errorMessage;
}

public function isValidNewUser($name, $email, $password)
{
  if (empty($name)) return setError('Please supply a name');
  if (empty($email)) return setError('Please supply an email address');
  if (empty($password)) return setError('Please supply a password');
  if (!this->isValidName($name)) return setError('Invalid name. The name should be between 10 and 25 characters. You can only use letters and numbers.');
  if ($this->existingName($name)) return setError('This name is already taken, please choose another');
  if ($this->existingEmail($email)) return setError('This email address is already registered');
  return TRUE;
}

public function addUser($name, $email, $password)
{
  $statement = $this->database->prepare("INSERT INTO users (name, email, pass, ip_address, http_client_ip, http_x_forwarded_for, http_user_agent) 
                                         VALUES(:name, :email, :password_hash, :ip_address, :http_client_ip, :http_x_forwarded_for, :http_user_agent)");  
  $statement->execute([':name'                 => $name,
                       ':email'                => $email,
                       ':password_hash'        => password_hash($password, PASSWORD_BCRYPT),
                       ':ip_address'           => $_SERVER['REMOTE_ADDR'],
                       ':http_client_ip'       => $_SERVER['HTTP_CLIENT_IP'],
                       ':http_x_forwarded_for' => $_SERVER['HTTP_X_FORWARDED_FOR'],
                       ':http_user_agent'      => $_SERVER['HTTP_USER_AGENT']]);
  return $this->database->lastInsertID();
}

通过使用两种方法,如果愿意,现在可以跳过验证。如果您想从其他地方导入用户,并且您知道列表是无错误的,这可能很有用。但是,即使使用验证,它也不会重定向,它只设置错误消息。我还没有写出isValidName()方法,因为这个代码示例已经很长了。

<#>Minor其他问题

  • $this->conn返回__construct()。构造函数总是返回它构造的类,因此其他东西的返回将无法工作。
  • 您可以将name参数缩写为$n,将email参数缩写为$e。为什么?$name$email不是更好吗?
  • 使用$this->password存储密码哈希。这样做并不少见,但令人困惑。$this->password里有什么?是密码还是散列?为什么不叫它$this->passwordHash呢?
  • 在不需要类级变量的情况下,可以使用大量的类级变量,例如$this->check_name中的CheckName()。更多的是,在CheckEmail(),你不会那么做。
  • 我认为在方法名称中使用Check并不能很好地描述该方法的功能。
  • 存在一个短数组语法,它将array()替换为[]。我更喜欢它,但是array()也是这样做的。

对未来的想法

有一个问题我还没有解决:通过将用户数据的验证与insert例程分开,我们就会面临这样的风险:这将无法正常工作。这是不太可能的,但假设两个不同的人,同名,完全同时注册。两种方法都可以根据数据库检查它们的名称,但都找不到,然后都插入。这些不太可能发生的事件往往发生在非常繁忙的地点。这些类型的错误很难调试。

解决这个问题有几种方法。一个经典的方法是使用转换。另一种方法是使用条件插入查询:“如果这个名称不存在,那么插入这个数据。”

不管怎样,我觉得你应该意识到这个小问题。

票数 5
EN
页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接:

https://codereview.stackexchange.com/questions/251835

复制
相关文章

相似问题

领券
问题归档专栏文章快讯文章归档关键词归档开发者手册归档开发者手册 Section 归档