diff --git a/.gitignore b/.gitignore index e56d4fb5..1825835a 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ Gemfile.lock !/examples/heroku_api/Gemfile.lock .DS_Store /coverage/ +.idea diff --git a/.ruby-version b/.ruby-version new file mode 100644 index 00000000..e70b4523 --- /dev/null +++ b/.ruby-version @@ -0,0 +1 @@ +2.6.0 diff --git a/README.md b/README.md index 72aacd64..707d80cd 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ use Committee::Middleware::RequestValidation, schema_path: 'docs/schema.json', c This piece of middleware validates the parameters of incoming requests to make sure that they're formatted according to the constraints imposed by a particular schema. -Options and their defaults: +### Configuration Options | name | Hyper-Schema | OpenAPI 3 | Description | |-----------:|------------:|------------:| :------------ | @@ -39,6 +39,7 @@ Options and their defaults: |optimistic_json| false | false | Will attempt to parse JSON in the request body even without a `Content-Type: application/json` before falling back to other options. | |raise| false | false | Raise an exception on error instead of responding with a generic error body. | |strict| false | false | Puts the middleware into strict mode, meaning that paths which are not defined in the schema will be responded to with a 404 instead of being run. | +|strict_reference_validation| always false | false | Raises an exception (`OpenAPIParser::MissingReferenceError`) on middleware load if the provided schema file contains unresolvable references (`$ref:"#/something/not/here"`). Not supported on Hyper-schema parser. Defaults to `false` on OpenAPI3 but will default to `true` in next major version. | |ignore_error| false | false | Validate and ignore result even if validation is error. So always return original data. | Non-boolean options: @@ -153,7 +154,7 @@ use Committee::Middleware::ResponseValidation, schema_path: 'docs/schema.json' This piece of middleware validates the contents of the response received from up the stack for any route that matches the JSON Schema. A hyper-schema link's `targetSchema` property is used to determine what a valid response looks like. -Option values and defaults: +### Configuration Options | name | Hyper-Schema | OpenAPI 3 | Description | |-----------:|------------:|------------:| :------------ | @@ -162,6 +163,7 @@ Option values and defaults: |ignore_error| false | false | Validate and ignore result even if validation is error. So always return original data. | |parse_response_by_content_type| false | false | Parse response body to JSON only if Content-Type header is 'application/json'. When false, this always optimistically parses as JSON without checking for Content-Type header. | |strict| false | false | Puts the middleware into strict mode, meaning that response code and content type does not defined in the schema will be responded to with a 500 instead of application's status code. | +|strict_reference_validation| always false | false | Raises an exception (`OpenAPIParser::MissingReferenceError`) on middleware load if the provided schema file contains unresolvable references (`$ref:"#/something/not/here"`). Not supported on Hyper-schema parser. Defaults to `false` on OpenAPI3 but will default to `true` in next major version. | No boolean option values: @@ -334,7 +336,6 @@ Committee 3.* has many breaking changes so we recommend upgrading to the latest Important changes are also described below. - ### Upgrading from Committee 4.* to 5.* Committee 5.* has few breaking changes so we recommend upgrading to the latest release on 4.* and fixing any deprecation errors you see before upgrading. @@ -342,6 +343,14 @@ Committee 5.* has few breaking changes so we recommend upgrading to the latest r - change `parse_response_by_content_type`'s default value from `false` to `true`. +#### Future Updates (5.*~) +OpenAPI3 Schema Users: Newly-added `strict_reference_validation` option defaults to `false` if not set. +From a later major version of Committee, we want to change it to `true` (or remove the option entirely and force +`true` to be passed to the OpenAPI3 parser (see: https://github.com/interagent/committee/issues/343). + +To remove deprecation warnings, pass `strict_reference_validation: true` as an option when loading the middleware +(because this is about schema parsing/initialization, the setting is valid for both request and response validation). + ### Setting schemas in middleware Committee 2.* supported setting `schema` to a string or a hash like this: diff --git a/committee.gemspec b/committee.gemspec index 27898058..d87f9aa8 100644 --- a/committee.gemspec +++ b/committee.gemspec @@ -19,7 +19,7 @@ Gem::Specification.new do |s| s.add_dependency "json_schema", "~> 0.14", ">= 0.14.3" s.add_dependency "rack", ">= 1.5" - s.add_dependency "openapi_parser", ">= 0.11.1", "< 1.0" + s.add_dependency "openapi_parser", "1.0.0.beta1" s.add_development_dependency "minitest", "~> 5.3" s.add_development_dependency "rack-test", "~> 0.6" diff --git a/lib/committee/drivers.rb b/lib/committee/drivers.rb index 98352c35..3c07a3ab 100644 --- a/lib/committee/drivers.rb +++ b/lib/committee/drivers.rb @@ -20,27 +20,27 @@ def self.driver_from_name(name) # load and build drive from JSON file # @param [String] schema_path # @return [Committee::Driver] - def self.load_from_json(schema_path) - load_from_data(JSON.parse(File.read(schema_path)), schema_path) + def self.load_from_json(schema_path, parser_options = {}) + load_from_data(JSON.parse(File.read(schema_path)), schema_path, parser_options) end # load and build drive from YAML file # @param [String] schema_path # @return [Committee::Driver] - def self.load_from_yaml(schema_path) + def self.load_from_yaml(schema_path, parser_options = {}) data = YAML.respond_to?(:unsafe_load_file) ? YAML.unsafe_load_file(schema_path) : YAML.load_file(schema_path) - load_from_data(data, schema_path) + load_from_data(data, schema_path, parser_options) end # load and build drive from file # @param [String] schema_path # @return [Committee::Driver] - def self.load_from_file(schema_path) + def self.load_from_file(schema_path, parser_options = {}) case File.extname(schema_path) when '.json' - load_from_json(schema_path) + load_from_json(schema_path, parser_options) when '.yaml', '.yml' - load_from_yaml(schema_path) + load_from_yaml(schema_path, parser_options) else raise "Committee only supports the following file extensions: '.json', '.yaml', '.yml'" end @@ -49,9 +49,19 @@ def self.load_from_file(schema_path) # load and build drive from Hash object # @param [Hash] hash # @return [Committee::Driver] - def self.load_from_data(hash, schema_path = nil) + def self.load_from_data(hash, schema_path = nil, parser_options = {}) if hash['openapi']&.start_with?('3.0.') - openapi = OpenAPIParser.parse_with_filepath(hash, schema_path) + # From the next major version, we want to ensure `{ strict_reference_validation: true }` + # as a parser option here, but since it may break existing implementations, just warn + # if it is not explicitly set. See: https://github.com/interagent/committee/issues/343#issuecomment-997400329 + opts = parser_options.dup + unless opts.key?(:strict_reference_validation) + Committee.warn_deprecated('openapi_parser will default to strict reference validation ' + + 'from next version. Pass config `strict_reference_validation: true` (or false, if you must) ' + + 'to quiet this warning.') + opts[:strict_reference_validation] = false + end + openapi = OpenAPIParser.parse_with_filepath(hash, schema_path, opts) return Committee::Drivers::OpenAPI3::Driver.new.parse(openapi) end @@ -61,6 +71,7 @@ def self.load_from_data(hash, schema_path = nil) Committee::Drivers::HyperSchema::Driver.new end + # TODO: in the future, pass `opts` here and allow optionality in other drivers? driver.parse(hash) end end diff --git a/lib/committee/middleware/base.rb b/lib/committee/middleware/base.rb index 091bedf1..33fcee1c 100644 --- a/lib/committee/middleware/base.rb +++ b/lib/committee/middleware/base.rb @@ -30,11 +30,12 @@ def call(env) class << self def get_schema(options) schema = options[:schema] - unless schema - schema = Committee::Drivers::load_from_file(options[:schema_path]) if options[:schema_path] - - raise(ArgumentError, "Committee: need option `schema` or `schema_path`") unless schema + if !schema && options[:schema_path] + # In the future, we could have `parser_options` as an exposed config? + parser_options = options.key?(:strict_reference_validation) ? { strict_reference_validation: options[:strict_reference_validation] } : {} + schema = Committee::Drivers::load_from_file(options[:schema_path], parser_options) end + raise(ArgumentError, "Committee: need option `schema` or `schema_path`") unless schema # Expect the type we want by now. If we don't have it, the user passed # something else non-standard in. diff --git a/test/data/openapi3/invalid_reference.yaml b/test/data/openapi3/invalid_reference.yaml new file mode 100644 index 00000000..c1a43bbb --- /dev/null +++ b/test/data/openapi3/invalid_reference.yaml @@ -0,0 +1,29 @@ +openapi: 3.0.1 +info: + version: 1.0.0 + title: OpenAPI3 Test + description: A Sample file +paths: + /characters: + get: + description: get characters + parameters: + - name: school_name + in: query + description: school name to filter by + required: false + style: form + schema: + type: array + items: + type: string + responses: + '200': + # Does not exist, `ThisIsDefined` exists + $ref: '#/components/responses/ThisIsNotDefinedAnywhere' + +components: + responses: + # Not used + ThisIsDefined: + type: object \ No newline at end of file diff --git a/test/drivers_test.rb b/test/drivers_test.rb index 7f71eff1..4f2dfedd 100644 --- a/test/drivers_test.rb +++ b/test/drivers_test.rb @@ -48,6 +48,19 @@ assert_kind_of Committee::Drivers::OpenAPI3::Schema, s end + it 'fails to load OpenAPI 3 with invalid reference' do + parser_options = { strict_reference_validation: true } + assert_raises(OpenAPIParser::MissingReferenceError) do + Committee::Drivers.load_from_file(open_api_3_invalid_reference_path, parser_options) + end + end + + # This test can be removed when the test above (raising on invalid reference) becomes default behavior? + it 'allows loading OpenAPI 3 with invalid reference as existing behavior' do + s = Committee::Drivers.load_from_file(open_api_3_invalid_reference_path) + assert_kind_of Committee::Drivers::OpenAPI3::Schema, s + end + it 'errors on an unsupported file extension' do e = assert_raises(StandardError) do Committee::Drivers.load_from_file('test.xml') diff --git a/test/middleware/base_test.rb b/test/middleware/base_test.rb index d85c4b17..dc8e9dcd 100644 --- a/test/middleware/base_test.rb +++ b/test/middleware/base_test.rb @@ -119,6 +119,20 @@ def app b = Committee::Middleware::Base.new(nil, schema_path: open_api_3_schema_path, parse_response_by_content_type: false) assert_kind_of Committee::Drivers::OpenAPI3::Schema, b.instance_variable_get(:@schema) end + + it "accepts OpenAPI3 parser config of strict_reference_validation and raises" do + assert_raises(OpenAPIParser::MissingReferenceError) do + Committee::Middleware::Base.new(nil, schema_path: open_api_3_invalid_reference_path, strict_reference_validation: true) + end + end + + it "does not raise by default even with invalid reference OpenAPI3 specification" do + b = Committee::Middleware::Base.new(nil, schema_path: open_api_3_invalid_reference_path, strict_reference_validation: false) + assert_kind_of Committee::Drivers::OpenAPI3::Schema, b.instance_variable_get(:@schema) + + b = Committee::Middleware::Base.new(nil, schema_path: open_api_3_invalid_reference_path) + assert_kind_of Committee::Drivers::OpenAPI3::Schema, b.instance_variable_get(:@schema) + end end private diff --git a/test/test_helper.rb b/test/test_helper.rb index e30e40d2..066db6af 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -112,3 +112,7 @@ def open_api_3_coverage_schema_path def open_api_3_0_1_schema_path "./test/data/openapi3/3_0_1.yaml" end + +def open_api_3_invalid_reference_path + "./test/data/openapi3/invalid_reference.yaml" +end \ No newline at end of file