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

fix: resolve issues with graphiql explorer #13555

Closed
wants to merge 7 commits into from
Closed

Conversation

afnx
Copy link
Contributor

@afnx afnx commented Jan 25, 2024

fix: implement a new jwt library (jose) for signing and validating jwts

fix: remove unsupported jsonwebtoken

fix: graphiql explorer implementation

chore: bump package versions

Description of changes

The GraphiQL IDE was not working because of a error appeared on the browser console (shown below). After investigating, I found that the issue was with JWT signing, which was caused by updates to some packages. The original library, jsonwebtoken, wasn't working correctly anymore, causing errors. To fix this, I switched to using the jose library, a well-known alternative. I also made adjustments to the GraphiQL IDE implementation to make it work with the changes.

Uncaught (in promise) Error: Error when generating OIDC token: invalid 'instanceof' operand g

Issue #, if available

aws-amplify/amplify-category-api#1269, aws-amplify/amplify-category-api#1369

Description of how you validated changes

I edited the existing tests to match the new jwt signing implementation.

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)
  • New AWS SDK calls or CloudFormation actions have been added to relevant test and service IAM policies
  • Pull request labels are added

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

fix: implement a new jwt library (jose) for signing and validating jwts

fix: remove unsupported jsonwebtoken

fix: graphiql explorer implementation

chore: bump package versions
@mauerbac
Copy link
Member

mauerbac commented Feb 19, 2024

Hi @afnx - sorry for the delay! I just pinged the team about this. Thanks for putting together the PR!

Copy link
Contributor

@phani-srikar phani-srikar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your contribution is really appreciated. Posted a few comments/questions. Also can you rebase your PR with the base branch? Thanks.

@afnx
Copy link
Contributor Author

afnx commented Feb 22, 2024

@phani-srikar Thank you for reviewing the changes. I replied to your questions and rebased the PR.

phani-srikar
phani-srikar previously approved these changes Feb 26, 2024
@afnx
Copy link
Contributor Author

afnx commented Feb 27, 2024

Hi @phani-srikar @mauerbac can you ping the team for the second review?

@phani-srikar
Copy link
Contributor

There are 2 CI jobs that are failing on the PR:

  • verify_api_extract: In order to fix this can you run yarn extract-api in your repository and commit the updates to any API.md files?
  • test: The unit tests in the package amplify-provider-awscloudformation are failing. Can you run yarn test in that package and fix any issues? Also run yarn test from the root of the repository to make sure it passes.

It might be easier for us to pull these changes in into a new branch in the parent repo and help you fix these things since you don't have visibility into the errors. Let us know.

@dpilch
Copy link
Member

dpilch commented Apr 3, 2024

Hi @afnx, sorry for the delay. I will work on getting the CI checks passing for this change.

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.

4 participants