TylerRichardson Posted June 8, 2013 Share Posted June 8, 2013 I have some login script code that i got from some tutorial and it works perfectly on another site of mine but i need to modify it to work with a new website: budgetme.org I can open the login form and submit it but the way the checkuser script is written it redirects to a login success and a login fail page respectivly. I want it to simple give a "wrong username" above the login form or redirct to the home page with the user logged in. Can someone please tell me how i can do this??? the jquerry im using for the login box is located on the website home page. Thanks so much!! Here is the check user script im using (modified to work with my system). <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <title>Check User</title> </head> <body> <? /* Check User Script */ // Start Session include 'db.php'; // Conver to simple variables $username = $_POST['username']; $password = $_POST['password']; if((!$username) || (!$password)){ echo "Please enter your username and password. <br />"; exit(); } // Convert password to md5 hash $password = md5($password); // check if the user info validates the db $sql = mysql_query("SELECT * FROM users WHERE username='$username' AND password='$password' "); $login_check = mysql_num_rows($sql); if($login_check > 0){ while($row = mysql_fetch_array($sql)){ foreach( $row AS $key => $val ){ $$key = stripslashes( $val ); } // Register some session variables! session_register('id'); $_SESSION['id'] = $id; session_register('username'); $_SESSION['username'] = $username; session_register('password'); $_SESSION['password'] = $password; session_register('sign_up'); $_SESSION['sign_up'] = $sign_up; session_register('email'); $_SESSION['email'] = $email; session_register('first'); $_SESSION['first'] = $first; session_register('last'); $_SESSION['last'] = $last; session_register('activation'); $_SESSION['activation'] = $activation; session_register('user_level'); $_SESSION['user_level'] = $user_level; session_register('total_amount'); $_SESSION['total_amount'] = $total_amount; session_register('budget_1_name'); $_SESSION['budget_1_name'] = $budget_1_name; session_register('budget_1_total'); $_SESSION['budget_1_total'] = $budget_1_total; session_register('budget_2_name'); $_SESSION['budget_2_name'] = $budget_2_name; session_register('budget_2_total'); $_SESSION['budget_2_total'] = $budget_2_total; session_register('budget_3_name'); $_SESSION['budget_3_name'] = $budget_3_name; session_register('budget_3_total'); $_SESSION['budget_3_total'] = $budget_3_total; session_register('budget_4_name'); $_SESSION['budget_4_name'] = $budget_4_name; session_register('budget_4_total'); $_SESSION['budget_4_total'] = $budget_4_total; session_register('budget_5_name'); $_SESSION['budget_5_name'] = $budget_5_name; session_register('budget_5_total'); $_SESSION['budget_5_total'] = $budget_5_total; mysql_query("UPDATE users SET last_login=now() WHERE userid='$userid'"); header( "Location: index.php" ); } } else { echo "The login information you entered does not match our system."; } ?> </body> </html> <? ob_flush(); ?> Quote Link to comment Share on other sites More sharing options...
denno020 Posted June 8, 2013 Share Posted June 8, 2013 I would suggest you set it out like this: <?php $loginError = false; if(isset($_POST['username'])){ //Check if a form has been submitted //Do all your login processing here. //... //... //... //... if($login_check > 0){ //Do all of the processing that you have header('Location: index.php'); exit(); }else{ $loginError = true; //Flag to turn on error messaging below } } ?> <!DOCTYPE html> <html> <head> </head> <body> <?php if($loginError){ echo "There is an error processing your login details. Please try again"; } ?> <form> <!-- form contents --> </form> </body> That will allow you to print out error messages wherever you want them, but placing the if($loginError) section of code wherever you want it to appear, so you could move it inside the form tags, and just before the inputs. Hopefully that will get the ball rolling for you. Denno Quote Link to comment Share on other sites More sharing options...
cpd Posted June 8, 2013 Share Posted June 8, 2013 (edited) It might work properly but its bloody awful! Not really sure what "// Start session" is meant to describe but it definitely doesn't appear to call session_start(). Is this even correct? Doing if(!$username || !$password) isn't good validation at all. All you're really testing is if the variable is initialised. A variable is initialised if it contains a space therefore, if the user submits a space as a username its valid; that's just wish-wash. Ideally you shouldn't allow spaces either side of a username when the user is created, then trim() the username just before validation upon login. session_register() was deprecated in 5.3 and removed in 5.4 [1] so you shouldn't really be using it although if you're still on an older version obviously you can; not sure how advisable this is. MD5 hasing a password is by no means satisfactory security. You should encrypt the password using multiple techniques all of which are covered by a nifty script called PHPass [2]. MySQL is going to be deprecated so look at moving to PDO or MySQLi. The way you're setting sessions isn't brilliant and the data you're setting is questionable. You're setting their password in the users session which isn't good unless you have taken precautions to help prevent the session from being hijacked. The "budgets" should really be stored in an array or serialised object. $_SESSION['budget'][0]['name'] = $budget_1_name; $_SESSION['budget'][0]['total'] = $budget_1_total; $_SESSION['budget'][1]['name'] = $budget_2_name; $_SESSION['budget'][1]['total'] = ...; Although I probably wouldn't have variables such as $budget_1_name as they would be better represented as an array. With regards to your actual question: 1) store the success/error message in a session; 2) redirect to the appropriate page, login page for errors or dashboard for a success; 3) display the message where you want within each page by testing if the session has been set; 4) unset the session ensuring it doesn't continuously pop up. [1] PHP. session_register. http://uk1.php.net/session_register [2] Openwall. Portable PHP Password hashing. http://www.openwall.com/phpass/ Edited June 8, 2013 by cpd Quote Link to comment Share on other sites More sharing options...
TylerRichardson Posted June 9, 2013 Author Share Posted June 9, 2013 Holy crap CPD, im pretty sure youre post melted my brain. Im pretty new to php (and coding outside of html and css in general) and im using the skeleton of a script i found online which would be why its out of date. Im honestly not looking for security right now, this is just a personal project im working on to get better at coding so i hope you arnt too offended by my terrible lack of regard for good coding haha. I kind of understand what youre saying at the end, what i have right now is a jquery dialog box so if the login fails ill redirect to an identical page with the dialog box autoOpen = true with the error message at the top which will be a TON easier than learning how to do an error message at the top of the form. Also Denno020, i dont get how the login check is equal or inequal to anything. can someone explain to me what this snippet of code does? $sql = mysql_query("SELECT * FROM users WHERE username='$username' AND password='$password' "); $login_check = mysql_num_rows($sql); also i am currently executing the login like this: <script> $(function() { $( "#login_form" ).dialog({ autoOpen: false, height: 300, width: 350, modal: true, buttons: { "Login": (function() { $("#login_info").submit(); }), Cancel: function() { $( this ).dialog( "close" ); } }, close: function() { allFields.val( "" ).removeClass( "ui-state-error" ); } }); $( "#login" ) .button() .click(function() { $( "#login_form" ).dialog( "open" ); }); }); </script> I have absolutly no idea if thatll work (On my html form i have an action set so im thinking that the submit function will do the same thing as html form submit would do) Quote Link to comment Share on other sites More sharing options...
cpd Posted June 9, 2013 Share Posted June 9, 2013 (edited) If you're trying to learn how to program you should really learn best practices as well. How you encrypt your password falls under best practices and running an MD5 hash on it only isn't one of them. Do you only have a dialogue box login? You don't have a login page? Moreover, why would you submit a form with javascript when you can do it with a normal submit button? You're not doing anything special with javascript so let the submit button do the work. Edited June 9, 2013 by cpd Quote Link to comment Share on other sites More sharing options...
TylerRichardson Posted June 9, 2013 Author Share Posted June 9, 2013 but if i have just a submit button it doesnt look right, it would make sense to have my buttons all in the same place for every dialog box i have (which is a lot). If you go to the site budgetme.org youll see when you submit any username and password it takes you to this weird error page which makes it seem like the submit button is working but the check user script isnt Quote Link to comment Share on other sites More sharing options...
denno020 Posted June 10, 2013 Share Posted June 10, 2013 In answer to your question about what the SQL query does, it will query the database (ask the database) for any records that have the username that matches what the user put into the form, and a password that also matches. Ideally this would only be one record, as all usernames in the database should be unique. The second line then counts the number of rows (database records) that were returned, if this number is 0, then there is no user with that username, or the password was wrong. If there is 1 record returned, then the user provided login details are correct. Denno Quote Link to comment Share on other sites More sharing options...
TylerRichardson Posted June 10, 2013 Author Share Posted June 10, 2013 ooohh thats clever, thanks. But also when i execute my login script it returns an error page saying that that specific line in my code is drawing a scripting error but it is still connecting to the database so i dont know why it wouldnt calculate that Quote Link to comment Share on other sites More sharing options...
Christian F. Posted June 10, 2013 Share Posted June 10, 2013 (edited) Actually, as with PHP 5.5 the PHPass libraries are outdated. So unless you're forced to work on a PHP version less than 5.3.7, I recommend using either Anthony Ferrara's PHP 5.5 compatibility layer or the native PHP 5.5 password functions. For more information I recommend watching the following video: As for the PHP error your script is complaining about: It tells you exactly what's wrong, and most likely why. You just have to pay attention to the details. Without knowing the exact error message we cannot help you with that, I'm afraid. Also, a slight clarification to cpd's previous post. Whenever you get an error status on a form submission you do not have to redirect the user. I prefer to only redirect the user on a successful submission, in order to prevent the user from having an (erroneous) re-submission by hitting F5/refresh. By avoiding this redirect upon failed submissions, the PHP script has access to all of the data sent by the user, which you can then use to re-populate the form again. Without having to jump through extra hoops, such as saving them into a session or something like that. Here's a quick pseudo-code example of how I recommend forms to be processed: if (!submitted ()) { // Show form return; } // Create an empty error array, for use in validation process. $error = array (); // Validate the input from the form, populating the error array upon failure. $val_1 = validate ($_POST['val_1'], $error); $val_2 = validate ($_POST['val_2'], $error); // Check if any field failed validation. if (!empty ($error)) { // Show error message(s) // Re-populate the field, remember to escape the data. // Show the form. return; } // All validation succeeded, process the data and check for DB errors. if (!$res = db->query ($sql)) { trigger_error ("database error!", E_USER_ERROR); } // If we have some checks based upon the result of the SQL query to do, then do it now. if ($res->num_rows != 1) { // Didn't return the expected number of rows. // Show form submission error, re-populate form and show it. return; } // Else we got a successful submission, redirect to prevent F5-resubmit. header ("Location: {$PHP_SELF}"); Edited June 10, 2013 by Christian F. Quote Link to comment Share on other sites More sharing options...
trq Posted June 10, 2013 Share Posted June 10, 2013 or the native PHP 5.5 password functions PHP5.5 isn't even at a stable release yet. I also doubt it will be included in any of the major Linux distros by default for at least another 12-18 months. Quote Link to comment Share on other sites More sharing options...
cpd Posted June 10, 2013 Share Posted June 10, 2013 (edited) Actually, as with PHP 5.5 the PHPass libraries are outdated. How come its out of date? I.e. what methods used are out of date? I've not checked for a while, but can't find anything that suggests it is with a quick read up. Unless you mean it doesn't take advantage of some of the newly available features due for release with 5.5? Edit: I meant to link https://github.com/rchouinard/phpass in a previous post! Haven't had to download it in a while. Edited June 10, 2013 by cpd Quote Link to comment Share on other sites More sharing options...
Christian F. Posted June 10, 2013 Share Posted June 10, 2013 It's explained in the video, at 21:50 and outwards. In short the main reasons are because compatibility layer is easier to use, (slightly) more secure, and has the ability to check whether or not a password needs to be rehashed. There are several minor reasons as well, but I won't go into them. However, if, for some reason, you are still running on PHP < 5.3 then PHPass is your only viable option. It's just showing its age, just like older versions of PHP. Quote Link to comment Share on other sites More sharing options...
Strider64 Posted June 10, 2013 Share Posted June 10, 2013 (edited) Ignore this Post - Sorry, I really need some caffeine in the morning.... Edited June 10, 2013 by Strider64 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.