Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a way to generate an empty JSON security array for an operation #1819

Closed
jez9999 opened this issue Sep 16, 2020 · 19 comments
Closed

Comments

@jez9999
Copy link

jez9999 commented Sep 16, 2020

In order to mark operations as requiring authorization in a Swagger document, you can individually mark them one-by-one as requiring security:

services.AddSwaggerGen(options => {
    options.OperationFilter<SwaggerRequireAuth>(apiScope);
});
//...
internal class SwaggerRequireAuth : IOperationFilter {
    private readonly string _apiScope;

    public SwaggerNoAnonymousAuth(string apiScope) {
        _apiScope = apiScope;
    }

    public void Apply(OpenApiOperation operation, OperationFilterContext context) {
        bool isAnonymous = context.MethodInfo.GetCustomAttributes(true).OfType<AllowAnonymousAttribute>().Count() > 0;

        if (!isAnonymous) {
            operation.Security = new List<OpenApiSecurityRequirement> {
                new OpenApiSecurityRequirement {
                    {
                        // Scheme required to access all non-anonymous API operations
                        new OpenApiSecurityScheme {
                            Reference = new OpenApiReference { Type = ReferenceType.SecurityScheme, Id = "oauth2" }
                        },
                        // Scopes required to access all non-anonymous API operations
                        new[] { _apiScope }
                    }
                }
            };
        }
    }
}

However, this will rather bloat the Swagger JSON file by separately specifying the same security for every single operation:

"security": [
  {
    "oauth2": [
      "my-api-scope"
    ]
  }
]

If your API has only a few operations that are anonymous, and don't require authentication, with every other operation requiring the same authentication, the more efficient way to indicate this is to apply a global security requirement and then specify an empty JSON security array for the few operations that are anonymous:

services.AddSwaggerGen(options => {
    options.AddSecurityRequirement(new OpenApiSecurityRequirement {
        {
            // Scheme required to access all non-anonymous API operations
            new OpenApiSecurityScheme {
                Reference = new OpenApiReference { Type = ReferenceType.SecurityScheme, Id = "oauth2" }
            },
            // Scopes required to access all non-anonymous API operations
            new[] { apiScope }
        }
    });
});
//...
internal class SwaggerNoAnonymousAuth : IOperationFilter {
    private readonly string _apiScope;

    public SwaggerNoAnonymousAuth(string apiScope) {
        _apiScope = apiScope;
    }

    public void Apply(OpenApiOperation operation, OperationFilterContext context) {
        bool isAnonymous = context.MethodInfo.GetCustomAttributes(true).OfType<AllowAnonymousAttribute>().Count() > 0;

        if (isAnonymous) {
            operation.Security = new List<OpenApiSecurityRequirement>();
        }
    }
}

... to generate:

"security": []

At least, that's what I wish it would generate, which should mark the few anonymous methods I have as anonymous in the Swagger document. Unfortunately, setting .Security to an empty list removes the security element altogether, meaning the operation gets the global security applied to it.

Can a way be added to get the Swagger document to have an empty JSON security array for certain operations?

@jez9999
Copy link
Author

jez9999 commented Sep 18, 2020

This appears to be a problem with the underlying OpenAPI.NET library being used by Swagger. I've opened an issue there: microsoft/OpenAPI.NET#521

@jez9999
Copy link
Author

jez9999 commented Dec 4, 2020

@domaindrivendev @sbebrys Any input into this? It looks like Swashbuckle uses its own version of OpenAPI.NET (the reference in Swashbuckle.AspNetCore.Swagger.csproj is to a relative file on the filesystem) so I'm guessing you could fix this without even waiting for OpenAPI.NET to do so upstream?

@eric-b
Copy link

eric-b commented Dec 16, 2020

I ran into same issue.

For OAS 3:
Security (emphasize is mine):

A declaration of which security mechanisms can be used for this operation. 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. To make security optional, an empty security requirement ({}) can be included in the array. This definition overrides any declared top-level security. To remove a top-level security declaration, an empty array can be used.

So if you add an empty OpenApiSecurityRequirement (just with new OpenApiSecurityRequirement()) into operation.Security, this should work.

Also, be aware if you include only one (empty) security requirement, Swagger UI will always call operation anonymously even if user is authenticated. As stated in quoted spec, you will have to add your default security requirement in addition to the empty one if you want Swagger UI to send authentication information when available).

For OAS 2:
This is like you stated, and this is indeed an issue in https://github.com/microsoft/OpenAPI.NET/.

@jez9999
Copy link
Author

jez9999 commented Dec 16, 2020

Yeah, and the empty object still only makes it optional. I want to make it anonymous only. An empty array is required.

@eric-b
Copy link

eric-b commented Dec 16, 2020

Did you tried an empty object in the array?
Unless I misunderstood your need, you want an array with a single empty security requirement. SwaggerUi will show it as an anonymous operation, and will never send authorization header.

@jez9999
Copy link
Author

jez9999 commented Dec 16, 2020

Didn't you just quote from the spec saying that that means optional security, rather than anonymous only?

@darrelmiller
Copy link

darrelmiller commented Dec 19, 2020

@jez9999 Re-reading the language of the specification I can see the confusion now. I'm not sure why we added the language about the empty array. I'm not exactly sure what is the value of saying "I'm not telling you what the security requirements are for this operation".
The comment around the empty object making it optional is assuming there are some other security schemes referenced in the requirements object. If the only thing in the security requirements array is an empty object, then anonymous is the only valid option.
I'll bring this up to the TSC.

@jez9999
Copy link
Author

jez9999 commented Dec 19, 2020

@darrelmiller The value of an empty array is the ability to define default security requirements, and then remove them for certain operations. The OpenAPI spec says that the way you do this is to define your security requirements against the entire API, then use an empty array for operations that can be called anonymously.

What I'm having to do now is define a default anonymous requirement and then add security for 95% of my operations. It works but as you can imagine it greatly bloats the JSON and is semantically unintuative.

@domaindrivendev
Copy link
Owner

@jez9999 it seems you have a solution to your original issue with a simple update to your operation filter:

operation.Security = new List<OpenApiSecurityRequirement>(new []
{
    new OpenApiSecurityRequirement()
});

Therefore I'm going to close this issue

@jez9999
Copy link
Author

jez9999 commented Jan 13, 2021

@domaindrivendev That doesn't create an empty array, it creates an array with one empty object in it. This is still not detected by sawgger-ui as an anonymous operation, and so still displays a padlock.

@eric-b
Copy link

eric-b commented Jan 14, 2021

@jez9999, swagger-ui should detect it, this is supported by its spec (quoted in my previous comment).

If you see a closed padlock on anonymous operation, maybe you are misinterpreting swagger-ui icons, like a lot of people (including me). This is another issue in swagger-ui 4402.

@jez9999
Copy link
Author

jez9999 commented Jan 14, 2021

@eric-b It doesn't detect it, and I'm not sure I disagree with swagger-ui's behaviour. Basically, the spec says that you list supported security operations, and add the empty object to make them all optional. I think this is a bit of a dodgy choice for the spec to be honest, as it allows for the rather ambiguous [{}] which says "all the security methods I've specified are optional, although I haven't specified any". This could be interpreted as anonymous, but it could also be interpreted as "optional security" and either would be reasonable.

IMHO Swashbuckle and/or OpenAPI.NET should be modified to have a property on OpenApiSecurityRequirement along the lines of IsBlankRequirement, which suppresses even the rendering of the object's open/close brackets. That way, such a requirement could be added and an empty array would result.

@eric-b
Copy link

eric-b commented Jan 14, 2021

I can confirm I use this "feature" and it works.

If you post an example of your swagger document, maybe me or someone else can try to spot why it does not work in your case.

I agree on need for OpenAPI.NET to support an empty array because [] and [ {} ] have same meaning (at operation level). And thanks to support of [ {} ], you can do something in OAS3 you could not with OAS2 (due to this limitation in OpenAPI.NET).

I think this is a bit of a dodgy choice for the spec to be honest, as it allows for the rather ambiguous [{}] which says "all the security methods I've specified are optional, although I haven't specified any".

I'm not sure to agree with this: if "[you] haven't specified any [security requirement at all in your document]", [{}] is useless. Security requirements [{}] at operation level means "I want to override security requirements at document level with anonymous authentication". In your case, you set a default security requirement at document level, so it does make sense to override it with [{}] at operation level, if you want this specific operation to be anonymous, unlike other operations of the document. I find the spec to be rather clear about this, albeit not intuitive. I think one goal of json is to be human readable and I don't find [ { } ] to be an intuitive value.

@jez9999
Copy link
Author

jez9999 commented Jan 14, 2021

@eric-b Can you point me to exactly where in the spec it clearly states that [{}] means "no security/anonymous"? I see "To make security optional, an empty security requirement ({}) can be included in the array." That's "optional", not "no security". It also states "To remove a top-level security declaration, an empty array can be used." which implies that you should use an empty array, not an array containing an empty object.

I can confirm I use this "feature" and it works.

Does it work in the sense that you use swagger-ui, and operations where global security is overridden using [{}] display nothing in the top-right corner instead of a padlock? The code does not support that:

      (!security || !security.count()) ? null :
        <AuthorizeOperationBtn
          isAuthorized={isAuthorized}
          onClick={() => {
            const applicableDefinitions = authSelectors.definitionsForRequirements(security)
            authActions.showDefinitions(applicableDefinitions)
          }}
        />
    }

It only checks !security || !security.count() - equating to an empty array. An array with one empty object would cause a padlock to be rendered.

@eric-b
Copy link

eric-b commented Jan 14, 2021

You are right there is a UI difference between [] and [{}] and my previous statement was not entirely correct. Empty array shows no padlock, while array with empty object shows a "closed padlock". Nonetheless, anonymous call works in both cases.

My understanding of padlocks in Swagger-UI is:

  • no padlock: there is no security requirement documented (we could have client certificate authentication, this is not supported by OAS security requirement).
  • padlock: security requirement documented (even anonymous one).
    • padlock open: we can call operation (we are authenticated or operation can be anonymous).
    • padlock closed: we can expect http 401 if operation is called (we are not authenticated).

IMHO:

  • "optional security" means you can be authenticated or not, and it will works.
  • "no security": I'm not sure what it means to you. I guess it could mean same thing as "optional security" or "security is not documented".

For spec pointer, I already gave a link in my first comment.

This is my interpretation, and I find spec quite clear in its logic and outcome described here. But you also proved it is not so obvious for everyone.

@jez9999
Copy link
Author

jez9999 commented Jan 14, 2021

@eric-b Yes, but I don't want any padlock appearing for certain of my operations as security simply doesn't make sense for them. Hence I want the empty array to trigger swagger-ui to do that.

@eric-b
Copy link

eric-b commented Jan 14, 2021

I understand. Empty object in array is a workaround in my use case.

As far as I know, if you want an empty array, you'll need to transform response from swagger endpoint with a custom middleware of your own. This is what I've done for an Owin application (OAS 2 does not support empty object in an array).

@jez9999
Copy link
Author

jez9999 commented Jan 14, 2021

Or you could change the Swagger OpenAPI.NET fork to allow an OpenApiSecurityRequirement that doesn't render opening/closing JSON tags which would be easier.

@eric-b
Copy link

eric-b commented Jan 14, 2021

Or you could change the Swagger OpenAPI.NET fork to allow an OpenApiSecurityRequirement that doesn't render opening/closing JSON tags which would be easier.

I agree on this, but decision is not mine :-). I was trying to help with a workaround, I'm not involved in Swashbuckle project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants