Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial work on security schema (only jwt bearer) #143

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
6 changes: 6 additions & 0 deletions lib/openapi_parser/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -272,4 +272,10 @@ def message
"#{@reference} #{@value.inspect} contains fewer than min items"
end
end

class ValidateSecurityError < OpenAPIError
def message
"access denied"
end
end
end
14 changes: 10 additions & 4 deletions lib/openapi_parser/request_operation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ class << self
# @param [OpenAPIParser::Config] config
# @param [OpenAPIParser::PathItemFinder] path_item_finder
# @return [OpenAPIParser::RequestOperation, nil]
def create(http_method, request_path, path_item_finder, config)
def create(http_method, request_path, path_item_finder, config, security_schemes = {})
result = path_item_finder.operation_object(http_method, request_path)
return nil unless result

self.new(http_method, result, config)
self.new(http_method, result, config, security_schemes)
end
end

Expand All @@ -25,18 +25,23 @@ def create(http_method, request_path, path_item_finder, config)
# @return [String]
# @!attribute [r] path_item
# @return [OpenAPIParser::Schemas::PathItem]
attr_reader :operation_object, :path_params, :config, :http_method, :original_path, :path_item
attr_reader :operation_object, :path_params, :config, :http_method, :original_path, :path_item, :security_schemes

# @param [String] http_method
# @param [OpenAPIParser::PathItemFinder::Result] result
# @param [OpenAPIParser::Config] config
def initialize(http_method, result, config)
def initialize(http_method, result, config, security_schemes)
@http_method = http_method.to_s
@original_path = result.original_path
@operation_object = result.operation_object
@path_params = result.path_params || {}
@path_item = result.path_item_object
@config = config
@security_schemes = security_schemes
end

def validate_security(security_schemes, headers)
operation_object.validate_security_schemes(security_schemes, headers)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OpenAPI Specification define that it is OK if either one of OpenAPI Object or Operation Object's security requirement is satisfied, so need to check both security fields.
https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#securityRequirementObject

When a list of Security Requirement Objects is defined on the OpenAPI Object or Operation Object, only one of the Security Requirement Objects in the list needs to be satisfied to authorize the request.

Now RequestOperation Object doesn't have OpenAPI's security field so we need pass it too.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is good to create validator which require Security Scheme Object and Security Requriement Object and validate seurity schemes specified by requirement objects for validating OpenAPI Object and Operation Object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ota42y I don't understand this part:

I think it is good to create validator which require Security Scheme Object and Security Requriement Object

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are adding the validation method to the Security Requriement Object, that's fine. I was thinking of using Security Scheme Object for validation and thought that would be a problem. Ignore it.

end

def validate_path_params(options = nil)
Expand Down Expand Up @@ -65,6 +70,7 @@ def validate_response_body(response_body, response_validate_options = nil)
# @param [OpenAPIParser::SchemaValidator::Options] options request validator options
def validate_request_parameter(params, headers, options = nil)
options ||= config.request_validator_options
validate_security(security_schemes, headers)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now users are using code under the assumption that this validation will not take place, and it may suddenly break at update time.
Therefore, it would be better to control this optionally or make it a separate method and have it called separately.

operation_object&.validate_request_parameter(params, headers, options)
end

Expand Down
2 changes: 2 additions & 0 deletions lib/openapi_parser/schemas.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@
require_relative 'schemas/media_type'
require_relative 'schemas/schema'
require_relative 'schemas/header'
require_relative 'schemas/security_schemes'
require_relative 'schemas/info'
require_relative 'schemas/security'
2 changes: 2 additions & 0 deletions lib/openapi_parser/schemas/classes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,7 @@ class MediaType < Base; end
class Schema < Base; end
class Components < Base; end
class Header < Base; end
class SecuritySchemes < Base; end
class Info < Base; end
class Security < Base; end
end
4 changes: 4 additions & 0 deletions lib/openapi_parser/schemas/components.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,9 @@ class Components < Base
# @!attribute [r] headers
# @return [Hash{String => Header}, nil] header objects
openapi_attr_hash_object :headers, Header, reference: true

# @!attribute [r] security_schemes
# @return [Hash{String => Header}, nil]
openapi_attr_hash_object :security_schemes, SecuritySchemes, reference: true, schema_key: :securitySchemes
end
end
2 changes: 1 addition & 1 deletion lib/openapi_parser/schemas/openapi.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def initialize(raw_schema, config, uri: nil, schema_registry: {})

# @return [OpenAPIParser::RequestOperation, nil]
def request_operation(http_method, request_path)
OpenAPIParser::RequestOperation.create(http_method, request_path, @path_item_finder, @config)
OpenAPIParser::RequestOperation.create(http_method, request_path, @path_item_finder, @config, components&.security_schemes)
end

# load another schema with shared config and schema_registry
Expand Down
29 changes: 28 additions & 1 deletion lib/openapi_parser/schemas/operation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ class Operation < Base
openapi_attr_values :tags, :summary, :description, :deprecated

openapi_attr_value :operation_id, schema_key: :operationId

openapi_attr_list_object :parameters, Parameter, reference: true

openapi_attr_list_object :security, Security, reference: false

# @!attribute [r] request_body
# @return [OpenAPIParser::Schemas::RequestBody, nil] return OpenAPI3 object
openapi_attr_object :request_body, RequestBody, reference: true, schema_key: :requestBody
Expand All @@ -30,5 +32,30 @@ def validate_request_body(content_type, params, options)
def validate_response(response_body, response_validate_options)
responses&.validate(response_body, response_validate_options)
end

def validate_security_schemes(securitySchemes, headers)
validate_results = security.map do |s| # s is security requirement object
# check all security
s.validate_security_requirements(securitySchemes, headers)
end
if validate_results.count(true) == 1
return true # accept
end
return OpenAPIParser::ValidateSecurityError

# securitySchemes&.each do |securityScheme|
# # check if the endpoint has security in schema
# if security
# # if security exists, check what securitySchemas used for enforcing
# security.each do |s|
# # securityScheme[0] is the securitySchema name
# if s == securityScheme[0]
# # securitySchema[1] is the values like "type", "scheme" and bearerFormat
# securityScheme[1].validate_security_schemes(securityScheme[1], headers)
# end
# end
# end
# end
end
end
end
7 changes: 7 additions & 0 deletions lib/openapi_parser/schemas/security.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module OpenAPIParser::Schemas
class Security < Base

def validate_security_requirements(securityScheme, headers)
end
end
end
20 changes: 20 additions & 0 deletions lib/openapi_parser/schemas/security_schemes.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
require 'jwt'
module OpenAPIParser::Schemas
class SecuritySchemes < Base

openapi_attr_values :type, :description, :scheme
openapi_attr_value :bearer_format, schema_key: :bearerFormat

def validate_security_schemes(securityScheme, headers)
if self.type == "http" && self.scheme == "bearer" && self.bearer_format == "JWT"
raise "need authorization" unless headers["AUTHORIZATION"]
raise "not bearer" unless headers["AUTHORIZATION"].split[0] == "Bearer"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


# check if the JWT token is being sent and try to decode.
# if JWT token does not exist or token cannot decode, then deny access
token = headers["AUTHORIZATION"].split[1]
JWT.decode token, nil, false
end
end
end
end
2 changes: 2 additions & 0 deletions openapi_parser.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ Gem::Specification.new do |spec|
spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) }
spec.require_paths = ['lib']

# production
spec.add_dependency 'jwt', '~> 2.5'
spec.add_development_dependency 'bundler', '>= 1.16'
spec.add_development_dependency 'fincop'

Expand Down
5 changes: 5 additions & 0 deletions spec/data/petstore-expanded.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,11 @@ paths:
$ref: '#/components/schemas/Pet'

components:
securitySchemes:
jwt:
type: "http"
scheme: "bearer"
bearerFormat: "JWT"
parameters:
test:
name: limit
Expand Down
2 changes: 2 additions & 0 deletions spec/openapi_parser/schemas/components_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
expect(subject.request_bodies['test_body'].class).to eq OpenAPIParser::Schemas::RequestBody
expect(subject.request_bodies['test_body'].object_reference).to eq '#/components/requestBodies/test_body'

expect(subject.security_schemes['jwt'].class).to eq OpenAPIParser::Schemas::SecuritySchemes

expect(subject.headers['X-Rate-Limit-Limit'].class).to eq OpenAPIParser::Schemas::Header
end
end
Expand Down