1
\$\begingroup\$

I have two links in my header:

  1. Register
  2. Login

I am not creating it dynamically. If the user clicks on "login," the login form will be visible, similarly for the register form.

My registration:

<?php
 include 'configdb.php';
 global $connection;

 if(($_SERVER['REQUEST_METHOD'] === 'POST') && !empty($_POST['email']) && !empty($_POST['password']))
 {
   /*
    *Get Details From User Table To Check Whether User Is Registered Already 
    *Used Prepare Statement To Avoid SQL Injection
    */
    if($getUserData = mysqli_prepare($connection,"SELECT emailID FROM users WHERE emailID = ?"))
    {
       mysqli_stmt_bind_param($getUserData,"s",$_POST['email']);
       mysqli_stmt_execute($getUserData);
       mysqli_stmt_store_result($getUserData);/*It Is Required To Get Number Of Rows By stmt_num_rows Method */
      //mysqli_stmt_fetch($getUserData);
    }
    /*
     *Check Whether ResultSet Is Empty
     *If Not Empty, Return This JSON Error Message
     */
    if (mysqli_stmt_num_rows($getUserData) > 0)
    {
     mysqli_stmt_close($getUserData);  
     die(
          '{
            "status":"1",
            "message":"Email Already Exists"    
           }'
        );
    }
    else
    {
      $email = htmlentities($_POST['email'], ENT_QUOTES, "UTF-8");
      $password = password_hash(htmlentities($_POST['password'], ENT_QUOTES, "UTF-8"), PASSWORD_DEFAULT);
      $com_code = md5(uniqid(rand())); /* It Will Be Used Later For Confirmation-link*/

      if($insertData = mysqli_prepare($connection, "INSERT INTO users (emailID, password, com_code) VALUES(?,?,?)"))
       {
         mysqli_stmt_bind_param($insertData, "sss", $email, $password, $com_code); 
         mysqli_stmt_execute($insertData);
         //print_r($insertData); - Handle error by checking num_rows in the object insertdata
       }
       else
       {
          print_r("Prepared statement error");
       }

       //$_SESSION['username'] = $_POST['email'];
       die('{
          "status":"0",
          "message":"Registration Successful"
        }');
    }
 }  
?>

My Ajax method

/**
* 
* @param form
* 
* AJAX METHOD FOR REGISTER
*/
function callAjaxRegisterSubmitMethod(form)
{
  $.ajax({
     type: "POST",
     url: "lib/registration_validate.php",
     data: $("#id_registerForm").serialize(),
     dataType: 'json',
     beforeSend: function()
     {
        $(".cls_register_login_error label, .cls_register_login_error .error_link").text('');
     },
     success: function(response,status,jqXHR)
            {
                //Show homepage
                if(response.status == 1)
                {
                    $(".cls_register_login_error .cls_register_login_error_message").text(response.message+". Try");
                    $(".cls_register_login_error .error_link").text("login");
                    $(".cls_register_login_error .error_link").addClass("enable_login");
                    $(".cls_register_login_error .error_link").removeClass("enable_register");
                    $(".cls_registerForm input[type='password']").val('');
                }
                else if(response.status == 0)
                {
                    console.log("Registered Successfully");
                    alert("Registered Successfully");
                    //$(".body_content").load("",function(){});
                }
                else 
                {
                    console.log("Common Success Error Page");
                }
            },
    error:function(jqXHR,status,error)
            {
                console.log("Common Error Page");
            },
    complete:function(response)
            {
            }

 });
}

Please let me know of any problems with this approach.

And I am planning to use load method to load my homepage into contnets div because I have fixed header and footer which is common for all the pages.

\$\endgroup\$
  • \$\begingroup\$ Couldn't you include the header and footer in all pages instead of getting the page content using ajax? \$\endgroup\$ – gusper Aug 1 '15 at 8:59
  • \$\begingroup\$ Yea I can do that. But I am not using cookies so far. I have to think about updating user details when navigating to next page \$\endgroup\$ – GoMan Aug 1 '15 at 9:04
1
\$\begingroup\$

htmlentities is used to protect against XSS, and should be used when echoing data, not when inserting it into the database.

Your approach leads to bad data (for example, now you always have to use htmlentities when checking a password, as it will otherwise be wrong), and weak security (using htmlentities before inserting data leads to it not being used when echoing - because it's already been encoded - which leads to data being forgotten, because nobody can remember what was encoded and what was not).

Misc

  • Functions: I would extract code blocks to functions. It makes those blocks reusable and leads to more readable code. Possible functions are eg getUserByEmail and insertUser.
  • rename $com_code to $confirmationCode, so it's clear what it is without reading the comments.
  • always use camelCase or always use snake_case for variables, don't mix this.
  • uniqid and rand are not good at creating random values. Even though a confirmation token isn't that sensitive, I would still use something like openssl_random_pseudo_bytes instead.
  • be more consistent with your spacing (eg after ,).
  • your error handling could be more consistent. Eg sometimes you handle the else case of mysqli_prepare, and sometimes you don't; you never check the result of mysqli_stmt_execute.
  • It is possible that the user will get the following error message: Prepared statement error [...] Registration Successful.
  • your comments are mostly not necessary. And those that are - eg the one about confirmation - would not be with better variable names and code extracted to well named functions (which could then have proper PHPDoc type comments). Having too many comments that don't add any new information is not good, because after a while a reader will just ignore all your comments, even the important ones.
\$\endgroup\$
  • \$\begingroup\$ Wow!! I will do all your suggestions and can I post a new one or edit here itself? \$\endgroup\$ – GoMan Aug 1 '15 at 8:53
  • \$\begingroup\$ @user3168736 see What you may and may not do after receiving answers. So you shouldn't edit your question with updated code, but you can post a self-answer with updated code or post a follow-up question (I would wait a little while though, maybe other people have more suggestions for this question). \$\endgroup\$ – tim Aug 1 '15 at 9:38
  • \$\begingroup\$ Hi Tim, I need your advice also on this question about dynamic content. Will you please help? \$\endgroup\$ – GoMan Oct 3 '15 at 13:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.