eclipse / microprofile-open-api

Microprofile open api
Apache License 2.0
131 stars 82 forks source link

@SecurityRequirementsSet not working #468

Closed kieltom closed 2 years ago

kieltom commented 3 years ago

I'm working on switching openapi v2 to openapi3 , we'll be using microprofile 2.0 with smallrye as the plugin that will generate the OAS from the code.

Now, we have different headers that are needed as a kind of authorization. I know i can put them on each resource, that seems just a lot of repetition, so i thought it was a good idea to put in the JaxRSConfiguration file as a SecurityScheme and add them as security to the OpenApiDefinition

@OpenAPIDefinition(
    info = @Info(
            ...
            )
    ),
    security = {
       @SecurityRequirement(name = Header1),
       @SecurityRequirement(name = Header2)
           },
    components = @Components(
             securitySchemes = {
                    @SecurityScheme(  apiKeyName = Header1 ... ) ,
                    @SecurityScheme(  apiKeyName = Header2 ....) 
    }),
    ...
    )

This is working, however it generates an OR, while it should be an AND (without the - )

security:
  - Header1: []
  - Header2: []

@SecurityRequirementsSet solves this problem, although i cannot use it somewhere in the @OpenApiDefinition

Is this a bug or is this intended as is.

Thx

MikeEdgar commented 3 years ago

For reference, this is about using multiple auth types for the API: https://swagger.io/docs/specification/authentication/#multiple .

@kieltom - I agree this seems like something that needs to be addressed with support in @OpenAPIDefinition, OpenAPI, and even Operation types. At the moment, I do not believe the @SecurityRequirementsSet is supported by the model, even though the annotation is in the API.

@phillip-kruger , @EricWittmann , @msmiths - please take a second look and let me know if I've missed something. If not, this might be a good candidate for the next minor release of the specification.

kieltom commented 3 years ago

Yes indeed, this is for using multiple auth types.

Thx for looking into it.

MikeEdgar commented 3 years ago

What are others' thoughts on this? I think there are a few options:

  1. Change the type of OpenAPIDefinition#security to SecurityRequirementsSet[]. This would be a breaking change for all applications using the property.
  2. Add new property to OpenAPIDefinition (e.g. securitySet) to support SecurityRequirementsSet[]. Not breaking for applications, but somewhat redundant to security. Anything you could do with security could also be done with securitySet.
  3. Leave OpenAPIDefinition as-is, but make SecurityRequirementsSet repeatable and support its use on Application classes, package-info, resource classes, and operation methods. This may still be confusing as well, since security would still be present on OpenAPIDefinition.

There may be others, but that's all I can think of at the moment.

MikeEdgar commented 2 years ago

I think an option for 3.1 is item 3 in the previous comment's list with the addition of also supporting @SecurityRequirements/@SecurityRequirement in the same locations consistently and also deprecating @OpenAPIDefinition#security with guidance to annotate the Application directly. This avoids making @OpenAPIDefinition bigger.

Azquelt commented 2 years ago

I think an option for 3.1 is item 3 in the previous comment's list with the addition of also supporting @SecurityRequirements/@SecurityRequirement in the same locations consistently and also deprecating @OpenAPIDefinition#security with guidance to annotate the Application directly. This avoids making @OpenAPIDefinition bigger.

I think this makes sense, though I think @CallbackOperation also needs a way to accept a list of @SecurityRequirementSet

MikeEdgar commented 2 years ago

Another option may be a mix of 2 and 3. Add securitySet to @CallbackOperation and @OpenAPIDefinition, and also support @SecurityRequirementSet (repeatable) wherever @SecurityRequirement is supported. Bottom line is that both would be supported in the same way.