Improvements To Member Registration Site Reg.php

I was thinking the same.
But have a look at my screen.

Guys,

Actually, another programmer edited my account_activation.php script by adding PREPARED STATEMENS as at the time I had no clue how to begin on it. I only built the script without sql injection prevention.
Since, I’m working on his edit, you guys can be kind enough check it out if there was any errors on the guy’s code or not. Because, if there were then I might aswell start all over. Else, continue from where he left-off.
This was his edit:

<?php
session_start();
include 'config.php';


if (!isset($_GET["email"], $_GET["account_activation_code"]) === true){
    $_SESSION['error'] = "Invalid Email Address! Invalid Account Activation Link! This email is not registered! Try registering an account if you do not already have one! <a href=\"register.php\">Register here!</a>";
    exit();
} else {
    $stmt = mysqli_prepare($conn, "SELECT usernames, account_actvations FROM users WHERE emails = ? AND accounts_activations_codes = ?");
    mysqli_stmt_bind_param($stmt, 'si', $_GET["email"],  $_GET["account_activation_code"]);
    mysqli_stmt_bind_result($stmt, $username, $userActivationState);

    if (mysqli_stmt_execute($stmt) && mysqli_stmt_fetch($stmt)){
        if ($userActivationState != 0){
            echo "Since your account is already activated, why are you trying to activate it again ? Do not do that again and just login from <a href=\"login.php\">this webpage</a> next time! Make a note of that webpage, ok ?";
            exit;
        }

        $userActivationState = 1;
        $stmt = mysqli_prepare($conn, "UPDATE users SET account_activations = ? WHERE usernames = ?");
        mysqli_stmt_bind_param($stmt, 'is', $userActivationState, $username);
        if (mysqli_stmt_execute($stmt)){
            echo "<h3 style='text-align:center'>Thank you for your confirming your email and activating your account.<br /> Redirecting you to the login page ...</h3>";

            $_SESSION["user"] = $username;

            header("location:home.php");
            exit;
        }
    } else {
        $email = htmlspecialchars($_GET['email']);
        $code = htmlspecialchars($_GET['account_activation_code']);
        echo "Invalid Email Address or Invalid Account Activation Link! This Email $email was not pending registration with this Account Activation Code $code!
        Try registering an account if you have not already done so! <a href=\"register.php\">Register here!</a>";
        exit;
    }
}

Do you reckon his code is fine ? No errors in your opinion the way he structured things that might be troublesome in the future when lots of people connect to my servers at once ?

If you think you can better it (to prevent bandwidth jam) then I’d liek to see your sample.
Let’s see if this forum can do better. :slight_smile:
Thanks!

TechnoBear, you still think I got the line numbers wrong ?
And, what do you think about the code in my previous post ? Can you spot any logical errors. I can’t see any but I’m no pro as you guys. Now, am I ?

I don’t understand why the experienced programmer, while editing my code, had those 2 different statements under the same variable name. I kept looking at the 2nd $stmt and couldn’t see where the heck the boolean was. Anyway, I renamed those 2 statements to $stmt_one & stmt_two.

Code now looks like this:

<?php

/*
ERROR HANDLING
*/
declare(strict_types=1);
ini_set('display_errors', '1');
ini_set('display_startup_errors', '1');
error_reporting(E_ALL);

include 'config.php';

if (!isset($_GET["email"], $_GET["account_activation_code"]) === true){
    $_SESSION['error'] = "Invalid Email Address! Invalid Account Activation Link! This email is not registered! Try registering an account if you do not already have one! <a href=\"register.php\">Register here!</a>";
    exit();
} else {
	    $email = htmlspecialchars($_GET['email']);
        $account_activation_code = htmlspecialchars($_GET['account_activation_code']);
		
    $stmt_one = mysqli_prepare($conn, "SELECT usernames, accounts_activations FROM users WHERE emails = ? AND accounts_activations_codes = ?");
    mysqli_stmt_bind_param($stmt_one, 'si', $email,  $account_activation_code);
    mysqli_stmt_bind_result($stmt_one, $username, $userActivationState);

    if (mysqli_stmt_execute($stmt_one) && mysqli_stmt_fetch($stmt_one)){
        if ($userActivationState != 0){
            echo "Since your account is already activated, why are you trying to activate it again ? Do not do that again and just login from <a href=\"login.php\">this webpage</a> next time! Make a note of that webpage, ok ?";
            exit;
        }

        $userActivationState = 1;
        $stmt_two = mysqli_prepare($conn, "UPDATE users SET accounts_activations = ? WHERE usernames = ?");
        mysqli_stmt_bind_param($stmt_two, 'is', $userActivationState, $username);
		echo $username;
        if (mysqli_stmt_execute($stmt_two)){
            echo "<h3 style='text-align:center'>Thank you for your confirming your email and activating your account.<br /> Redirecting you to the login page ...</h3>";

            $_SESSION["user"] = $username;

            header("location:home.php");
            exit;
        }
    } else {

        echo "Invalid Email Address or Invalid Account Activation Link! This Email $email was not pending registration with this Account Activation Code $code!
        Try registering an account if you have not already done so! <a href=\"register.php\">Register here!</a>";
        exit;
    }
}

I get error:

Fatal error: Uncaught TypeError: mysqli_stmt_bind_param() expects parameter 1 to be mysqli_stmt, boolean given in /home/user/public_html/e-id/activate_account.php:32 Stack trace: #0 /home/user/public_html/e-id/activate_account.php(32): mysqli_stmt_bind_param(false, ‘is’, 1, ‘ui_man’) #1 {main} thrown in /home/user/public_html/e-id/activate_account.php on line 32

Note, it is saying error in line 32 (bind param result).
Line 32 looks like this:
mysqli_stmt_bind_param($stmt_two, ‘is’, $userActivationState, $username);

Full context of the code (from line 17 to 40):

$email = htmlspecialchars($_GET['email']);
        $account_activation_code = htmlspecialchars($_GET['account_activation_code']);
		
    $stmt_one = mysqli_prepare($conn, "SELECT usernames, accounts_activations FROM users WHERE emails = ? AND accounts_activations_codes = ?");
    mysqli_stmt_bind_param($stmt_one, 'si', $email,  $account_activation_code);
    mysqli_stmt_bind_result($stmt_one, $username, $userActivationState);

    if (mysqli_stmt_execute($stmt_one) && mysqli_stmt_fetch($stmt_one)){
        if ($userActivationState != 0){
            echo "Since your account is already activated, why are you trying to activate it again ? Do not do that again and just login from <a href=\"login.php\">this webpage</a> next time! Make a note of that webpage, ok ?";
            exit;
        }

        $userActivationState = 1;
        $stmt_two = mysqli_prepare($conn, "UPDATE users SET accounts_activations = ? WHERE usernames = ?");
        mysqli_stmt_bind_param($stmt_two, 'is', $userActivationState, $username);
		echo $username;
        if (mysqli_stmt_execute($stmt_two)){
            echo "<h3 style='text-align:center'>Thank you for your confirming your email and activating your account.<br /> Redirecting you to the login page ...</h3>";

            $_SESSION["user"] = $username;

            header("location:home.php");
            exit;

So basically, the error is saying that, on this particular line:

$stmt_two = mysqli_prepare($conn, "UPDATE users SET accounts_activations = ? WHERE usernames = ?");

The result (boolean) became FALSE. In other words, the script failed to UPDATE the db.
Hence it shows the error:
mysqli_stmt_bind_param(false, ‘is’, 1, ‘ui_man’) #1

Now my question is, if the script failed to UPDATE the bd then how come it managed to insert the strong text$userActivationState (who’s value is: 1) into the db ?
Note the “1” shown in the error. Note the 3rd parameter:
mysqli_stmt_bind_param(false, ‘is’, 1, ‘ui_man’) #1 {main} thrown in /home/user/public_html/e-id/activate_account.php on line 32
Also, there is no db connection error.
On the other hand, checking the db (column: acounts_activations) I do not see the entry “1” as expected but “0”. That means, there was a FAIL to UPDATE that column.
Now, it boils down to one question: Why is it failing to UPDATE the db ? There is no connection error. DB details are fine. Username, password, etc. No problem in that line. So, what is the problem then to UPDATE the column ?
Strange!

As previously mentioned when an error occurs try var_dump(…); on previous parameters to ensure the values are the correct type and have valid values otherwise it is pointless to continue.

2 Likes

Is there any chance that the first query might return more than one row?

1 Like

Yes.
When the user registers for his account, he gets an email with the account activation link which he must click.
Link comes in this format:
$account_activation_link = http://www.“.$site_domain.”/“.$social_network_name.”/activate_account.php?email=“.$_POST[‘email’].”&account_activation_code=“.$account_activation_code.”;

When the user clicks this link from his email, the account_activation.php takes over (this is the one I’m having the issues with). The script grabs the email and the acc activation code from the link (GET method). And then checks these details against 2 columns (emails & accounts_activations_codes). If there are no matches then shows error that the email is not registered.
Else, it changes the value “0” (meaning: pending registration) to “1” (meaning: registered through the account activation link by confirming the email) on the column “accounts_activations”. Note the variable: $userActivationState.
It also grabs the username from the “usernames” column. Finally, auto logs the user into his new account with his session id so he does not have to enter the username & password.
Simple basic script.

Anyway, what is your point regarding the 2 columns it checks into (usernames and accounts_activations_codes) ?

Not to be overly cruel, especially to someone who is not here to defend themselves, but I would question the “experience level” of whomever is helping you. In any event:

$stmt = mysqli_prepare('whatever ...');
if ($stmt === false) {
    echo "oopsie, the prepare statement failed\n";
    exit();

Your life would be so much easier if you were willing to take even basic advice and use PDO with exceptions. I am honestly puzzled by your mindset.

2 Likes

Well, the guy did it in a rush and did mention he has not tested it. Volunteer like you guys. Got to give him some credit for atleast bothering to write-up the PREPARED STATEMENTS and edit it.

PDO I will look into when I’m intermediate.
Going one step at a time.
PDO looks complicated, anyway. As of now.

I believe you are referring to this:


 if (mysqli_stmt_execute($stmt_one) && mysqli_stmt_fetch($stmt_one)){
        if ($userActivationState != 0){
            echo "Since your account is already activated, why are you trying to activate it again ? Do not do that again and just login from <a href=\"login.php\">this webpage</a> next time! Make a note of that webpage, ok ?";
            exit;
        }

        $userActivationState = 1;
        $stmt_two = mysqli_prepare($conn, "UPDATE users SET accounts_activations = ? WHERE usernames = ?");
        mysqli_stmt_bind_param($stmt_two, 'is', $userActivationState, $username);
		echo $username;
        if (mysqli_stmt_execute($stmt_two)){
            echo "<h3 style='text-align:center'>Thank you for your confirming your email and activating your account.<br /> Redirecting you to the login page ...</h3>";

            $_SESSION["user"] = $username;

            header("location:home.php");
            exit;
        }
    } else {

        echo "Invalid Email Address or Invalid Account Activation Link! This Email $email was not pending registration with this Account Activation Code $code!
        Try registering an account if you have not already done so! <a href=\"register.php\">Register here!</a>";
        exit;
    }

Correct ?

I think the logic he did was just use mine. And the logic was this …
Script searches for email and account activation code on db after grabbing them via GET method from the account activation link. If match found then that means the user is pending registration. If no match found then user is not pending registration. Hence, the user gets shown the “invalid email” message.
So, how would you improve this part, then ? Let us hear about your way.

EDIT:
It just occured to me that there is a serious security issue here.
I’m gonna use ahundiak’s username in the example. And so, don’t mind.
ahundiak register’s to my site. He mistypes his email (even on the confirmation field) and the account activation link gets emailed to a different person who has similar email address. That person clicks the activation link and my system auto logs him into the activated account. That person can now change the password and hijack ahundiak’s account. Update his email from within the account, etc.
This method of activating an account and auto logging the user in is a serious security flaws used by a lot of forums, etc. I can’t remember if this forum uses this same method or not but TechnoBear can verify it. Unless ofcourse, he’ll allow me to open a 2nd account just to check this out.
Good thing you brought this issue up ahundiak! (And, I thought you were just a sarcastic fellow who likes to stab people. I was wrong. Your’e welcome to criticize all my systems/methods on all my threads from now on. But, keep it civilized by keeping the tone down, like all people here (unlike other forums). :wink: ).

If you insist on using procedural code then you have to understand the concept of error checking. Nothing to do with logic and such. Even now I don’t think you understand that when the prepare statement fails, you can’t do anything with it.

Short of completely writing your code for you, I honestly don’t know how anyone can help you.

5 Likes

Ah! So, you overlooked the part of the flaw in the system mentioned in my previous post!
Yeah, I understand that if PREPARED STATEMENTS fail then I can;t use that template anymore. But, the big question is: Why should it fail ?

http://php.net/manual/en/mysqli-stmt.bind-result.php
http://php.net/manual/en/mysqli-stmt.fetch.php

Manual usually shows 2 examples of a function. Pdo and not. But on PREPARED STATEMENTS it seems they show only Pdo:
http://php.net/manual/en/mysqli.quickstart.prepared-statements.php

Anyway, going on the hunt now:

https://www.google.com/search?q=mysqli+prepared+statements+tutorial&oq=mysqli+prepared+statements+tutorial&gs_l=psy-ab.3..0i7i30k1j0i7i5i30k1.154926.157005.0.157292.7.7.0.0.0.0.273.273.2-1.1.0....0...1.1.64.psy-ab..6.1.271.uNA2Dv7Qp9U

You tell me. Fire up your mysql admin console and try to execute the update statement. Shouldn’t take more than a few seconds to understand the problem.

Once again, error checking is basic stuff. You are trying to understand what is happening without error checking and who really knows? A prepare statement is failing. Figure out why and move on.

1 Like

$stmt_two is failing. It seems!
Here’s the error:

*string(23) "EDITED@mail.com" string(40) “c1dfd96eea8cc2b62785275bca38ac261256e278” object(mysqli_stmt)#2 (10) { [“affected_rows”]=> int(0) [“insert_id”]=> int(0) [“num_rows”]=> int(0) [“param_count”]=> int(2) [“field_count”]=> int(2) [“errno”]=> int(0) [“error”]=> string(0) “” [“error_list”]=> array(0) { } [“sqlstate”]=> string(5) “00000” [“id”]=> int(1) } object(mysqli_stmt)#2 (10) { [“affected_rows”]=> int(0) [“insert_id”]=> int(0) [“num_rows”]=> int(0) [“param_count”]=> int(2) [“field_count”]=> int(2) [“errno”]=> int(0) [“error”]=> string(0) “” [“error_list”]=> array(0) { } [“sqlstate”]=> string(5) “00000” [“id”]=> int(1) } NULL FAILURE to UPDATE db

Here’s the code using the var_dump:

<?php

/*
ERROR HANDLING
*/
declare(strict_types=1);
ini_set('display_errors', '1');
ini_set('display_startup_errors', '1');
error_reporting(E_ALL);

include 'config.php';

if (!isset($_GET["email"], $_GET["account_activation_code"]) === true){
    $_SESSION['error'] = "Invalid Email Address! Invalid Account Activation Link! This email is not registered! Try registering an account if you do not already have one! <a href=\"register.php\">Register here!</a>";
    exit();
} 
else 
{
	$email = htmlspecialchars($_GET['email']);
	echo var_dump($email);
	
	$account_activation_code = htmlspecialchars($_GET['account_activation_code']);
	echo var_dump($account_activation_code);
		
	$stmt_one = mysqli_prepare($conn, "SELECT usernames, accounts_activations FROM users WHERE emails = ? AND accounts_activations_codes = ?");
	echo var_dump($stmt_one);
	
	mysqli_stmt_bind_param($stmt_one, 'si', $email,  $account_activation_code);
	echo var_dump($stmt_one);
	
	mysqli_stmt_bind_result($stmt_one, $username, $userActivationState);
	echo var_dump($userActivationState);

	if (mysqli_stmt_execute($stmt_one) && mysqli_stmt_fetch($stmt_one))
	{
		if ($userActivationState != 0)
		{
			echo var_dump($userActivationState);
			echo "Since your account is already activated, why are you trying to activate it again ? Do not do that again and just login from <a href=\"login.php\">this webpage</a> next time! Make a note of that webpage, ok ?";
			exit;
		}

		$userActivationState = 1;
		echo var_dump($userActivationState);
		
		$stmt_two = mysqli_prepare($conn, "UPDATE users SET accounts_activations = ? WHERE usernames = ?");
		mysqli_stmt_bind_param($stmt_two, 'is', $userActivationState, $username);		
		echo var_dump($stmt_two);
		
		if (mysqli_stmt_execute($stmt_two))
		{
			echo "<h3 style='text-align:center'>Thank you for your confirming your email and activating your account.<br /> Redirecting you to the login page ...</h3>";
			$_SESSION["user"] = $username;
			header("location:home.php");
			exit;
		}
	} 
	else 
	{
		echo "FAILURE to UPDATE db";
		exit;
	}
}

Here’s the code without the var_dump:

<?php

/*
ERROR HANDLING
*/
declare(strict_types=1);
ini_set('display_errors', '1');
ini_set('display_startup_errors', '1');
error_reporting(E_ALL);

include 'config.php';

if (!isset($_GET["email"], $_GET["account_activation_code"]) === true){
    $_SESSION['error'] = "Invalid Email Address! Invalid Account Activation Link! This email is not registered! Try registering an account if you do not already have one! <a href=\"register.php\">Register here!</a>";
    exit();
} else {
	    $email = htmlspecialchars($_GET['email']);
        $account_activation_code = htmlspecialchars($_GET['account_activation_code']);
		
    $stmt_one = mysqli_prepare($conn, "SELECT usernames, accounts_activations FROM users WHERE emails = ? AND accounts_activations_codes = ?");
    mysqli_stmt_bind_param($stmt_one, 'si', $email,  $account_activation_code);
    mysqli_stmt_bind_result($stmt_one, $username, $userActivationState);

    if (mysqli_stmt_execute($stmt_one) && mysqli_stmt_fetch($stmt_one)){
        if ($userActivationState != 0){
            echo "Since your account is already activated, why are you trying to activate it again ? Do not do that again and just login from <a href=\"login.php\">this webpage</a> next time! Make a note of that webpage, ok ?";
            exit;
        }

        $userActivationState = 1;
        $stmt_two = mysqli_prepare($conn, "UPDATE users SET accounts_activations = ? WHERE usernames = ?");
        mysqli_stmt_bind_param($stmt_two, 'is', $userActivationState, $username);
		echo $username;
        if (mysqli_stmt_execute($stmt_two)){
            echo "<h3 style='text-align:center'>Thank you for your confirming your email and activating your account.<br /> Redirecting you to the login page ...</h3>";

            $_SESSION["user"] = $username;

            header("location:home.php");
            exit;
        }
    } else 
	{
		echo "FAILURE to UPDATE db";
		exit;
	}
}

Here’s the error message:

FAILURE to UPDATE db

But, I don’t understand why the following is failing. I don’t see any typo of column names. No error says the PREPARED STATEMENT coding is wrong, either. Puzzling!

$stmt_two = mysqli_prepare($conn, "UPDATE users SET accounts_activations = ? WHERE usernames = ?");
mysqli_stmt_bind_param($stmt_two, 'is', $userActivationState, $username);

Following working fine:

	$stmt_one = mysqli_prepare($conn, "SELECT usernames, accounts_activations FROM users WHERE emails = ? AND accounts_activations_codes = ?");	
	
	mysqli_stmt_bind_param($stmt_one, 'si', $email,  $account_activation_code);	
	
	mysqli_stmt_bind_result($stmt_one, $username, $userActivationState);
	

As previously mentioned when an error occurs try var_dump(…); on previous parameters to ensure the values are the correct type and have valid values otherwise it is pointless to continue.


Please explain in simple terms why it is necessary to display the results of the Pdo failure.

As mentioned the previous parameters should be checked and not the results.

You should go back to find the cause and not forward to find the result.

1 Like

Add this line

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);

right under

error_reporting(E_ALL);

or this topic will go in circles for another 200 posts,

1 Like

I didn’t make one - I asked whether it might return more than one row. I can’t see why it would in a live environment, but in a test situation you might have bad data.

In any case, I believe (but I don’t use mysqli, so you need to check further) that you can’t start another query until you’ve called fetch() to retrieve all the rows from the first query.

http://php.net/manual/en/mysqli-stmt.execute.php

1 Like

Ooops!

You mean not to do it like this:

	mysqli_stmt_bind_result($stmt_one, $username, $userActivationState);
	echo var_dump($userActivationState);

But to do it like this:

        echo var_dump($userActivationState);
	mysqli_stmt_bind_result($stmt_one, $username, $userActivationState);
	

Ok. Got you! :slight_smile:

Thanks mate! Done!
This was added on a nother page but I missed it on this one.
Thanks for reminding!

Thanks for the link.
But this does not make sense. Grammatical errors. Right ?

If the statement is UPDATE, DELETE, or INSERT, the total number of affected rows can be determined by using the mysqli_stmt_affected_rows() function.
Likewise, if the query yields a result set the mysqli_stmt_fetch() function is used.

http://php.net/manual/en/mysqli-stmt.execute.php

Anyway, going through these links for half hr now. need to read & re-read for probably about 2 more hrs before a little will sink into the subconscious. Mysqli syntax is messy.

http://php.net/manual/en/mysqli.quickstart.prepared-statements.php
http://php.net/manual/en/mysqli.prepare.php
http://php.net/manual/en/mysqli-stmt.bind-result.php
http://php.net/manual/en/mysqli-stmt.get-result.php
http://php.net/manual/en/mysqli-stmt.fetch.php
http://php.net/manual/en/mysqli-result.fetch-array.php

But just because I’m gonna spend 2hrs on these does not mean I will remember them all in one night. Got to revise them every night for about a wk probably before I get to grip with things.
Too many things to do. Too many lines in to write in prepared statements. You learn one page and forget it as soon as you move-on to the next page.

Look what I just found:
https://www.youtube.com/results?search_query=prepared+statements+in+php
https://www.youtube.com/results?search_query=mysqli+prepared+statements+in+php
https://www.youtube.com/results?search_query=pdo+prepared+statements+in+php

Tex tutorial:
https://www.tutorialrepublic.com/php-tutorial/php-mysql-prepared-statements.php

That’s ok. You do that, take your time to understand everything and then come back if you have any questions.