OpenAPI.NET icon indicating copy to clipboard operation
OpenAPI.NET copied to clipboard

No difference between no security requirement and empty security requirement on Operations

Open VisualBean opened this issue 2 years ago • 4 comments

Describe the bug There is no way of telling 'empty' from 'not defined' security requirement objects, during deserialization. I.e no way of distinguishing the 2 different security requirement objects

paths:
  /pets:
    post:
      responses:
        '201':
          description: Created
      security: []
    get:
      responses:
        '200':
          description: A paged array of pets

Why is this important?

The specification states the following for Operation Security

To remove a top-level security declaration, an empty array can be used.

i.e security: []

however it is expected that if security is not defined on an operation level

get:
  responses:
    '200':
      description: A paged array of pets

That the global security is applied, defined on the top level document object.

in the following spec

openapi: '3.0.0'
info:
  version: 1.0.0
  title: Swagger Petstore
  license:
    name: MIT
security:
  - Authorization:
    - readwrite
servers:
  - url: http://petstore.swagger.io/v1
paths:
  /pets:
    post:
      responses:
        '201':
          description: Created
      security: []
    get:
      responses:
        '200':
          description: A paged array of pets
components:
  securitySchemes:
    Authorization:
      type: http
      scheme: bearer
      bearerFormat: JWT

post deserialization, it is impossible to determine which security is actually applied between the 2 operations, as both security requirements are deserialized into the same value public IList<OpenApiSecurityRequirement> Security { get; set; } = new List<OpenApiSecurityRequirement>(); which ends up being empty in both cases.

The reason for this is most likely that during reading a new OpenApiSecurityRequirement() and as this is a dictionary, there is no way of specifying a null key, so to speak.

The problem being that the semantics between the 2 operations are different in the spec.

To Reproduce Deserialize the above spec.

Expected behavior To have a way of differentiating the 2 scenarios. (feasibly one would be empty, the other would be null.

Screenshots/Code Snippets

var input = @"openapi: '3.0.0'
info:
  version: 1.0.0
  title: Swagger Petstore
  license:
    name: MIT
security:
  - Authorization:
    - readwrite
servers:
  - url: http://petstore.swagger.io/v1
paths:
  /pets:
    post:
      responses:
        '201':
          description: Created
      security: []
    get:
      responses:
        '200':
          description: A paged array of pets
components:
  securitySchemes:
    Authorization:
      type: http
      scheme: bearer
      bearerFormat: JWT";
var document = new OpenApiStringReader().Read(input, out var diagnostics);

VisualBean avatar Oct 11 '23 09:10 VisualBean

Thank you for bringing this to our attention. We have two options here to solve this problem. We can either add a new AnonymousAccess property to the OpenApiOperation object to allow users to explicitly state they want to override the security with no security. Or the other option is to stop defaulting the Security property to an empty list and treat a non-null value as an indication that they want to override the security.


// Option 1 - Breaking change
var operation = new OpenApiOperation();
operation.Security = null;  // Default value

// Set Anonymous access
operation.Security = new List<OpenApiSecurityRequirement>() [];  


// Option 2 - Non breaking change
var operation = new OpenApiOperation();
operation.Security = new List<OpenApiSecurityRequirement>() [];  // Default value
operation.AnonymousAccess = false;  // Anonymous access default

// Set Anonymous access
operation.AnonymousAccess = true;  

We can do Option 2 in the current major version of OpenAPI.NET because it is not breaking, however, if we go with Option 1 we will need to wait until the v2 release.

darrelmiller avatar Oct 17 '23 13:10 darrelmiller

Personally, I think nulling it, is more in line with what is expected from a dotnet point of view.

  • We specifically found empty vs we found nothing. so to speak.

Edited: to state the desired option without option number.

~~Option 1 does have the added effect of it, then being the ONLY list, that isn't newed up automatically. also it 'might' technically be a breaking change for everyone relying on the existing behaviour. So waiting for v2 might be the safest bet, in any case.~~

VisualBean avatar Oct 17 '23 14:10 VisualBean

I think option 1 and 2 have been flipped around, from the description to the examples: Option 1 adding AnonymousAccess, Option 2 defaulting to null.

Defaulting to null is the breaking change, so you both seem in agreement to wait for a v2 with it.

Null vs. default is similar to how it would be treated in F# (to my understanding) with option, where Some [] is the explicit override, and None is the lack of the property. The None tends to be mapped from null. I'd feel more comfortable with this option 2, explicit null, rather than a new property whose name I can't see mapping to the OpenAPI spec itself.

crudbee avatar Oct 18 '23 07:10 crudbee

Hello,

Any update on this issue ? Is there a known workaround other than not using global security specifications ?

Thanks

corentinvds avatar Oct 17 '24 06:10 corentinvds

Hi! Has a decision been reached on how to best proceed? Returning null when no security property is set on the operation?

There seems to be a v2 version on it's way as preview5 has been released recently. I might be interested in contributing a fix (since I need a fix 😬) but then it's good to be sure what the approach should be.

paulmendix avatar Feb 05 '25 14:02 paulmendix

@paulmendix We would be most appreciative of a PR. However, this work should be only done after we have completed the NRT work #1971 and #2146

darrelmiller avatar Mar 12 '25 16:03 darrelmiller

Thanks for the update! I will keep an eye on these issues

paulmendix avatar Apr 06 '25 19:04 paulmendix

@paulmendix both the NRT and perf work are complete if you'd still like to contribute by sending a PR our way.🙂

MaggieKimani1 avatar Apr 11 '25 09:04 MaggieKimani1

Thanks @MaggieKimani1 I will give it a go

paulmendix avatar Apr 11 '25 12:04 paulmendix

Having silently watched in on this for over a year, thank you very much for seeing this through to main :D

crudbee avatar May 13 '25 08:05 crudbee