1

I have created a password reset function in PHP.

It's working just fine...........except that, for some reason, I'm unable to set the recipient's email address : "TO"

The code works this way :

(a) the user is asked to provide his login/username (b) php sends an sql query to the database; (c) if the username is found, php takes the email-address, and sends a Reset Link via email (d) this reset-link has a unique "token" attached to it (e) the user clicks on the link in his email, and is re-directed to a new page where he resets his password

Everything is working fine...........except for the email structure itself. The email comprises : TO, CC, SUBJECT, BODY, and HEADERS.

Everything is being shown..........except the actual "TO".

In fact, the only reason I know that the code works is because I'm getting a copy of the email, via the the "CC"

Here is my code :

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

  $login = $_POST['login'];

  $query = "select * from personal_data where login='$login'";
  $result = mysqli_query($conn,$query);
  $count=mysqli_num_rows($result);
  $rows=mysqli_fetch_array($result);


  if($count==0) {

 echo "Sorry; that username does not exist in our database";
 }

else {

 function getRandomString($length) 
   {
$validCharacters = "ABCDEFGHIJKLMNPQRSTUXYVWZ123456789!#+=%&/?*$";
$validCharNumber = strlen($validCharacters);
$result = "";

for ($i = 0; $i < $length; $i++) {
    $index = mt_rand(0, $validCharNumber - 1);
    $result .= $validCharacters[$index];
 }
 return $result;    }

$token=getRandomString(40);
$q="insert into token (token,login) values ('".$token."','".$login."')";
mysqli_query($conn,$q);

function mailresetlink($to,$token){

$to = $rows['email'];
$subject = "Password Reset";
$uri = 'http://'.$_SERVER['HTTP_HOST'] ;
$message = '
<html>
<head>
<title>Password Reset Link</title>
</head>
<body>
<p>We received a Password-Reset request from your account.</p>
<p>Click on the following link to reset your password : <a   
href="'.$uri.'/PHP/password_reset?token='.$token.'">Reset Password</a></p>
</body>
</html>
 ';
 $headers = "MIME-Version: 1.0" . "\r\n";
 $headers .= "Content-type:text/html;charset=iso-8859-1" . "\r\n";
 $headers .= 'From: Support<support@xxxxx.com>' . "\r\n";
 $headers .= 'Bcc: Info<info@xxxxx.com>' . "\r\n";

 if(mail($to, $subject, $message, $headers))     {

  echo "A password reset link has been sent to your email address."

    }
 }

 if(isset($_POST['login'])) { 
    mailresetlink($email,$token);

    exit();
            } 

     }
  }
Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
phpnewbie2015
  • 366
  • 2
  • 4
  • 17
  • 2
    **Danger**: You are **vulnerable to [SQL injection attacks](http://bobby-tables.com/)** that you need to [defend](http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php) yourself from. – Quentin May 10 '15 at 19:26
  • 2
    Add error reporting to the top of your file(s) right after your opening PHP tag for example ` – Funk Forty Niner May 10 '15 at 19:33
  • you just overwrote your question completely. That should have been made as a new question. Please do a rollback to your original post, otherwise I stand at getting downvoted for it. – Funk Forty Niner May 11 '15 at 15:49
  • How do I rollback? Btw, I found the reason for the error : whenever a new token is generated, both the token and the username (login), are input into the database table "token". Well, after I changed my code, the login is no longer being sent to the database table. Just the token. – phpnewbie2015 May 11 '15 at 15:51
  • It's ok, I rolled back your question to the original post. That's not how things are done on Stack. Your new issue should have been a new question, while my answer solved the original question. Plus, check my comment in my answer about what could be going wrong. – Funk Forty Niner May 11 '15 at 15:51
  • the login's not going to the database probably because the user already exists. Again, you should be posting a new question, if my comment didn't solve it. Error checking on DB would have signaled an error, am sure. – Funk Forty Niner May 11 '15 at 15:53
  • ??? well, yes, the user exists. It's not a new user. It's an existing user, who has forgotten his password. – phpnewbie2015 May 11 '15 at 15:55
  • replace `mysqli_query($conn,$q);` with `mysqli_query($conn,$q) or die(mysqli_error($conn));` where you have `$q="insert into token (token,login)...` I think your query failed, because as I said earlier, I tested this with 2 tables and all worked flawlessly. I always test my answers ;-) double check all your tables and column types/lengths. – Funk Forty Niner May 11 '15 at 15:57
  • I added the die(mysqli_error), but got no response. So, I reverted back to my earlier format. So, now : (a) both the login and token are being sent to the database table; (b) the token is added to the link, as it should. But.......when I click on the link, it's already INVALID. – phpnewbie2015 May 11 '15 at 16:06
  • Then I suggest you post a new question with both pieces of code you're using such as the one for the validation, including your db schema etc. Or, keep trying and check if it gets deleted from your table after being validated. Something is failing somewhere and it will be quite hard to pinpoint the problem at this point. I'll keep an eye out for your new question, if you decide to post a new one. – Funk Forty Niner May 11 '15 at 16:11
  • It works now. I decided to go back to square one...........erased every change I made, including your suggestions. Then, I took things step-by-step : after each step, I made sure to TEST it first, before going forward. And, everything worked...........right up to the point where you suggested I do this : "Move getRandomString and mailresetlink after the if block.........." This is where it stopped working (LOL). No idea why )) – phpnewbie2015 May 11 '15 at 16:36
  • So, I simply skipped that last bit. – phpnewbie2015 May 11 '15 at 16:36
  • you mean what I suggested in the other answer you mean, right? I don't think I said anything about moving anything to you, but to the other guy. I only tested the other person's answer before I posted my answer, which their answer failed in more ways than one. Well, I'm glad that this was finally and truly resolved. *Cheers* – Funk Forty Niner May 11 '15 at 16:40

2 Answers2

1

The reason why your code is not working is due to a few things.

One of which is that $rows needs to reside inside the function mailresetlink($to,$token) function's parameter.

Change that to function mailresetlink($to,$token,$rows) and do the same for the one inside if(isset($_POST['login'])){...}

if(isset($_POST['login'])) { 
    mailresetlink($email,$token,$rows);

    exit();
            } 

Plus, if it isn't a typo or a bad paste; there is also a missing semi-colon in this line:

echo "A password reset link has been sent to your email address."
                                                                 ^ right there

Having done all of the above, successfully sent all of the information to Email during my test.


Sidenote: Your present code is open to SQL injection. Use mysqli with prepared statements, or PDO with prepared statements, they're much safer.

Community
  • 1
  • 1
Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
  • 2
    WORKED BRILLIANTLY !! Thanks, Fred. Oh, btw, regarding the SQL Injection issue, I assume you mean doing the following : $login = mysqli_real_escape_string($conn, $_POST['login']); ?? – phpnewbie2015 May 11 '15 at 15:03
  • @phpnewbie2015 You're quite welcome, glad to know I was able to help out. As for injection, yeah that's a start. You could also look into XSS injection. Here are a few articles on the subject http://en.wikipedia.org/wiki/Cross-site_scripting and https://www.owasp.org/index.php/Cross-site_Scripting_%28XSS%29 – Funk Forty Niner May 11 '15 at 15:04
  • Thanks again, Fred )))))))))) Really appreciate it. – phpnewbie2015 May 11 '15 at 15:06
  • Uh-oh. Something's wrong. (((( The email is getting sent. But...........the "token" is NOT being attached to the link. (((((( I did two things : (a) I copied your code (Moved getRandomString and mailresetlink after the if block)..........and the "token" was no longer attached to the reset-link (b) I went back to using my earlier format (i.e., keeping the getRandomString and mailresetlink BEFORE the "if" block............and the "token" was attached. But.........it never worked. – phpnewbie2015 May 11 '15 at 15:37
  • When I click on the RESET PASSWORD link that comes in the email, I always get the : LINK IS INVALID, OR PASSWORD has already been changed. (which is what is supposed to happen if the reset-link has already been used once). I have updated my original post to show the full code (as you suggested it to me). – phpnewbie2015 May 11 '15 at 15:38
  • @phpnewbie2015 your query may be failing, because I tested this on my own table. Add error reporting to the top of your file(s) right after your opening PHP tag for example ` – Funk Forty Niner May 11 '15 at 15:39
  • @phpnewbie2015 there is something odd that did happen when I originally tested your code with this part of your code `$validCharacters = "ABCDEFGHIJKLMNPQRSTUXYVWZ123456789!#+=%&/?*$";` - the `$` sign plays a special role in PHP and that may be why it's failing, so I changed it in my copy to `$validCharacters = "ABCDEFGHIJKLMNPQRSTUXYVWZ123456789!#+=%&/?*";`. Try that and possibly maybe getting rid of other characters like the `&` and `?`. Or, choose another method for using random characters with a hashing mechanism too. – Funk Forty Niner May 11 '15 at 15:43
0

You cannot define functions in if or while or whatever scope. Define them before or after you intend to use them. Try with the following code:

<?php
if ( isset($_POST['submit']) ) {

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


    $query = "select * from personal_data where login='$login'";
    $result = mysqli_query($conn, $query);
    $count = mysqli_num_rows($result);
    $rows = mysqli_fetch_array($result);


    if ($count == 0) {

        echo "Sorry; that username does not exist in our database";
    } else {


        if (isset($_POST['login'])) {
            mailresetlink($email, $token, $rows);

            exit();
        }

    }
}

function getRandomString($length)
{
    $validCharacters = "ABCDEFGHIJKLMNPQRSTUXYVWZ123456789!#+=%&/?*$";
    $validCharNumber = strlen($validCharacters);
    $result = "";

    for ($i = 0; $i < $length; $i++) {
        $index = mt_rand(0, $validCharNumber - 1);
        $result .= $validCharacters[$index];
    }
    return $result;
}

$token = getRandomString(40);
$q = "insert into token (token,login) values ('" . $token . "','" . $login . "')";
mysqli_query($conn, $q);

function mailresetlink($to, $token, $rows)
{

    $to = $rows['email'];
    $subject = "Password Reset";
    $uri = 'http://' . $_SERVER['HTTP_HOST'];
    $message = '
        <html>
        <head>
            <title>Password Reset Link</title>
        </head>
        <body>
        <p>We received a Password-Reset request from your account.</p>

        <p>Click on the following link to reset your password : <a
                href="' . $uri . '/PHP/password_reset?token=' . $token . '">Reset Password</a></p>
        </body>
        </html>
        ';
    $headers = "MIME-Version: 1.0" . "\r\n";
    $headers .= "Content-type:text/html;charset=iso-8859-1" . "\r\n";
    $headers .= 'From: Support <support@xxxxx.com>' . "\r\n";
    $headers .= 'Bcc: Info <info@xxxxx.com>' . "\r\n";

    if (mail($to, $subject, $message, $headers)) {

        echo "A password reset link has been sent to your email address.";

    }
}
?>

Also, pay attention to Quentin's advice about preventing SQL injection.

What I did was:

  • Moved getRandomString and mailresetlink after the if block
  • Added parameter $rows to mailresetlink function, so it can find use of the $rows variable (which was out of the scope)

You also need to define $email, because it's not being set anywhere, so I did it for you (I guess you also have an input field with the name of email somewhere.

Test it, it should work.

Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
Nikola
  • 371
  • 1
  • 9
  • 19
  • 1
    Better double-check this `'/PHP/password_reset?token=' . $token . '` that token will never get echo'd in the email, because of the `exit();` in the `mailresetlink()` function, plus `$token = getRandomString(40);` also needs to go inside the `function mailresetlink($to, $token, $rows){...}` – Funk Forty Niner May 11 '15 at 01:05