From 2f97c0357c3f680280abe74827d702a97b1b8bc8 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sat, 30 Sep 2023 17:22:27 +0200
Subject: [PATCH] Properly reject invalid ActionCable subscriptions

Previously, we were not properly rejecting the submission, so that the channel name was still evaluated (leading to errors). Now, we handle these cases as well.

Fixes CODEOCEAN-V2
---
 app/channels/la_exercises_channel.rb        | 22 ++++++++++--
 app/channels/pg_matching_channel.rb         | 13 +++++---
 app/channels/synchronized_editor_channel.rb | 37 ++++++++++++++-------
 3 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/app/channels/la_exercises_channel.rb b/app/channels/la_exercises_channel.rb
index 278dac36..163e9f11 100644
--- a/app/channels/la_exercises_channel.rb
+++ b/app/channels/la_exercises_channel.rb
@@ -2,7 +2,10 @@
 
 class LaExercisesChannel < ApplicationCable::Channel
   def subscribed
-    stream_from specific_channel
+    set_and_authorize_exercise
+    set_and_authorize_study_group
+
+    stream_from specific_channel unless subscription_rejected?
   end
 
   def unsubscribed
@@ -12,7 +15,20 @@ class LaExercisesChannel < ApplicationCable::Channel
   private
 
   def specific_channel
-    reject unless StudyGroupPolicy.new(current_user, StudyGroup.find(params[:study_group_id])).stream_la?
-    "la_exercises_#{params[:exercise_id]}_channel_study_group_#{params[:study_group_id]}"
+    "la_exercises_#{@exercise.id}_channel_study_group_#{@study_group.id}"
+  end
+
+  def set_and_authorize_exercise
+    @exercise = Exercise.find(params[:exercise_id])
+    reject unless ExercisePolicy.new(current_user, @exercise).implement?
+  rescue ActiveRecord::RecordNotFound
+    reject
+  end
+
+  def set_and_authorize_study_group
+    @study_group = @exercise.study_groups.find(params[:study_group_id])
+    reject unless StudyGroupPolicy.new(current_user, @study_group).stream_la?
+  rescue ActiveRecord::RecordNotFound
+    reject
   end
 end
diff --git a/app/channels/pg_matching_channel.rb b/app/channels/pg_matching_channel.rb
index ac337ae9..01ffa15f 100644
--- a/app/channels/pg_matching_channel.rb
+++ b/app/channels/pg_matching_channel.rb
@@ -3,7 +3,8 @@
 class PgMatchingChannel < ApplicationCable::Channel
   def subscribed
     set_and_authorize_exercise
-    stream_from specific_channel
+
+    stream_from specific_channel unless subscription_rejected?
   end
 
   def unsubscribed
@@ -13,10 +14,6 @@ class PgMatchingChannel < ApplicationCable::Channel
     stop_all_streams
   end
 
-  def specific_channel
-    "pg_matching_channel_exercise_#{@exercise.id}"
-  end
-
   def waiting_for_match
     @current_waiting_user = PairProgrammingWaitingUser.find_or_initialize_by(user: current_user, exercise: @exercise)
     @current_waiting_user.status_waiting!
@@ -40,8 +37,14 @@ class PgMatchingChannel < ApplicationCable::Channel
     ActionCable.server.broadcast(specific_channel, {action: 'joined_pg', users: pg.users.map(&:to_page_context)})
   end
 
+  def specific_channel
+    "pg_matching_channel_exercise_#{@exercise.id}"
+  end
+
   def set_and_authorize_exercise
     @exercise = Exercise.find(params[:exercise_id])
     reject unless ExercisePolicy.new(current_user, @exercise).implement?
+  rescue ActiveRecord::RecordNotFound
+    reject
   end
 end
diff --git a/app/channels/synchronized_editor_channel.rb b/app/channels/synchronized_editor_channel.rb
index ce28fe24..9c7ccc7e 100644
--- a/app/channels/synchronized_editor_channel.rb
+++ b/app/channels/synchronized_editor_channel.rb
@@ -2,7 +2,10 @@
 
 class SynchronizedEditorChannel < ApplicationCable::Channel
   def subscribed
-    stream_from specific_channel
+    set_and_authorize_exercise
+    authorize_programming_group
+
+    stream_from specific_channel unless subscription_rejected?
 
     # We generate a session_id for the user and send it to the client
     @session_id = SecureRandom.uuid
@@ -28,17 +31,6 @@ class SynchronizedEditorChannel < ApplicationCable::Channel
     ActionCable.server.broadcast(specific_channel, message)
   end
 
-  def specific_channel
-    reject unless ProgrammingGroupPolicy.new(current_user, programming_group).stream_sync_editor?
-    "synchronized_editor_channel_group_#{programming_group.id}"
-  rescue NoMethodError => e
-    Sentry.capture_exception(e, extra: {current_user:, programming_group:, session_id: @session_id, identifier: @identifier})
-  end
-
-  def programming_group
-    current_contributor if current_contributor.programming_group?
-  end
-
   def editor_change(message)
     change = message.deep_symbolize_keys
 
@@ -63,6 +55,16 @@ class SynchronizedEditorChannel < ApplicationCable::Channel
     ActionCable.server.broadcast(specific_channel, message)
   end
 
+  private
+
+  def specific_channel
+    "synchronized_editor_channel_group_#{programming_group.id}"
+  end
+
+  def programming_group
+    current_contributor if current_contributor.programming_group?
+  end
+
   def create_message(action, status)
     {
       action:,
@@ -80,4 +82,15 @@ class SynchronizedEditorChannel < ApplicationCable::Channel
       session_id: @session_id,
     }
   end
+
+  def set_and_authorize_exercise
+    @exercise = Exercise.find(params[:exercise_id])
+    reject unless ExercisePolicy.new(current_user, @exercise).implement?
+  rescue ActiveRecord::RecordNotFound
+    reject
+  end
+
+  def authorize_programming_group
+    reject unless ProgrammingGroupPolicy.new(current_user, programming_group).stream_sync_editor?
+  end
 end