Implement IEndpointMetadataProvider on Result types like Forbid, Unauthorized, etc

[dotnetspark]

Recently, I noticed some types at https://github.com/dotnet/aspnetcore/tree/main/src/Http/Http.Results/src are not implementing IEndpointMetadataProvider.PopulateMetadata(...). Doing so would allow types like ForbidHttpResult and UnauthorizedHttpResult to shown up in Swagger/OpenAPI when used.

[captainsafia]

Triage: We discussed that there might have been some intentionality to ForbidHttpResult and UnauthorizedHttpResult not implementing this API because they are not static (might not know the status code as a result of calling Forbid) and don't have response types that need to be documented (unauthorized is just a status code).

For those reasons, we opted out of including behavior by default for these, especially because people can implement their own IEndpointMetadataProvider to produce these annotations. Alternatively, people can use the untyped results, e.g. Results.Unauthorized.

Are there are result types that suffer from this issue that would be reasonable to document given the explanation above?

[ghost]

Hi @ylr-research. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[ghost]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.

[ghost]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.

[ghost]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.

[captainsafia]

Triage: We discussed that there might have been some intentionality to ForbidHttpResult and UnauthorizedHttpResult not implementing this API because they are not static (might not know the status code as a result of calling Forbid) and don't have response types that need to be documented (unauthorized is just a status code).

For those reasons, we opted out of including behavior by default for these, especially because people can implement their own IEndpointMetadataProvider to produce these annotations. Alternatively, people can use the untyped results, e.g. Results.Unauthorized.

Are there are result types that suffer from this issue that would be reasonable to document given the explanation above?

@davidfowl What are your thoughts on this?

[davidfowl]

There are 2 things that typed results do:

1. Give you strong type checks.
2. Express metadata for open API.

I think we should do this for UnauthorizedHttpResult as the workaround currently is to fallback to manual produces metadata. ForbidHttpResult is a bit trickier because it relies on the authentication handler implementation of Forbid. We might need another result type here that's purely a 403 without going through the authN system.

If there are situations where people want to avoid advertising the 401 in their APIs after this change, we'd need a way to suppress this metadata, but also keep the type safety. I don't know how big of a deal this is today or how easy it needs to be though.

[ghost]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.