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.59k stars 10.06k forks source link

Identity Endpoints API - Optional "CallbackUrl" #50904

Open augustevn opened 1 year ago

augustevn commented 1 year ago

Background and Motivation

I'd like an optional string? CallbackUrl since I'll likely have multiple frontends on different (sub)domains, consuming the same Auth API. Passing that (sub)domain URL all the way through, would enable that.

For example, https://www.leashr.com & https://www.belgiandoggos.be use the same API for authentication & authorization. Setting either one in my appSettings.json won't cut it.

So, I prefer you just make the parameters available so we can form our own URLs and decide whether it goes to a frontend or backend. Then, I can also choose to use route parameters rather than query string parameters on my Blazor frontends.

Feel free to watch my implementation of RC1: https://youtu.be/yGYpN1hFPAg?si=zlLykBIas_WBjXWN

Proposed API


    Task SendConfirmationEmailAsync<TUser>(TUser user, string email, string code, string confirmationLink, string? callbackUrl) where TUser : class
    {
        return SendEmailAsync(email, code, confirmationLink, callbackUrl);
    }

    // Merged ResetCode & ResetLink into one.
    Task SendPasswordResetEmailAsync<TUser>(TUser user, string email, string resetCode, string resetLink, string? callbackUrl) where TUser : class
    {
        return SendEmailAsync(email, resetCode, resetLink, callbackUrl);
    }

    public sealed class NoOpEmailSender : IEmailSender
    {
        public Task SendEmailAsync(string email, string code, string link, string? callbackUrl) => Task.CompletedTask;
    }

DTO

You'll need to add the string? CallbackUrl property to your request & response DTO's as well to pass the value all the way down and back to the consumer.

Usage Examples


    public async Task SendConfirmationEmailAsync<TUser>(TUser user, string email, string code, string confirmationLink, string? callbackUrl) where TUser : class
    {
        var frontendConfirmationLink = $"<a href='{callbackUrl}/{user.UserId}/{code}'>clicking here</a>";

        await SendEmailAsync(email, $"{user.Name}, confirm your email", $"Please confirm message (in my native language) {frontendConfirmationLink}.");
    }

    // Merged ResetCode & ResetLink into one.
    public async Task SendPasswordResetEmailAsync<TUser>(TUser user, string email, string resetCode, string resetLink, string? callbackUrl) where TUser : class
    {
        var frontendResetLink = $"<a href='{callbackUrl}/{email}/{resetCode}'>clicking here</a>";

        await SendEmailAsync(email, $"{user.Name}, reset your password", $"Reset password message (in my native language): {frontendResetLink}");
    }

Alternative Designs

The formed links are fine as an example but a pain to extract from & reformat. The subject & message are likely to be translated or changed, so mainly serve as an example as well. I don't think you need either of the above but feel free to leave those in.

Risks

Will need good documentation to prevent confusion about being able to use the provided link but also to use the parameters to form your own link.

ghost commented 1 year ago

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.

halter73 commented 1 year ago

There was a bug in RC1 where the links passed to the IEmailSender were relative rather than absolute (#50296). Now that that's fixed, the confirmationLink should actually be usable without an extra callbackUrl.

I think that accepting an optional redirectUrl could be interesting for the SendConfirmationEmailAsync so the user ends up somewhere more useful than a plaintext page that says "Thank you for confirming your email.", but I don't think it makes too much sense for SendPasswordResetCodeAsync since there's no GET endpoint associated with it to link to. SendPasswordResetCodeAsync expects the app to have custom UI that accepts the reset code and POSTs it to the /resetPassword endpoint using something like HttpClient or fetch without navigating the browser to that page.

Even for email confirmation, you could encode the real confirmationLink in the query string of your frontend URL and then have the frontend make a GET request to the real confirmationLink it decodes from the query string in the background.

We cannot remove the existing IEmailSender<TUser> methods at this point without breaking people. We can add new overloads, but that risks overwhelming people with too many overloads. Another option might be to flow through an IDictionary<string, object?> property bag to HttpContext.Items or AuthenticationProperties.Parameters for maximum flexibility.

If you do not need to customize the callbackUrl per email, but instead it's per-user or per-application config, you could configure the callbackUrl as an field on TUser or the IEmailSender<TUser>. Last resort, you can just copy IdentityApiEndpointRouteBuilderExtensions which contains MapIdentityApi<TUser> and send emails however you want. It only references public API from other classes, so it should be easy to copy.

augustevn commented 1 year ago

There was a bug in RC1 where the links passed to the IEmailSender were relative rather than absolute (#50296). Now that that's fixed, the confirmationLink should actually be usable without an extra callbackUrl.

I think that accepting an optional redirectUrl could be interesting for the SendConfirmationEmailAsync so the user ends up somewhere more useful than a plaintext page that says "Thank you for confirming your email.", but I don't think it makes too much sense for SendPasswordResetCodeAsync since there's no GET endpoint associated with it to link to. SendPasswordResetCodeAsync expects the app to have custom UI that accepts the reset code and POSTs it to the /resetPassword endpoint using something like HttpClient or fetch without navigating the browser to that page.

Even for email confirmation, you could encode the real confirmationLink in the query string of your frontend URL and then have the frontend make a GET request to the real confirmationLink it decodes from the query string in the background.

We cannot remove the existing IEmailSender<TUser> methods at this point without breaking people. We can add new overloads, but that risks overwhelming people with too many overloads. Another option might be to flow through an IDictionary<string, object?> property bag to HttpContext.Items or AuthenticationProperties.Parameters for maximum flexibility.

If you do not need to customize the callbackUrl per email, but instead it's per-user or per-application config, you could configure the callbackUrl as an field on TUser or the IEmailSender<TUser>. Last resort, you can just copy IdentityApiEndpointRouteBuilderExtensions which contains MapIdentityApi<TUser> and send emails however you want. It only references public API from other classes, so it should be easy to copy.


No, I understood that from other threads. The absolute URI seems fixed in RC2 but it leads to my API which very unusual for an SPA + API setup.

For both confirmation or resetting the password, I'd prefer to have the user end up on a well formatted / dynamic front-end Blazor page. Now, I construct that link myself by extracting the information like the code & userId to lead the user to a 'reset password form' or the 'login form' page using route parameters instead of query string parameters.

I don't find it user-friendly for the user to have to copy their code into my API or a front-end field somewhere, hence a link that they simply click on, would be preferred. One that doesn't lead to my API. Setting one in my appSettings.json doesn't cover the multiple front-ends on one API scenario.

I'm not sure about the passing extra properties on the TUser, aren't you using DTOs that strip away all the extra properties that I'd pass on /register or on /forgotPassword? I think that is exactly the flexibility that we're missing. Passing down extra properties on the TUser or just all the way down.

I like the property bag idea & I'll take a look at IdentityApiEndpointRouteBuilderExtensions.

Thanks for taking your time to respond.

augustevn commented 1 year ago

I just tested passing extra info on the TUser, doesn't work. That would be the requirement to pass extra data on /register, /forgotPassword etc.

I used this Entity model:

public class User : IdentityUser
{
    public string? Name { get; set; }

    public DateTime CreatedAt { get; set; }
    public string? CreatedBy { get; set; }
    public DateTime? LastModifiedAt { get; set; }
    public string? LastModifiedBy { get; set; }
}

If I /register a user and pass along:

{
   "name": "auguste",
  "email": "test@gmail.com",
  "password": "Test1234"
}

The name doesn't end up in the database so that wouldn't work for any other properties either. I think that's the last improvement on flexibility you could do to make these endpoints the preferred way over custom JWT stateless SPA + API auth.

It doesn't come out of the SendConfirmationLinkAsync(User user, string email, string confirmationLink) either due to the DTO usage, I assume. So for now, that TUser parameter is not useful.

image

augustevn commented 1 year ago

Even for email confirmation, you could encode the real confirmationLink in the query string of your frontend URL and then have the frontend make a GET request to the real confirmationLink it decodes from the query string in the background.

Is a good suggestion but also not feasable on the multiple front-end, one API scenario. As long a we can't pass extra variables (due to DTOs) all the way down, we're stuck hard-coding a front-end URL, which only support one SPA on one API scenario.

The only solution I see: Pass extra properties on DTOs RegisterUserRequest, ForgotPasswordRequest, ... And to get them back from the SendConfirmationEmailAsync & SendResetPasswordResetCodeAsync.

Whether that happens through the TUser or as extra RedirectUrl parameter, doesn't matter to me. Although, it would be nice to store more user info than just the email at /register.

augustevn commented 1 year ago

An additional question arose on how to validate extra passed data.