-
Notifications
You must be signed in to change notification settings - Fork 21
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
Clarify Instructions for passport-jwt in README #32
Comments
Would you be willing to send a PR? |
@tmeasday I'd be glad to. Just to clarify, passport-jwt won't catch the error and respond with a 401 on its own as is right? I toyed with it a bit and wasn't able to find a way to make it work. I ended up doing a check within the graphqlExpress function like this:
|
Hi @azjkjensen -- wouldn't it be the |
@tmeasday for each call to the API, if the JWT is not included on the request the server should throw a 401. That's the standard as far as I know. Usually passport-jwt would throw the error on it's own if it detects the wrong/no token. But for some reason with the way we have things set up here it doesn't, therefore requiring the check for user I mentioned above. I am not familiar enough with passport-jwt and the configuration we've added in this repo, so I figured my fix is reliable enough. But there should be a way to fix this more in the background. |
Ahh right. Well I think if you are talking about a Perhaps all we need is some documentation/advice and an easy way to 401 when the user is missing on authenticated queries. |
You make a great point, I wasn't considering that it's not uncommon to have public and protected endpoints. I'll take a look and see what I can throw together as far as 401 handling goes. |
I just began using cgs and I found it a bit difficult to figure out the passport-jwt part. I think it would be very helpful to have instructions in the README regarding how to check for a failed authentication attempt, as passport-jwt won't automatically send back a 401 when you use it the way we have.
The text was updated successfully, but these errors were encountered: