fix(postgraphql): don't throw on auth header if auth not enabled #437
Conversation
| @@ -388,7 +388,7 @@ export default function createPostGraphQLHttpRequestHandler (options) { | |||
| if (debugGraphql.enabled) | |||
| debugGraphql(printGraphql(queryDocumentAst).replace(/\s+/g, ' ').trim()) | |||
|
|
|||
| const jwtToken = getJwtToken(req) | |||
| const jwtToken = getJwtToken(req, !!options.jwtSecret) | |||
There was a problem hiding this comment.
Let’s use your old implementation from #433. I think that’s much cleaner. I don’t think it is too important to have the validation in https://github.com/postgraphql/postgraphql/blob/d4fd6a4009fea75dbcaa00d743c985148050475e/src/postgraphql/withPostGraphQLContext.ts#L121, but thanks so much for thinking about it!
We don’t have to delete the validation because it is still be useful for withPostGraphQLContext by itself. I don’t think missing the validation in this one case will be too big a deal. Do you agree?
There was a problem hiding this comment.
Sounds good! Updated the PR with the original version.
nbushak
force-pushed
the
postgraphql-auth
branch
from
ffc9922
to
f663252
Compare
|
Hi folks, is there anything I need to do to get this reviewed at this point? Sorry, I'm sort of a n00b with Github pull requests :) Used Phabricator at my last company. |
|
Hi all ... is there any update on when this might be reviewed? |
|
Sorry, both Caleb and I are ran off our feet right now. Normal service will resume in a couple weeks. |
|
No problem at all!! Thanks for the update!!
…Sent from my iPhone
On 3 May 2017, at 17:57, Benjie Gillam <notifications@github.com> wrote:
Sorry, both Caleb and I are ran off our feet right now. Normal service will
resume in a couple weeks.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#437 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AG49L_VEjErPJrX4-0gD6xQvyZzfKE0mks5r2LILgaJpZM4M5mxP>
.
|
|
Thanks @nbushak 🙏 |
The documentation says "And if you don’t want authorization, just don’t set the --secret argument and PostGraphQL will ignore all authorization information!"
It turns out postgraphql does read auth information and throws an error if the format is not as expected. This prevents folks from using the Authorization header for non-JWT, like Basic auth.
#433 was my first go at this change.