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

Add API Gateway IAM (AWS_IAM) Authorizers #827

Merged
merged 17 commits into from
Mar 15, 2019

Conversation

horike37
Copy link
Contributor

@horike37 horike37 commented Feb 27, 2019

Issue #802 #781, if available:

Description of changes:
Add API Gateway IAM (AWS_IAM) Authorizers

Description of how you validated changes:
Please verify this PR with following the RFC of #781,

Checklist:

  • Write/update tests
  • make pr passes
  • Update documentation
    Regarding documentation, I didn't update those because I took a look at stuff under doc directory but I was not able to find related to Authorizers features. Ping me if missed it.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@horike37 horike37 marked this pull request as ready for review February 28, 2019 13:27
@codecov-io
Copy link

codecov-io commented Feb 28, 2019

Codecov Report

Merging #827 into develop will increase coverage by 0.02%.
The diff coverage is 95.91%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #827      +/-   ##
===========================================
+ Coverage    94.56%   94.58%   +0.02%     
===========================================
  Files           67       67              
  Lines         2834     2865      +31     
  Branches       508      519      +11     
===========================================
+ Hits          2680     2710      +30     
- Misses          82       83       +1     
  Partials        72       72
Impacted Files Coverage Δ
samtranslator/model/eventsources/push.py 88.77% <100%> (+0.03%) ⬆️
samtranslator/model/api/api_generator.py 96.25% <100%> (+0.08%) ⬆️
samtranslator/model/apigateway.py 97.22% <100%> (+0.89%) ⬆️
samtranslator/swagger/swagger.py 97.46% <90%> (-0.87%) ⬇️

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 b4baac6...c281b55. Read the comment docs.

Copy link
Contributor

@brettstack brettstack left a comment

Choose a reason for hiding this comment

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

This is awesome. Greatly appreciate the PR. A few minor comments; the largest one probably the scenario where Authorizers isn't defined - I don't think that's necessary for AWS_IAM

samtranslator/model/api/api_generator.py Outdated Show resolved Hide resolved
samtranslator/swagger/swagger.py Outdated Show resolved Hide resolved
samtranslator/swagger/swagger.py Outdated Show resolved Hide resolved
@horike37
Copy link
Contributor Author

horike37 commented Mar 1, 2019

Hey @brettstack, thank you for the review 👍
I have updated all of them. Please check it again.

@sthulb sthulb requested a review from brettstack March 1, 2019 11:28
Copy link
Contributor

@brettstack brettstack left a comment

Choose a reason for hiding this comment

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

LGTM! I just had a small request to add a code comment. All that's remaining is to update this doc https://github.com/awslabs/serverless-application-model/blob/master/versions/2016-10-31.md#awsserverlessapi with your new additions and we'll get them merged in. I'm super excited about getting this in and I many others are as well.

samtranslator/swagger/swagger.py Outdated Show resolved Hide resolved
@brettstack
Copy link
Contributor

We'd also appreciate an example using AWS_IAM. If you're not up for it, we'll ask someone else if they'd like to have contribute one. See this example as a reference https://github.com/awslabs/serverless-application-model/tree/master/examples/2016-10-31/api_lambda_token_auth.

@keetonian keetonian self-requested a review March 6, 2019 02:19
@keetonian keetonian changed the base branch from master to develop March 8, 2019 01:30
@keetonian
Copy link
Contributor

Looks like I broke things with my last merge. I'll fix that.

@horike37
Copy link
Contributor Author

horike37 commented Mar 11, 2019

Hey, @brettstack @keetonian. I have fixed the conflicts and added the documentation and source comment.

We'd also appreciate an example using AWS_IAM. If you're not up for it, we'll ask someone else if >they'd like to have contribute one. See this example as a reference >https://github.com/awslabs/serverless-application-model/tree/master/examples/2016-10-31/api_lambda_token_auth.

Sorry, it is difficult for me to spend more time to do it, would be great if you can ask someone else.

Copy link
Contributor

@brettstack brettstack left a comment

Choose a reason for hiding this comment

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

🎉

Copy link
Contributor

@keetonian keetonian left a comment

Choose a reason for hiding this comment

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

This is great! Thank you!

@brettstack brettstack merged commit 972b610 into aws:develop Mar 15, 2019
@horike37 horike37 deleted the add-aws_iam-authorizer branch March 16, 2019 04:20
@horike37
Copy link
Contributor Author

Thanks @brettstack @keetonian 😄
When will this be released? This is needed for for my job.

@brettstack
Copy link
Contributor

Policy says we can't give dates, but I can say we want to get this and other features/fixes out asap.

If you need it now, you can try using SAM CLI and these instructions for using a dev version of SAM https://github.com/awslabs/aws-sam-cli/blob/develop/DEVELOPMENT_GUIDE.md#4-optional-install-development-version-of-sam-transformer. Running sam validate --debug you'll get the transformed template.

@Jun711
Copy link

Jun711 commented Apr 8, 2019

@brettstack @horike37
While waiting for this pull request to be released, would using a securityDefinitions work? thanks

securityDefinitions: 
   AwsIamAuthorizer:
      type: "apiKey"
      name: "Authorization"
      in: "header"
      x-amazon-apigateway-authtype: "awsSigv4"

and, under my AWS::Serverless::Api path method, I add a security item that points to the securityDefinition.

 security: 
 - AwsIamAuthorizer: []

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.

None yet

5 participants