[Security][Guard] check if session exist before using it #19218
Conversation
|
Can you please add a test covering the fix? |
|
Ok, I will add it. |
|
Ok tests added. |
|
AppVeyor is not passing :( |
|
And now AppVeyor is passing, strange... |
| /** | ||
| * @author Jean Pasdeloup <jpasdeloup@sedona.fr> | ||
| */ | ||
| class MockFormLoginAuthenticator extends AbstractFormLoginAuthenticator |
There was a problem hiding this comment.
This class should be moved at the bottom of the test file.
|
|
||
| if (!$targetPath) { | ||
| if (!isset($targetPath) || !$targetPath) { |
There was a problem hiding this comment.
I would prefer to explicitly initialize $targetPath to nullabove and make a check on null here.
There was a problem hiding this comment.
Ok, done.
I also improved the tests.
|
|
||
| public function testAuthenticateWithoutSession() | ||
| { | ||
| $authenticator = new MockFormLoginAuthenticator(); |
There was a problem hiding this comment.
Why not use a PHPUnit mock instead?
|
Thank you @pasdeloup. |
…pasdeloup) This PR was squashed before being merged into the 2.8 branch (closes #19218). Discussion ---------- [Security][Guard] check if session exist before using it | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #18958 | License | MIT | Doc PR | - As stated by @Shekhovtsovy when the Guard component is used without the Symfony full stack (for instance in Laravel), $request->getSession() may be null. An additionnal PR will be needed for 3.1 but it may be better to check this one before. Commits ------- a3f7510 [Security][Guard] check if session exist before using it
As stated by @Shekhovtsovy when the Guard component is used without the Symfony full stack (for instance in Laravel), $request->getSession() may be null.
An additionnal PR will be needed for 3.1 but it may be better to check this one before.