Reject Reflexes with the wrong connection identifier

[leastbad]

Type of PR (feature, enhancement, bug fix, etc.)

Failed Experiment

Description

This PR should not be merged in its current form. My goal is to present the problem, what I attempted to do to fix it, and then what actually worked after a few days of struggling.

@RolandStuder and I have been talking about how to handle a wide variety of edge cases that occur when you have multiple tabs or devices open and using an authenticated app. The most obvious concern is the case where a user logs out of one tab but can still use another tab to execute Reflex actions with the original authenticated context. In fairness, this is a major issue with non-WS apps as well, but that's beyond the scope of this PR.

My first idea was to modify SR so that when the ActionCable Connection is first established, SR would send the connection.connection_identifier to the client. It would then be sent to the server as part of the standard payload so that channel.rb could compare it against the connection.connection_identifier. If you're faster than I was, you'll already spot the issue: if SR can receive the message, the connection identifier is going to match. Whoops! So this is a PR instead of an Issue because I suspect that this code could be helpful when development on v4 starts in earnest.

---

There was a number of things I looked into to address this, and the first was Warden's callbacks: https://github.com/wardencommunity/warden/wiki/Callbacks

Warden has a before_logout callback which could be very helpful if someone wanted to go through all of the active connections in ActionCable.server.remote_connections and nuke all connections for a given user on the server side.

---

Another thing that I discovered is that ActionCable's connection_identifier is not always in the same order (which is weird!). It appears to randomize the order of identifiers.

Also: the to_gid_param method is great:

User.first.to_gid_param
=> "Z2lkOi8vcHJvZ2VuaXRvci9Vc2VyLzE"

---

Solution

If you're ever using a script like this:

import CableReady from 'cable_ready'
import consumer from './consumer'

let reconnecting = false

consumer.subscriptions.create('SessionChannel', {
  received (data) {
    if (data.cableReady)
      CableReady.perform(data.operations, {
        emitMissingElementWarnings: false
      })
  },

  connected () {
    reconnecting = false
    document.addEventListener('reconnect', this.reconnect)
  },

  disconnected () {
    document.removeEventListener('reconnect', this.reconnect)
    if (reconnecting) consumer.connect()
  },

  reconnect () {
    reconnecting = true
    consumer.disconnect()
  }
})

so that you can force users to reconnect after they log in or log out like this:

class Users::SessionsController < Devise::SessionsController
  def create
    super do
      cable_ready[SessionChannel].dispatch_event(name: "reconnect").broadcast_to(request.session.id)
    end
  end
  def destroy
    super do
      cable_ready[SessionChannel].dispatch_event(name: "reconnect").broadcast_to(request.session.id)
    end
  end
end

... you're going to find that env["warden"].user gets out-of-sync with the user that just logged in. The solution is to force Warden to check the authentication status before assigning your connection identifiers:

module ApplicationCable
  class Connection < ActionCable::Connection::Base
    identified_by :session_id, :current_user

    def connect
      env["warden"].authenticated?
      self.current_user = env["warden"].user
      self.session_id = request.session.id
      reject_unauthorized_connection unless current_user || session_id
    end
  end
end

In the end, I found a solution that makes a lot of sense and gets implemented on the application side without any changes to SR:

class ApplicationReflex < StimulusReflex::Reflex
  class IdentityError < RuntimeError; end
  delegate :current_user, :session_id, to: :connection

  before_reflex do
    raise IdentityError.new("invalid user") if connection.env["warden"].user && invalid_user?
  end

  private

  def invalid_user?
    connection.connection_identifier.exclude?(connection.env["warden"].user.to_gid_param) || connection.connection_identifier.exclude?(request.session.id)
  end
end

It should work just fine in SR 3.5! So I'll figure out the best way to convey these ideas in the docs.

Checklist

• [x] My code follows the style guidelines of this project
• [x] Checks (StandardRB & Prettier-Standard) are passing
• [x] This is not a documentation update