Logon authorisation problem

[chieftan]

Hopefully someone can help me,

I am trying to create a members area where a user ID and password is entered to access the area. If the user does not exist they have to create one and this is entered into a mysql database (this bit works fine), but when the user then tries to logon it fails in the code and I cannot find where, the code for the logon inspection is written below:-

<?php

sessiom_start();

if (isset($_POST['userid']) && isset($_POST['pass']))

//if the user has just tried to login
$username="web21-logon";
$password="logon";
$database="web21-logon";

$userid = $_POST['userid'];
$newpass = $_POST['pass'];

mysql_connect(localhost,$username,$password);

@mysql_select_db($database) or die( "Unable to select database");

$query = 'select * from login'
."where Username='$userid'"
." and Password='$newpass'";

$result=mysql_query($query);
if ($result->num_rows >0)
{
//if they are in the database register the user ID
$_SESSION['valid_user']=$userid;
}
mysql_close();

?>

<h1>Home page</h1>
<?
if (isset($_SESSION['valid_user']))
{
echo 'You are logged in as: '.$_SESSION['valid_user'].' <br />';
echo '<a href="logout.php">Log out</a><br />;
}

?>

 

[jpadie]

this line

mysql_connect(localhost,$username,$password);

is probably incorrect. you should put localhost in quotes.

you should also, as a matter of practice, trap errors:

mysql_connect(localhost,$username,$password) or die (mysql_error();

add a space in front of the word "where" in your sql

session_start();

should be

session_start();

a better sql block might look like this:

<?
// use count so that you are only ever retrieving the 
// minimum information necessary for your query

$query = "	select 
 				count(*)  
			from 
				login
 			where 
				Username='$userid' 
                and 
				Password='$newpass'";
                   
$result=	mysql_query($query);
//add error checking 
if ($result === FALSE) die(mysql_error()); 

//	compare to a known quantity.  as userid must be unique ensure that 
//  resultant count value is equal to 1
if (mysql_result($result,0,0) == 1) {
	//if they are in the database register the user ID
	$_SESSION['valid_user']=$userid;
} else { 
	// you should explicitly unset the element just in case
	// it was previously set for the session.
	
	unset($_SESSION['valid_user']); }
?>

 

[jpadie]

i would also add that you need to escape and cleanse your variables before using them in a query.

ie

$userid = trim($_POST['userid']);
$newpass = trim($_POST['pass']);

$query = "    select
                 count(*)  
            from
                login
             where
                Username='".mysql_escape_string($userid)."'
                and 
              Password='".mysql_escape_string($newpass)."'";

lastly ... it's not a great idea to store password in clear text in a database. consider using crypt() or at least md5() to encode your passwords.