dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.16k stars 9.92k forks source link

[9.0-preview.4] Remove default tag with the assembly name from OpenAPI document #55884

Open joegoldman2 opened 3 months ago

joegoldman2 commented 3 months ago

Is there an existing issue for this?

Is your feature request related to a problem? Please describe the problem.

Similar to #55832, I wonder what the purpose of the default tag which contains the name of the assembly is, at document level and for each operation.

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddOpenApi();

var app = builder.Build();

if (app.Environment.IsDevelopment())
{
    app.MapOpenApi();
}

app.MapGet("/", () => "Hello world!");

app.Run();

It produces the following documentation:

{
  "openapi": "3.0.1",
  "info": {
    "title": "OrderApi | v1",
    "version": "1.0.0"
  },
  "paths": {
    "/": {
      "get": {
        "tags": [
          "OrderApi"
        ],
        "responses": {
          "200": {
            "description": "OK",
            "content": {
              "text/plain": {
                "schema": {
                  "type": "string"
                }
              }
            }
          }
        },
        "x-aspnetcore-id": "3859a3eb-53ea-41d9-95bb-6b491227c87f"
      }
    }
  },
  "tags": [
    {
      "name": "OrderApi"
    }
  ]
}

where OrderApi is the name of the assembly.

It leaks the name of the assembly which can be an internal name, and a user may not want to disclose it if the document is internet-facing.

Describe the solution you'd like

The tag containing the name of the assembly should not be added by default, even if it can easily be removed using a transformer.

Additional context

No response

fksalviano commented 3 months ago

I'll try to fix It by adding a Title property to OpenApiOptions class and use the value setted on AddOpenApi(o => o.Title) instead of Assembly Name.

fksalviano commented 3 months ago

I was just think about the "title": "OrderApi | v1" section of json file generated.

joegoldman2 commented 3 months ago

For the title, something like that is already possible:

builder.Services.AddOpenApi(options =>
{
    options.UseTransformer((document, context, cancellationToken) =>
    {
        document.Info = new()
        {
            Title = "Checkout API",
            Version = "v1",
            Description = "API for processing checkouts from cart."
        };
        return Task.CompletedTask;
    });
});

This issue is about the default tag with the assembly name that can also be removed using a transformer but which I don't think should be there by default.

fksalviano commented 3 months ago

Yes, i found this comment on code of OpenApiGenerator class on GetOperationTags() method where is using the ApplicationName instead of methodInfo Name:

// If the declaring type is null or compiler-generated (e.g. lambdas),
// group the methods under the application name.
 controllerName = _environment?.ApplicationName ?? string.Empty;
fksalviano commented 3 months ago

Using latest version of .NET 9 preview 4 and the version 9.0.0-preview.4.24267.6 of OpenApi, the default tag is OK, the assembly name is API and the tag is correct:

{
  "openapi": "3.0.1",
  "info": {
    "title": "API | v1",
    "version": "1.0.0"
  },
  "paths": {
    "/api/weatherforecast": {
      "get": {
        "tags": [
          "WeatherForecast"
        ],
        "description": "Get weather forecasts sample endpoint",
        "responses": {
          "200": {
            "description": "OK",
            "content": {
fksalviano commented 3 months ago

Right, I´m thinking tath changing the result of GetOperationTags() method to an empty list instead a list with one tag containig the application name should resolve this cases:

        if (methodInfo.DeclaringType is not null && !TypeHelper.IsCompilerGeneratedType(methodInfo.DeclaringType))
        {
            return new List<OpenApiTag>() { new OpenApiTag() { Name = methodInfo.DeclaringType.Name } };            
        }
        else
        {
            // If the declaring type is null or compiler-generated (e.g. lambdas),
            // returns an empty list of tagas
            return new List<OpenApiTag>();
        }
fksalviano commented 3 months ago

Tags removed from json when there is no one specified.

In my tests was necessary to change the result of GetTags() method too on OpenApiDocumentService class checking if the route values controller is diferent from assembly name, so with those two changes (this one and the above, on OpenApiGenerator) I could generate the json without tags:

    private List<OpenApiTag>? GetTags(ApiDescription description)
    {
        var actionDescriptor = description.ActionDescriptor;
        if (actionDescriptor.EndpointMetadata?.OfType<ITagsMetadata>().LastOrDefault() is { } tagsMetadata)
        {
            return tagsMetadata.Tags.Select(tag => new OpenApiTag { Name = tag }).ToList();
        }
        // If no tags are specified, use the controller name as the tag. This effectively
        // allows us to group endpoints by the "resource" concept (e.g. users, todos, etc.)
        var controllerName = description.ActionDescriptor.RouteValues["controller"];
        if (controllerName != hostEnvironment.ApplicationName)
            return [new OpenApiTag { Name =  controllerName}];
        else
            return new List<OpenApiTag>();
    }
{
  "openapi": "3.0.1",
  "info": {
    "title": "API-Clean-VS",
    "description": ".Net Core API sample using Clean Architecture and Vertical Slice",
    "version": "v1"
  },
  "paths": {
    "/api/weatherforecast": {
      "get": {
        "description": "Get weather forecasts sample endpoint",
        "responses": {
          "200": {
            "description": "OK",
            "content": {
captainsafia commented 3 months ago

Thanks for filing this issue!

I wonder what the purpose of the default tag which contains the name of the assembly is, at document level and for each operation.

The default tags appears in both the document and the operation-level because that's considered a best-practice by the OpenAPI specification. The document-level tags property should aggregate all the tags that are used at the operation-level.

Yes, i found this comment on code of OpenApiGenerator class on GetOperationTags() method where is using the ApplicationName instead of methodInfo Name:

The OpenApiGenerator isn't actually the codepath invoked for generating the tags here. The code used to do the generation is here:

https://github.com/dotnet/aspnetcore/blob/83aa6b17021ae02c93aeddd58c11af35fe0bb14b/src/OpenApi/src/Services/OpenApiDocumentService.cs#L165-L175

As you can see, it uses the controller-name as the default value. This tends to be useful for controller-based apps where the controller name is associated with the resource that the endpoint is associated with (e.g. UserController, TodoController, etc).

This assumption does break for minimal APIs though where there are no controllers and the RouteValues['controller] value defaults to the assembly name here.

I suppose we could change the behavior here to only use RouteValues['controller'] as the value for APIs that are truly controller-based. I'll have to see if there is a reliable semantic on ApiDescription that we can use to check for this.

Edit: I was originally trying to get this in for preview5 but I think more noodling is required here. I don't think that simply skipping over RouteValues['controller'] if it equals the application name is the most bullet-proof approach.