-1

Here is mysql_* code:

Activation mail and hash check

PDO:

Do anyone sees the solution ?

    if (isset($_GET['email']) && !empty($_GET['email']) AND isset($_GET['hash']) && !empty($_GET['hash'])){
        // Verify data
        $search = $db->prepare("SELECT email, hash, active FROM users WHERE email=:email AND hash=:hash AND active=0"); 
        $search->bindParam(':email', $_POST['email'], PDO::PARAM_STR);
        $search->bindParam(':hash', $_POST['hash'], PDO::PARAM_STR);
        $search->execute();
        //$match  = $search->fetch(PDO::FETCH_ASSOC);
        $match = $search->rowCount();

There is a problem in this part of condition

    if($match > 0){
                // We have a match, activate the account
                $db->prepare("UPDATE users SET active= 1 WHERE email=:email AND hash=:hash AND active=0");
                $db->bindParam(':email', $_POST['email'], PDO::PARAM_STR);
                $db->bindParam(':hash', $_POST['hash'], PDO::PARAM_STR);
                $db->execute();




    echo '<div class="statusmsg">Your account has been activated, you can now login</div>';
    }else{
                // No match -> invalid url or account has already been activated.
                echo '<div class="statusmsg">The url is either invalid or you already have activated your account.</div>';
            }


    }else{
        // Invalid approach
        echo '<div class="statusmsg">Invalid approach, please use the link that has been send to your email.</div>';
}

The condition finishes the code here:

The url is either invalid or you already have activated your account.

But it should finish the code here:

Your account has been activated, you can now login.

Community
  • 1
  • 1
Grofman
  • 29
  • 5

3 Answers3

1

You check $_GET but later using $_POST.

Djaevel
  • 184
  • 5
  • @Grofman: Please don't edit others responses to your question only because your question is that much unclear you can't accept simple help comments (wrongly posted as an answer). Thanks. – hakre May 19 '17 at 09:37
  • I'm deleted answer, i really appreciate advice, sorry – Grofman May 19 '17 at 09:43
  • @Grofman seems your database doesn't fill the rowCount at select. http://php.net/manual/en/pdostatement.rowcount.php > For most databases, PDOStatement::rowCount() does not return the number of rows affected by a SELECT statement you have on this site an example how to solve this. – Djaevel May 19 '17 at 09:47
0

The problem in the flow is bound to this line of code:

$match = $search->rowCount();

You're executing a SELECT query and the rowCount is only available on INSERT, DELETE or UPDATE queries.

Instead, to find out if there is at leas match, you can use fetch() to get the "first" row, and if it exists, there was a match:

...
$match = $search->fetch();
if ($match) {
    ...

As another alternative approach to prevent that line of code you can in theory as well remove the first SELECT query in full and execute the second UPDATE query first and then check with the rowCount whether or not the user was updated.

There is more left open to comment with your code, like the logic you validate the hashes as they have no time-limit and the generation of the hash is even unknown but often critical.

Reference:

Community
  • 1
  • 1
hakre
  • 193,403
  • 52
  • 435
  • 836
0

You have few mistakes in your code,

First the $_POST that you are using here does not exists.

$search->bindParam(':email', $_POST['email'], PDO::PARAM_STR);
$search->bindParam(':hash', $_POST['hash'], PDO::PARAM_STR);

You should be using $_GET rowCount returns the number of rows affected by the last DELETE, INSERT, or UPDATE statement executed by the corresponding PDOStatement object. Thefore rowcount, is not reliable on select,

below is working code that you can use to achieve what you looking for.

if (isset($_GET['email']) && !empty($_GET['email']) AND isset($_GET['hash']) && !empty($_GET['hash'])){

        $email = $_GET['email'];
        $hash = $_GET['hash'];

        $search = $db->prepare("SELECT email, hash, active FROM users WHERE email=:email AND hash=:hash AND active=0"); 
         $search->bindParam(':email', $email, PDO::PARAM_STR);
        $search->bindParam(':hash', $hash, PDO::PARAM_STR);
        $search->execute();

        $match = $search->fetchall(PDO::FETCH_ASSOC);

        if(count($match) > 0){
            //then match exists activate the profile

            $stmt = $db->prepare("UPDATE users SET active= 1 WHERE email= ?  AND hash= ? AND active=0")->execute(array($email,$hash));

            if(!$stmt){

                    print_r($db->errorInfo());

            }else{
                    //account activated
                 echo '<div class="statusmsg">Your account has been activated, you can now login</div>';
            }

        }else{

             // No match -> invalid url or account has already been activated.
                echo '<div class="statusmsg">The url is either invalid or you already have activated your account.</div>';
        }

    }else{

        // Invalid approach
        echo '<div class="statusmsg">Invalid approach, please use the link that has been send to your email.</div>';
    }
Masivuye Cokile
  • 4,754
  • 3
  • 19
  • 34
  • Thank you so much !!!!!!!!!!!!!!!!!!! Thank you!!! Thank you!!! Thank you!!! Thank you !!!Thank you – Grofman May 19 '17 at 10:27
  • Someone gave me a negative assessment and i don't have 15 points to give you ... Does not matter, surely thank you so much! – Grofman May 19 '17 at 10:37
  • @Grofman: You give 15 Reps by accepting in any case. And another hint: If you look at the code, you can spare the first query in full. The update query would only run (and update) if that user/hash exists. Bonus is, you would get rowCount set then. The `fetchall` proposed here still does not always work with rowcount and in any case should not be done because it is not necessary. You only want to update. The database solves that for you already. Just check if updating worked and how many rows were affected. – hakre May 19 '17 at 11:30
  • @hakre if you are the downvoter please explain what is it for? as for as my code is concern there's no `rowCount` here – Masivuye Cokile May 19 '17 at 12:26
  • I did not downvote, but if you do a fetchAll operation only to count rows, this is wrong advice (as it should never be necessary), instead suggest a change that makes that query superfluous. I can imagine that triggerd the d/v by someone (but not me). I already have this reference in my answer which outlines this quite well: http://stackoverflow.com/a/16776839/367456 – hakre May 19 '17 at 13:11