check if user is logged in

Question

Hi,

I have some basic code that I place at the header of every page to make sure that the user is logged in. I was hoping someone could take a look at it and give me some suggestions:

if ($_SESSION['logged_in'] == 1) {
        $handle = dbconnect::init;
        $result = $handle->select()->from('session_id')
                                   ->where('session_id=?', $_SESSION['SID'])
                                   ->columns('ip');
        $check = $result->fetchAll();
        if ($check[0]->ip != $_SERVER['REMOTE_ADDR']) { //user has changed networks
                                                        // or someone is trying 
                                                        // to switch cookies on us
            return false;
        }
    } else {
        return false;
    }

Thanks!

Answer 1

function checkLoggedIn () {
    // Return early if we are not logged in. Also, by using empty we
    // avoid warnings of the 'undefined index' kind.
    if (empty($_SESSION['logged_in'])) {
        return false;
    } 

    $handle = YourDbClass::getConnection();

    $result = $handle->select()->from('session_id')
                               ->where('session_id=?', $_SESSION['SID'])
                               ->columns('ip');
    $check = $result->fetchAll();
    if ($check[0]->ip != $_SERVER['REMOTE_ADDR']) { //user has changed networks
                                                    // or someone is trying 
                                                    // to switch cookies on us
        return false;
    }
    return true;
}

Your code looks pretty good to me. I wrapped it in a function so you don't need to duplicate it on every page, just require your util.php or whatever you want to call your function library. Then just call checkLoggedIn(). If it returns false, the user is not logged in and you may send an error page, exit or whatever. If it returns true, you can proceed.

Answer 2

Do you have a special need for pulling the remote ip from the database? Would be easier to store the remote ip within _SESSION instead of bothering the database with another query.
You might want to give the user an option to disable this feature as they might have to connect to your server through transparent proxies with changing ip addresses, e.g. http://webmaster.info.aol.com/proxyinfo.html says:

AOL Members' requests for internet objects are usually handled by the AOL Proxy system. When a member requests multiple documents for multiple URLs, each request may come from a different proxy server. Since one proxy server can have multiple members going to one site, webmasters should not make assumptions about the relationship between members and proxy servers when designing their web site.

nit picky: You should first test if there is at least one record before you try to access it. Might be something like:

if ( !isset($check[0]) || $check[0]->ip!=$_SERVER['REMOTE_ADDR'] )