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.

Sign up for GitHub

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

Jump to bottom

feat(apigateway): lambda request authorizer #5642

Merged
merged 11 commits into from
Feb 24, 2020
Merged

feat(apigateway): lambda request authorizer #5642

merged 11 commits into from
Feb 24, 2020

Conversation

CaerusKaru
Copy link
Contributor

@CaerusKaru CaerusKaru commented Jan 5, 2020 •
edited by nija-at
Loading

Commit Message

feat(apigateway): lambda request authorizer (#5642)

This creates a common LambdaAuthorizer base class so that the
token and request authorizers can share common functionality.

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@nija-at nija-at changed the title feat(apigateway): add lambda request authorizer construct feat(apigateway): lambda request authorizer Jan 14, 2020
nija-at
nija-at previously requested changes Jan 14, 2020
View reviewed changes
Copy link
Contributor

@nija-at nija-at left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Fix the build and branch conflicts. I'd like to check the PR out and give it a whirl.

packages/@aws-cdk/aws-apigateway/lib/authorizers/lambda.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-apigateway/lib/authorizers/lambda.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-apigateway/test/authorizers/test.lambda.ts Outdated Show resolved Hide resolved
nija-at
nija-at reviewed Jan 14, 2020
View reviewed changes
Copy link
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

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

Also, update the README.md with information about request authorizer.

@mergify mergify bot dismissed nija-at’s stale review January 15, 2020 03:53

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@CaerusKaru
Copy link
Contributor Author

@nija-at Not really sure what to do about the build failures, it seems like a JSII issue, not a TS one (for instance, my IDE has no issue with the code as is). I think I've pared down and simplified things as much as possible otherwise.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot dismissed nija-at’s stale review January 17, 2020 02:37

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: c09c842
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@nija-at nija-at mentioned this pull request Feb 20, 2020
Closed
2 tasks
@CaerusKaru
Copy link
Contributor Author

@nija-at should be ready for final review now 👍

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 4d39d16
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

nija-at
nija-at previously requested changes Feb 21, 2020
View reviewed changes
Copy link
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

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

Sorry about the multiple iterations. This is very close...

packages/@aws-cdk/aws-apigateway/lib/authorizers/identity-source.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-apigateway/lib/authorizers/identity-source.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-apigateway/package.json Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-apigateway/lib/authorizers/identity-source.ts Outdated Show resolved Hide resolved
Adam Plumer added 10 commits February 21, 2020 14:12
feat(apigateway): add lambda request authorizer construct
a201af5 
This creates a common LambdaAuthorizer base class so that the
token and request authorizers can share common functionality.
fixup! feat(apigateway): add lambda request authorizer construct
6eae6ca
fixup! feat(apigateway): add lambda request authorizer construct
38834fd
fixup! feat(apigateway): add lambda request authorizer construct
7259c70
fixup! fixup! feat(apigateway): add lambda request authorizer construct
d4872bf
fixup! feat(apigateway): add lambda request authorizer construct
8792b5a
fixup! feat(apigateway): add lambda request authorizer construct
9fc3c58
fixup! feat(apigateway): add lambda request authorizer construct
7dcdd41
fixup! feat(apigateway): add lambda request authorizer construct
af65fd6
fixup! feat(apigateway): add lambda request authorizer construct
ddbf565
@mergify mergify bot dismissed nija-at’s stale review February 21, 2020 19:24

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: ddbf565
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Feb 24, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 6f7f354
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Feb 24, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@mergify mergify bot merged commit 031932d into aws:master Feb 24, 2020
@ahmadiqbal34
Copy link

First of all, thank you @CaerusKaru for the contribution! My team had just realized last week that request authorizers weren't supported yet and were considering contributing ourselves. Needless to say, we were pleasantly surprised seeing you were already working on this.

Secondly, what is the publish cycle for the aws-cdk package? Really looking forward to this addition.

@nija-at
Copy link
Contributor

nija-at commented Feb 24, 2020

Our publish cycle is weekly, so you should likely expect this to be available later this week.

If, for some reason, we can't get it out this week, it will be available next week.

Thanks!

@CaerusKaru
Copy link
Contributor Author

Many thanks to @nija-at for sticking with me on this PR. If only things could be as simple as you think they are when you start 😉

@CaerusKaru CaerusKaru deleted the auth branch February 25, 2020 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RomainMuller RomainMuller RomainMuller left review comments

@nija-at nija-at nija-at approved these changes

@eladb eladb Awaiting requested review from eladb

Assignees

@nija-at nija-at

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@CaerusKaru @aws-cdk-automation @eladb @ahmadiqbal34 @nija-at @RomainMuller