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

feat(AWS HTTP API): Add support for AWS IAM authorization #9195

Merged
merged 1 commit into from Mar 30, 2021

Conversation

pgrzesik
Copy link
Contributor

Add support for AWS_IAM authorization for httpApi

Closes: #8210

@codecov
Copy link

codecov bot commented Mar 29, 2021

Codecov Report

Merging #9195 (ec9d00f) into master (6674660) will decrease coverage by 0.26%.
The diff coverage is 70.89%.

❗ Current head ec9d00f differs from pull request most recent head e2826ee. Consider uploading reports for the commit e2826ee to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9195      +/-   ##
==========================================
- Coverage   87.24%   86.97%   -0.27%     
==========================================
  Files         310      315       +5     
  Lines       11485    11665     +180     
==========================================
+ Hits        10020    10146     +126     
- Misses       1465     1519      +54     
Impacted Files Coverage Δ
lib/configSchema.js 100.00% <ø> (ø)
...n/variables/eventually-report-resolution-errors.js 100.00% <ø> (ø)
lib/configuration/variables/sources/file.js 100.00% <ø> (ø)
scripts/serverless.js 50.67% <31.76%> (-13.40%) ⬇️
...on/variables/sources/instance-dependent/get-ssm.js 92.50% <92.50%> (ø)
...ion/variables/sources/instance-dependent/get-cf.js 95.65% <95.65%> (ø)
lib/classes/PluginManager.js 97.49% <100.00%> (+<0.01%) ⬆️
...ation/variables/resolve-unresolved-source-types.js 100.00% <100.00%> (ø)
...ion/variables/sources/instance-dependent/get-s3.js 100.00% <100.00%> (ø)
...on/variables/sources/instance-dependent/get-sls.js 100.00% <100.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6674660...e2826ee. Read the comment docs.

@pgrzesik pgrzesik requested a review from medikoo March 29, 2021 15:56
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

@pgrzesik looks great! I have just one doubt towards configuration schema setup, please see my comment

type: { const: 'aws_iam' },
},
additionalProperties: false,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Configuring definition like that is slightly problematic as it'll trigger vague mode for validation of other authorizers, as now any validation failure will now be reported simply with "Unsupported configuration format" without providing a hint which exactly property is misconfigured.

It's due to fact that two versions for object variant are provided, and validator won't know errors of which to report, hence it'll report just message that something is wrong.

(you may also confirm on that manually).

I think best way to recover is to use one object notation, and then inline ensure that for AWS IAM case it's just type property that provided, and throw meaningful error if any other properties are found

Copy link
Contributor Author

@pgrzesik pgrzesik Mar 30, 2021

Choose a reason for hiding this comment

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

I've adjusted the schema so it should be able to accept all possible versions + added additional validation to ensure that we're working with valid configuration.

@pgrzesik pgrzesik force-pushed the add-support-for-http-api-iam-authorizer branch from 96bf7db to e2826ee Compare March 30, 2021 08:43
@pgrzesik pgrzesik requested a review from medikoo March 30, 2021 09:08
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

Looks great 👍

@pgrzesik pgrzesik merged commit d3c6e43 into master Mar 30, 2021
@pgrzesik pgrzesik deleted the add-support-for-http-api-iam-authorizer branch March 30, 2021 09:34
@cgguevara
Copy link

is there an example about how to use a external lambda authorizer in httpApi ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS HTTP API: Support IAM and Lambda authorizers
3 participants