skoruba / IdentityServer4.Admin

The administration for the IdentityServer4 and Asp.Net Core Identity
MIT License
3.57k stars 1.15k forks source link

Error management when editing profile #351

Open aiscrim opened 5 years ago

aiscrim commented 5 years ago

When something wrong happens in the ManageController/Index action (e.g. the user changes the email address but it's already in use by another user) an exception is thrown, resulting in a 500 response. Moreover, the details of the error are not clear enough, even locally: in the 'email address already in use' example above, all I get is:

ApplicationException: Unexpected error occurred setting email for user with ID ...

And nothing goes to the logs, so there's no way of understanding what's the problem apart from debugging it in VS.

For me it's quite urgent to solve this issue, so if you are busy I can fix it myself and open a PR, just let me know.

skoruba commented 5 years ago

Please, open a PR if you can, because I am currently busy. Do you try to setup email which is used for another user? I'll be happy for any fix/details on this. 😊 Thanks!

skoruba commented 5 years ago

I think this line is your issue: https://github.com/skoruba/IdentityServer4.Admin/blob/master/src/Skoruba.IdentityServer4.STS.Identity/Controllers/ManageController.cs#L80 Please, check the result in this line. Thanks.

aiscrim commented 5 years ago

Yes, exactly, and as you see when that call fails an exception is thrown. Instead I think it will be better to return a user-friendly message. I'll do it!

skoruba commented 5 years ago

You are right, in this case will be better to have some nice message. :) Thanks @aiscrim

aiscrim commented 5 years ago

Working on the fix I discovered a few other related flaws in the ManageController:

If you agree I can fix the first two issues in the same PR. The last one is a bit trickier for me to solve because it involves creating new localized strings in all the supported languages, so I'd just ignore it atm.