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

feat(options) postgraphile: add contextOptions to allow custom jwt claims #3915

Merged
merged 7 commits into from Apr 3, 2023

Conversation

MarkLyck
Copy link
Contributor

@MarkLyck MarkLyck commented May 7, 2022

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

This adds contextOptions to list of possible options for the postgraphile handler.

Currently the postgraphile handler only exposes buildOptions (called just options in the docs) which per the postgraphile docs cannot be used to specify pgSettings

Our production postgres deployment makes use of row level security rules that require custom jwt claims, which is fully supported in postgraphile via the pgSettings object.

However graphql-mesh doen't provide any way to define pgSettings. This PR allows the user to change postgraphile's context settings (which is where pgSettings needs to be defined).

Just like the regular postgraphile build options, it is optional whether or not to specify this.

Fixes #3913

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • I tested my changes locally using the use-case mentioned in the ticket.
  • Given the only test there is for this handler is it('dummy', async () => {}); I'm not familiar enough with the test methods for the graphql-mesh server, to set up a full test method and suit for adding this extra option.

Test Environment: Real project using graphql-mesh with postgraphile.

  • OS: MacOS
  • "@graphql-mesh/postgraphile": "^0.20.9",
  • NodeJS: 16.x & 18.0

Checklist:

  • I have followed the CONTRIBUTING doc and the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests and linter rules pass locally with my changes

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

For my use-case I only need the pgSettings to be exposed in a function with access to the request or context so I can add the necessary jwt claim tokens to get postgraphile to respect our postgres Row Level Security rules.

My first thought was to only expose pgSettings, but in case anyone needs to overwrite some of the other context options in the future, I opted to expose a contextOptions function, that just like the regular options spreads and overwrites and default settings if the user chooses to do so.

I would be happy to have a discussion if there's any other way to implement this, but it seems like a pretty straight forward option.

@changeset-bot
Copy link

changeset-bot bot commented May 7, 2022

🦋 Changeset detected

Latest commit: e2ff960

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 47 packages
Name Type
@graphql-mesh/postgraphile Patch
@graphql-mesh/types Patch
postgres-geodb-example Patch
@graphql-mesh/cli Patch
auth0-example Patch
cloudflare-workers Patch
example-gcp Patch
graphql-file-upload-example Patch
grpc-example Patch
grpc-reflection-example Patch
hasura-openbrewery-geodb Patch
hello-world-esm Patch
json-schema-hello-world Patch
covid-mesh Patch
json-schema-example Patch
json-schema-fhir Patch
json-schema-file-upload Patch
json-schema-subscriptions Patch
mongoose-example Patch
mysql-employees Patch
mysql-rfam Patch
neo4j-example Patch
nextjs-apollo-example Patch
nextjs-sdk-example Patch
odata-microsoft-graph-example Patch
odata-msgraph-programmatic-ts Patch
odata-msgraph-programmatic Patch
odata-trippin-example Patch
javascript-wiki Patch
typescript-location-weather-example Patch
openapi-meilisearch Patch
openapi-stackexchange Patch
openapi-stripe Patch
openapi-subscriptions Patch
openapi-youtrack Patch
openwhisk-example Patch
programmatic-batching-example Patch
reddit-example Patch
country-info-example Patch
soap-demo Patch
soap-netsuite Patch
spacex-cfw Patch
chinook Patch
thrift-calculator Patch
type-merging-batching-example Patch
federation-gateway Patch
gateway-example Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented May 7, 2022

@MarkLyck is attempting to deploy a commit to the The Guild Team on Vercel.

A member of the Team first needs to authorize it.

@MarkLyck MarkLyck changed the title add contextOptions postgraphile handler: add contextOptions to allow custom jwt claims May 7, 2022
@MarkLyck MarkLyck changed the title postgraphile handler: add contextOptions to allow custom jwt claims postgraphile handler: add contextOptions to allow custom jwt claims May 7, 2022
@MarkLyck MarkLyck changed the title postgraphile handler: add contextOptions to allow custom jwt claims feat(handler-options) postgraphile: add contextOptions to allow custom jwt claims May 9, 2022
@MarkLyck MarkLyck changed the title feat(handler-options) postgraphile: add contextOptions to allow custom jwt claims feat(options) postgraphile: add contextOptions to allow custom jwt claims May 9, 2022
@theguild-bot theguild-bot mentioned this pull request Aug 11, 2022
@ardatan ardatan force-pushed the master branch 2 times, most recently from c3c0b9e to 696f9c7 Compare February 14, 2023 05:53
@ardatan ardatan merged commit aea1347 into ardatan:master Apr 3, 2023
12 checks passed
@theguild-bot theguild-bot mentioned this pull request Apr 24, 2023
@theguild-bot theguild-bot mentioned this pull request Sep 28, 2023
This was referenced Apr 30, 2024
This was referenced May 7, 2024
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.

Support custom jwt claims in postgraphile handler.
2 participants