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

sign_in IntegrationHelper produces errors when trying to load a session #4696

Closed
RocketPuppy opened this issue Nov 7, 2017 · 9 comments
Closed

Comments

@RocketPuppy
Copy link

RocketPuppy commented Nov 7, 2017

Rails 5 API mode, devise cookie auth works just fine in the actual application, meaning all middleware and initializers are set up properly.

I have an ApiController that looks like this:

class ApiController < ActionController::API
end

And all my controllers inherit from it. I have multiple user scopes. The integration test helpers are set up according to the documentation:

# test/test_helper.rb

class ActionDispatch::IntegrationTest
  include Devise::Test::IntegrationHelpers
end

In a test:

class FooControllerTest < ActionDispatch::IntegrationTest
  test "should be successful" do
    sign_in CustomUser.first
    get foo_url, xhr: true, as: :json

    assert(@response.successful?) # fail
  end
end

In the response body I get this error message, implying things are failing even before they get to my controllers.

> undefined method `[]=' for nil:NilClass

actionpack (5.1.4) lib/action_dispatch/request/session.rb, line 217
-------------------------------------------------------------------


  212             load! unless loaded?
  213           end
  214
  215           def load!
  216             id, session = @by.load_session @req
> 217             options[:id] = id
  218             @delegate.replace(stringify_keys(session))
  219             @loaded = true
  220           end
  221
  222           def stringify_keys(other)


App backtrace
-------------

 - actionpack (5.1.4) lib/action_dispatch/request/session.rb:217:in `load!'
 - actionpack (5.1.4) lib/action_dispatch/request/session.rb:212:in `load_for_write!'
 - actionpack (5.1.4) lib/action_dispatch/request/session.rb:197:in `merge!'
 - actionpack (5.1.4) lib/action_dispatch/request/session.rb:17:in `create'
 - actionpack (5.1.4) lib/action_dispatch/middleware/session/abstract_store.rb:71:in `prepare_session'
 - rack (2.0.3) lib/rack/session/abstract/id.rb:231:in `context'
 - rack (2.0.3) lib/rack/session/abstract/id.rb:226:in `call'
 - actionpack (5.1.4) lib/action_dispatch/middleware/cookies.rb:613:in `call'
 - warden (1.2.7) lib/warden/manager.rb:36:in `block in call'
 - warden (1.2.7) lib/warden/manager.rb:35:in `catch'
 - warden (1.2.7) lib/warden/manager.rb:35:in `call'
 - rack (2.0.3) lib/rack/etag.rb:25:in `call'
 - rack (2.0.3) lib/rack/conditional_get.rb:25:in `call'
 - rack (2.0.3) lib/rack/head.rb:12:in `call'
 - actionpack (5.1.4) lib/action_dispatch/middleware/callbacks.rb:26:in `block in call'
 - activesupport (5.1.4) lib/active_support/callbacks.rb:97:in `run_callbacks'
 - actionpack (5.1.4) lib/action_dispatch/middleware/callbacks.rb:24:in `call'
 - better_errors (2.3.0) lib/better_errors/middleware.rb:84:in `protected_app_call'
 - better_errors (2.3.0) lib/better_errors/middleware.rb:79:in `better_errors_call'
 - better_errors (2.3.0) lib/better_errors/middleware.rb:57:in `call'
 - actionpack (5.1.4) lib/action_dispatch/middleware/debug_exceptions.rb:59:in `call'
 - actionpack (5.1.4) lib/action_dispatch/middleware/show_exceptions.rb:31:in `call'
 - railties (5.1.4) lib/rails/rack/logger.rb:36:in `call_app'
 - railties (5.1.4) lib/rails/rack/logger.rb:24:in `block in call'
 - activesupport (5.1.4) lib/active_support/tagged_logging.rb:69:in `block in tagged'
 - activesupport (5.1.4) lib/active_support/tagged_logging.rb:26:in `tagged'
 - activesupport (5.1.4) lib/active_support/tagged_logging.rb:69:in `tagged'
 - railties (5.1.4) lib/rails/rack/logger.rb:24:in `call'
 - actionpack (5.1.4) lib/action_dispatch/middleware/remote_ip.rb:79:in `call'
 - actionpack (5.1.4) lib/action_dispatch/middleware/request_id.rb:25:in `call'
 - rack (2.0.3) lib/rack/runtime.rb:22:in `call'
 - activesupport (5.1.4) lib/active_support/cache/strategy/local_cache_middleware.rb:27:in `call'
 - actionpack (5.1.4) lib/action_dispatch/middleware/executor.rb:12:in `call'
 - actionpack (5.1.4) lib/action_dispatch/middleware/static.rb:125:in `call'
 - rack (2.0.3) lib/rack/sendfile.rb:111:in `call'
 - rack-cors (1.0.2) lib/rack/cors.rb:97:in `call'
 - railties (5.1.4) lib/rails/engine.rb:522:in `call'
 - rack-test (0.7.0) lib/rack/mock_session.rb:30:in `request'
 - rack-test (0.7.0) lib/rack/test.rb:249:in `process_request'
 - rack-test (0.7.0) lib/rack/test.rb:125:in `request'
 - actionpack (5.1.4) lib/action_dispatch/testing/integration.rb:261:in `process'
 - actionpack (5.1.4) lib/action_dispatch/testing/integration.rb:16:in `get'
 - actionpack (5.1.4) lib/action_dispatch/testing/integration.rb:348:in `block (2 levels) in <module:Runner>'

I did some inspection of some of that code and there are a few things that seemed out of place to me.

  1. ActionDispatch::Request:Session.create includes a previous session.
def self.create(store, req, default_options)
  session_was = find req
  session     = Request::Session.new(store, req)
  session.merge! session_was if session_was

  set(req, session)
  Options.set(req, Request::Session::Options.new(store, default_options))
  session
end

If I break here I see that session_was is set, and it is attempted to merge! into the new session. This is where load! is eventually called on the new session. As an experiment I tried forcing session_was to nil. This caused the test to succeed! I imagine because it didn't try to merge in the old session, but was the old session even necessary?

  1. The newly created session in point 1 fails at load! because it doesn't have a rack.session.options header. I'm not sure why this is, but it is added in the Options.set line.

  2. If I remove the sign_in call in the test, I get to my controller code successfully.

On the whole this feels like some sort of user error on my part, but I can't seem to figure out just what the problem might be.

@tegon
Copy link
Member

tegon commented Dec 13, 2017

Hello @RocketPuppy, thanks for your report.
Indeed, the code where you're getting the error is not even from devise, but from action_dispatch.
I recommend that you post this question on StackOverflow. There a wider community will be able to help you. We try to keep only issues here.

Thank you!

@tegon tegon closed this as completed Dec 13, 2017
@mrstif
Copy link

mrstif commented Mar 15, 2018

@RocketPuppy did you end up figuring out what this was?

I backtracked through the code and realised that this is happening since Warden::Manager places a previous session in the env... though I cannot understand why:

env[Rack::RACK_SESSION]
{"warden.user.user.session"=>{"last_request_at"=>1521141461}}

this actually doesn't happen in normal Rails mode...

@RocketPuppy
Copy link
Author

I did not. I hacked my way around it by writing a test helper that intercepts the rails API requests.

@mrstif
Copy link

mrstif commented Mar 26, 2018

@RocketPuppy I seem to have found the reason why this to be happening.

Apparently, in rails-api mode, the ActionDispatch::Cookies and ActionDispatch::Session::CookieStore middlewares are inserted in the end of the middleware stack, which doesn't occur in normal Rails mode.

Due to this, those middlewares are included after Warden::Manager which messes up something in request specs...

Therefore, this solved it:

Rails.application.config.middleware.insert_before Warden::Manager, ActionDispatch::Cookies
Rails.application.config.middleware.insert_before Warden::Manager, ActionDispatch::Session::CookieStore

Hope it helps

@tegon
Copy link
Member

tegon commented Mar 28, 2018

Yeah, warden needs to run after a session middleware in order to work - which I guess it doesn't make much sense in an API application since they usually don't keep a session.
But anyway, thanks for digging into this!

@emersonthis
Copy link
Contributor

I recently chased my tail for a while trying to figure this out. I am using Devise with Rails in api mode. In my case, the API is powering a public single page app, so I'm using a standard session authentication instead of tokens.

I wonder if the gem could somehow incorporate the fix above so that it works smoothly in API mode. Or if that's undesirable, how would you feel about a PR to mention this "gotcha" in the README?

@tegon
Copy link
Member

tegon commented Mar 13, 2019

I'm not sure if Devise as an engine should assume an application's middleware chain structure in order to achieve this. Of course, we can know for how a stock Rails application looks like, but we can't know how other applications look like - i.e. they will have custom middlewares many times.
About adding this in the README, I think it's great. I'm thinking in a new section that includes instructions or limitations on API mode so that we can later add other known ones. WDYT?

@emersonthis
Copy link
Contributor

emersonthis commented Mar 13, 2019 via email

@tegon
Copy link
Member

tegon commented Mar 13, 2019

Yeah, that would be great! Thanks!

KyleRAnderson added a commit to KyleRAnderson/console-backend that referenced this issue Jan 29, 2021
I found an entry in the Devise README which gave notice of exactly this
error happening when the Warden middleware is initialized before the
session middleware.
The solution was to explicitly define the proper order in the test
environment.
See https://github.com/heartcombo/devise#testing and
heartcombo/devise#4696.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants