eclipse / microprofile-open-api

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

Support generation of empty SecurityRequirement via annotation #483

Closed patrick-vonsteht closed 2 years ago

patrick-vonsteht commented 3 years ago

Hi,

What do I want to achieve?

I use the smallrye-openapi implemenation of the microprofile-open-api specification to generate an openapi.json from code. I want to achieve to following output in the openapi.json:

"security" : [ 
  { "ApiKeyAuth" : [ ] }, 
  { "BearerTokenAuth" : [ ] }, 
  { }
]

Why do I need this?

This kind of specification is needed to achieve optional authentication as per the OpenAPI specification. See here https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#openapi-object, in the description for the security element:

"A declaration of which security mechanisms can be used across the API. The list of values includes alternative security requirement objects that can be used. Only one of the security requirement objects need to be satisfied to authorize a request. Individual operations can override this definition. To make security optional, an empty security requirement ({}) can be included in the array."

What do I currently have?

I can add the following annotations in my code to get the entries for "ApiKeyAuth" and "BearerTokenAuth":

@SecurityScheme(securitySchemeName = "ApiKeyAuth", description = "ApiKey Authentification",
        type = SecuritySchemeType.APIKEY, apiKeyName = "apikey", in = SecuritySchemeIn.HEADER)
@SecurityScheme(securitySchemeName = "BearerTokenAuth", description = "JWT Bearer Token Authentification",
        type = SecuritySchemeType.HTTP,  scheme = "bearer", bearerFormat = "JWT")
@SecurityRequirement(name = "ApiKeyAuth")
@SecurityRequirement(name = "BearerTokenAuth")

What do I need?

I also need to add the empty security requirement, but there doesn't seem to be a way to add it through annotations. Particularly, I cannot add a SecurityRequirement without any arguments, because the name parameter is mandatory. So this doesn't work:

@SecurityRequirement()

Am I missing something, or is this a missing feature?

What is my current workaround?

Currently I implemented a custom OASFilter that adds the empty security requirement:

public class CustomOASFilter implements OASFilter {

    @Override
    public Operation filterOperation(Operation operation) {
        SecurityRequirement sec = new SecurityRequirementImpl();
        List<SecurityRequirement> secList = operation.getSecurity();
        List<SecurityRequirement> newSecList = secList != null ? new ArrayList<>(secList) : new ArrayList<>();
        newSecList.add(sec);
        operation.setSecurity(newSecList);
        return operation;
    }
}
MikeEdgar commented 3 years ago

Hi @patrick-vonsteht - your workaround is one of the two possibilities. The other would be to provide a static openapi.yaml file that includes the empty security requirement and would be merged with the result of the annotation scan. You are right that this isn't supported using only annotations with the current version.

MikeEdgar commented 2 years ago

Proposed solution for 3.1 is to specify a default name value of empty string. When both name and scopes are empty, the empty security requirement ({}) is used.

Azquelt commented 2 years ago

Should you already be able to represent this with @SecurityRequirementSet({})?

Assuming I'm understanding it correctly, @SecurityRequirement(name="foo") applied to an operation is the same as @SecurityRequirementSet({@SecurityRequirement(name="foo")}) ?

In that case, it would make sense for @SecurityRequirementSet with an empty array to mean what Patrick wants.

Edit: though I do see #468 reporting that it's not working properly, so we'd need to look at that first.

MikeEdgar commented 2 years ago

I take the empty object in the example to be an instance of a SecurityRequirement itself. I think the intent is that the user can specify multiple @SecurityRequirements (either in a @SecurityRequirements or @SecurityRequirementSet) and one of those entries is empty versus the list being empty.

patrick-vonsteht commented 2 years ago

Yes, as MikeEdgar says, the goal is to have multiple security requirements and one of those should be empty. This is needed to express optional authentication.

Azquelt commented 2 years ago

I take the empty object in the example to be an instance of a SecurityRequirement itself. I think the intent is that the user can specify multiple @SecurityRequirements (either in a @SecurityRequirements or @SecurityRequirementSet) and one of those entries is empty versus the list being empty.

I don't think that's quite right.

In the OpenAPI spec, both the OpenAPI object and the Operation object allow an array of Security Requirement Objects. These indicate alternative authentication options, only one of them needs to be satisfied.

In JSON, that might look like this, indicating that either api_key or petstore_auth can be used to authenticate

    {
      "security": [
        {
          "api_key": {}
        },
        {
          "petstore_auth": [
            "write:pets",
            "read:pets"
          ]
        }
      ]
    }

However, each Security Requirement Object can itself list multiple schemes. This indicates that multiple authentication schemes are required, i.e. all of them need to be satisfied.

In JSON, that might look like this, indicating that both api_key and petstore_auth must be used to authenticate:

    {
      "security": [
        {
          "api_key": {},
          "petstore_auth": [
            "write:pets",
            "read:pets"
          ]
        }
      ]
    }

In MP Open API, we have two annotations, @SecurityRequirement and @SecurityRequirementSet. Looking at the javadoc and the data that these annotations hold, I think

This would mean that the first example above could be represented by:

@SecurityRequirement(name = "api_key")
@SecurityRequirement(name = "petstore_auth", scopes = {"write:pets", "read:pets"})

or equivalently

@SecurityRequirementSet({@SecurityRequirement(name = "api_key")})
@SecurityRequirementSet({@SecurityRequirement(name = "petstore_auth", scopes = {"write:pets", "read:pets"})})

Whereas the second example above would be represented by:

@SecurityRequirementSet({
    @SecurityRequirement(name = "api_key"),
    @SecurityRequirement(name = "petstore_auth", scopes = {"write:pets", "read:pets"})
})

Patrick's requirement was this (formatted to match the other examples):

{
  "security" : [ 
    {
      "ApiKeyAuth" : [ ] 
    }, 
    {
      "BearerTokenAuth" : [ ] 
    }, 
    { }
  ]
}

This would be represented by

@SecurityRequirement(name = "ApiKeyAuth") // Security Requirement Object with one entry
@SecurityRequirement(name = "BearerTokenAuth") // Security Requirement Object with one entry
@SecurityRequirementSet() // Security Requirement Object with no entries

That doesn't mean we can't also have a different syntax to represent a Security Requirement Object with no entries, but I do think this case is covered by the existing annotations. Either way, we would want to add a TCK for this scenario.

Of course, I may have misunderstood something in either spec so please let me know if there's something I've missed or have interpreted wrongly.

MikeEdgar commented 2 years ago

@Azquelt - I follow you now and I agree. I think the confusion is that the security properties of @OpenAPIDefinition and @CallbackOperation are of type @SecurityRequirement[] whereas they probably should be of type @SecurityRequirementSet[]. I think the solution to #468 will help with that situation.

@SecurityRequirement(name = "ApiKeyAuth") // Security Requirement Object with one entry
@SecurityRequirement(name = "BearerTokenAuth") // Security Requirement Object with one entry
@SecurityRequirementSet() // Security Requirement Object with no entries

This one just doesn't jump out (to me) as the obvious way to use these annotations, but I do agree. I think it would be more clear if each @SecurityRequirement were wrapped in @SecurityRequirementSet as in your earlier example.

MikeEdgar commented 2 years ago

Just putting this here for reference. This issue is concerned with representing an empty Map<String, List<String>>, best accomplished with @SecurityRequirementSet({}) as previously described by Andrew.

Annotation Equivalent Type Comment
@SecurityRequirement Map.Entry<String, List<String>>
@SecurityRequirements List<Map<String, List<String>>> Maps are singletons
@SecurityRequirementSet Map<String, List<String>> Corresponds to SecurityRequirement model