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:
*<?php*
*
*
*if (loggedin==true)*
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)*
* {*
* ?>*
*<div class="message">You have successfuly been logged in. You can now
access the admin area.<br />*
*<?php header("Location: index_admin.php"); ?></div>*
*<?php*
* }*
* if($usr_level == newbie)*
* {*
* ?>*
*<div class="message">You have successfuly been logged in. You can now
access to the newbie area.<br />*
*<?php header("Location: index_newbe.php"); ?></div>*
*<?php*
* }*
* if($usr_level == advanced)*
* {*
* ?>*
*<div class="message">You have successfuly been logged in. You can now
access the advanced area.<br />*
*<?php header("Location: index_advanced.php"); ?></div>*
*<?php*
* }*
* *
*}*
*
*
*?> *
There's a lot to say about this.
First and foremost, as mentioned by someone else, you can't send location
headers once output has begun. Moreover, there's no guarantee that the
user's browser will display any messages contained in the page body, even
if therre are any.
This is easy to get around, however, by using a page refresh. This is
demonstrated below.
Second, I'm assuming you've set up the various access levels as defined
constants. If you haven't, the above code isn't going to work. I'm going
to assume you have, but you've not shown us this. You've also not shown
us how $usr_level gets its value.
Third, you don't seem to have set up your HTML page. I'm guessing that
you've not actually done this and you need to.
Finaly, my belief is that if you need to cut and paste code, you're doing
it wrong. Instead of having all these separate tests and HTML blocks, we
could do this much more simply with a switch statement and a couple of
variables.
Note: I see you've used "newbe" in your newbie page redirect, instead of
"newbie". I'm going to assume this was deliberate. If not, you can get
rid of $page_suffix entirely and just use $access_name instead.
Given all the above, here's how I'd rewrite this code:
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;
} // end switch on $usr_level
?>
<html>
<head>
<title>Redirecting...</title>
<meta http-equiv="REFRESH" content="10;url=http://<?php echo
$_SERVER["HTTP_HOST"] . "/index_$page_suffix.html"?>">
</head>
<body>
<div class="message">You have successfully been logged in. You can now
access the <?php echo $access_name ?> area.<br /></div>
</body>
</html>
<?php
} // end if logged in
?>
The above also assumes your index_* files are in the webroot, you can edit
the refresh line if this is wrong.
HTH,
Geoff.
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
