ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

send student id for LTI 1.3 student_context_card 

why:
* LTI 1.1 tools launching from this placement receive the
`ext_lti_student_id` launch parameter that corresponds to
the student from which the tool was launched
* this provides parity for LTI 1.3 tools to receive a similar parameter,
`https://www.instructure.com/lti_student_id`

closes INTEROP-8058
flag=none

test plan:
* install a 1.3 tool with the student_context_card placement enabled
* Course -> People -> click on a Student
* a card will slide out from the right side, and will have a button
to launch the 1.3 tool
* launch the tool
* scroll to the bottom of the decoded id_token
* the lti_student_id custom claim should be present and
match the id of the student you clicked on
* bonus:
  * change the id in the URL to a non-student - it should fail
  * change the id to a non-number like SQL injection - it should fail
  * remove the `&student_id=1` query param entirely - the launch should
  succeed, but not include the lti_student_id claim
* extra bonus:
  * test this with a 1.1 tool to confirm it's still working as intended

Change-Id: Ie3aeebb549c14b978fe84e28748275478685fba4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/335539
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Paul Gray <paul.gray@instructure.com>
QA-Review: Paul Gray <paul.gray@instructure.com>
Product-Review: Alexis Nast <alexis.nast@instructure.com>
This commit is contained in:
Xander Moffatt 2023-12-18 15:32:28 -07:00
parent 62595bd4f0
commit a872bbeb80
10 changed files with 156 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/controllers/external_tools_controller.rb
View File

@ -767,7 +767,7 @@ class ExternalToolsController < ApplicationController
@context.user_has_been_student?(student)
raise Lti::Errors::UnauthorizedError unless can_launch
adapter.generate_post_payload(student_id: student.global_id)
adapter.generate_post_payload_for_student_context_card(student_id: student.global_id)
elsif tool.extension_setting(selection_type, "required_permissions")
can_launch = tool.visible_with_permission_check?(selection_type, @current_user, @context, session)
raise Lti::Errors::UnauthorizedError unless can_launch

23
app/models/lti/lti_advantage_adapter.rb
View File

@ -108,6 +108,24 @@ module Lti
login_request(resource_link_request.generate_post_payload_for_homework_submission(*args))
end
# Generates a login request pointing to a cached launch (ID token)
# suitable for student context card LTI launches.
#
# These launches occur when a teacher launches a tool from the
# student context card, which shows when clicking on the name
# from the gradebook or course user list.
#
# See method-level documentation of "generate_post_payload" for
# more details.
#
# For information on how the cached ID token is eventually retrieved
# and sent to a tool, please refer to the inline documentation of
# app/controllers/lti/ims/authentication_controller.rb
def generate_post_payload_for_student_context_card(student_id:)
@opts[:student_id] = student_id
login_request(resource_link_request.generate_post_payload)
end
# Generates a login request pointing to a general-use
# cached launch (ID token).
#
@ -133,10 +151,7 @@ module Lti
# For information on how the cached ID token is eventually retrieved
# and sent to a tool, please refer to the inline documentation of
# app/controllers/lti/ims/authentication_controller.rb
def generate_post_payload(student_id: nil)
# Takes a student ID parameter for compatibility with the LTI 1.1 method
# (in LtiOutboundAdapter), but we don't use it here yet. See INTEROP-7227
# and student_context_card spec in external_tools_controller_spec.rb
def generate_post_payload
login_request(generate_lti_params)
end

10
app/models/lti/lti_outbound_adapter.rb
View File

@ -86,12 +86,11 @@ module Lti
self
end
def generate_post_payload(assignment: nil, student_id: nil)
def generate_post_payload(assignment: nil)
raise("Called generate_post_payload before calling prepare_tool_launch") unless @tool_launch
hash = @tool_launch.generate(@overrides)
hash[:ext_lti_assignment_id] = assignment&.lti_context_id if assignment&.lti_context_id.present?
hash[:ext_lti_student_id] = student_id if student_id
begin
Lti::Security.signed_post_params(
hash,
@ -121,6 +120,13 @@ module Lti
generate_post_payload
end
def generate_post_payload_for_student_context_card(student_id:)
raise("Called generate_post_payload_for_student_context_card before calling prepare_tool_launch") unless @tool_launch
@overrides[:lti_student_id] = student_id
generate_post_payload
end
def launch_url(post_only: false)
raise("Called launch_url before calling prepare_tool_launch") unless @tool_launch

1
gems/lti_outbound/lib/lti_outbound/tool_launch.rb
View File

@ -152,6 +152,7 @@ module LtiOutbound
hash["oauth_callback"] = "about:blank"
hash["ext_platform"] = overrides[:platform] if overrides.key?(:platform)
hash["ext_lti_student_id"] = overrides[:lti_student_id] if overrides.key?(:lti_student_id)
@variable_expander.expand_variables!(hash)
hash

6
gems/lti_outbound/spec/lti_outbound/tool_launch_spec.rb
View File

@ -166,6 +166,12 @@ describe LtiOutbound::ToolLaunch do
expect(hash["resource_link_title"]).to eq "new tool name"
end
it "includes ext_lti_student_id when provided in overrides" do
lti_student_id = "123"
hash = tool_launch.generate(lti_student_id:)
expect(hash["ext_lti_student_id"]).to eq lti_student_id
end
describe "selected_html" do
it "gets escaped and assigned to the key text if passed in" do
tool_launch = LtiOutbound::ToolLaunch.new(url: "http://www.yahoo.com",

1
lib/lti/messages/jwt_message.rb
View File

@ -74,6 +74,7 @@ module Lti::Messages
add_lti11_legacy_user_id!
add_lti1p1_claims! if include_lti1p1_claims?
add_extension("placement", @opts[:resource_type])
add_extension("lti_student_id", @opts[:student_id].to_s) if @opts[:student_id].present?
@expander.expand_variables!(@message.extensions)
@message.validate! if validate_launch

71
spec/controllers/external_tools_controller_spec.rb
View File

@ -145,12 +145,14 @@ describe ExternalToolsController do
end
context "when current user is a teacher" do
subject { get :show, params: { course_id: @course.id, id: tool.id } }
before do
user_session(@teacher)
get :show, params: { course_id: @course.id, id: tool.id }
end
it "creates a login message" do
subject
expect(assigns[:lti_launch].params.keys).to match_array %w[
iss
login_hint
@ -165,21 +167,73 @@ describe ExternalToolsController do
end
it 'sets the "login_hint" to the current user lti id' do
subject
expect(assigns[:lti_launch].params["login_hint"]).to eq Lti::Asset.opaque_identifier_for(@teacher)
end
it "caches the the LTI 1.3 launch" do
subject
expect(cached_launch["https://purl.imsglobal.org/spec/lti/claim/message_type"]).to eq "LtiResourceLinkRequest"
end
it 'sets the "canvas_domain" to the request domain' do
subject
message_hint = JSON::JWT.decode(assigns[:lti_launch].params["lti_message_hint"], :skip_verification)
expect(message_hint["canvas_domain"]).to eq "localhost"
end
it "defaults placement to context navigation" do
subject
expect(cached_launch["https://www.instructure.com/placement"]).to eq "course_navigation"
end
context "in the student_context_card placement" do
subject { get :show, params: { course_id: @course.id, id: tool.id, placement: "student_context_card", student_id: }.compact }
let(:student_id) { raise "override" }
before do
tool.student_context_card = { enabled: true }
tool.save!
end
context "without student_id param" do
let(:student_id) { nil }
it "does not include lti_student_id in launch" do
subject
expect(cached_launch).not_to have_key("https://www.instructure.com/lti_student_id")
end
end
context "with non-existent student_id param" do
let(:student_id) { "wrong" }
it "returns a JSON error" do
subject
expect(response).to be_not_found
end
end
context "with non-student student_id param" do
let(:student_id) { @teacher.id.to_s }
it "returns a JSON error" do
subject
expect(response).to be_unauthorized
end
end
context "with valid student_id param" do
let(:student) { student_in_course(course: @course, active_all: true).user }
let(:student_id) { student.id }
it "includes lti_student_id in launch" do
subject
expect(cached_launch["https://www.instructure.com/lti_student_id"]).to eq(student.global_id.to_s)
end
end
end
end
context "when Lti::LaunchDebugLogger is enabled" do
@ -336,6 +390,21 @@ describe ExternalToolsController do
end
end
end
context "in the student_context_card placement" do
before do
tool.student_context_card = { enabled: true }
tool.save!
end
let(:student) { student_in_course(course: @course, active_all: true).user }
let(:student_id) { student.id }
it "includes ext_lti_student_id in the launch" do
get :show, params: { course_id: @course.id, id: tool.id, student_id:, placement: :student_context_card }
expect(assigns[:lti_launch].params["ext_lti_student_id"]).to eq(student.global_id.to_s)
end
end
end
context "ContentItemSelectionResponse" do

22
spec/lib/lti/messages/jwt_message_spec.rb
View File

@ -1071,4 +1071,26 @@ describe Lti::Messages::JwtMessage do
end
end
end
describe "inst-specific extension claims" do
subject { decoded_jwt["https://www.instructure.com/#{claim}"] }
context "placement claim" do
let(:claim) { "placement" }
it "matches the resource_type" do
expect(subject).to eq opts[:resource_type]
end
end
context "lti_student_id claim" do
let(:claim) { "lti_student_id" }
let(:student_id) { "123" }
let(:opts) { { student_id: } }
it "uses student_id from opts" do
expect(subject).to eq student_id
end
end
end
end

13
spec/models/lti/lti_advantage_adapter_spec.rb
View File

@ -82,6 +82,15 @@ describe Lti::LtiAdvantageAdapter do
@course
end
describe "#generate_post_payload_for_student_context_card" do
let(:login_message) { adapter.generate_post_payload_for_student_context_card(student_id:) }
let(:student_id) { "123" }
it "includes extension lti_student_id claim in the id_token" do
expect(params["https://www.instructure.com/lti_student_id"]).to eq(student_id)
end
end
describe "#generate_post_payload" do
context 'when the message type is "LtiDeepLinkingRequest"' do
let(:opts) { { resource_type: "editor_button", domain: "test.com" } }
@ -212,10 +221,6 @@ describe Lti::LtiAdvantageAdapter do
end
end
it "accepts a student_id parameter" do
expect(adapter.generate_post_payload(student_id: 123).keys).to include("iss")
end
context "when no i18n locale is set in the request" do
it "sets the canvas_locale in the message hint to the default i18n locale" do
expect(Canvas::Security.decode_jwt(login_message["lti_message_hint"])["canvas_locale"]).to eq "en"

19
spec/models/lti/lti_outbound_adapter_spec.rb
View File

@ -376,6 +376,25 @@ describe Lti::LtiOutboundAdapter do
end
end
describe "#generate_post_payload_for_student_context_card" do
let(:student_id) { user_model.global_id }
it "raises a not prepared error if the tool launch has not been prepared" do
expect do
adapter.generate_post_payload_for_student_context_card(student_id:)
end.to raise_error(RuntimeError, "Called generate_post_payload_for_student_context_card before calling prepare_tool_launch")
end
it "add student id to the overrides" do
tool_launch = double("tool launch", generate: {}, url: "http://example.com/launch")
allow(LtiOutbound::ToolLaunch).to receive(:new).and_return(tool_launch)
adapter.prepare_tool_launch(return_url, variable_expander)
adapter.generate_post_payload_for_student_context_card(student_id:)
expect(tool_launch).to have_received(:generate).with(hash_including(lti_student_id: student_id))
end
end
describe ".consumer_instance_class" do
around do |example|
orig_class = Lti::LtiOutboundAdapter.consumer_instance_class