Skip to content

Conversation

@wouterj
Copy link
Member

@wouterj wouterj commented Aug 25, 2020

Q A
Branch? 5.1
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #37534
License MIT
Doc PR n/a

The RememberMeAuthenticator wrongly assumed the implementation details of AbstractRememberMeServices::autoLogin(). This means that (a) the authenticator did not work with other - custom - implementations of RememberMeServicesInterface and (b) there was a potentional to get an "Call to a member function getUser() on null" error.

This code removes all assumptions of the autoLogin() logic, other than that stated in the PHPdoc:

* No attempt whatsoever is made to determine whether the browser has requested
* remember-me services or presented a valid cookie. Any and all such determinations
* are left to the implementation of this method.
*
* If a browser has presented an unauthorised cookie for whatever reason,
* make sure to throw an AuthenticationException as this will consequentially
* result in a call to loginFail() and therefore an invalidation of the cookie.
*
* @return TokenInterface|null
*/
public function autoLogin(Request $request);

@wouterj wouterj force-pushed the issue-37534/rememberme-getuser branch from fb2c12d to 93aea91 Compare August 25, 2020 15:26
@PhilETaylor
Copy link
Contributor

I have tested #37943 against my replication steps in #37534 and it resulted in a graceful failure as expected. 🎉🎉 Thanks for this @wouterj

@fabpot
Copy link
Member

fabpot commented Aug 25, 2020

Thank you @wouterj.

@fabpot fabpot merged commit df7950d into symfony:5.1 Aug 25, 2020
@wouterj wouterj deleted the issue-37534/rememberme-getuser branch August 25, 2020 19:18
@fabpot fabpot mentioned this pull request Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants