Open apanaligan opened 1 year ago
Thanks for the suggestion. We're filing this away for considering possible future enhancements.
We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.
We are open to improving MapIdentityApi
's extensibility, but we need to be careful to not to overly inhibit our ability to add functionality. Right now, we could add new service parameters to existing minimal route handlers and add entire new endpoints without breaking people, and it's important we can continue to do so.
Of course, rather than include all the service parameters in the decorator signatures, we could limit the decorators to only being Func<HttpContext, RequestDTO, Task<IResult>>
for each endpoint, but what if you wanted to inspect the response DTO? You could do Func<HttpContext, RequestDTO, Task<ResponseDTO>>
, but what if you wanted to return a different kind of response?
These decorators are strongly typed rather than just being a Delegate
because MapIdentityApi
relies on the new Request Delegate Generator which requires strong typing in order to get AOT support. Designing an API that would allow you to effectively change RegisterRequest
and potentially other DTOs to require additional fields in a trim safe way seems challenging, but I'm open to suggestions.
And even if we got that all figured out, what if we added another endpoint after .NET 9 to allow you to register with an external access token instead of a password? Now any decorator that was wrapping the existing /register endpoint to enforce extra registration requirements would be skipped for people registering with external providers.
Trying to reuse the existing /register endpoint for this with its RegisterRequest
would be problematic because Password
is supposed to be non-nullable. We could argue that the access token is the Password
and add a new optional bool to indicate what it really represents, but that would definitely muddy up the API.
A change to support external logins might need be opt-in anyway, but you could argue just adding the external auth provider should be a sufficient opt-in just like it is for AddDefaultUI
Razor pages. However, that argument becomes worse if people are already adding extra registration requirements using decorators.
The good news is that since we made all the MapIdentityApi DTOs public, it's now much easier to copy the code into your project. You can take IdentityApiEndpointRouteBuilderExtensions.cs as-is and just rename MapIdentityApi<TUser>
to MyMapIdentityApi<TUser>
and call that instead. You do not need to copy any other files because it only calls into public API.
And if you really want to go with the decorator approach, you can write a custom endpoint filters which is now much easier with the DTOs being public. For example:
app.MapIdentityApi<MyUser>().AddEndpointFilterFactory((filterFactoryContext, next) =>
{
var parameters = filterFactoryContext.MethodInfo.GetParameters();
if (parameters.Length >= 1 && parameters[0].ParameterType == typeof(RegisterRequest))
{
return async invocationContext =>
{
// Validate some extra required parameters were included in the query string using
// invocationContext.HttpContext.Request.Query or something.
// ...
return await next(invocationContext);
};
}
return next;
});
That said, I recommend copying IdentityApiEndpointRouteBuilderExtensions.cs for the most flexibility and robustness. Relying on non-public route handers to keep the exact same signature they currently have is bound to be fragile as it's not public API at least as of now.
Apologies for the potentially silly question but can we not just have the identity endpoints exposed as regular functions? So we could write our own endpoints, have our own request and response bodies, do all the additional validation work on the custom properties, and then just call the base identity functions like register and pass the user object?
Apologies for the potentially silly question but can we not just have the identity endpoints exposed as regular functions? So we could write our own endpoints, have our own request and response bodies, do all the additional validation work on the custom properties, and then just call the base identity functions like register and pass the user object?
This is exactly what I am looking for as in the previous features in MVC!
Apologies for the potentially silly question but can we not just have the identity endpoints exposed as regular functions?
This is a very good question. I think maybe these should be exposed as regular functions. It's a nice in-between solution in case you don't want just the default behavior, but you also don't want to copy hundreds of lines of code.
The reason we didn't do it immediately in .NET 8 is because it would limit some of the changes we could make. For example, we'd potentially have to keep around legacy overloads of functions that don't accept all the parameters supported by future versions of the given endpoint.
This is the same basic reason the request and response DTOs were not public in early .NET 8 previews, but we switched course on that to improve customizability. If we feel people are happy enough with the current core implementation of these endpoint functions and there's enough demand, we'd certainly consider making these public. If someone wants to take a stab at and API proposal, please do.
Here's the "API proposal" issue template. Feel free to just copy the markdown into this issue if you want to keep the context. We can apply the right labels to this issue so it shows up on https://apireview.net/?g=aspnetcore for review.
Being able to customize the endpoints would be great but given the issues mentioned i propose a different solution. What if only the option to disable specific endpoints was provided.
Image i would want custom logic inside my /register endpoint. instead of providing a delegate or something similar i could just disable the original endpoint and create my own endpoint with the same route. that would already solve alot of problems (for me atleast). That would also help with hiding endpoints that are unused by the application to enhance the security.
Would that be something feasible?
Here is my attempt at extracting the base functions
Some remarks:
TUser
. Still looking for a good solution here.But other than that it seems to be working fine. Here is my Program.cs setup:
// Disable the routes /register and /info
app.MapGroup("/auth").MyMapIdentityApi<AuthUser>([IdentityApiEndpoint.PostRegister, IdentityApiEndpoint.GetInfo]);
and here is my custom registration endpoint
[AllowAnonymous]
[HttpPost("register")]
public async Task<Results<Ok, ValidationProblem>> Register([FromBody] RegisterRequestDTO registration)
{
if (string.IsNullOrEmpty(registration.FirstName.Trim()))
{
Dictionary<string, string[]> err = new() { { "RequiredFieldEmpty", ["First Name must not be empty"] } };
return TypedResults.ValidationProblem(err);
}
if (string.IsNullOrEmpty(registration.LastName.Trim()))
{
Dictionary<string, string[]> err = new() { { "RequiredFieldEmpty", ["Last Name must not be empty"] } };
return TypedResults.ValidationProblem(err);
}
AuthUser user = new()
{
FirstName = registration.FirstName,
LastName = registration.LastName,
};
RegisterRequest reg = new()
{
Email = registration.Email,
Password = registration.Password
};
return await MyIdentityApiEndpointRouteBuilderExtensions.Register(_serviceProvider, reg, HttpContext, user);
}
Apologies for the potentially silly question but can we not just have the identity endpoints exposed as regular functions?
This is a very good question. I think maybe these should be exposed as regular functions. It's a nice in-between solution in case you don't want just the default behavior, but you also don't want to copy hundreds of lines of code.
The reason we didn't do it immediately in .NET 8 is because it would limit some of the changes we could make. For example, we'd potentially have to keep around legacy overloads of functions that don't accept all the parameters supported by future versions of the given endpoint.
This is the same basic reason the request and response DTOs were not public in early .NET 8 previews, but we switched course on that to improve customizability. If we feel people are happy enough with the current core implementation of these endpoint functions and there's enough demand, we'd certainly consider making these public. If someone wants to take a stab at and API proposal, please do.
Here's the "API proposal" issue template. Feel free to just copy the markdown into this issue if you want to keep the context. We can apply the right labels to this issue so it shows up on https://apireview.net/?g=aspnetcore for review.
"The reason we didn't do it immediately in .NET 8 is because it would limit some of the changes we could make. For example, we'd potentially have to keep around legacy overloads of functions that don't accept all the parameters supported by future versions of the given endpoint."
Does that mean it's considered to expose the full api methods in future? This will be amazing!!
I just wanted to say that copying the extension methods class into my project is a great bridge solution. Combined with the interface based stores with generic parameters for the entities, it gives me all the flexibility I need.
Is there an existing issue for this?
Is your feature request related to a problem? Please describe the problem.
No response
Describe the solution you'd like
After reading about the Identity API endpoints, I immediately got excited, tried it and was able to make it work after 10 minutes! (Yes! All identity features almost instantly created and working).
One thing came to mind is on up to what extent these endpoints are extensible (e.g. Decorator pattern) to support custom workflow mostly for registration or login?
I explored the code that creates the routes and it seems the behavior are directly added to the routes pipeline. For instance, the RegisterRequest DTO contains only Email and Password. Normally, you would like to throw in a few more fields captured in your registration process.
If I missed anything that relates to this feature, I would appreciate if somebody can point them out to me.
Additional context
No response