Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

session handler 403 error should be optional #762

Closed
mark81 opened this issue Jan 15, 2015 · 11 comments
Closed

session handler 403 error should be optional #762

mark81 opened this issue Jan 15, 2015 · 11 comments

Comments

@mark81
Copy link

mark81 commented Jan 15, 2015

There is a problem in DB session handler. When client IP changes F3 throws 403 error and session is being destroyed. This looks like session hijack prevention, but in mobile scenarios it's causing serious problems. It's very easy to reproduce this error on any F3 website using DB session handler.

Example

Let's say there is a website using DB session handler, follow these steps:

  • Init session - open your website on mobile device on 3G (network)
  • Connect to WI-FI (or change IP address some other way)
  • Refresh website. Session is wiped.

It happens because session handler compares previous IP with current IP, as well as previous USER_AGENT with current one.

I understand in some scenarios this is important for security, but I think there should be a way to configure session security features and make both checks optional.

@bcosca can you liaise on solution?

For now I had to manually change the code in /db/sql/session.php to prevent it, which is not perfect. It's important for my app to preserve session on mobile devices, for given amount of time.

cheers

@ptdev
Copy link
Contributor

ptdev commented Jan 19, 2015

I also came across this issue (if we can call it an issue) using the cache based session handler (session.php).

To avoid changing the framework's core files, I've created a custom error handler that detects 403 errors and, in my case, show a "session has expired" type message or redirect to login. This still logs the user out but at least you can define some action to take or message to show after that. It also has the disadvantage that you'll lose all other default errors from f3 so you'll probably also want to add some code to manage or display the other status codes on your handler.

As for your specific case, mobile users are in fact much more likely to have ip changes during the session, so I guess the option to completely disable the ip verification would be the way to go.

Personally, I'd like to see the ability to define some sort of callback (like ONERROR and ONREROUTE) that get's called when csrf protection kicks in. Maybe something to consider for v4.

@mark81
Copy link
Author

mark81 commented Jan 19, 2015

@ptdev, yes, it is an issue mainly because of mobile users, I would like to avoid writing my own handlers, since F3 already does the job. It would be enough to just make them more flexible in terms of level of security checks.

@bcosca
Copy link
Owner

bcosca commented Jan 20, 2015

Have you considered extending the DB\SQL\Session class instead? It's a lot simpler to override the constructor than go the circuitous route of passing the buck to the error handler.

@mark81
Copy link
Author

mark81 commented Jan 20, 2015

@bcosca thanks for reply. Yes but the problem is db\sql\session.php constructor calling its own parent::_construct() so its not obvious to overload it.

Another option is to duplicate DB Session handler and invoke it into the project like:

namespace DB\SQL;

class MySession extends Mapper {
    //whole code from db\sql\session.php + your changes
}

Why not to add something like 'SESSION_SECURITY` setting in hive which could control the level of session security checks? This would solve the problem all the way without any hacks.

@KOTRET
Copy link
Collaborator

KOTRET commented Jan 20, 2015

i've taken a look into the class and yes - the security-check should be externalized into its own function to be overwriteable. The hive should not be touched in this case - its a module / extension, not core.

@ptdev
Copy link
Contributor

ptdev commented Jan 20, 2015

Agreed, I want to be able to control what happens when csrf kicks in. I believe most people will want to redirect to login and/or show a "session expired" or "session has been terminated" message when that happens. The csrf protection should have a configurable output function.

@mark81
Copy link
Author

mark81 commented Jan 21, 2015

sounds good @KOTRET. In the meanwhile I just duplicated session handler (copied the whole class) into separate file and just modified security checks.

@ikkez
Copy link
Collaborator

ikkez commented May 3, 2015

you can now set a custom handler for suspicious sessions as 4th parameter

@ikkez ikkez closed this as completed May 3, 2015
@Rayne
Copy link
Contributor

Rayne commented May 3, 2015

@ikkez Third parameter for Jig and Mongo.

@ethanpil
Copy link

Can we have some sample code and more information in the documentation?

@ikkez
Copy link
Collaborator

ikkez commented Jul 29, 2015

Alright, I've written it down now. http://fatfreeframework.com/session#onsuspect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants