From ee378092206e1b203c780b55d75b47f5f4a9334a Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 3 Aug 2026 14:55:12 -0700 Subject: [PATCH] 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. --- .../concerns/room_streams_are_authorized.rb | 13 +++ app/channels/room_messages_channel.rb | 55 ++++++++++++ app/views/rooms/show.html.erb | 2 +- .../turbo_streams_authorization.rb | 3 + test/channels/room_messages_channel_test.rb | 86 +++++++++++++++++++ 5 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 app/channels/concerns/room_streams_are_authorized.rb create mode 100644 app/channels/room_messages_channel.rb create mode 100644 config/initializers/turbo_streams_authorization.rb create mode 100644 test/channels/room_messages_channel_test.rb diff --git a/app/channels/concerns/room_streams_are_authorized.rb b/app/channels/concerns/room_streams_are_authorized.rb new file mode 100644 index 0000000..ad45270 --- /dev/null +++ b/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 diff --git a/app/channels/room_messages_channel.rb b/app/channels/room_messages_channel.rb new file mode 100644 index 0000000..1575a4f --- /dev/null +++ b/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 diff --git a/app/views/rooms/show.html.erb b/app/views/rooms/show.html.erb index 8f6bd07..ef76ecd 100644 --- a/app/views/rooms/show.html.erb +++ b/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 %> diff --git a/config/initializers/turbo_streams_authorization.rb b/config/initializers/turbo_streams_authorization.rb new file mode 100644 index 0000000..9837ea6 --- /dev/null +++ b/config/initializers/turbo_streams_authorization.rb @@ -0,0 +1,3 @@ +Rails.application.config.to_prepare do + Turbo::StreamsChannel.prepend RoomStreamsAreAuthorized +end diff --git a/test/channels/room_messages_channel_test.rb b/test/channels/room_messages_channel_test.rb new file mode 100644 index 0000000..9c2d112 --- /dev/null +++ b/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