Changes Unknown when pulling ad7fb92 on dzirtusss:add-csrf into * on shakacode:master*.

We probably don't want to suck in lodash, at least not all of it. Are we using lodash elsewhere in this module? We could consider bringing in a tiny bit of it.

Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

Temporary. Lodash not needed.

Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

Changes Unknown when pulling 6b57be0 on dzirtusss:add-csrf into * on shakacode:master*.

Actually I think we should not need to include jsonHelper into gem and specifically include json Accept and inContent-Type into headers because it is already included by ??? as follows:

Content-Type: application/json;charset=UTF-8
Accept: application/json, text/plain, */*
Cookie: _rails-react-tutorial_session=...

And everything works fine.

Maybe it will not work with fetch same way but will need to update first from axios to isomorphic-fetch in tutorial.

I think it will be ok if we use fetch like:

credentials: 'same-origin',
header: RubyOnRails.authenticityHeader(),

Was also thinking does this sounds better names? It better be more clear. Any opinions?

getCSRFHeader()
getCSRFToken()

Coverage remained the same at 82.36% when pulling e202218 on dzirtusss:add-csrf into a6373f0 on shakacode:master.

Coverage remained the same at 82.36% when pulling e202218 on dzirtusss:add-csrf into a6373f0 on shakacode:master.

Coverage remained the same at 82.36% when pulling 3af481d on dzirtusss:add-csrf into a6373f0 on shakacode:master.

Review status: 0 of 3 files reviewed at latest revision, 4 unresolved discussions.

CHANGELOG.md, line 11 [r5] (raw file):

- React on Rails now correctly parses single-digit version strings from package.json [#491](https://github.com/shakacode/react_on_rails/pull/491) by [samphilipd ](https://github.com/samphilipd ).
- Fixed assets symlinking to correctly use filenames with spaces. Begining in [#510](https://github.com/shakacode/react_on_rails/pull/510), ending in [#513](https://github.com/shakacode/react_on_rails/pull/513) by [dzirtusss](https://github.com/dzirtusss) 
- Added getAuthenticityToken() and authenticityHeader() javascript helpers for easier use when working with CSRF protection tag generated by Rails [#517](https://github.com/shakacode/react_on_rails/pull/517) by [dzirtusss](https://github.com/dzirtusss) 

update names?

node_package/src/ReactOnRails.js, line 82 [r2] (raw file):

node_package/src/ReactOnRails.js, line 96 [r5] (raw file):

authenticityHeader(options) {

addAuthenticityHeaders(options = {})

Comments from Reviewable

Review status: 0 of 3 files reviewed at latest revision, 4 unresolved discussions.

node_package/src/ReactOnRails.js, line 96 [r5] (raw file):

withAuthenticityHeaders(otherHeaders = {})

Comments from Reviewable

Review status: 0 of 3 files reviewed at latest revision, 4 unresolved discussions.

node_package/src/ReactOnRails.js, line 96 [r5] (raw file):

Looks best if we go with authenticityHeaders(otherHeaders)

var myInit = { method: 'GET',
               headers: ReactOnRails.authenticityHeaders(),
               mode: 'cors',
               cache: 'default' };

fetch('flowers.jpg',myInit)
.then(function(response) {
  return response.blob();
})
.then(function(myBlob) {
  var objectURL = URL.createObjectURL(myBlob);
  myImage.src = objectURL;
});

or with other options:

var otherHeaders = { foo: 'bar' };
var headers = ReactOnRails.authenticityHeaders(otherHeaders);
var myInit = { method: 'GET',
               headers: headers,
               mode: 'cors',
               cache: 'default' };

fetch('flowers.jpg',myInit)
.then(function(response) {
  return response.blob();
})
.then(function(myBlob) {
  var objectURL = URL.createObjectURL(myBlob);
  myImage.src = objectURL;
});

Comments from Reviewable

We're ready to go!

1. Adjust the naming for the docs.

Reviewed 1 of 2 files at r4, 2 of 2 files at r5.
Review status: all files reviewed at latest revision, 8 unresolved discussions.

node_package/src/ReactOnRails.js, line 82 [r2] (raw file):

Note, this one is missing. Please add with the other apis

/**
*  Allow directly calling the page loaded script in case the default events that trigger react rendering are not sufficient, such as when loading JavaScript asynchronously with TurboLinks:
* More details can be found here: https://github.com/shakacode/react_on_rails/blob/master/docs/additional-reading/turbolinks.md
*/
  reactOnRailsPageLoaded() {
    ClientStartup.reactOnRailsPageLoaded();
  },

node_package/src/ReactOnRails.js, line 76 [r5] (raw file):

  /**
   * Returns CSRF authenticity token inserted by Rails csrf_meta_tags

We can consider putting this in the internal API.

const token = ReactOnRails.authenticityHeaders()['X-CSRF-Token'];

That way we only add one method to the API.

@robwise How does this fit with https://www.friendsandguests.com/

CC: @alexfedoseev

node_package/src/ReactOnRails.js, line 82 [r5] (raw file):

  getAuthenticityToken() {
    const token = document.querySelector('meta[name="csrf-token"]');
    return token ? token.content : null;

Very important: Let's put in this code in a small file, called Authenticity.js which contains the two authenticity functions. See how this file delegates other chunks.

Then there's the test for this in a separate file.

We also still need a basic test at the top level to verify the public api calls exist.

node_package/src/ReactOnRails.js, line 86 [r5] (raw file):

  /**
   * Returns header with csrf authenticity token

I'd like to see some detailed doc on this:

References
* http://guides.rubyonrails.org/security.html#cross-site-request-forgery-csrf
* https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch
* https://github.com/matthew-andrews/isomorphic-fetch

Example:

blah, blah

and we need this material duplicated: https://github.com/shakacode/react_on_rails/blob/master/docs/api/javascript-api.md

@AlexKVal Know of any way that we can generate this file?

The README.md should have a couple sentence blurb on about using isormorphic-fetch vs. using the built in jQuery fetch (https://github.com/rails/jquery-ujs) that already puts in the headers.

node_package/tests/ReactOnRails.test.js, line 138 [r5] (raw file):

var otherHeaders = { foo: 'bar' };
var headers = ReactOnRails.authenticityHeaders(otherHeaders);
var myInit = { method: 'GET',
headers: headers,
mode: 'cors',
cache: 'default' };
Yes -- let's stick to ES6 syntax.

node_package/tests/ReactOnRails.test.js, line 148 [r5] (raw file):

    'authenticityHeader returns valid header with CFRS token'
  );
});

Nice tests!

See comment above on putting this into a smaller file.

We still need a test here that verifies this as well. I'm OK with some duplication for this small amount of code.

Maybe @AlexKVal has an opinion.

Comments from Reviewable

Review status: all files reviewed at latest revision, 8 unresolved discussions.

node_package/src/ReactOnRails.js, line 82 [r2] (raw file):

node_package/src/ReactOnRails.js, line 76 [r5] (raw file):

The rest is not necessarily specific to rails and therefore is a bit out of scope for this project IMO. You're making assumptions that everyone using ReactOnRails wants X-Requested-With. Not sure that's really true, for example they may be trying to debug something. This also would make it extremely clear what the method does just by the name, whereas authenticityHeaders could mean pretty much anything under the sun.

node_package/src/ReactOnRails.js, line 86 [r5] (raw file):

Comments from Reviewable

Review status: all files reviewed at latest revision, 8 unresolved discussions.

node_package/src/ReactOnRails.js, line 82 [r2] (raw file):

Java is notorious for calling everything getXXXX and setXXXXX.

node_package/src/ReactOnRails.js, line 76 [r5] (raw file):

Let's add the two methods

authenticityToken()

ajaxHeaders(otherHeaders)

node_package/src/ReactOnRails.js, line 86 [r5] (raw file):

Comments from Reviewable

Review status: all files reviewed at latest revision, 9 unresolved discussions.

node_package/src/ReactOnRails.js, line 76 [r5] (raw file):

railsAuthenticityToken()

and

railsAuthenticityHeaders(otheHeaders)

node_package/src/ReactOnRails.js, line 95 [r5] (raw file):

    return Object.assign(options, {
      'X-CSRF-Token': ReactOnRails.getAuthenticityToken(),
      'X-Requested-With': 'XMLHttpRequest'

I did some research on the X-Requested-With header. This is specific to Rails. ReactOnRails is specific to Rails. So this is OK and appropriate.

Naturally, for API requests to other servers, we won't send this or the X-CSRF-Token.

Comments from Reviewable

Done.

Review status: 0 of 6 files reviewed at latest revision, 10 unresolved discussions.

CHANGELOG.md, line 11 [r5] (raw file):

node_package/src/ReactOnRails.js, line 82 [r2] (raw file):

node_package/src/ReactOnRails.js, line 76 [r5] (raw file):

node_package/src/ReactOnRails.js, line 82 [r5] (raw file):

node_package/src/ReactOnRails.js, line 95 [r5] (raw file):

node_package/src/ReactOnRails.js, line 96 [r5] (raw file):

node_package/tests/ReactOnRails.test.js, line 138 [r5] (raw file):

node_package/tests/ReactOnRails.test.js, line 148 [r5] (raw file):

Comments from Reviewable

Reviewed 6 of 6 files at r6.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

Comments from Reviewable

Reviewed 1 of 1 files at r7.
Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks broke.

README.md, line 420 [r7] (raw file):

#### Using Rails built-in CSRF protection in JavaScript

Rails has built-in protection for Cross-Site Request Forgery (CSRF), see [Rails Documentation](http://guides.rubyonrails.org/security.html#cross-site-request-forgery-csrf). To nicely utilize this feature in JavaScript requests, React on Rails is offerring two helpers that can be used as following:

Should we mention somewhere that jquery-ujs adds this jQuery's ajax methods?

README.md, line 426 [r7] (raw file):

// reads from DOM csrf token generated by Rails in <%= csrf_meta_tags %>
csrfToken = ReactOnRails.authenticityToken();               

extra trailing spaces here -- see red dots.

Comments from Reviewable

Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks broke.

README.md, line 420 [r7] (raw file):

README.md, line 426 [r7] (raw file):

node_package/src/ReactOnRails.js, line 86 [r5] (raw file):

Comments from Reviewable

and we need this material duplicated:
https://github.com/shakacode/react_on_rails/blob/master/docs/api/javascript-api.md
@AlexKVal Know of any way that we can generate this file?

@justin808 I am definitely not an expert in automated documentation although I dare to suppose smth like that https://github.com/cbou/markdox would be fit for it.
And o/c it is for another issue.
🍒

Coverage remained the same at 82.36% when pulling 426adaa on dzirtusss:add-csrf into e293a94 on shakacode:master.

Review status: 6 of 7 files reviewed at latest revision, 7 unresolved discussions.

README.md, line 433 [r8] (raw file):

If you are using jquery-ujs for AJAX calls, than these helpers are not needed and jquery-ujs library updates header automatically, see jquery-ujs documentation.

Minor grammar fix:

If you are using [jquery-ujs](https://github.com/rails/jquery-ujs) for AJAX calls, than these helpers are not needed because the [jquery-ujs](https://github.com/rails/jquery-ujs) library updates header automatically, see [jquery-ujs documentation](https://robots.thoughtbot.com/a-tour-of-rails-jquery-ujs#cross-site-request-forgery-protection).

Comments from Reviewable

One tiny change!

Reviewed 1 of 1 files at r8.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

Comments from Reviewable

Review status: 6 of 7 files reviewed at latest revision, 5 unresolved discussions.

README.md, line 433 [r8] (raw file):

Comments from Reviewable

Reviewed 1 of 1 files at r10.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

Comments from Reviewable