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

Ill-timed Rails ActionCable requests can prevent login_as from working in tests #182

Open
agraves opened this issue Jan 30, 2020 · 2 comments

Comments

@agraves
Copy link

agraves commented Jan 30, 2020

I recognize that this issue isn't purely a Warden issue -- there are Rails and Devise interactions. That said, this was hideously difficult to track down and I think Warden is best positioned to help users. I also have a fix in mind but wanted to discuss first.

Repro

Essentially, Rails ActionCable requests can exhibit exactly the same problem you guys ran into with #45 . My repro case looks like this:

  1. Write Capybara / Selenium test that loads a React app with a web browser
  2. login_as create(:user), scope: :user
  3. Unfortunately, an ActionCable connection triggers a call to /cable. This could be caused by anything from logging in in the middle of a test, system load, test pollution, phase of the moon, etc.
  4. My ActionCable setup looks at env['warden'].user, consuming the one-time data generated by login_as
  5. The next line of my test does something basic like visit '/my_profile and is, maddeningly, not logged in.

Pseudocode

So just to be clear, the test looks like this:

login_as create(:user), scope: :user
visit '/my_profile'

But because I'm using a headless browser to run a react app, sometimes an ActionCable request hits the db before the request for /my_profile. This is hideously difficult to debug because I doubt many users have any idea how login_as works under the hood. Especially because of the way ActionCable works and acquires sessions, it tends to be very intermittent. Even with the same db seed, I'd see 1-2 tests fail every 3-4 runs on a full suite.

Fixes

  1. We can just accept that some users are gonna get screwed by this and hope that they eventually figure out that they need to do what I did, which is just to call:

Warden::Test::WardenHelpers.asset_paths=%r(/(assets/|cable)}

in their test setup. We can add something big and angry to the docs.

  1. We could just adjust the default asset_paths to match /cable. It's tricky because rails doesn't stop you from mounting ActionCable on another path, but it would probably save most users from running into this issue. There's also precedent for it.

  2. We could be smarter about how Warden behaves. Specifically, we could try to figure out when we're inside ActionCable and prevent ourselves from consuming the on_next_request trigger. I don't know how this would be done and it might risk coupling Warden and Rails. If there's a clean way to do this, that seems like it would be best -- the fundamental problem is that on_next_request is silently failing to do its real job, which is applying the cookie -- but I don't know how we would go about it.

Conclusion

Anyways, let me know what you think. Happy to help you repro it or to try implementing one of the fixes.

@agraves agraves changed the title ill-timed Rails ActionCable requests consume Ill-timed Rails ActionCable requests can prevent login_as from working in tests Jan 30, 2020
@agraves
Copy link
Author

agraves commented Jan 31, 2020

So, doing a little more research on this. Looks like my app/channels/application_cable/connection.rb was like this:

+    def find_verified_user
+      if env['warden'].user
+        env['warden'].user
+      else
+        reject_unauthorized_connection
+      end
+    end

And doing a bit more research, it looks like the recommendation is to do something like this:

      def find_verified_user
        if verified_user = User.find_by(id: cookies.encrypted[:user_id])
          verified_user
        else
          reject_unauthorized_connection
        end
      end

I guess my question now is -- would any invocation of env['warden'].user consume whatever state login_as puts it in? i.e. is this a "just don't screw with env['warden'].userdirectly" issue? Or is there something about theActionCable` request (like the asset requests?) that predisposes it to eat these cookies?

@jsmestad
Copy link
Collaborator

@agraves yeah it sounds difficult to track down. How would you suppose we approach options 2 or 3 without adding additional dependencies from Rails?

If you have any ideas, we'd be open to a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants