cozzy1984 Posted February 6, 2008 Share Posted February 6, 2008 Have been using a script i got from evolt.com for a login and modified it slightly, and it works. Although i am now looking to check it the account is active or not and if so login and if not display a message. Have added 'active' field to database which is an int of deafult '0'. here is my code: <?php $loginerror = ""; /** * 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 } /* 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 } } /** * checkLogin - Checks if the user has already previously * logged in, and a session with the user has already been * established. Also checks to see if user has been remembered. * If so, the database is queried to make sure of the user's * authenticity. Returns true if the user has logged in. */ function checkLogin(){ /* Check if user has been remembered */ if(isset($_COOKIE['cookname']) && isset($_COOKIE['cookpass'])){ $_SESSION['username'] = $_COOKIE['cookname']; $_SESSION['password'] = $_COOKIE['cookpass']; } /* Username and password have been set */ if(isset($_SESSION['username']) && isset($_SESSION['password'])){ /* Confirm that username and password are valid */ if(confirmUser($_SESSION['username'], $_SESSION['password']) != 0){ /* Variables are incorrect, user not logged in */ unset($_SESSION['username']); unset($_SESSION['password']); return false; } return true; } /* User not logged in */ else{ return false; } } /** * Determines whether or not to display the login * form or to show the user that he is logged in * based on if the session variables are set. */ function displayLogin(){ global $loginerror; global $logged_in; if($logged_in){ echo "<div id='userbox'> <h2 class='first'>Welcome $_SESSION[username],</h2> <ul> <li><a href=''>My Ads</a></li> <li><a href=''>Create Advert</a></li> <li><a href=''>My Details</a></li> <li class='logout'><a href='logout.php'>Logout</a></li> </ul> </div>"; } else{ include "loginform.php"; echo "<p class='error' style='color:#d72b1a; font: bold 11px/1.2 Arial, Helvetica, sans-serif; text-align:center;'>$loginerror</p>"; } } /** * Checks to see if the user has submitted his * username and password through the login form, * if so, checks authenticity in database and * creates session. */ if(isset($_POST['sublogin'])){ $_POST['user'] = trim($_POST['user']); /* Checks that username is in database and password is correct */ $md5pass = md5($_POST['pass']); $result = confirmUser($_POST['user'], $md5pass); /* Check that all fields were typed in */ if(!$_POST['user'] || !$_POST['pass']){ $loginerror = "You didn't fill in a required field"; } /* Spruce up username, check length */ else if(strlen($_POST['user']) > 30){ $loginerror = "Username is longer than 30 characters"; } /* Check error codes */ else if($result == 1){ $loginerror = "Username doesn't exist"; } else if($result == 2){ $loginerror = "Incorrect Password"; } /* Username and password correct, register session variables */ $_POST['user'] = stripslashes($_POST['user']); $_SESSION['username'] = $_POST['user']; $_SESSION['password'] = $md5pass; /** * This is the cool part: the user has requested that we remember that * he's logged in, so we set two cookies. One to hold his username, * and one to hold his md5 encrypted password. We set them both to * expire in 100 days. Now, next time he comes to our site, we will * log him in automatically. */ if(isset($_POST['remember'])){ setcookie("cookname", $_SESSION['username'], time()+60*60*24*100, "/"); setcookie("cookpass", $_SESSION['password'], time()+60*60*24*100, "/"); } } /* Sets the value of the logged_in variable, which can be used in your code */ $logged_in = checkLogin(); ?> was thinking of adding: $q = "select password,active from users where username = '$username'"; and then else if(active=='0'{ return 3; //Indicates inactive account } and of course the error at the bottom. Am i on the right course or not? Quote Link to comment Share on other sites More sharing options...
revraz Posted February 6, 2008 Share Posted February 6, 2008 Yep. Quote Link to comment Share on other sites More sharing options...
cozzy1984 Posted February 6, 2008 Author Share Posted February 6, 2008 nice one, wasn't sure if i was able to refer to active =='0' saying it was from the query $q = "select password,active from users where username = '$username'"; Thought i might have to somehow put it into another variable. Altho in saying that it doesn't seem to be working the way I tried. Quote Link to comment Share on other sites More sharing options...
revraz Posted February 6, 2008 Share Posted February 6, 2008 I didn't know you meant it literally, I thought you meant the idea. You still need to fetch the data. Quote Link to comment Share on other sites More sharing options...
The Little Guy Posted February 6, 2008 Share Posted February 6, 2008 It would look more like this: <?php$q = "select password,active from users where username = '$username'"; $sql = mysql_query($q); $row = mysql_fetch_array($sql); // your code here else if($row['active']=='0'{ return 3; //Indicates inactive account } ?> Quote Link to comment Share on other sites More sharing options...
Wolphie Posted February 6, 2008 Share Posted February 6, 2008 A query to confirm a login would be more secure if it was similar to <?php session_start(); $password = $_POST['password']; $password = mysql_real_escape_string($password); $password = htmlentities($password); $password = htmlspecialchars($password); $password = MD5($password); $username = $_POST['username']; $username = mysql_real_escape_string($username); $username = htmlentities($username); $username = htmlspecialchars($username); $sql = sprintf( "SELECT * FROM `users` WHERE `username` = '%s' AND `password` = '%s' LIMIT 1", $username, $password ); $sql = mysql_query( $sql ) or die( 'Error: ' . mysql_error() ); $obj = mysql_fetch_object( $sql ); if ( $obj ) { $_SESSION['logged'] = true; echo 'Logged in successfully'; } else { $_SESSION['logged'] = false; echo 'Invalid details'; } ?> But the answer to your question is yes, you are on the right track.[ Quote Link to comment Share on other sites More sharing options...
cozzy1984 Posted February 7, 2008 Author Share Posted February 7, 2008 Thanks for all your help, that worked well 'The Little Guy'. Am still trying to learn PHP and am doing a website involving a lot of it for university so your help is much appreciated. Still don't know much about the security aspect of it all but would like to look into it more when i get time but think i'll be starting on getting php mailer set up and email registration and activation next. Is the code i am using very unsecure Wolphie? Quote Link to comment Share on other sites More sharing options...
trq Posted February 7, 2008 Share Posted February 7, 2008 Wolphie's code is way overboard and most of it is simply not needed. While mysql_real_escape_string() should be used on all user inputted data, stripping html out of a user name should be done at time of registration. Simply make sure you limit the chars a user can have in there user name. Quote Link to comment Share on other sites More sharing options...
Wolphie Posted February 7, 2008 Share Posted February 7, 2008 Woops, yes you're right Thorpe - Since i'm only selecting the data, not writing it. Thanks for the clarification. Quote Link to comment Share on other sites More sharing options...
Wolphie Posted February 7, 2008 Share Posted February 7, 2008 As for your code being un-secure. It is vulnerable i must say, using sprintf() with every query that requires a variable should be used. And so should mysql_real_escape_string() with every user inputted data as Thorpe stated above. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.