Check for session variable to log out a user is not working

I've done something wrong. :)

I have a ControllerBase that almost all my Controllers extend. In it's initialize(), it contains a check for $this->session->get('userId') which was set when they logged in. If it's empty, it kicks them to a logout() in the ControllerBase to force them to the login screen.This seems to work everywhere except one controller, but I'm not sure it isn't just working most of the time.

In one Controller, the WikiController, it doesn't want to work. I confirmed it IS firing the logout() when WikiController functions are called, by putting in a die('here'); . But it proceeds to fire the subsequent code in WIkiController which involves some model functions/MySQL. The SQL fails because the info from the userId is necessary, and I get some error output from phalcon like it never tried to log them out.

My intention was to have the inherited function from ControllerBase always check to see that a userId existed in the session, then log them out if not.

Can you post the relevant code from ControllerBase and from WikiController? Without having seen the code yet, the only thing I can think of is perhaps the check isn't exiting or returning properly, which is allowing the action's code to run.

I've implemented this via custom component (though I suppose the built-in ACL would work too). In my bootstrap I've attached the component to the dispatcher's events manager:

$DI->set('dispatcher',function(){
        $EM = new \Phalcon\Events\Manager();

        // Check user is logged in
        $EM->attach('dispatch:beforeDispatch',new \Component\Auth());

        $Dispatcher = new \Phalcon\Mvc\Dispatcher();
        $Dispatcher->setDefaultNamespace('Controller');
        $Dispatcher->setEventsManager($EM);

        return $Dispatcher;
    });

\Component\Auth then has a beforeDispatch() method that checks the user is logged in. I feel this is a bit better, because this event is fired before the controller gets loaded - saving a few nanoseconds.



2.2k
edited May '18

Here is the code for the ControllerBase:

<?php

use Phalcon\Mvc\Controller;
use Phalcon\Loader;

class ControllerBase extends Controller
{

    public function initialize()
    {

        /* Login logic */
        if ( $_POST['loginAttempt'] == "true" )
        {
            //Store the password the user supplied
            $this->session->set( 'username', $_POST['username'] );
            $password = $_POST['password'] ;

            /* Some other, not applicable logic takes place right here, omitted */

                if ( isset($userData[0]->userId) )
                {
                    $this->session->set( 'userId', $userData[0]->userId );
                    $this->session->set( 'userData', $userData[0] );
                    $this->session->set( 'apiKey', $token );
                }
                else //Makes it blank, which causes the logic later below to kick them
                {
                    $this->session->set( 'userId', '' );
                }

                //Get user's groups from the LDAP server
                $this->session->set( 'userGroups', $ldapObj->memberOf() );
            }
        }

        //If no userId is present in the session (or session doesn't exist, so 
        //it's empty), kick 'em out 
        if ( empty( $this->session->get('userId') ) )
        {
            //Log them out/redirect to login page
            $this->logout();
        }
        /* End Login logic */

    }

    /**
    * 
    * Log out current user
    */
    function logout()
    {
        //Destroy session if it exists
        if ( !empty( $this->session->get('userId') ) )
        {
            $this->session->destroy();
        }

        //Redirect to the login page
        return $this->dispatcher->forward(array( 
           'controller' => 'login', 'action' => 'index' 
        )); 
    }

Here is the WikiController applicable code:

<?php

class WikiController extends ControllerBase
{

    /* This is to always redirect to the View Action, in case something brings the user to the .com/wiki/ page
    function indexAction() */
    {
        //Redirect to the report run
        return $this->dispatcher->forward(array( 
           'controller' => 'wiki', 'action' => 'view', 'params' => array( 0 => 'system:all' ) 
        )); 
    }

    function viewAction($wiki_word) {
        $wikis = new WikiModel();
        /* Promptly set a few things then asks the model for some MySQL results, which is where the errors pop 
        onto the browser's output, I don't know how I get where if the session userId is not set, since I assume the 
        logout() should have been called, and by putting a die('here'); into logout(), I confirmed it IS being called, but 
        it is not kicking them to the login page */

        }

Hmmm, I'm not seeing a lot, other than the fact the WikiController::indexAction() function declaration is commented out.

If you put a die() in LoginController::indexAction(), do you hit that?

Also, as a bit of a sidenote, rather than setting a session variable to an empty string, you can remove() it:

$this->session->remove('userId');

Then, when your checking for it's existence, rather than using empty() you can use has():

if($this->session->has('userId')


2.2k

The WikiController::indexAction() commented out is a typo on my part. :)

I put a die('here'); in the ControllerBase logout() and I got the 'here', so I know it's firing. But it's like the wikiController function just proceeds along it's way.

Hmmm, I'm not seeing a lot, other than the fact the WikiController::indexAction() function declaration is commented out.

If you put a die() in LoginController::indexAction(), do you hit that?

Also, as a bit of a sidenote, rather than setting a session variable to an empty string, you can remove() it:

$this->session->remove('userId');

Then, when your checking for it's existence, rather than using empty() you can use has():

if($this->session->has('userId')
edited May '18

Don't access post directly, use $this->request->getPost() etc.



2.2k

I usually don't; I think that was an artifact of me porting hte code over from the old CodeIgniter (where I had been ignorant of POST access when I wrote it) and I just overlooked fixing it. Thanks for pointing it out! Will fix!

Don't access post directly, use $this->request->getPost() etc.

Again, hmmmm. Try just calling $this->dispatcher->forward..., but not returning it, then die() right after it.

Accessing $_POST is perfectly fine. $this->request is just syntactical sugar so you can access the array in an object-like manner.

I think it's better to use still methods, since you can already filter it or set default value.