From 40a7d3d10a26733ff2f2d005710383ede96eef9b Mon Sep 17 00:00:00 2001 From: Evan Battaglia Date: Tue, 10 Sep 2024 15:13:31 -0230 Subject: [PATCH] Dyn Reg: forbid invalid display_type values display_type is a Canvas extension. The check in the InternalLtiConfiguration schema is stricter (it actually checks the value is one of the valid types) than the Dyn Reg schema in lib/schemas/lti/ims/lti_tool_configuration.rb, so will cause problems later when we convert the Dyn Reg tool config to the internal tool config. refs INTEROP-8538 Test plan: - specs - we also need to make sure in the DB there are no invalid values here. Change-Id: I138473265cdc57b979fa79068c716a32f9f64feb Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/357129 Tested-by: Service Cloud Jenkins Reviewed-by: Steven McGee Reviewed-by: Xander Moffatt QA-Review: Evan Battaglia Product-Review: Evan Battaglia --- lib/schemas/internal_lti_configuration.rb | 4 +++- lib/schemas/lti/ims/lti_tool_configuration.rb | 2 +- spec/lib/schemas/lti/ims/oidc_registration_spec.rb | 6 ++++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/schemas/internal_lti_configuration.rb b/lib/schemas/internal_lti_configuration.rb index 2636f5ce849..aa3a8433211 100644 --- a/lib/schemas/internal_lti_configuration.rb +++ b/lib/schemas/internal_lti_configuration.rb @@ -22,6 +22,8 @@ module Schemas # Represents the "internal" JSON schema used to configure an LTI 1.3 tool, # as stored in Lti::ToolConfiguration and used in Lti::Registration. class InternalLtiConfiguration < Base + VALID_DISPLAY_TYPES = %w[default full_width full_width_in_context in_nav_context borderless].freeze + # Transforms a hash conforming to the LtiConfiguration schema into # a hash conforming to the InternalLtiConfiguration schema. def self.from_lti_configuration(lti_config) @@ -100,7 +102,7 @@ module Schemas canvas_icon_class: { type: "string" }, required_permissions: { type: "string" }, windowTarget: { type: "string", enum: %w[_blank] }, - display_type: { type: "string", enum: %w[default full_width full_width_in_context in_nav_context borderless] }, + display_type: { type: "string", enum: VALID_DISPLAY_TYPES }, url: { type: "string", description: "Defers to target_link_uri for 1.3 tools" }, target_link_uri: { type: "string" }, visibility: { type: "string", enum: %w[admins members public] }, diff --git a/lib/schemas/lti/ims/lti_tool_configuration.rb b/lib/schemas/lti/ims/lti_tool_configuration.rb index f1de80d1816..76cc89cc81d 100644 --- a/lib/schemas/lti/ims/lti_tool_configuration.rb +++ b/lib/schemas/lti/ims/lti_tool_configuration.rb @@ -68,7 +68,7 @@ module Schemas::Lti::IMS Lti::IMS::Registration::PLACEMENT_VISIBILITY_EXTENSION => { type: %w[string null].freeze, enum: [nil, *Lti::IMS::Registration::PLACEMENT_VISIBILITY_OPTIONS].freeze }.freeze, Lti::IMS::Registration::DISPLAY_TYPE_EXTENSION => - { type: %w[string null].freeze }.freeze, + { type: %w[string null].freeze, enum: [nil, *Schemas::InternalLtiConfiguration::VALID_DISPLAY_TYPES] }.freeze, Lti::IMS::Registration::LAUNCH_WIDTH_EXTENSION => { type: %w[integer string null] }.freeze, Lti::IMS::Registration::LAUNCH_HEIGHT_EXTENSION => diff --git a/spec/lib/schemas/lti/ims/oidc_registration_spec.rb b/spec/lib/schemas/lti/ims/oidc_registration_spec.rb index 5a3cf114a1e..365368a1da8 100644 --- a/spec/lib/schemas/lti/ims/oidc_registration_spec.rb +++ b/spec/lib/schemas/lti/ims/oidc_registration_spec.rb @@ -365,11 +365,13 @@ describe Schemas::Lti::IMS::OidcRegistration do expect_no_errors(message: { key => "public" }) end - it "requires Canvas extension display_type (if present & non null) to be a string" do + it "requires Canvas extension display_type (if present & non null) to be a from an enum" do key = Lti::IMS::Registration::DISPLAY_TYPE_EXTENSION expect(errors(message: { key => 123 })).to include("display_type") + expect(errors(message: { key => "something-invalid" })).to include("display_type") expect_no_errors(message: { key => nil }) - expect_no_errors(message: { key => "_blank" }) + expect_no_errors(message: { key => "in_nav_context" }) + expect_no_errors(message: { key => "default" }) end it "requires Canvas extension launch_width (if present & non null) to be an integer or string" do