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

[5.8] Remove unnecessary X-CSRF-TOKEN header from our Axios instance #5083

Merged
merged 1 commit into from Aug 21, 2019

Conversation

jessarcher
Copy link
Member

@jessarcher jessarcher commented Aug 21, 2019

In bootstrap.js we currently add a X-CSRF-TOKEN HTTP header (note the 'C') to the Axios instance that we instantiate, using the value of a <meta> tag added by the auth scaffolding. This is not necessary because Axios already has similar functionality enabled by default where it will add a X-XSRF-TOKEN HTTP header (note the second 'X') using the value of the XSRF-TOKEN cookie.

On a current installation of Laravel, our Axios instance requests have both the X-CSRF-TOKEN and the X-XSRF-TOKEN HTTP headers. The only difference is the X-CSRF-TOKEN value is unencrypted, while the X-XSRF-TOKEN value is encrypted.

I believe we can safely remove the X-CSRF-TOKEN HTTP header configuration from bootstrap.js because Laravel already verifies requests using the X-XSRF-TOKEN HTTP header in the VerifyCsrfToken middleware. The same is also now true for Passport >=7.4 TokenGuard for users consuming their API with JavaScript.

I have verified that this works on a fresh Laravel 5.8 installation for Axios POST requests going through our web middleware group, as well as requests going through the auth:api middleware when Passport is configured as linked above.

I have submitted a draft PR at laravel/docs#5382 to update the docs if this is accepted.

This is unnessecery code because Axios already automatically adds
a X-XSRF-TOKEN header from the XSRF-TOKEN cookie encrypted value on
same-origin requests. The `VerifyCsrfToken` middleware and Passport's
`TokenGuard` already allow using the `X-XSRF-TOKEN` header.
@jessarcher jessarcher changed the title Remove unessecery X-CSRF-TOKEN header from our Axios instance Remove unnecessary X-CSRF-TOKEN header from our Axios instance Aug 21, 2019
@driesvints driesvints changed the title Remove unnecessary X-CSRF-TOKEN header from our Axios instance [5.8] Remove unnecessary X-CSRF-TOKEN header from our Axios instance Aug 21, 2019
@GrahamCampbell
Copy link
Member

Maybe this is best targetting 6.0?

@driesvints
Copy link
Member

@GrahamCampbell not sure why since this isn't a breaking change. The skeleton is only used for new installs.

@taylorotwell taylorotwell merged commit aa74fcb into laravel:master Aug 21, 2019
jenky added a commit to jenky/laravel that referenced this pull request Aug 23, 2019
Remove manual adding of X-CSRF-TOKEN header (laravel#5083)
@reinink reinink mentioned this pull request Aug 27, 2019
@tillkruss
Copy link
Contributor

What about the meta tag? Can’t that be removed as well?

@jessarcher
Copy link
Member Author

What about the meta tag? Can’t that be removed as well?

laravel/echo still depends on it

I can't think of any reason off the top of my head why it couldn't be modified to use the XSRF-TOKEN cookie as well though.

ncatanchin added a commit to ncatanchin/laravel that referenced this pull request Aug 30, 2019
Omranic added a commit to rinvex/cortex that referenced this pull request Sep 3, 2019
This is unnessecery code because Axios already automatically adds
a X-XSRF-TOKEN header from the XSRF-TOKEN cookie encrypted value on
same-origin requests. The `VerifyCsrfToken` middleware and Passport's
`TokenGuard` already allow using the `X-XSRF-TOKEN` header.
Omranic added a commit to rinvex/cortex that referenced this pull request Nov 23, 2019
* release/v2.2.0: (24 commits)
  Bump version
  Apply fixes from StyleCI
  Upgrade Laravel to v6.5
  Update clockwork .gitignore file
  Override config files
  Upgrade to Laravel v6.3
  Add missing config options
  Fix phone number input display issue
  Add new reauthentication config option
  Update config files to Laravel v6.2
  Cast process.env.MIX_HASHIDS_LENGTH to number to fix JS error
  Update project to Laravel v6.2
  Upgrade to Laravel v6 and update composer / npm packages
  Update jquery.validation library
  Use singular guard names for email verification brokers
  Use singular for passwords
  Enforce consistency
  Remove manual adding of X-CSRF-TOKEN header (laravel/laravel#5083)
  Update config files & enforce consistency
  Update media config options
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants