Fix endpoint api/users/reset-password to set tenant ID. #742
Conversation
| @@ -21,7 +21,7 @@ public async Task<string> ForgotPasswordAsync(ForgotPasswordRequest request, str | |||
| // For more information on how to enable account confirmation and password reset please | |||
| // visit https://go.microsoft.com/fwlink/?LinkID=532713 | |||
| string code = await _userManager.GeneratePasswordResetTokenAsync(user); | |||
| const string route = "account/reset-password"; | |||
| string route = $"account/reset-password?token={code}"; | |||
There was a problem hiding this comment.
The token (code) is already added a few lines lower using the AddQueryString method...
This should probably be something like ?tenant={_currentTenant.Id} instead (or also use the AddQueryString method).
Have you tested this? Not 100% sure how this is supposed to work... but I suppose this url that is sent out is pointing to the front-end UI, where the user can input a new password... then that UI in turn issues a call to the backend again... so in that last call, the TenantId can actually be added as a header (and the controller method should then just have the [TenantIdHeader] attribute?
Like I said... not 100% sure... just my 2 cents ;-) (I personally am using AAD, so no need for password resets or the like)
There was a problem hiding this comment.
I tested that the endpoint works., but I am still not sure this is exactly what we want. Yes, comments are helpful thanks... let me rework and come back to you.
There was a problem hiding this comment.
@fretje , I have made further changes to this taking into account your suggestions. The tenancy is now pulled from the header using the attribute. But my point of confusion now is what to say in that email message? I've currently updated this to show the API endpoint and not a 'passwordResetUrl', as this is probably a frontend routing decision? Any suggestions? The confirm email route is using a GET request, but I do not suppose we can do that here as the url would contain sensitive information such as password!
There was a problem hiding this comment.
No, you need to go to your frontend from the link in the mail, where the user is then presented with a form to fill in their new password. From there, the front-end then issues a request to the "reset-password" api (in the backend) providing the actual new password which has been input by the user, and also the tenant and code which have been parsed from the url (querystring).
I mean... you can't hand over an url to the back-end and let the user figure out how to issue a post request to that url with the right json payload, right?
There was a problem hiding this comment.
The confirm email url is going straight to the backend, as that one doesn't need any other input from the user. I do think it should actually redirect back to the front-end again to be able to show a more user-friendly "your email has been confirmed" message, with an extra button to go back to the login page... but that's something else... ;-)
There was a problem hiding this comment.
"I mean... you can't hand over an url to the back-end and let the user figure out how to issue a post request to that url with the right json payload, right?"
I agree. But this project is a backend / api boilerplate, should we put some documentation in explaining how this should be setup, maybe the frontend route could come from config? Or are we looking to actually create a page for this in the boilerplate?
There was a problem hiding this comment.
There is https://github.com/fullstackhero/blazor-wasm-boilerplate which is a blazor front-end for this api... there's also an angular one, but that one hasn't been updated in a while.
There was a problem hiding this comment.
@fretje So that's another issue, updating those. I might be able to look at the Angular one at some point in the future! For now then I think I will draw a line under this. I think it's better than how I found it, the endpoint now works at least.
Thanks for your hard work on an amazing project.
…eader to ResetPasswordAsync method; Update message in ForgotPasswordAsync;
jay-cascade
changed the title
| @@ -21,7 +21,7 @@ public async Task<string> ForgotPasswordAsync(ForgotPasswordRequest request, str | |||
| // For more information on how to enable account confirmation and password reset please | |||
| // visit https://go.microsoft.com/fwlink/?LinkID=532713 | |||
| string code = await _userManager.GeneratePasswordResetTokenAsync(user); | |||
| const string route = "account/reset-password"; | |||
| const string route = $"api/users/reset-password"; | |||
There was a problem hiding this comment.
You still need to add the tenantid in the url... Btw, the url is prepended with "{origin}" in the line below this, which means the front-end ui... so it makes no sense to put "api" in there... better leave it as is for now... a corresponding page will need to exist in the front-end (I see even in the blazor front-end this hasn't been added yet though).
There was a problem hiding this comment.
Okay, so maybe if I get some down time I can start looking at Blazor. It certainly looks interesting. in the meantime, I will keep this branch up to date with main. Thanks.
Refer to issue: #741
Refer to discussion: #740