-
Notifications
You must be signed in to change notification settings - Fork 31
Add support for multi-resource statements #52
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
Conversation
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.
Thank you for having added this. I just suggested one small change
|
||
public init(action: String, effect: Effect, resource: String) { | ||
self.action = [action] |
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.
I would call the other init() method to avoid duplicating code
Hi @sebsto I have ran the soundness.sh script but apparently it changed a few files that weren't supposed to be edited in this PR. You want me to revert the last commit? |
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.
Thank you for the proposed changes.
I would keep only the change in the file Sources/AWSLambdaEvents/APIGatewayLambdaAuthorizers.swift
Can you revert the other changes made by Swift Format ? I have another PR in progress that takes care of these.
Thanks
@swift-server-bot test this please |
1 similar comment
@swift-server-bot test this please |
@swift-server-bot add to allowlist |
@yim-lee can you help and give me permanent authorisation to trigger a test on any PR on this project and on Lambda runtime project ? Thank you |
@Buratti Can you also pull and merge the changes from the main branch ? I just merged a PR that has non-conflicting changes on the LambdaAuthorizer.swift file. Thank you and sorry for the trouble |
Hi @sebsto! Sure! I'll push the changes in a few hours. |
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
@Buratti one more little soundness fix |
Hi @sebsto! I'm sorry, forgot to run it again. It should be good now! |
Merged. Thank you ! |
Adds support for multiple actions and resources in the same Policy's Statement
Motivation:
It is common practice to return a statement that describes which integrations are available to the authorized principal. Such statement usually allows the
"execute-api:Invoke"
action on a list of integrations ARN.The current design of the
APIGatewayLambdaAuthorizerPolicyResponse
didn't allow that and instead forces the user to generate a long list of statements, each allowing access to just one integration.Modifications:
action
andresource
are now arrays, a new initializer has been added which takes arrays instead of strings. The existing initializer has been changed to automatically initialize a one-element array.A new test has been added to validate the correct encoding and decoding.