search

BetterErrors / better_errors

Better error page for Rack apps
MIT License
6.88k stars 437 forks source link

Invalid CSRF Token error in Better Errors console #476

Closed MrJoy closed 4 years ago

MrJoy commented 4 years ago

I'm running Rails 6.0.3.3. I'm using TurboLinks 5.2.0, StimulusJS 1.1.1, and rails-ujs 6.0.3 via Webpacker 5.2.1, and not directly using Sprockets (except for ActiveAdmin). Browser is Safari 13.1.2 on macOS 10.15.6.

My layout includes output of csrf_meta_tags in the <head> section, and my form includes the authenticity_token hidden field.

When I attempt to trigger BetterErrors in a controller action (update), it comes up enough that I see a stack frame, but I don't see the source view / REPL / request info / locals / instance vars on the right. I only see an error about "Invalid CSRF Token". The /__better_errors endpoint is available and it works fine, however.

The problem does not occur with BetterErrors 2.7.1.

I'm not sure If I've got something subtly wrong, or if there's a bug (possibly related to TurboLinks, or the use of Webpacker).

What information can I provide to help diagnose this issue?

RobinDaugherty commented 4 years ago

It sounds like TurboLinks might be causing this. Would it be possible to remove it and see if that's correct?

Note that Better Errors' CSRF token is not related to Rails' csrf_meta_tags or verify_authenticity_token. It looks like we need to change our error message to be more clear about that.

aldavidson commented 4 years ago

We're seeing the same issue, and we are not using TurboLinks

RobinDaugherty commented 4 years ago

I agree, I just double-checked and I've been testing with Turbolinks enabled. Can you help me find a reproducible case? I'm linking to a controller action that fails and returns a Better Errors console.

Can you tell me what URL you're using, in other words how do you reach the Rails server?

It would also help to see the network request that is posting to /__better_errors/:id/variables, and see any JS errors in the console that are being emitted.

MrJoy commented 4 years ago

It's a POST to /reminder_rules/:id, where /reminder_rules, and the relevant portion from routes.rb is:

  authenticate :user do
    resources :reminder_rules, except: %i[show]
  end

I'll attach a .har of the POST, and subsequent call to /__better_errors/:id/variables (once I figure out how to do so on GH Issues). It looks like the cookie for BE's CSRF token is being set on the POST request, but is not being included in the request to the variables endpoint.

MrJoy commented 4 years ago

(File renamed from .har because Github is being prickly.)

localhost.txt

MrJoy commented 4 years ago

I can confirm that disabling Turbolinks does not help. (Removed it from Gemfile, and package.json, ran bundle and yarn install, removed the code to start it in my JS bundle, removed the options to stylesheet_pack_tag and javascript_pack_tag.)

MrJoy commented 4 years ago

I can also confirm that the issue occurs on Chrome 85.0.4183.102, and with GET requests (e.g. the /edit endpoint for the same resource).

MrJoy commented 4 years ago

I'll see about trying to make a simple repro case later tonight.

manuelmeurer commented 4 years ago

This happens for me with 2.8.1 as well: https://take.ms/CCXc8 https://take.ms/BpipT

chuckd commented 4 years ago

Seeing the same here. The BetterErrors-CSRF-Token cookie isn't being sent to the variables endpoint. I haven't worked out why yet. The token is being POSTed correctly in the body of the request, just the cookie not sent.

dramalho commented 4 years ago

Doing a +1 here - Rails 4.2 , the 2.8.x releases all throw the CSFR token on the right frame (chrome / safari)

Tioneb12 commented 4 years ago

I have the same issue with rails 6.0.3.2, ruby 2.6.6 and better-errors 2.8.1

chuckd commented 4 years ago

In my case the cookie Path is getting set to the path where the error happens, which means the cookie doesn't get sent with the /__better_errors... request. Can be fixed by adding path: "/" at lib/better_errors/middleware.rb:116.

        response.set_cookie(CSRF_TOKEN_COOKIE_NAME, value: csrf_token, httponly: true, path: "/", same_site: :strict)

I would make a PR but I don't know if this is 100% the right thing to do. I don't know much about Rack and I took a quick look but it wasn't super obvious.

alexanderadam commented 4 years ago

In my case the cookie Path is getting set to the path where the error happens, which means the cookie doesn't get sent with the /__better_errors... request. Can be fixed by adding path: "/" at lib/better_errors/middleware.rb:116

In my case this doesn't fix anything, since the cookie is already there at first and thus I'm not even reaching what's inside the block of unless request.cookies[BetterErrors::Middleware::CSRF_TOKEN_COOKIE_NAME].

But afterwards, when checking the same thing in the method internal_call, the cookie isn't available anymore in line 154‽

Commenting out the CSRF checks

return invalid_csrf_token_json_response unless request.cookies[BetterErrors::Middleware::CSRF_TOKEN_COOKIE_NAME] == body['csrfToken']

and

return invalid_csrf_token_json_response unless request.cookies[BetterErrors::Middleware::CSRF_TOKEN_COOKIE_NAME]

works, though.

I guess, the recently introduced CSRF check is still buggy in some cases.

PS: I'm not sure whether it makes any difference, but I'm using BetterErrors together with binding_of_caller

pollinoco commented 4 years ago

I have the same problem:

chuckd commented 4 years ago

In my case the cookie Path is getting set to the path where the error happens, which means the cookie doesn't get sent with the /__better_errors... request. Can be fixed by adding path: "/" at lib/better_errors/middleware.rb:116

In my case this doesn't fix anything, since the cookie is already there at first ...

Did you delete the existing cookie? It should then get re-set with the correct path.

alexanderadam commented 4 years ago

Did you delete the existing cookie? It should then get re-set with the correct path.

Yes I did and still had the same behaviour. But I still didn't get a cookie in the check of internal_call. Only skipping the CSRF check helped in my case.

RobinDaugherty commented 4 years ago

Thank you @chuckd that's exactly the issue! I was hoping someone would provide a reproducible case, but you identified the issue on the nose. I've got a fix in the works!

dramalho commented 4 years ago

Awesome Robin 👏

RobinDaugherty commented 4 years ago

2.8.2 should fix this issue. Please let me know here if you still see this issue!

Tioneb12 commented 4 years ago

On rubygems, the current version is always at 2.8.1 ;)