Add support for JWT secret callbacks to allow JWKS (required for Auth0)

[MaxDesiatov]

I'm using Auth0 and secrets for JWT middleware in their example code is fetched through jwks. This isn't possible with current postgraphql API, even v4 as it only accepts secrets supplied as strings, not callback function.

Specifically I would like jwtSecret property to accept something like

import * as jwks from 'jwks-rsa';
...

app.use(postgraphql(connectionString, 'public', {
      ...
      jwtAudiences: ['custom.audience.id'],
      jwtSecret: jwks.expressJwtSecret({
          cache: true,
          rateLimit: true,
          jwksRequestsPerMinute: 5,
          jwksUri
      })
      graphiql: true,
      ...
}));

I see that it will probably be relatively simple to refine code at https://github.com/postgraphql/postgraphql/blob/5979eafcb2fe4c675870b265aeca8ff68d7d053e/src/postgraphql/withPostGraphQLContext.ts#L125-L130 but would it require more changes for PgJWTPlugin as well for everything to work?

[benjie]

Cool 👍 I'd love a PR for this.

In PgJWTPlugin, we use pgJwtSecret off of the options object here:

https://github.com/graphile/graphile-build/blob/7ecec2c3a851c5ef683f87332ac163c8690c1625/packages/graphile-build-pg/src/plugins/PgJWTPlugin.js#L84

There's no reason I can think of that this couldn't be a synchronous function.

However, it sounds like you want it to be an asynchronous function... This definitely poses a few complications.

---

One option we have is to make pg2gql asynchronous, but I'm not certain what the performance overhead would be of making all the code that uses it currently into promises... I'd have to look into it before I was happy with that change being made.

Since resolvers already automatically await promises we could make it so that pg2gql returns either a value or a promise to a value and let GraphQL sort it out. This would have less of a performance overhead (since only async mappers would take the hit), but is maybe complicated for arrays?

[Click to expand] I've just done a little test, and actually that seems to be fine for arrays...

const {
  GraphQLSchema,
  GraphQLObjectType,
  GraphQLInt,
  GraphQLList,
  graphql,
} = require("graphql");

async function main() {
  const schema = new GraphQLSchema({
    query: new GraphQLObjectType({
      name: "Query",
      fields: {
        foo: {
          type: new GraphQLList(GraphQLInt),
          resolve() {
            return [Promise.resolve(1), 2, Promise.resolve(3)];
          },
        },
      },
    }),
  });
  const query = `{ foo }`;
  const result = await graphql(schema, query);
  console.log(result);
}

main();

{ data: { foo: [ 1, 2, 3 ] } }

We'd have to refactor any code that uses the result of pg2gql, but I think at the moment it's just returned by resolvers (needs checking) and so wouldn't require much effort.

gql2pg would probably want similar treatment for consistency, that might require more work.

---

An alternative approach would be to put the JWT secret on each GraphQL context, via:

https://github.com/postgraphql/postgraphql/blob/5979eafcb2fe4c675870b265aeca8ff68d7d053e/src/postgraphql/withPostGraphQLContext.ts#L74-L77

and then expose context to pg2gql. This would require getting a JWT secret for every request ahead of time though - I guess they're cached but I'm not sure how expensive this is?

---

Another approach would be to get new secrets on a timer (say every 5 minutes) and make the synchronous pgJwtSecret function mentioned above return it.

---

Another approach could be to handle JWT fields in a custom way, using a custom resolver. This may be the best way of solving it for just this one issue; but given this has come up I wouldn't be surprised if it comes up again, so it's probably worth solving generically.

---

Hmmm...

[MaxDesiatov]

I think from API perspective, it's beneficial to support the function signature jwks-rsa provides, and express-jwt middleware accepts, especially as postgraphql by itself is already an express-compatible middleware.

Looking more at the documentation and code it looks like PgJWTPlugin.js generates new tokens, which is not something I'm interested in. The "secret" you get from JWKS dynamically is actually an Auth0 public key that allows you to verify tokens generated by Auth0. And Auth0 tokens are the only tokens I work with. It looks like PgJWTPlugin.js would require yet another secret to generate it's own tokens, I'm not sure that tokens signed by Auth0 public key are secure in any way.

As for solving this generically, I'm not quite sure why JWT token parsing and verification is so tightly coupled with postgraphql. Wouldn't it be better to leave that to a separate user-provided auth middleware, which then only passes a role to postgraphql middleware?

[benjie]

I've never used Auth0, so excuse my lack of familiarity. Yes PgJWTPlugin is responsible for generating JWTs and serving them through GraphQL.

If you're only interested in verifying JWTs then you only need to worry about the section of withPostGraphQLContext that you referenced above.

I'm not quite sure why JWT token parsing and verification is so tightly coupled with postgraphql. Wouldn't it be better to leave that to a separate user-provided auth middleware, which then only passes a role to postgraphql middleware

PostGraphQL can be used as a stand-alone application (by running it via the CLI), so it makes sense to bundle these features in as they are required to secure the database. I'd be happy to accept a PR that adds a way for people to pass in pre-request settings to use for the PostgreSQL connection so that authentication can be performed outside of PostGraphQL; maybe something like getSettingsFromRequest that can be passed to the library and is used by setupPgClientTransaction

https://github.com/postgraphql/postgraphql/blob/233f55c53f8c982cb566b48b63e0fa5bcc7ed1fd/src/postgraphql/withPostGraphQLContext.ts#L96-L98

[MaxDesiatov]

Would it be reasonable for a postgraphql user to supply their own setupPgClientTransaction? The signature of withPostGraphQLContext might need to be changed to allow that, maybe in a backward-compatible manner. Although while v4 is not released yet, isn't now the timeframe for this kind of API change to be introduced?

[benjie]

Maybe that's the most flexible way of doing it, but I fear that it would be very easy to do wrong (e.g. forgetting to commit the query when an error occurs) and mistakes there may be blamed on the library. I'd rather make sure that we maintain control of the "connect > begin > [CODE HERE] > commit > release connection" flow. Is there a reason we can't just achieve it by passing more items to be set via set_config()?

[MaxDesiatov]

From what I see in the current implementation, commit happens in withPostGraphQLContext, which also wraps setupPgClientTransaction in a try block, so both early returns and throws are guarded regardless of what's passed. I think even with fully configurable setupPgClientTransaction the flow is maintained, because setupPgClientTransaction is called and guarded in the right place of the flow you've described, if I'm correct.

[benjie]

Sweet, in which case 👍 (Sorry, I'm working on another project right now so haven't had a chance to scan over the code again.)

[MaxDesiatov]

closing this after #556 merged to both v3 and v4

[benjie]

If you fancied writing a Guide for this on graphile.org that would be awesome!

[MaxDesiatov]

would you like this as a section of https://www.graphile.org/postgraphile/security/ or a separate doc page?

[benjie]

A separate guide specifically about Auth0 would be great. You need to add an entry to nav.json and create the relevant file with the correct frontmatter in src/pages/postgraphile/. Also you have to restart gatsby every time you add a file for some reason.

I can give you more guidance if you need it later but I'm busy this afternoon

[lasersox]

Would love to see an example or some documentation!

[levino]

This should really be done.