Authorize the room message stream at subscribe time

Message content is delivered over turbo streams, which ran on the stock
Turbo::StreamsChannel. That channel verifies the signature on the stream name and
nothing else. The name carries no expiry and no binding to a user, so one read off
the page while a member kept working after the membership was revoked.

Revocation made this worse rather than better. Membership#after_destroy_commit
disconnects the user with reconnect: true, and the client replays its subscriptions
on the new socket: RoomChannel re-checks membership and rejects, while the turbo
subscription re-verified only the signature and was accepted.

RoomMessagesChannel re-checks membership on every subscribe, deriving the room from
the verified stream name so there is no parameter to point elsewhere. Since the
subscriber names the channel it wants, the stock channel would otherwise be a way
around that check, so it now turns these stream names away and this is the only door.

app/channels/concerns/room_streams_are_authorized.rb

@@ -0,0 +1,13 @@
+# Prepended onto Turbo::StreamsChannel. The subscriber names the channel it wants, so
+# authorizing room messages only in RoomMessagesChannel would leave the stock channel as
+# a way around it: same signed stream name, no membership check. Turn those names away
+# here and RoomMessagesChannel becomes the only door.
+module RoomStreamsAreAuthorized
+  def subscribed
+    if RoomMessagesChannel.guarded_stream?(verified_stream_name_from_params)
+      reject
+    else
+      super
+    end
+  end
+end

app/channels/room_messages_channel.rb

@@ -0,0 +1,55 @@
+# Authorizes the room message stream when the subscription is made, so that revoking a
+# membership actually stops delivery.
+#
+# Turbo's stock channel verifies only the signature on the stream name. That name carries
+# no expiry and no binding to a user, so one harvested while a member keeps working after
+# the membership is gone. Reconnecting makes it worse rather than better: revoking a
+# membership disconnects the user with reconnect: true, and the client then replays its
+# subscriptions on the fresh socket.
+#
+# The room is derived from the verified stream name rather than taken as a parameter, so
+# there's nothing for a subscriber to point somewhere else. The subscriber also doesn't
+# get to choose the channel: Turbo::StreamsChannel turns these stream names away, so this
+# is the only way onto them. See config/initializers/turbo_streams_authorization.rb.
+class RoomMessagesChannel < ApplicationCable::Channel
+  extend  Turbo::Streams::StreamName
+  include Turbo::Streams::StreamName::ClassMethods
+
+  STREAM_SUFFIX = "messages"
+
+  class << self
+    # True for the stream names this channel exists to guard, whoever is asking.
+    def guarded_stream?(stream_name)
+      stream_name.to_s.split(":", 2).second == STREAM_SUFFIX
+    end
+
+    def subscribable_room(user, stream_name)
+      gid_param, suffix = stream_name.to_s.split(":", 2)
+
+      if suffix == STREAM_SUFFIX && room = room_from(gid_param)
+        user.rooms.find_by(id: room.id)
+      end
+    end
+
+    private
+      def room_from(gid_param)
+        GlobalID::Locator.locate gid_param, only: Room
+      rescue ActiveRecord::RecordNotFound
+        nil
+      end
+  end
+
+  def subscribed
+    if stream_name = authorized_stream_name
+      stream_from stream_name
+    else
+      reject
+    end
+  end
+
+  private
+    def authorized_stream_name
+      stream_name = verified_stream_name_from_params
+      stream_name if stream_name.present? && self.class.subscribable_room(current_user, stream_name)
+    end
+end

app/views/rooms/show.html.erb

@@ -17,7 +17,7 @@
     <%= render partial: "messages/message", collection: @messages, cached: true %>
   <% end %>
 
-  <%= turbo_stream_from @room, :messages %>
+  <%= turbo_stream_from @room, :messages, channel: "RoomMessagesChannel" %>
   <%= button_to_jump_to_newest_message %>
 <% end %>
 

config/initializers/turbo_streams_authorization.rb

@@ -0,0 +1,3 @@
+Rails.application.config.to_prepare do
+  Turbo::StreamsChannel.prepend RoomStreamsAreAuthorized
+end

test/channels/room_messages_channel_test.rb

@@ -0,0 +1,86 @@
+require "test_helper"
+
+class RoomMessagesChannelTest < ActionCable::Channel::TestCase
+  tests RoomMessagesChannel
+
+  setup do
+    @room = rooms(:designers)
+    @signed_stream_name = Turbo::StreamsChannel.signed_stream_name [ @room, :messages ]
+  end
+
+  test "a member may subscribe to a room's message stream" do
+    stub_connection(current_user: users(:kevin))
+
+    subscribe signed_stream_name: @signed_stream_name
+
+    assert subscription.confirmed?
+    assert_has_stream Turbo.signed_stream_verifier.verified(@signed_stream_name)
+  end
+
+  test "a user who was never a member may not subscribe" do
+    stub_connection(current_user: users(:bender))
+
+    subscribe signed_stream_name: @signed_stream_name
+
+    assert subscription.rejected?
+  end
+
+  test "a revoked member may not re-subscribe with a stream name harvested while a member" do
+    stub_connection(current_user: users(:kevin))
+
+    subscribe signed_stream_name: @signed_stream_name
+    assert subscription.confirmed?, "kevin must start out able to subscribe"
+
+    @room.memberships.revoke_from users(:kevin)
+
+    subscribe signed_stream_name: @signed_stream_name
+
+    assert subscription.rejected?
+  end
+
+  test "an unsigned stream name is rejected" do
+    stub_connection(current_user: users(:kevin))
+
+    subscribe signed_stream_name: Turbo.signed_stream_verifier.verified(@signed_stream_name)
+
+    assert subscription.rejected?
+  end
+
+  test "a missing stream name is rejected" do
+    stub_connection(current_user: users(:kevin))
+
+    subscribe
+
+    assert subscription.rejected?
+  end
+
+  test "a validly signed stream name for another room the user isn't in is rejected" do
+    stub_connection(current_user: users(:bender))
+
+    subscribe signed_stream_name: Turbo::StreamsChannel.signed_stream_name([ rooms(:hq), :messages ])
+
+    assert subscription.rejected?
+  end
+end
+
+class RoomMessagesViaStockTurboChannelTest < ActionCable::Channel::TestCase
+  tests Turbo::StreamsChannel
+
+  # The subscriber picks the channel, so the stock channel has to turn these names away
+  # too. Otherwise a revoked member just names Turbo::StreamsChannel instead.
+  test "the stock turbo channel refuses to serve a room message stream" do
+    stub_connection(current_user: users(:kevin))
+
+    subscribe signed_stream_name: Turbo::StreamsChannel.signed_stream_name([ rooms(:designers), :messages ])
+
+    assert subscription.rejected?
+  end
+
+  test "the stock turbo channel still serves the room list stream" do
+    stub_connection(current_user: users(:kevin))
+
+    subscribe signed_stream_name: Turbo::StreamsChannel.signed_stream_name([ :rooms ])
+
+    assert subscription.confirmed?
+  end
+end