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

Improvements to client auth error logging #6214

Merged
merged 1 commit into from Dec 15, 2015
Merged

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Dec 14, 2015

The main point of this PR is to output a useful error in the server console whenever an error occurs during the client auth flow.

There are quite a few different things that can be wrong. I've now got it so that there's a different error for:

  1. client credentials missing
  2. client credentials invalid
  3. url mismatch

In the first two cases, the error sent to the client is just 'Access denied'. The nice error only appears on the server.

In the latter case we already have a much nicer error for the client.

Please note: I also added one other change to this whilst I was there - I made the localhost override only work for non-production blogs. I think that makes sense?

no issue

  • If client credentials are missing, or not valid, output a clear message in the server console
  • Still defaults to sending the 'access denied to url' error to the frontend

no issue

- If client credentials are missing, or not valid, output a clear message in the server console
- Still defaults to sending the 'access denied to url' error to the frontend
@ErisDS
Copy link
Member Author

ErisDS commented Dec 15, 2015

Removed my derpy switch back to localhost only being for production, and added a comment so I don't forget this again.

sebgie added a commit that referenced this pull request Dec 15, 2015
Improvements to client auth error logging
@sebgie sebgie merged commit e5ca725 into TryGhost:master Dec 15, 2015
@ErisDS ErisDS deleted the auth-errors branch February 7, 2016 16:14
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

Successfully merging this pull request may close these issues.

None yet

2 participants