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

Ability to avoid session creation for anonymous page hit (with patch) #30332

Open
ant5 opened this Issue Feb 21, 2019 · 4 comments

Comments

Projects
None yet
4 participants
@ant5
Copy link

ant5 commented Feb 21, 2019

Description
Sites (with statefull Firewall) with secured area always start session for any anonymous hit of url in secured area. This is related to PathTrait feature. Every hit generate a session which store initial URL (PathTrait) that may be used later to redirect user to initial page after authentication. But in the case that this was a bot/anonymous/DoS hit the session just left in session storage for it lifetime.

It may be good to avoid such behavour for non-standart authentication schemes and DoS tolerance.

Proposed solution is to introduce earlySession(): bool function in AuthenticationEntryPointInterface which must be consulted everytime when making decision of creating (storing data in) anonymous session. At this time only saveTargetPath == savePathTrait affected.
The earlySession() function is:

+    /**
+     * Returns whether to start session before authentication.
+     *
+     * The true value means to start session and enable PathTrait feature (ability to
+     * redirect/forward user to initially hitted page after authentication).
+     *
+     * The false value mean no session start (disabling PathTrait feature)
+     * before authentication avoiding generating and storing session for anonymous
+     * page hit.
+     *
+     * @return bool
+     */
+    public function earlySession();

Example
In case of earlySession implementation avoiding session start will look like this:

class MyAuthenticator extends AbstractGuardAuthenticator {
...
    /*
      disable session for anonymous
    */
    public function earlySession()
    {
      return false;
    }

...
}

Patch is here:
https://gist.github.com/ant5/a95afbfaa9df2c188653038a1ecd68f4

@linaori

This comment has been minimized.

Copy link
Contributor

linaori commented Feb 22, 2019

This does make sense for anonymous pages, but in order to be anonymous, it has to have verified that you're not logged-in first in case of authenticators, which requires the session unless stateless. How do you propose to address that?

@javiereguiluz can you switch the HttpFoundation with a Security label? 😄

@ant5

This comment has been minimized.

Copy link
Author

ant5 commented Feb 22, 2019

It doesn't affect anonymous sessions. This is only for case of secured area when firewall trigger "authentication required" state. To be more precisely it affect only startAuthentication() function.

I've slightly modified my patch to be more clear:
https://gist.github.com/ant5/e81f94cea7404ca8e520d0367ef27427

The only real change is:

-        if (!$this->stateless) {
+        if (!$this->stateless && $this->authenticationEntryPoint->earlySession()) {
             $this->setTargetPath($request);
         }

@linaori

This comment has been minimized.

Copy link
Contributor

linaori commented Feb 22, 2019

Authorization happens after authentication, as authorization triggers the requirement for authentication. Due to this sequence, there will always be an attempt of authentication, which requires the session.

@xabbuh xabbuh added Security and removed HttpFoundation labels Feb 22, 2019

@ant5

This comment has been minimized.

Copy link
Author

ant5 commented Feb 22, 2019

I think you've confused by function name "startAuthentication()". Despite it name it is for "begin login process". So "an attempt of authentication" is not affected.

When "an attempt of authentication" is negative Symfony begin login process by calling "startAuthentication()". The login process implicity autostart a session only for storing initial URL. My patch is for take control over the session autostart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.