Support of redirect after successful login and remember me functionality

[cmyker]

Support of redirect after successful login for hiding auth token from url and browser history
Support of symfony's built-in remember me functionality

[lunetics]

+1

[Nyholm]

I like the idea of a remember me cookie. Even though you must have a specific use case to actually allow it. For my application it feels kind of strange to authenticate users by a Url and then expecting them to click on the logout button themselves. But I can see the value of it.

I don't understand why you need the special query parameter to redirect after successful authentication. Can you please elaborate on this? How does it differ from using http://example.com/redirect?_al=[token]. ?

[jmikola]

For alphabetizing, these should be re-ordered:

use Symfony\Component\Security\Http\SecurityEvents;
use Symfony\Component\Security\Http\Authentication\AuthenticationSuccessHandlerInterface;
use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
use Symfony\Component\Security\Http\Firewall\ListenerInterface;
use Symfony\Component\Security\Http\RememberMe\RememberMeServicesInterface;

[jmikola]

I'm a bit confused how the remember-me integration relates (if at all), to the following TODO item we already have in AutoLoginListener:

TODO: This authentication method should be considered the same as remember-me according to AuthenticationTrustResolver. That will entail either refactoring to extend RememberMeToken or adding a service to compose the existing AuthenticationTrustResolver and implement custom logic to respect our own token class.

Thinking back, the original goal was to pick up the query parameter as if it were a remember-me cookie, giving it the same level of trust (below a form-based login).

[Nyholm]

I like the idea of a remember me cookie. Even though you must have a specific use case to actually allow it. For my application it feels kind of strange to authenticate users by a Url and then expecting them to click on the logout button themselves. But I can see the value of it.

I don't understand why you need the special query parameter to redirect after successful authentication. Can you please elaborate on this? How does it differ from using http://example.com/redirect?_al=[token].?

Ping @cmyker, I think you missed my question.

[cmyker]

It is not for redirecting, it is for setting the remeber_me cookie. Symfony's stock remember me functionality is looking for it in request to enable it. By default this parameter is "_remember_me". But it is changeable.

[Nyholm]

It is not for redirecting

Hm, sorry, Im not sure I understand. Are you saying this PR is not for redirecting?

It looks like it is always redirecting when an auto login token is found. And also: https://github.com/jmikola/JmikolaAutoLoginBundle/pull/15/files#diff-04c6e90faac2675aa89e2176d2eec7d8R107

[cmyker]

Please read above

I just took this code from Symfony\Component\Security\Http\Firewall\AbstractAuthenticationListener::onSuccess because it was used by symfony's default authentication routine I was stepping through in debugger. By doing this I achieved correct redirect after successful login, which I think is good for security reasons because it is really easy to expose you login details by just coping and then sending that link to colleague. It was necessary to generate a proper response to make remeber_me cookie work.
Add a line note

[Nyholm]

I am not against the feature at all. Don't get me wrong, I like it. I cant make up my mind if it should be a part of this library/bundle or not.

Let me introduce a scenario for you. I send out weekly subscriptions to my users. In every email I attach a link to http://example.com/secure/change-email-subscription. Naturally I would like them to be logged in automatically, so I use the link http://example.com/secure/change-email-subscription?_al=token.
As far as I have understood this PR, after a successful login we let the Symfony authentication listener handle the login which may redirect the user to (depending on configuration)

• /you-are-logged-in (default path)
• the page you last tried to visit (referer)
• the value in _target_path query parameter.

Issue 1) I currently force my users to always see /you-are-logged-in after they login. So I need to create another instance of AuthenticationListener with different config to be able to use this library and send user directly to a page.

To make sure the user gets to the page i want, I should use the link http://example.com/secure/change-email-subscription?_al=token&_target_path=/secure/change-email-subscription

Issue 2) If the user that clicks the link is authenticated already, he will not be redirected and still suffer from the security issue you are highlighting.

which I think is good for security reasons because it is really easy to expose you login details by just coping and then sending that link to colleague.

To address issue 1 and 2 and to solve the "remember me" functionality, I think it would be better to create a new controller for a url like http://example.com/redirect?_al=token&remember=1&_target_path=/secure/change-email-subscription

But including the controller in this library would violate the common resue principle and it might also be application specific so I suggest it should be done in the application instead.

[cmyker]

Sorry, but where your assumptions come from? Have you tested your scenarios above?

Issue 1) I currently force my users to always see /you-are-logged-in after they login. So I need to create another instance of AuthenticationListener with different config to be able to use this library and send user directly to a page.

is not true, http://example.com/secure/change-email-subscription?_al=token will redirect you after login right to that page, but without ?_al=token

Issue 2) If the user that clicks the link is authenticated already, he will not be redirected and still suffer from the security issue you are highlighting.

is wrong also, on my installation it it redirects to the requested page regardless of login overwrite settings.

The main issue here is that some developers want to have this functionality and some not. So lets create a config option for them to decide do they need this redirect or not. The only thing we need to decide - should we turn it on by default. I propose no.
I can achieve setting remember_me cookie in the response by listening on response event.

[g4vroche]

Hi,
Any plans on merging this PR ?

At least the redirect part to remove the token is an absolute must have.
It should even be activated by default if not for BC.
Without redirect to remove the token, we time travel back in the old ?PHPSESSID time.

And these days, with integrated support chats on websites, that shares a user's current URL to ambassadors, the user doesn't even have to do a copy/paste to leak an access to its account (true story).

Let us know if any more work needs to be done before merging, or if we should split the PR to separate the remember_me feature, since this part looks like being more controversial (IMHO it is a totally legit option to have too, with a config switch of course).

[jmikola]

Let us know if any more work needs to be done before merging, or if we should split the PR to separate the remember_me feature, since this part looks like being more controversial (IMHO it is a totally legit option to have too, with a config switch of course).

You've convinced me that the redirect option is a good idea. I should be able to extract that bit from the current PR. I'm still undecided on RememberMe but will review that again when I get a chance to merge the redirect bit.

Apologies for letting this sit as long as it has. I managed to find some time to merge and release the Symfony 3.0 compatibility fixes today, so hopefully I can revisit this within a week or two.