0

Before you say it: I know the passwords should be encrypted/hashed, but I want to get this down first: I have this login function and a SQL database. However, the login function doesn't seem to work and I haven't the faintest idea why. I am probably missing something stupid, but have been struggling with this for a while now. Any help would be appreciated!

NOTE: the file db_connect.php is really just a basic connecting to the database, nothing wrong there

FUNCTION.PHP:

<?
function login($username, $password, $con) 
{       
    $myQuery = "SELECT * FROM Members WHERE Username = '$username' and Password = '$password';";
    $result = mysqli_query($con, $myQuery);

    if (mysql_num_rows($result) == 0)
    {
        return false;
    } 
    else
    {
return true;
    }
}
?>

PROCESS-LOGIN.PHP:

<?php
include 'db_connect.php';
include 'functions.php';

if (isset($_POST['username'], $_POST['pword'])) {
    $username = $_POST['username'];
    $password = $_POST['pword']; // The hashed password.

    if (login($username, $password) == true) {
        // Login success 
        header('Location: welcome.html');
    } 
    else 
    {
        // Login failed 
        header('Location: index.html');
    }
} 
else {
    // The correct POST variables were not sent to this page. 
    echo 'Invalid Request';
}
?>
  • Get into the habit, when using database calls, of testing at _every_ stage of the process. When you connect, test the connection. When you prepare a query, test the result. When you run a query, test the result. When you fetch results, test that too! The function `mysqli_query` returns `false` if there's a problem. – halfer Dec 09 '13 at 23:34

4 Answers4

1

You are mixing MySQLi (mysqli_query) with MySQL (mysql_num_rows) - decide for either one (preferably the former).

If you are using MySQL, the parameters for mysql_query are in wrong order.

In addition to that you are failing to pass the connection to the login as a parameter (as WoLfulus mentioned).

Some additional info as you seem to be learning:

  • The return statement of login can be simplified to return mysql_num_rows($result) == 1;. This will return TRUE if one record was found and FALSE otherwise - no need for an if/else statement here, you already have the logic you need.
  • Right now anyone can access welcome.html without logging in by simply typing the address in the browser. This can be avoided by using sessions.
  • Since you don't properly escape the user input (which one should never trust!), you are vulnerable to SQL injections. mysql_real_escape_string is a start but no 100% solution. If you used prepared statements on the other hand, you wouldn't need to worry.
Community
  • 1
  • 1
kero
  • 10,647
  • 5
  • 41
  • 51
  • thanks for the response but its still not working. I added $con and standardized to mysql_*. Still does not work... –  Dec 09 '13 at 23:50
  • "Does not work" is awfully vague. Do as halfer said in the comment on your question and try to debug as much as possible. Make use of `mysql_error()` as well – kero Dec 09 '13 at 23:52
  • Sorry. There is no error being returned. Debugging is proving unhelpful. –  Dec 09 '13 at 23:54
1

You are not providing the $con parameter to login function.

function login($username, $password, $con)

You are calling it as 

login($username, $password)

Try providing the connection argument to see if it works.

Also note the answer kingkero made. You are using functions from different libraries.

WoLfulus
  • 1,948
  • 1
  • 14
  • 21
1

Some things I noticed

  • Are you using method="POST" in your form?
  • Your SQL query is vulnerable to SQL injections
  • your mixing mysql_* with mysqli_* functions
  • missing $con parameter for login function
Philipp
  • 15,377
  • 4
  • 35
  • 52
0

I'm answering since I don't have enough reputation to comment your question.. But you should keep your variables outside the quotes and add mysql_real_escape_string() to prevent mysql injection..

$myQuery = "SELECT * FROM Members WHERE Username = '$username' and Password = '$password';";

Should be:

$myQuery = "SELECT * FROM Members WHERE Username = '". mysql_real_escape_string($username) ."' and Password = '". mysql_real_escape_string($password) ."';";
  • Why? Inside double quotes it is ok to have variables, is it not? – kero Dec 09 '13 at 23:30
  • No! It's not - have a look at prepared statements or escaping: http://php.net/manual/de/mysqli.prepare.php – Philipp Dec 09 '13 at 23:33
  • @Philipp My comment was just about the concatenation contra to having it inside the query (as halfer mentioned). Also [`mysql_real_escape_string` doesn't fully protect against SQL injections](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) – kero Dec 09 '13 at 23:41