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

API: Make the 401 error format consistent #5499

Closed
ErisDS opened this issue Jun 30, 2015 · 2 comments
Closed

API: Make the 401 error format consistent #5499

ErisDS opened this issue Jun 30, 2015 · 2 comments
Labels
affects:api Affects the Ghost API good first issue [triage] Start here if you've never contributed before.

Comments

@ErisDS
Copy link
Member

ErisDS commented Jun 30, 2015

At the moment the error returned from the API when you don't provide authentication is in a different format / structure to other error messages from the API.

There's a super easy way to see what I mean and test it using curl and the authentication/setup endpoint:

curl http://localhost:2368/blog/ghost/api/v0.1/authentication/setup/
{"setup":[{"status":true}]}

curl -X POST http://localhost:2368/blog/ghost/api/v0.1/authentication/setup/
{"errors":[{"message":"Setup has already been completed.","errorType":"NoPermissionError"}]}

curl -X PUT http://localhost:2368/blog/ghost/api/v0.1/authentication/setup/
{"type":"error","message":"Please Sign In","status":"passive"}

The GET and POST requests both return a JSON response in the correct format, GET returns a 200 error with a top level key and some data. POST returns a 403 error with a top level errors key an an error object.

However the PUT request returns a 401 error with an error object without a top level key.

There is a rudimentary explanation of the design principles behind the Ghost API over on the wiki which should shed some more light on what is wrong with this response.

The code generating the response is here: https://github.com/TryGhost/Ghost/blob/master/core/server/middleware/middleware.js#L122

And an example of how errors are sent correctly is here: https://github.com/TryGhost/Ghost/blob/master/core/server/middleware/api-error-handlers.js#L12

This should be a straightforward change, but some testing needs to be done to ensure it gets handled properly by the ember admin application.

@ErisDS ErisDS added good first issue [triage] Start here if you've never contributed before. affects:api Affects the Ghost API labels Jun 30, 2015
@kowsheek
Copy link
Contributor

Are there existing tests for this?

@ErisDS ErisDS mentioned this issue Jun 30, 2015
31 tasks
@ErisDS ErisDS added this to the Current Backlog milestone Jun 30, 2015
kowsheek added a commit to kowsheek/Ghost that referenced this issue Jun 30, 2015
issue TryGhost#5499

- Update error object being sent from authenticate method
@jxhn
Copy link
Contributor

jxhn commented Jul 9, 2015

I think this can be closed, it may just be the PR missing the closes reference?

@ErisDS ErisDS closed this as completed Jul 9, 2015
@ErisDS ErisDS removed this from the Current Backlog milestone Oct 13, 2015
@ErisDS ErisDS mentioned this issue Oct 20, 2015
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:api Affects the Ghost API good first issue [triage] Start here if you've never contributed before.
Projects
None yet
Development

No branches or pull requests

3 participants