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: Added support for Authorizers #64

Merged
merged 7 commits into from
Mar 25, 2022

Conversation

bobbydeveaux
Copy link
Contributor

Description

Adding a new section Authorizers which allows anyone to create multiple authorisers of varying types.

Motivation and Context

We needed to add a JWT and OIDC Auth to our project API Gateway

How Has This Been Tested?

Deployed into our own sandbox account and working great.

@bobbydeveaux bobbydeveaux changed the title Adding Authorizers with Ability to add Implicit JWT plus Proxy requests and Examples feat: Adding Authorizers with Ability to add Implicit JWT plus Proxy requests and Examples Mar 23, 2022
Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Looks pretty good.

Please update an example in examples/complete-http to use the feature you are adding in this PR.

README.md Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Outdated
dynamic "jwt_configuration" {
for_each = lookup(each.value, "audience", null) == null ? tolist([]) : tolist([lookup(each.value, "audience", null)])
content {
audience = [lookup(each.value, "audience", null)]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
audience = [lookup(each.value, "audience", null)]
audience = jwt_configuration.value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH - Both sub-elements (issuer and audience are both optional - the for_each should be based on the either of them being present, but not necessarily both...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made this change assuming that audience and issuer would both be present. But it's possible to supply just issuer.

Copy link
Member

Choose a reason for hiding this comment

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

Then let's change for_each on line 193 to handle that at least one value (audience or issuer) should be present.

bobbydeveaux and others added 5 commits March 23, 2022 10:17
Co-authored-by: Anton Babenko <anton@antonbabenko.com>
Co-authored-by: Anton Babenko <anton@antonbabenko.com>
Co-authored-by: Anton Babenko <anton@antonbabenko.com>
@antonbabenko antonbabenko changed the title feat: Adding Authorizers with Ability to add Implicit JWT plus Proxy requests and Examples feat: Added support for Authorizers Mar 25, 2022
@antonbabenko antonbabenko merged commit 5cd32e0 into terraform-aws-modules:master Mar 25, 2022
antonbabenko pushed a commit that referenced this pull request Mar 25, 2022
## [1.6.0](v1.5.1...v1.6.0) (2022-03-25)

### Features

* Added support for Authorizers ([#64](#64)) ([5cd32e0](5cd32e0))
@antonbabenko
Copy link
Member

Thank you @bobbydeveaux for the contribution!

I have updated the code in examples to make it executable.

@antonbabenko
Copy link
Member

This PR is included in version 1.6.0 🎉

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants