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

[jwt] fix unauthorized being thrown in onRequest #3149

Merged
merged 3 commits into from
Dec 22, 2023
Merged

Conversation

EmrysMyrddin
Copy link
Collaborator

Description

The JWT plugin was parsing and checking the jwt token in the onRequest hook. The implementation was simply throwing an error when not authenticated or on bad jwt token. The problem is that errors thrown in onRequest hook are not handled by yoga and are bubling up directly to the actual HTTP server implementation.

When using express, koa or fastify, this was resulting in a response with 500 status code.
When using Node's http server, this was resulting in simply crashing the server.

To fix this, we should always using onRequestParse over onRequest.

fixes #3147

Copy link

changeset-bot bot commented Dec 22, 2023

🦋 Changeset detected

Latest commit: 8f302c7

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

This PR includes changesets to release 1 package
Name Type
@graphql-yoga/plugin-jwt 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

Copy link
Contributor

github-actions bot commented Dec 22, 2023

Apollo Federation Subgraph Compatibility Results

Federation 1 SupportFederation 2 Support
_service🟢
@key (single)🟢
@key (multi)🟢
@key (composite)🟢
repeatable @key🟢
@requires🟢
@provides🟢
federated tracing🟢
@link🟢
@shareable🟢
@tag🟢
@override🟢
@inaccessible🟢
@composeDirective🟢
@interfaceObject🟢

Learn more:

Copy link
Contributor

github-actions bot commented Dec 22, 2023

💻 Website Preview

The latest changes are available as preview in: https://e3ca9496.graphql-yoga.pages.dev

Copy link
Contributor

github-actions bot commented Dec 22, 2023

✅ Benchmark Results

     ✓ no_errors{mode:graphql}
     ✓ expected_result{mode:graphql}
     ✓ no_errors{mode:graphql-jit}
     ✓ expected_result{mode:graphql-jit}
     ✓ no_errors{mode:graphql-response-cache}
     ✓ expected_result{mode:graphql-response-cache}
     ✓ no_errors{mode:graphql-no-parse-validate-cache}
     ✓ expected_result{mode:graphql-no-parse-validate-cache}

     checks.......................................: 100.00% ✓ 407724     ✗ 0     
     data_received................................: 1.7 GB  14 MB/s
     data_sent....................................: 82 MB   686 kB/s
     http_req_blocked.............................: avg=1.45µs   min=961ns    med=1.31µs   max=246.29µs p(90)=1.93µs   p(95)=2.12µs  
     http_req_connecting..........................: avg=2ns      min=0s       med=0s       max=135.08µs p(90)=0s       p(95)=0s      
     http_req_duration............................: avg=376.25µs min=215.45µs med=331.74µs max=14.87ms  p(90)=533.66µs p(95)=551.88µs
       { expected_response:true }.................: avg=376.25µs min=215.45µs med=331.74µs max=14.87ms  p(90)=533.66µs p(95)=551.88µs
     ✓ { mode:graphql-jit }.......................: avg=284.76µs min=215.45µs med=263.67µs max=14.87ms  p(90)=293.19µs p(95)=305.58µs
     ✓ { mode:graphql-no-parse-validate-cache }...: avg=562.07µs min=472.87µs med=533.48µs max=10ms     p(90)=572.66µs p(95)=610.15µs
     ✓ { mode:graphql-response-cache }............: avg=349.03µs min=274.72µs med=331.66µs max=6.24ms   p(90)=358.75µs p(95)=368.73µs
     ✓ { mode:graphql }...........................: avg=372.95µs min=290.09µs med=341.8µs  max=13.05ms  p(90)=380.65µs p(95)=423.12µs
     http_req_failed..............................: 0.00%   ✓ 0          ✗ 203862
     http_req_receiving...........................: avg=33.07µs  min=15.14µs  med=32.85µs  max=6.83ms   p(90)=39µs     p(95)=40.79µs 
     http_req_sending.............................: avg=8.34µs   min=5.81µs   med=7.42µs   max=292.15µs p(90)=10.86µs  p(95)=11.67µs 
     http_req_tls_handshaking.....................: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting.............................: avg=334.84µs min=181.76µs med=290.94µs max=14.8ms   p(90)=492.37µs p(95)=508.63µs
     http_reqs....................................: 203862  1698.82372/s
     iteration_duration...........................: avg=583.7µs  min=387.37µs med=535.44µs max=15.33ms  p(90)=743.32µs p(95)=765.72µs
     iterations...................................: 203862  1698.82372/s
     vus..........................................: 1       min=1        max=1   
     vus_max......................................: 2       min=2        max=2   

Copy link
Contributor

github-actions bot commented Dec 22, 2023

🚀 Snapshot Release (rc)

The latest changes of this PR are available as rc on npm (based on the declared changesets):

Package Version Info
@graphql-yoga/plugin-jwt 2.1.1-rc-20231222142638-8f302c70 npm ↗︎ unpkg ↗︎

@peterklingelhofer
Copy link
Contributor

Hi @EmrysMyrddin, amazing work here. I made the onRequest => onRequestParse change to my package over here and I can confirm that the 401 does indeed now bubble up with my Express set up. Thank you so much!

@EmrysMyrddin EmrysMyrddin merged commit b9d2afc into main Dec 22, 2023
27 of 28 checks passed
@EmrysMyrddin EmrysMyrddin deleted the fix-jwt-error branch December 22, 2023 15:16
@EmrysMyrddin
Copy link
Collaborator Author

You're welcome 😃 I'm justing waiting for the CI and will release it 😃 Should be available in a few minutes

@EmrysMyrddin
Copy link
Collaborator Author

Now available in version 2.1.1 :-)

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.

Express integration returns 500 instead of 401 with useJWT() plugin
2 participants