Skip to content
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

Define and implement a coherent site-wide error handling policy #196

Closed
2 of 6 tasks
spruce-bruce opened this issue Oct 25, 2017 · 1 comment
Closed
2 of 6 tasks

Comments

@spruce-bruce
Copy link
Collaborator

spruce-bruce commented Oct 25, 2017

Currently errors are handled a couple different unpredictable ways.

Some errors encountered by the node-oidc-provider redirect the user back to the client app with #error=message in the url (if the response_type doesn't match the client this happens)

Other errors encountered by the node-oidc-provider dump an error to the screen. These errors can be intercepted and styled to be nicer. If a redirect_uri is wrong on a login link you get an error like this, same when client id is not found.

If there is an error encountered by user management screens controlled by the platform then we dump the json error to the screen.

We should define some rules for when to display an error and when to redirect to the client to handle the error. (something like, if this error could possibly happen during normal production use of the platform then it should always redirect back to the client. If the error is caused by some misconfiguration or misuse that couldn't/shouldn't happen during production then we show an error screen).

We should also style up an error screen to use so we stop dumping json to the screen

We should also stop using ?status=whateveer and start using #status=whatever when we redirect back to the client app.

Tasks

  • Introduce an error template that can be used by the node-provider library to render errors
  • Extend the error template to work as a generic error screen for hapi endpoints as well
  • Update the error template to know about the environment. If production, don't display any debugging info. If not production, display lots of debugging info.
  • Review errors currently being returned to app by node-provider and the hapi server and make them conform to the same structure (hopefully with some sort of CODE value and some sort of MESSAGE value). Make sure we return errors to the app the same way.
  • Update user routes GET/POST endpoints to redirect to the app with an #error when query validation fails or to show the error screen when redirect uri can't be validated. Make sure POST endpoints still render the form when payload validation fails.
  • Document all error codes that might be returned to the client app
@spruce-bruce
Copy link
Collaborator Author

OAuth error responses documented here: https://tools.ietf.org/html/rfc6749#section-4.1.2.1
OIDC authentication error response documented here: http://openid.net/specs/openid-connect-core-1_0.html#AuthError

The rule is: If the client or redirect_uri can't be validated then you MUST NOT redirect the user automatically back to the redirect_uri. All other OIDC/OAuth errors are handled by redirecting to redirect_uri with an error response in the uri.

I'm going to make our user GET/POST endpoints adhere to that same rule with one exception: email tokens. If email tokens are invalid (usually because of the token expiring) then I'm going to keep them on the ID server and show them an error message.

@synbot synbot added the blocking Automatically assigned to indicate this issue is a blocker for another issue. label Jul 31, 2018
@synbot synbot removed the blocking Automatically assigned to indicate this issue is a blocker for another issue. label Feb 20, 2019
@synbot synbot added the blocking Automatically assigned to indicate this issue is a blocker for another issue. label Feb 22, 2019
@synbot synbot closed this as completed Feb 26, 2019
@synbot synbot removed the blocking Automatically assigned to indicate this issue is a blocker for another issue. label Feb 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants