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

Alb jwt verifier #190

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

NicolasViaud
Copy link

@NicolasViaud NicolasViaud commented Jan 14, 2025

#109

Description of changes:
Proposition for an AwsAlbJwksCache implementation.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ottokruse
Copy link
Contributor

🎉 great work @NicolasViaud ! And super important.

Will review incrementally, likely over the next few days.

For now, can you run npx prettier -w . to apply the repo's code formatting scheme? Thanks!

@ottokruse
Copy link
Contributor

And maybe to be sure npx eslint --fix src while at it

const regions = albArn.map(extractRegion);
const uniqueRegions = Array.from(new Set(regions));
if (uniqueRegions.length > 1) {
throw new ParameterValidationError("Multiple regions found in ALB ARNs");
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch hadn’t considered this yet! Side effect of allowing multiple ALB ARNs in one configuration is that you have to guard against this now.
I think it’s not unreasonable to throw this error. It’s probably very niche to create a verifier for multiple ALBs in different regions, I can’t see why you would ever want that.
What's your take?

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.

2 participants