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 18 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add region check
add unit test for alb-verifier.ts (reach 100% coverage)
  • Loading branch information
NicolasViaud committed Jan 28, 2025
commit 2c79a0d5c89f46852e812a4870283d27a348364f
17 changes: 11 additions & 6 deletions src/alb-verifier.ts
Original file line number Diff line number Diff line change
@@ -213,8 +213,8 @@ export class AlbJwtVerifier<
): AlbJwtPayload {
const { decomposedJwt, jwksUri, verifyProperties } =
this.getVerifyParameters(jwt, properties);
this.verifyDecomposedJwtSync(decomposedJwt, jwksUri, verifyProperties);
try {
this.verifyDecomposedJwtSync(decomposedJwt, jwksUri, verifyProperties);
validateAlbJwtFields(decomposedJwt.header, verifyProperties);
} catch (err) {
if (
@@ -242,8 +242,8 @@ export class AlbJwtVerifier<
): Promise<AlbJwtPayload> {
const { decomposedJwt, jwksUri, verifyProperties } =
this.getVerifyParameters(jwt, properties);
await this.verifyDecomposedJwt(decomposedJwt, jwksUri, verifyProperties);
try {
await this.verifyDecomposedJwt(decomposedJwt, jwksUri, verifyProperties);
validateAlbJwtFields(decomposedJwt.header, verifyProperties);
} catch (err) {
if (
@@ -295,17 +295,22 @@ export function validateAlbJwtFields(
}
}

function defaultJwksUri(albArn: string | string[] | null): string {
if (albArn === null) {
export function defaultJwksUri(albArn: string | string[] | null): string {
if (!albArn) {
throw new ParameterValidationError("ALB ARN cannot be null");
}
const regionRegex = /^[a-z]{2}-[a-z]+-\d{1}$/;

const extractRegion = (arn: string): string => {
const arnParts = arn.split(":");
if (arnParts.length < 4) {
if (arnParts.length < 4 || arnParts[0] !== "arn" || arnParts[1] !== "aws" || arnParts[2] !== "elasticloadbalancing") {
throw new ParameterValidationError(`Invalid load balancer ARN: ${arn}`);
}
return arnParts[3];
const region = arnParts[3];
if (!regionRegex.test(region)) {
throw new ParameterValidationError(`Invalid AWS region in ARN: ${region}`);
}
return region;
};

if (Array.isArray(albArn)) {
152 changes: 151 additions & 1 deletion tests/unit/alb-verifier.test.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

This file has a lot of lines :)

Are all these tests necessary to reach 100% coverage? Maybe they are

Copy link
Author

@NicolasViaud NicolasViaud Feb 10, 2025

Choose a reason for hiding this comment

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

Yes, indeed :)
There is probably more than 100% coverage because I try to test more real use cases rather than wanted to do just the 100% lines coverage.
I can remove some unit test in order to reach just the 100% coverage, if you think that it will require too much effort to maintain them in the future. Remove test is definitely something simpler than create :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't looked closely at this file yet, the size of it was kinda holding me off haha. Will take a closer look and have a more informed opinion.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I totally understand and sorry about that.
Most of the tests are inspired from other verifiers, even in the name and in the structure, so it should be familiar.
I add also some test, and trying the same structure aswell (hierarchy of describe function)

Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@ import {
} from "./test-util";
import { decomposeUnverifiedJwt } from "../../src/jwt";
import { JwksCache, Jwks } from "../../src/jwk";
import { AlbJwtVerifier } from "../../src/alb-verifier";
import { AlbJwtVerifier, defaultJwksUri } from "../../src/alb-verifier";
import {
ParameterValidationError,
JwtInvalidClaimError,
@@ -327,6 +327,87 @@ describe("unit tests alb verifier", () => {
).toMatchObject({ hello: "world" });
});

test("albArn null", () => {
const kid = keypair.jwk.kid;
const userPoolId = "us-east-1_123456";
const issuer = `https://cognito-idp.us-east-1.amazonaws.com/${userPoolId}`;
const albArn =
"arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188";
const clientId = "my-client-id";
const exp = 4000000000; // nock and jest.useFakeTimers do not work well together. Used of a long expired date instead

const signedJwt = signJwt(
{
typ: "JWT",
kid,
alg: "ES256",
iss: issuer,
client: clientId,
signer: albArn,
exp,
},
{
hello: "world",
exp,
iss: issuer,
},
keypair.privateKey
);
const verifier = AlbJwtVerifier.create({
albArn: null,
issuer,
clientId,
});
verifier.cacheJwks(keypair.jwks);

expect.assertions(1);
expect(verifier.verifySync(signedJwt)).toMatchObject({
hello: "world",
});
});

test("albArn undefined", () => {
const kid = keypair.jwk.kid;
const userPoolId = "us-east-1_123456";
const issuer = `https://cognito-idp.us-east-1.amazonaws.com/${userPoolId}`;
const albArn =
"arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188";
const clientId = "my-client-id";
const exp = 4000000000; // nock and jest.useFakeTimers do not work well together. Used of a long expired date instead

const signedJwt = signJwt(
{
typ: "JWT",
kid,
alg: "ES256",
iss: issuer,
client: clientId,
signer: albArn,
exp,
},
{
hello: "world",
exp,
iss: issuer,
},
keypair.privateKey
);
const verifier = AlbJwtVerifier.create({
albArn: undefined as unknown as null,
issuer,
clientId,
});
verifier.cacheJwks(keypair.jwks);

expect.assertions(2);
expect(() => verifier.verifySync(signedJwt)).toThrow(
"AlbArn must be provided or set to null explicitly"
);
expect(() => verifier.verifySync(signedJwt)).toThrow(
ParameterValidationError
);
});

test("clientId null", async () => {
const region = "us-east-1";
const userPoolId = "us-east-1_123456";
@@ -894,4 +975,73 @@ describe("unit tests alb verifier", () => {
});
});
});

describe("defaultJwksUri", () => {
test("happy flow with us-east-1 region", () => {
const albArn =
"arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188";

expect(defaultJwksUri(albArn)).toBe("https://public-keys.auth.elb.us-east-1.amazonaws.com");
});

test("happy flow with eu-west-2 region", () => {
const albArn =
"arn:aws:elasticloadbalancing:eu-west-2:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188";

expect(defaultJwksUri(albArn)).toBe("https://public-keys.auth.elb.eu-west-2.amazonaws.com");
});

test("happy flow with multi ALB ARN", () => {
const albArns = [
"arn:aws:elasticloadbalancing:eu-west-2:123456789012:loadbalancer/app/my-load-balancer-1/50dc6c495c0c9188",
"arn:aws:elasticloadbalancing:eu-west-2:123456789012:loadbalancer/app/my-load-balancer-2/901e7c495c0c9188",
]

expect(defaultJwksUri(albArns)).toBe("https://public-keys.auth.elb.eu-west-2.amazonaws.com");
});

test("invalid load balancer ARN - too short", () => {
const albArn =
"arn:aws:elasticloadbalancing";

expect(()=>defaultJwksUri(albArn)).toThrow(new ParameterValidationError(`Invalid load balancer ARN: arn:aws:elasticloadbalancing`));
});

test("invalid load balancer ARN - invalid region 1", () => {
const albArn =
"arn:aws:elasticloadbalancing:.:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188";

expect(()=>defaultJwksUri(albArn)).toThrow(new ParameterValidationError(`Invalid AWS region in ARN: .`));
});

test("invalid load balancer ARN - invalid region 2", () => {
const albArn =
"arn:aws:elasticloadbalancing:/:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188";

expect(()=>defaultJwksUri(albArn)).toThrow(new ParameterValidationError(`Invalid AWS region in ARN: /`));
});

test("invalid load balancer ARN - invalid region 3", () => {
const albArn =
"arn:aws:elasticloadbalancing:?:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188";

expect(()=>defaultJwksUri(albArn)).toThrow(new ParameterValidationError(`Invalid AWS region in ARN: ?`));
});

test("invalid load balancer ARN - invalid region 4", () => {
const albArn =
"arn:aws:elasticloadbalancing:=:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188";

expect(()=>defaultJwksUri(albArn)).toThrow(new ParameterValidationError((`Invalid AWS region in ARN: =`)));
});

test("invalid load balancer ARN with multiple regions", () => {
const albArns = [
"arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer-1/50dc6c495c0c9188",
"arn:aws:elasticloadbalancing:eu-west-2:123456789012:loadbalancer/app/my-load-balancer-2/901e7c495c0c9188",
]

expect(()=>defaultJwksUri(albArns)).toThrow(new ParameterValidationError("Multiple regions found in ALB ARNs"));
});
});
});