Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6.x] Change cookie helper signature to match CookieFactory #31974

Merged
merged 1 commit into from Mar 15, 2020

Conversation

@dmason30
Copy link
Contributor

dmason30 commented Mar 15, 2020

TL;DR
This changes the cookie() helper signature to match the CookieFactory::make function. These appear to have been mismatched since #23200.

Set to 6.x as recommended by @GrahamCampbell in #31970

Problem
I was looking into the below warning we were getting on our frontend SPA in Chrome. I had already set the secure config option to true in config/session.php. So I decided to investigate why the cookie was not being sent with the secure flag as expected.

A cookie associated with a cross-site resource at https://example.com/ was set without the SameSite attribute. A future release of Chrome will only deliver cookies with cross-site requests if they are set with SameSite=None and Secure. You can review cookies in developer tools under Application>Storage>Cookies and see more details at https://www.chromestatus.com/feature/5088147346030592 and https://www.chromestatus.com/feature/5633521622188032.

Investigation
The cookie was being returned by using the response->cookie method as described in the docs (https://laravel.com/docs/7.x/responses#attaching-cookies-to-responses).

return response($content)->cookie('token', $jwtToken);

After looking into that method I realised it is using the cookie() helper function under the hood and this had the$secure = false as one of its arguments this explained why it was not using the setting specified in the session.php config.

Workaround
To provide an immediate fix for our application I changed the code to use the Cookie::make factory method directly since this does use the configured value as the method signature for that method has $secure = null. So we changed our response code to this:

return response($content)->cookie(\Cookie::make('token', $jwtToken));

Fix
Changed cookie() helper function signature from $secure = false to $secure = null to match the CookieFactory::make function .

@GrahamCampbell GrahamCampbell merged commit 07f225a into laravel:6.x Mar 15, 2020
7 checks passed
7 checks passed
P7.2 - Sprefer-lowest
Details
P7.2 - Sprefer-stable
Details
P7.3 - Sprefer-lowest
Details
P7.3 - Sprefer-stable
Details
P7.4 - Sprefer-lowest
Details
P7.4 - Sprefer-stable
Details
continuous-integration/styleci/pr The analysis has passed
Details
@dmason30 dmason30 deleted the dmason30:cookie-helper-fix branch Mar 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.