graphile / postgraphile-apollo-server

41 stars 5 forks source link

headers in request are empty #2

Closed bardeut closed 5 years ago

bardeut commented 5 years ago

const { http: req } = graphqlRequest;

req.headers is empty resulting in ignoring the jwtToken in request.

I ended up sending my jwtToken inside ApolloServer definition:

context: ({ req }) => ({
          jwtTokenFromApollo: getJwtToken(req)
        }),

copy the code and use: const { jwtTokenFromApollo} = graphqlContext; to get the jwt to work, instead of getting it from graphqlRequest.

benjie commented 5 years ago

We do that here:

https://github.com/graphile/postgraphile-apollo-server/blob/d94ae7d8b05d50532f68f6a67618867ec8607897/index.js#L55

Which is called from here:

https://github.com/apollographql/apollo-server/blob/875944ea8358caa2140e6f7fff861f7a8f0149e4/packages/apollo-server-core/src/requestPipeline.ts#L275-L281

Which references requestContext which is built here:

https://github.com/apollographql/apollo-server/blob/ceee5db460f002a80b837ff0dba1743d4237aaf0/packages/apollo-server-core/src/runHttpQuery.ts#L234

Which is called from these two places:

This function just passes through the httpRequest.request as http:

https://github.com/apollographql/apollo-server/blob/ceee5db460f002a80b837ff0dba1743d4237aaf0/packages/apollo-server-core/src/runHttpQuery.ts#L384

So we're looking at the processHTTPRequest function:

https://github.com/apollographql/apollo-server/blob/ceee5db460f002a80b837ff0dba1743d4237aaf0/packages/apollo-server-core/src/runHttpQuery.ts#L179-L184

Which is called from runHttpQuery in the same file, which is called from all the different modules; you've not said which one you're using so I'm going to guess express:

https://github.com/apollographql/apollo-server/blob/ceee5db460f002a80b837ff0dba1743d4237aaf0/packages/apollo-server-express/src/expressApollo.ts#L35-L41

Which processes the request via nodeHttpToRequest:

https://github.com/apollographql/apollo-server/blob/ae9da10e625cf283568ba6d29cea8c3e69a7a03f/packages/apollo-server-core/src/nodeHttpToRequest.ts#L4-L19

That does seem to include the headers; and in my previous testing the headers including JWTs were working (see example here: https://github.com/graphile/postgraphile-example-apollo-server )

So we'll need more information to figure this out. Can you share more code?

bardeut commented 5 years ago

Thanks for the reply.

I am using apollo-server-express:


makeSchemaAndPlugin(
  myPgPool,
  SCHEMA,
  postgraphqlOptions
).then (({schema, plugin }) => {

      const server = new ApolloServer({
        schema: schema,
        tracing: true,
        engine: false,
        cacheControl: true,
        formatError: error => {
          console.log(error);
          return error;
        },
        formatResponse: result => {
          return result;
        },
        context: ({ req }) => ({
          jwtTokenFromApollo: getJwtToken(req)
        }),
        plugins: [plugin]
      });

      withSession(app);

      app.use('/graphql', withSessionValidator);

      server.applyMiddleware({app, path: '/graphql'});

      let engineOptions = {
        apiKey:  apolloApiKey,
        logging: {
          level:  'INFO'
        },
        origins: [
          {
            supportsBatch: true
          }
        ],
        sessionAuth: {
          header: 'Authorization'
        }
      }

      const engine = new ApolloEngine(engineOptions)

      // Call engine.listen instead of app.listen(port)
      engine.listen({
          port: port,
          expressApp: app
        },
        () => console.info(`GraphQL service listening on port ${port}`)
      )
    })
    .catch(error => {
      console.error(error)
    })
benjie commented 5 years ago

Where are you using jwtTokenFromApollo?

bardeut commented 5 years ago

As I wrote before, I was trying to solve the issue since we are in a rush to upgrade postgraphql to postgraphile. I copied the code from index.js here and added the line: const { jwtTokenFromApollo} = graphqlContext; under line 55: const { http: req } = graphqlRequest;. I'm using it in: const jwtToken = jwtSecret ? jwtTokenFromApollo : null; instead of line 65 in index.js : const jwtToken = jwtSecret ? getJwtToken(req) : null;

benjie commented 5 years ago

OIC šŸ‘ If there's a regression here I suspect it's on the Apollo Server side rather than this plugin (this is particularly likely with the headers being empty as you say). JWT was working when I wrote this.

Out of interest, what are your reasons for using Apollo Server rather than the PostGraphile server - is it just the Apollo Engine metrics or is there more to it?

bardeut commented 5 years ago

We are using apollo engine mostly for the caching options (using memcache store in Prod). We just switched from postgraphql to postgraphile so I didn't get the change to check the postgraphile server on it's own.

benjie commented 5 years ago

Do you find you need much in the way of caching with PostGraphile? It's significantly faster than PostGraphQL was.

Aside, this issue is fixed in v0.1.1 šŸ‘

bardeut commented 5 years ago

Thanks for the fix :) The upgrade is still in staging, so I have no idea...I will know better soon.

benjie commented 5 years ago

Let me know when you know :)

bardeut commented 5 years ago

Hi, Just giving you an update... Unfortunately we still have to use the apollo server cache, otherwise our website is very slow. I'm sure we have optimizations to do but for now I see a big difference with and without the cache.

benjie commented 5 years ago

I'd be very interested to know more about what you are caching, and what the invalidation rules are on it. Would you be interested in showing me your stack on a screen sharing session for an hour or so at some point?

bardeut commented 5 years ago

Well actually we just scheduled for Wednesday. Iā€™m the one with the PayPal issues :)

Bar Deutsch

On 5 Aug 2019, at 16:43, Benjie Gillam notifications@github.com wrote:

I'd be very interested to know more about what you are caching, and what the invalidation rules are on it. Would you be interested in showing me your stack on a screen sharing session for an hour or so at some point?

ā€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

benjie commented 5 years ago

Ah; fantastic - catch up on Wednesday :+1: