json-api-dotnet / JsonApiDotNetCore

A framework for building JSON:API compliant REST APIs using ASP.NET and Entity Framework Core.
https://www.jsonapi.net
MIT License
676 stars 159 forks source link

Constrain available operations #1560

Closed bkoelman closed 3 months ago

bkoelman commented 3 months ago

DESCRIPTION

The current behavior is that, by default, all operations on all resource types are exposed. To reduce this, we've advised users to write custom filter logic by overriding the operations action method. See here for an example.

While this works, it's unfortunate that JADNC can't "see" which operations are exposed. OpenAPI generation should be able to take this into account.

Furthermore, it's an odd default to always expose everything, even when a resource defines a subset of endpoints. For example:

[Resource(GenerateControllerEndpoints = JsonApiEndpoints.Post | JsonApiEndpoints.Patch)]
public class Article : Identifiable<long>
{
}

It would make more sense to apply the subset from GenerateControllerEndpoints by default, to determine which operations are exposed. But it must be possible to override that, because custom controllers or custom action methods may exist, which are excluded from controller auto-generation.

PROPOSED SOLUTION

Provide an extensibility point to indicate which operations are exposed. This enables OpenAPI generation to take this into account when producing schemas. The default implementation would look at GenerateControllerEndpoints. JADNC should call this extensibility point from the base operations controller action method, for each operation in the request. And produce an error with the list of operations in the request that aren't accessible.

public interface IAtomicOperationFilter
{
    bool IsEnabled(ResourceType resourceType, WriteOperationKind writeOperation);
}

IMPACT

This is a breaking change, which improves security. GenerateControllerEndpoints defaults to All, so users that have never set this attribute won't be affected. Users that did, but weren't aware they should have written custom logic, will now silently benefit from a more consistent REST API surface. Any custom logic that users already wrote to reduce the surface will remain intact. But users that don't use auto-generated controllers (GenerateControllerEndpoints set to None, or no [Resource] attribute) and have added an operations controller will now need to implement the interface. Failing to do so will block all operations on explicit (non-auto-generated) controllers. The good thing is that updating will result in a compile error, because BaseJsonApiOperationsController gets an extra constructor parameter for the interface above. This should trigger users to investigate what happened and adapt accordingly. This breaking change needs to be mentioned in the documentation and the release notes.

To simplify reverting to the old behavior, we can provide a static property on the interface:

public interface IAtomicOperationFilter
{
    public static IAtomicOperationFilter AlwaysEnabled { get; } = new AlwaysEnabledOperationFilter(); // always returns true
}

So that users can pass it for the newly added constructor parameter:

public sealed class OperationsController(
    IJsonApiOptions options, IResourceGraph resourceGraph, ILoggerFactory loggerFactory,
    IOperationsProcessor processor, IJsonApiRequest request, ITargetedFields targetedFields)
    : JsonApiOperationsController(options, resourceGraph, loggerFactory, processor, request, targetedFields,
    /* --> */ IAtomicOperationFilter.AlwaysEnabled /* <-- */);