Im working on a login script and i need it to check a field in my database table called "active" which stores a 0 or 1, 0 representing inactive account and a 1 representing an active account. When the user registers to the site it sets the active field to 0 by default. I need the login script to check when the account is active or inactive, if inactive display an error, if active let them login. Can anyone help me, the code commented starting at the line /* Check account is active or inactive */ is where im having difficulty.
my code is:
PHP Code:
<?php
$validation = "";
/**
* Checks whether or not the given username is in the
* database, if so it checks if the given password is
* the same password in the database for that user.
* If the user doesn't exist or if the passwords don't
* match up, it returns an error code (1 or 2).
* On success it returns 0.
*/
function confirmUser($username, $password){
global $conn;
/* Add slashes if necessary (for query) */
if(!get_magic_quotes_gpc()) {
$username = addslashes($username);
}
/* Verify that user is in database */
$q = "select password from users where username = '$username'";
$result = mysql_query($q,$conn);
if(!$result || (mysql_numrows($result) < 1)){
return 1; //Indicates username failure
}
/* Validate that password is correct */
if($password == $dbarray['password']){
return 0; //Success! Username and password confirmed
}
else{
return 2; //Indicates password failure
}
/* Check account is active or inactive */
$q = "select password,active from users where username = '$username'";
$sql = mysql_query($q);
$row = mysql_fetch_array($sql);
Personally, I'd do things a bit different. For security reasons, when dealing with logins I try to remain as ambigous as I can about why a login actually failed. After a few attempts I'd like freeze the account. If a person is having trouble logging in and can't figure out why I'd reccomend them to the Forgot My Password page where I'd issue a new password via signup email.
Secondly, I'd save a few queries and only run one query something like this:
PHP Code:
SELECT COUNT(*) FROM `users` WHERE username = '$username' AND password = '$password' AND active = 1;
If the count returns 1 then you know its a valid user/password combo else the login is invalid.
Finally, are you storing plain text passwords? If so, I'd recommend at the very leasting hashing them with a hash alogorithm
__________________
Most of my questions/posts are fairly straightforward and simple. I post long verbose messages in an attempt to be thorough.
Personally, I'd do things a bit different. For security reasons, when dealing with logins I try to remain as ambigous as I can about why a login actually failed. After a few attempts I'd like freeze the account. If a person is having trouble logging in and can't figure out why I'd reccomend them to the Forgot My Password page where I'd issue a new password via signup email.
Secondly, I'd save a few queries and only run one query something like this:
PHP Code:
SELECT COUNT(*) FROM `users` WHERE username = '$username' AND password = '$password' AND active = 1;
If the count returns 1 then you know its a valid user/password combo else the login is invalid.
Finally, are you storing plain text passwords? If so, I'd recommend at the very leasting hashing them with a hash alogorithm
Hi thanks for the reply, im having difficulty with the following code below, im i doing this completing wrong. Im storing the passwords as plain text btw
PHP Code:
$q = "select password,active from users where username = '$username'";
$sql = mysql_query($q);
$row = mysql_fetch_array($sql);
What data type is the column active? If its an int then you need to remove the quotes from the 0. You may also want to check and see if register globals is off. If not someone could just do something like this. http://yoursite.com/login.php?result=1&username=Aero. This will now allow a user with username Aero access to wherever you don't want them to be. You really should be hashing the passwords. Here is a good article on writing secure php. http://www.ilovejackdaniels.com/php/writing-secure-php/
__________________
||||If you are getting paid to do a job, don't ask for help on it!||||
What data type is the column active? If its an int then you need to remove the quotes from the 0. You may also want to check and see if register globals is off. If not someone could just do something like this. http://yoursite.com/login.php?result=1&username=Aero. This will now allow a user with username Aero access to wherever you don't want them to be. You really should be hashing the passwords. Here is a good article on writing secure php. http://www.ilovejackdaniels.com/php/writing-secure-php/
Aerospace thanks for the reply, the active field type is "int", i have removed the quotes but it still doesnt check if the user is active or not. Is there anythin else i can try?
My code is
Code:
function confirmUser($username, $password){
global $conn;
/* Add slashes if necessary (for query) */
if(!get_magic_quotes_gpc()) {
$username = addslashes($username);
}
/* Verify that user is in database */
$q = "select password, active from users where username = '$username'";
$result = mysql_query($q,$conn);
if(!$result || (mysql_numrows($result) < 1)){
return 1; //Indicates username failure
}
/* Retrieve password from result, strip slashes */
$dbarray = mysql_fetch_array($result);
$dbarray['password'] = stripslashes($dbarray['password']);
$password = stripslashes($password);
/* Validate that password is correct */
if($password == $dbarray['password']){
return 0; //Success! Username and password confirmed
}
else {
return 2; //Indicates password failure
}
else if($row['active']=='0'{
return 3; //Indicates inactive account
}
}
Last edited by PRodgers4284; 02-21-2008 at 08:20 PM..
I've made it a little more secure. I would do something like this
PHP Code:
<?php function confirmUser($username, $password) { $val = ''; global $conn; // check to see if magic_quotes_gpc is 1 or 0 if (ini_get('magic_quotes_gpc')) { // if 0 stripslashes $username = stripslashes($username); } // use mysql_real_escape_string since its what its meant for in place of addslashes $username = mysql_real_escape_string($username,$conn);
/* Verify that user is in database */ $q = "select password, active from users where username = '$username'"; $result = mysql_query($q,$conn) or die(mysql_error()); if(mysql_num_rows($result) != 1) { $val = 1; //Indicates username failure } else if(mysql_num_rows($result) == 1) { /* Retrieve password from result, strip slashes */ $dbarray = mysql_fetch_array($result); $dbpass = stripslashes($dbarray['password']); $password = stripslashes($password); $active = $dbarray['active'];
// if they are a user and account is active check their password // if they are a user but account is inactive then don't check their password and return 3 for inactive\ $val = ($active == 1) ? (strcmp($dbpass,$password) == 0) ? 0 : 2 : 3; // the above is a ternary operator in a ternary operator. Its the equivalent of the below // if($active == 1) // { // if(strcmp($dbpass,$password) == 0) // { // $val = 0; // } // else // { // $val = 2; // } // } // else // { // $val = 3; // }
} else { // if some how the if else clause gets to here it will set $val to -1 $val = -1; } // finally we return val // $val = 0 username and password are good // $val = 2 password is wrong but again just use a stand error like Login Error // $val = 3 account is inactive // $val = -1 there is likely the same username twice in the database or something went wrong return $val; } ?>
Usage:
PHP Code:
echo confirmUser('Aero','dog'); // will return an integer according to the check
Read the comments as they help you learn.
__________________
||||If you are getting paid to do a job, don't ask for help on it!||||