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

chore: proposed refactor for authorizers design #5584

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Dec 30, 2019

  • rename AuthorizerBase to Authorizer. This class should actually have the CfnAuthorizer instantiation, but will only be introduced when an additional authorizer is included.
  • simplify AuthorizerBase dramatically
  • move logic to cache restApiId from AuthorizerBase to TokenAuthorizer. When an additional authorizer is added, we will refactor.
  • remove the usage Authorizer.token. It is non-idiomatic in this context since we support one authorizer reused multiple times.

Elad Ben-Israel added 3 commits December 30, 2019 14:27
- rename `AuthorizerBase` to `Authorizer`. This class should actually have the `CfnAuthorizer` instantiation, but will only be introduced when an additional authorizer is included.
- simplify `AuthorizerBase` dramatically
- move logic to cache `restApiId` from `AuthorizerBase` to `TokenAuthorizer`. When an additional authorizer is added, we will refactor.
- remove the usage `Authorizer.token`. It is non-idiomatic in this context since we support one authorizer reused multiple times.
…zer' into benisrae/apigateway-authorizers-proposal
@eladb eladb requested a review from nija-at as a code owner December 30, 2019 13:34
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Dec 30, 2019
@mergify
Copy link
Contributor

mergify bot commented Dec 30, 2019

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

@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

@eladb eladb changed the title Proposed refactor for authorizers design chore: proposed refactor for authorizers design Dec 30, 2019
@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 merged commit 962039d into nija-at/apigateway-lambdaauthorizer Jan 2, 2020
@nija-at nija-at deleted the benisrae/apigateway-authorizers-proposal branch January 2, 2020 10:07
mergify bot pushed a commit that referenced this pull request Jan 2, 2020
* feat(apigateway): L2 support for lambda token authorizers

* Address PR comments

* More PR feedback

* Restructure binding

* Restructuring classes to allow for Authorizer.token() and Authorizer.iam() experience

* PR feedback

* Authorizer -> Authorization
* drop using Physical Name

* Switch to eslint recommended import style

* chore: proposed refactor for authorizers design (#5584)

* simplify authorizers class design

- rename `AuthorizerBase` to `Authorizer`. This class should actually have the `CfnAuthorizer` instantiation, but will only be introduced when an additional authorizer is included.
- simplify `AuthorizerBase` dramatically
- move logic to cache `restApiId` from `AuthorizerBase` to `TokenAuthorizer`. When an additional authorizer is added, we will refactor.
- remove the usage `Authorizer.token`. It is non-idiomatic in this context since we support one authorizer reused multiple times.

* moved Authorizer to authorizer.ts

* fix broken references and types

Co-authored-by: Niranjan Jayakar <16217941+nija-at@users.noreply.github.com>

* Documentation updates & PR feedback

Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants