Skip to content

Identity Endpoints API - Optional "CallbackUrl" #50904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
augustevn opened this issue Sep 24, 2023 · 6 comments
Open

Identity Endpoints API - Optional "CallbackUrl" #50904

augustevn opened this issue Sep 24, 2023 · 6 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-identity Includes: Identity and providers enhancement This issue represents an ask for new feature or an enhancement to an existing one
Milestone

Comments

@augustevn
Copy link

augustevn commented Sep 24, 2023

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.

@augustevn augustevn added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Sep 24, 2023
@ghost ghost added the area-identity Includes: Identity and providers label Sep 24, 2023
@mkArtakMSFT mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Oct 26, 2023
@mkArtakMSFT mkArtakMSFT added this to the Backlog milestone Oct 26, 2023
@ghost
Copy link

ghost commented Oct 26, 2023

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
Copy link
Member

halter73 commented Nov 1, 2023

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
Copy link
Author

augustevn commented Nov 2, 2023

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
Copy link
Author

augustevn commented Nov 2, 2023

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": "[email protected]",
  "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
Copy link
Author

augustevn commented Nov 2, 2023

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
Copy link
Author

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

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-identity Includes: Identity and providers enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

No branches or pull requests

4 participants