fix(security): T2 2026 permission and security fixes

app/api/authentication_api.rb

@@ -13,6 +13,7 @@ class AuthenticationApi < Grape::API
   helpers AuthenticationHelpers
   helpers AuthorisationHelpers
   helpers LtiHelper
+  helpers FederatedIdentityHelper
 
   #
   # Sign in - only mounted if AAF and SAML auth is NOT used (database auth)
@@ -79,7 +80,7 @@ class AuthenticationApi < Grape::API
       token = user.generate_authentication_token!
 
       # Return user details
-      present :user, user, with: Entities::UserEntity
+      present :user, user, with: Entities::UserEntity, theme_owner_id: user.id
       present :auth_token, token.authentication_token
       present :auth_token_expiry, token.auth_token_expiry
       set_refresh_cookie_in_response(remember)
@@ -108,12 +109,11 @@ class AuthenticationApi < Grape::API
 
       logger.info "Authenticate #{user_id_data[:email]} from #{request.ip}"
 
-      # Lookup using login_id if it exists
-      # Lookup using email otherwise and set login_id
-      # Otherwise create new
-      user = User.find_by(login_id: user_id_data[:login_id]) ||
-             User.find_by(username: user_id_data[:username]) ||
-             User.find_by(email: user_id_data[:email]) ||
+      # Lookup on what the identity provider asserted, otherwise create new
+      user = user_for_asserted_identity(login_id: user_id_data[:login_id],
+                                        email: user_id_data[:email],
+                                        derived_username: user_id_data[:username],
+                                        source: request.ip) ||
              User.create do |new_user|
                # Update new user with details from the SAML response
                Doubtfire::Application.config.institution_settings.update_user_from_saml_response(
@@ -154,7 +154,11 @@ class AuthenticationApi < Grape::API
         protocol = Rails.env.development? ? 'http' : 'https'
         host = "#{protocol}://#{host}"
       end
-      redirect "#{host}/sign_in?authToken=#{onetime_token.authentication_token}&username=#{user.username}"
+      redirect AuthenticationHelpers.frontend_sign_in_url(
+        host: host,
+        auth_token: onetime_token.authentication_token,
+        username: user.username
+      )
     end
 
     # Saml 2 logout callback
@@ -215,12 +219,11 @@ class AuthenticationApi < Grape::API
 
       logger.info "Authenticate #{user_id_data[:email]} from #{request.ip}"
 
-      # Lookup using login_id if it exists
-      # Lookup using email otherwise and set login_id
-      # Otherwise create new
-      user = User.find_by(login_id: user_id_data[:login_id]) ||
-             User.find_by(username: user_id_data[:username]) ||
-             User.find_by(email: user_id_data[:email]) ||
+      # Lookup on what the identity provider asserted, otherwise create new
+      user = user_for_asserted_identity(login_id: user_id_data[:login_id],
+                                        email: user_id_data[:email],
+                                        derived_username: user_id_data[:username],
+                                        source: request.ip) ||
              User.create do |new_user|
                # Update new user with details from the LTI response
                Doubtfire::Application.config.institution_settings.update_user_from_lti_response(
@@ -294,12 +297,11 @@ class AuthenticationApi < Grape::API
 
       logger.info "Authenticate #{email} from #{request.ip}"
 
-      # Lookup using login_id if it exists
-      # Lookup using email otherwise and set login_id
-      # Otherwise create new
-      user = User.find_by(login_id: login_id) ||
-             User.find_by(username: email[/(.*)@/, 1]) ||
-             User.find_by(email: email) ||
+      # Lookup on what the identity provider asserted, otherwise create new
+      user = user_for_asserted_identity(login_id: login_id,
+                                        email: email,
+                                        derived_username: email[/(.*)@/, 1],
+                                        source: request.ip) ||
              User.find_or_create_by(login_id: login_id) do |new_user|
                role = Role.aaf_affiliation_to_role_id(attrs[:edupersonscopedaffiliation])
                first_name = (attrs[:givenname] || attrs[:cn]).capitalize
@@ -344,7 +346,11 @@ class AuthenticationApi < Grape::API
         protocol = Rails.env.development? ? 'http' : 'https'
         host = "#{protocol}://#{host}"
       end
-      redirect "#{host}/sign_in?authToken=#{onetime_token.authentication_token}&username=#{user.username}"
+      redirect AuthenticationHelpers.frontend_sign_in_url(
+        host: host,
+        auth_token: onetime_token.authentication_token,
+        username: user.username
+      )
     end
   end
 
@@ -375,7 +381,7 @@ class AuthenticationApi < Grape::API
         logger.info "Login #{params[:username]} from #{request.ip}"
 
         # Respond user details with new auth token
-        present :user, user, with: Entities::UserEntity
+        present :user, user, with: Entities::UserEntity, theme_owner_id: user.id
         present :auth_token, token.authentication_token
         present :auth_token_expiry, token.auth_token_expiry
         set_refresh_cookie_in_response(params[:remember])
@@ -502,7 +508,7 @@ class AuthenticationApi < Grape::API
       end
       # Return user details
       token = current_user.generate_authentication_token!(token_type: :general, force_new: false)
-      present :user, current_user, with: Entities::UserEntity
+      present :user, current_user, with: Entities::UserEntity, theme_owner_id: current_user.id
       present :auth_token, token.authentication_token
       present :auth_token_expiry, token.auth_token_expiry
     else

app/api/d2l_integration_api/d2l_api.rb

@@ -53,11 +53,11 @@ class D2lApi < Grape::API
 
       d2l = unit.d2l_assessment_mapping
 
-      if d2l.id != params[:id].to_i
+      if d2l.nil? || d2l.id != params[:id].to_i
         error!({ error: 'D2L details not found' }, 404)
       end
 
-      d2l.destroy if d2l.present?
+      d2l.destroy
       status 204
     end
 
@@ -75,7 +75,7 @@ class D2lApi < Grape::API
 
       d2l = unit.d2l_assessment_mapping
 
-      if d2l.id != params[:id].to_i
+      if d2l.nil? || d2l.id != params[:id].to_i
         error!({ error: 'D2L details not found' }, 404)
       end
 

app/api/learning_outcomes_api.rb

@@ -5,6 +5,7 @@ class LearningOutcomesApi < Grape::API
   helpers AuthorisationHelpers
   helpers MimeCheckHelpers
   helpers CsvHelper
+  helpers ContextModelHelpers
 
   before do
     authenticated?
@@ -26,8 +27,8 @@ class LearningOutcomesApi < Grape::API
   end
   post '/:context_type_plural/:context_id/outcomes' do
     # find context model dynamically
-    context_type = params[:context_type_plural].singularize.camelize
-    context_model = context_type.classify.constantize.find(params[:context_id])
+    context_type = context_type_for(params[:context_type_plural])
+    context_model = context_model_for(params[:context_type_plural], params[:context_id])
 
     unless authorise? current_user, context_model, :update
       error!({ error: 'You are not authorised to create outcomes in this context.' }, 403)
@@ -77,8 +78,7 @@ class LearningOutcomesApi < Grape::API
   end
   put '/:context_type_plural/:context_id/outcomes/:id' do
     # find context model dynamically
-    context_type = params[:context_type_plural].singularize.camelize
-    context_model = context_type.classify.constantize.find(params[:context_id])
+    context_model = context_model_for(params[:context_type_plural], params[:context_id])
 
     unless authorise? current_user, context_model, :update
       error!({ error: 'You are not authorised to update outcomes in this context.' }, 403)
@@ -130,8 +130,7 @@ class LearningOutcomesApi < Grape::API
   end
   delete '/:context_type_plural/:context_id/outcomes/:id' do
     # find context model dynamically
-    context_type = params[:context_type_plural].singularize.camelize
-    context_model = context_type.classify.constantize.find(params[:context_id])
+    context_model = context_model_for(params[:context_type_plural], params[:context_id])
 
     unless authorise? current_user, context_model, :update
       error!({ error: 'You are not authorised to delete outcomes in this context.' }, 403)
@@ -169,8 +168,7 @@ class LearningOutcomesApi < Grape::API
   get '/:context_type_plural/:context_id/outcomes/csv' do
     # find context model dynamically
     include_tlos = params[:includes_tlos] || false
-    context_type = params[:context_type_plural].singularize.camelize
-    context_model = context_type.classify.constantize.find(params[:context_id])
+    context_model = context_model_for(params[:context_type_plural], params[:context_id])
 
     unless authorise? current_user, context_model, :update
       error!({ error: 'You are not authorised to download outcomes for this context.' }, 403)
@@ -197,8 +195,7 @@ class LearningOutcomesApi < Grape::API
     ensure_csv!(params[:file][:tempfile])
 
     # find context model dynamically
-    context_type = params[:context_type_plural].singularize.camelize
-    context_model = context_type.classify.constantize.find(params[:context_id])
+    context_model = context_model_for(params[:context_type_plural], params[:context_id])
 
     unless authorise? current_user, context_model, :upload_csv
       error!({ error: 'Not authorised to upload CSV of outcomes' }, 403)

app/api/lti_api.rb

@@ -71,30 +71,54 @@ class LtiApi < Grape::API
       error!({ error: "Missing required fields:  #{missing.join(', ')}" }, 400)
     end
 
-    # if current_user.role_id != Role.student_id
-    #   return status 204
-    # end
+    # The token names the person the launch was issued for, and its roles decide
+    # what that person gets. Apply it to that person, and only while that person
+    # is the one holding the session.
+    unless lti_member_is?(member, current_user)
+      error!({ error: 'This LTI token was not issued for the signed in user.' }, 403)
+    end
 
-    # role = unit.role_for(current_user)
-    # if !role.nil? && role != Role.student
-    #   # error!({ error: 'Failed to enrol, user is already staff.' }, 400)
-    #   return status 204
-    # end
+    subject = current_user
 
     unit_role = Doubtfire::Application.config.institution_settings.should_employ_lti_member(member)
-    unless unit_role.nil?
-      unit.employ_staff(current_user, unit_role)
-    end
-
-    unless Doubtfire::Application.config.institution_settings.should_enrol_lti_member(member)
+    enrol_member = Doubtfire::Application.config.institution_settings.should_enrol_lti_member(member)
+
+    project = nil
+    consumed = ConsumedLtiToken.find_by(jti: token['jti'])
+
+    if consumed.present?
+      # The web client carries the one launch token for the whole session and
+      # calls this route on every mount of the dashboard, so the subject
+      # presenting their own token again is not a replay. The token is spent
+      # either way, so nothing is applied a second time.
+      unless consumed.spent_by?(subject)
+        error!({ error: 'This LTI token has already been used.' }, 403)
+      end
+
+      project = unit.projects.find_by(user_id: subject.id) if enrol_member
+    else
+      begin
+        ActiveRecord::Base.transaction do
+          # Spend the token before anything is applied. A concurrent replay
+          # loses on the unique index and rolls the whole enrolment back.
+          ConsumedLtiToken.consume!(token, user: subject)
+
+          unit.employ_staff(subject, unit_role) unless unit_role.nil?
+
+          # TODO: which campus?
+          project = unit.enrol_student(subject, nil) if enrol_member
+        end
+      rescue ConsumedLtiToken::AlreadyUsed
+        error!({ error: 'This LTI token has already been used.' }, 403)
+      end
+    end
+
+    if project.nil?
       # error!({ error: 'User can not be enrolled into this unit.' }, 404)
-      return status 204
+      status 204
+    else
+      present project, with: Entities::ProjectEntity, user: subject, for_student: true, in_project: true
     end
-
-    # TODO: which campus?
-    project = unit.enrol_student(current_user, nil)
-
-    present project, with: Entities::ProjectEntity, user: current_user, for_student: true, in_project: true
   end
 
   desc 'Enrol a list of students into a linked Lti unit'

app/api/settings_public_api.rb

@@ -0,0 +1,27 @@
+require 'grape'
+
+class SettingsPublicApi < Grape::API
+  # This endpoint is required before sign-in.
+  # Keep this response explicitly allowlisted.
+  desc 'Return public branding details for the Doubtfire front end'
+  get '/settings/public' do
+    response = {
+      externalName: Doubtfire::Application.config.institution[:product_name],
+      hasLogo: Doubtfire::Application.config.institution[:has_logo],
+      logoUrl: Doubtfire::Application.config.institution[:logo_url],
+      logoLinkUrl: Doubtfire::Application.config.institution[:logo_link_url]
+    }
+
+    present response, with: Grape::Presenters::Presenter
+  end
+
+  desc 'Return public privacy policy details'
+  get '/settings/privacy' do
+    response = {
+      privacy: Doubtfire::Application.config.institution[:privacy],
+      plagiarism: Doubtfire::Application.config.institution[:plagiarism]
+    }
+
+    present response, with: Grape::Presenters::Presenter
+  end
+end

app/helpers/authentication_helpers.rb

@@ -1,4 +1,5 @@
 require 'onelogin/ruby-saml'
+require 'uri'
 
 #
 # The AuthenticationHelpers include functions to check if the user
@@ -31,7 +32,7 @@ def user_auth_token_type(user_param, auth_param, token_type)
     if user.present? && token.present?
       # has the tolken not expired?
       if token.auth_token_expiry > Time.zone.now
-        logger.info("Authenticated #{user.username} from #{request.ip}")
+        logger.info("Authenticated user_id=#{user.id} from #{request.ip}")
         :valid
       else
         # Token is timed out - destroy it and return error
@@ -40,7 +41,10 @@ def user_auth_token_type(user_param, auth_param, token_type)
         :token_expired
       end
     elsif token.present?
-      logger.info("Error logging in for #{user_param} / #{auth_param} from #{request.ip}")
+      # Never echo the presented credential. This branch is reached for invalid
+      # and expired tokens, which are exactly the values an attacker may try to
+      # force into application logs.
+      logger.info("Error logging in with an invalid one-time token from #{request.ip}")
       :error
     else
       :missing_details
@@ -52,6 +56,19 @@ def user_auth_token_type(user_param, auth_param, token_type)
   #
   module_function
 
+  # Keep one-time sign-in credentials out of the query string. Query strings
+  # are routinely captured by reverse-proxy access logs, browser history, and
+  # telemetry. A URI fragment is not sent in the HTTP request; the web client
+  # consumes and removes it before initialising telemetry.
+  def frontend_sign_in_url(host:, auth_token:, username:)
+    callback_fragment = URI.encode_www_form(
+      authToken: auth_token,
+      username: username
+    )
+
+    "#{host.to_s.delete_suffix('/')}/sign_in##{callback_fragment}"
+  end
+
   def authenticated_via_refresh_token?
     auth_param = cookies['refresh_token']
     user_param = cookies['username']
@@ -220,7 +237,7 @@ def set_refresh_cookie_in_response(remember)
       token = current_user.auth_tokens.where(token_type: :refresh_token).last
 
       # Generate a new token when the old one is absent or getting close to expiring
-      if token.nil? || token.auth_token_expiry <= Time.zone.now - 12.hours
+      if token.nil? || token.auth_token_expiry <= Time.zone.now + 12.hours
         token = current_user.generate_authentication_token!(token_type: :refresh_token)
       end
 

app/helpers/context_model_helpers.rb

@@ -0,0 +1,20 @@
+module ContextModelHelpers
+  CONTEXT_MODELS = {
+    'units' => Unit,
+    'task_definitions' => TaskDefinition
+  }.freeze
+
+  def context_model_for(context_type_plural, context_id)
+    context_class_for(context_type_plural).find(context_id)
+  end
+
+  def context_type_for(context_type_plural)
+    context_class_for(context_type_plural).name
+  end
+
+  private
+
+  def context_class_for(context_type_plural)
+    CONTEXT_MODELS.fetch(context_type_plural.to_s)
+  end
+end