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
*                        $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:


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'] =
*                                               $_SESSION['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 =

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
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;
  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:

*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>*
*                        }*
*                 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>*
*                        }*
*                 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>*
*                        }*
* *
*?> *

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";
  case newbie:
   $access_name = "newbie";
   $page_suffix = "newbe";
  case advanced:
   $access_name = "advanced";
   $page_suffix = "advanced";
 } // end switch on $usr_level

<meta http-equiv="REFRESH" content="10;url=http://<?php echo $_SERVER["HTTP_HOST"] . "/index_$page_suffix.html"?>">

<div class="message">You have successfully been logged in. You can now
access the <?php echo $access_name ?> area.<br /></div>

 } // 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.


PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to