Form Trouble / Confirming Passwords?

[scm22ri]

Hi Everyone,

 

I'm having a hard time confirming passwords in my sign up form. My email confirming is working fine but if a user signs up for my website with two different passwords they automatically get signed up. In other words, my error checking isn't taking care of the problem. I've been working on this for quite a while and not sure what the problem is. Any help would be appreciated it. Thanks.

 

http://whatsmyowncar...n/join_form.php

 

<?php
// Set error message as blank upon arrival to page


// This is a function that I can apply to any words/phrases
function letscapthis($element){
$element = strtolower($element);
return ucwords($element);
}
// Above is the function called "letscapthis"

$errorMsg = "";
// First we check to see if the form has been submitted
if (isset($_POST['username'])){
//Connect to the database through our include
include_once "connect_to_mysql.php";
// Filter the posted variables
$username = ereg_replace("[^A-Za-z0-9]", "", $_POST['username']); // filter everything but numbers and letters
$country = ereg_replace("[^A-Z a-z0-9]", "", $_POST['country']); // filter everything but spaces, numbers, and letters
$state = ereg_replace("[^A-Z a-z0-9]", "", $_POST['state']); // filter everything but spaces, numbers, and letters
$state = letscapthis($state);
$city = ereg_replace("[^A-Z a-z0-9]", "", $_POST['city']); // filter everything but spaces, numbers, and letters
$city = letscapthis($city);

$email = stripslashes($_POST['email']);
$email = strip_tags($email);
$email = mysql_real_escape_string($email);

$email2 = stripslashes($_POST['email2']);
$email2 = strip_tags($email2);
$email2 = mysql_real_escape_string($email2);

$password = ereg_replace("[^A-Za-z0-9]", "", $_POST['password']); // filter everything but numbers and letters
$password2 = ereg_replace("[^A-Za-z0-9]", "", $_POST['password2']); // filter everything but numbers and letters
// Check to see if the user filled all fields with
// the "Required"(*) symbol next to them in the join form
// and print out to them what they have forgotten to put in
if((!$username) || (!$country) || (!$state) || (!$city) || (!$email) || (!$email2) || (!$password) || (!$password2) ){

$errorMsg = "You did not submit the following required information!<br /><br />";
if(!$username){
$errorMsg .= "--- User Name";
} if(!$country){
$errorMsg .= "--- Country";
} if(!$state){
$errorMsg .= "--- State";
} if(!$city){
$errorMsg .= "--- City";
} else if($email !== $email2){
$errorMsg = 'ERROR: Your Email fields below do not match<br />';
} else if($password !== $password2){
$errorMsg = 'ERROR: Your Password fields below do not match<br />';
}
} else {
// Database duplicate Fields Check
$sql_username_check = mysql_query("SELECT id FROM members WHERE username='$username' LIMIT 1");
$sql_email_check = mysql_query("SELECT id FROM members WHERE email='$email' LIMIT 1");
$username_check = mysql_num_rows($sql_username_check);
$email_check = mysql_num_rows($sql_email_check);
if ($username_check > 0){
$errorMsg = "<u>ERROR:</u><br />Your User Name is already in use inside our system. Please try another.";
} else if ($email_check > 0){
$errorMsg = "<u>ERROR:</u><br />Your Email address is already in use inside our system. Please try another.";
} else {
// Add MD5 Hash to the password variable
$hashedPass = md5($password);
// Add user info into the database table, claim your fields then values
$sql = mysql_query("INSERT INTO members (username, country, state, city, email, password, signupdate)
VALUES('$username','$country','$state','$city','$email','$hashedPass', now())") or die (mysql_error());

echo 'Thanks for submitting your information.<br /><br />
To return to the homepage, <a href="index.php">click here</a>';

} // Close else after database duplicate field value checks
} // Close else after missing vars check
} //Close if $_POST
?>

Edited October 22, 2012 by scm22ri

[shaddowman]

I think the condition for the email and password should have a seperate IF segment. Based in the code If the city returns true. Password and Email will not be evaluated.

[Psycho]

Not to be rude, but there is a lot a poor execution in that code. But, first let's look at the specific problem you have asked. I know that the new forums are not good about maintaining spacing in the code. So, it was hard to "see" the problem until I reformatted it. The condition to check if the passwords match is wrapped within a condition that only executes if there are missing fields (note the email check is also wrapped in that condition). So, the checks to see fi the passwords match or if the emails match will ONLY occur if there were missing fields int he post data. In other words, if all the fields have values, those check will NOT occur. If the code is properly indented based upon the structure it is pretty easy to see the problem.

 

Some other things:

 

1. The ereg_ functions have been deprecated for years. You should definitely replace them with preg_ functions.

2. I would advise NEVER modifying the user input. Your regular expression could change "#password#" to the very insecure "password". Also, you are excluding certain characters such as the hyphen in data such as country, state, city, etc. That is a perfectly valid character for that type of data. Don't exclude characters without having a valid reason to do so, and when you do fail the validation and make the user fix it. Why are you restricting the input so much anyway? Even the letscapthis() function is a little overboard. Since you are apparently allowing input for other countries forcing the data to ucwords() is not appropriate.

3. You don't trim() anything. So, for some fields just spaces would be treated as valid input.

 

There are other things as well, but that is what jumped out at me

[scm22ri]

The condition to check if the passwords match is wrapped within a condition that only executes if there are missing fields (note the email check is also wrapped in that condition). So, the checks to see fi the passwords match or if the emails match will ONLY occur if there were missing fields int he post data. In other words, if all the fields have values, those check will NOT occur. If the code is properly indented based upon the structure it is pretty easy to see the problem.

 

Correct if I'm wrong but I would have to put my comparison in other if statement? I just tried with all if statements (below) and the code isn't working.

 

 

if((!$username) || (!$country) || (!$state) || (!$city) || (!$email) || (!$email2) || (!$password) || (!$password2) ){


$errorMsg = "You did not submit the following required information!<br /><br />";
if (!$username){
$errorMsg .= "--- User Name<br/>";
} if (!$country){
$errorMsg .= "--- Country<br/>";
} if (!$state){
$errorMsg .= "--- State<br/>";
} if (!$city){
$errorMsg .= "--- City<br/>";
} if ($email !== $email2){
$errorMsg .= 'ERROR: Your Email fields below do not match<br />';
} if ($password !== $password2){
$errorMsg .= 'ERROR: Your Password fields below do not match<br />';
}
}

 

Thanks

Edited October 22, 2012 by scm22ri

[Psycho]

I think you missed the point of what I was saying. All those checks are within an initial check to see if any required fields are missing. So, the check for comparing the email and comparing the password are within the condition that is run when required fields are empty. You would only run those checks if all the required fields have values!

 

I would suggest not nesting separate if() conditions like that as it makes it difficult to "see" the logical flow. You need to think about all the validations that need to be performed and determine the correct order. Are there some validations that if they fail others should not be performed? Those answers will help determine the correct process. Here is one possible solution

 

//Check for all required fields
// Don't need to check email2 and password2 since those will be taken care of in the matching logic
if((!$username) || (!$country) || (!$state) || (!$city) || (!$email) || (!$password) )
{
   //One or more required fields are missing
   $errorMsg = "You did not submit the following required information!<br /><br />";
   if (!$username) {
    $errorMsg .= "--- User Name<br/>";
   }
   if (!$country) {
    $errorMsg .= "--- Country<br/>";
   }
   if (!$state) {
 $errorMsg .= "--- State<br/>";
   }
   if (!$city) {
    $errorMsg .= "--- City<br/>";
   }
   if (!$email) {
    $errorMsg .= "--- Email<br/>";
   }
   if (!$password) {
    $errorMsg .= "--- Password<br/>";
   }
}

//Check if email and passwords match
if(!$errorMsg)
{
   if ($email !== $email2) {
    $errorMsg .= 'ERROR: Your Email fields below do not match<br />';
   }
   if ($password !== $password2) {
    $errorMsg .= 'ERROR: Your Password fields below do not match<br />';
   }
}

//Perform username and email check
if(!$errorMsg)
{
   $sql_username_check = mysql_query("SELECT id FROM members WHERE username='$username' LIMIT 1");
   $sql_email_check = mysql_query("SELECT id FROM members WHERE email='$email' LIMIT 1");
   $username_check = mysql_num_rows($sql_username_check);
   $email_check = mysql_num_rows($sql_email_check);
   if ($username_check > 0) {
    $errorMsg .= "<u>ERROR:</u><br />Your User Name is already in use inside our system. Please try another.";
   }
   if ($email_check > 0) {
    $errorMsg .= "<u>ERROR:</u><br />Your Email address is already in use inside our system. Please try another.";
   }
}

if(!$errorMsg)
{
   //All validations passed - insert the record
}