onsuspect (SQL) redirecting to /login

[Thomas Preissler]

Hi,

I am trying to enforce relogin when the onsuspect() handler is kicking in.

My code so far:

    $dice->addRule('DB\\SQL\\Session', ['shared' => true, 'constructParams' => [

        'session',

                FALSE,

                function($session, $id) {

                        $fw=\Base::instance();

                        $session->destroy($id);

                        $session->close();

//                      unset($fw->{'COOKIE.'.session_name()});

                        $fw->clear('COOKIE.'.session_name());

//                      $fw->redirect('/');

                        header("Location: index.php");

                        exit;

//                      $fw->error(401);

                        return true;

                }

    ]]);

    $session = $dice->create('DB\\SQL\\Session');

But I am getting a "session_commit(): Cannot call session save handler in a recursive manner".

I have tried various things, reroute(), redirect(), now even header() and in a way I am getting always the same error.

I can manage by using the default onsuspect handler to throw a 403, but how would I get them to login again?

Cheers

Thomas

[ved]

Hi Thomas,

First of all, that F3 code seems to be wrapped in some other class or library ($dice?) so if the problem comes from that part of the code it will be hard for us to debug with just that info.

Now for the apparent issue at hand: How exactly are you differentiating logged in users from non-logged users?

Usually this is done by using a session variable. For instance: SESSION.loggedin = true.

When the session is suspect, you can do whatever you like to the session or your app. For instance: destroy the session or set SESSION.loggedin = false and redirect to login.

Can you try something simple like:

function($session) {
    $fw=\Base::instance();
    $fw->clear('SESSION');
    $fw->reroute('/login');
}

[Thomas Preissler]

Thanks ved.

On Tuesday, June 6, 2017 at 9:04:44 PM UTC+1, ved wrote:

First of all, that F3 code seems to be wrapped in some other class or library ($dice?) so if the problem comes from that part of the code it will be hard for us to debug with just that info.

$dice is 

$dice = new \Dice\Dice;

, a PHP dependency Injection Container. https://r.je/dice.html
 

Now for the apparent issue at hand: How exactly are you differentiating logged in users from non-logged users?

My app is not allowing non-logged in users, ie. you have to login. The user is stored in SESSION.user.
 

Usually this is done by using a session variable. For instance: SESSION.loggedin = true.

When the session is suspect, you can do whatever you like to the session or your app. For instance: destroy the session or set SESSION.loggedin = false and redirect to login.

Can you try something simple like:

function($session) {
    $fw=\Base::instance();
    $fw->clear('SESSION');
    $fw->reroute('/login');
}

With this I am getting

Internal Server Error

session_destroy(): Cannot call session save handler in a recursive manner

[/vendor/bcosca/fatfree-core/base.php:2152] Base->error()
[/vendor/bcosca/fatfree-core/base.php:456] session_destroy()
[/web/index.php:49] Base->clear()
[/vendor/bcosca/fatfree-core/base.php:1776] CCWA\{closure}()
[/vendor/bcosca/fatfree-core/db/sql/session.php:72] Base->call()
[/vendor/bcosca/fatfree-core/base.php:273] session_start()
[/vendor/bcosca/fatfree-core/base.php:414] Base->ref()
[/app/lib/CCWA/App.php:21] Base->get()
[/vendor/level-2/dice/Dice.php:66] CCWA\App->__construct()
[/vendor/level-2/dice/Dice.php:43] Dice\Dice->Dice\{closure}()
[/web/index.php:67] Dice\Dice->create()
[/web/index.php:72] CCWA\boot()

 The way I see and understand this - it appears Dice is getting in the way. Just a gut feeling, can't really explain it.


Cheers

Thomas

[ved]

 The way I see and understand this - it appears Dice is getting in the way. Just a gut feeling, can't really explain it.

Maybe. Can't you try your app without Dice and see if it the same error persists?

Also make sure you have the latest version from fatfree-core just in case you're running an outdated version. 

[Thomas Preissler]

I have removed now Dice and made various things much simpler. FF is on 3.6.0. My code is now like this:

        $db = new \DB\SQL(
                $f3->get('devdb'),
                $f3->get('devdbusername'),
                $f3->get('devdbpassword'),
                [\PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION]
        );
        // TODO Registry or Hive, DB or db, check - shouldn't be used really anymore
        \Registry::set('db', $db);
        $f3->set('DB', $db);

        \Registry::set('session', new \DB\SQL\Session(
                $f3->get('DB'),
                'session',
                FALSE,

                function($session) {
                        $fw=\Base::instance();
                        $fw->clear('SESSION');

                        $fw->reroute('@login');
                }
        )); // needed for CSRF

 It's failing now on the $f2->clear command with "session_destroy(): Cannot call session save handler in a recursive manner".

The way I am testing my "IP address change" is that I login, grab the session_id, change the IP in the session table, then click on my app somewhere.
Or is this "wrong", in a way?

I am on PHP 7.1.5. Seems to be quite common on that.
It's a bug?/feature? on PHP 7.1.X: https://github.com/php/php-src/pull/2196


Cheers

Thomas

[ved]

Please try updating to 3.6.1 or using the latest dev-master from fatfree-core and see if the issue persists.

3.6.1 and later versions implemented quite a few fixes for php7+ so it would be of value to try that version and see if it already got fixed.

BTW, I'm also using php 7.1.5 and I'm not experiencing that issue.

[Thomas Preissler]

Interesting. Thanks again.

I have updated now to F3 3.6.1 among some other composer upgrades (mostly packages used in -dev).

Your simplified onsuspect() handler is still in place, but I am still getting the same error:

session_destroy(): Cannot call session save handler in a recursive manner

Are you using SQL based sessions, too?

Dice for example is still in composer, but not loaded anymore. Does this matter? I mean,

are composer packages still loaded even, if they are not instantiated?

[ved]

No, actually I took that out of an app I have using the cache based Session class.

But I assumed that the handling of sessions is the same across all session handlers and all that changes is the actual storage method. But I may be wrong.

Are you setting DEBUG=3 on your app?

Is that the entire error message that shows up? No line numbers or stack trace, etc?

[Thomas Preissler]

The stacktrace is as follows:

13 Whoops\Exception\ErrorException 

…/vendor/bcosca/fatfree-core/base.php442

12 session_destroy

…/vendor/bcosca/fatfree-core/base.php442

11 Base clear

…/web/index.php193

10 CCWA\{closure}

…/vendor/bcosca/fatfree-core/base.php1787

9 Base call

…/vendor/bcosca/fatfree-core/db/sql/session.php72

8 DB\SQL\Session read

[internal]0

7 session_start

…/vendor/bcosca/fatfree-core/base.php259

6 Base ref

…/vendor/bcosca/fatfree-core/base.php307

5 Base exists

…/app/lib/CCWA/View/Main.php16

4 CCWA\View\Main __construct

…/app/lib/CCWA/Controller/Base.php25

3 CCWA\Controller\Base beforeroute

…/vendor/bcosca/fatfree-core/base.php1784

2 Base call

…/vendor/bcosca/fatfree-core/base.php1609

1 Base run

…/web/index.php199

0 CCWA\boot

…/web/index.php203

So it seems the code is doing the destroy, while still inside the session-read. PHP 7.1.5 it seems doesn't like that.

Removing my own onsuspect handler to let the original F3 execute, gives me a similar error:

session_commit(): Cannot call session save handler in a recursive manner

I can live without that feature. Without the onsuspect feature. I could potentially implement this manually as part of the authorize() function.

This way then I am not inside any sessionhandler and can invalidate it just fine.

What do you think?

Cheers

Thomas

[ved]

Ok, I'm sorry to admit that I kind of mislead you when I said my onsuspect handler was working fine.

Well, it was working the last time I checked, but I had also only recently upgraded to 7.1 and the app in question is seldom used or tested and rarely triggers the callback.

So, after also updating to latest f3-core I ran some manual tests a few minutes ago (I personally just use chrome tools and toggle the device view and it's enough to trigger it) I can confirm that I also have that exact same issue.

Maybe you could please file a bug report on github issues ?

EDIT:

Maybe it's not really a bug and now we can't just use any session related stuff inside the callback?

In that case I guess we could use the regular 403 and then use a custom ONERROR to catch 403's and do the rest of the stuff?

Either way, some clarification by the devs would help. I'm sure they'll step in this thread soon enough

EDIT 2:

Using ONERROR to destroy the session or redirect also triggers the error on my end.

[Thomas Preissler]

Bug reported:

https://github.com/bcosca/fatfree-core/issues/219

[Thomas Preissler]

OFF TOPIC:

What do you mean by "toggle the device view" - what do you do there in Chrome?

[ved]

Cool, thanks for filing the potential bug.

As for the Chrome thing, this is what I mean: https://developers.google.com/web/tools/chrome-devtools/device-mode/

When enabling the device mode, your browser behaves like a mobile device (that you can choose from a pre-defined list, add your own, or use a responsive/resizable viewport).

Since that also changes the user agent, I'm guessing that's what triggers F3's onsuspect calls.