[SOLVED] Member System Function Trouble

[eRott]

Hey there,

 

I have a simple question regarding my current user login function. When a login attempt fails, an error message should be returned and displayed to the user. That however, is not the case. Nothing is returned and no PHP following the login form is executed (like when you use exit;). You will find the code below. Any ideas? Thanks.

 

index.php

if (!isset($_POST['login']))
{
include 'global/inc/login_form.inc.php';
}
else
{
// Attempt to log in with the provided username & password
$result = user_login(strip_tags(htmlspecialchars(strtolower($_POST['username']))), strip_tags(htmlspecialchars(strtolower($_POST['password']))));

if ($result == FALSE)
{
	// Reshow the form with the error
	$login_error = 'Incorrect login information';
	include 'global/inc/login_form.inc.php';
}
else
{
	$username = $_SESSION['username'];
 	$query = "SELECT usergroup FROM users WHERE username = '$username'";
	$result = mysql_query($query) or die(mysql_error());
	$row = mysql_fetch_assoc($result);

	if ($row['usergroup'] == "1") {
		echo 'Login successful...';
		echo '<meta http-equiv="refresh" content="0;url=admin.php" />';
	} else {
		echo 'Login successful...';
		echo '<meta http-equiv="refresh" content="0;url=member.php" />';
	}
}
}

 

login_form.inc.php

<?php
if (isset($login_error)) {
echo "There was an error: <b>" . $login_error . "</b>, please try again.<br /><br />";
}
?>

<form method="post">
<input class="input" type="text" name="username" value="username" maxlength="8" onfocus="if (this.value=='username')this.value='';" onblur="if (this.value=='')this.value='username'" />
<input class="input" type="password" name="password" value="password" maxlength="16" onfocus="if (this.value=='password')this.value='';" onblur="if (this.value=='')this.value='password'" />
<input class="button" type="submit" name="login" value="login" />
</form>

 

user_login()

function user_login($username, $password)
{
// Grab the salt from the database using the username
$query = "SELECT salt FROM users WHERE username='$username' limit 1";
$result = mysql_query($query);
$user = mysql_fetch_array($result);

// Using the salt, encrypt the given password to see if it
// matches the one in the database
$encrypted_pass = md5(md5($password).$user['salt']);

// Grab the user information using the username & encrypted pass
$query = "SELECT userid, username, usergroup FROM users WHERE username='$username' AND password='$encrypted_pass'";
$result = mysql_query($query);
$user = mysql_fetch_array($result);
$numrows = mysql_num_rows($result) or die(mysql_error());

// Store the data in the session
$_SESSION['userid'] = $user['userid'];
$_SESSION['username'] = $user['username'];
$_SESSION['usergroup'] = $user['usergroup'];

if ($numrows == 1)
{
	return TRUE;
}
else
{
	return FALSE;
}
}

[waynew]

FOR THE LOVE OF GOD, LEARN TO DEBUG YOUR CODE.

 

My apologies for shouting; but it's for your own good.

 

Firstly; lets see if you have an error with you query.

 

Change:

 

// Grab the user information using the username & encrypted pass
$query = "SELECT userid, username, usergroup FROM users WHERE username='$username' AND password='$encrypted_pass'";
$result = mysql_query($query);

 

To

 

// Grab the user information using the username & encrypted pass
$query = "SELECT userid, username, usergroup FROM users WHERE username='$username' AND password='$encrypted_pass'";
$result = mysql_query($query) or trigger_error(mysql_error()); //check if query is ok

 

And see what happens.

[eRott]

Debugging is just what I am trying to do. No worries though =). Your shouting has had no damaging effects upon my hearing; it has only put a damper on my ego. So I'm a little forgetful. I will note though, I normally just use die(). Either way, no, there is not problem with the query. Same result as described in my first post.

[waynew]

Could you echo out $numrows and see what you get?

[eRott]

Hmm. Well, to be honest, I'm not too sure how or where. $numrows simply contains the number of rows in the result of the previous query (which retrieves the userid, username and usergroup). With the way I have it setup, it is limited by 1 result. Meaning $numrows will always return either a 1 or what I assume would be a 0 (when there are no rows).

 

EDIT: I have tried:

 

if ($numrows == 1)
{
	return TRUE;
}
else
{
	echo $numrows;
}

 

// Try and login with the given username & pass
$result = user_login(strip_tags(htmlspecialchars(strtolower($_POST['username']))), strip_tags(htmlspecialchars(strtolower($_POST['password']))));
echo $result;

 

I am unsure if that is what you were asking me to do.

[Maq]

You don't tell your form where to submit.

 

Another problem is that "login_form.inc.php" doesn't have access and doesn't know what "$login_error" even is.

 

The last problem is that I don't see session_start() anywhere in your scripts.  Wherever you use sessions you need this.

 

* NOT TESTED *  - Put function on the same page along with the submission.

 

session_start();
if(isset($_POST['login']))
{
   // Attempt to log in with the provided username & password
   $result = user_login(strip_tags(htmlspecialchars(strtolower($_POST['username']))), strip_tags(htmlspecialchars(strtolower($_POST['password']))));
   if($result === FALSE)
   {
      //Reshow the form with the error
      $login_error = 'Incorrect login information';
      include 'global/inc/login_form.inc.php';
   }
   else
   {
      $username = $_SESSION['username'];
      $query = "SELECT usergroup FROM users WHERE username = '$username'";
      $result = mysql_query($query) or die(mysql_error());
      $row = mysql_fetch_assoc($result);
      
  if ($row['usergroup'] == "1") {
         echo 'Login successful...';
         echo '';
      } 
  else 
  {
         echo 'Login successful...';
         echo '';
      }
   }
   if (isset($login_error)) {
      echo "There was an error: " . $login_error . ", please try again.

";
   }
}
?>

</pre>
<form action="<?php%20echo%20%24_SERVER%5B'PHP_SELF'%5D;%20?>" method="POST">



</form>
<br><br><br>function user_login($username, $password)<br>{<br>   // Grab the salt from the database using the username<br>   $query = "SELECT salt FROM users WHERE username='$username' limit 1";<br>   $result = mysql_query($query);<br>   $user = mysql_fetch_array($result);<br><br>   // Using the salt, encrypt the given password to see if it<br>   // matches the one in the database<br>   $encrypted_pass = md5(md5($password).$user['salt']);<br><br>   // Grab the user information using the username & encrypted pass<br>   $query = "SELECT userid, username, usergroup FROM users WHERE username='$username' AND password='$encrypted_pass'";<br>   $result = mysql_query($query);<br>   $user = mysql_fetch_array($result);<br>   $numrows = mysql_num_rows($result) or die(mysql_error());<br><br>   // Store the data in the session<br>   $_SESSION['userid'] = $user['userid'];<br>   $_SESSION['username'] = $user['username'];<br>   $_SESSION['usergroup'] = $user['usergroup'];<br>   <br>   if ($numrows == 1)<br>   {<br>      return TRUE;<br>   }<br>   else<br>   {<br>      return FALSE;<br>   }<br>}<br><br

[eRott]

1] It posts to the same page and so the form action is not required.

2] Currently investigating --> Update: removed $login_error entirely and opted for straight echo.

3] Aye. The session is started. I just didn't paste everything. Too much code.

 

I mean, everything works just fine when a user provides the correct login information. What needs to happen, happens. The only problem is when a user enters incorrect login information. It is then that the problem, as I described in my first post, occurs.

 

When a login attempt fails, an error message should be returned and displayed to the user. That however, is not the case. Nothing is returned and no PHP following the login form is executed (like when you use exit;). You will find the code below. Any ideas? Thanks.

 

I have gone ahead and removed the $login_error variable entirely and opted simply for just echoing an error (I have no idea why I didn't just do that in the first place...). Which rules out the variable being the problem. As a result, I am fairly certain it has something to do with the function itself; relating to this bit of code (and $numrows) in particular:

 

if ($numrows == 1)
{
return TRUE;
}
else
{
return FALSE;
}

 

I've gone ahead an copied the updated code.

 

index.php

<?php

if (!isset($_POST['login']))
{
include 'global/inc/login_form.inc.php';
}
else
{
// Attempt to login with the given username & password
$result = user_login(strip_tags(htmlspecialchars(strtolower($_POST['username']))), $_POST['password']);

if ($result == FALSE)
{
	// Reshow the form with the error
	echo "Incorrect login information, please try again.";
	include 'global/inc/login_form.inc.php';
}
elseif ($result == TRUE)
{
	$username = $_SESSION['username'];
	$query = "SELECT usergroup FROM users WHERE username = '$username'";
	$result = mysql_query($query) or die(mysql_error());
	$row = mysql_fetch_assoc($result);

	if ($row['usergroup'] == "1") {
		echo 'Login successful...';
		echo '<meta http-equiv="refresh" content="0;url=admin.php" />';
	} else {
		echo 'Login successful...';
		echo '<meta http-equiv="refresh" content="0;url=member.php" />';
	}
}
}

?>

 

login_form.inc.php

<form method="post">
<input class="input" type="text" name="username" value="username" maxlength="8" onfocus="if (this.value=='username')this.value='';" onblur="if (this.value=='')this.value='username'" />
<input class="input" type="password" name="password" value="password" maxlength="16" onfocus="if (this.value=='password')this.value='';" onblur="if (this.value=='')this.value='password'" />
<input class="button" type="submit" name="login" value="login" />
</form>

 

user_login()

<?php
function user_login($username, $password)
{
// Grab the salt from the database using the username
$query = "SELECT salt FROM users WHERE username='$username' limit 1";
$result = mysql_query($query);
$user = mysql_fetch_array($result);

// Using the salt, encrypt the given password to see if it
// matches the one in the database
$encrypted_pass = md5(md5($password).$user['salt']);

// Grab the user information that matches the username & encrypted password
$query = "SELECT userid, username, usergroup FROM users WHERE username='$username' AND password='$encrypted_pass'";
$result = mysql_query($query);
$user = mysql_fetch_array($result);
$numrows = mysql_num_rows($result) or die(mysql_error());

// Store the data in the session
$_SESSION['userid'] = $user['userid'];
$_SESSION['username'] = $user['username'];
$_SESSION['usergroup'] = $user['usergroup'];

if ($numrows == 1)
{
	return TRUE;
}
else
{
	return FALSE;
}
}
?>

[Maq]

Change this portion to:

- Added some error checking.

- Echoed out your second query.

- Only assigned the session variables if the query returned > 0 rows.

 

   // Grab the user information that matches the username & encrypted password
   $query = "SELECT userid, username, usergroup FROM users WHERE username='$username' AND password='$encrypted_pass'";
   echo "Query=> " . $query;
   $result = mysql_query($query) or die(mysql_error());
   $user = mysql_fetch_array($result);
   $numrows = mysql_num_rows($result) or die(mysql_error());
   if ($numrows > 0)
   {
      // Store the data in the session
      $_SESSION['userid'] = $user['userid'];
      $_SESSION['username'] = $user['username'];
      $_SESSION['usergroup'] = $user['usergroup'];
      return TRUE;
   }
   else
   {
      return FALSE;
   }

 

You should also put this at the top of your script, for syntactical sake:

 

ini_set ("display_errors", "1");
error_reporting(E_ALL);

[eRott]

The query:

 

Query=> SELECT userid, username, usergroup FROM users WHERE username='qwerty' AND password='3fe2caaf8b3560f6434ce9620450857f'

 

The result of error_reporting:

 

Notice: Use of undefined constant n - assumed 'n' in /home/erott/public_html/subdue/index.php on line 45

Notice: Use of undefined constant Y - assumed 'Y' in /home/erott/public_html/subdue/index.php on line 46

Notice: Use of undefined constant j - assumed 'j' in /home/erott/public_html/subdue/index.php on line 47

 

Lines 45, 46 and 47:

 

$month = date(n);
$year = date(Y);
$today = date(j);

[eRott]

shameful bump incoming...

 

The solution to the date() notices was to surround the format in single quotations.

 

$month = date('n');
$year = date('Y');
$today = date('j');

 

Other then that, does anyone have any thoughts as to what's causing my problem as explained in this previous post? I will continue my debugging efforts in the meantime.

 

Thanks.

[eRott]

Changing:

 

$numrows = mysql_num_rows($info) or die(mysql_error());

 

To:

 

$numrows = mysql_num_rows($info);

 

Solves the problem. I believe what was happening was when a set of incorrect login information is passed the the function, nothing is returned / selected from the database because the user obviously doesn't exist. As a result, I'm guessing mysql_num_rows() either simply doesn't fire or returns some invalid value.. or something. Either way, using die() resulted in the script termination (for whatever reason) which prevented (obviously) the error message and anything else from being displayed.

 

EDIT: Working user_login()

function user_login($username, $password)
{
// Grab the salt from the database using the username
$salt = mysql_query("SELECT salt FROM users WHERE username = '".$username."' LIMIT 1") or die(mysql_error());
$user = mysql_fetch_array($salt);

// Using the salt, encrypt the given password to see if it
// matches the one in the database
$encrypted_pass = md5(md5($password).$user['salt']);

// Grab the user information that matches the username & encrypted password
$info = mysql_query("SELECT userid, username, usergroup FROM users WHERE username = '".$username."' AND password = '".$encrypted_pass."' LIMIT 1") or die(mysql_error());
$user = mysql_fetch_array($info);
$numrows = mysql_num_rows($info);

if ($numrows > 0)
{
	// Store the data in the session
	$_SESSION['userid'] = $user['userid'];
	$_SESSION['username'] = $user['username'];
	$_SESSION['usergroup'] = $user['usergroup'];

	return TRUE;
}
else
{
	return FALSE;
}
}

 

Thank you waynewex and Maq.

Take it easy.