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 <svc.cloudjenkins@instructure.com>
Reviewed-by: Steven McGee <steve.mcgee@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Evan Battaglia <ebattaglia@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>

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] },

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 =>

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