PHP, SQL, User Database

Question

I hope I formatted the code properly. I am having trouble making this if statement to work. I've searched and from what it looks like this statement should work. However, when I run it no matter the password if the username starts with kacey then it goes to echo "Logged in as: " . kacey; Likewise, if I put the input to kaceyfeewaf, it still goes to echo "Logged in as: " . $myuser; This happens regardless of the password I put in. the line $result['username'] should validate to KACEY.

$sql = "SELECT * FROM $dbTable WHERE username = $myuser AND password = $mypass";
$result = mysql_query($sql);

if($result['username'] = $myuser && $result['password'] = $mypass;) 
{
    echo "Logged in as: " . $myuser;    
} else {
    echo "Fail "; 
  }

You are using assignment operator in place of comparison operator in the if statement. — Amit Singh, Aug 09 '15 at 14:29

score 4 · Accepted Answer · edited May 23 '17 at 11:46

There are a few issues here.

Firstly, the variables you have in your query are strings, therefore they require to be quoted:

WHERE username = '$myuser' AND password = '$mypass'

Having or die(mysql_error()) to mysql_query() would have signaled the syntax error.

Then you're assigning instead of comparing with

if($result['username'] = $myuser && $result['password'] = $mypass;)

use two equals ==

However, that isn't how you check if those rows exist.

You need to use mysql_num_rows() or use a while loop while using a function to fetch/iterate over results found.

Here is an MySQLi example using mysqli_num_rows():

$conn=mysqli_connect("hostname","username","password","db");

$check_select = mysqli_query($conn, "SELECT * FROM `users` 
                                     WHERE email = '$email' AND pw='$pass'"); 

$numrows=mysqli_num_rows($check_select);

if($numrows > 0){
// do something
}

Now, we don't know where those variables have been assigned, and if from a form that it's using a POST method with matching name attributes.

I.e.:

<form action="" method="post">
<input type="text" name="username">
...
</form>

$username = $_POST['username'];

Another thing which is unknown to us is the MySQL API you're using to connect with. Make sure that you are indeed using the same one as you are using to query with, being mysql_. Different APIs do not intermix, such as mysqli_ or PDO. Use the same one from connection to querying.

Add error reporting to the top of your file(s) which will help find errors.

<?php 
error_reporting(E_ALL);
ini_set('display_errors', 1);

// rest of your code

Sidenote: Displaying errors should only be done in staging, and never production.

I noticed you may be storing passwords in plain text. If this is the case, it is highly discouraged.

I recommend you use CRYPT_BLOWFISH or PHP 5.5's password_hash() function. For PHP < 5.5 use the password_hash() compatibility pack.

Here is a PDO solution pulled from one of ircmaxell's answers:

https://stackoverflow.com/a/29778421/

Just use a library. Seriously. They exist for a reason.

PHP 5.5+: use password_hash()
PHP 5.3.7+: use password-compat (a compatibility pack for above)
All others: use phpass

Don't do it yourself. If you're creating your own salt, YOU'RE DOING IT WRONG. You should be using a library that handles that for you.

$dbh = new PDO(...);

$username = $_POST["username"];
$email = $_POST["email"];
$password = $_POST["password"];
$hash = password_hash($password, PASSWORD_DEFAULT);

$stmt = $dbh->prepare("insert into users set username=?, email=?, password=?");
$stmt->execute([$username, $email, $hash]);

And on login:

$sql = "SELECT * FROM users WHERE username = ?";
$stmt = $dbh->prepare($sql);
$result = $stmt->execute([$_POST['username']]);
$users = $result->fetchAll();
if (isset($users[0]) {
    if (password_verify($_POST['password'], $users[0]->password) {
        // valid login
    } else {
        // invalid password
    }
} else {
    // invalid username
}

This looks a lot cleaner then what I was doing to check. I am having a difficult time getting the if($numrows > 0) { } to work, I'm trying to research it to understand it better. Thanks! — Kacey Gambill, Aug 09 '15 at 14:40
The note about quotibg should come with a warning not to use because it is susceptible to SQL injection. The answer is correct about not doing it oneself, but a more explicit warning should still be there. — Arc, Aug 11 '15 at 11:31

score 1 · Answer 2 · answered Aug 09 '15 at 14:06

1

You should use == instead of simple = for your if condition

answered Aug 09 '15 at 14:06

rob

2,136
8
29
37

score 1 · Answer 3 · answered Aug 09 '15 at 14:08

1

First of all delete that if stmt and make new one where you check for num rows. If there is num rows > 0 you have valid login. And then print needed results from database od current query.

Edit:

You have = insted of == or === so stmt is always true.

answered Aug 09 '15 at 14:08

Stefan Cvetkovic

144
2
8

PHP, SQL, User Database

3 Answers3