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

muhammadn · 2023-02-15T15:29:58Z

No description provided.

ota42y

Thanks for adding the feature!
Certainly supporting only JWT to begin with is a very good start.

lib/openapi_parser/schemas/security_schemes.rb

lib/openapi_parser/schemas/components.rb

muhammadn · 2023-02-21T20:50:29Z

@ota42y You can clone my branch. It is almost complete but problem with the MyBearerToken which can be any name in comment above.

Update: This is done! 🎉

muhammadn · 2023-02-21T23:34:23Z

@ota42y Mostly done. I will figure out how to get the headers to check for a JWT token, or if it is a valid token.

I've done some others, i'm very appreciative of your time you took to answer me, 🙇‍♂️, so i contributed some PRs for other stuff that is missing (servers is done, info is not that important but it's simple so i decided to do it)

muhammadn

I've finished it including tests.

ota42y

Thank you!!! I think it is a very good code.
Some of the validation was not up to OpenAPI Specification, so that needs to be fixed, but I think we're almost there!

ota42y · 2023-02-23T05:48:34Z

lib/openapi_parser/schemas/security_schemes.rb

+    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"


Please define new error class.
https://github.com/ota42y/openapi_parser/blob/master/lib/openapi_parser/errors.rb

ota42y · 2023-02-23T06:37:25Z

lib/openapi_parser/request_operation.rb

+  end
+
+  def validate_security(security_schemes, headers)
+    operation_object.validate_security_schemes(security_schemes, headers)


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.

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.

@ota42y I don't understand this part:

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

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.

ota42y · 2023-02-23T06:57:33Z

lib/openapi_parser/schemas/operation.rb

@@ -30,5 +30,21 @@ 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)
+      securitySchemes&.each do |securityScheme|


We must checks for a Security Scheme Object with a corresponding name if there is a SecurityRequriment Object so we don't need check all securitySchemes.

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

I have determined that the validate passes if only one security scheme is valid, and if two or more are valid, the validate does not pass.
( We need check validate_results.count(true) == 1, not validate_results.count(true) >= 1 )
Is this OK? My English is not very good, so I doubt if I am reading it correctly.

Only one of the security requirement objects need to be satisfied to authorize a request.

@ota42y
your english is ok...! I have to understand, security requirement object does not have schema information about type, scheme and bearerFormat is only in SecuritySchema object.

So this code below cannot be used in security.rb which is security requirements object (the latest code that i just pushed)

# this code cannot be used in security.rb (security requirement object) def validate_security_schemes(securityScheme, headers) if self.type == "http" && self.scheme == "bearer" && self.bearer_format == "JWT" end end

@ota42y Hi. any updates? I'm thinking of doing my own fork because i cannot find the solution with:

def validate_security_schemes(securitySchemes, headers) validate_results = security.map do |s| # s is security requirement object # check all security s.validate_security_schemes(securitySchemes, headers) <-- this needs to be object, cannot use "self" with `securitySchemes end if validate_results.count(true) == 1 return true # accept end return ValidateSecurityError() end

security requirement object does not have schema information about type, scheme and bearerFormat is only in SecuritySchema object.

You are right. I confused security scheme object and secuirty requirement object 🙇

lib/openapi_parser/schemas/security_schemes.rb

ota42y · 2023-02-23T07:26:12Z

lib/openapi_parser/request_operation.rb

@@ -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)


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.

ota42y · 2023-02-23T08:12:27Z

lib/openapi_parser/schemas/security_schemes.rb

+        # 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]
+        value = JWT.decode token, nil, false


How much detail to verify here is a difficult question.
When verifying a signed JWT, the user expects the signature to be correct.
But we must passed key and various options to the library.
The options are numerous and require the same amount of effort as creating a wrapper for the jwt library.
Also, the OpenAPI definition does not expect JWT to have detailed information about signature verification.

One solution would be to have the user pass in a callback what validation is to be performed, and this library would only call the callback.
This would allow users to, for example, check for certain values in the JWT or force disabling of the JWT for some users only.

In this case, instead of limiting the bearerFormat=JWT, I think it would be possible to pass a Security Scheme Object to the callback so that users can validate non-JWT formats as well, thus allowing for a wider range of applications.

ota42y · 2023-03-02T14:56:10Z

Late reply, but I got back to you, thanks for pointing it out right!

muhammadn added 2 commits February 15, 2023 23:29

initial work on security schema (only jwt bearer)

5ffec5c

add more code for jwt token

ef238a1

muhammadn mentioned this pull request Feb 16, 2023

securityScheme support #144

Open

ota42y requested changes Feb 18, 2023

View reviewed changes

lib/openapi_parser/schemas/security_schemes.rb Outdated Show resolved Hide resolved

lib/openapi_parser/schemas/components.rb Outdated Show resolved Hide resolved

muhammadn added 6 commits February 20, 2023 15:32

add security

57c9be1

remove unnecessary code

eb9c334

Almost there...

93e0732

Almost there...

a8edef7

remove old file

6b7b75a

add more improvements

2cb7f30

update working code

fa56cee

muhammadn added 3 commits February 22, 2023 08:19

add jwt validation

fea000d

fix a bug

8a778a8

fix tests

f48001b

muhammadn commented Feb 22, 2023

View reviewed changes

ota42y requested changes Feb 23, 2023

View reviewed changes

muhammadn added 4 commits February 24, 2023 22:19

include jwt library

3c7cf65

merge files

3ff37b3

update code

616f0bc

cleanup

89b7c08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

muhammadn commented Feb 15, 2023

ota42y left a comment

muhammadn commented Feb 21, 2023 •

edited

Loading

muhammadn commented Feb 21, 2023 •

edited

Loading

muhammadn left a comment

ota42y left a comment

ota42y Feb 23, 2023

ota42y Feb 23, 2023

ota42y Feb 23, 2023

muhammadn Feb 27, 2023

ota42y Mar 2, 2023

ota42y Feb 23, 2023

muhammadn Feb 24, 2023 •

edited

Loading

muhammadn Feb 28, 2023 •

edited

Loading

ota42y Mar 2, 2023

ota42y Feb 23, 2023

ota42y Feb 23, 2023

ota42y commented Mar 2, 2023

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

Are you sure you want to change the base?

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

Conversation

muhammadn commented Feb 15, 2023

ota42y left a comment

Choose a reason for hiding this comment

muhammadn commented Feb 21, 2023 • edited Loading

muhammadn commented Feb 21, 2023 • edited Loading

muhammadn left a comment

Choose a reason for hiding this comment

ota42y left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muhammadn Feb 24, 2023 • edited Loading

Choose a reason for hiding this comment

muhammadn Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ota42y commented Mar 2, 2023

muhammadn commented Feb 21, 2023 •

edited

Loading

muhammadn commented Feb 21, 2023 •

edited

Loading

muhammadn Feb 24, 2023 •

edited

Loading

muhammadn Feb 28, 2023 •

edited

Loading