PHP loop seems to go wrong

Question

After typing in username and password I redirect the user to a page where a text message is send that he has to fill in (for some not a good 2FA but thats not the discussion :)

What should happen is this: webpage gets loaded with a form and a text message is send to the user at the same time. User types in the code he received in the text message. Codes are compared and user gets redirected.

The part of the text sending works but after that something goes wrong. I logged everything and see that the comparison itself works but different values are compared as another (second) text is send. I do not know why this is? Why is it sending a second text after the post is submitted?

<?php
// Initialize the session
session_start();
// Check if the user is already logged in, if yes then redirect him to welcome page
if(!isset($_SESSION["loggedin"]) || $_SESSION["loggedin"] !== true){
    header("location: index.php");
    exit;
}

error_reporting(E_ALL);
error_reporting(-1);
ini_set('error_reporting', E_ALL);
ini_set('error_log', 'error.log');


    // SMS script
    $otp = rand(100000,999999);
    error_log($otp);
    $mobiel = $_SESSION["mobielnummer"] ; 
    $tekst = "Je+beveiligingscode+is+:+".$otp."";
    $api_key = '****'; 

    $verzoek = "https://*****************" ;

    $xml = file_get_contents($verzoek);


// Processing form data when form is submitted
if($_SERVER["REQUEST_METHOD"] == "POST"){
 
    // Check if bevcode is empty
    if(!empty(trim($_POST["bevcode"]))){
        $bevcode = trim($_POST["bevcode"]);

        // compare vars
            if ($bevcode === $otp) {
                $_SESSION["smsoke"] = true;
                header("location: home.php");
            }
            else {
                $login_err = "Dit is een onjuiste code.";
                error_log($bevcode);
                error_log($otp);
            }
      
    } 
}



?>

Well you are generating a new random number every time this script executes ... You need to store the generated random value somewhere, like in the session, and then take the value from there, when you want to compare it with what the user submitted via the form. — C3roe
– C3roe, Commented Mar 15, 2022 at 15:24
"Why is it sending a second text after the post is submitted?" - because you do not have the code that does this wrapped into any sort of condition at all, you do it every time this script executes. — C3roe
– C3roe, Commented Mar 15, 2022 at 15:25
Side note: Why are you setting error reporting level three different ways in a row? Once is enough. — M. Eriksson
– M. Eriksson, Commented Mar 15, 2022 at 15:31
I understand, What kind of condition would be best? A check if post has been submitted? — James
– James, Commented Mar 15, 2022 at 15:33

IMSoP · Accepted Answer · 2022-03-15 17:30:04Z

Computers are very literal about doing what you tell them. To you, it's obvious that the first block of code should run only the first time, and the second block from then on; but to a computer, the first block isn't in an if statement, so will run every time.

So your code currently does this the first time a user requests it:

Generate a random number
Send that random number by SMS
The user didn't submit a number, so skip that block

Then they submit the form, and you get a second request, where this happens (note carefully the order things happen):

The user submits the number they were previously sent
Generate a new random number
Send the new random number by SMS
The user submitted a number, so compare it against the new random number, which you only just generated it; it's not going to match

What you want to happen is for either one block or the other to execute; you can use your existing if($_SERVER["REQUEST_METHOD"] == "POST" for this, just move all the random number choosing and SMS sending part into an else block underneath it.

Then you'll have this on the first request:

User didn't submit a form, so skip the if block but go into the else block
Generate a random number
Send it via SMS

And in the second request:

User submitted a form, so go into the if block
Compare the submitted number against ... something
Skip the else block

You'll notice you now don't actually have a number to compare against; you want to compare against the number you picked in the first request, but you didn't save that anywhere. You need to store it in a session, or a database, or a file, so that you know what the correct answer is for this particular user.

This was the answer, thank you! It didn't work directly after doing this, the comparison failed because one was an integer and one was a string, I also had to do intval to get it working.

Collectives™ on Stack Overflow

PHP loop seems to go wrong

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related