-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: skip checks on non serverless api resources #6471
Conversation
samcli/commands/deploy/auth_utils.py
Outdated
@@ -101,6 +101,11 @@ def _auth_id(resources_dict, event_properties, identifier): | |||
""" | |||
resource_name = event_properties.get(identifier, "") | |||
api_resource = resources_dict.get(resource_name, {}) | |||
|
|||
# Auth does not apply to ApiGateway::RestApi resources so return true and continue | |||
if api_resource and api_resource["Type"] == "AWS::ApiGateway::RestApi": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this apply to AWS::ApiGatewayV2::Api
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yea good catch. I think it should since if the event refers to an httpApi then it would have to be this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple minor comments, looks good overall!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Which issue(s) does this change fix?
#6310
Why is this change necessary?
if an api event has a ref to a non serverless api resource we still perform checks as if its a serverless api resource.
How does it address the issue?
skips checking for auth on
Aws::ApiGateway::RestApi
resourcesWhat side effects does this change have?
Mandatory Checklist
PRs will only be reviewed after checklist is complete
make pr
passesmake update-reproducible-reqs
if dependencies were changedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.