diff --git a/Gemfile b/Gemfile index 40c781dc41..2d8b4932a9 100644 --- a/Gemfile +++ b/Gemfile @@ -97,7 +97,7 @@ gem 'browser' gem 'diff-lcs', '~> 1.2' gem 'hiredis-client' gem 'httpclient', github: '3scale/httpclient', branch: 'ssl-env-cert' -gem 'json-schema', git: 'https://github.com/3scale/json-schema.git' +gem 'json-schema' gem 'json_schemer' gem 'local-fastimage_resize', '~> 3.4.0', require: 'fastimage/resize' gem 'kt-paperclip', '~> 7.2' diff --git a/Gemfile.lock b/Gemfile.lock index f587a9279a..518d5bf38e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -12,13 +12,6 @@ GIT specs: httpclient (2.8.3) -GIT - remote: https://github.com/3scale/json-schema.git - revision: 26487618a68443e94d623bb585cb464b07d36702 - specs: - json-schema (3.0.0) - addressable (>= 2.4) - GIT remote: https://github.com/3scale/rspec-html-matchers.git revision: 58b19212afcf66540da729214287d85f586fa7cd @@ -163,8 +156,8 @@ GEM activerecord (>= 3.0) acts_as_tree (2.9.1) activerecord (>= 3.0.0) - addressable (2.8.0) - public_suffix (>= 2.0.2, < 5.0) + addressable (2.8.7) + public_suffix (>= 2.0.2, < 7.0) afm (0.2.2) after_commit_queue (1.1.0) rails (>= 3.0) @@ -219,7 +212,7 @@ GEM ransack (~> 4.1) base64 (0.2.0) bcrypt (3.1.13) - bigdecimal (3.1.8) + bigdecimal (3.1.9) binding_of_caller (1.0.0) debug_inspector (>= 0.0.1) bootsnap (1.16.0) @@ -438,7 +431,10 @@ GEM railties (>= 4.2.0) thor (>= 0.14, < 2.0) json (2.7.1) - json_schemer (2.3.0) + json-schema (5.1.1) + addressable (~> 2.8) + bigdecimal (~> 3.1) + json_schemer (2.4.0) bigdecimal hana (~> 1.3) regexp_parser (~> 2.0) @@ -609,7 +605,7 @@ GEM pry-stack_explorer (0.6.1) binding_of_caller (~> 1.0) pry (~> 0.13) - public_suffix (4.0.7) + public_suffix (6.0.1) raabro (1.4.0) racc (1.8.1) rack (2.2.13) @@ -708,7 +704,7 @@ GEM reform-rails (0.2.6) activemodel (>= 5.0) reform (>= 2.3.1, < 3.0.0) - regexp_parser (2.6.1) + regexp_parser (2.10.0) representable (3.0.4) declarative (< 0.1.0) declarative-option (< 0.2.0) @@ -1034,7 +1030,7 @@ DEPENDENCIES inherited_resources (~> 1.14.0) jquery-rails (= 4.6) json (~> 2.7, >= 2.7.1) - json-schema! + json-schema json_schemer jwt (~> 1.5.2) kt-paperclip (~> 7.2) diff --git a/app/lib/three_scale/json_validator.rb b/app/lib/three_scale/json_validator.rb index 5e93031ea6..37527151ba 100644 --- a/app/lib/three_scale/json_validator.rb +++ b/app/lib/three_scale/json_validator.rb @@ -6,8 +6,7 @@ class JSONValidator < ::JSON::Validator # or add them manually later with ThreeScale::JSONValidator.add_schema(schema) AUTOLOAD_SCHEMA_FILES = [ 'app/lib/three_scale/swagger/schemas/*.schema.json', - 'app/lib/three_scale/swagger/schemas/1.2/*.json', - 'app/lib/three_scale/policies/schemas/*.schema.json' + 'app/lib/three_scale/swagger/schemas/1.2/*.json' ].freeze private_constant :AUTOLOAD_SCHEMA_FILES diff --git a/app/lib/three_scale/policies/schemas/apicast-policy-v1.1.schema.json b/app/lib/three_scale/policies/schemas/apicast-policy-v1.1.schema.json new file mode 100644 index 0000000000..00d7c64f61 --- /dev/null +++ b/app/lib/three_scale/policies/schemas/apicast-policy-v1.1.schema.json @@ -0,0 +1,114 @@ +{ + "$id": "http://apicast.io/policy-v1.1/schema", + "type": "object", + "$schema": "http://json-schema.org/draft-07/schema#", + "definitions": { + "schema": { + "$id": "#/definitions/schema", + "$schema": "http://json-schema.org/draft-07/schema#", + "default": {} + }, + "version": { + "$id": "#/definitions/version", + "type": "string", + "title": "The Policy Version", + "description": "A semantic version of a policy.", + "examples": [ + "1.3.4", + "0.1" + ], + "pattern": "^((\\d+\\.)?(\\d+\\.)?(\\*|\\d+))|builtin$" + } + }, + "properties": { + "name": { + "$id": "/properties/name", + "type": "string", + "title": "The Policy Name", + "description": "Name of the policy.", + "examples": [ + "Basic Authentication" + ], + "minLength": 1 + }, + "summary": { + "$id": "/properties/summary", + "type": "string", + "title": "The Policy Summary", + "description": "Short description of what the policy does", + "examples": [ + "Enables CORS (Cross Origin Resource Sharing) request handling." + ], + "maxLength": 75 + }, + "description": { + "$id": "/properties/description", + "oneOf": [ + { "type": "string", + "minLength": 1 }, + { "type": "array", "items": { "type": "string" }, + "minItems": 1 + } + ], + "title": "The Policy Description", + "description": "Longer description of what the policy does.", + "examples": [ + "Extract authentication credentials from the HTTP Authorization header and pass them to 3scale backend.", + [ "Redirect request to different upstream: ", " - based on path", "- set different Host header"] + ] + }, + "order": { + "$id": "/properties/order", + "type": "object", + "title": "Order restrictions of the policy", + "description": "Specifies before or after which policies the policy should be placed in the chain.", + "properties": { + "before": { + "type": "array", + "items": { + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "version": { + "type": "string" + } + }, + "required": [ + "name", + "version" + ] + }, + "description": "The policy should be placed before these ones in the chain." + }, + "after": { + "type": "array", + "items": { + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "version": { + "type": "string" + } + }, + "required": [ + "name", + "version" + ] + }, + "description": "The policy should be placed after these ones in the chain." + } + } + }, + "version": { + "$ref": "#/definitions/version" + }, + "configuration": { + "$ref": "#/definitions/schema" + } + }, + "required": ["name", "version", "configuration", "summary"] +} diff --git a/app/lib/three_scale/policies/schemas/manifest.schema.json b/app/lib/three_scale/policies/schemas/apicast-policy-v1.schema.json similarity index 94% rename from app/lib/three_scale/policies/schemas/manifest.schema.json rename to app/lib/three_scale/policies/schemas/apicast-policy-v1.schema.json index 20a88ce5a5..2110eb1115 100644 --- a/app/lib/three_scale/policies/schemas/manifest.schema.json +++ b/app/lib/three_scale/policies/schemas/apicast-policy-v1.schema.json @@ -1,11 +1,11 @@ { - "$id": "http://apicast.io/policy-v1/schema#manifest", + "$id": "http://apicast.io/policy-v1/schema", "type": "object", "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { "schema": { "$id": "#/definitions/schema", - "$ref": "http://json-schema.org/draft-07/schema#", + "$schema": "http://json-schema.org/draft-07/schema#", "default": {} }, "version": { diff --git a/app/lib/three_scale/policies/schemas/json-draft-07.schema.json b/app/lib/three_scale/policies/schemas/json-draft-07.schema.json deleted file mode 100644 index 85079d89cd..0000000000 --- a/app/lib/three_scale/policies/schemas/json-draft-07.schema.json +++ /dev/null @@ -1,168 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "$id": "http://json-schema.org/draft-07/schema#", - "title": "Core schema meta-schema", - "definitions": { - "schemaArray": { - "type": "array", - "minItems": 1, - "items": { "$ref": "#" } - }, - "nonNegativeInteger": { - "type": "integer", - "minimum": 0 - }, - "nonNegativeIntegerDefault0": { - "allOf": [ - { "$ref": "#/definitions/nonNegativeInteger" }, - { "default": 0 } - ] - }, - "simpleTypes": { - "enum": [ - "array", - "boolean", - "integer", - "null", - "number", - "object", - "string" - ] - }, - "stringArray": { - "type": "array", - "items": { "type": "string" }, - "uniqueItems": true, - "default": [] - } - }, - "type": ["object", "boolean"], - "properties": { - "$id": { - "type": "string", - "format": "uri-reference" - }, - "$schema": { - "type": "string", - "format": "uri" - }, - "$ref": { - "type": "string", - "format": "uri-reference" - }, - "$comment": { - "type": "string" - }, - "title": { - "type": "string" - }, - "description": { - "type": "string" - }, - "default": true, - "readOnly": { - "type": "boolean", - "default": false - }, - "examples": { - "type": "array", - "items": true - }, - "multipleOf": { - "type": "number", - "exclusiveMinimum": 0 - }, - "maximum": { - "type": "number" - }, - "exclusiveMaximum": { - "type": "number" - }, - "minimum": { - "type": "number" - }, - "exclusiveMinimum": { - "type": "number" - }, - "maxLength": { "$ref": "#/definitions/nonNegativeInteger" }, - "minLength": { "$ref": "#/definitions/nonNegativeIntegerDefault0" }, - "pattern": { - "type": "string", - "format": "regex" - }, - "additionalItems": { "$ref": "#" }, - "items": { - "anyOf": [ - { "$ref": "#" }, - { "$ref": "#/definitions/schemaArray" } - ], - "default": true - }, - "maxItems": { "$ref": "#/definitions/nonNegativeInteger" }, - "minItems": { "$ref": "#/definitions/nonNegativeIntegerDefault0" }, - "uniqueItems": { - "type": "boolean", - "default": false - }, - "contains": { "$ref": "#" }, - "maxProperties": { "$ref": "#/definitions/nonNegativeInteger" }, - "minProperties": { "$ref": "#/definitions/nonNegativeIntegerDefault0" }, - "required": { "$ref": "#/definitions/stringArray" }, - "additionalProperties": { "$ref": "#" }, - "definitions": { - "type": "object", - "additionalProperties": { "$ref": "#" }, - "default": {} - }, - "properties": { - "type": "object", - "additionalProperties": { "$ref": "#" }, - "default": {} - }, - "patternProperties": { - "type": "object", - "additionalProperties": { "$ref": "#" }, - "propertyNames": { "format": "regex" }, - "default": {} - }, - "dependencies": { - "type": "object", - "additionalProperties": { - "anyOf": [ - { "$ref": "#" }, - { "$ref": "#/definitions/stringArray" } - ] - } - }, - "propertyNames": { "$ref": "#" }, - "const": true, - "enum": { - "type": "array", - "items": true, - "minItems": 1, - "uniqueItems": true - }, - "type": { - "anyOf": [ - { "$ref": "#/definitions/simpleTypes" }, - { - "type": "array", - "items": { "$ref": "#/definitions/simpleTypes" }, - "minItems": 1, - "uniqueItems": true - } - ] - }, - "format": { "type": "string" }, - "contentMediaType": { "type": "string" }, - "contentEncoding": { "type": "string" }, - "if": { "$ref": "#" }, - "then": { "$ref": "#" }, - "else": { "$ref": "#" }, - "allOf": { "$ref": "#/definitions/schemaArray" }, - "anyOf": { "$ref": "#/definitions/schemaArray" }, - "oneOf": { "$ref": "#/definitions/schemaArray" }, - "not": { "$ref": "#" } - }, - "default": true -} diff --git a/app/lib/three_scale/policies/specification.rb b/app/lib/three_scale/policies/specification.rb index e23b2c040e..7bdab39386 100644 --- a/app/lib/three_scale/policies/specification.rb +++ b/app/lib/three_scale/policies/specification.rb @@ -6,18 +6,54 @@ class Specification def initialize(doc) @errors = ActiveModel::Errors.new(self) @doc = doc - @validator = JSONValidator.new(doc) end attr_reader :errors, :doc - JSON_SCHEMA = {'$ref' => 'http://apicast.io/policy-v1/schema#'}.freeze + SCHEMAS_PATH = "app/lib/three_scale/policies/schemas/" + POLICY_SCHEMAS_FILENAMES = %w[apicast-policy-v1.1.schema.json apicast-policy-v1.schema.json].freeze + DEFAULT_POLICY_SCHEMA_ID = "http://apicast.io/policy-v1.1/schema" + + POLICY_SCHEMAS = POLICY_SCHEMAS_FILENAMES.each_with_object({}) do |schema_filename, schemas| + policy_schema = JSON.parse(File.read(File.join(SCHEMAS_PATH, schema_filename))) + id = policy_schema["$id"] + schemas[id] = JSONSchemer.schema(policy_schema) if id + end.freeze def valid? return false if errors.any? - @validator.fully_validate(JSON_SCHEMA).each { |error| errors.add(:base, error) } + + schemer = POLICY_SCHEMAS[schema_id] + unless schemer + errors.add(:base, "unsupported schema") + return false + end + + validate(schemer) + errors.empty? end + + private + + def validate(schemer) + schemer&.validate(doc).to_a.map { _1.fetch('error') }.each do |error| + errors.add(:base, error) + end + rescue JSONSchemer::UnknownRef => exception + errors.add(:base, "unknown ref: #{exception.message}") + end + + def schema_id + @schema_id ||= begin + schema = doc&.[]("$schema") + # Stripping URL fragment for compatibility. Previously, the schema ID was defined as "http://apicast.io/policy-v1/schema#manifest", + # or even "http://apicast.io/policy-v1/schema#manifest#" + # After changing the validator to `json_schemer`, the schema ID is now defined as "http://apicast.io/policy-v1/schema" + # to avoid issues with the existing schemas in the database or those coming from APIcast + schema.present? ? UriUtils.strip_fragment(schema.sub(/\#$/, '')) : DEFAULT_POLICY_SCHEMA_ID + end + end end end end diff --git a/app/lib/uri_utils.rb b/app/lib/uri_utils.rb new file mode 100644 index 0000000000..0166f21ab1 --- /dev/null +++ b/app/lib/uri_utils.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module UriUtils + module_function + + def strip_fragment(uri_string) + new_uri = URI(uri_string) + new_uri.fragment = nil + new_uri.to_s + rescue URI::InvalidURIError + nil + end +end diff --git a/doc/active_docs/policy_registry_api.json b/doc/active_docs/policy_registry_api.json index de5a3b56ef..cc5e7c99d9 100644 --- a/doc/active_docs/policy_registry_api.json +++ b/doc/active_docs/policy_registry_api.json @@ -92,7 +92,7 @@ "description": "ID of the policy. It can be an integer value or a combination 'name-version' of the policy (e.g. 'mypolicy-1.0')", "required": true, "schema": { - "type": "integer" + "type": "string" } } ], diff --git a/test/fixtures/policies/apicast-policy.json b/test/fixtures/policies/apicast-policy.json index 8fc279d39a..524a11fa07 100644 --- a/test/fixtures/policies/apicast-policy.json +++ b/test/fixtures/policies/apicast-policy.json @@ -1,5 +1,5 @@ { - "$schema": "http://apicast.io/policy-v1/schema#manifest#", + "$schema": "http://apicast.io/policy-v1/schema", "name": "Upstream", "summary": "Allows to modify the upstream URL of the request based on its path.", "description": diff --git a/test/unit/policy_test.rb b/test/unit/policy_test.rb index 665e006e1b..7aab41a3aa 100644 --- a/test/unit/policy_test.rb +++ b/test/unit/policy_test.rb @@ -59,7 +59,7 @@ class PolicyTest < ActiveSupport::TestCase policy.schema = '{"foo": "bar"}' refute policy.valid? - assert_includes policy.errors[:schema], 'The property \'#/\' did not contain a required property of \'name\' in schema http://apicast.io/policy-v1/schema#' + assert_includes policy.errors[:schema], "object at root is missing required properties: name, version, configuration, summary" policy.schema = file_fixture('policies/apicast-policy.json').read policy.version = policy.schema['version'] diff --git a/test/unit/three_scale/json_validator_test.rb b/test/unit/three_scale/json_validator_test.rb index 9297a3dee4..a32aeffb83 100644 --- a/test/unit/three_scale/json_validator_test.rb +++ b/test/unit/three_scale/json_validator_test.rb @@ -16,17 +16,15 @@ class ThreeScale::JSONValidatorTest < ActiveSupport::TestCase test 'extract the schema draft' do assert_equal 4, @klass.send(:schema_draft, { '$schema' => 'http://json-schema.org/draft-04/schema#' }) - assert_equal 7, @klass.send(:schema_draft, { '$schema' => 'http://json-schema.org/draft-07/schema#' }) assert_equal 0, @klass.send(:schema_draft, {}) end test 'extract the schema id' do assert_equal 'my-id', @klass.send(:schema_id, { 'id' => 'my-id', '$schema' => 'http://json-schema.org/draft-04/schema#' }) - assert_equal 'my-id', @klass.send(:schema_id, { '$id' => 'my-id', '$schema' => 'http://json-schema.org/draft-07/schema#' }) end test 'schema build' do - schema_json = { '$id' => 'http://example.com/schema-v1/my-schema#frag', '$schema' => 'http://json-schema.org/draft-07/schema#' } + schema_json = { 'id' => 'http://example.com/schema-v1/my-schema#frag', '$schema' => 'http://json-schema.org/draft-04/schema#' } schema = @klass.build_schema schema_json assert_kind_of JSON::Schema, schema end diff --git a/test/unit/three_scale/policies/specification_test.rb b/test/unit/three_scale/policies/specification_test.rb index 1efc5d37ac..b63432022a 100644 --- a/test/unit/three_scale/policies/specification_test.rb +++ b/test/unit/three_scale/policies/specification_test.rb @@ -12,7 +12,32 @@ class ThreeScale::Policies::SpecificationTest < ActiveSupport::TestCase test 'valid? returns false for an invalid specification and errors returns why' do specification = ThreeScale::Policies::Specification.new({"foo" => "bar"}) - refute specification.valid?, 'specification should not be valid' - assert_includes specification.errors[:base], 'The property \'#/\' did not contain a required property of \'name\' in schema http://apicast.io/policy-v1/schema#' + assert_not specification.valid?, 'specification should not be valid' + assert_includes specification.errors[:base], "object at root is missing required properties: name, version, configuration, summary" + end + + test "use the default policy schema if the document doesn't include the schema ID" do + doc = JSON.parse(file_fixture('policies/apicast-policy.json').read) + doc.delete("$schema") + + specification = ThreeScale::Policies::Specification.new(doc) + assert specification.valid?, 'specification should be valid' + end + + test "unsupported schema" do + doc = JSON.parse(file_fixture('policies/apicast-policy.json').read) + doc["$schema"] = "invalid" + + specification = ThreeScale::Policies::Specification.new(doc) + assert_not specification.valid?, 'specification should not be valid' + assert_includes specification.errors[:base], "unsupported schema" + end + + test "support policies with #manifest in schema ID" do + doc = JSON.parse(file_fixture('policies/apicast-policy.json').read) + doc["$schema"] = "http://apicast.io/policy-v1/schema#manifest#" + + specification = ThreeScale::Policies::Specification.new(doc) + assert specification.valid?, 'specification should be valid' end end