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(api-graphql): server side iam auth mode is ineffective #12992

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

HuiSF
Copy link
Contributor

@HuiSF HuiSF commented Feb 9, 2024

Description of changes

  1. Changed the api-rest exported internal post function for api-graphql to make it server side usable (can inject the instance of AmplifyClassV6 from the server context)
  2. Changed the api-graphql server graphql method wrapper implementation to ensure it's an await -able function so Amplify server context can ensure destroying context only after this function has completed
  3. Updated test suites

Issue #, if available

#12984
#12931
#12867

Description of how you validated changes

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)

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

@HuiSF HuiSF force-pushed the hui/fix/api-graphql/server-side-iam branch from 05a2f3a to c99632c Compare February 9, 2024 17:21
@HuiSF HuiSF changed the title fix(api-graphql): server side iam auth mode is not effective fix(api-graphql): server side iam auth mode is ineffective Feb 9, 2024
packages/api-graphql/__tests__/GraphQLAPI.test.ts Dismissed Show dismissed Hide dismissed
packages/api-graphql/__tests__/GraphQLAPI.test.ts Dismissed Show dismissed Hide dismissed
packages/api-graphql/__tests__/GraphQLAPI.test.ts Dismissed Show dismissed Hide dismissed
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AllanZhengYP Please review this, ty!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! What's the difference of this change v.s. using the server version of internal post? https://github.com/aws-amplify/amplify-js/blob/hui/fix/api-graphql/server-side-iam/packages/api-rest/src/internals/server.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GQL impl. doesn't pick the client vs server versions, it just passing the Amplify instances down into the post API.

@HuiSF HuiSF force-pushed the hui/fix/api-graphql/server-side-iam branch from c99632c to 9870bb5 Compare February 12, 2024 18:06
@HuiSF HuiSF force-pushed the hui/fix/api-graphql/server-side-iam branch from 9870bb5 to 0b189ca Compare February 12, 2024 20:34
@HuiSF HuiSF marked this pull request as ready for review February 12, 2024 20:35
@HuiSF HuiSF requested review from a team as code owners February 12, 2024 20:35
Copy link
Contributor

@AllanZhengYP AllanZhengYP left a comment

Choose a reason for hiding this comment

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

The internal POST handler part of change looks good to me! Thank you @HuiSF

@HuiSF HuiSF merged commit e316a2e into main Feb 13, 2024
30 checks passed
@HuiSF HuiSF deleted the hui/fix/api-graphql/server-side-iam branch February 13, 2024 17:57
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.

None yet

4 participants