swagger-core icon indicating copy to clipboard operation
swagger-core copied to clipboard

Multiple SecurityRequirements annotations with AND condition

Open miloszwatroba opened this issue 5 years ago • 6 comments

Hi! I found a problem with adding multiple security requirements via annotations. I see that there is an old, closed ticket #1261, which was concerning this issue. Although the ticket was closed, I think that the issue was not resolved. I see that the possibility to add multiple requirements with "AND" condition was added to the swagger-models, whereas the ticket was actually concerning swagger-annotations.

I would like to bring it up again. I've just looked through the documentation and code, and it seems that there is no possibility to add multiple SecurityRequirement annotations and tie them with an "AND" condition. By default, the requirements added within that annotation are generated with an "OR" conjunction.

We can see that in SecurityRequirements.java, there is only an array of SecurityRequirement allowed, which in turn has only name and scopes parameters. This is not in line with the model, where the SecurityRequirement is actually a Map of names and scopes, which allows for the "AND" condition. Therefore, I don't see any option to use the annotations with an "AND" condition.

Am I missing something? Is there a reason for the fact that the swagger-annotations lack this functionality?

miloszwatroba avatar May 08 '20 13:05 miloszwatroba

Would a PR to to address this be welcome? It might not be possible to change this while maintaining backwards compatibility.

karlvr avatar Apr 09 '21 03:04 karlvr

@karlvr thanks for looking into this! I am not sure though what the problem is about.

You can define multiple security requirements by using multiple @SecurityRequirement annotations which will be resolved into multiple SecurityRequirement entries in the Operation.security map.

Can you possibly clarify the use case?

frantuma avatar Apr 09 '21 10:04 frantuma

@frantuma thank you!

This is a short journey... it starts at https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#operation-object and scroll down to security.

... 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.

But note that the array is an array of https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#securityRequirementObject where that is actually a dictionary of security schemes and the required scopes (if any).

The annotations allow us to specify an array of security schemes, which is actually just the second part (immediately above), and not the first part where we can specify an array of alternative collections of security schemes.

It's a pretty subtle distinction in the spec that's easy to miss. I wondered for a while why the Security Requirement Object was a dictionary rather than having properties scheme and scopes. And this is why.

Also note from the security property on operation:

To make security optional, an empty security requirement ({}) can be included in the array.

I think this is also something that's often missed based on my experience of having missed it, gone looking for it, and then found it.

karlvr avatar Apr 09 '21 23:04 karlvr

@karlvr Thanks a lot, journey has been enlightening! So yes, PRs are more than welcome, maybe a way to get this is without breaking compatibility could be adding an annotation like (stripping javadocs, and maybe with some better naming..):

@Target({ANNOTATION_TYPE})
@Retention(RetentionPolicy.RUNTIME)
public @interface SecurityRequirementEntry {
    String name();
    String[] scopes() default {};
}

and have it added to SecurityRequirement to be used in alternative to the "single key version":

public @interface SecurityRequirement {
    String name();
    String[] scopes() default {};

	SecurityRequirementEntry[] entries() default {};
}

please let me know what you think and thanks for looking into this.

frantuma avatar Apr 10 '21 09:04 frantuma

After thinking a bit about your solution and its pros and cons, I have to agree that it's the most reasonable one.

Have you submitted it already as a PR?

amorozov avatar Jul 13 '22 01:07 amorozov

@amorozov Thanks for looking into this and nope, no PRs yet

frantuma avatar Jul 13 '22 07:07 frantuma