berhir / AspNetCore.SpaYarp

An alternative approach to the new ASP.NET Core SPA templates in .NET 6. It uses YARP as proxy to forward requests to the SPA dev server.
MIT License
99 stars 12 forks source link

Replace the policy name attribute in MapSpaYarp with IEndpointConventionBuilder #31

Open bve-wd opened 8 months ago

bve-wd commented 8 months ago

Return the IEndpointConventionBuilder instead of passing a policyName. This allows more flexibility in the configuration of the endpoint such as AllowAnonymous or RequireAuthorization.

Also added a section in the Readme.md and fixed some small issues which prevented me from building (e.g. the version in global.json).

Increased the version from 2.0.1 to 2.0.2.

bve-wd commented 3 months ago

I just looked again at my changes and saw that I introduced a breaking change. Even re-adding the policyName parameter would be a breaking change though, since the return value changed, which could break existing code if the IEndpointRouteBuilder is used in a chained configuration. Thus, this change should increase the major version, if semantic versioning is used.

However, there was no change for a long time, so I am not sure if it makes sense to change anything in my PR.

HanoRossouw commented 3 months ago

I just looked again at my changes and saw that I introduced a breaking change. Even re-adding the policyName parameter would be a breaking change though, since the return value changed, which could break existing code if the IEndpointRouteBuilder is used in a chained configuration. Thus, this change should increase the major version, if semantic versioning is used.

However, there was no change for a long time, so I am not sure if it makes sense to change anything in my PR.

I can confirm it does cause an issue if you do not handle the null in the pipeline when the spa.proxy.json file is not present.

var point = app.MapSpaYarp();

if(point != null)
{
    point.AllowAnonymous();
}

but honestly is a minor issue compared to having to send in a policyname or allow anonymous boolean flag to control endpoint auth

bve-wd commented 3 months ago

Currently there is not even a possibility to call AllowAnonymous() for the SpaYarp catch all path. That was the main reason for the change, since it is currently unconvenient to allow anonymous access if MapSpaYarp() is combined with AddAuthorization() when an authenticated user is requested by default. It would be possible though, to create a custom authorization policy without rules, such that it behaves like AllowAnonymous, but that is not an elegant solution if the convention builder can be exposed instead.

bve-wd commented 3 weeks ago

I consider this projekt as dead and will not continue pushing changes to it. Feel free to accept or reject my PR.