0

So this if/else statement is for a simple login form with Google Recaptcha attached. I've got the Recaptcha part working fine, it's just that when I got to enter my username and password, even if correct, I can't seem to login. This only occurred once I added the ReCaptcha. The only thing the ReCaptcha changed was another condition for the if statement to check for and shouldn't be causing issues.

Here's my validate.php file for reference, the if statement in question is at the bottom:

<?php

if (isset($_POST['submit'])) {
    $userid = $_POST["userid"];
    $password = $_POST["password"];
    $secretkey = "_SECRET_KEY_";
    $responsekey = $_POST["g-recaptcha-response"];
    $useripaddress = $_SERVER["REMOTE_ADDR"];

    $url = "https://www.google.com/recaptcha/api/siteverify?secret={$secretkey}&response={$responsekey}&remoteip={$useripaddress}";
    $response = file_get_contents($url);
    // $response = json_decode($response);
    echo $response;
}

require_once("scripts/thecrab.php"); // This connects to the db

$userid = htmlspecialchars($_POST['userid']);
$password = htmlspecialchars($_POST['password']);

$query = "SELECT userid from users where userid = ? and password = PASSWORD(?)";
$stmt = $pdo->prepare($query);
$stmt->execute([$userid, $password]);

if ($stmt->rowCount() && $response->success === "true") {
    $_SESSION['valid_recipe_user'] = $userid;
    echo "<h2>Log In Successful</h2><br>\n";
    echo "<a href=\"index.php\"><img src=\"images/image-11.png\"></a>\n";
} else {
    echo "<h2>Sorry, your user account was not validated.</h2><br>\n";
    echo "<a href=\"index.php?content=login\">Try again</a><br>\n";
    echo "<a href=\"index.php\">Return to Home</a>\n";
}

Here's the exact if statement and condition in question:

if ($stmt->rowCount() && $response->success === "true") {
    // Successful Login. Meaning the userid and password are in the database AND the Google ReCAPTCHA response->success has the value of EXACTLY true.
} else {
    // Incorrect Login
}

Even with a correct username and password that does exist in the database, it will not execute the if statement and jumps to the else, which does not log me in.

Brxxn
  • 110
  • 1
  • 12
  • 3
    **Never store plain text passwords!** Please use ***PHP's [built-in functions](http://jayblanchard.net/proper_password_hashing_with_PHP.html)*** to handle password security. If you're using a PHP version less than 5.5 you can use the `password_hash()` [compatibility pack](https://github.com/ircmaxell/password_compat). ***It is not necessary to [escape passwords](http://stackoverflow.com/q/36628418/1011527)*** or use any other cleansing mechanism on them before hashing. Doing so *changes* the password and causes unnecessary additional coding. – Jay Blanchard Jul 17 '17 at 17:52
  • 2
    I hope that **secret** key, is not your real key. – chris85 Jul 17 '17 at 17:53
  • `if($stmt->rowCount()` What do you think that returns? – Jay Blanchard Jul 17 '17 at 17:53
  • @JayBlanchard I appreciate the heads up but this is not production code. I am testing this on a live site with an SSL Certificate to have the ReCaptcha function properly. – Brxxn Jul 17 '17 at 17:53
  • 3
    If you don't have time to do it right the first time, when will you find the time to add it later? I hate when people say *"I'm not that far along..."* or *"This site will not be public..."* or *"It's only for school, so security doesn't matter..."*. If teachers and professors are not talking about security from day one, they're doing it wrong. Challenge them. They're teaching sloppy and dangerous coding practices which students will have to unlearn later. I also hate it when folks say, *"I'll add security later..."* or *"Security isn't important now..."* or *"Ignore the security risk..."*. – Jay Blanchard Jul 17 '17 at 17:53
  • `SSL` has no affect on plain text passwords, don't do it. That would stop(hinder) a main in the middle attack, not if someone gets/has access to your DB. – chris85 Jul 17 '17 at 17:54
  • `var_dump($response->success);` – AbraCadaver Jul 17 '17 at 18:09
  • @AbraCadaver That comes back as NULL, whether or not I fill out the ReCaptcha. – Brxxn Jul 17 '17 at 18:16
  • Then that's the problem as null does not equal true. You need to add the output from a `print_r()` of `$response = json_decode(file_get_contents($url));` – AbraCadaver Jul 17 '17 at 18:18
  • @AbraCadaver Not sure if you looked at my code but I had $response = file_get_contents($url); followed by $response = json_decode($response); but that throws an error stating "Object of class stdClass could not be converted to string" That error only occurs whenever I decode the url. – Brxxn Jul 17 '17 at 18:27
  • No, that comes from trying to echo an object `echo $response;`. Try `print_r()` like I suggested. – AbraCadaver Jul 17 '17 at 18:31

2 Answers2

3

Boolean != String

Change

$response->success === "true"

to

$response->success === true

Triple equal checks the datatype as well. So boolean true will not be equal to string 'true'. BTW, you need not type check here. Simple == will do!


Or to be frank, this is just enough:

if ($stmt->rowCount() && $response->success)
Thamilhan
  • 13,040
  • 5
  • 37
  • 59
  • Thanks! That makes sense that == will suffice as Google will ONLY return a boolean, a 0 or a 1, true or false, so there's no need to check the type as it'll never change unless Google decides to. I totally overlooked the fact that I'm having it check for a string containing "true". Well while removing the quotations, I still receive the same issue; even with a correct username and password as well as filling out the ReCaptcha, I still can not login. – Brxxn Jul 17 '17 at 17:58
1

In your comparison, you have $response->success === "true". This compares not by value, but by type.

If success is bool, you can use $response->success === true. However, simpler and enough is $response->success == true, which will auto-convert string / int (whatever) from $response->success to bool

Martin Perry
  • 9,232
  • 8
  • 46
  • 114