Skip to content

[AbstractRememberMeServices] add support for the remember_me parameter in the query #3460

Closed
wants to merge 1 commit into from

4 participants

@tamirvs
tamirvs commented Feb 27, 2012

Bug fix: no
Feature addition: yes
Backwards compatibility break: no

I found myself having to pass the _remember_me parameter in the query instead of as a POST parameter, because I use OpenID which don't support POSTing, and I think it could be helpfull and harmless feature.

@asm89
asm89 commented Feb 27, 2012

+1. Having the same 'problem' with oauth2.

@schmittjoh schmittjoh commented on the diff Feb 27, 2012
...curity/Http/RememberMe/AbstractRememberMeServices.php
@@ -283,7 +283,11 @@ protected function isRememberMeRequested(Request $request)
return true;
}
- $parameter = $request->request->get($this->options['remember_me_parameter'], null, true);
@schmittjoh
schmittjoh added a note Feb 27, 2012

Just use $request->get() instead.

@asm89
asm89 added a note Feb 27, 2012

That will also allow other sources (PATH and COOKIE). Sure you want to add those too?

Also $request->get() is slow, as per the notice here: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Request.php#L460. On the other hand it is only run on login success.

@asm89
asm89 added a note Feb 27, 2012

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fabpot fabpot closed this in ddeac9a Mar 2, 2012
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.