help with a registration form

[dylfs]

Currently I'm trying to get a registration form to work adding a username and password into a database. That bit I understand and have managed to do aswell as getting the password to hash using  the line below:

$hashed = password_hash($password, PASSWORD_DEFAULT);

 

What I'm trying to do now is have the passwords check to see if they are the same, if so the user is able to register, if not they are sent back to the registration page(which was working before I started tampering with the next bit). I also want to checkto see if the username is taken, the following code is what I have so far:

<?php
session_start();

/* Attempt MySQL server connection. Assuming you are running MySQL
server with default setting (user 'root' with no password) */
$link = mysqli_connect("localhost", "root", "", "rockinrochester");
 
// Check connection
if($link === false){
    die("ERROR: Could not connect. " . mysqli_connect_error());
}
 
$sql= mysql_query("SELECT FROM register (username, password) WHERE username=$username");
 if(mysql_num_rows($sql)>=1)
   {
    echo"name already exists";
   }
 else
    {
   	$username = $_POST['username'];
	$password = $_POST['password'];
	$password2 = $_POST['password2'];
	$hashed = password_hash($password, PASSWORD_DEFAULT);
	
if($password != $password2) {   //checks to see if passwords match, if they don't it redirects person back to registration form
		header('Location: register.php');
		echo "passwords didn't match";
	    
}
else {
	
 $sql = "INSERT INTO register (username, password) VALUES ( '$username','$hashed')"; // if passwords match inputs them into database
	
}

if(mysqli_query($link, $sql)){
   header('Location: registered.php'); // if it works relocated person to registered.html
} else{
    echo "ERROR: Could not able to execute $sql. " . mysqli_error($link); // if it fails it prints an error message
}
 
// Close connection
mysqli_close($link);
    }


?>

<?php
session_start();

/* Attempt MySQL server connection. Assuming you are running MySQL
server with default setting (user 'root' with no password) */
$link = mysqli_connect("localhost", "root", "", "rockinrochester");
 
// Check connection
if($link === false){
    die("ERROR: Could not connect. " . mysqli_connect_error());
}
 

	$username = $_POST['username'];
	$password = $_POST['password'];
	$password2 = $_POST['password2'];
	$hashed = password_hash($password, PASSWORD_DEFAULT);

$sql = mysql_query("SELECT FROM register(username, password, ) WHERE username=$username");

 if(mysql_num_rows($sql)>=1)
   {
    echo"name already exists";
   }
 else{

if($password != $password2) {   //checks to see if passwords match, if they don't it redirects person back to registration form
		header('Location: register.php');
		echo "passwords didn't match";
	     }


else {
	
    $sql = "INSERT INTO register (username, password) VALUES ( '$username','$hashed')"; // if passwords match inputs them into database
	
}

if(mysqli_query($link, $sql)){
   header('Location: registered.php'); // if it works relocated person to registered.html
} else{
    echo "ERROR: Could not able to execute $sql. " . mysqli_error($link); // if it fails it prints an error message
}
 
// Close connection
mysqli_close($link);

    }


?>

at the minute It's throwing back the following error

Quote

Fatal error: Uncaught Error: Call to undefined function mysql_query() in D:\xampp\htdocs\final\registerscript.PHP:19 Stack trace: #0 {main} thrown in D:\xampp\htdocs\final\registerscript.PHP on line 19

 

Edited April 4, 2019 by dylfs
changing code

[gw1500se]

Simple spelling error. It should be mysqli_query. However, you really should be using prepared statements. Never ever use POSTed data directly into a query.

[dylfs]

8 minutes ago, gw1500se said:

Simple spelling error. It should be mysqli_query. However, you really should be using prepared statements. Never ever use POSTed data directly into a query.

Cheers, should of noticed that!

It's for a college assignment, but I'll look into prepared statements!

[mac_gyver]

there a bit more than a missing i. all the php database statements must be from the same extension. you have mix of mysqli and mysql statements. also, the SELECT query syntax is incorrect, there's no $username variable at the point where you are using it, and you would need to have single-quotes around the $username variable since it is string data.

here's a list of things your form processing code should do -

1) detect that the current user isn't logged in (there's no point in allowing registration if the current user is already registered and logged in.)

2) detect that a post method form was submitted (this will prevent form processing code from running until the form has been submitted.) your form processing code and the form should be on the same page as this results in the least amount of code and the best User eXperience (UX.)

3) trim the submitted form data so that you can detect if all white-space characters were entered (this can done with a single line of code using array_map().)

4) validate all the inputs before using them, storing validation errors in an array. this array is also an error flag. if the array is empty, there are no errors. to display the errors at the appropriate point in the html documented when you re-display the form, access elements in this array.

5) if there are no validation errors, simply run the INSERT query and detect if there was a duplicate key error (requires that the username column be defined as a unique index.)

6. use a prepared query, as already mentioned,  when supplying external/unknown data to the sql query statement. this will actually simply the sql query syntax since the php variable(s), single-quotes around them, and any concatenation dots or {} (which you are not using) are all removed and replace with a simple ? place-holder for each value.

7. use exceptions to handle all the database statement errors and in most cases let php catch and handle any error, where it will use its' error_reporting, display_errors, and log_errors settings to control what happens with the actual error information. you would then remove any error handling logic you have now, simplifying the code. the exception to this is when detecting the insertion/update of duplicate data (and other data value errors) - see item #5 in this list.

lastly, the php mysqli extension is overly complicated and inconsistent, especially when dealing with prepared queries. if you are just starting out, learn the php PDO extension instead. an addition benefit of learning the PDO extension is that the same php statements can be used with about a dozen different database types (the actual sql query syntax may be different), so you won't have to keep learning a different set of php statements should you ever need to use a different type of database.

[mac_gyver]

see the following example showing the items listed above -

<?php
session_start();

// detect if the current visitor is already logged in/registered
if(isset($_SESSION['user_id']))
{
	header('location:index.php'); // go somewhere else
	die;
}

require 'pdo_connection.php'; // put database connection code in an external .php file and require it when needed
// note: the connection code should - set the error mode to exceptions, set emulated prepared queries to false, and set the default fetch mode to assoc

$errors = []; // an array to hold errors
$post = []; // an array to hold a trimmed working copy of the form data

// post method form processing
if($_SERVER['REQUEST_METHOD'] == 'POST')
{
	// trim the submitted data
	$post = array_map('trim',$_POST); // if any of the form fields are arrays, use a recursive trim call-back function here instead of php's trim function

	// validate the submitted data
	if($post['username'] == '')
	{
		$errors['username'] = "Username is empty.";
	}
	if($post['password'] == '')
	{
		$errors['password'] = "Password is empty.";
	}
	if($post['password2'] == '')
	{
		$errors['password2'] = "Confirm password is empty.";
	}
	
	// if no password errors, compare password/confirm password
	if(empty($errors['password']) && empty($errors['password2']) && $post['password'] != $post['password2'])
	{
		$errors['confirm'] = "Password and the confirm password don't match";
	}
	
	// if no errors, use the submitted data
	if(empty($errors))
	{
		$sql = "INSERT INTO register (username, password) VALUES (?, ?)";
		$stmt = $pdo->prepare($sql);
		
		try { // a 'local' try/catch to handle a specific error type 
			$stmt->execute([
				$post['username'],
				password_hash($post['password'], PASSWORD_DEFAULT)
				]);
				
		} catch (PDOException $e) {
			if($e->errorInfo[1] == 1062) // duplicate key error number 
			{
				$errors['username'] = "Username is already in use.";
			} else { 
				throw $e; // re-throw the pdoexception if not handled by this logic 
			} 
		}
	}
	
	// if no errors, success
	if(empty($errors))
	{
		header('Location: registered.php'); // if it works relocated person to registered.html
		die;
	}
}



// at the point of (re)displaying the form, use the data in $errors to display any error messages and in $post to repopulate the form fields (you may not desire to populate password fields)
// any 'dynamic' values should have htmlentities() applied when they are being output on a web page to help prevent cross site scripting
?>

output a complete and valid html document starting here...

<?php
// display any errors
if(!empty($errors))
{
	echo implode('<br>',$errors);
}

// output the form
?>
<form method='post'>
Username: <input type='text' name='username' value='<?php echo htmlentities($post['username'] ?? '',ENT_QUOTES); ?>'><br>
Password: <input type='text' name='password' value='<?php echo htmlentities($post['password'] ?? '',ENT_QUOTES); ?>'><br>
Confirm password: <input type='text' name='password2' value='<?php echo htmlentities($post['password2'] ?? '',ENT_QUOTES); ?>'><br>
<input type='submit'>
</form>

 

[dylfs]

cheers for the code I'll take a look at it and see if I can get my head around it

Another quick question.

If I use the following code on a 'Registration form', do I also need to use it on the 'Login page' to match the passwords?

$hashed = password_hash($password, PASSWORD_DEFAULT);

 

[Barand]

You use password_verify() to verify passwords.

[dylfs]

10 hours ago, Barand said:

You use password_verify() to verify passwords.

Been trying to use password_verify() for the last hour and a half with a friend who's at least semi compitent with php. we're at a halt as to why the code doesn't seem to work. It's just throwing back

	echo "Invalid login, please return to the previous page.";invalid 

login form:

<?php

$username = "root";
$password = "";
$hostname = "localhost";
$db = "Rockinrochester";

$conn = mysqli_connect($hostname, $username, $password, $db) //his

or die ("Unable to connect to MySQL"); 

echo "Connected to MySQL<br>";

if(mysqli_connect_errno())
	{
		echo "Failed to connect to MySQL: " . mysqli_connect_error();
	}

echo "Connected to Db<br>";

$username = trim($_POST['username']);
$password = trim($_POST['password']);

$sql = "SELECT * FROM register WHERE username='$username' LIMIT 1";
$res = mysqli_query($conn, $sql);
$result_data = mysqli_fetch_assoc($res);

if(count($result_data) > 1) {
	$collected_password = $result_data['password'];
}
$numRows = mysqli_num_rows($res);


if (isset($collected_password) && password_verify($password, $collected_password) && $numRows == 1) {
	
	echo "Login Successful.";
	

} else {
	echo "Invalid login, please return to the previous page.";
	echo "<br>";
	echo $password;
	echo "<br>";
	echo $collected_password;
	exit();
}

mysqli_close($conn);

?>

 

Register form - seems to be working alright, it's hashing the passwords within the database.

 

<?php
session_start();

/* Attempt MySQL server connection. Assuming you are running MySQL
server with default setting (user 'root' with no password) */
$link = mysqli_connect("localhost", "root", "", "rockinrochester");
 
// Check connection
if($link === false){
    die("ERROR: Could not connect. " . mysqli_connect_error());
}
 
// Escape user inputs for security
$username = mysqli_real_escape_string($link, $_REQUEST['username']);
$password = mysqli_real_escape_string($link, $_REQUEST['password']);

$hashed_password = password_hash($password, PASSWORD_DEFAULT);
 
// Attempt insert query execution
$sql = "INSERT INTO register (username, password) VALUES ( '$username','$hashed_password')";

// check if query succeeded 
$check_result = mysqli_query($link, $sql);

if($check_result == true){
	

	header('Location: registered.php');} 
else {
	$_SESSION['error'] = "Username already exists";
    echo "ERROR: Could not able to execute $sql. " . mysqli_error($link);
	header('Location: register.php');
}
 
// Close connection
mysqli_close($link);

?>

[gw1500se]

As you can see from your error message echo $password and $collected_password are both empty strings. You say the insert is working but did you run the query manually to verify it is correct in the database? You are also not doing any error checking after the query.

[dylfs]

7 minutes ago, gw1500se said:

As you can see from your error message echo $password and $collected_password are both empty strings. You say the insert is working but did you run the query manually to verify it is correct in the database? You are also not doing any error checking after the query.

Sorry It's for a college course and our teacher left halfway through so my php knowledge is limited to slideshows of code and brief explenations on stuff. How would I go about running the query manually to check ? using xamp

Edited April 5, 2019 by dylfs

[gw1500se]

Connect to the MySQL database from command line (mysql -u root). You will be prompted for the password. Then enter 'use rockinrochester;'. After that enter the query substituting the values rather than the variables. That will prove that the data was properly entered in the database.

Since this is a class project I won't give you the solution outright.

Edited April 5, 2019 by gw1500se

[mac_gyver]

1 hour ago, dylfs said:

if (isset($collected_password) && password_verify($password, $collected_password) && $numRows == 1) {

by lumping this logic into one statement, you will never know which condition is failing, which would help pin down where the problem is. also, the 1st and 3rd conditions mean the same thing - the username was found.

your program logic should be -

1) execute the SELECT query to find the row matching the username. if you use exceptions to handle errors, as was stated in a reply above, you won't have to add conditional logic at each statement that can fail. your main code will only need to deal with error free execution since program flow will automatically transfer to the exception handler if there is an error.

2) fetch and test if the username was or was not found - you can do this with one statement. also, the negative condition (not found) is often shorter code, so by inverting the logic test and dealing with the negative/not condition first, you get its' code out of the way.

3) if the username was found, verify the password. If the password verifies, save the user's id in a session variable to identify who the logged in user is.

while you should setup and output the same 'invalid username/password' message if the username didn't match or the password didn't verify, by testing these conditions separately, you have specific points in your logic where you can output or log debugging information that would tell you which condition is failing. the following logic is all you need - 

// execute the SELECT query here...		
if(!$row = mysqli_fetch_assoc($res))
{
	// username not found
	$errors['login'] = "Invalid Username/Password.";
} else {
	// username found, verify password hash
	if(!password_verify($password,$row['password']))
	{
		// password doesn't match
		$errors['login'] = "Invalid Username/Password.";
	} else {
		// password matches
		$_SESSION['user_id'] = $row['id'];
	}
}

 

Edited April 5, 2019 by mac_gyver

[dylfs]

sorry you're going to have to bear with me I'm still stupidly lost with this, so i've taken your code and tried putting it in my login form

 

<?php

$username = "root";
$password = "";
$hostname = "localhost";
$db = "rockinrochester";

$conn = mysqli_connect($hostname, $username, $password, $db) 

or die ("Unable to connect to MySQL"); 

echo "Connected to MySQL<br>";

if(mysqli_connect_errno())
	{
		echo "Failed to connect to MySQL: " . mysqli_connect_error();
	}

echo "Connected to Db<br>";

$username = trim($_POST['username']);
$password = trim($_POST['password']);
$res = mysqli_query($conn, $sql);

$sql = "SELECT * FROM register WHERE username='$username' LIMIT 1";
if(!$row = mysqli_fetch_assoc($res))
{
	echo "1";
	$errors['login'] = "Invalid Username/Password.";
} else {
	echo "2";
	if(!password_verify($password,$row['password']))
	{
		// password doesn't match
		$errors['login'] = "Invalid Username/Password.";
		echo "3";
	} else {
		// password matches
		$_SESSION['user_id'] = $row['id'];
		echo "4";
	}
}




mysqli_close($conn);

?>

 

obviously each if and else are going to requre different bits of code/headers in dependent on what I want them to do. I tried adding echo's to each loop to help see what's going on, but when I'm running the code I'm encountering three errors:

Quote

Notice: Undefined variable: sql in D:\xampp\htdocs\final\loginscript.php on line 23

Warning: mysqli_query(): Empty query in D:\xampp\htdocs\final\loginscript.php on line 23

Warning: mysqli_fetch_assoc() expects parameter 1 to be mysqli_result, bool given in D:\xampp\htdocs\final\loginscript.php on line 26

 

[Barand]

19 minutes ago, dylfs said:

$res = mysqli_query($conn, $sql);

$sql = "SELECT * FROM register WHERE username='$username' LIMIT 1";

If you define $sql BEFORE you try to execute it with mysqli_query() then you might get a better result