I have two links in my header:
- Register
- 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.
-
\$\begingroup\$ Couldn't you include the header and footer in all pages instead of getting the page content using ajax? \$\endgroup\$gusjap– gusjap2015年08月01日 08:59:33 +00:00Commented Aug 1, 2015 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\$Gibbs– Gibbs2015年08月01日 09:04:45 +00:00Commented Aug 1, 2015 at 9:04
1 Answer 1
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
andinsertUser
. - 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
andrand
are not good at creating random values. Even though a confirmation token isn't that sensitive, I would still use something likeopenssl_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 ofmysqli_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.
-
\$\begingroup\$ Wow!! I will do all your suggestions and can I post a new one or edit here itself? \$\endgroup\$Gibbs– Gibbs2015年08月01日 08:53:47 +00:00Commented Aug 1, 2015 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– tim2015年08月01日 09:38:25 +00:00Commented Aug 1, 2015 at 9:38
-