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

90% - Made validate token call callback with err on invalid token #22

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

scaleupcto
Copy link
Contributor

The existing validateToken method had unreachable code for case INVALID_TOKEN, instead it was throwing the uncaught error from validateOps. The express middleware wrapper caught this behaviour and called its callback with the correct error.

Instead just have validateToken do that in the first place.

@scaleupcto scaleupcto changed the title Made validate token call callback with err on invalid token 90% - Made validate token call callback with err on invalid token Sep 19, 2016
@rsinger
Copy link
Member

rsinger commented Sep 19, 2016

Would it be worth adding a simple unit test to token_validation_tests to make sure we don't ever undo your good work here?

@scaleupcto
Copy link
Contributor Author

There is an existing test case on the validateHTTP, which should still pass, but guess no harm to do validateToken directly too

@scaleupcto
Copy link
Contributor Author

Actually, there wasn't one for no token, just an existing invalid token. Have added one that covers both scenarios (as one method depends on the other).

@rsinger
Copy link
Member

rsinger commented Sep 19, 2016

👍

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.

2 participants