0

I can't for the life of me figure out why this code doesn't work. I'm new to MySQL queries so I may be doing something wrong there.

Here's my login.php

<?php
/**
 * Created by PhpStorm.
 * User: Michael
 * Date: 10/25/2015
 * Time: 4:35 AM
 */
require 'connect.php';

if(empty($_POST['Login_Username']) || empty($_POST['Login_Password'])){
    header("Location: http://www.socksdevsite.com/PHP_Files/Display_Files/Login/displayloginfailed.php");
} else {
    $username = "'DevSock'";
    if($db->query('SELECT username FROM users WHERE username = $username')){
        echo "Found user";
    }else {
        echo "Didn't find user";
    }
}

My connect.php

<?php
/**
 * Created by PhpStorm.
 * User: Michael
 * Date: 10/25/2015
 * Time: 11:58 AM
 */

$db = mysqli_connect('127.0.0.1', 'username', 'password') or die("Error logging in. Please notify an administrator!");
$db->select_db('Website_Storage');
echo "Connected and DB selected <br>";

And finally a screenshot of my phpMyAdmin.

enter image description here

halfer
  • 19,824
  • 17
  • 99
  • 186
Dev Sock
  • 171
  • 1
  • 2
  • 12
  • what's your output like – Ahmad Hammoud Oct 25 '15 at 20:18
  • You should check the `rowCount` instead just trying if the query ran. Your query will run whatever username you put. – Akshay Oct 25 '15 at 20:18
  • @Akshay why do I need to use rowCount here? I'm checking if the data submitted in the form is equal to the data within the mysql column. It would only return true if that were the case right? – Dev Sock Oct 25 '15 at 20:25
  • Aside from the SQL injection (mentioned in an answer below) your screenshot suggests you are not hashing your passwords - which will put your users' security at risk if you do get hacked. That needs changing as well. – halfer Oct 25 '15 at 20:36
  • @DevSock - the way your code is working now is that you're checking to see if your query runs successfully, and assuming that if it does, then it's a valid login. But it's possible for a successful query to not return any rows, so you need instead to check how many rows you get returned. Does that make it clearer? – andrewsi Oct 26 '15 at 01:46

2 Answers2

2

Firstly your problem is the following:

WHERE username =$username'

Secondly change the code to: 

WHERE username ='$username'"

Because $username is a string check out out when to use single and double quotes

Lastly you should use prepared statements for the login system, you are already using mysqli_ so the transition shouldn't be too difficult. Something like this:

if ($stmt = $mysqli->prepare('SELECT password FROM users WHERE username = ?')) {
    // Bind parameters (s = string, i = int, b = blob, etc), hash the password using the PHP password_hash function.
    $username = $_POST['username']; 
    $stmt->bind_param('s', $username);
    if(!$stmt->execute()){
    trigger_error("there was an error....".$mysqli->error, E_USER_WARNING);
    } 
    $stmt->store_result();

You could also use PDO! Check out this, Stackoverflow on PDO and mysqli pros and cons

Check out this SO question on Stopping SQL injections

Community
  • 1
  • 1
Small Legend
  • 733
  • 1
  • 6
  • 20
  • $username contains the single quotes so why would there be a need to include them within the query. Wouldn't that cause an issue instead? – Dev Sock Oct 25 '15 at 20:22
  • Like I said I'm new to the whole MySQL thing. What exactly are prepared statements? – Dev Sock Oct 25 '15 at 20:23
  • Strings should be inserted with 'string'. Prepared statements are more sercure – Small Legend Oct 25 '15 at 20:25
  • I'm confused, because I don't even know what a prepared statement is to begin with. – Dev Sock Oct 25 '15 at 20:28
  • @DevSock any luck in implementing? – Small Legend Oct 25 '15 at 20:47
  • Its all relatively confusing for me, so I've been googling the sections of your code to figure out what they do. I haven't been able to successfully implement it yet though. I didn't even know there was a password hash function and I have no clue how to use it. Nor do I know what you mean by bind parameters. So I'm just trying to work to figure it out. – Dev Sock Oct 25 '15 at 20:49
  • It was all confusing to me too just over a week ago. You'll get the hang of it mate@DevSock – Small Legend Oct 25 '15 at 20:50
  • bind_param — Binds variables to a prepared statement as parameters@DevSock – Small Legend Oct 25 '15 at 20:52
  • I've added another link I recommend you check out @DevSock it helped me out a lot. – Small Legend Oct 25 '15 at 21:08
  • 1
    You're an absolute miracle worker my friend. I've got it working now because of you and I truly understand whats going on. +1 and marked as the answer for this question. – Dev Sock Oct 25 '15 at 21:14
  • @DevSock I'm glad I helped, good job mate! – Small Legend Oct 25 '15 at 21:15
0

Using PDO you can use something like this:

if (empty($_POST['Login_Username']) || empty($_POST['Login_Password'])){
    header("Location: http://www.socksdevsite.com/PHP_Files/Display_Files/Login/displayloginfailed.php");
} else {
    //Make the connection using PDO
    try {
        $conn = new PDO("mysql:host=localhost;dbname=mysql", $username, $password);
        echo "PDO connection object created";
        $username = "DevSock";
        //SQL query
        $sql = "SELECT username FROM users WHERE username = :username";
        //Prepare your query
        $stmt = $conn->prepare($sql);
        //Execute your query binding variables
        $stmt->execute(array(':username' => $username));
        //Fetch all results
        $result = $stmt->fetchAll();
        if (count($result) > 0) {
            foreach ($result as $row) {
                //Do something here
                echo '<p>'.$row['username'].'</p>';
            }
        } else {
            echo "Didn't find user";
        }
    }
    catch(PDOException $e) {
        echo $e->getMessage();
    }
}

You can find examples for PDO here: http://www.phpro.org/tutorials/Introduction-to-PHP-PDO.html

Kostas Mitsarakis
  • 4,772
  • 3
  • 23
  • 37