-
Notifications
You must be signed in to change notification settings - Fork 437
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
Error handling support for better user experience #976
Comments
We really need this. In order not to break current API, I suggest that |
In V12, we added an I think throwing the error through an observable is a nice idea when logging in. The errors mentioned above can be thrown when logging in or when checking the auth (so coming back from the sts), is that correct? Thanks |
Just checked: Can you provide a sample with a config from our samples and a reproducible error? What I do see is an error at the server, but we need it when coming back. If you provide a sample with an error, we can have a look. |
Yes @FabianGosebrink. The errors can come when logging-in or when checking the auth (hence, also when refreshing with the iframe). The errors from the STS are included in the callback URL, so the library should check those and add them to the error object thrown. I have to check whether I can provide you with some reproducible errors. Let me get back to you at a later stage, please. :) |
We would have to map the errors in the url to a consistent error message. We already have a "new" error property in V12. Maybe this is doing it well, but I can test with your example then. Thanks! |
In the case of IAT rejected as Gabriel mentioned, when using checkAuthIncludingServer() the errorMessage provided in the LoginResponse is undefined. [WARN] 0-Client - authCallback Validation, iat rejected id_token was issued too far away from the current time I am looking forward to the full error handling feature to support all the different error scenarios so we can log to developers and notify users what is actually happening. |
Is this issue still relevant? |
I think it is. There's only an error property in the response, not really that it handles the different scenarios. |
We are handling the different scenarios or events with the |
The main problem I see with this is not actually the way errors are returned, but the errors themselves, as they are not typed at all. Most of the errors are part of the spec, and can be typed. Maybe a union, for those errors that cannot be mapped, so default to a generic object. The known errors should be objects, with errors codes along message descriptions. This is to allow the client app to identify the error by code, and not necessarily use the string (or show it in the console). Showing it to the client will not work for i18n apps. |
In my experience, the library just ignores the error message returned from the server. I have a failed login, redirecting to something like this:
Instead of containing the error message from the url. the checkAuth produces the error message |
Is your feature request related to a problem? Please describe.
I have successfully implemented this library in my project, however like in any other real-live app, errors do happen. Sometimes these errors are beyond the scope of our implementation, but still, they frustrate the experience of the users who do not know what to do.
Currently, when an error happens logging in the user is just redirected to the
unauthorizedRoute
with a generic "Unauthotized" message and error screen. There is no way to inform the user of what happened, since both this library, or the Authentication server may return an error.I cannot find any mention to error handling in the docs.
Describe the solution you'd like
Ideally, this library offers a mechanism to access the error returned, wither in the subscription observer of the
checkAuth
method, or as an event, or any other way the responsible team consider.The most common error that happens in our app is "iat validation", but we cannot identify this error and inform the users that their time is not correct, and they have to fix this. We noticed more people than expected have their system time set ahead, in order to be always on time for their meetings and appointments.
However, other errors are found such a code which was issued with a different state etc.
Moreover, OIDC spec defines a set of errors that the server may return, which ideally can be accessed through the library (
error
,error_description
,error_uri
,state
).Ref: https://openid.net/specs/openid-connect-core-1_0.html#AuthError
Describe alternatives you've considered
I thought about these possible solutions:
Subscriber observer error.
This error could be sent to the "Unauthorized" route using
OidcService.lastError$
for instance, where the last error would be available so the route can either handle it, or display it to the user in a UX-friendly way.The text was updated successfully, but these errors were encountered: