Refactor LTI sign-in process to avoid accessing current_user

Author: MrSerth

app/controllers/concerns/lti.rb

@@ -93,7 +93,7 @@ def require_valid_exercise_token
     @exercise = if proxy_exercise.nil?
                   Exercise.find_by(token: params[:custom_token])
                 else
-                  proxy_exercise.get_matching_exercise(current_user)
+                  proxy_exercise.get_matching_exercise(@user)
                 end
     refuse_lti_launch(message: t('sessions.oauth.invalid_exercise_token')) unless @exercise
   end
@@ -186,9 +186,9 @@ def send_score_for(submission, user, score)
     end
   end
 
-  def set_current_user
-    @current_user = ExternalUser.find_or_create_by(consumer_id: @consumer.id, external_id: @provider.user_id)
-    current_user.update(email: external_user_email(@provider), name: external_user_name(@provider))
+  def set_user
+    @user = ExternalUser.find_or_create_by(consumer_id: @consumer.id, external_id: @provider.user_id)
+    @user.update(email: external_user_email(@provider), name: external_user_name(@provider))
   end
 
   def set_study_group_membership
@@ -201,10 +201,10 @@ def set_study_group_membership
               StudyGroup.find_or_create_by(external_id: @provider.resource_link_id, consumer: @consumer)
             end
 
-    study_group_membership = StudyGroupMembership.find_or_create_by(study_group: group, user: current_user)
+    study_group_membership = StudyGroupMembership.find_or_create_by(study_group: group, user: @user)
     study_group_membership.update(role: external_user_role(@provider))
     session[:study_group_id] = group.id
-    current_user.store_current_study_group_id(group.id)
+    @user.store_current_study_group_id(group.id)
   end
 
   def set_embedding_options
@@ -231,7 +231,7 @@ def set_embedding_options
   end
 
   def store_lti_session_data(parameters)
-    @lti_parameters = LtiParameter.find_or_initialize_by(external_user: current_user,
+    @lti_parameters = LtiParameter.find_or_initialize_by(external_user: @user,
       study_group_id: session[:study_group_id],
       exercise: @exercise)
 

app/controllers/sessions_controller.rb

@@ -4,7 +4,7 @@ class SessionsController < ApplicationController
   include Lti
 
   %i[require_oauth_parameters require_valid_consumer_key require_valid_oauth_signature require_unique_oauth_nonce
-     require_valid_launch_presentation_return_url require_valid_lis_outcome_service_url set_current_user require_valid_exercise_token
+     require_valid_launch_presentation_return_url require_valid_lis_outcome_service_url set_user require_valid_exercise_token
      set_study_group_membership set_embedding_options].each do |method_name|
     before_action(method_name, only: :create_through_lti)
   end
@@ -32,7 +32,7 @@ def create_through_lti
       redirect_to(new_exercise_programming_group_path(@exercise))
     else
       redirect_to(implement_exercise_path(@exercise),
-        notice: t("sessions.create_through_lti.session_#{lti_outcome_service?(@exercise, current_user) ? 'with' : 'without'}_outcome",
+        notice: t("sessions.create_through_lti.session_#{lti_outcome_service?(@exercise, @user) ? 'with' : 'without'}_outcome",
           consumer: @consumer))
     end
   end
@@ -84,7 +84,7 @@ def redirect_to_survey
       # It gives a bonus point to users who opened the survey
       begin
         lti_parameters = params.slice(*Lti::SESSION_PARAMETERS).permit!.to_h
-        provider = build_tool_provider(consumer: current_user.consumer, parameters: lti_parameters)
+        provider = build_tool_provider(consumer: @user.consumer, parameters: lti_parameters)
         provider.post_replace_result!(1.0)
       rescue IMS::LTI::InvalidLTIConfigError, IMS::LTI::XMLParseError, Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNREFUSED, Errno::ECONNRESET, SocketError, EOFError, OpenSSL::SSL::SSLError
         # We don't do anything here because it is only a bonus point and we want the users to do the survey
@@ -103,12 +103,12 @@ def redirect_to_survey
         newtest: 'Y', # force a new LimeSurvey session
         xi_platform: 'openhpi' # pass a platform identifier
       ).tap do |qp|
-      if current_user
+      if @user
         # add a user pseudo ID if applicable
-        qp[:xi_pseudo_id] = Digest::SHA256.hexdigest(current_user.external_id)
-        qp[:co_study_group_id] = current_user.current_study_group_id
-        qp[:co_rfcs] = current_user.request_for_comments.includes(:submission).where(submission: {study_group_id: current_user.current_study_group_id}).size.to_s
-        qp[:co_comments] = current_user.comments.includes(:submission).where(submission: {study_group_id: current_user.current_study_group_id}).size.to_s
+        qp[:xi_pseudo_id] = Digest::SHA256.hexdigest(@user.external_id)
+        qp[:co_study_group_id] = @user.current_study_group_id
+        qp[:co_rfcs] = @user.request_for_comments.includes(:submission).where(submission: {study_group_id: @user.current_study_group_id}).size.to_s
+        qp[:co_comments] = @user.comments.includes(:submission).where(submission: {study_group_id: @user.current_study_group_id}).size.to_s
       end
     end
 

spec/concerns/lti_spec.rb

@@ -272,7 +272,7 @@ class Controller < AnonymousController
     let(:parameters) { ActionController::Parameters.new({}) }
 
     it 'stores data in the session' do
-      controller.instance_variable_set(:@current_user, create(:external_user))
+      controller.instance_variable_set(:@user, create(:external_user))
       controller.instance_variable_set(:@exercise, create(:fibonacci))
       expect(controller.session).to receive(:[]=).with(:external_user_id, anything)
       expect(controller.session).to receive(:[]=).with(:pair_programming, anything)
@@ -281,7 +281,7 @@ class Controller < AnonymousController
 
     it 'creates an LtiParameter Object' do
       expect do
-        controller.instance_variable_set(:@current_user, create(:external_user))
+        controller.instance_variable_set(:@user, create(:external_user))
         controller.instance_variable_set(:@exercise, create(:fibonacci))
         controller.send(:store_lti_session_data, parameters)
       end.to change(LtiParameter, :count).by(1)

spec/controllers/sessions_controller_spec.rb

@@ -124,9 +124,9 @@
 
       before { allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:valid_request?).and_return(true) }
 
-      it 'assigns the current user' do
+      it 'assigns the current user as @user' do
         perform_request
-        expect(assigns(:current_user)).to be_an(ExternalUser)
+        expect(assigns(:user)).to be_an(ExternalUser)
         expect(session[:external_user_id]).to eq(user.id)
       end