My code works fine, but is this a efficient code, like on line 16 (commented) I was thinking to use $row and compare with a variable mentioned above, rather than writing another SQL query.

I tried using variable and $row['field name'], but it was throwing an error "Trying to access array offset on value of type null".

<?php
require('../private/autoload.php');
if(isset($_GET['token'])){
    $msg ="Email verified successfully, Thankyou.";
    $token = $_GET['token'];
    $email_status = "active";
    $sql = "SELECT `email_token`, `email_status` FROM `users` where `email_token` = ? AND `email_status` = 'inactive' LIMIT 1";
    $stmt = $conn->prepare($sql);
    $stmt->bind_param("s",$token);
    $stmt->execute();
    $result = $stmt->get_result();
    $exist = $result->num_rows;
    if($exist == 0 ){

        // $row = $result->fetch_array(MYSQLI_ASSOC); 
    $sql = "SELECT `email_token`, `email_status` FROM `users` where `email_token` = ? AND `email_status` = ? LIMIT 1";   //Line 16
        $stmt = $conn->prepare($sql);
        $stmt->bind_param("ss",$token,$email_status);
        $stmt->execute();
        $result = $stmt->get_result();
        $exist = $result->num_rows;
        if($exist == 1 ){
        ?>
            <script>
                alert("Email already verified.");
                window.location = "../public/index.php";
            </script>;

        <?php exit(); ?>
        
        <?php }else{ ?>
            <script>
                alert("User not found.");
                window.location = "../public/index.php";
            </script>;
      <?php  } 

    }else{
        $sql = "UPDATE `users` SET `email_status`= ? where `email_token` = ?  LIMIT 1";
        $stmt = $conn->prepare($sql);
        $stmt->bind_param("ss",$email_status,$token);
        $stmt->execute();
        $stmt->close();  
        $_SESSION['msg'] = $msg;
        $_SESSION['token'] = $token;
        header('Location: mobile_verify.php');     
    }
}else{
    header('Location: index.php');
    die();
}
 
$conn->close();
?>

CodePudding user response：

The suggestion is to reduce the number of queries by combining the aspects of both. A user can't be "active" or "inactive" at the same time, so there is no need to do two queries.

The first query is obsolete, because all information is contained in the second query.

(please also read comments of the initial post)

CodePudding user response：

1. Typically, you do not want to be executing a "writing" process on the server-side from a $_GET request, but I am imagining that you are sending an email to the user and they are merely clicking a hyperlink so this is a tolerable scenario.
2. I don't know how cryptographically secure your token is (a UUID is a good idea), but relying on just one data point may not be enough. You may want to include a second data point in the payload such as the md5() encoded email that you sent to, or an expression of the expiry of the token. These secondary data points don't need to be cryptographically secure, but should elimate an accidental data collision or a successful brutal force attack.
3. I recommend that your response NOT give too much detail about a failed outcome. Giving these types of clues will benefit hackers more than you'll want.

Very simply, one execute an UPDATE query then depending on the number of affected rows, redirect where desired. Make sure that you are starting the session at the start of every page where the session needs to be maintained.

Untested recommendation:

$token = $_GET['token'] ?? null;
if ($token) {
    require('../private/autoload.php');              
    $sql = "UPDATE users
            SET email_status='active'
            WHERE email_status='inactive'
              AND email_token=? LIMIT 1";
    $stmt = $conn->prepare($sql);
    $stmt->bind_param("s", $token);
    $stmt->execute();
    if ($stmt->affected_rows) {
        $_SESSION['msg'] = "Email verified successfully, Thank you.";
        $_SESSION['token'] = $token; // why store this in the session?
        header('Location: mobile_verify.php');
        exit();
    }
}
// missing or invalid submission
header('Location: index.php');
exit();