Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
AWS Lambda HTTP Security Integration #17192
AWS Lambda HTTP Security Integration #17192
Changes from all commits
7ca2fc8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Shouldn't this default to true? If lambda is sending a security principal I would assume it is for a reason?
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.
The whole mechanism abstraction adds overhead to every request which will exist if the user isn't using AWS security. I don't know yet if users will always use AWS security, so I'd like to keep the default false.
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 don't really like all these custom principal implementations, given that we have io.quarkus.security.identity.SecurityIdentity#getAttributes() that can be used to store any additional information attached to the identity (this applies to all the different ones, not just here).
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 just don't agree. Things just don't map one to one to an attribute list. JWT has claims and scopes (which is a set). IAM has a bunch of different nested metadata. Also, we already have a specific type for this stuff in the event, doesn't make any sense to pull AWS specific security metadata into a untyped string keyed hashmap. All this metadata is already unmarshalled too and to just fill up a hashmap with this unmarshalled data is double the work.