Check a cable connection's session while it's open (#376)

* Check a cable connection's session while it's open

Signing out or changing the password ends sessions and then disconnects
their connections. A connection that found its session just before isn't
subscribed to its disconnects yet, so it missed that and stayed open.

Each connection now looks its session up every minute while it's open, and
closes once the session is gone, asking the browser to reconnect so one
holding a current session comes back. That also covers any other way a
connection misses the disconnect, such as a subscription that takes long to
register, and a lookup that fails is simply tried again.

* Check the session on the server's heartbeat rather than a timer of its own

A connection's own timer could outlive it: closing can run before opening finishes, or a channel's teardown can fail before the connection's. The heartbeat only reaches connections that are open, so nothing is left to stop.

* Skip the session check on a rejected connection

A rejected connection stays registered until its socket closes, and has no session to check, so its heartbeat mustn't fail and stop the heartbeat reaching the connections after it.

app/channels/application_cable/connection.rb

@@ -2,19 +2,42 @@ module ApplicationCable
   class Connection < ActionCable::Connection::Base
     include Authentication::SessionLookup
 
+    SESSION_CHECK_INTERVAL = 1.minute
+
     identified_by :current_user
 
     def connect
       self.current_user = find_verified_user
+      @session_checked_at = Time.now
+    end
+
+    # Signing out or changing the password ends sessions and then disconnects their connections, but a connection can
+    # miss that: one that found its session just before isn't listening yet. So on the server's heartbeat, which
+    # reaches every open connection, each one looks its session up again now and then, and closes once it's gone. A
+    # rejected connection has no session, and stays registered until its socket closes.
+    def beat
+      super
+      check_session if @session_checked_at&.before?(SESSION_CHECK_INTERVAL.ago)
     end
 
     private
       def find_verified_user
-        if verified_session = find_session_by_cookie
-          verified_session.user
+        if @verified_session = find_session_by_cookie
+          @verified_session.user
         else
           reject_unauthorized_connection
         end
       end
+
+      def check_session
+        @session_checked_at = Time.now
+        perform_work -> { close_unless_session_remains }, :call
+      end
+
+      def close_unless_session_remains
+        unless Session.exists?(@verified_session.id)
+          close reason: ActionCable::INTERNAL[:disconnect_reasons][:unauthorized]
+        end
+      end
   end
 end

test/channels/application_cable/connection_test.rb

@@ -18,4 +18,53 @@ class ApplicationCable::ConnectionTest < ActionCable::Connection::TestCase
 
     assert_reject_connection { connect }
   end
+
+  test "closes once its session has ended without it hearing" do
+    cookies.signed[:session_token] = sessions(:david_safari).token
+    connect
+    sessions(:david_safari).destroy
+
+    travel 2.minutes
+    connection.beat
+
+    assert socket.closed
+    assert_equal "disconnect", socket.transmissions.last[:type]
+    assert socket.transmissions.last[:reconnect]
+  end
+
+  test "keeps checking its session while open" do
+    cookies.signed[:session_token] = sessions(:david_safari).token
+    connect
+
+    travel 2.minutes
+    connection.beat
+    assert_not socket.closed
+
+    sessions(:david_safari).destroy
+    travel 2.minutes
+    connection.beat
+    assert socket.closed
+  end
+
+  test "checks its session at most once a minute" do
+    cookies.signed[:session_token] = sessions(:david_safari).token
+    connect
+    sessions(:david_safari).destroy
+
+    travel 30.seconds
+    connection.beat
+
+    assert_not socket.closed
+  end
+
+  test "a rejected connection's heartbeat doesn't fail" do
+    request = ActionCable::Connection::TestSocket.build_request(ActionCable.server.config.mount_path || "/cable", cookies: cookies)
+    rejected = ApplicationCable::Connection.new \
+      ActionCable::Connection::TestServer.new(ActionCable.server), ActionCable::Connection::TestSocket.new(request)
+    rejected.handle_open
+
+    travel 2.minutes
+
+    assert_nothing_raised { rejected.beat }
+  end
 end