seriousme / fastify-openapi-glue

A plugin for the Fastify webserver to autogenerate a Fastify configuration based on a OpenApi(v2/v3) specification.
MIT License
196 stars 33 forks source link

and connected security requirements not possible. #546

Closed FalkNisius closed 6 months ago

FalkNisius commented 6 months ago

in the definition of security requirements object is defined, that it can be reference more than one security scheme. All referenced schemes must be satisfied.

As example two schemes:

components:
  securitySchemes:
    apiTenant: 
      type: apiKey
      in: header       
      name: X-ORGANIZATION
    bearerAuth:
      type: http
      scheme: bearer
      bearerFormat: b64token

And a path definition using both and connected:

paths:
  /example:
    get:
      security:
        - apiTenant: []
          bearerAuth: ['used:scope']

The add schemes function in SecurityHandlers fetched only the first found security scheme, and not all.

As workaround it is possible to call the method for the second scheme from the method of the first scheme, but I think it is not intented from the view of the openapi specification.

Thanks for the fantastic work.

FalkNisius commented 6 months ago

in the ParserBase the parseSecurity method takes only the first property (Object.keys(item)[0]) and not the complete object. In my opinion the extracted name shoul also be checked against the securitySchemes definition if it exists.

seriousme commented 6 months ago

Hi,

thanks for asking !

I checked the specification of the OpenApi securityRequirementObject And it says:

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.

Therefore the plugin keeps on checking security requirements until the first one succeeds. See the following test: https://github.com/seriousme/fastify-openapi-glue/blob/1bb7e15778e95f8f8e743826d3fd5c9c21700d42/test/test-securityHandlers.v3.js#L74-L84 (please ignore the labels, its about the goodAuthCheck and failingAuthCheck )

So the only way to circumvent this behaviour is indeed to chain the checks in the handlers themselves as you described.

Does this help ?

Kind regards, Hans

FalkNisius commented 6 months ago

Your quote from the spec is right, but the wrong part. If I specify

  security:
     - apiTenant: []
     - bearerAuth: ["some:scope"]

You are complete right, and the plugin does what it's supposed to do.

If I specify

  security:
     - apiTenant: []
       bearerAuth: ["some:scope"]

or in i a more JSON style

  security: [{"apiTenant": [], "bearerAuth": ["some:scope"]}]

Than the security requirement object contains two schemes apiTenant and bearerAuth and both are combined with AND. I quote the paragraph above at the same place in the spec.

Security Requirement Objects that contain multiple schemes require that all schemes MUST be satisfied for a request to be authorized. This enables support for scenarios where multiple query parameters or HTTP headers are required to convey security information.

The parser fetched only the first scheme of the security requirements object.

Doe's this help?

Kind Regards, Falk

seriousme commented 6 months ago

Yes it does, thanks for highlighting ! Sounds indeed like a bug, I will look into it.

Kind regards, Hans

seriousme commented 6 months ago

The fix is in https://github.com/seriousme/fastify-openapi-glue/pull/548 which results in more clean code for this functionality. Thanks for raising the issue!

Released as 4.4.3 on NPM.

Can you please check your use case ?

Kind regards, Hans

FalkNisius commented 6 months ago

the fix worked in my small test case as expected.

Many thanks for the fast help Falk

seriousme commented 6 months ago

Closing the issue now, feel free to reopen or open a new issue should you encounter anything else.

Kind regards, Hans