keycloak
Show full error details in admin and account consoles
Updates the admin and account consoles so that the full details of error messages coming from the REST API are shown. For example, given the following response from the API:
{
"error": "It is illegal to add execution to a built in flow",
"error_description": "For more on this error consult the server log at the debug level."
}
Before
After
This PR also unifies the logic for alerts between the admin and account consoles by using the same components from the shared UI library.
Closes #30705
@jonkoops does this mean that when using the js admin-client when an error occurs it will log them in the keycloak server logs? I was stuck thinking the following was not working kcAdminClient.setAccessToken(token) but it was a simple permissions error.
I was about to work on a PR in the fetchWithError.ts, because no matter what the keycloak server returns as an error it always says Network response was not OK. instead of logging what the keycloak server returned. But if the error detail is now going to show in the keycloak server (which at the moment it does not), that PR might not be necessary.
does this mean that when using the js admin-client when an error occurs it will log them in the keycloak server logs?
No, this is about errors from the REST API not being displayed in full in the alerts in the admin and account consoles. Whether or not errors are logged on the server-side is another story. See https://github.com/keycloak/keycloak/issues/30705
@keycloak/ui it seems that these changes fail some of the tests, likely because the notifications being checked now have a different text associated with them. I unfortunately don't have time to work on this further, so I would appreciate if it if someone could take the time to fix these.
Assigning this to @edewit as he has volunteered to fix this one up :1st_place_medal:
I've been struggling with this error message myself and I'm happy this is now fixed. This change seems to be larger than I expected, so I'm afraid it would be hard to backport to KC24/KC25. If you think there is a chance to backport a light-weight version, please do if your time permits.
If you think there is a chance to backport a light-weight version, please do if your time permits.
I discussed this with @edewit as well, and we're quite sure a minimal back-port is not possible. We'd have to do a pretty similar change to what is here in this PR, as well as checking all those changed imports. It's possible, but not sure this would outweigh the costs.
@ssilvert as the UI team lead, what is your take on this? Do you feel effort should be put into back-porting this?
we're quite sure a minimal back-port is not possible.
Sure, no pressure. Just asking, especially as the server is usually not in debug mode to provide those details, and someone using the UI might a different person who has access to the logs.
Yes, for sure. I originally set the impact for this bug as low, but I'd honestly consider it more like high impact considering the implications for debugging for users and quality of bug reports. Perhaps we should consider a back-port in some shape or form.
If you think there is a chance to backport a light-weight version, please do if your time permits.
I discussed this with @edewit as well, and we're quite sure a minimal back-port is not possible. We'd have to do a pretty similar change to what is here in this PR, as well as checking all those changed imports. It's possible, but not sure this would outweigh the costs.
@ssilvert as the UI team lead, what is your take on this? Do you feel effort should be put into back-porting this?
@jonkoops @edewit If it's a lot of effort, I don't think a backport is needed. We can just be happy that we get better error messages going forward.
