Skip to content

Commit

Permalink
set albArn mandatory
Browse files Browse the repository at this point in the history
add unit tests for cache.ts and alb-cache.ts
  • Loading branch information
NicolasViaud committed Feb 4, 2025
1 parent 2c79a0d commit 3de4328
Show file tree
Hide file tree
Showing 6 changed files with 200 additions and 127 deletions.
8 changes: 5 additions & 3 deletions src/alb-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { createPublicKey } from "crypto";
import {
JwkInvalidKtyError,
JwksNotAvailableInCacheError,
JwksValidationError,
JwkValidationError,
JwtBaseError,
JwtWithoutValidKidError,
} from "./error.js";
Expand All @@ -27,7 +29,7 @@ export class AlbUriError extends JwtBaseError {}
*
* Security considerations:
* It's important that the application protected by this library run in a secure environment. This application should be behind the ALB and deployed in a private subnet, or a public subnet but with no access from a untrusted network.
* This security requierement is essential to be respected otherwise the application is exposed to several security risks. This class can be subject to a DoS attack if the attacker can control the kid.
* This security requirement is mandatory otherwise the application is exposed to several security risks like DoS attack by injecting a forged kid.
*
*/
export class AwsAlbJwksCache implements JwksCache {
Expand Down Expand Up @@ -152,10 +154,10 @@ export class AwsAlbJwksCache implements JwksCache {
const jwksUriWithKid = this.expandWithKid(jwksUri, kid);
this.jwkCache.set(jwksUriWithKid, jwkWithKid);
} else {
throw new Error("TODO");
throw new JwkValidationError("JWK does not have a kid");
}
} else {
throw new Error("TODO");
throw new JwksValidationError("Only one JWK is expected in the JWKS");
}
}

Expand Down
46 changes: 20 additions & 26 deletions src/alb-verifier.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AwsAlbJwksCache } from "./alb-cache";
import { assertStringArrayContainsString } from "./assert.js";
import { JwtInvalidClaimError, ParameterValidationError } from "./error.js";
import { AlbJwtInvalidClientIdError, AlbJwtInvalidSignerError, JwtInvalidClaimError, ParameterValidationError } from "./error.js";
import { Jwk, JwksCache } from "./jwk.js";
import { AlbJwtHeader, AlbJwtPayload, JwtHeader } from "./jwt-model.js"; // todo consider creating a specific type for AWS ALB JWT Payload
import { JwtVerifierBase, JwtVerifierProperties } from "./jwt-verifier.js";
Expand Down Expand Up @@ -60,9 +60,8 @@ export type AlbJwtVerifierProperties = {
* Set this to the expected value of the `signer` claim in the JWT (JWT header).
* If you provide a string array, that means at least one of those ALB ARNs
* must be present in the JWT's signer claim.
* Pass null explicitly to not check the JWT's signer--if you know what you're doing
*/
albArn: string | string[] | null;
albArn: string | string[];
} & Partial<AlbVerifyProperties>;

/**
Expand All @@ -86,9 +85,8 @@ export type AlbJwtVerifierMultiProperties = {
* Set this to the expected value of the `signer` claim in the JWT (JWT header).
* If you provide a string array, that means at least one of those ALB ARNs
* must be present in the JWT's signer claim.
* Pass null explicitly to not check the JWT's signer--if you know what you're doing
*/
albArn: string | string[] | null;
albArn: string | string[];
} & AlbVerifyProperties;

/**
Expand All @@ -99,7 +97,7 @@ export type AlbJwtVerifierSingleUserPool<T extends AlbJwtVerifierProperties> =
Properties<AlbVerifyProperties, T>,
T &
JwtVerifierProperties<AlbVerifyProperties> & {
albArn: string | string[] | null;
albArn: string | string[];
audience: null;
},
false
Expand All @@ -114,7 +112,7 @@ export type AlbJwtVerifierMultiUserPool<
Properties<AlbVerifyProperties, T>,
T &
JwtVerifierProperties<AlbVerifyProperties> & {
albArn: string | string[] | null;
albArn: string | string[];
audience: null;
},
true
Expand All @@ -140,7 +138,7 @@ export class AlbJwtVerifier<
SpecificVerifyProperties extends Partial<AlbVerifyProperties>,
IssuerConfig extends JwtVerifierProperties<SpecificVerifyProperties> & {
audience: null;
albArn: string | string[] | null;
albArn: string | string[];
},
MultiIssuer extends boolean,
> extends JwtVerifierBase<SpecificVerifyProperties, IssuerConfig, MultiIssuer> {
Expand Down Expand Up @@ -262,23 +260,21 @@ export function validateAlbJwtFields(
header: JwtHeader,
options: {
clientId?: string | string[] | null;
albArn?: string | string[] | null;
albArn?: string | string[];
}
): void {
// Check ALB ARN (signer)
if (options.albArn !== null) {
if (options.albArn === undefined) {
throw new ParameterValidationError(
"albArn must be provided or set to null explicitly"
);
}
assertStringArrayContainsString(
"ALB ARN",
header.signer,
options.albArn
// todo create new error type
if (options.albArn === undefined) {
throw new ParameterValidationError(
"albArn must be provided"
);
}
assertStringArrayContainsString(
"ALB ARN",
header.signer,
options.albArn,
AlbJwtInvalidSignerError
);
// Check clientId
if (options.clientId !== null) {
if (options.clientId === undefined) {
Expand All @@ -289,16 +285,14 @@ export function validateAlbJwtFields(
assertStringArrayContainsString(
"Client ID",
header.client,
options.clientId
// todo create new error type
options.clientId,
AlbJwtInvalidClientIdError
);
}
}

export function defaultJwksUri(albArn: string | string[] | null): string {
if (!albArn) {
throw new ParameterValidationError("ALB ARN cannot be null");
}
export function defaultJwksUri(albArn: string | string[]): string {

const regionRegex = /^[a-z]{2}-[a-z]+-\d{1}$/;

const extractRegion = (arn: string): string => {
Expand Down
9 changes: 9 additions & 0 deletions src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,15 @@ export class CognitoJwtInvalidTokenUseError extends JwtInvalidClaimError {}

export class CognitoJwtInvalidClientIdError extends JwtInvalidClaimError {}


/**
* Amazon ALB specific erros
*/

export class AlbJwtInvalidSignerError extends JwtInvalidClaimError {}

export class AlbJwtInvalidClientIdError extends JwtInvalidClaimError {}

/**
* JWK errors
*/
Expand Down
97 changes: 97 additions & 0 deletions tests/unit/alb-cache.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import { AwsAlbJwksCache } from "../../src/alb-cache";
import {
JwksValidationError,
JwkValidationError,
JwtWithoutValidKidError,
} from "../../src/error";
import { allowAllRealNetworkTraffic, disallowAllRealNetworkTraffic, generateKeyPair } from "./test-util";

describe("unit tests AwsAlbJwksCache", () => {
const jwksUri = "https://public-keys.auth.elb.eu-west-1.amazonaws.com";
let keypair: ReturnType<typeof generateKeyPair>;
const getDecomposedJwt = (kid?: string) => ({
header: {
alg: "EC256",
kid: kid ?? keypair.jwk.kid ?? "kid",
},
payload: {},
});
const getAlbResponseArrayBuffer = () => {
const encoder = new TextEncoder();
return encoder.encode(keypair.publicKeyPem).buffer;
}
beforeAll(() => {
keypair = generateKeyPair({
kid: "00000000-0000-0000-0000-000000000000",
kty: "EC",
alg: "ES256",
});
disallowAllRealNetworkTraffic();
});
afterAll(() => {
allowAllRealNetworkTraffic();
});

test("ALB JWKS cache error flow: kid empty", () => {
const jwksCache = new AwsAlbJwksCache();
expect.assertions(1);
return expect(
jwksCache.getJwk(jwksUri, { header: { alg: "EC256" }, payload: {} })
).rejects.toThrow(JwtWithoutValidKidError);
});

test("ALB JWKS cache returns cached JWK", () => {
const jwksCache = new AwsAlbJwksCache();
jwksCache.addJwks(jwksUri, keypair.jwks);
expect(jwksCache.getCachedJwk(jwksUri, getDecomposedJwt())).toEqual(
keypair.jwk
);
});

test("ALB JWKS add cache return multiple JWK exception", () => {
const jwksCache = new AwsAlbJwksCache();
expect(()=>jwksCache.addJwks(jwksUri, {
keys: [keypair.jwk, keypair.jwk]
})).toThrow(JwksValidationError);
});

test("ALB JWKS add cache return no kid", () => {
const jwksCache = new AwsAlbJwksCache();
expect(()=>jwksCache.addJwks(jwksUri, {
keys: [{
kty: "EC",
alg: "ES256"
}]
})).toThrow(JwkValidationError);
});

test("ALB JWKS get JWKS return not implemented exception", () => {
const jwksCache = new AwsAlbJwksCache();
expect.assertions(1);
return expect(jwksCache.getJwks()).rejects.toThrow(
"Method not implemented."
);
});

test("ALB JWKS cache fetches URI one attempt at a time", async () => {
/**
* Test what happens when the the JWKS URI is requested multiple times in parallel
* (e.g. in parallel promises). When this happens only 1 actual HTTPS request should
* be made to the JWKS URI.
*/
const fetcher = {
fetch: jest.fn(async () => getAlbResponseArrayBuffer()),
};
const jwksCache = new AwsAlbJwksCache({
fetcher,
});
const promise1 = jwksCache.getJwk(jwksUri, getDecomposedJwt());
const promise2 = jwksCache.getJwk(jwksUri, getDecomposedJwt());
expect.assertions(2);
expect(promise1).toEqual(promise2);
await Promise.all([promise1, promise2]);
expect(fetcher.fetch).toHaveBeenCalledTimes(1);
});

});

102 changes: 4 additions & 98 deletions tests/unit/alb-verifier.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import {
ParameterValidationError,
JwtInvalidClaimError,
JwtInvalidIssuerError,
FailedAssertionError,
AlbJwtInvalidSignerError,
AlbJwtInvalidClientIdError,
} from "../../src/error";
import { createPublicKey } from "crypto";

Expand Down Expand Up @@ -327,87 +328,6 @@ 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";
Expand Down Expand Up @@ -578,7 +498,7 @@ describe("unit tests alb verifier", () => {

expect.assertions(1);
expect(() => albVerifier.verifySync(signedJwt)).toThrow(
FailedAssertionError
AlbJwtInvalidSignerError
);
});

Expand Down Expand Up @@ -622,7 +542,7 @@ describe("unit tests alb verifier", () => {

expect.assertions(1);
expect(() => albVerifier.verifySync(signedJwt)).toThrow(
FailedAssertionError
AlbJwtInvalidClientIdError
);
});
});
Expand Down Expand Up @@ -730,20 +650,6 @@ describe("unit tests alb verifier", () => {
});
});

test("can't extract region when null albArn and undefined jwksUri", async () => {
const region = "us-east-1";
const userPoolId = "us-east-1_123456";
const clientId = "my-client-id";
const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`;

expect(() => {
AlbJwtVerifier.create({
issuer,
clientId,
albArn: null,
});
}).toThrow(ParameterValidationError);
});
});
});

Expand Down
Loading

0 comments on commit 3de4328

Please sign in to comment.