Rails 4: Session can not be set after invalid CSRF token

[pierre-pretorius]

Dear nbudin

When logging in with openid you will see the following warning when a successful openid request is processed:

WARNING: Can't verify CSRF token authenticity

In Rails 3 it doesn't really matter because you override the store? method to always return true which means the session is recreated after it is destroyed by the handle_unverified_request logic. To refresh your memory, look at #19. Note that before you overrided store? you sent the csrf token through in the return_to parameters as discussed in #17 which won't work anymore as discussed #19.

The problem is in Rails 4 (actionpack 4) when a unverified request is detected it completely removes the ability to write anything to the session:

def handle_unverified_request
  request = @controller.request
  request.session = NullSessionHash.new(request.env)
  request.env['action_dispatch.request.flash_hash'] = nil
  request.env['rack.session.options'] = { skip: true }
  request.env['action_dispatch.cookies'] = NullCookieJar.build(request)
end

You can view it on line 110 in https://github.com/rails/rails/blob/4-0-stable/actionpack/lib/action_controller/metal/request_forgery_protection.rb

This means that even though the store method of the warden session_serializer is called, it doesn't recreate the session cookie which doesn't cause a successful login.

Do you know of anyone using this gem successfully with Rails 4? If my investigation is correct and you can not use this gem with Rails 4 I will start investigating how to fix this problem.

Regards, Pierre

[nbudin]

I think you are probably correct. The only thing I could recommend at this point would be to create a subclass of Devise::SessionsController that skips the CSRF check and use that as your sessions controller.

If that works, I can ship a subclass of SessionsController along with the gem and instruct people to use it.

[pierre-pretorius]

Hi @nbudin

The rails documentation advise disabling the CSRF token for certain actions/request types with skip_before_action. The patch below makes your gem work with Rails 4 but I'm unsure if this is the best way of doing it. Do you see any problems with it?

ApplicationController.class_eval do
  skip_before_action :verify_authenticity_token, if: :openid_provider_response?

  def openid_provider_response?
    !!env[Rack::OpenID::RESPONSE]
  end
end

[pierre-pretorius]

Note that I could not apply this patch to only the session controller because when the openid provider response comes in and only the session controller is patched, the following steps occur:

Started GET "/users/sign_in?...
...
Authenticating with OpenID for mapping User
...
Processing by ContextController#root as HTML
Can't verify CSRF token authenticity
...

Even though the path /users/sign_in maps to the devise session controller, when authentication is successful it may process any other controllers action (in our case context#root) as part of the same request.

We specify this landing page (context#root) with the following in our routes file. Logged in users will never see the login page when they visit / or /users/sign_in:

authenticated do
  match 'users/sign_in' => 'context#root', via: [:get, :post, :patch, :delete]
  root :to => 'context#root', as: :context_root
end

[nbudin]

That seems like a fairly good way to do it. I'm going to have to look into Rack::OpenID::RESPONSE a little more just to make sure we're not opening up users to CSRF attacks by accident, but this seems like a very promising approach. Thank you!

[nbudin]

Well, as far as I can tell this ought to be safe. The only way that env key should be settable is by something higher on the Rack stack passing a 401 down with a special rack-openid header. I don't see any way a malicious user could cause that to happen from a different controller, and even if it did it'd prevent a page render. (Famous last words, right?)

Anyway, I'll look into how to make this patch easy to install. Probably by mixing that method into ApplicationController and recommending that people write in the skip_before_action line themselves.

[pierre-pretorius]

Hi @nbudin

Thank you for the commit and release of version 1.1.5. Unfortunately the skip_before_filter you added in b276a39 doesn't trigger in my Rails 4 app. I can see that the openid_provider_response? method is available in all controllers and that the skip_before_filter is added to ActionController::Base, but it looks like that doesn't skip the protect_from_forgery in added to ApplicationController. I can confirm that the openid_provider_response? method is never called. With 1.1.5 I could now reduce the patch required to fix our app to:

ApplicationController.class_eval do
  skip_before_filter :verify_authenticity_token, if: :openid_provider_response?
end

Alternatively modifying the protect_from_forgery in ApplicationController also fixes the problem:

  protect_from_forgery :unless => :openid_provider_response?

I think the same problem exists in rails 3 as well, you can perhaps place a break point or raise statement inside the openid_provider_response? method to ensure that it is called with your current solution. Unfortunately I do not have any other suggestion except for the two listed above.

[nbudin]

Hi! Sorry about the problems. I have another idea, which I'm going to push code for shortly. Would you mind reviewing the patch once it's up?

[nbudin]

I've just tested the fix myself and it looks like it works, at least in my minimal test app. Going to release 1.1.6 for this fix.

[pierre-pretorius]

Thank you @nbudin this issue is resolved in 1.1.6.