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

Login error responses #47

Closed
heymarkreeves opened this issue Aug 28, 2014 · 7 comments
Closed

Login error responses #47

heymarkreeves opened this issue Aug 28, 2014 · 7 comments
Assignees
Labels

Comments

@heymarkreeves
Copy link
Collaborator

Hi, Jamie!

Can you confirm that for the /tokens (login) endpoint, we should act on the following:

  • Email submitted does not exist - 500 internal server error
  • Email exists, but wrong password - 403 Forbidden error

I just want to be sure we're acting on those rather than explicit errors in a returned JSON packet. We're seeing those as server errors from the Ajax call.

One consideration: If there was an actual application/server error, those error values might trigger false errors to the user. I.e., getting a 500 error from the server and saying "Your email address was not found," when the user had actually registered and used the correct address.

Thanks!

Mark

@jamiefolsom
Copy link
Collaborator

Your description sounds accurate to the current API, but I am not sure it's optimal. I will do some research. Ideally, the response would be the correct status code for a non-existent user.

@heymarkreeves
Copy link
Collaborator Author

Hi, @jamiefolsom! This is actually similar to #48. It may be optimal to get back json-encoded messages we can act on for consistency? Thanks!

@jamiefolsom
Copy link
Collaborator

Hi @circa1977 OK, so I'm not sure I know what the status of this is now -- are we sticking with status code only responses, no payload, just changing which ones are returned? For example, it doesn't seem right to return 500 for a routine, correct request, IMHO. Thanks.

@heymarkreeves
Copy link
Collaborator Author

Hi @jamiefolsom - Per #48, I think:

  • Email submitted does not exist would return 404
  • Email exists but wrong password... This I'm not sure. This Stack Overflow thread suggests a 400 response for a validation error. 403 is an option in those answers, too.

@jamiefolsom
Copy link
Collaborator

Hi @circa1977 I wasn't clear, sorry; I can figure out which codes to return, and document those for you, but I want to be sure before I make changes that you're in agreement with the API returning payload-free status-code-only responses to all requests that warrant them (e.g. for signups attempting to create accounts with an already-used email, or for login attempts where the email doesn't exist or for login attempts with existing email, wrong password). Just above my question from this morning, it appeared to still be an open question whether your front end needed our API to return a JSON payload in all cases. I think I am belaboring something that we are beyond now, but I don't want to make assumptions. Thanks!

@heymarkreeves
Copy link
Collaborator Author

Hi, @jamiefolsom! I appreciate the confirmation. Either way, we're going to have to create a function that will parse the response, so I'm on board with the status code approach. We have plenty of examples of parsing those in JavaScript/jQuery, so -- barring any unforeseen issues -- we should be fine to make that work and stick with a true RESTful implementation.

@mailbackwards
Copy link
Collaborator

@circa1977 This is now in PR #52.

  • Email exists and valid password returns a 200 with the authentication token in the payload
  • Email does not exist returns a 404 with boilerplate payload ("User does not exist")
  • Email exists but wrong password returns a 403 with boilerplate payload ("Authentication failed")

The error payloads are not necessary if they complicate things, but since the 200 response does require a payload (the authentication token) I kept them in for now. I can retool this if there's a better approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants