React-starter-kit: CSRF Protection?

[hluedeke]

Hi there! I want to start off by saying that this boilerplate is great! Thanks for sharing!

I have been looking through the code and can't seem to find CSRF protection anywhere. Is this the case? If so, I'd be curious to know why it hasn't been implemented. Is there something I am missing?

Thanks!

[langpavel]

Hi, good question!
At first, you can use general purpose express middleware and tweak fetch.
At second you should handle security token correctly.
This is behind limits of starter kit, but advices are welcomed!

[hluedeke]

Ok, I'll play around with it and see what I can do. It might be a good thing to add for the sake of completeness (or at least documentation talking about how to implement it). As I research, I'll try and post relevant libraries/methods here.

[langpavel]

@hluedeke Can you share something with us?

[hluedeke]

I have been pulled away to another part of the project for the time being, but it's on my todo list! As soon as I have something I will post it. I plan on looking into csurf as a starting point.

[hluedeke]

I'm finally getting a chance to sit down and take a look at this. I've downloaded csurf and started to implement it using the Redux state, and I found something very interesting. As I was monitoring network requests to graphql, I noticed that there is a _csrf cookie already being sent (in addition to csurf). I wonder what is setting this and if it's being monitored or verified anywhere.

Anyone know what is setting this cookie? I would love to move on to other things if there is already something handling csrf.

[langpavel]

@hluedeke This cookie can be set from different app you are run on localhost in past.. please clear cookies, then make new request and check if you see cookie again ;-)

[hluedeke]

@langpavel Well golly gee, that was it. I clear "locally stored data" all the time in Chrome, but doing a full wipe cleared the cookie. So thanks! I will continue with original plans and post the code (or at least the method I used) when finished.

[hluedeke]

Implementing csurf has not been difficult, save for one thing: sending requests via graphiql.

At the moment, my implementation is as follows (using Redux):

install and import csurf into server.js file:
import csrf from 'csurf';

Then I add it to server.js in front of graphql:

//
// Register CSRF Middleware
// -----------------------------------------------------------------------------
app.use(csrf({ cookie: true })); // <-- this goes before graphql

//
// Register API middleware
// -----------------------------------------------------------------------------
app.use('/graphql' ...

Add it to the Redux store:

const store = configureStore({
      user: req.user ?
      {
        ...req.user,
        csrf: req.csrfToken ? req.csrfToken() : null, // CSRF
      } : null,
    }, {
      cookie: req.headers.cookie,
});

Then all fetch instances include a csrf header:

return fetch('/graphql',
      {
        method: 'post',
        headers: {
          Accept: 'application/json',
          'Content-Type': 'application/json',
          'csrf-token': getState().user.csrf
        }
...

This works very nicely as is in the application. The only roadblock is now graphiql always returns an "invalid csrf token" error since it does not have state/does not send the correct headers. The next step is to figure out how to get around this.

[hluedeke]

UPDATE: I found some info on the csurf site that made me question whether CSRF was even necessary for our project.

I ran a little test to see how vulnerable we were in the present state. I created a form with an action to our exposed site from my local box (aka different domains) with a valid graphql query. When I posted the form, I received a valid JSON response that let me know I had just updated values from my exposed site. Be forewarned, graphql is not locked down by default!! It is quite vulnerable to CSRF attacks!!

K, that was all. Just justifying my dive into this subject :)

[langpavel]

@hluedeke

• At first, You should understand and setup CORS.
• Next you can set CSRF token on each request to your domain using httponly cookie.
• At second, you can use samesite cookie (very nice article) for CSRF token.
• If you use creadentials: 'include' (or samesite) on your fetch request, cookie will be send. Then you can remove CSRF token from client side entirely
• Next, you can check Referer for old browsers.

[hluedeke]

@langpavel I really like this approach. It is much cleaner and solves all issues mentioned above. Thank you

[hluedeke]

Just wanted to share my code changes for anyone else interested in implementing this:

In server.js, just before graphql code

app.use(csrf({ cookie: true, value: (req) => (req.cookies.csrfToken) }));

In app.get() logic

app.get('*', async (req, res, next) => {
  try {
    res.cookie('csrfToken', req.csrfToken ? req.csrfToken() : null, { sameSite: true, httpOnly: true });
...

[timtutt]

@hluedeke - thanks for sharing this. This worked perfectly in my project.

[langpavel]

Any idea why we should NOT include something like this in master branch?

[hluedeke]

I did have some trouble with it for server-side fetching, but that was back when I first cloned the project several months ago. A lot has changed since then, including the fetching before routes.

[langpavel]

@hluedeke Yeah, we are trying keep things up to date so there are changes..
I'm maintaining my branches here and one more project and I must say that I have no big issues when merging to private project.
There are some changes which need more attention than others, but all is moving to be better every week.
When you merge continuously, you give updated packages from us, so next time, try go with real fork, git is great for this :-)

But question is if we should introduce CSRF token to master branch because I believe it is wanted feature. On the other side, situation will look better with same-site cookies supported only in modern browsers of course..

[hluedeke]

I don't really see a reason to not include. I agree that it's a good security measure to include in the project.

I suppose I learn new things everyday - it never occurred to me to continuously merge. It's probably a little late for that at this point, but I will definitely remember for the future!

[iDVB]

@hluedeke Having a hard time getting this working.
I followed your steps above and it seems to work for all requests except for the first one, which errors out with.... Error: Forbidden
Which I believe is the error that csurf is throwing when the token is not there? But it won't be there until the site runs at least once, no?

Also, are you not supposed to limit this to pages that submit forms? All the csurf examples are from pages that post.