Enforce privileged LTI launch role for course linking and creation

Changelog.md

@@ -33,6 +33,7 @@
 - Fixed the "Fix" link being clipped off the screen for long QR-scan error messages on the exam scan log table (#8070)
 - Fixed Assign Scans returning a raw 404 instead of redirecting back to the Groups page once all groups have been assigned (#8059)
 - Allow new instructors without existing courses to create a course via LTI launch (#8061)
+- Enforce privileged LTI launch role for course linking and creation- (#8130)
 
 ### 📚 Documentation changes
 - Made documentation site table-of-contents/section listings consistent (#8062)

app/controllers/lti_deployments_controller.rb

@@ -1,11 +1,12 @@
 class LtiDeploymentsController < ApplicationController
   skip_verify_authorized except: [:choose_course]
-  skip_forgery_protection except: [:choose_course]
+  skip_forgery_protection except: [:choose_course, :create_course]
 
   before_action :authenticate, :check_course_switch, :check_record,
                 except: [:get_config, :launch, :public_jwk, :redirect_login]
   before_action(except: [:get_config, :launch, :public_jwk, :redirect_login]) { authorize! }
   before_action :check_host, only: [:launch, :redirect_login]
+  before_action :check_lti_launch_role, only: [:choose_course, :create_course]
 
   USE_SECURE_COOKIES = !Rails.env.local?
 
@@ -151,6 +152,7 @@ def redirect_login
       has_privileged_role = lti_data[:user_roles].intersect?(LtiDeployment::LTI_PRIVILEGED_ROLES)
       has_ta_role = lti_data[:user_roles].include?(LtiDeployment::LTI_ROLES[:ta])
       if has_privileged_role && !has_ta_role
+        session[:lti_privileged_deployments] = Array(session[:lti_privileged_deployments]) | [lti_deployment.id]
         redirect_to choose_course_lti_deployment_path(lti_deployment)
       else
         redirect_to course_not_set_up_lti_deployment_path(lti_deployment)
@@ -254,4 +256,15 @@ def construct_redirect_with_port(url, endpoint: nil)
     referer_host = referer_host_with_port if referer.to_s.start_with?(referer_host_with_port)
     URI("#{referer_host}#{endpoint}")
   end
+
+  # Only allow linking or creating a course for LTI deployments that the current
+  # user launched from the LMS with a privileged (non-TA) role. The launch role
+  # is recorded in the session by #redirect_login.
+  def check_lti_launch_role
+    return if Array(session[:lti_privileged_deployments]).include?(record&.id)
+
+    @title = I18n.t('lti.launch_required_title')
+    @message = I18n.t('lti.launch_required')
+    render 'message', status: :forbidden
+  end
 end

config/locales/common/en.yml

@@ -38,6 +38,8 @@ en:
     grade_sync_complete: Grade Sync Complete
     grade_sync_instructions: This will only synchronize grades that have been released.
     launch: Launch MarkUs
+    launch_required: To link or create a course, please launch MarkUs from your LMS course.
+    launch_required_title: Launch Required
     line_item_created: Gradebook item created.
     lti_configuration: LTI configuration
     lti_course_link_html: Course linked to external course

spec/controllers/lti_deployments_controller_spec.rb

@@ -14,24 +14,39 @@
     let(:test_rlid) { 'a-unique-resource-link-id-12345' }
     let!(:lti) { create(:lti_deployment, resource_link_id: test_rlid) }
 
+    before do
+      session[:lti_privileged_deployments] = [lti.id]
+    end
+
     describe 'get' do
       it 'is inaccessible unless logged in' do
         get :choose_course, params: { id: lti.id }
         expect(response).to have_http_status(:found)
       end
 
       it 'is accessible when logged in' do
-        session[:lti_deployment_id] = lti.id
         get_as instructor, :choose_course, params: { id: lti.id }
         expect(response).to have_http_status(:ok)
       end
-    end
 
-    describe 'post' do
-      before do
-        session[:lti_deployment_id] = lti.id
+      context 'when the user did not launch this deployment with a privileged LMS role' do
+        before do
+          session[:lti_privileged_deployments] = []
+        end
+
+        it 'responds with forbidden' do
+          get_as instructor, :choose_course, params: { id: lti.id }
+          expect(response).to have_http_status(:forbidden)
+        end
+
+        it 'renders the launch required message' do
+          get_as instructor, :choose_course, params: { id: lti.id }
+          expect(response).to render_template('message')
+        end
       end
+    end
 
+    describe 'post' do
       context 'when picking a course' do
         it 'redirects to a course on success' do
           post_as instructor, :choose_course, params: { id: lti.id, course: course.id }
@@ -55,6 +70,23 @@
           expect(lti.resource_link_id).to eq(test_rlid)
         end
 
+        context 'when the user did not launch this deployment with a privileged LMS role' do
+          before do
+            session[:lti_privileged_deployments] = []
+          end
+
+          it 'does not link the course' do
+            post_as instructor, :choose_course, params: { id: lti.id, course: course.id }
+            lti.reload
+            expect(lti.course).to be_nil
+          end
+
+          it 'responds with forbidden' do
+            post_as instructor, :choose_course, params: { id: lti.id, course: course.id }
+            expect(response).to have_http_status(:forbidden)
+          end
+        end
+
         context 'when the user does not have permission to link' do
           let(:course2) { create(:course) }
           let(:instructor2) { create(:instructor, course: course2) }
@@ -79,9 +111,12 @@
       { id: lti_deployment.id, display_name: 'Introduction to Computer Science', name: lti_deployment.lms_course_name }
     end
 
+    before do
+      session[:lti_privileged_deployments] = [lti_deployment.id]
+    end
+
     context 'as an instructor with a standard term' do
       before do
-        session[:lti_deployment_id] = lti_deployment.id
         post_as instructor, :create_course, params: course_params
       end
 
@@ -118,10 +153,6 @@
         create(:lti_deployment, lms_term_name: 'Default Term', lms_course_name: 'csc108')
       end
 
-      before do
-        session[:lti_deployment_id] = lti_deployment.id
-      end
-
       it 'slugifies the term name as the suffix' do
         post_as instructor, :create_course, params: {
           id: lti_deployment.id,
@@ -134,7 +165,6 @@
 
     context 'as an admin user' do
       before do
-        session[:lti_deployment_id] = lti_deployment.id
         post_as admin_user, :create_course, params: course_params
       end
 
@@ -155,7 +185,6 @@
     context 'when a course already exists' do
       before do
         create(:course, name: expected_name)
-        session[:lti_deployment_id] = lti_deployment.id
       end
 
       it 'does not create a new course' do
@@ -174,10 +203,6 @@
         create(:lti_deployment, lms_course_name: 'csc108 fall 3000!', lms_term_name: 'Fall 2026')
       end
 
-      before do
-        session[:lti_deployment_id] = lti_deployment.id
-      end
-
       it 'creates a new course with a sanitized name and appends suffix' do
         post_as instructor, :create_course, params: course_params
         expect(Course.exists?(name: 'CSC108-FALL-3000-20269')).not_to be_nil
@@ -188,10 +213,6 @@
       # NOTE: the default filter in config/dummy_lti_config.rb only accepts course names starting with 'csc'
       let!(:lti_deployment) { create(:lti_deployment, lms_course_name: 'sta130', lms_term_name: 'Fall 2026') }
 
-      before do
-        session[:lti_deployment_id] = lti_deployment.id
-      end
-
       it 'does not create a new course' do
         post_as instructor, :create_course, params: course_params
         expect(Course.count).to eq(1)
@@ -219,6 +240,44 @@
         expect(Role.find_by(user: new_instructor_user, course: course, type: 'Instructor')).not_to be_nil
       end
     end
+
+    context 'when the user did not launch this deployment with a privileged LMS role' do
+      before do
+        session[:lti_privileged_deployments] = []
+      end
+
+      it 'does not create a course' do
+        post_as instructor, :create_course, params: course_params
+        expect(Course.find_by(name: expected_name)).to be_nil
+      end
+
+      it 'responds with forbidden' do
+        post_as instructor, :create_course, params: course_params
+        expect(response).to have_http_status(:forbidden)
+      end
+
+      it 'renders the launch required message' do
+        post_as instructor, :create_course, params: course_params
+        expect(response).to render_template('message')
+      end
+    end
+
+    context 'when the deployment is already linked to a course' do
+      let(:linked_course) { create(:course) }
+
+      before do
+        lti_deployment.update!(course: linked_course)
+        post_as instructor, :create_course, params: course_params
+      end
+
+      it 'responds with not found' do
+        expect(response).to have_http_status(:not_found)
+      end
+
+      it 'does not create a new course' do
+        expect(Course.find_by(name: expected_name)).to be_nil
+      end
+    end
   end
 
   describe '#public_jwk' do