Merge branch 'main' into cycomachead/141-implement-mass-assignment-idor-fixes/1

* main: (25 commits)
  chore: autocorrect RSpec/MatchWithSimpleRegex offenses
  chore: use Rails.env.local? for developer provider gate
  refactor: simplify developer login gate and trim comments
  fix: prevent account takeover by splitting user lookup paths
  refactor: remove redundant TokenRefreshable from SessionController
  feat: update assignments table styling and sync functionality
  feat: extend user sessions via token refresh and persistent cookies
  tweak postion of the page size dropdown
  style(test): fix rubocop layout offenses in staging interceptor spec
  feat: implement email interceptor for staging environment
  Bump json from 2.19.5 to 2.20.0
  Bump rubocop-rspec from 3.9.0 to 3.10.2
  Bump sentry-rails from 6.5.0 to 6.6.2
  Bump axe-core-rspec from 4.11.3 to 4.12.0
  Bump brakeman from 8.0.4 to 8.0.5
  feat: update pagination and page size options for pending requests
  Use 3.3.10 locally and add safety assured to migration
  refactor: trust top-level base dates, drop all_dates parsing
  refactor: simplify Canvas base-date handling and document the API
  fixup spec and includes call
  ...

Author: cycomachead

.tool-versions

@@ -1 +1 @@
-ruby 3.3
+ruby 3.3.10

Gemfile.lock

@@ -80,24 +80,12 @@ GEM
       activerecord (>= 6.0.0)
       activesupport (>= 6.0.0)
     ast (2.4.3)
-    axe-core-api (4.11.3)
+    axe-core-api (4.12.0)
       dumb_delegator
-      ostruct
-      virtus
-    axe-core-cucumber (4.11.3)
-      axe-core-api (= 4.11.3)
-      dumb_delegator
-      ostruct
-      virtus
-    axe-core-rspec (4.11.3)
-      axe-core-api (= 4.11.3)
-      dumb_delegator
-      ostruct
-      virtus
-    axiom-types (0.1.1)
-      descendants_tracker (~> 0.0.4)
-      ice_nine (~> 0.11.0)
-      thread_safe (~> 0.3, >= 0.3.1)
+    axe-core-cucumber (4.12.0)
+      axe-core-api (= 4.12.0)
+    axe-core-rspec (4.12.0)
+      axe-core-api (= 4.12.0)
     base64 (0.3.0)
     benchmark (0.5.0)
     bigdecimal (4.1.2)
@@ -112,7 +100,7 @@ GEM
       msgpack (~> 1.2)
     bootstrap (5.3.8)
       popper_js (>= 2.11.8, < 3)
-    brakeman (8.0.4)
+    brakeman (8.0.5)
       racc
     builder (3.3.0)
     capybara (3.40.0)
@@ -127,15 +115,13 @@ GEM
     capybara-screenshot (1.0.27)
       capybara (>= 1.0, < 4)
       launchy
-    cgi (0.5.1)
+    cgi (0.5.2)
     chartkick (5.2.1)
     childprocess (5.1.0)
       logger (~> 1.5)
     codeclimate-test-reporter (1.0.7)
       simplecov
     coderay (1.1.3)
-    coercible (1.0.0)
-      descendants_tracker (~> 0.0.1)
     concurrent-ruby (1.3.7)
     connection_pool (3.0.2)
     crack (1.0.1)
@@ -180,8 +166,6 @@ GEM
     debug (1.11.1)
       irb (~> 1.10)
       reline (>= 0.3.8)
-    descendants_tracker (0.0.4)
-      thread_safe (~> 0.3, >= 0.3.1)
     diff-lcs (1.6.2)
     docile (1.4.1)
     domain_name (0.6.20240107)
@@ -247,9 +231,8 @@ GEM
       multi_xml (>= 0.5.2)
     hypershield (0.6.1)
       activerecord (>= 7.2)
-    i18n (1.14.8)
+    i18n (1.15.2)
       concurrent-ruby (~> 1.0)
-    ice_nine (0.11.2)
     importmap-rails (2.2.3)
       actionpack (>= 6.0.0)
       activesupport (>= 6.0.0)
@@ -263,7 +246,7 @@ GEM
     jbuilder (2.15.1)
       actionview (>= 7.0.0)
       activesupport (>= 7.0.0)
-    json (2.19.5)
+    json (2.20.0)
     jwt (3.2.0)
       base64
     language_server-protocol (3.17.0.5)
@@ -372,15 +355,15 @@ GEM
     pg (1.6.3-x86_64-darwin)
     pg (1.6.3-x86_64-linux)
     popper_js (2.11.8)
-    pp (0.6.3)
+    pp (0.6.4)
       prettyprint
     prettyprint (0.2.0)
     prism (1.9.0)
     pry (0.16.0)
       coderay (~> 1.1)
       method_source (~> 1.0)
       reline (>= 0.6.0)
-    psych (5.3.1)
+    psych (5.4.0)
       date
       stringio
     public_suffix (7.0.5)
@@ -503,9 +486,10 @@ GEM
       rubocop (>= 1.72)
       rubocop-performance (>= 1.24)
       rubocop-rails (>= 2.30)
-    rubocop-rspec (3.9.0)
+    rubocop-rspec (3.10.2)
       lint_roller (~> 1.1)
-      rubocop (~> 1.81)
+      regexp_parser (>= 2.0)
+      rubocop (~> 1.86, >= 1.86.2)
     ruby-progressbar (1.13.0)
     rubyzip (3.3.0)
     safely_block (1.0.0)
@@ -524,10 +508,10 @@ GEM
       rexml (~> 3.2, >= 3.2.5)
       rubyzip (>= 1.2.2, < 4.0)
       websocket (~> 1.0)
-    sentry-rails (6.5.0)
+    sentry-rails (6.6.2)
       railties (>= 5.2.0)
-      sentry-ruby (~> 6.5.0)
-    sentry-ruby (6.5.0)
+      sentry-ruby (~> 6.6.2)
+    sentry-ruby (6.6.2)
       bigdecimal
       concurrent-ruby (~> 1.0, >= 1.0.2)
       logger
@@ -564,7 +548,6 @@ GEM
       memoist3 (~> 1.0.0)
       win32ole
     thor (1.5.0)
-    thread_safe (0.3.6)
     tilt (2.7.0)
     timecop (0.9.11)
     timeout (0.6.1)
@@ -582,10 +565,6 @@ GEM
     uri (1.1.1)
     useragent (0.16.11)
     version_gem (1.1.9)
-    virtus (2.0.0)
-      axiom-types (~> 0.1)
-      coercible (~> 1.0)
-      descendants_tracker (~> 0.0, >= 0.0.3)
     web-console (4.2.1)
       actionview (>= 6.0.0)
       activemodel (>= 6.0.0)
@@ -603,7 +582,7 @@ GEM
     win32ole (1.9.3)
     xpath (3.2.0)
       nokogiri (~> 1.8)
-    zeitwerk (2.7.5)
+    zeitwerk (2.8.2)
 
 PLATFORMS
   aarch64-linux

app/controllers/api/v1/requests_controller.rb

@@ -9,36 +9,38 @@ def index
 
       def create
         find_extension_params
-        # Get External Assignment object to find initial due date
-        assignment_response = @canvas_facade.get_assignment(
-          @course_to_lms.external_course_id.to_i,
-          @assignment.external_assignment_id.to_i
-        )
-        if assignment_response.status != 200
-          render json: assignment_response.to_json, status: :internal_server_error
+        course_id = @course_to_lms.external_course_id.to_i
+        assignment_id = @assignment.external_assignment_id.to_i
+
+        # Query the base ("Everyone") dates. get_base_dates reads the assignment
+        # with override_assignment_dates=false, whose top-level dates are the
+        # base dates for any number of overrides. See docs/Canvas_Dates_API.md.
+        base_dates = @canvas_facade.get_base_dates(course_id, assignment_id)
+        if base_dates.nil?
+          render json: { error: 'Could not fetch assignment dates from Canvas' }.to_json,
+                 status: :internal_server_error
           return
         end
-        assignment_json = JSON.parse(assignment_response.body)
 
         # Provision Extension
-        response = @canvas_facade.provision_extension(
-          @course_to_lms.external_course_id.to_i,
-          params[:student_uid].to_i,
-          @assignment.external_assignment_id.to_i,
-          params[:new_due_date]
-        )
-        unless response.success?
-          render json: response.body, status: response.status
+        begin
+          override = @canvas_facade.provision_extension(
+            course_id,
+            params[:student_uid].to_i,
+            assignment_id,
+            params[:new_due_date]
+          )
+        rescue CanvasFacade::CanvasAPIError, FailedPipelineError, NotFoundError => e
+          render json: { error: e.message }.to_json, status: :internal_server_error
           return
         end
-        assignment_override = JSON.parse(response.body)
 
         @extension = Extension.new(
           assignment_id: @assignment.id,
           student_email: nil,
-          initial_due_date: assignment_json['due_at'],
-          new_due_date: assignment_override['due_at'],
-          external_extension_id: assignment_override['id'],
+          initial_due_date: base_dates['due_at'],
+          new_due_date: override.override_due_date || params[:new_due_date],
+          external_extension_id: override.id,
           last_processed_by_id: nil
         )
         unless @extension.save

app/controllers/application_controller.rb

@@ -1,4 +1,6 @@
 class ApplicationController < ActionController::Base
+  include TokenRefreshable
+
   before_action :authenticated!, unless: -> { excluded_controller_action? }
 
   rescue_from LmsFacade::LmsAPIError, with: :handle_lms_api_error
@@ -54,7 +56,11 @@ def authenticated!
       elsif current_user.lms_credentials.empty?
         return handle_authentication_failure('User has no credentials.')
       elsif current_user.lms_credentials.first.expire_time < Time.zone.now
-        return handle_authentication_failure('You have been logged out.')
+        # The Canvas access token has expired. Rather than logging the user out
+        # immediately, try to use the stored (long-lived) refresh token to obtain
+        # a fresh access token so the session can continue. We only log the user
+        # out when there is no refresh token or the refresh actually fails.
+        return handle_authentication_failure('You have been logged out.') unless refresh_user_token(current_user)
       end
     end
     true

app/controllers/course_settings_controller.rb

@@ -64,6 +64,8 @@ def course_settings_params
       :auto_approve_days,
       :auto_approve_extended_request_days,
       :max_auto_approve,
+      :enable_min_hours_before_deadline,
+      :min_hours_before_deadline,
       :enable_gradescope,
       :gradescope_course_url,
       :extend_late_due_date,

app/controllers/courses_controller.rb

@@ -35,6 +35,7 @@ def show
       @assignments = @course.enabled_assignments
     else
       @assignments = @course.assignments
+      @assignments_last_synced_at = assignments_last_synced_at
     end
     render_role_based_view
   end
@@ -51,6 +52,8 @@ def new
     # TODO: Why do some courses have empty enrollments?
     existing_canvas_ids = @user.courses.pluck(:canvas_id)
     @courses_teacher = filter_courses(@courses, UserToCourse.staff_roles, existing_canvas_ids)
+    # Track if any teacher courses, so we still show the semester filter even if the selected semester filters out all courses.
+    @has_any_teacher_courses = @courses_teacher.any?
     @courses_student = filter_courses(@courses, [ UserToCourse::STUDENT_ROLE ], existing_canvas_ids)
 
     if @selected_semester.present?
@@ -125,6 +128,16 @@ def enrollments_last_synced_at
     nil
   end
 
+  # Returns the time assignments were last synced from the LMS, or nil if never synced.
+  def assignments_last_synced_at
+    synced_at = @course.course_to_lms&.recent_assignment_sync&.dig('synced_at')
+    return nil if synced_at.blank?
+
+    Time.zone.parse(synced_at.to_s)
+  rescue ArgumentError, TypeError
+    nil
+  end
+
   def set_course
     @course = Course.find_by(id: params[:id])
     redirect_to courses_path, alert: 'Course not found.' unless @course
@@ -149,6 +162,8 @@ def filter_by_semester(courses, semester)
   end
 
   # TODO: This should be moved to the Canvas Facade
+  # TODO: Canvas enrollments can have multiple roles,
+  # we SHOULD only look at the first one that matches our known roles.
   def filter_courses(courses, roles, exclude_ids = [])
     missing_enrollments = courses.select { |course| course['enrollments'].blank? }
     Rails.logger.warn("Canvas API by #{current_user.id}: Courses with missing enrollments: #{missing_enrollments.pluck('id').join(', ')}") unless missing_enrollments.empty?

app/controllers/session_controller.rb

@@ -1,6 +1,4 @@
 class SessionController < ApplicationController
-  include TokenRefreshable
-
   ## Login work flow explained here
   # Currently the login only supports third-party authentication with Canvas.
   # But the structure to support multiple login methods is largely in place.
@@ -65,14 +63,25 @@ def omniauth_callback
       expires_at: expires_at
     )
 
-    # Persist / update the user just like `create`
-    user = find_or_create_user(user_data, access_token)
+    developer = auth.provider == 'developer'
+    user =
+      if developer
+        developer_lookup_or_create(user_data, access_token)
+      else
+        canvas_lookup_or_create(user_data, access_token)
+      end
 
-    # Auto-enroll developer login users in test courses
-    if auth.provider == 'developer'
-      ensure_developer_test_enrollments(user)
+    # Either path returns nil when it refuses the login (e.g. canvas_lookup
+    # declining to re-key an existing account to a new canvas_uid).
+    if user.nil?
+      redirect_to root_path,
+                  alert: 'We could not link your account. Please contact an administrator.'
+      return
     end
 
+    # Auto-enroll developer login users in test courses
+    ensure_developer_test_enrollments(user) if developer
+
     redirect_to courses_path, notice: "Logged in! Welcome, #{user_data['name']}!"
   rescue StandardError => e
     Rails.logger.error("OmniAuth callback error: #{e.message}")
@@ -101,10 +110,43 @@ def ensure_developer_test_enrollments(user)
     end
   end
 
-  # TODO: Refactor.
-  def find_or_create_user(user_data, auth_token)
-    auth_token.token
-    user = nil
+  def developer_login_allowed?
+    Rails.env.local?
+  end
+
+  # Canonical production login path. Canvas owns identity, so users are keyed by
+  # canvas_uid: we refresh the email on a canvas_uid match and create a new
+  # account when the canvas_uid is unknown. We return nil (rather than re-keying)
+  # when a different canvas_uid presents an email that already belongs to an
+  # account, leaving the existing account untouched.
+  def canvas_lookup_or_create(user_data, auth_token)
+    user = User.find_by(canvas_uid: user_data['id'])
+
+    if user
+      user.email = user_data['email']
+    elsif User.exists?(email: user_data['primary_email'])
+      Rails.logger.warn(
+        "Refusing to link canvas_uid=#{user_data['id']} to existing account " \
+        "with email=#{user_data['primary_email']}"
+      )
+      return nil
+    else
+      user = User.new(canvas_uid: user_data['id'])
+      user.assign_attributes(
+        email: user_data['email'],
+        name: user_data['name']
+      )
+    end
+
+    persist_login!(user, auth_token)
+  end
+
+  # Developer / test masquerade path. INTENTIONALLY matches an existing user by
+  # email so a developer can log in *as* them without Canvas. Gated to dev/test
+  # only; returns nil otherwise. Do not use this matching for the Canvas path.
+  def developer_lookup_or_create(user_data, auth_token)
+    return nil unless developer_login_allowed?
+
     if User.exists?(email: user_data['primary_email'])
       user = User.find_by(email: user_data['primary_email'])
       user.canvas_uid = user_data['id']
@@ -118,6 +160,13 @@ def find_or_create_user(user_data, auth_token)
         name: user_data['name']
       )
     end
+
+    persist_login!(user, auth_token)
+  end
+
+  # Shared finalization for both login paths: persist the user, refresh their
+  # LMS credentials, and establish the session.
+  def persist_login!(user, auth_token)
     user.save!
     update_user_credential(user, auth_token)