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

Allow x-api-key for public rest api #13422

Merged
merged 13 commits into from
Aug 22, 2024

Conversation

ashika112
Copy link
Member

@ashika112 ashika112 commented May 23, 2024

Description of changes

We should check only custom authorization to override the IAM auth. This bring v6 in parity with v5 feature.

V5 Ref:

if (typeof params.headers['Authorization'] !== 'undefined') {

Issue #, if available

#13310
#13378

Description of how you validated changes

  • Manual testing
  • Unit tests

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.

@pjmendozaa
Copy link

Please aws-team I have the problem, I see that is release that problem with x-api-key will be resolved.

@ashika112 ashika112 marked this pull request as ready for review June 11, 2024 23:29
@ashika112 ashika112 requested review from a team as code owners June 11, 2024 23:29
@ashika112 ashika112 changed the title refactor iamAuthApplicable Allow x-api-key for public rest api Jun 11, 2024
Copy link
Member

@HuiSF HuiSF left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!
Added some nit-picking and sanity check comments.

packages/api-rest/src/utils/iamAuthApplicable.ts Outdated Show resolved Hide resolved
packages/api-rest/src/utils/iamAuthApplicable.ts Outdated Show resolved Hide resolved
packages/api-rest/src/apis/common/publicApis.ts Outdated Show resolved Hide resolved
Copy link
Member

@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.

Thanks a lot for the fix! @ashika112 I only have a few questions.

@ashika112 ashika112 merged commit 13cf744 into aws-amplify:main Aug 22, 2024
30 checks passed
@ashika112 ashika112 deleted the fix/rest-api-key branch August 22, 2024 18:23
@gerrytsui
Copy link
Contributor

THANK YOU.
Version 6.5.3 is working without requiring our override of the x-api-key condition in /node_modules/@aws-amplify/api-rest/src/apis/common/handler.js

... just like it was BEFORE Jan of 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.

6 participants