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-less store/restoreRequest #302

Open
mabar opened this issue Jun 8, 2022 · 2 comments
Open

Session-less store/restoreRequest #302

mabar opened this issue Jun 8, 2022 · 2 comments

Comments

@mabar
Copy link
Contributor

mabar commented Jun 8, 2022

Current implementation of Presenter->store/restoreRequest() uses session for storage.

Pluses are:

  • Implementation is completely hidden, no-one can manipulate request for phishing.
  • Whole request is stored, without any data lost. When submiting a form, being logged out and logging back in the form is still processed properly.
  • Unlimited number of requests can be stored.

Minuses are:

  • Starting session. Quite common (and afaik main) use-case for (re)storing request is redirect of unauthenticated user to login page. In intranet applications and administration, this is used even for landing page. And therefore every single bot access creates session file.
  • Whole request is stored. This includes request body, which can be quite big and sent with an intention to spam application and with an exception of forms submitting probably never intentional.

So an ideal implementation should not start session, keep phishing protection, add protection agains spamming session storage with useless data and yet, keep the forms re-submit working.


What are (not) possible implementations?

  • An url parameter. Even if used just for requests with no body, it still opens posibility for phishing, possibly even when domain is checked, so nope.
  • A http-only cookie? Safe from spam, but still not enough for storing request body and limits number of stored requests (and I believe people who open browser with 20 saved tabs, being logged out and wanting all of them to still work after login exist, I am one of them)

So what is possible? I think we need an interface to allow alternative storages which can drop expired keys immediatelly:

  • Instead of session could be used e.g. redis. Currently expiration of request key is controlled by Nette session mechanism. If user comes back after 10 minutes (current default expiration), key is removed by Nette. But bot will never come back with same session id, data are removed only when session itself expires and garbage collector removes them. Time for that removal could be weeks. Benefit of redis is it can drop data immediatelly.
interface RequestStorage
{

  /**
   * @return Globally unique request ID
   */
  public function store(Request $request, \DateTimeInterface $expiration): string;
  
  public function restore(string $requestId): ?Request;
  
}

Alternative could be to introduce SessionStorage interface in nette/http. Such interface would accept expiration time of session key and pass it to underlying storage, which could delete key immediatelly after expiration and not wait for another user request.
Worst case scenario with such storage is that an empty session remains until session itself expires.


To store request body only when needed, Presenter->storeRequest() could have a parameter bool $keepRequestBody = true. In my case I would store request body only if $this->firewall->getLastExpiredLogin()) !== null is true. Only users who were already logged in would have whole request saved, not newcomers, whose could be bots. Implementation for nette/security users should be quite similar.


While it does not solve the problem perfectly, it allows developers to reduce it to bare minimum and have control over it. And it could remain backward compatible with current implementation in Presenter.

@JanTvrdik
Copy link
Contributor

Instead of session could be used e.g. redis

I don't understand. If you can use redis for request storage, why are you not using redis for sessions?

@mabar
Copy link
Contributor Author

mabar commented Jun 10, 2022

@JanTvrdik Problem is native session mechanism itself is not enough to delete session keys immediately after expiration. While the stored request lifetime is just 10 minutes, session itself can exist for weeks and the key is not deleted until session expires or Nette uses this session.

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

No branches or pull requests

2 participants