Skip to content

[Login] Why it's necessary to have a phpsessid to login? #3703

Closed
joanteixi opened this Issue Mar 27, 2012 · 13 comments
@joanteixi

We're in a scenario where there is a login form created by Sencha, that sends username and password directly to login_check. We created an own Listener to bypass the csrf protection. The first time you do login, the AbstractAuthenticationListener throws an exception:

            if (!$request->hasPreviousSession()) {
                throw new SessionUnavailableException('Your session has timed-out, or you have disabled cookies.');
            }

Wich will be the best approach to resolve this?

Joan

@stof
Symfony member
stof commented Mar 27, 2012
@gillest
gillest commented Apr 19, 2012

So far, we had to make it as a workaround...
https://github.com/gillest/HackSessionBundle

@joanteixi

nice. I didn't know that was possible create a session like you do in the listener. Thks, i will try.

@gillest
gillest commented Apr 20, 2012

Well, to tell you the truth, I would fully prefer an answer/solution from @schmittjoh cause it remains an horrible hacky solution...

@joanteixi

oh... maybe... but my solution was worst... belive me... (something like redirect to an empty action... glups...)

@gillest
gillest commented Apr 24, 2012

By the way, seeing the message "Your session has timed-out, or you have disabled cookies.", I made a use case trying to make raise a time-out on the server side: the message is not displayed which is bad.
If this is not a security check, this seems to be even more useless...
We wait a bit more cause I do not want to make a PR without any answer from @schmittjoh .

@marcw
marcw commented May 14, 2012

@schmittjoh Would you please give us an answer?

@Dattaya
Dattaya commented May 25, 2012

I'd like to hear schmittjoh's answer too.

@oyerli
oyerli commented Jun 7, 2012

Any progress on this issue? It causes problems if we want to use a caching system (i.e when the session auto_start is off).

@netandreus

+1 me interesting too
@gillest solution working fine, just don't forget to clear cache for production environment

@vicb
vicb commented Jul 4, 2012

@schmittjoh I would really appreciate your opinion on this issue.

Sessions are now auto-started and people are even more likely not to have a previous session when they submit their login form.

I can not understand why this check is here: you have to submit the form to trigger the listener so I think it is useless.

The question is: can this check be removed, if not why is it useful here (and should we move it somewhere else) ?

Thanks for your help.

@asm89
asm89 commented Jul 11, 2012

@vicb I think it can be safely removed from the AbstractAuthenticationListener. The next method called in the sequence is the abstract attemptAuthentication() method. Listeners that inherit from the class can check for a previous session in there and throw the appropriate exception if needed. In core the UsernamePasswordFormAuthenticationListener would only have to check for that if a csrf provider is injected.

It would be a BC break though for people implementing their own listeners that do depend on a previous session being active.

If you want to go through with it I'd be happy to send a PR.

@schmittjoh Am I seeing this correctly?

@fabpot fabpot added a commit that referenced this issue Mar 23, 2013
@fabpot fabpot merged branch adrienbrault/security-feature (PR #4776)
This PR was merged into the master branch.

Discussion
----------

[2.2] [Security] Add an option to disable the hasPreviousSession() check in AbstractAuthenticationListener

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/adrienbrault/symfony.png?branch=security-feature)](http://travis-ci.org/adrienbrault/symfony)
Fixes the following tickets: #3703
Todo: Add this option to the symfony doc security configuration reference
License of the code: MIT
Documentation PR: N/A

As stated in #3703, all authentication listeners that inherit from AbstractAuthenticationListener, only work when a previous session has been created.
This PR allows to change the default behavior in the security.yml file.

Example:

```yml
security:
    firewalls:
        secured_area:
            pattern:    ^/demo/secured/
            form_login:
                check_path: /demo/secured/login_check
                login_path: /demo/secured/login
                require_previous_session: false # The default value is true
            logout:
                path:   /demo/secured/logout
                target: /demo/
            #anonymous: ~
            #http_basic:
            #    realm: "Secured Demo Area"
```

PS: While removing my old commit, it closed the #4774 PR ...

Commits
-------

0562463 [Security] Add an option to disable the hasPreviousSession() check in AbstractAuthenticationListener
aa26e66
@fabpot fabpot closed this Mar 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.