Re: [PHP] Login with Remember me Feature
Thank you for all the helpful input so far! I have now tried to implement the changes you suggested, but I unfortunately keep getting an error in line 114, in {-bracket in the switch statement. I know it is not very desirable to send all the code in a mail, but I think this is the best solution to find where the error(s) are located. Also when it comes to implementing the loggedin-function as Geoff Shang so kindly suggested for the config.php. I keep getting an error message that says that there is an error in the * "return true;" - line * *function loggedin() {* *if (isset($_SESSIONS['username']) || isset($_COOKIE['username'])) return true; else return false;* *}* So for now this code-block is the same as it used to be, because this done not generate any errors. When it comes to the function loggedin() inside the connexions.php, I am not sure where to call the function. Should this be just before the comparing of the password? ..or before the switch statement? *connextion.php* ** * * *http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd";> http://www.w3.org/1999/xhtml";>* * Connexion * * * * * ** *You have successfuly been loged out. Home should be placed her??* ** *//We compare the submited password and the real one, and we check if the user exists * * if($dn['password']==$password and mysql_num_rows($req)>0) {* *//If the password is ok, we set the $loginok var to true $loginok = true;* *//If the password is good, we dont show the form $form = false; * * // If the user is alredy logged in if ($loginok) { if ($remember=="on") setcookie("username", $username, time()+3600*48);* * else* * //We save the user name in the session username and the user Id in the session userid* * $_SESSION['username'] = $username; $_SESSION['userid'] = $dn['id']; $_SESSION['usr_level'] = $dn['usr_level']; * * // if (loggedin()){ --> should be placed her?? * * switch ($usr_level) * * { case admin: $access_name = "admin"; $page_suffix = "admin"; break;* * case newbie: $access_name = "newbie"; $page_suffix = "newbe"; break;* *case advanced: $access_name = "advanced"; $page_suffix = "advanced"; break } //close the switch-looop* * } // close the if-logged in - loop * * ?>* * Redirecting... ">* ** *You have successfully been logged in. You can now access the area.* * * *'.$message.''; } // close the display-block* * //We display the form, redirect back to login-page header("Location: header_login.php"); } // close the display message if-loop* * * *?>* * * * *
Re: [PHP] Login with Remember me Feature
On Sun, 14 Aug 2011, Alekto Antarctica wrote: I have tried to implement a cookie to remember the login for 48 hours, but it still logs the user out after the default 24min for a session like this: * //We compare the submited password and the real one, and we check if the user exists* *if($dn['password']==$password and mysql_num_rows($req)>0)* You don't show us anything before this, so we have to assume it's all good up to here. *{* ** *//If the password is ok, we set the $loginok var to true* *$loginok = true;* *//If the password is good, we dont show the form* *$form = false;* ** *}* Maybe I'm just like this, but I always comment my closing braces. I've been in situations where I'm missing one or I need to review code I wrote months ago and understand its logic, and I find this practice useful. Yes, in this case the opening is a few lines up, but you could have a code block that runs for hundreds of lines, and it's good to remember what started it. * if ($loginok = true)* * {* First, off, as someone else mentioned, this should presumably be: if ($loginok == true) This one mistake will mean that $loginok will always be true. Second, since if statements are always looking for true conditions, you can simply type: if ($loginok) Finally, since $loginok is assigned the true value in the previous block, then, unless it is also possibly assigned elsewhere, you can just put the below code in the same code block as the above code, rather than closing and starting a new one with this if statement. * if ($remember=="on") * *setcookie("username", $username, time()+7200*24);* This is not very intuitive. You're saying to add 2 hours times 24, which is a bit strange if you're trying to understand the code. I'dve found 3600*48 much more intuitive. A comment mightn't go astray here either. * elseif ($remember=="") Are these the only two values that $remember can have? May as well just use else here without testing for another condition (either the user is remembering or they're not). * * //We save the user name in the session username and the user Id in the session userid* I think we might have an left brace missing here, unless it's gotten lost in translation. Also, I notice you're storing username and userid here, but above only stored username in the cookie. * $_SESSION('username')=$username; * This line should read: $_SESSION['username']=$username; I see the next line has it right. I'm surprised that your code didn't generate an error for this one, and since it didn't, this may indicate that this code is never reached (possibly due to the elseif test above). * $_SESSION['userid'] = $dn['id'];* * $_SESSION['usr_level'] = $dn['usr_level'];* I see a mixing of styles here. While it's all perfectly good syntax, you may want to find a style you like and stick to it. I personally find $foo = $bar; much more readable than $foo=$bar; or $foo = $bar; but each to their own. Another problem I am now facing, is to check whether to user is logged in, and if it is the user should be redirected from the index-page(with the login-form) to its user area based on the user level(newbie, advanced or admin). For now I have written a function, in the config.php. *function loggedin()* *{* * if (isset($_SESSIONS['username']) || isset($_COOKIE['username']))* * {* * $loggedin = true;* * return $loggedin;* * }* *}* As someone else pointed out, you could simply return true instead of assigning to a variable. They also pointed out that you don't return false if the person is not logged in. You could rewrite the above function like so: function loggedin() { if (isset($_SESSIONS['username']) || isset($_COOKIE['username'])) return true; else return false; } However, this doesn't actually check the values of these items, it simply checks to see if they have been set. I have both tried to include the config.php into the index-page(login-form) and into the connexions.php script (where cookie is implemented). Along with this code: * You need to call a function with parentheses, even if it takes no arguments, like so: if (loggedin() == true) or simply if (loggedin()) *{* * if($usr_level == admin)* *{* * ?>* *You have successfuly been logged in. You can now access the admin area.* ** ** *You have successfuly b
Re: Re: [PHP] Login with Remember me Feature
On 14 Aug 2011 at 14:23, Alekto Antarctica wrote: > *function loggedin()* > *{* > * if (isset($_SESSIONS['username']) || isset($_COOKIE['username']))* > * {* > * $loggedin = true;* > * return $loggedin;* > * }* > *}* Why not justreturn true; And what happens if your "if" doesn't evaluate to true? What do you return then? > * * > * > *if (loggedin==true)* > *{* Should this be: if ($loggedin==true) ... -- Cheers -- Tim -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Login with Remember me Feature
On Aug 14, 2011, at 8:23 AM, Alekto Antarctica wrote: Hi guys! I have now tried to take some of your hints into consideration, by encrypting the password with md5 adding a salt. As some of you pointed out, this code is the work of a newbie, that is totally correct, so please bear with me ;) I have tried to implement a cookie to remember the login for 48 hours, but it still logs the user out after the default 24min for a session like this: * //We compare the submited password and the real one, and we check if the user exists* *if($dn['password']==$password and mysql_num_rows($req)>0)* *{* ** *//If the password is ok, we set the $loginok var to true* *$loginok = true;* *//If the password is good, we dont show the form* *$form = false;* ** *}* * if ($loginok = true)* * {* * if ($remember=="on") * *setcookie("username", $username, time()+7200*24);* * elseif ($remember=="") * * //We save the user name in the session username and the user Id in the session userid* * $_SESSION('username')= $username; * * $_SESSION['userid'] = $dn['id'];* * $_SESSION['usr_level'] = $dn['usr_level'];* * * *.* *.* *.* *.* *.* *.* *}* Another problem I am now facing, is to check whether to user is logged in, and if it is the user should be redirected from the index-page(with the login-form) to its user area based on the user level(newbie, advanced or admin). For now I have written a function, in the config.php. *function loggedin()* *{* * if (isset($_SESSIONS['username']) || isset($_COOKIE['username']))* * {* * $loggedin = true;* * return $loggedin;* * }* *}* I have both tried to include the config.php into the index- page(login-form) and into the connexions.php script (where cookie is implemented). Along with this code: ** *You have successfuly been logged in. You can now access the admin area.* ** ** *You have successfuly been logged in. You can now access to the newbie area.* ** ** *You have successfuly been logged in. You can now access the advanced area.* ** * * * * This does not redirect an alredy logged in user to its user area... I know this is messy, but if some of you can spot some improvements that hopfully can fix my cookie and redirect problem, please let me know. Tanks a lot! You can't issue headers after you've sent output to the client. headers must be sent before any other output. If you have messages to be output based on the current script, you have to pass them to the redirected script for them to be output to the client. You can do this by passing them on the query string or in a session variable; there are likely other ways of doing this as well. Note that you don't have to pass the actual text of the message if you use message codes instead, which would also aid in being able to translate the output if so desired. In the code above, since the message seems tied to the particular area the user has access to and that is tied to a particular script, you could just put the message with each particular script. -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Login with Remember me Feature
Hi guys! I have now tried to take some of your hints into consideration, by encrypting the password with md5 adding a salt. As some of you pointed out, this code is the work of a newbie, that is totally correct, so please bear with me ;) I have tried to implement a cookie to remember the login for 48 hours, but it still logs the user out after the default 24min for a session like this: * //We compare the submited password and the real one, and we check if the user exists* *if($dn['password']==$password and mysql_num_rows($req)>0)* *{* ** *//If the password is ok, we set the $loginok var to true* *$loginok = true;* *//If the password is good, we dont show the form* *$form = false;* ** *}* * if ($loginok = true)* * {* * if ($remember=="on") * *setcookie("username", $username, time()+7200*24);* * elseif ($remember=="") * * //We save the user name in the session username and the user Id in the session userid* * $_SESSION('username')=$username; * * $_SESSION['userid'] = $dn['id'];* * $_SESSION['usr_level'] = $dn['usr_level'];* * * *.* *.* *.* *.* *.* *.* *}* Another problem I am now facing, is to check whether to user is logged in, and if it is the user should be redirected from the index-page(with the login-form) to its user area based on the user level(newbie, advanced or admin). For now I have written a function, in the config.php. *function loggedin()* *{* * if (isset($_SESSIONS['username']) || isset($_COOKIE['username']))* * {* * $loggedin = true;* * return $loggedin;* * }* *}* I have both tried to include the config.php into the index-page(login-form) and into the connexions.php script (where cookie is implemented). Along with this code: ** *You have successfuly been logged in. You can now access the admin area.* ** ** *You have successfuly been logged in. You can now access to the newbie area.* ** ** *You have successfuly been logged in. You can now access the advanced area.* ** * * * * This does not redirect an alredy logged in user to its user area... I know this is messy, but if some of you can spot some improvements that hopfully can fix my cookie and redirect problem, please let me know. Tanks a lot!
Re: [PHP] Login with Remember me Feature
On Sun, Aug 7, 2011 at 10:03 PM, Donovan Brooke wrote: > alekto wrote: > >> Hi, >> I have implemented a "remember" me feature in my login-script, but I can't >> get it to function! >> > > > If I might be so bold... then you haven't implemented the feature yet, > right? ;-) > > > > I want to make it possible for the users to stay logged in for 30 days. >> This is what I got this far: >> > > > You have a logic problem... If I were you, I would write it out more > simplistically first... something like: > > if session cookie > keep logged in > else, if remember me > if verifiable >set session cookie and redirect > > Of course, that is not an example of exact logic to use, and is just a > method example of how you can solve your problem. As others have suggested, > I would first start reading about ob_start,ob_end_clean(which > works well before a header redirect), and ob_end_flush. > > I agree about only needing to store the user ID in your cookie's (session > and rememberme) (hashed perhaps), and not the password. > > My last comment would be a kind request to strip out all unnecessary html > etc.. when posting questions to the list. I usually would not take the time > to look through a mess like that. ;-) > > Donovan > > -- > D Brooke > > > -- > PHP General Mailing List (http://www.php.net/) > To unsubscribe, visit: http://www.php.net/unsub.php > > I'm going to play the third side of this thread and ask if anyone other than me sees any clear security issues with code like that, even if username and password were taken out of the cookie, and it was hashed in the DB, there is still a security issue with thinking this way which in today's world should not be an overlooked practice. And i mean i see that the person here is a newbie, the code looks pretty bad, but i think it's worth mentioning that looking at best security practices for the situation is as trivial as figuring out your classes and methods. Knowing how to prevent people like, well even me, from running sql scripts from your website via forms, or stealing user sessions is essential in today's web world... You're writing some client-facing code, maybe you should look at how to write it and keep the client secure? You could at least add session and request tokens to make the persistent sessions at least a bit more secure, that's of course on top of hashing passwords (with a salt), and not storing user names and passwords in the cookie. Also escaping doesn't work, if you don't believe me, listen to the keynote that Dan Kaminsky gave at the last HOPE conference, he gives a good overview of why... Please either use parameterized queries, or the awesome hack that is base 64, don't assume that just because the function is called mysql_real_escape_string, that it actually knows what it is doing; unicode is a powerful weapon in the wrong hands! Also use === for string comparison as 42 == "test"! ~Alex -- The trouble with programmers is that you can never tell what a programmer is doing until it’s too late. ~Seymour Cray
Re: [PHP] Login with Remember me Feature
alekto wrote: Hi, I have implemented a "remember" me feature in my login-script, but I can't get it to function! If I might be so bold... then you haven't implemented the feature yet, right? ;-) I want to make it possible for the users to stay logged in for 30 days. This is what I got this far: You have a logic problem... If I were you, I would write it out more simplistically first... something like: if session cookie keep logged in else, if remember me if verifiable set session cookie and redirect Of course, that is not an example of exact logic to use, and is just a method example of how you can solve your problem. As others have suggested, I would first start reading about ob_start,ob_end_clean(which works well before a header redirect), and ob_end_flush. I agree about only needing to store the user ID in your cookie's (session and rememberme) (hashed perhaps), and not the password. My last comment would be a kind request to strip out all unnecessary html etc.. when posting questions to the list. I usually would not take the time to look through a mess like that. ;-) Donovan -- D Brooke -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Login with Remember me Feature
Hello alekto, I've got several notes to point out: 1. You can't do neither a header(), nor a SetCookie() after any echo on the page. The out-of-php pieces of the page included. 2. Don't, please please don't store raw passwords in the database! Hash them, better even adding a salt. The guy who had been writing code of our project before me stored raw passwords, and I lost an amount of time to encrypt them live so users wouln't notice anything happening. Please don't repeat this mistake) 3. Don't store passwords in the cookies, they can be easily stolen. the username is quite enough: if it is there and it is not empty, then you can verify if such a user exists. -- With best regards from Ukraine, Andre Skype: Francophile My blog: http://oire.org/menelion (mostly in Russian) Twitter: http://twitter.com/m_elensule Facebook: http://facebook.com/menelion -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php