sign up issues with username/email validation

[Oasweaz]

the aim of this code is to make this sign up users and check if they 
werr already signed up before

with the code below, this is what happens.
on signup, it validates email and username that are already signedup which
works just fine.

PROBLEM: it takes(sign's up) a new username and flags an existing email(email already exists)
also, it takes in(sign's up) an new email and flags an existing username(username already exist)
but also signup new emails and new usernames both.
i have been trying to make this work to no avail.
how do i make it check first and validate first before signing up any data

 

first code is for is signup page

second code for controller 

<?php

require "function2.php";

$errors = array();

if($_SERVER['REQUEST_METHOD'] == "POST"){
    $errors = signup($_POST);

    if(count($errors) == 0){
        header("Location:verify.php");
        // echo "Account Success!!!";
        // $errors[] = "Account Success!!!";
        die;
    }

}


?>

<?php

require "function2.php";

$errors = array();

if($_SERVER['REQUEST_METHOD'] == "POST"){
    $errors = signup($_POST);

    if(count($errors) == 0){
        header("Location:verify.php");
        // echo "Account Success!!!";
        // $errors[] = "Account Success!!!";
        die;
    }

}


?>

<?php

session_start();

function signup($data){
	$errors = array();


#validate

	if(!preg_match('/^[a-zA-Z]+$/', $data['username'])){
		$errors[] = "Alert!!!, username must be letters with no space";
	}

	if(!filter_var($data['email'], FILTER_VALIDATE_EMAIL)){
		$errors[] = "Alert!!!, enter correct email address";
	}

	if(strlen(trim($data['password1'])) < 5){
		$errors[] = "Alert!!!, password must be more than 5 characters";
	}


	if($data['password1'] != $data['password2']){
		$errors[] = "Alert!!!, password do not match";
	}

	
//save to db

	if(count($errors) == 0){
		$arr['username'] = $data['username'];
		$arr['email'] = $data['email'];
		$arr['password1'] = $data['password1'];
		$arr['date'] = date("Y-m-d H-i-m");
	
		

#check email/username does exists


	$con = mysqli_connect("localhost", "root", "", "stervest_db" );

	if(isset($_POST['insert'])){

	$email = $_POST['email'];
	$username = $_POST['username'];



			$a = "select username from user2 where username = '$username' limit 1";
			$aa =mysqli_query($con, $a);

		if(mysqli_num_rows($aa)>0){
				$errors[] = "Alert!!, username already exists";
			
		}
			$b = "select email from user2 where email = '$email' limit 1";
			$bb =mysqli_query($con, $b);


			if(mysqli_num_rows($bb)>0){
				$errors[] = "Alert!!, email already exists";
			
		}

	}	


		$query ="insert into user2(username, email, password1, date) values(:username, :email, :password1, :date)";
		$row = signup_push($query, $arr);

	
}
//var_dump($data);


	return $errors;
}


function signup_push($query, $vars = array()){
	
	$string ="mysql:host=localhost;dbname=stervest_db"; 
	$conn = new PDO($string, 'root', '');

	if(!$conn){
		return false;
	}

	$stm  = $conn->prepare($query);
	$check = $stm->execute($vars);

	if($check){

		$data = $stm->fetchAll(PDO::FETCH_OBJ);
		if(count($data)>0){
			return $data;
		}

	}

	return false;
}


function checked_verified(){
		$vars = array();

	$vars['email']= $data['email'];
	$vars['email_verified'] = $_SESSION['USER']->email_verified;

	$query = "select * from user2 where email = :email && email_verified = :email_verified limit 1";

	$row = signup_push($query, $vars);
}

<?php

require "function2.php";

$errors = array();

if($_SERVER['REQUEST_METHOD'] == "POST"){
	$errors = signup($_POST);

	if(count($errors) == 0){
		header("Location:verify.php");
		// echo "Account Success!!!";
		// $errors[] = "Account Success!!!";
		die;
	}

}




?>

 

Edited November 28, 2022 by Oasweaz
mis-order

[Barand]

Don't check if a user exisits by first running a query. Instead, you should put a UNIQUE constraint on the columns and trap duplicate key errors when they occur on inserting a record.

Why a mix of mysqli and PDO. Just stick with PDO.

Don't create a new connection for each query as you do in you signup_push function - that will really slow you down. Create the connection once and pass it in the function arguments.

INSERT, UPDATE and DELETE queries do not create result sets. You are inserting then using fetchAll() to count non-existant results. PDO::row_count() will tell you how many were affected by the query.

[Oasweaz]

24 minutes ago, Barand said:

Don't check if a user exisits by first running a query. Instead, you should put a UNIQUE constraint on the columns and trap duplicate key errors when they occur on inserting a record.

Why a mix of mysqli and PDO. Just stick with PDO.

Don't create a new connection for each query as you do in you signup_push function - that will really slow you down. Create the connection once and pass it in the function arguments.

INSERT, UPDATE and DELETE queries do not create result sets. You are inserting then using fetchAll() to count non-existant results. PDO::row_count() will tell you how many were affected by the query.

Wow that makes sense.. i really got good headsup there with me creating two separate connections.. i was trying to set a form of indirect loop of connection that runs separate for checking and updating rows.

 

Do you mind giving a way of giving a UNIQUE constraint on the columns and trap duplicate key errors when they occur on inserting a record.

[Barand]

There's an example here, courtesy of @Psycho

[ginerjm]

Perhaps if you found a decent mysql reference manual you'll see samples of both items you are asking about.

[Barand]

1 hour ago, Oasweaz said:

Do you mind giving a way of giving a UNIQUE constraint on the columns

This example ensures every student has a unique matriculation number

CREATE TABLE `student` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `firstName` varchar(50) NOT NULL,
  `lastName` varchar(50) NOT NULL,
  `matricNo` varchar(50) NOT NULL,
  `DOB` date NOT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `idx_student_matricNo` (`matricNo`)
) ;

 

[maxxd]

This is not related to your actual question, but I'd feel remiss if I didn't point out that you're storing plain-text passwords in your database right now. Look at https://www.php.net/manual/en/function.password-hash.php and https://www.php.net/manual/en/function.password-verify.php. In the long run, your customers will thank you.

[Oasweaz]

On 11/28/2022 at 4:08 PM, Barand said:

This example ensures every student has a unique matriculation number

CREATE TABLE `student` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `firstName` varchar(50) NOT NULL,
  `lastName` varchar(50) NOT NULL,
  `matricNo` varchar(50) NOT NULL,
  `DOB` date NOT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `idx_student_matricNo` (`matricNo`)
) ;

 

This refers to an sql statement as to the actual php in context of my question.. I really appreciate your input. But a little bit more help is needed as to my question..

[Oasweaz]

On 11/29/2022 at 12:40 AM, maxxd said:

This is not related to your actual question, but I'd feel remiss if I didn't point out that you're storing plain-text passwords in your database right now. Look at https://www.php.net/manual/en/function.password-hash.php and https://www.php.net/manual/en/function.password-verify.php. In the long run, your customers will thank you.

Ok.. am taking a look thank you 👍

[mac_gyver]

you have far too much code for this task. it is filled with repetitive logic, copying of variables to other variables for nothing, and while it is adding user/validation errors to an array, it isn't testing that array for the main activity of executing the INSERT query, which is why it is always inserting the new data values. i recommend that you start over, with just the logic you need - Keep It Simple (KISS) and Don't Repeat Yourself (DRY.)

you should make one database connection in your main code, then supply it to any function that needs it, as a call-time parameter. you should use the much simpler and more modern PDO database extension. you should also use exceptions for database statement errors (the PDO extension always uses exceptions for any connection error and starting with php8 always uses exceptions for the rest of the statements that can fail - query, prepare, execute, and exec.)

your post method form processing code should -

1. detect if a post method form has been submitted
2. keep all the form data as a set, in a php array variable
3. trim all the input data at once. after you do item #2 on this list, you can trim all the data using one single line of code
4. validate each input separately, storing user/validation errors in an array, using the field name as the array index
5. after the end of the validation logic, if there are no errors (the array holding the errors will be empty), use the submitted form data
6. as already stated, the correct way of determining if uniquely defined database columns/fields are duplicate, is to just attempt to insert the data and test if the query produced a duplicate index error number in the exception catch logic. for all other error numbers, just rethrow the exception and let php handle it. since you have more than one unique field, it is at this point where you would execute a (one) SELECT query to find which fields contain duplicate values. you would add error messages to the array holding the user/validation errors for each field that is found to contain duplicate values. 
7. after the end of post method form processing logic, if there are no errors, redirect to the exact same url of the current page to cause a get request for the page
8. if you want to display a one-time success message, store it in a session variable, then test, display, and clear that session variable at the appropriate location in the html document.
9. if there are errors at step #5 or #7, the code will continue on to display the html document, display any errors, redisplay the form, populating the fields with any existing form data.
10. any dynamic value that gets output in a html context needs to have htmlentities() applied to it to help prevent cross site scripting.

[Psycho]

On 12/2/2022 at 11:17 AM, Oasweaz said:

This refers to an sql statement as to the actual php in context of my question.. I really appreciate your input. But a little bit more help is needed as to my question..

The query that Barand provided to create a table shows how to create a table in the database with a unique constrain on a field. His earlier response where he linked to an old post of mine showed how you would execute an INSERT query against a table with a unique constraint. So, let's restate all this another way:

You need to configure your table such that the username and email fields have a unique constraint. That will prevent the DB from accepting any INSERT\UPDATE requests that would generate duplicate values in either of those fields. But, in doing so, any such queries would fail and you need to execute your logic a little differently.

Step 1 is to set up those fields in the DB to be unique. The query Barand provided would create a new table from scratch. You could also create an update query for the table to modify its structure for those unique constraints (e.g. "ALTER TABLE `table_name` ADD UNIQUE(`field_name`);"). Or, if you are like me, I just use PHP Admin's console to modify tables. Open the Table in PHP MyAdmin and select the "Structure" tab. In the list of fields, there is a column on the right where the label UNIQUE is shown. Just click that label to make that field unique in the table.

Step 2. Now that those fields are unique IF someone was to try and create a new profile with a username or email address that already exists in the table, the query will fail. So, you need to approach the INSERT logic differently. The code Barand linked to previously provides an example. Basically, you want to wrap the INSERT (or an UPDATE) query within a try{} block. Then, if it fails, you will have a catch{} block to determine IF the failure was due to a duplicate constraint or not. Now, your case is a little unique since there will be two potential unique constraints and you will want to advise the user "which" of the fields (or both) are a duplicate, so you need more logic than the example provided.

Here is some sample code. Note that I have not tested it and expect I may have made some typos, but the point is about showing what the logic would look like

$pdo = new PDO("mysql:host=$hostname;dbname=$databasename;charset=utf8", $username, $password);
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); // Enables exception mode

try {
    //Attempt to execute the INSERT query
    $query ="INSERT INTO user2(username, email, password1, date) values(:username, :email, :password1, :date)";
    $stmt = $pdo->prepare($query);
    $stmt->execute($arr);
} catch (PDOException $e) {
    //Insert failed, check if it was due to duplicate constraint
    if ($e->getCode() == 1062) {
        // Duplicate username and/or email address - figure out which one(s)
        $query = "SELECT SUM(username=:username) as dupe_username,
                         SUM(email=:email) as dupe_email
                 FROM `photos`"
        $stmt = $pdo->prepare($query);
        $stmt->execute($inputArr); //This arr should only have the username & email values
        $result = $stmt->fetchAll(PDO::FETCH_ASSOC);
        $errors = "";
        if($result['dupe_username']) { $errors .= "Username must be unique"; }
        if($result['dupe_email']) { $errors .= "Email address must be unique"; }
    } else {
        //Some other error occured
        throw $e;// in case it's any other error
    }
}

 

[Oasweaz]

Wow.. this is the biggest eye opener have ever seen.. you took your time to write all these.. wow.. amazing.. the theory makes a million Sense.. 

 

Do you mind helping out with q practical view of the aspect of... Setting a key trap that checks for a duplicate username and email already signed up in the db. Since you made it clear that I should maintain my PDO class... I might come up with other questions.. thanks a million times

 

 

[Oasweaz]

On 12/6/2022 at 5:11 PM, Psycho said:

The query that Barand provided to create a table shows how to create a table in the database with a unique constrain on a field. His earlier response where he linked to an old post of mine showed how you would execute an INSERT query against a table with a unique constraint. So, let's restate all this another way:

You need to configure your table such that the username and email fields have a unique constraint. That will prevent the DB from accepting any INSERT\UPDATE requests that would generate duplicate values in either of those fields. But, in doing so, any such queries would fail and you need to execute your logic a little differently.

Step 1 is to set up those fields in the DB to be unique. The query Barand provided would create a new table from scratch. You could also create an update query for the table to modify its structure for those unique constraints (e.g. "ALTER TABLE `table_name` ADD UNIQUE(`field_name`);"). Or, if you are like me, I just use PHP Admin's console to modify tables. Open the Table in PHP MyAdmin and select the "Structure" tab. In the list of fields, there is a column on the right where the label UNIQUE is shown. Just click that label to make that field unique in the table.

Step 2. Now that those fields are unique IF someone was to try and create a new profile with a username or email address that already exists in the table, the query will fail. So, you need to approach the INSERT logic differently. The code Barand linked to previously provides an example. Basically, you want to wrap the INSERT (or an UPDATE) query within a try{} block. Then, if it fails, you will have a catch{} block to determine IF the failure was due to a duplicate constraint or not. Now, your case is a little unique since there will be two potential unique constraints and you will want to advise the user "which" of the fields (or both) are a duplicate, so you need more logic than the example provided.

Here is some sample code. Note that I have not tested it and expect I may have made some typos, but the point is about showing what the logic would look like

$pdo = new PDO("mysql:host=$hostname;dbname=$databasename;charset=utf8", $username, $password);
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); // Enables exception mode

try {
    //Attempt to execute the INSERT query
    $query ="INSERT INTO user2(username, email, password1, date) values(:username, :email, :password1, :date)";
    $stmt = $pdo->prepare($query);
    $stmt->execute($arr);
} catch (PDOException $e) {
    //Insert failed, check if it was due to duplicate constraint
    if ($e->getCode() == 1062) {
        // Duplicate username and/or email address - figure out which one(s)
        $query = "SELECT SUM(username=:username) as dupe_username,
                         SUM(email=:email) as dupe_email
                 FROM `photos`"
        $stmt = $pdo->prepare($query);
        $stmt->execute($inputArr); //This arr should only have the username & email values
        $result = $stmt->fetchAll(PDO::FETCH_ASSOC);
        $errors = "";
        if($result['dupe_username']) { $errors .= "Username must be unique"; }
        if($result['dupe_email']) { $errors .= "Email address must be unique"; }
    } else {
        //Some other error occured
        throw $e;// in case it's any other error
    }
}

 

You are the right man for this struggle i am stuck with.. wow.. amazing.. you understood me exactly and knew what i intend to do.. 

 

Am going to try your methods and give you a feedback.. thanks a million times

[Barand]

47 minutes ago, Oasweaz said:

Setting a key trap that checks for a duplicate username and email already signed up in the db.

Just run a couple of queries to find those that occur more than once

SELECT username
     , COUNT(*) as tot
FROM photos
GROUP BY username
HAVING tot > 1;



SELECT email
     , COUNT(*) as tot
FROM photos
GROUP BY email
HAVING tot > 1;

Once you've eliminated the dupes already there you can set up the DUPLICATE constraints.

[Psycho]

OK, I just realized that the prior code I posted has a problem - not a "technical" problem as it should do what it was intended to do. But, in retrospect, it allows for an attack vector. Ideally, you should not "leak" data about existing accounts. In the sample code I provided, a malicious user could identify if a particular email address was already used and then iterate through various usernames to see which one was used for that account (or vise versa).

Instead, I would suggest that the account signup page always present the user will a message along the lines of "Your account request has been received, please check your email for more info". Of course, if there were data validation errors (e.g. not a valid email format, username blank or too short) you can present the user with those problems on screen. Going back to the "valid" submission and the potential duplicate errors, here is what I would do.

1. Both the email and username or unique: Create a temp account and send an email with a link to confirm the account. Do not allow the user to "use" the account until it is confirmed. This prevents a malicious users from creating an account under someone else's email
2. The email address is a duplicate (the username may or may not be a duplicate): Send an email to the existing user for that account with a message telling them that you are notifying them that a request was made to create a new account for their email address, but an account already exists
3. The username is a duplicate, but the email address is not: This is a tough one. If the email is the user ID then it is covered in #2 above. But, in this situation it is quite possible that two people might want to use the same username, but you don't want to tell a malicious user about existing usernames. The best option I can think of is to send the sign-up user an email to the email address provided telling them the username is not unique. It still leaks data, but it would be harder to automate/brute force because they also have to check the email - although that can be automated as well.